[nwparker]

	Files	Added	Deleted	Net
Test	42	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​3198	$\color{#cf222e}{\Huge{\mathbf{−}}}$​192	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​3006
Prod	36	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​632	$\color{#cf222e}{\Huge{\mathbf{−}}}$​310	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​322

ELI5

Git operations preserve configured SSH arguments and unusual filenames, protect worktree ownership, and prevent older queued maintenance or delayed probes from overriding newer activity. Session search now negotiates the host and client agent catalogs so independently updated versions can safely exchange results. SSH automatic review drafts now receive their complete changes instead of failing on a restricted command or silently losing a diff. Deleted prepared checkouts can be reclaimed only after verifying their exact owner, and temporary maintenance config failures retry within a fixed budget.

What Changed

• Reuse the existing draft builder on SSH. Its selected-base fetch uses the existing validated remote-tracking fetch; its two diff formats use a narrow git.reviewDiff host method with a full object ID and fixed arguments. The generic executor stays restricted. Old relays return an actionable reconnect error; transport, timeout, output-limit, and malformed-response failures cannot produce an incomplete draft.
• If a prepared checkout has disappeared, use the existing administration reader to find one exact backlink in its owning repository. Require direct administration, regular backlink/marker files, real-path containment, and the unchanged exact ownership reason before Git removes it. Foreign, ambiguous, replaced, or unverifiable owners remain untouched.
• Report opt-out config failures and use the existing bounded maintenance deferral policy. Deliberate opt-out retains its six-hour cooldown. A single bounded superseded flag fences older queued writers and delayed config, filesystem, and index probes, including after timer rollover or tracked-entry eviction. Already admitted writers finish; successful index writes retain their cooldown.
• Add optional supported-agent catalogs to existing session-search requests and status responses, preserving main’s independently advertised Qoder and Jcode history flags. Filter results before querying or building cursors, preserve strict older decoders, and treat an explicitly requested agent as proof of that older client’s decoder. Empty catalogs are authoritative; session counts cannot attest decoder support.
• Preserve shell-quoted SSH arguments and Windows DOS/UNC paths without tokenizing and rebuilding them. Insert BatchMode first because OpenSSH keeps the first value.
• Check all non-bare worktrees' administration records, including rebase update-ref reservations, before branch deletion. Native and relay reuse one reader and refuse unverifiable deletion.
• Reuse the compact NUL change-list parser for legacy relay output so tabs, newlines, Unicode, renames, and copies keep literal paths; remove the duplicate parser.
• Combine preparation-lock and common-directory discovery into one Git query, retaining unambiguous fallbacks for newline-containing paths.
• Cancel obsolete queued maintenance and recheck idle eligibility before either writer starts. Admitted writers finish normally. Validate equal tips with merge-base, then omit the empty diff and zero-count reads.
• Bound the rare missing-ripgrep Git probe using the existing process runner and cancellation. Isolate the built-runtime smoke profile and fix the reproduced real-zsh initial-silence fixture race.

Why

Reuse the existing parser, administration reader, ownership verifier, draft builder, scheduler, stream transport, and process runner. Application source totals 315 net lines across 32 application modules, excluding tests, fixtures, and the shell test harness. Most of the overall additions are regression tests. The additional ownership validation and narrow SSH method fix demonstrated defects; no reliability registry or dependency is added.

Rebuilding SSH argument text loses quoting. Raw diff output exceeded the large-repository budget. Killing a running writer can strand locks. Widening generic execution would grant commands unrelated to review drafts. The rejected cache admission policy regressed another repository's warm-up. The final approach preserves these boundaries and the existing memory budget.

Linked Issue

Maintainer-requested adversarial audit and follow-up to #24346, #24602, and #24923.

Visual Proof

N/A — no layout or controls change. Real Git contracts, encoded SSH wire tests, scheduling probes, and the public CLI smoke flow verify the behaviors.

Benchmarks

macOS, a disposable linked worktree, 30 alternating before/after pairs per real Git version. These time the lock-pointer queries, including child-process overhead, rather than the full create flow. Other validation was running on this workstation.

Git version	Processes before → after	Median ms before → after	p95 ms before → after
2.55.0	2 → 1	5.10 → 4.62	7.56 → 7.45
2.25.5	2 → 1	2.12 → 2.07	2.63 → 2.69

The process reduction is deterministic for ordinary paths. The older binary's p95 increased slightly; no universal tail-latency improvement is claimed. Paths containing newlines deliberately use three calls: the combined query plus the two unambiguous fallbacks.

Controlled correctness/capacity probe	Before	After
Obsolete maintenance writer starts after queued cancellation or lost idle eligibility	1	0
Writer starts on the same owner's next idle attempt	1	1
Legacy diff bytes: 50,000 added paths, each 200 characters, real Git 2.25.5	10,150,000	10,150,000

Scheduling figures use the actual engine, admission scheduler, and command runner with controlled capture/filesystem probes; they are counts, not disk-latency benchmarks. Already-running writers retain their grant until completion and receive no child abort signal. Compact NUL output parses all 50,000 entries within the unchanged 10 MiB budget; the rejected raw format emitted 11,700,000 bytes. Final parsing took 8.48 ms in one captured run, which establishes no comparative parser speedup.

The final equal-tip comparison audit used 60 alternating pairs per binary and route on macOS with real Git at the native/relay execution boundary. These run the exported comparison functions, including ref selection and summary construction; Electron/IPC, admission queues, SSH network, WSL shells, and UI latency are excluded. Other validation was running on this workstation. Each output and child count was asserted; 32 additional real-Git parity records cover annotated tags, blobs, trees, missing objects, and missing refs.

Git / route	Processes before → after	Median ms before → after	p95 ms before → after
2.55.0 native	6 → 4	25.614 → 17.257	36.401 → 35.999
2.55.0 relay	6 → 4	19.718 → 12.694	34.824 → 17.218
2.25.5 native	6 → 4	10.325 → 6.658	16.438 → 12.985
2.25.5 relay	6 → 4	10.550 → 6.732	13.823 → 10.485

The already-resolved comparison component drops three children to one. Git's merge-base still validates the objects; only the empty diff and zero counts are inferred. This equal-tip shortcut adds no cache, capability, RPC, or type-probe subprocess. Timing improvements are specific to this equal-commit fixture.

The cleanup recovery benchmark used 20 attempts per shape and binary on macOS. It times discard only; fixture setup is excluded. Runner-call counts include the first failed spawn attempt when its checkout directory is absent. These are small disposable fixtures, not large-repository throughput measurements.

Git / checkout	Calls before → after	Removed before → after	Median ms before → after	p95 ms before → after
2.25.5 existing	2 → 2	20/20 → 20/20	5.21 → 4.97	5.95 → 7.11
2.25.5 deleted	1 → 2	0/20 → 20/20	1.89 → 8.79	2.03 → 10.88
2.55.0 existing	2 → 2	20/20 → 20/20	9.61 → 10.05	11.87 → 11.26
2.55.0 deleted	1 → 2	0/20 → 20/20	1.84 → 11.21	2.26 → 12.33

Deleted-checkout timing compares immediate failure with successful cleanup; it is a correctness gain, not a speedup. Existing-checkout timings are mixed. Recovery adds no Git inventory process and uses the administration reader's existing eight-worker filesystem limit.

Controlled fake-clock maintenance tests distinguish a deliberate opt-out from a failed config probe. A transient failure previously settled a six-hour cooldown; now the first retry waits two quiet periods (20 minutes with production settings). Persistent failure stops after seven probes and six retries, with zero maintenance writers and zero remaining timers. Disposal, deadlines, and newer activity fence retries.

The sparse-cache proposal was rejected after actual-source measurements. With one repository's 512 results warm, a proposed 128-entry admission history made a newly active 200/512-row repository probe every row on every scan, where the existing LRU warms after one scan. The original bounded LRU is retained. No performance gain is claimed for this rejected approach.

The mixed-version search repair also measures calls at the existing remote-method boundary. These controlled counts exclude transport latency; there is no new runtime method or stream opcode.

Search route	Remote calls before → after
Selected host, complete search	3 → 2
All remote hosts, complete search per host	2 → 2
Legacy-only agent selection	1 → 1
CodeBuddy-only or Jcode-only selection, catalog proof needed	1 → 2

The catalog check adds a call for a narrow selection requiring host support proof. The complete-search path reuses its status response. For superseded same-owner maintenance, the reproduced obsolete writer count changes from one to zero and the next eligible quiet attempt still starts one writer. Eighteen new assertions fail against the previous engine; all 69 focused tests pass after the production fix, including 27 admission-race cases.

Testing

• Automated regression tests added/updated
• Manual public orca host list --json verification
• Rebuilt headless public CLI smoke passed
• Exact-head CI: 55 successful checks, nine intentional skips, zero failures/pending checks

Merged as 62451920ed161a342dc870f93a570d4cd126428b on 2026-10-03 after all gates passed. Reviewed final head: 93409c0962a7707b6a0a744549c05cdaebe9bbe9, integrated with main aca2d51e0e872e14d2698b6108bdc76bc3a49e4f including #24998 history negotiation, #24997 Windows ARM setup, and #24995 exact Linux package assertions. Integration retains all upstream non-search source blobs and both independent history flags; catalog projection preserves the real older-parser contracts that flag-only negotiation cannot cover.

The final integrated Git run passed 4,005 tests / 37 conditional skips across 384 passing / five skipped suites. The search/wire/CLI/web run passed 860 tests across 79 files. Five merged-main/workflow suites passed 79 tests / six existing platform skips. Implementer integration checks passed 163 focused tests and full pnpm tc; root’s rebuilt desktop/relay/CLI/web/mobile/native pnpm build and changed-code quality checks pass (zero new findings across 74 files).

CI at the earlier production head exposed stale exact CLI request expectations and three process mocks that omitted child-termination callbacks. The CLI expectations now include the complete catalog and both advertised history flags. The relay suite runs all 35 cases under both two- and four-slot budgets, asserts zero queued/occupied grants after every case, and proves a timed-out result retains its grant until child termination. Before fixing the mock callbacks, six cases fail; all 70 cases pass after. The focused ownership/termination run passes 104 tests; broader relay/SSH coverage passes 562 with one existing conditional binary-fixture skip; CLI/client coverage passes 148. No production release policy, timeout, or assertion was weakened. Main’s renamed parameterized Qoder/Jcode test suite retains both its scenarios and the catalog tests.

The new SSH tests pass 51/51 on Git 2.25.5 and 2.55.0. Actual encoded-wire tests compare the entire draft with local Git, fetch a changed remote base, stream a large UTF-8 patch with ACK cleanup, preserve dirty/staged worktree data, and cover old hosts, invalid requests/replies, restricted generic commands, cancellation, timeout, and output limits.

The final real-binary gate passes 43 cases on Git 2.55.0, and 41 cases plus two expected update-ref feature skips on Git 2.25.5; five workflow assertions also pass per run. The dedicated relay draft case runs in all three CI lanes: 2.25.5, 2.38.1, and 2.49.1. New deleted-checkout cases pass 10/10 on both local binaries. Broader preparation coverage passes 54 cases with one expected atomic-add feature skip on 2.25.5 in a clean child environment; modern ownership/draft coverage passes 68/68. Initial ad hoc baseline runs inherited this machine's unsupported newer zdiff3 config; no user config was changed. An atomic-add-only fixture is explicitly scoped to Git 2.33+, and lock-reason assertions use the cross-version administration-backed reader.

The final rebuilt headless runtime at 93409c0962a7707b6a0a744549c05cdaebe9bbe9 passed public CLI repo registration, worktree create/show, terminal nonce round trip, removal, and shutdown in an isolated profile/repository with ORCA_BACKGROUND_LAUNCH=1. No window was shown or focused. Its profile, repository, worktree, and listening port were verified absent/closed; Node native ABI was restored.

A final fresh public orca status --json and orca host list --json succeeded for the installed local Mac and connected Linux SSH host. These installed routes do not deploy this branch remotely. Three paired environments previously returned remote_runtime_unavailable and remain unverifiable. The final exact-head Windows SSH matrix, including ARM inbox provisioning/host tests, and all five full unit shards passed. Git compatibility, cross-version wire compatibility, typecheck, static analysis, packaging, SSH watcher isolation, and native/persistence checks passed. Complete exact-head rollup: 55 successful checks, nine intentional skips, zero failures or pending checks.

Review

Two independent direct Codex 6.1-sol ultra adversarial readiness reviewers completely read the prior 77-file diff, both complete CI test repairs, and the full overlapping main integration. Both independently pinned all 78 complete changed files at 93409c0962a7707b6a0a744549c05cdaebe9bbe9 against main aca2d51e0e872e14d2698b6108bdc76bc3a49e4f; binary-diff SHA-256 from git diff --binary is 0e28051879f6109e170a0401e91541110995a0915bc4868193c460ff7fb15250. Unchanged reviewed files are verified byte for byte; every changed integration file is reread. Zero unresolved P0/P1 findings. No Claude or orchestration was used. Final exact-head CI and the fully paginated bot audit are clean for merge, with the documented P2 thread intentionally open.

Four direct Codex severity reviews plus four corrected blind peer ratings agree there are no P0/P1 issues. They retain two P2 limits and distinguish two unproven harmful mechanisms as residual risks. The proposed cache rewrite was rejected on measured regressions; the final audit supports no further speculative performance change.

Actual Git source clones: ~/projects/git at 0f8e75abebff0877cae681a3d5ff31ac47f54220, and built Git 2.25.5 at ~/projects/git-2.25.5-orca-audit, 42ce4c7930ff494136256554cbed730857084c70. Cross-references cover shell arguments, linked administration, missing-checkout validation/removal, branch and sequencer reservations, rev-parse ordering, diff formats, MIDX ownership, merge-base validation, and identical-commit counts. OpenSSH's actual -G output confirms first-option precedence.

The branch incorporates main at aca2d51e0e872e14d2698b6108bdc76bc3a49e4f. All nine non-search files changed on main were independently verified identical to main; the workflow additionally retains all four Git compatibility fixture paths. Main’s independent Qoder/Jcode flags and dual-family tests coexist with the authoritative catalog fields; the old Qoder-only test is replaced by the renamed parameterized suite. Earlier Git feature blobs are preserved through integration except the explicitly reviewed maintenance follow-up.

Fully paginated bot audits cover all four PRs. The superseded-maintenance finding is fixed and its CodeRabbit thread is resolved. Pullfrog’s metadata-free intermediate-host limitation is acknowledged as the P2 below and its thread stays open. The original #24346 marker read/unlink thread also remains acknowledged and open. Final fully paginated audit verifies all current comments, reviews, threads, thread replies, and 64 exact-head checks across the four PRs. CodeRabbit’s manually refreshed incremental review covers ef8dfae through 93409c0, reports no actionable comments, and rates merge risk minimal. Its inherited configured-text-converter hardening suggestion is assessed against existing local draft behavior; the nonblocking docstring warning does not override repository instructions to keep comments concise and non-obvious.

Three prior P2 items are fixed here: missing prepared-checkout cleanup, transient opt-out failure cooldown, and SSH automatic draft generation. Two P2 limits and two residual risks remain explicit:

Remaining item	Scope and practical limit
P2: scans wider than the global 512-entry cache can churn	Existing bounded-memory performance limit. A proposed admission policy proved cross-repository regressions and was rejected.
P2: metadata-free older hosts cannot attest CodeBuddy/ZCode support	Current clients conservatively omit these niche agents on indistinguishable old hosts, including an intermediate host that actually supports them. Update the host to advertise its catalog. Explicit filters sent by older clients are accepted as decoder proof; current/current supports all 24 agents. Core Claude/Codex/OpenCode search stays usable.
Residual: exact marker read and unlink are separate operations	Original #24346 thread stays acknowledged and open. No filesystem compare-and-delete atomicity or demonstrated harmful outcome is claimed.
Residual: forced MIDX timeout can leave an uncertain Windows lock	Original #24346 records acceptance. Ordinary owner cancellation preserves admitted writers; the 60-second hard timeout remains. Unknown locks are never deleted, and no corruption is established.

Agent skill upstream boundary

• Not applicable; no skill-installer material changed.

Notes

Git 2.25 remains supported. Native, WSL, SSH, folder-workspace, and review-provider routes keep their execution boundaries. git.reviewDiff is a new SSH relay method using existing response streaming; old relays degrade with a reconnect error, and old clients keep their existing methods. Optional catalog fields extend existing public session-search schemas; no new public search method, stream opcode, persisted setting, or pairing state is added. Cancellation and capture limits remain enforced by the existing host runner.

Checklist

• User behavior, mechanism, and alternatives explained
• Visual proof marked N/A with reason
• Cross-platform, SSH, folder, Git-version, and wire compatibility considered
• Local regression tests, typecheck, changed-code quality, and full build passed
• Two independent final reviews and rebuilt runtime smoke passed
• Final bot audit and exact-head CI clean before merge

[coderabbitai]

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

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: a5424334-5562-430e-bd5f-ce35d13697d6

📥 Commits

Reviewing files that changed from the base of the PR and between ef8dfae and 93409c0.

📒 Files selected for processing (20)

• src/cli/handlers/search.test.ts
• src/main/ai-vault-search/session-search-agent-compatibility.test.ts
• src/main/ai-vault-search/session-search-scope-entry-points.test.ts
• src/main/ai-vault-search/session-search-service-registry.test.ts
• src/main/ai-vault-search/session-search-service-registry.ts
• src/main/git/repo-maintenance-admission.test.ts
• src/main/ipc/ai-vault-search.test.ts
• src/main/runtime/rpc/methods/ai-vault-search.test.ts
• src/preload/api/ai-vault-bridge.ts
• src/relay/git-handler-review-draft.test.ts
• src/relay/session-search-transport.test.ts
• src/renderer/src/web/preload-api/web-ai-vault-search-compat.test.ts
• src/shared/ai-vault-search-agent-compatibility.ts
• src/shared/ai-vault-search-agent-history.test.ts
• src/shared/ai-vault-search-client.ts
• src/shared/ai-vault-search-contract.test.ts
• src/shared/ai-vault-search-contract.ts
• src/shared/repo-maintenance-schedule.ts
• src/shared/repo-ref-maintenance.ts
• tests/e2e/cross-version-wire/qoder-history-search-downgrade.unit.test.ts

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

---

📝 Walkthrough

Walkthrough

The changes update worktree branch reservations, preparation-lock lookup, and discard ownership checks. They add admission controls and opt-out probe handling for repository maintenance. Relay branch diffs now use NUL-delimited name-status records, and native and relay branch comparisons handle matching commit OIDs directly. The SSH command policy validates and rewrites configured commands, while SSH review-draft generation uses dedicated relay fetch and diff requests. Search requests and status responses exchange supported-agent catalogs, with compatibility filtering across transports. The changes also update filesystem fallback handling and runtime, integration, and compatibility tests.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 93409

Repository maintenance no longer lets a stale queued writer run after a repository becomes active again, and no other open issues were identified. The change looks ready to merge once the remaining Windows SSH and CI checks finish.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 93409

The changes constrain remote read requests and strengthen ownership and stale-work checks. No introduced security defect was established, but incomplete authentication and repository-access coverage prevents a minimal-risk assessment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The remote diff executes with the relay process's existing filesystem authority. The handler has no principal-to-repository check, so its potential repository exposure follows that account's access. Caller-selected repository access predates this PR in branchDiff and commitDiff; no new tenant or privilege boundary was established.

Security Findings and Attack Paths

• observed — Request-controlled revision and format values are constrained before Git execution: only full object IDs and fixed formats are accepted, extra execution fields are ignored, and a literal path separator terminates the command. Tests assert these controls and continued rejection of unrestricted generic diff requests. These controls do not prove repository authorization or eliminate all host-configured Git behavior.

Trust Boundaries and Controls

• observed — Credential-bearing reconnects validate the endpoint credential before attaching the connection to dispatch. The listener also refuses connections before required credential publication. Credentialless connections are marked unproved, and the inspected Git handler does not consume session identity for repository authorization.
• observed — Although preload now treats search delivery as same-build IPC, the main process constructs relay-mode clients for both SSH and runtime legs before delivery. Those clients remove remote source paths and resume commands and redact degraded-root details, preserving the existing remote-data boundary.

Resilience and Maintainability Implications

• observed — Git reads reject timeout, cancellation, and output-limit failures rather than returning partial success; admission is held until child termination. Recovery tests assert preservation of foreign or replaced ownership markers, a reappearing folder, and another repository's registration. These are inspected assertions, not a claim that tests were executed during this review.

Hardening Proposals

• proposed — Consider explicitly disabling text conversion for automatic review patches, or documenting its trust requirements. The fixed patch flags disable external diff but do not explicitly disable configured text converters. Existing staged-diff policy also permits patch rendering; this is a hardening proposal, not a verified newly introduced exploit.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.08% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 75 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 describes two major changes: preserving SSH review context and protecting worktree ownership. It does not cover the full scope, but it accurately summarizes important parts of the ch…
Description check	✅ Passed	The description is detailed and covers the required sections, including the user-facing change, rationale, linked issues, visual-proof status, testing, review, and checklist. The linked issues are ide…

• 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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3e66c0aa-4baa-4dd6-bd32-c760795d9043

📥 Commits

Reviewing files that changed from the base of the PR and between b032867 and 4bbcec5.

📒 Files selected for processing (34)

• .github/workflows/pr.yml
• config/scripts/git-binary-compatibility-workflow.test.mjs
• config/scripts/runtime-serve-terminal-smoke.mjs
• src/main/git/command-runner/git-exec-file.ts
• src/main/git/command-runner/git-exec-options.ts
• src/main/git/local-repo-ref-maintenance.ts
• src/main/git/remove-worktree-branch-cleanup.test.ts
• src/main/git/repo-maintenance-admission.test.ts
• src/main/git/repo-pack-index-maintenance.test.ts
• src/main/git/repo-pack-index-maintenance.ts
• src/main/git/runner-network-ssh-policy.test.ts
• src/main/git/runner-wsl-network-ssh-policy.test.ts
• src/main/git/worktree-branch-removal-host.test.ts
• src/main/git/worktree-branch-removal.ts
• src/main/git/worktree-preparation-lock-real-git.test.ts
• src/main/git/worktree-preparation-lock.test.ts
• src/main/git/worktree-preparation-lock.ts
• src/main/git/worktree-rebase-update-refs-real-git.test.ts
• src/relay/git-handler-branch-cleanup.test.ts
• src/relay/git-handler-branch-cleanup.ts
• src/relay/git-handler-branch-diff.test.ts
• src/relay/git-handler-diff-read-coalescing.test.ts
• src/relay/git-handler-ops.ts
• src/relay/git-handler-utils.ts
• src/shared/git-binary-compatibility.test.ts
• src/shared/git-change-list.test.ts
• src/shared/git-change-list.ts
• src/shared/git-resolution-binary-compatibility.test-cases.ts
• src/shared/git-ssh-policy-env.test.ts
• src/shared/git-ssh-policy-env.ts
• src/shared/git-worktree-admin.test.ts
• src/shared/git-worktree-admin.ts
• src/shared/repo-ref-maintenance-policy.ts
• src/shared/repo-ref-maintenance.ts

💤 Files with no reviewable changes (1)

• src/relay/git-handler-utils.ts

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

[pullfrog]

ℹ️ Minor suggestions only.

Reviewed changes

Reviewed the full 34-file diff at 4bbcec5 (single commit) against main (b0328670).

• SSH argument preservation — git-ssh-policy-env.ts drops tokenize-and-rebuild and instead inserts -o BatchMode=yes right after the OpenSSH executable (first-value-wins), with quoteBareWindowsPathWords for bare Windows paths and shell-syntax guards that fall back to passthrough.
• Branch reservation guard — isBranchInDetachedWorktree becomes isBranchReservedByWorktreeOperation; every non-bare worktree is inspected and rebase-merge/update-refs is parsed so rebase --update-refs auxiliary branches are protected. Native and relay share the reader.
• NUL change-list parser reuse — branchDiffEntries switches to --name-status -z and the shared parseGitChangeList; the duplicate line parser is deleted.
• Combined preparation-lock query — one rev-parse --git-path locked --git-common-dir, with a separate-query fallback for newline-containing paths.
• Background write admission — new admissionSignal/canStart cancel queued maintenance admission and recheck eligibility, while already-running writers finish.
• Test/CI wiring — the new real-Git reservation test joins the compatibility lanes; the runtime smoke test gets its own dev profile.

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

[pullfrog]

ℹ️ No new issues in the UNC-path increment. One pre-existing gap in the same expression remains open.

Reviewed changes

Reviewed the new commit since the prior pullfrog review at 4bbcec5: dbe4ed4 ("preserve bare UNC SSH paths"), a production plus test delta in quoteBareWindowsPathWords (src/shared/git-ssh-policy-env.ts).

• Preserved bare UNC prefixes — the unescape callback now returns the original two-character \\ prefix (uncPrefixOffset) rather than collapsing it to a single backslash, while \\\\ still collapses to \\.
• Added exact regression coverage — a standalone preserves a bare UNC executable path test plus it.each rows for \\, \\\\, -i\\, -oIdentityFile=\\, and -o IdentityFile=\\ forms, each asserted against real /bin/sh argv. Removing the uncPrefixOffset change makes the standalone test fail, so the coverage is real.

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

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the new commit since the prior pullfrog review at dbe4ed4: a0ecadb ("unescape shell operators in Windows SSH paths"), a production plus test delta in quoteBareWindowsPathWords (src/shared/git-ssh-policy-env.ts).

• Widened the Windows-path unescape class — the callback now strips the shell-escape backslash before ;, &, |, <, >, (, ), and backtick, not just whitespace, quotes, and backslash. An unquoted ssh -i C:\keys\A\&B\key now reaches ssh as C:\keys\A&B\key; the uncPrefixOffset UNC branch is untouched.
• Added real-shell regression coverage — it.each rows assert the rewritten argv through /bin/sh for each escaped metacharacter, including both backtick forms.

This closes the prior review's escaped-metacharacter finding. I re-ran src/shared/git-ssh-policy-env.test.ts (48 tests) and they pass. The GitHub thread for that finding has been resolved.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: dfc3c7e6-885b-4e26-b094-5bf2e74ec951

📥 Commits

Reviewing files that changed from the base of the PR and between 4bbcec5 and 58f432c.

📒 Files selected for processing (3)

• src/main/runtime/orca-runtime-tests/worktree-removal-failed-retry.spec.ts
• src/shared/git-ssh-policy-env.test.ts
• src/shared/git-ssh-policy-env.ts

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

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the incremental delta since the prior pullfrog review — 58f432c..32bc77b, a single test-only hunk in src/shared/git-ssh-policy-env.test.ts.

• Guarded the real-OpenSSH probe — keeps the enforced first value even when a later option disables BatchMode now takes vitest's ({ skip }) context and wraps runProcess in a .catch that dynamically skips with OpenSSH is not installed on a child-spawn ENOENT, rethrowing every other error. runProcess rejects with the Node errno error when ssh is absent, so the skip fires only for a genuinely missing binary, and the skip() throw is not swallowed by any surrounding handler.

All prior pullfrog threads remain resolved; no open threads. No production surface in this increment.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the increment since the prior pullfrog review at 32bc77b: ab2d406 ("perf(git): skip equal-tip reads and bound relay discovery"), a 7-file delta containing two production paths plus their tests.

• Equal-tip branch-compare short-circuit — native getBranchCompare and relay branchCompare now skip the diff/rev-list reads when baseOid === headOid && mergeBase === headOid, returning the empty ready summary directly. The extra mergeBase === headOid gate keeps identical non-commit tips (blobs) on the existing no-merge-base path, and the native branch still honors cancellation through options.signal?.throwIfAborted().
• Bounded relay Git-ancestry probe — runListFilesScan replaces the fire-and-forget execFile with runProcess (timeoutMs: 5_000, the caller's signal, and a strict code === 0 && !timedOut verdict), re-checks cancellation immediately after the probe, and keeps ENOENT / non-zero / failure cases falling back to the readdir walk.
• Regression coverage — exact-argv 40/64-char tests asserting no diff/rev-list and no loadBranchChanges call, a cancellation-during-validation test, real-ref equal-commit and blob-tip tests for native and relay, plus probe timeout/spawn-failure/cancellation rows. All 33 tests in the four affected files pass locally.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the increment since the prior pullfrog review: db8a70b ("test(shells): wait for initial zsh output before sending input"), a test-harness-only delta in src/main/zsh-startup-hook-pty-harness.ts and src/main/zsh-scoped-histfile.live-shell.test.ts.

• Gated the real-zsh harness on first output — lastDataAt now starts null and waitForQuiet reports idleFor = 0 until the shell emits its first byte, so the harness waits out a quiet .zshenv instead of typing PS1=... 250ms after spawn. The old behavior queued that line into compinit's prompt and leaked a command not found: y.
• Added a reproducing regression test — a 600ms .zshenv plus an insecure fpath makes compinit -D ask on the first output; the test asserts the warning surfaced, no initialization aborted, no command not found: y, and HISTFILE scoped. It fails against the pre-fix harness, so it is real coverage, not theatre.

No production surface in this increment.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the increment since the prior pullfrog review at db8a70b: commits 0bd88c98, a merge of main (e7b5b1d), and ef8dfae. The substantive new surface is the SSH review-draft path, deleted-checkout ownership recovery, and the bounded maintenance config-probe retry.

• SSH review-draft relay path — execSshReviewDraft maps exactly the selected-base fetch and the two diff formats to a narrow git.fetchRemoteTrackingRef / new git.reviewDiff pair, while every other probe stays on the restricted generic executor (which rejects fetch and non-cached diff, so a non-matching shape fails loudly). git.reviewDiff pins a full object id, worktreePath, and a fixed flag set; ReviewDraftContextError now propagates through safeExec, so an old relay, timeout, or output limit aborts generation instead of emitting a commit-only draft.
• Deleted-checkout ownership recovery — verifyDiscardPreparationOwner locates one exact backlink in the owning repository's administration and requires a unique, direct, real-path-contained registration with a regular, byte-exact lock marker before the double-force removal. Foreign, ambiguous, replaced, and symlinked owners all fail closed.
• Bounded config-probe retry — an unreadable auto-maintenance config now throws instead of reporting an opt-out; the failure spends the bounded deferral budget (seven probes / six backed-off retries), preserves a newer arm, and routes a deadline cancel to timed_out without a retry.
• Fixture relocation — moved the review-draft real-Git compatibility case from src/shared into a standalone src/relay test and added it to the 2.25.5 / 2.38.1 / 2.49.1 matrix lanes, with the matching workflow assertion.
• Comment-only — rewrote the worktree-sparse-checkout-cache.ts why-comment.

All prior review threads are resolved. I independently re-verified the relay git.reviewDiff validation, the discard ownership gate chain, and the maintenance deferral budget, plus one specialist pass on each of those three lenses; none found a defect.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 75c2a546-6029-4c5d-a618-e9b3341b56fe

📥 Commits

Reviewing files that changed from the base of the PR and between ab2d406 and ef8dfae.

📒 Files selected for processing (29)

• .github/workflows/pr.yml
• config/scripts/git-binary-compatibility-workflow.test.mjs
• src/main/git/local-repo-ref-maintenance-probe-retry.test.ts
• src/main/git/local-repo-ref-maintenance.test.ts
• src/main/git/local-repo-ref-maintenance.ts
• src/main/git/worktree-preparation-discard-real-git.test.ts
• src/main/git/worktree-preparation-discard.ts
• src/main/git/worktree-preparation-lock-real-git.test.ts
• src/main/git/worktree-sparse-checkout-cache.ts
• src/main/ipc/filesystem/filesystem-git-pull-request-generation-handlers.ts
• src/main/providers/ssh-git-provider.ts
• src/main/providers/ssh-review-draft-context.test.ts
• src/main/providers/ssh-review-draft-context.ts
• src/main/runtime/runtime-git-generation-context.ts
• src/main/text-generation/pull-request-context.ts
• src/main/zsh-scoped-histfile.live-shell.test.ts
• src/main/zsh-startup-hook-pty-harness.ts
• src/relay/git-handler-object-diff-operations.ts
• src/relay/git-handler-registration.ts
• src/relay/git-handler-review-draft.test.ts
• src/relay/git-review-draft-binary-compatibility.test.ts
• src/shared/child-process/__fixtures__/child-process-import-allowlist.txt
• src/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txt
• src/shared/child-process/windows-console-visibility.test.ts
• src/shared/git-worktree-admin.test.ts
• src/shared/git-worktree-admin.ts
• src/shared/repo-maintenance-schedule.ts
• src/shared/repo-ref-maintenance.ts
• src/shared/review-draft-context-error.ts

💤 Files with no reviewable changes (2)

• src/shared/child-process/fixtures/windows-console-visibility-allowlist.txt
• src/shared/child-process/fixtures/child-process-import-allowlist.txt

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

[pullfrog]

Important

One mixed-version regression: a current client or host withholds codebuddy/zcode from a v1.4.218/v1.4.219 peer that can parse them, because the new legacy fallback is one generation too old.

Reviewed changes

Reviewed the single commit since the prior pullfrog review at ef8dfae: 880af21 ("Fence superseded maintenance and preserve mixed-version session search"), an 18-file delta with two production surfaces.

• Mixed-version session-search negotiation — new optional supportedAgents request/status fields plus compatibleSearchAgents/needsSearchAgentNegotiation narrow a relay request and filter response hits to what the peer advertises; same-build IPC keeps the full catalog.
• Stopped leaking new tags to old peers — the client now negotiates any non-legacy agent instead of only qoder, so a pre-Qoder host no longer receives qoder/jcode filters it cannot parse.
• Superseded maintenance fencing — arm() marks an in-flight same-key attempt superseded, and every canWrite() gate re-checks it, so a newer arm cancels queued-but-unstarted work without interrupting an admitted writer.
• Bounded config-probe retry preserved — probeOptOut/maintain now take and re-check canWrite, so a superseded probe no longer spends a phase cooldown.

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

[pullfrog]

The fallback treats any host that advertises neither supportedAgents nor supportsQoderHistory as if it predates CodeBuddy/ZCode, but those tags shipped in v1.4.218/v1.4.219 (and the PR's own b49 fixture) before Qoder. A current client explicitly filtering codebuddy/zcode against such a host now returns unsupported-agent, and a current host drops those hits for a v1.4.218/219 client — both regressions versus the pre-commit behavior, which only gated qoder.

Technical details

# The legacy fallback is one generation too old

## Affected sites
- `src/shared/ai-vault-search-agent-compatibility.ts:36-41` — the fallback base is `LEGACY_SEARCH_AGENTS` (v1.4.211's set), so `codebuddy`/`zcode` are classified as unsupported whenever a host advertises no capability.
- `src/shared/ai-vault-search-client.ts:58-68` — `needsSearchAgentNegotiation(['codebuddy'])` is `true`, so the client fetches status, `compatibleSearchAgents` returns `[]`, and it answers `{ kind: 'unavailable', reason: 'unsupported-agent' }` before ever calling `aiVault.searchSessions`.
- `src/main/ai-vault-search/session-search-service-registry.ts:42-55,83-85` — the host computes `compatibleAgents` the same way and filters `result.hits` to it, so a capability-less client's unfiltered search loses `codebuddy`/`zcode` hits it could have parsed.

## Evidence
Released tags `v1.4.218`/`v1.4.219`: `src/shared/ai-vault-types.ts` includes `codebuddy` and `zcode` (no `qoder`/`jcode`), and `ai-vault-search-contract.ts` carries `supportedAgents` and `supportsQoderHistory` on neither the request nor the status schema. The PR's own `b49abdb1...` fixture is the same: its request parser accepts `codebuddy`/`zcode` (see `tests/e2e/cross-version-wire/qoder-history-search-downgrade.unit.test.ts`, "an actual b49 explicit %s request proves the old client can read that agent") yet advertises no capability. `v1.4.211`–`v1.4.217` are the only refs where `LEGACY_SEARCH_AGENTS` is the correct floor.

## Required outcome
- A current client with an explicit `codebuddy`/`zcode` filter must not be refused for a host that can parse it, and a current host must not drop those hits for a client that can parse them.
- `qoder`/`jcode` stay withheld from hosts that cannot parse them.

## Open questions for the human
- No capability field separates `<=v1.4.217` from `v1.4.218/219`, so the client cannot tell them apart. Is dropping `codebuddy`/`zcode` on those hosts an accepted trade-off? If not, the host-side rule ("An explicit tag also proves the requesting parser understands that agent", `session-search-service-registry.ts:48-50`) is the model to mirror for the explicit-filter case, or a version floor needs to be advertised.

[nwparker]

Confirmed and recorded as a remaining P2 compatibility limit. The actual v1.4.211 and b49 request/response parsers establish that a metadata-free status cannot distinguish the 20-tag host from the later CodeBuddy/ZCode host. An outgoing explicit tag proves the requesting client can decode it, not that the host can parse it; mirroring the incoming rule would restore a whole-request failure on the older host. Incoming explicit b49 CodeBuddy/ZCode filters already remain supported, with real frozen-parser tests. Unfiltered metadata-free clients and current clients talking to those intermediate hosts conservatively retain the common catalog. Updated hosts advertise supportedAgents; explicit catalogs, including an empty one, are authoritative. The PR body lists the intermediate-host loss and host-update remedy. Four direct Codex severity reviews retained this as P2, with no P0/P1 finding. I am leaving this thread open so the limitation is not represented as fixed. Main’s newly merged independent Jcode history flag is being preserved during integration; it does not change the indistinguishable pre-Qoder boundary.

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the incremental delta since the prior pullfrog review at 880af21: commit 94d8114 ("test: model Git child termination and search catalogs"), a two-file test-only delta with no production change.

• Updated the CLI search RPC assertions — search.test.ts now expects the shared client's supportedAgents: [...AI_VAULT_AGENTS] on both strict aiVault.searchSessions assertions, matching createSessionSearchClient, which appends that field for every non-IPC transport. The assertions had been left stale when the 880af21 client change started sending it.
• Modeled child termination in the relay review-draft test — git-handler-review-draft.test.ts now installs a per-test GitAdmissionScheduler (describe.each([2, 4])), releases the admission grant from onChildTerminated on every mocked runProcess, and asserts no queued work or used budget leaks in afterEach.
• Added a grant-hold regression test — holds a timed-out read grant until the child termination is reported asserts budgets.general.baseUsed === 1 after a timed-out read and 0 only once onChildTerminated fires, so an eager grant release would fail it.

The prior review's mixed-version codebuddy/zcode finding at ai-vault-search-agent-compatibility.ts:40 is untouched by this commit and remains an accepted, author-acknowledged P2 whose thread the author intentionally left open.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

@coderabbitai review

The branch is stable at 93409c0962a7707b6a0a744549c05cdaebe9bbe9, integrated with main aca2d51e0e872e14d2698b6108bdc76bc3a49e4f. Please refresh the paused review against this head. Two independent direct Codex reviews have verified all 78 changed files and the main integration; final CI is running. The prior superseded-writer issue is addressed in 880af21 with queued-admission and delayed-probe regression cases. The PR body documents the remaining P2 limits and residual risks.

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.