Repository navigation
perf(staged): keep gh and git child processes off the tokio runtime - #1013
Merged
Merged
Conversation
Async Tauri commands and web `dispatch` arms ran gh/git/child processes inline on the shared tokio runtime. A worker parked in a synchronous child wait stops driving the timer and I/O drivers, so one slow `gh` call could freeze every other command, event and web request. Move each of those calls to the blocking pool, following the existing `get_pr_for_repo` / `blocking_store_json` precedent: - github_commands: the 11 gh-backed commands and `check_existing_local_branch` run their helper under `spawn_blocking`; the matching web arms go through a new `blocking_json` helper (which `blocking_store_json` now delegates to). - diff_commands: `get_diff_files`, `get_file_diff`, `get_file_at_ref` (Tauri and web). - util_commands: `get_available_openers` (mdfind + icon threads) and `open_in_app` move their bodies into sync helpers run on the pool. - branches: `setup_worktree` reuses `setup_worktree_sync` under `spawn_blocking` via a shared `setup_worktree_impl` (same `BranchWithWorkdir` return and error strings; it now also emits `worktree-setup-progress` and returns the branch re-read after setup, so a de-duplicated local name is visible). `setup_worktree_from_pr` and `create_remote_branch` get shared `_impl`s that run their unchanged bodies on the pool, and the web arms call them instead of duplicating them. `rename_branch`'s local rename runs on the pool too. - project_commands: `add_project_repo_impl` resolves the default branch on the pool before its first store write, so no await falls between writes (the web transport drops a request future at its next await). - Sweep finds beyond the original list: web `pin_repo`, `detect_default_branch`, `is_sq_available` and `create_project`'s default branch lookup; the `gh auth token` / origin lookups in the async GitHub REST helpers; `find_sq_binary` in `run_branch_action_impl`; and `update_clone_to_remote_head` in `detect_actions_for_repo_context`. The web `setup_worktree_and_run_prerun` arm is detached so a dropped request cannot skip the prerun spawn after the worktree is created. Add a source-scan guard test that walks every `#[tauri::command] async fn` and `dispatch` arm (plus async helpers they call, one level down), strips offloaded closures, and fails naming the offending command or arm and the call it found. The sync `create_project` / `create_branch` main-thread paths and a `tokio::process` rewrite of `spawn_gh_with_timeout` are left for later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…den the guard Address the review of 35854bf: - Clone fallback: `ensure_local_clone_with_progress` now applies the captured login-shell env to its HTTPS `git clone` fallback for every caller (via `git::cli::configure_captured`), matching `ensure_local_clone`. All callers go through `setup_worktree_sync`, which only runs on the blocking pool. - Clone errors: streamed stderr keeps only `\n`-terminated lines, drops `\r` progress updates, and caps the error text at the last 20 lines, so the actual git error line survives a long progress stream. - `start_project_session` (Tauri and web) builds the project context, which reads the commit log through git, on the blocking pool. - `git config --global` for the session git identity is read off the runtime in `prepare_resume`, `prepare_branch_session_start` and `start_queued_session_for_branch`, before any claim, so no await falls between the claim and the launch. - Agent discovery (login-shell `command -v`) runs on the blocking pool for branch/review provider resolution, action providers, short-name generation and the web `queue_branch_session` arm. - `add_project_repo_impl` resolves the default branch before reading the project's repos, so a parallel add can't race the branch name. The source-level guard moves to `web_server/blocking_guard_tests.rs` and now indexes every fn by file and name, follows imports, awaited async helpers (including `git::`/`blox::`) and sync helpers up to 12 calls deep, and skips plain method calls such as store reads. Each allow-list entry carries its reason. It catches the `start_project_session` → `get_full_commit_log` and `launch_resume` → `git config --global` misses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
An ALLOWED entry exempts a call for a whole fn, so one that no longer
matches would keep suppressing that call there if it came back on a
path that does block. `Crate` now records every entry a scan matches,
and the guard fails after the walk on any it never matched, naming the
file, fn and call, like the `unscanned` check does for commands.
Checked by temporarily adding a bogus `("prs.rs", "no_such_fn",
"Command::new(")` entry: the test failed with
"prs.rs: `Command::new(` in `no_such_fn`", then passed once reverted.
Also correct the `extra_env` comment in `prepare_resume`: only
`resume_session_for_store` has no await after its claim. The continue
path claims first, so the git identity read is one more await after
the claim there, like `branch_head_sha`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Async Tauri commands and web
dispatcharms were runninggh,git, and other child processes directly on the shared tokio runtime. While a worker sits in a synchronous child wait, it stops driving the timer and I/O drivers. One slowghcall could freeze every other command, event, and web request. This PR moves those calls onto the blocking pool, using the same approach as the existingget_pr_for_repo/blocking_store_jsoncode.Calls moved off the runtime
check_existing_local_branchnow run underspawn_blocking. Their web arms use a newblocking_jsonhelper, andblocking_store_jsonnow delegates to it. Thegh auth tokenand origin lookups in the async GitHub REST helpers moved too.get_diff_files,get_file_diff,get_file_at_ref(Tauri and web).get_available_openersandopen_in_app.setup_worktree,setup_worktree_from_pr,create_remote_branchandrename_branchnow share_impls that run on the pool, so the web arms call these instead of keeping their own copies.setup_worktreenow also emitsworktree-setup-progressand returns the branch re-read after setup. The websetup_worktree_and_run_prerunarm is detached, so a dropped request can't skip the prerun step after the worktree is created.add_project_repo_implresolves the default branch before its first store write.start_project_sessionbuilds the project context (which reads the git commit log) on the pool. The session git identity (git config --global) is read off the runtime before the claim inprepare_resume,prepare_branch_session_startandstart_queued_session_for_branch. Agent discovery (login-shellcommand -v) runs on the pool.pin_repo,detect_default_branch,is_sq_available,create_project's default branch lookup,find_sq_binaryinrun_branch_action_impl, andupdate_clone_to_remote_headindetect_actions_for_repo_context.Clone fallback
ensure_local_clone_with_progressnow applies the captured login-shell env to its HTTPSgit clonefallback, the same wayensure_local_clonedoes.\rprogress updates, and is capped at the last 20 lines, so the real git error isn't buried under progress output.Blocking guard test
web_server/blocking_guard_tests.rsscans the source of every#[tauri::command] async fnand everydispatcharm, and fails if it finds a blocking child-process call that isn't offloaded. It indexes fns by file and name, follows imports and async helpers (includinggit::andblox::), and follows sync helpers up to 12 calls deep. EachALLOWEDexemption records its reason, and the test also fails on any exemption that no longer matches a call.Out of scope
The sync
create_project/create_branchmain-thread paths, and rewritingspawn_gh_with_timeoutontokio::process, are left for later.Test plan
ALLOWEDentry and passes once it's removed🤖 Generated with Claude Code