Skip to content

fix(git): reduce queries and preserve data across execution hosts - #24602

Merged
nwparker merged 7 commits into
mainfrom
nwparker/git-query-efficiency
Oct 3, 2026
Merged

nwparker merged 7 commits into
mainfrom
nwparker/git-query-efficiency

Conversation

@nwparker

@nwparker nwparker commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 101 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4540 $\color{#cf222e}{\Huge{\mathbf{−}}}$​804 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3736
Prod 97 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2240 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1617 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​623

ELI5

Opening a repository, comparing branches, and staging many files now launch fewer Git processes. Native and SSH work also preserve staged data, configured SSH identities, and worktree-owned history when operations fail or are canceled.

What Changed

  • Combine change names and line counts into one NUL-delimited query; share the parser across native and remote comparisons. Batch repository discovery, reuse one configuration snapshot per operation, and resolve default branches in one bounded query.
  • Stage, unstage, and restore tracked files with one literal path stream. Unstaged discard restores the staged version; rename unstage handles both names, including repositories without a first commit. Empty staged blobs remain empty, and corrupt reads cannot masquerade as deletion or enter the settled cache.
  • Reuse the existing process runner, admission scheduler, and read leases on the SSH host. Preserve binary bytes, reject truncated parsed output, propagate cancellation, and retain process capacity until the child really exits. Read commands have the existing two-minute deadline; supported older Node hosts retain a conservative capacity.
  • Preserve configured SSH commands and push destinations. Refresh missing SSH creation bases through the existing host-owned fetch operation. Stop implicit forced submodule removal, recover old-Git worktree locks on the owning host, protect detached rebase/bisect branches, and reject portable Unicode branch-name collisions.
  • Make machine-readable history ignore signature/color settings and keep status line-count reads from refreshing the index. Preserve repository paths containing whitespace/newlines with a rare ambiguous-output fallback.

Why

These changes remove repeated subprocess work at its shared execution boundaries. Existing helpers are extended or moved into shared modules rather than adding another runner, scheduler, cache, or RPC. Correctness checks remain where they protect a fresh snapshot or retained data; measured safety costs are included below.

Most added lines are regression tests and fixtures; module moves account for much of the apparent production churn. The final clone and WSL repairs add a net 34 production lines across six existing modules. There is no new reliability-registry entry; executable regression tests and CI contracts cover the behavior.

Linked Issue

Follow-up to #24346 (merged). Requested application-wide Git performance and safety audit; no new tracking issue.

Visual Proof

Real Source Control controls in a hidden, isolated macOS Electron app. Discarding unstaged edits retains the staged version; unstaging a rename removes both names from the index and preserves the new file. Git assertions verified file/index/HEAD contents. Native window visibility/focus checks recorded zero activation calls.

Before discard After discard
Before discard After discard

After rename unstage:

After rename unstage

Benchmarks

Actual Git 2.55.0 on macOS, paired before/after samples with warmup on the same fixtures. Other validation was running, so wall times describe this host rather than a universal guarantee. Counts are actual subprocesses for the stated operation. The comparison baseline's two queries already ran concurrently.

Operation / fixture Samples Git processes before → after Median ms before → after
Normal repository registration 20 pairs 4 → 2 51.68 → 23.82
Unpublished upstream status resolution 30 pairs 11 → 4 62.55 → 38.83
Unpublished pull resolution 30 pairs 6 → 4 42.81 → 34.51
Configured push resolution 30 pairs 6 → 2 38.65 → 16.95
Default base, 10,000 unrelated packed tags (shipping query) 30 pairs 5 → 1 31.74 → 6.68
Compare 1,200 changed files 20 pairs 2 → 1 30.07 → 25.75
Stage 201 paths 20 3 → 1 63.69 → 22.74
Unstage 201 paths 20 3 → 1 35.23 → 31.97
Restore 201 tracked paths, mutation only 20 3 → 1 75.14 → 54.41
Relay commit metadata 20 2 → 1 18.98 → 8.64
Relay empty-index blob baseline 30 2 → 1 12.50 → 5.91

The 1,200-file comparison reduces median child CPU from 30.94 to 19.66ms; p95 wall time is 37.80 → 33.45ms. Combined output grows 55,800 → 93,000 bytes, within the existing bound. Parallel non-root blob reads keep two processes and saved roughly 30–43ms across three real project blobs; root commits only read one side.

Final clone repair: a controlled wrapper around an actual local Git clone emits 12 MiB of progress after Git succeeds. The pre-audit path succeeds; the intermediate capped path incorrectly rejects and stops progress at 42%; the repaired path succeeds with matching cloned HEAD, 100% progress, final diagnostics, and no live child after settlement. Clone capture retains 4 KiB per stream instead of materializing up to 10 MiB. For the actual capture sinks (twelve preallocated 1 MiB chunks, final buffer materialization, 30 alternating pairs after five warmups), median bookkeeping wall time is 0.293→0.015ms, CPU 0.342→0.017ms, and captured bytes 10,485,760→4,096. This is a capture microbenchmark, not clone or network latency. Parsed Git output still terminates and rejects on overflow.

The restored relay retry-expiry timer adds no Git commands. For 4,096 immediately completed leases in 32 batches of 128 (30 alternating pairs after five warmups), actual shared-class median wall time is 0.91→1.38ms and CPU 0.95→1.42ms. This is lease bookkeeping, not network or UI latency.

Safety costs are deliberate and measured:

Safety behavior Measurement Before → after
Host admission, 20 simultaneous status requests / 1,200 files 12 alternating pairs Peak children 20 → 4; batch median 129.25 → 155.88ms
Configured SSH identity preflight, local exact-ref fetch 40 pairs Processes 1 → 2; median 38.73 → 47.23ms
Same SSH preflight with one simulated 50ms operation RPC 30 pairs Median 93.58 → 99.81ms; RPC count remains one
Portable collision guard, 8,001 heads / 216KB, core.ignoreCase=false 20 pairs Processes 3 → 3; Git median 29.63 → 31.68ms, plus 4.87ms median JS validation/folding
Same collision guard, core.ignoreCase=true 20 pairs Processes 4 → 3; Git median 37.24 → 31.85ms, plus the same JS work

Configuration savings do not imply fewer SSH round trips: both implementations execute all Git subprocesses inside one operation RPC. The host-admission benchmark measures batch completion, not UI latency. Unchanged discard classification/cleanup reads are excluded from the tracked restore row.

Fresh localized proof after the catalog repair: the French dialogs use the current English fallback; Discard (MM→M) and Restore (MD→M) both preserve staged bytes/index IDs and leave HEAD unchanged. The rebuilt app stayed hidden and unfocused with zero presentation/activation calls.

French discard confirmation
French restore confirmation

Testing

  • I manually tested these changes locally

  • Automated tests added/updated

  • Final repaired production source: 5,362 passed / 91 opt-in skips, 544 suites across three disjoint runs covering Git, process capture, repository/worktree IPC, host routing, provider integrations, and Source Control. Additional focused clone integration tests cover noisy success, final failure diagnostics, and cancellation; the WSL suite adds 15 startup/policy cases.

  • Latest fixture-only CI repair: 1,503 passed / 1 opt-in skip across eight full suites, including the entire runtime service plus Azure DevOps, Bitbucket, hosted-review eligibility, default-base resolution, and branch-collision contracts. All 15 original CI failures were reproduced before repair. Their fixtures still answered the former default-base queries, omitted the new Unicode collision catalog, or expected history without signature/color suppression. Updated nine test/fixture files to provide the actual NUL-delimited snapshots and exact command expectations; startup, base-selection, provider, WSL routing, deadlines, and workflow assertions remain intact. No production code changed; existing benchmarks remain applicable. Full typecheck and changed-code quality pass.

  • 40/40 contracts on each of real Git 2.25.5, 2.38.1, and 2.49.1: native macOS baseline and network-disabled Linux containers. Both suites now assert the selected version; Docker fixtures use the host UID/GID, and baseline submodule scripts are built and cached. CI runs both suites at all three boundaries.

  • Full pnpm tc, changed-code quality/type-aware/React/design gates, staged oxlint/formatting, and git diff --check pass. Full anti-slop analysis also passes; the remaining full-repository lint stages run in CI.

  • Full pnpm build passed on the latest rebased branch (desktop, relay, CLI, web, mobile web, and native host targets).

  • Real regression coverage includes populated submodule retention, packed/Unicode branch aliases, detached rebase/bisect use, empty/corrupt blobs and repair, literal/newline paths, unborn HEAD, rename unstage, configured SSH command/variant precedence, canceled/overflowing children, and process capacity retained until exit.

  • Repaired a pre-existing real-Git cancellation-test race: wait for the fetch barrier subscription before aborting, rather than treating a lock-file marker as proof that registration has finished. The original failure and the repair were reproduced on both parent and current source; all 19 real preparation tests pass with cleanup assertions unchanged. Production is unchanged by this final fixture repair.

  • Updated discard descriptions to fresh message IDs and registered their current English source, so existing non-English HEAD/all-changes translations cannot describe the new index-preserving behavior. The generated runtime subset is current. 643 Source Control and i18n tests pass across 72 suites, including real catalog loading for both descriptions in all six built-in locales; catalog, extraction, and coverage checks pass.

  • Fixed a reproduced mobile CI dependency-staging race by joining the production typecheck before the next pnpm entrypoint; both checks still run. Main independently supplied the E2E test typing fix during the final rebase.

  • Actual production relay entry bundled and loaded with os.availableParallelism unavailable on an existing Node 18 runtime; scheduler capacity falls back to two and admission acquire/release works.

Actual platform coverage: native macOS Git and hidden Electron; Linux Git 2.25.1 in a network-disabled container; native/relay real-Git parity and Windows/WSL routing/quoting/cancellation fixtures. Live Windows and a live SSH deployment were not available locally; Windows inbox/preview OpenSSH, macOS OpenSSH, WSL, and cross-version host CI lanes remain required.

Full local pnpm lint stops at five pre-existing mobile import-cycle warnings after mobile dependencies are installed; the entire mobile tree matches the baseline. Root lint and full anti-slop pass; the remaining lint stages pass separately, and the changed-code gate reports zero new findings. CI runs its native cycle audit before the mobile install.

Review

Final adversarial audit: two independent direct Codex 6.1-sol ultra reviewers cross-referenced the cloned Git source; the merged changes retain their cleanup, host, and compatibility guards. A final fully paginated bot audit verifies all original threads resolved and all original head checks successful or intentionally skipped. Merged follow-up #24945 preserves configured SSH quoting and complete review drafts, protects auxiliary branches reserved by rebase update-refs, reuses compact diff parsing, and fences superseded queued maintenance. It passed two independent final 78-file reviews, 55 exact-head CI checks with nine intentional skips, real Git 2.25.5/2.55.0 contracts, and an isolated branch-built public CLI smoke. Its benchmarks and explicit P2/residual limits are in that PR; the benchmark results above describe this original PR.

Two independent direct Codex 6.1-sol ultra review loops, plus a third Codex cross-review and parent review. Pullfrog reported lost 30-second hung-read expiry; the opt-in relay expiry is restored without aborting existing leases. Eleven new tests cover all three diff endpoints, replacement ownership, and timer cleanup, and two independent reviewers returned clean. Four additional findings-severity seats and four blind peer ratings unanimously classified the four retained gaps as P2, with no release blocker. The final readiness review used two additional independent direct Codex 6.1-sol ultra scopes and re-reviewed CI repairs and the latest main-branch relay interaction, with zero outstanding production candidates after the final catalog repair. The final CodeRabbit review also identified excessive clone progress being treated as parsed-output overflow and WSL login startup exceeding the SSH configuration probe deadline. Both were reproduced and fixed: clone-only bounded diagnostic tails reuse the existing buffer, and WSL policy startup gets a finite 10-second budget clamped by positive caller deadlines. Configuration failures remain closed and native probes retain their 2.5-second limit. Two independent direct Codex reviewers re-reviewed both repairs clean. Authors excluded their own implementation from independent verdicts. All reproduced P0/P1 findings were fixed and re-reviewed; final reviewed scopes have no outstanding P0/P1. The loops also fixed the confirmed filename-overflow, missing read-deadline, buffered WSL cancellation, and canceled-submodule-cache P2s.

The latest fixture-only CI repair was independently reviewed in full by two direct Codex 6.1-sol ultra reviewers, both clean. The reviewed nine-file diff is byte-identical after commit hooks; no production code or behavior assertion was changed.

Agent skill upstream boundary

  • Not applicable; no skill-installer code or material changed.

Notes

Retained P2s and measured follow-up candidates:

  • Network/mutation commands retain no default deadline, and most relay requests still use status priority rather than reserved interactive capacity.
  • Non-interruption submodule catalog errors can cache an empty result for five seconds; canonical repository lookup failures also need a separate retry policy. Relay blob failures that are neither absence nor interruption still lack an explicit failed-view state, even though they no longer imply deletion or trigger HEAD substitution.
  • The sparse-worktree cache's 512-entry bound can thrash far above 512 active worktrees (5,000 warm entries still required 5,150 reads and 5,000 stats); a measured 513-entry run missed only once. Current-Git sparse and old-Git existence annotation workers can continue scheduling after cancellation; a separate follow-up is being validated.
  • Concurrent worktree configuration writers can collide on Git's config lock; this change does not introduce repository-wide metadata write serialization.
  • Topological history can walk large reachable histories without a commit graph. Broad fetches elsewhere retain their ref/tag/maintenance policy. Branch-picker/review fallback paths still have a few redundant remote reads; removing the fork ancestry query requires stricter count/error semantics first.
  • Clone SSH preflight reads configuration from its supplied cwd, which may differ from destination-specific template/conditional configuration. Existing triangular publication still sets upstream, and some conflict-base policy remains origin-specific.
  • From fix(worktrees): keep creation fast in large repositories #24346: absent/unmarked prepared registrations remain conservatively preserved when ownership cannot be proved; transient maintenance-config failures retain the six-hour cooldown; refresh/discard target locking retains two extra subprocesses.

Checklist

  • This PR is small and focused (this is the requested broad Git audit)
  • I explained the user-facing change, mechanism, and approach
  • Before/after visual proof attached
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH, folder-workspace, Git-version, and wire compatibility considered
  • Relevant tests, full typecheck/build, changed-code lint/design gates pass; CI covers the full-repository lint command

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a431672d-bb58-49cf-86de-ee7af4a60c62
📥 Commits

Reviewing files that changed from the base of the PR and between 2c8b471 and 384839d.

📒 Files selected for processing (21)
  • src/main/azure-devops/client.test.ts
  • src/main/bitbucket/client.test.ts
  • src/main/git/command-runner/git-ssh-policy-env.ts
  • src/main/git/runner-wsl-network-ssh-policy.test.ts
  • src/main/runtime/orca-runtime-test-fixtures.spec.ts
  • src/main/runtime/orca-runtime-tests/hooks-and-hosted-review-part-03.spec.ts
  • src/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle-part-02.spec.ts
  • src/main/runtime/orca-runtime-tests/ssh-worktree-lifecycle.spec.ts
  • src/main/runtime/orca-runtime-tests/worktree-removal-and-reconciliation.spec.ts
  • src/main/runtime/orca-runtime-tests/worktree-setup-and-startup-part-05.spec.ts
  • src/main/source-control/hosted-review-creation-eligibility.test.ts
  • src/relay/git-command-admission.test.ts
  • src/relay/git-handler-clone-progress-capture.test.ts
  • src/relay/git-handler-command-termination.ts
  • src/relay/git-handler.ts
  • src/shared/child-process/bounded-output-sink.test.ts
  • src/shared/child-process/bounded-output-sink.ts
  • src/shared/child-process/run-process-bytes.test.ts
  • src/shared/child-process/run-process.ts
  • src/shared/growing-byte-buffer.test.ts
  • src/shared/growing-byte-buffer.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The pull request updates Git process execution, including admission, timeouts, cancellation, output capture, and SSH configuration handling. It changes repository and worktree inspection, ref resolution, branch safety, and SSH worktree base fetching. Source-control changes update diff parsing, blob error handling, and bulk staging, unstaging, and discard operations. The pull request also changes CI checks and source-control confirmation text and rename paths.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 38483

The reviewed changes are mostly tests and fixtures, and the earlier concerns about clone output limits and WSL SSH startup have been addressed. No actionable merge-blocking risk remains in this increment. Live WSL and SSH behavior was not exercised.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 38483

The reviewed paths strengthen cancellation and output handling while preserving configured SSH behavior. No new command or credential authority was established. Some uncertainty remains around failed process termination, recovery, and affected repository mutations.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Relay admission budgets are shared within the relay process rather than isolated per repository. Consequently, unresolved child ownership can consume capacity needed by other repositories in that process. The evidence does not establish cross-tenant, cross-service, or environment-wide exposure.

Trust Boundaries and Controls

  • inferred — The reviewed relay changes preserve existing Git configuration authority rather than introducing a new command source. Repository or explicit SSH configuration could already select executable behavior before this PR; the new policy probe makes that selection explicit without establishing additional credential privileges or broader request reachability.

Resilience and Maintainability Implications

  • observed — Pre-aborted requests and spawn failures release admission without leaving an active child. Queued cancellation does not release another operation's grant, and capture failure retains the active child's grant until termination is reported. These controls protect shared process-capacity ownership across interruption and repetition.

Hardening Proposals

  • proposed — Consider detecting admission retained beyond a settled termination failure and documenting a host-owned recovery procedure that verifies process quiescence before restoring capacity. Do not release uncertain grants solely because a caller deadline elapsed.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 75 functions across 70 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main changes: reducing Git queries and preserving data across execution hosts.
Description check ✅ Passed The description covers the required sections with clear change details, rationale, linked issue context, visual proof, extensive testing, review notes, and checklists. It also documents known limitati…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one rough edge worth a look.

Reviewed changes

  • Shared Git primitives. git-command-classification (verbatim move from wsl-direct-git-read-commands, plus diff-tree), git-ssh-policy-env, git-admission-scheduler, git-change-list, git-common-directory, git-worktree-admin, git-default-base-ref, git-blob-absence, git-pathspec-stdin, and git-command-timeout now live under src/shared/ and are re-exported by the main-process modules they replace.
  • Fewer subprocesses. Change name + numstat collapse into one NUL-delimited --raw --numstat -z query across branch/commit compare (main + relay); repo registration batches booleans/paths; default base ref resolves in one for-each-ref; branch/remote/upstream/push-target reads share one config --list -z snapshot.
  • Index authority. Stage/unstage/restore send NUL-delimited literal pathspecs via --pathspec-from-file=-; unstage moved to git reset; discard restores from the index (drops --source=HEAD) so staged content survives; renames mutate both old and new paths.
  • Worktree safety. Host-owned lock and detached rebase/bisect reads (annotateWorktreeLocksFromAdmin, isBranchInDetachedWorktree), no implicit force-removal of submodule worktrees, and a portable case-folded branch-collision guard.
  • Relay/SSH. Every relay Git command now goes through the admission scheduler and runGitToTermination (byte capture, output-limit kill, read deadline, clone progress via observeStderr, cancellation propagation); network commands discover configured core.sshCommand/ssh.variant on both hosts.

I verified the load-bearing compatibility claim with the repo's own harness: --pathspec-from-file/--pathspec-file-nul exist for add, reset, and restore as of Git 2.25.0 (git-scm release notes), CI's 2.25.5 lane runs the new stdin test unguarded, and no fallback is needed. The new maxEntries bound on GitStatusReadLeaseOwner is safe (overflow entries are inert and their load rejections stay handled); the only consequence of the bound itself duplicates the old InFlightPromiseDedupe overflow behavior.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread src/relay/git-handler.ts Resolved
@nwparker
nwparker force-pushed the nwparker/git-query-efficiency branch from f7d1a88 to 4ea5ee5 Compare October 2, 2026 09:21
@nwparker
nwparker requested a review from brennanb2025 as a code owner October 2, 2026 10:46

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f66b126f-e305-458f-aaed-8d909c29c2d6

📥 Commits

Reviewing files that changed from the base of the PR and between b49abdb and 2c8b471.

📒 Files selected for processing (184)
  • .github/actions/prepare-git-compatibility/action.yml
  • .github/workflows/mobile.yml
  • .github/workflows/pr.yml
  • config/scripts/ci-background-step-barriers.test.mjs
  • config/scripts/git-binary-compatibility-workflow.test.mjs
  • src/main/git/command-runner/git-command-timeout.test.ts
  • src/main/git/command-runner/git-command-timeout.ts
  • src/main/git/command-runner/git-exec-admission-lifetime.test.ts
  • src/main/git/command-runner/git-exec-file.ts
  • src/main/git/command-runner/git-process-env.ts
  • src/main/git/command-runner/git-spawn.ts
  • src/main/git/command-runner/git-ssh-policy-env.ts
  • src/main/git/command-runner/git-subprocess-admission.test.ts
  • src/main/git/command-runner/git-subprocess-admission.ts
  • src/main/git/git-network-safety-real-git.test.ts
  • src/main/git/remote.test.ts
  • src/main/git/remove-worktree.test.ts
  • src/main/git/repo-branch-conflict-batched-probe.test.ts
  • src/main/git/repo-branch-conflict.test.ts
  • src/main/git/repo-branch-conflict.ts
  • src/main/git/repo-default-base-ref.ts
  • src/main/git/repo-default-remote.test.ts
  • src/main/git/repo-detection-batching.test.ts
  • src/main/git/repo-detection.ts
  • src/main/git/repo-remote-drift.test.ts
  • src/main/git/repo.test.ts
  • src/main/git/repo.ts
  • src/main/git/runner-buffer-cancellation.test.ts
  • src/main/git/runner-command-exec.test.ts
  • src/main/git/runner-network-ssh-policy.test.ts
  • src/main/git/runner-windows-host-environment.test.ts
  • src/main/git/runner-wsl-direct-read.test.ts
  • src/main/git/runner-wsl-login-shell-capture.test.ts
  • src/main/git/source-control/blob-absence-real-git.test.ts
  • src/main/git/source-control/branch-change-entries.ts
  • src/main/git/source-control/bulk-pathspec-command-line-budget.test.ts
  • src/main/git/source-control/bulk-pathspec-stdin.test.ts
  • src/main/git/source-control/discard-changes.ts
  • src/main/git/source-control/effective-upstream-status-probe.ts
  • src/main/git/source-control/file-diff.ts
  • src/main/git/source-control/git-blob-read.ts
  • src/main/git/source-control/staging-discard-index-safety.test.ts
  • src/main/git/source-control/staging.ts
  • src/main/git/source-control/status-line-stats.ts
  • src/main/git/source-control/submodule-status.ts
  • src/main/git/source-control/wsl-tracked-pathspec-banner.test.ts
  • src/main/git/status-branch-compare.test.ts
  • src/main/git/status-diff-settled-cache.test.ts
  • src/main/git/status-diff.test.ts
  • src/main/git/status-discard-and-bulk-staging.test.ts
  • src/main/git/status-submodule.test.ts
  • src/main/git/status-wsl-pathspecs.test.ts
  • src/main/git/status.test.ts
  • src/main/git/upstream.test.ts
  • src/main/git/upstream.ts
  • src/main/git/worktree-branch-removal-host.test.ts
  • src/main/git/worktree-branch-removal.ts
  • src/main/git/worktree-create-git-executor-real-git.test.ts
  • src/main/git/worktree-create-preparation-real-git.test.ts
  • src/main/git/worktree-graph-listing.test.ts
  • src/main/git/worktree-list-porcelain.test.ts
  • src/main/git/worktree-list-reader.ts
  • src/main/git/worktree-removal.ts
  • src/main/git/worktree-safety-real-git.test.ts
  • src/main/git/wsl-direct-git-read-commands.ts
  • src/main/gitea/client.test.ts
  • src/main/github/default-branch-stale-pr.test.ts
  • src/main/gitlab/client-mr-test-harness.ts
  • src/main/ipc/repos-add-linked-worktree.test.ts
  • src/main/ipc/repos-remote-base-ref-queries.test.ts
  • src/main/ipc/repos-remote-test-harness.ts
  • src/main/ipc/repos/local-repo-registration.ts
  • src/main/ipc/repos/repo-clone-lifecycle.ts
  • src/main/ipc/worktree-remote-ssh-branch-conflict.test.ts
  • src/main/ipc/worktree-remote.ts
  • src/main/ipc/worktrees-ssh-base-ref-resolution.test.ts
  • src/main/ipc/worktrees-ssh-branch-conflict-suffixing.test.ts
  • src/main/ipc/worktrees-ssh-create-base-prefetch.test.ts
  • src/main/ipc/worktrees-ssh-local-base-refresh-overlap.test.ts
  • src/main/ipc/worktrees-ssh-local-base-refresh.test.ts
  • src/main/ipc/worktrees/removal/register-worktree-forget-handlers.ts
  • src/main/providers/working-directory-validation.ts
  • src/main/runtime/repo-worktree-admin-fingerprint.test.ts
  • src/main/runtime/repo-worktree-admin-fingerprint.ts
  • src/main/runtime/runtime-git-status-admission.test.ts
  • src/main/runtime/runtime-preserved-branch-cleanup.ts
  • src/main/runtime/runtime-repository-clone-controller.ts
  • src/main/source-control/repo-default-branch.test.ts
  • src/main/text-generation/pull-request-context.ts
  • src/relay/git-buffer-overflow.ts
  • src/relay/git-command-admission.test.ts
  • src/relay/git-diff-cancellation.test.ts
  • src/relay/git-handler-blob-readers.test.ts
  • src/relay/git-handler-branch-cleanup.ts
  • src/relay/git-handler-branch-diff-ops.ts
  • src/relay/git-handler-command-termination.ts
  • src/relay/git-handler-commit-diff-ops.ts
  • src/relay/git-handler-commit-metadata.test.ts
  • src/relay/git-handler-comparison-operations.ts
  • src/relay/git-handler-diff-blobs.test.ts
  • src/relay/git-handler-diff-retry.test.ts
  • src/relay/git-handler-discard-operations.ts
  • src/relay/git-handler-object-diff-operations.ts
  • src/relay/git-handler-operation-context.ts
  • src/relay/git-handler-ops.ts
  • src/relay/git-handler-push-target.test.ts
  • src/relay/git-handler-read-operations.ts
  • src/relay/git-handler-registration.ts
  • src/relay/git-handler-status-ops.ts
  • src/relay/git-handler-submodule-ops.test.ts
  • src/relay/git-handler-submodule-ops.ts
  • src/relay/git-handler-working-tree-changes.test.ts
  • src/relay/git-handler-worktree-change-operations.ts
  • src/relay/git-handler-worktree-list.ts
  • src/relay/git-handler-worktree-operations.ts
  • src/relay/git-handler-worktree-ops.test.ts
  • src/relay/git-handler-worktree-paths.test.ts
  • src/relay/git-handler-worktree-remove.ts
  • src/relay/git-handler.ts
  • src/relay/git-porcelain-local-parity.test.ts
  • src/relay/git-push-target-local-parity.test.ts
  • src/relay/git-status-branch-line-total.test.ts
  • src/relay/git-status-upstream-negative-cache.ts
  • src/relay/git-stdout-stream.test.ts
  • src/relay/git-stdout-stream.ts
  • src/relay/relay-command-env.test.ts
  • src/relay/relay-command-env.ts
  • src/renderer/src/components/right-sidebar/source-control-discard-confirmation.test.ts
  • src/renderer/src/components/right-sidebar/source-control-discard-localization.test.ts
  • src/renderer/src/components/right-sidebar/source-control/commit/discard-all-sequence.ts
  • src/renderer/src/components/right-sidebar/source-control/commit/discard-confirmation.ts
  • src/renderer/src/components/right-sidebar/source-control/commit/rename-mutation-paths.test.ts
  • src/renderer/src/components/right-sidebar/source-control/commit/source-control-entry-mutation-failures.test.tsx
  • src/renderer/src/components/right-sidebar/source-control/commit/use-bulk-actions.ts
  • src/renderer/src/components/right-sidebar/source-control/commit/use-entry-mutations.ts
  • src/renderer/src/components/right-sidebar/source-control/listing/section-file-list.tsx
  • src/renderer/src/components/right-sidebar/source-control/listing/uncommitted-entry-row.tsx
  • src/renderer/src/components/right-sidebar/source-control/listing/uncommitted-sections.tsx
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json
  • src/shared/child-process/__fixtures__/child-process-import-allowlist.txt
  • src/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txt
  • src/shared/child-process/bounded-output-sink.ts
  • src/shared/child-process/process-spec.ts
  • src/shared/child-process/run-process-bytes.test.ts
  • src/shared/child-process/run-process.ts
  • src/shared/child-process/windows-console-visibility.test.ts
  • src/shared/git-admission-candidate-heap.ts
  • src/shared/git-admission-node-compatibility.test.ts
  • src/shared/git-admission-scheduler.ts
  • src/shared/git-admission-state.ts
  • src/shared/git-admission-waiter-queue.ts
  • src/shared/git-binary-compatibility.test.ts
  • src/shared/git-blob-absence.test.ts
  • src/shared/git-blob-absence.ts
  • src/shared/git-branch-line-total.test.ts
  • src/shared/git-branch-line-total.ts
  • src/shared/git-change-list.test.ts
  • src/shared/git-change-list.ts
  • src/shared/git-command-classification.ts
  • src/shared/git-command-failure-text.ts
  • src/shared/git-command-timeout.ts
  • src/shared/git-common-directory.ts
  • src/shared/git-default-base-ref.ts
  • src/shared/git-effective-upstream.ts
  • src/shared/git-fork-sync.test.ts
  • src/shared/git-fork-sync.ts
  • src/shared/git-history.ts
  • src/shared/git-pathspec-stdin.ts
  • src/shared/git-push-target-resolution.ts
  • src/shared/git-resolution-binary-compatibility.test-cases.ts
  • src/shared/git-resolution-config-snapshot.test.ts
  • src/shared/git-rev-list-output.test.ts
  • src/shared/git-rev-list-output.ts
  • src/shared/git-ssh-policy-env.test.ts
  • src/shared/git-ssh-policy-env.ts
  • src/shared/git-status-read-lease-expiry.test.ts
  • src/shared/git-status-read-lease-owner.ts
  • src/shared/git-worktree-admin.test.ts
  • src/shared/git-worktree-admin.ts
  • src/shared/text-search.ts
  • src/shared/worktree/submodule-removal.test.ts
  • src/shared/worktree/submodule-removal.ts
  • src/shared/wsl-paths.ts
💤 Files with no reviewable changes (5)
  • src/shared/worktree/submodule-removal.test.ts
  • src/shared/child-process/fixtures/child-process-import-allowlist.txt
  • src/shared/child-process/fixtures/windows-console-visibility-allowlist.txt
  • src/shared/worktree/submodule-removal.ts
  • src/relay/relay-command-env.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Comment thread src/relay/git-handler.ts Resolved

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Restored the relay hung-read expiry. GitStatusReadLeaseOwner gained an optional maxInFlightMs; the relay diff dedupe passes 30_000, so a renderer retry after its row timeout starts a fresh read instead of joining a still-hung one until the 120 s deadline. The expiry detaches without aborting existing callers, and the new git-status-read-lease-expiry suite covers expiry, ownership after replacement, overflow, and timer cleanup.
  • Made clone --progress diagnostics overflow-safe. runGitToTermination/runProcess/createOutputSink now accept outputCapture: 'tail'; the relay clone passes a 4 KiB cap and killOnOutputLimit: false, so a chatty clone keeps its final fatal/progress tail instead of failing with git output exceeded maxBuffer. Clone progress notifications still parse every chunk.
  • Raised the WSL SSH-config probe budget. The probe deadline is now min(caller timeout, 10 s) on WSL and min(caller timeout, 2.5 s) natively, so a cold login shell no longer dies before the network command starts while configuration failures stay closed.
  • Regenerated i18n for the index-preserving discard copy. Discard confirmations use semantic sourceControl.discard.* IDs, the English runtime-required catalog is regenerated, and non-English catalogs fall back to the current English text.
  • Hardened the CI/fixture plumbing. Mobile pnpm entrypoints are serialized, the real-Git lanes pin GIT_EXEC_PATH and a hermetic HOME/XDG_CONFIG_HOME, the baseline action builds the git-submodule helper set, and the worktree-safety fixture asserts the selected Git version.

I re-ran the touched suites locally (git-status-read-lease-expiry, git-handler-diff-retry, bounded-output-sink, growing-byte-buffer, runner-wsl-network-ssh-policy, git-handler-clone-progress-capture, git-command-admission, source-control-discard-localization, index-safety); all passed. The one prior Pullfrog finding and both CodeRabbit findings are addressed by these commits and their threads are resolved.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@nwparker
nwparker merged commit 328caa2 into main Oct 3, 2026
65 checks passed
brennanb2025 added a commit that referenced this pull request Oct 16, 2026
…modules

Drop the automatic --force retry. A forced removal deletes the submodule's
repository under .git/worktrees/<id>/modules, including commits, local
branches and stashes that were never pushed, and the parent's dirty check
cannot see them (the retry #24602 removed for the same reason).

Instead classify Git's "working trees containing submodules cannot be moved
or removed" refusal as a new Force Delete reason, 'submodules', after the
lock check and the force guard. The renderer classifies the error string,
so older SSH and paired hosts get it too with no wire change. The toast,
the delete dialog and the Space manager row show one plain warning naming
what Force Delete would lose instead of Git's text.

Keep --ignore-submodules=none in the pre-teardown dirty check so a user's
ignore setting cannot hide submodule edits from it.

Replace the retry test with one real-Git test: a clean checkout whose
submodule holds a local-only commit is refused without --force, stays
registered, keeps the commit, and its refusal classifies as 'submodules'.
brennanb2025 added a commit that referenced this pull request Oct 16, 2026
…t has submodules (#28878)

* fix(worktree): remove a clean checkout that contains submodules

Git refuses to remove any worktree with an initialized submodule unless --force is set, even when nothing is uncommitted. After that specific refusal, re-check the checkout including files inside the submodule and pass --force once. A dirty submodule still blocks the delete. A second --force is not added, because that would also clear a Git lock.

* fix(worktree): offer Force Delete for a checkout Git refuses over submodules

Drop the automatic --force retry. A forced removal deletes the submodule's
repository under .git/worktrees/<id>/modules, including commits, local
branches and stashes that were never pushed, and the parent's dirty check
cannot see them (the retry #24602 removed for the same reason).

Instead classify Git's "working trees containing submodules cannot be moved
or removed" refusal as a new Force Delete reason, 'submodules', after the
lock check and the force guard. The renderer classifies the error string,
so older SSH and paired hosts get it too with no wire change. The toast,
the delete dialog and the Space manager row show one plain warning naming
what Force Delete would lose instead of Git's text.

Keep --ignore-submodules=none in the pre-teardown dirty check so a user's
ignore setting cannot hide submodule edits from it.

Replace the retry test with one real-Git test: a clean checkout whose
submodule holds a local-only commit is refused without --force, stays
registered, keeps the commit, and its refusal classifies as 'submodules'.

* fix(worktree): clarify the submodule Force Delete warning and hide View for it

The warning now says Git deletes the workspace only with Force Delete, the submodule
refusal no longer offers View (there are no changes to show), the real-Git test pins
that Force Delete then succeeds, and a comment no longer describes the removed retry.

* fix(worktree): name submodules when a changed submodule blocks the delete, and clear a stale delete toast

With a commit inside a submodule that the parent has not recorded, the parent sees `M vendor`, so
Orca's dirty check refused first with the generic "It has changed files". Force Delete then destroyed
the never-pushed submodule commit without warning. On a dirty refusal the check now asks
`git status --porcelain=v2` whether a changed entry is a submodule, and if so reports a refusal the
app shows with the submodule Force Delete warning. The detail keeps the dirty sentence, so older
apps still offer Force Delete.

After the Space manager deleted a workspace (its row Force, or a later delete), the failure toast for
that workspace stayed up and still offered Force Delete. A Space delete now dismisses it.

* fix(worktree): warn about submodules on any refused delete of a checkout that has them

Review found the warning still vanished when a submodule commit was already recorded and some other
file was dirty: the parent's status shows no submodule change, yet Force Delete destroys the commit.
The changed-files check now asks whether the checkout has a populated submodule (a .gitmodules path
holding a .git, as Git's own refusal checks) instead of whether one changed. The refusal keeps the
changed paths after the detail, and View stays available since there are changes to view.

* test(worktree): mock the .gitmodules probe as missing in the preflight unit tests

* refactor(worktree): attach the dirty-check status without a type assertion

* fix(worktree): key the delete failure toast by host and worktree id

A Space delete on one host must not dismiss another host's failure toast for a worktree with the same id.

* fix(worktree): dismiss the failure toast by the store row's host after a Space row Force
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant