Worktree V2 Phase 3: conflict-safe checkout handoff - #48
Merged
gannonh merged 18 commits intoAug 7, 2026
Conversation
added 13 commits
August 6, 2026 16:21
…odes Provider capability DTO, preview/confirm/status/recover types, typed blockers, recovery states, and HANDOFF RPC channels for the Phase 3 conflict-safe checkout handoff. Clients confirm by transaction ID and preview fingerprint only; no paths or payload bytes cross into clients. Refs #42
Backends may advertise safe execution-CWD rebinding through an optional ExecutionCwdRebindCapability (advertise, rebind, verify). resolveHandoffCapability gates handoff on a complete adapter that advertises executionCwdRebindable; anything else resolves to a typed unsupported-provider blocker and preserves V1. Transcript identity (sdkCwd) is never touched by the contract. Refs #42
…t tuples Apply review nits: drop dead WorktreeHandoffPathSummary, label preview source with serverId, and make blocker/direction exhaustiveness a compile-time property instead of a duplicated list. Refs #42
Implement server-owned current-to-managed handoff previews and confirmation with durable journaling, snapshot projection, path leases, runtime proof, and immutable transcript identity. Gate cleanup and session binding behind verified provider rebinds, preserve included ignored files, and fail closed on unsafe or incomplete recovery metadata.\n\nRefs #42
Quiesce the runtime, snapshot and verify the managed source, release the managed worktree (branch preserved), switch the current checkout to the branch, project the snapshot, verify identity, rebind the runtime with proof, and commit the session binding. Failures after release return recovery-required with the retained snapshot as the authority; orphan GC never removes a snapshot referenced by a pending/failed handoff journal.\n\nRefs #42
Snapshot current, remove only captured transferable state, return current to the recorded ref to free the branch, materialize/restore/verify the managed target, rebind the runtime with proof, and commit the binding. Managed-to-current now retains the released record in snapshotted state as the hand-back target. Failures return recovery-required with the retained snapshot; the handed branch is never materialized while checked out elsewhere.\n\nRefs #42
…ollback recover() now rolls back interrupted handoffs per direction from journal steps and the retained snapshot authority: remove txn-owned targets/branches, restore cleaned sources, re-materialize released managed targets, and return current to the handed branch. Rollback is idempotent across crashes, failed journal entries transition to recovered so restarts never re-fence a resolved session, and a rolled-back outcome is a typed blocker.\n\nRefs #42
Add the deterministic handoff adapter factory for credential-free state-machine coverage and a session-side runtime gate: a handoff commit arms the runtime as unverified, and the first Send (and every Send after a restart, since verified proofs are never persisted) requires the live adapter to prove file/shell/MCP/provider resolution of the exact destination. Failure persists recovery-required and blocks Send; a later Send re-attempts the proof so a fixed runtime resumes.\n\nRefs #42
Wire the handoff RPCs through the Electron API, add a pure dialog state machine and HandoffDialog (preview, typed blockers, name editing, confirm, committed/recovery results, Recover), and surface handoff actions plus a status-driven recovery badge in the Changes panel for the bound session. Remote-owned servers label previews with their serverId and expose no local reveal. All user-facing strings carry 7-locale i18n parity.\n\nRefs #42
… dialog Handoff controls now appear only when the session's provider adapter advertises safe execution-CWD rebinding (server-derived handoffCapable on the session DTO), with the typed blocker kept as the server backstop. Confirm/recover buttons stay mounted with spinners while in flight, name edits are allowed while a blocker keeps the preview unusable, and the release notes carry the user-visible feature.\n\nRefs #42
Record the server-owned handoff engine for Worktree V2 Phase 3: fingerprint-bound previews with typed blockers, journaled idempotent steps, snapshot-backed rollback, the immutable transcript vs. execution-CWD split with the provider capability gate, and the session runtime proof before Send. Update the ADR index/log, docs roadmap and bundle log, and the online git-worktrees page with the three directions, blockers, included/ignored policy, branch-release ordering, and recovery.\n\nRefs #42
Author @worktree-v2 handoff E2E specs for preview/confirm/committed and blocked surfaces, plus the env-gated KATA_HANDOFF_DETERMINISTIC_ADAPTER seam so the real Electron app exercises the handoff UI without claiming live provider continuity. Production adapters remain disabled until credentialed UAT.\n\nRefs #42
…ge gaps Add handoff-level tests for invalid-name, branch-occupied-outside-journal, cleanup-in-progress, flags-disabled, destination-detached, git-operation-in-progress blockers and a byte-for-byte binary/rename/deletion/executable c2m transfer. The E2E spec keeps only the credential-backed UI UAT flow; the unsupported-provider gating stays unit-tested per AC-1.\n\nRefs #42
17 tasks
📝 WalkthroughWalkthroughThe PR adds conflict-safe worktree handoff across shared protocols, provider runtime verification, Git services, session management, Electron UI, recovery, documentation, localization, and automated tests. ChangesWorktree Handoff
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07f249114c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (23)
packages/shared/src/protocol/worktree-handoff.ts (1)
66-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the
handoff-rolled-backentry.The block comment states each code "corresponds to a precondition the server checks before any mutation" and that "a blocked handoff claims no mutation".
handoff-rolled-backreports a completed rollback after a mutation attempt. It contradicts both statements. Update the comment to say the tuple also carries post-recovery outcomes, or move this code to a separate outcome union.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/protocol/worktree-handoff.ts` around lines 66 - 107, Update the block comment above WORKTREE_HANDOFF_BLOCKER_CODES to clarify that the tuple includes post-recovery outcomes in addition to precondition blockers, while preserving the existing handoff-rolled-back entry.packages/shared/src/agent/backend/__tests__/deterministic-handoff-adapter.test.ts (1)
42-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this test; it asserts the opposite of its title.
The title states the adapter "fails the handoff gate". The body asserts
resolution.supportedistrue. The inline comment confirms the capability gate passes and only the later proof gate rejects. Rename the test to match the asserted behavior. Also prefer a static import forresolveHandoffCapability, as inhandoff-capability.test.ts.♻️ Proposed change
- test('an adapter with missing categories fails the handoff gate', async () => { - const { resolveHandoffCapability } = await import('../handoff-capability') + test('an adapter with missing categories still passes the capability gate but yields an incomplete proof', async () => { const adapter = createDeterministicHandoffAdapter({ missingChecks: ['shell'] })Add the static import at the top of the file:
import { createDeterministicHandoffAdapter } from '../deterministic-handoff-adapter' +import { resolveHandoffCapability } from '../handoff-capability'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/agent/backend/__tests__/deterministic-handoff-adapter.test.ts` around lines 42 - 53, Rename the test around createDeterministicHandoffAdapter to state that missing categories pass the capability handoff gate while the live proof is incomplete, matching resolution.supported being true. Replace the dynamic import of resolveHandoffCapability with a static top-level import, consistent with handoff-capability.test.ts.packages/shared/src/agent/backend/deterministic-handoff-adapter.ts (1)
20-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive
missingChecksfromPROOF_CATEGORIES.The category list exists twice. If a fifth category is added, the two lists can drift.
♻️ Proposed change
+const PROOF_CATEGORIES = ['file', 'shell', 'mcp', 'provider'] as const +export type ProofCategory = (typeof PROOF_CATEGORIES)[number] + export interface DeterministicHandoffAdapterOptions { /** Adapter identity stamped into capabilities and proofs. */ adapterId?: string /** rebindExecutionCwd throws when true (runtime cannot quiesce/rebind). */ failRebind?: boolean /** verifyExecutionCwd throws when true (proof acquisition fails). */ failVerify?: boolean /** Proof categories to omit; the handoff gate requires all four. */ - missingChecks?: Array<'file' | 'shell' | 'mcp' | 'provider'> + missingChecks?: ProofCategory[] /** Rebind destinations recorded for later assertions (shared array). */ rebindLog?: string[] } - -const PROOF_CATEGORIES = ['file', 'shell', 'mcp', 'provider'] as const🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/agent/backend/deterministic-handoff-adapter.ts` around lines 20 - 26, Derive the missingChecks type from PROOF_CATEGORIES instead of duplicating the string-union list in the options declaration. Update the relevant type annotation near PROOF_CATEGORIES so adding a category automatically updates accepted missingChecks values.packages/shared/src/agent/backend/__tests__/handoff-capability.test.ts (1)
117-126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test cannot fail.
RecordingHandoffAdapterholds no reference to the backend. It cannot callupdateSdkCwdunder any implementation. SosdkCwdWritesis always empty, and the assertion proves nothing about the production contract. To make the test meaningful, construct the adapter with the backend and assert that a real rebind path leavessdkCwduntouched. Alternatively, remove the test and cover transcript-identity preservation where a production adapter is wired to a backend.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/agent/backend/__tests__/handoff-capability.test.ts` around lines 117 - 126, Make the transcript-identity test exercise a real mutation-capable path instead of relying on RecordingHandoffAdapter.sdkCwdWrites being empty by construction. Update RecordingHandoffAdapter and its setup in makeBackend so the adapter can reference the backend, then have rebindExecutionCwd invoke the production update path and assert sdkCwd remains unchanged; otherwise remove this test and add the assertion at the production adapter/backend integration point.packages/shared/src/agent/backend/types.ts (1)
338-344: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a typed union for
checks.
checks: string[]accepts any string. The handoff gate requires the four proof categoriesfile,shell,mcp, andprovider. A shared exported constant plus a derived union would let the compiler catch a malformed proof, in the same wayWORKTREE_HANDOFF_BLOCKER_CODESderivesWorktreeHandoffBlockerCode.♻️ Proposed typing
+export const EXECUTION_CWD_PROOF_CHECKS = [ + 'file:read', + 'shell:cwd', + 'mcp:list', + 'provider:cwd', +] as const; + +export type ExecutionCwdProofCheck = (typeof EXECUTION_CWD_PROOF_CHECKS)[number]; + export interface ExecutionCwdProof { adapterId: string; destinationPath: string; verifiedAt: number; /** Concrete checks performed, e.g. `file:read`, `shell:cwd`, `mcp:list`, `provider:cwd`. */ - checks: string[]; + checks: ExecutionCwdProofCheck[]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/agent/backend/types.ts` around lines 338 - 344, Replace the unrestricted checks: string[] field in ExecutionCwdProof with a union derived from a shared exported proof-category constant containing file, shell, mcp, and provider. Follow the existing WORKTREE_HANDOFF_BLOCKER_CODES and WorktreeHandoffBlockerCode pattern so malformed check values are rejected at compile time.packages/shared/src/agent/backend/handoff-capability.ts (1)
39-42: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
handoffCapability()against throwing.
isCompleteCapabilityonly checks that the members are functions. A degraded adapter can still throw insidehandoffCapability(). The exception then escapes this gate, and the caller receives a generic error instead of the documented typedunsupported-providerblocker.🛡️ Proposed fix
- const capability = adapter.handoffCapability() - if (!capability || capability.executionCwdRebindable !== true) { + let capability: WorktreeHandoffProviderCapability | undefined + try { + capability = adapter.handoffCapability() + } catch { + return { supported: false, blocker: 'unsupported-provider' } + } + if (!capability || capability.executionCwdRebindable !== true) { return { supported: false, blocker: 'unsupported-provider' } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/agent/backend/handoff-capability.ts` around lines 39 - 42, Update the capability check around handoffCapability() to catch exceptions thrown by degraded adapters and return the documented { supported: false, blocker: 'unsupported-provider' } result. Keep the existing executionCwdRebindable validation unchanged for successfully returned capabilities.packages/server-core/src/git/__tests__/worktree-handoff.test.ts (4)
541-552: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPin the expected blocker code.
Lines 550-551 pass for any blocker, including
flags-disabledorhandoff-in-progress. The test therefore does not prove that a missing managed worktree is detected. Assert the exact code the service returns for this state.♻️ Proposed assertion
- // The missing checkout is detected at the source-context inspection first. - expect(p.blocked).toBeDefined() - expect((p.blocked?.code as WorktreeHandoffBlockerCode) ?? '').toBeTruthy() + // The missing checkout is detected at the source-context inspection first. + expect(p.blocked?.code satisfies WorktreeHandoffBlockerCode | undefined).toBe('unsupported-snapshot')Replace
'unsupported-snapshot'with the code the service actually emits for a missing source checkout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/__tests__/worktree-handoff.test.ts` around lines 541 - 552, Update the test `blocks when the managed worktree is missing` to assert the exact `WorktreeHandoffBlockerCode` returned for a missing source checkout, replacing the truthiness check with the service’s actual missing-worktree blocker code. Keep the existing `p.blocked` presence assertion and cleanup setup unchanged.
532-532: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the top-level
node:fsimport.Line 2 already imports
writeFileSync. Line 532 re-imports it dynamically and shadows that binding. Line 755 does the same forwriteFileSyncandchmodSync. AddchmodSync,mkdirSync, andrmSyncto the static import at line 2 and remove the dynamic imports at lines 298, 532, 544, and 755.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/__tests__/worktree-handoff.test.ts` at line 532, Update the top-level node:fs import to include chmodSync, mkdirSync, and rmSync, then remove the redundant dynamic imports at the referenced test locations, reusing the existing static bindings throughout worktree-handoff.test.ts.
458-458: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify this assertion.
The inline ternary with an awaited
.then()is hard to read. The''fallback also compares a HEAD SHA to an empty string whenexpectedBranchis null.managedSessionalready guarantees a V2 record with a branch, so resolve the branch SHA before the assertion.♻️ Proposed simplification
- expect(await headOf(harness.repo)).toBe(record.expectedBranch ? await git(harness.repo, ['rev-parse', record.expectedBranch]) .then((value) => value.trim()) : '') + const handedSha = (await git(harness.repo, ['rev-parse', record.expectedBranch!])).trim() + expect(await headOf(harness.repo)).toBe(handedSha)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/__tests__/worktree-handoff.test.ts` at line 458, In the managedSession test setup, resolve the expected branch SHA before the assertion using the guaranteed V2 record branch, then compare headOf(harness.repo) directly with that resolved value. Remove the inline ternary, empty-string fallback, and chained await from the expect expression.
826-828: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
failingAdapterdoes not need to be async.
createDeterministicHandoffAdapteris synchronous. Theasyncwrapper forcesawait failingAdapter()at lines 872, 901. Line 471 already calls the factory directly. Make the helper synchronous, or drop it and call the factory inline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/__tests__/worktree-handoff.test.ts` around lines 826 - 828, Make failingAdapter synchronous because createDeterministicHandoffAdapter returns immediately; remove async and update the call sites around lines 872 and 901 to stop awaiting it, or inline the factory call while preserving the failRebind configuration.packages/shared/src/protocol/__tests__/worktree-handoff-contracts.test.ts (2)
20-23: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the exact direction values, not only the count.
WORKTREE_HANDOFF_DIRECTIONSvalues are wire values. The current assertions pass if a direction is renamed, for examplehand-back→handback. A rename breaks the RPC contract with the renderer and the server without any test failure. Pin the exact set.♻️ Proposed assertion
it('defines exactly the three supported handoff directions', () => { - expect(WORKTREE_HANDOFF_DIRECTIONS).toHaveLength(3) - expect(new Set(WORKTREE_HANDOFF_DIRECTIONS).size).toBe(3) + expect([...WORKTREE_HANDOFF_DIRECTIONS].sort()).toEqual([ + 'current-to-managed', + 'hand-back', + 'managed-to-current', + ]) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/protocol/__tests__/worktree-handoff-contracts.test.ts` around lines 20 - 23, Update the test covering WORKTREE_HANDOFF_DIRECTIONS to assert the exact expected wire-value set in addition to its length and uniqueness checks. Pin all three supported direction strings so renames or substitutions fail the contract test.
40-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese
inchecks cannot fail.
inputis a hand-written literal.'path' in inputis false because the literal omitspath, not becauseWorktreeHandoffConfirmInputforbids it. If the interface later gains an optionalcheckoutPath, this test still passes and the "never paths or patches" guarantee is silently lost.Assert against a runtime source of truth instead, for example an exported allowed-key list or a sanitizer function that the confirm handler also uses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/protocol/__tests__/worktree-handoff-contracts.test.ts` around lines 40 - 56, The test in the confirmation contract case currently validates only omissions from a hand-written literal, not the runtime contract. Replace the `'in'` checks and local key set around `WorktreeHandoffConfirmInput` with assertions against the exported allowed-key source or sanitizer used by the confirm handler, ensuring path, patch, and client-nominated path fields remain rejected.packages/server-core/src/git/__tests__/handoff-runtime-gate.test.ts (1)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore
KATA_FEATURE_GIT_WORKSPACE_V1after the suite.The
beforeEachsets the flag and nothing restores it.packages/server-core/src/git/__tests__/worktree-handoff.test.tssaves and restoresKATA_FEATURE_WORKTREE_V2for this reason. Follow the same pattern so the flag cannot leak into other suites that share a process.♻️ Proposed fix
+let previousGitWorkspaceV1: string | undefined beforeEach(() => { + previousGitWorkspaceV1 = process.env.KATA_FEATURE_GIT_WORKSPACE_V1 process.env.KATA_FEATURE_GIT_WORKSPACE_V1 = '1' }) +afterEach(() => { + if (previousGitWorkspaceV1 === undefined) delete process.env.KATA_FEATURE_GIT_WORKSPACE_V1 + else process.env.KATA_FEATURE_GIT_WORKSPACE_V1 = previousGitWorkspaceV1 +})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/__tests__/handoff-runtime-gate.test.ts` around lines 18 - 20, Update the test suite’s environment setup around beforeEach so KATA_FEATURE_GIT_WORKSPACE_V1 is saved before being set and restored after each test, following the existing save-and-restore pattern in the related worktree-handoff tests. Ensure restoration handles the variable’s original unset or defined state.packages/server-core/src/git/worktree-snapshot-service.ts (1)
731-734: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGuard
entry.linkTextbefore the symlink comparison.
readlinkSync(dest) !== entry.linkTexttreats anundefinedlinkTextas a content difference and reports "Destination file differs".restoreFileEntryhandles the same case at Lines 989-991 with an explicit "Symlink entry has no link text" error. Match that behavior so a malformed manifest is distinguishable from a genuine conflict.♻️ Proposed fix
if (entry.mode === '120000') { - if (existing !== 'symlink' || readlinkSync(dest) !== entry.linkText) { + if (typeof entry.linkText !== 'string') { + throw new WorktreeSnapshotError('SNAPSHOT_RESTORE_FAILED', `Symlink entry has no link text: ${entry.path}`) + } + if (existing !== 'symlink' || readlinkSync(dest) !== entry.linkText) { throw new WorktreeSnapshotError('SNAPSHOT_RESTORE_FAILED', `Destination file differs: ${entry.path}`) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/worktree-snapshot-service.ts` around lines 731 - 734, Update the symlink handling in the snapshot restore flow around the existing mode check to validate that entry.linkText is defined before comparing it with readlinkSync(dest). For a missing linkText, raise the same explicit malformed-manifest error used by restoreFileEntry; only perform the existing destination-difference check when linkText is present.packages/server-core/src/git/managed-worktree-service.ts (1)
559-578: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one path-derivation helper between
resolveWorktreePathandcreateWorktree.
resolveWorktreePathrecomputes the materialization root, repo key, display fragment, and leaf thatcreateWorktreederives at Lines 376-402. The handoff preview binds the returned path into its fingerprint, and confirm then relies oncreateWorktreeproducing the identical path. If either derivation changes independently, the fingerprint covers a path the creation never uses, and the mismatch is silent.Extract the leaf and destination computation into a single private helper and call it from both methods.
Note also that
resolveWorktreePathomits theworkspaceIdsafe-component check thatcreateWorktreeapplies at Line 385, so a preview can compute a path that creation later rejects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/managed-worktree-service.ts` around lines 559 - 578, Extract the shared worktree destination derivation from createWorktree and resolveWorktreePath into one private helper, including workspaceId safe-component validation, materialization root resolution, repo-key computation, display-fragment handling, and leaf construction. Update both methods to call this helper so preview and creation produce the same path and validation behavior.packages/server-core/src/git/worktree-journal.ts (1)
205-210: 🧹 Nitpick | 🔵 TrivialFailed handoff entries pin their snapshots indefinitely.
recovernow resolvesfailedentries, which is correct. Note the retention consequence:compact()at Lines 228-233 keepsfailedentries forever, andgcOrphanedSnapshotsinworktree-lifecycle-service.tstreats afailedhandoff entry'sretainedSnapshotIdas referenced. A handoff that fails and is never explicitly recovered therefore retains its snapshot payload for the life of the server.The bias toward retaining user work is correct. Consider adding a metric or an inventory surface for failed handoff entries so operators can see accumulating retained payloads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/git/worktree-journal.ts` around lines 205 - 210, Add an operator-visible metric or inventory surface for unresolved failed handoff entries, including their retained snapshot identifiers or payload counts. Update the worktree journal/lifecycle reporting around recover, compact, or gcOrphanedSnapshots without changing retention behavior: failed entries must remain retained until explicitly recovered.packages/server-core/src/handlers/rpc/git.test.ts (1)
472-509: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the disabled flag and the typed-error mapping.
The test sets both
FLAGandV2_FLAGto'1'and exercises only the success path. Two behaviors added inpackages/server-core/src/handlers/rpc/git.tsstay untested:
assertWorktreeV2Enabled()on all four handlers. No case asserts a rejection whenV2_FLAGis unset.throwTypedWorktreeHandoffErrorat Lines 141-146. No case asserts that a thrownWorktreeHandoffErrorbecomes aCodedErrorcarryingWORKTREE_HANDOFF_ERROR_CODE, and that an unrelated error is rethrown unchanged.The second one matters most: it is the contract the renderer uses to distinguish a typed handoff blocker from a generic failure.
Do you want me to generate these test cases?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/handlers/rpc/git.test.ts` around lines 472 - 509, The handoff RPC tests cover only enabled success paths and omit disabled-flag and typed-error behavior. Extend the tests around the shared handoff handlers to assert all four reject when V2_FLAG is unset, and add coverage for throwTypedWorktreeHandoffError verifying WorktreeHandoffError becomes a CodedError with WORKTREE_HANDOFF_ERROR_CODE while unrelated errors are rethrown unchanged.apps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.ts (2)
149-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a case without
retainedSnapshotId.
recoveryResultFromStatusomits the key whenstatus.retainedSnapshotIdis absent (line 108 ofhandoff-controls.ts). The test only covers the present case. Add a second assertion that the key is absent from the result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.ts` around lines 149 - 168, Extend the recoveryResultFromStatus tests with a case using an active status that omits retainedSnapshotId, and assert the resulting recovery-required object does not contain that key. Keep the existing retainedSnapshotId-present case unchanged.
189-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the reducer guard branches.
The suite covers the happy paths. The guard branches in
reduceHandoffDialogstay untested:
confirmdispatched from a non-previewphase, or with a blocked preview (line 203 ofhandoff-controls.ts).name-changeddispatched fromloading,error, orcommitted(line 199).recoverdispatched from a non-recovery-requiredphase (line 221).recovery-from-statuswith a non-recovery-requiredresult (line 213).Each branch returns the unchanged state. Add assertions so a future edit cannot silently remove them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.ts` around lines 189 - 253, Extend the “handoff dialog state machine” tests to cover reducer guard branches: verify confirm leaves state unchanged from a non-preview phase and for a blocked preview; name-changed is unchanged in loading, error, and committed phases; recover is unchanged outside recovery-required; and recovery-from-status is unchanged for a non-recovery-required result. Use deep state equality assertions against the original state for each case.e2e/tests/git/worktree-v2-handoff.spec.ts (1)
75-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the localized Cancel locator with a stable test id.
getByRole("button", { name: /Cancel/ })matches the translated labelgit.handoff.cancel. The test breaks if the app runs under a non-English locale, and the regex also matches any other button whose accessible name containsCancel. Adddata-testid="handoff-cancel-button"to the cancelButtonat line 429 ofHandoffDialog.tsxand select it here.♻️ Proposed change
- await page.getByRole("button", { name: /Cancel/ }).click(); + await page.getByTestId("handoff-cancel-button").click();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tests/git/worktree-v2-handoff.spec.ts` at line 75, Replace the localized role-based locator in the worktree handoff test with the stable `handoff-cancel-button` test id, and add that `data-testid` to the cancel `Button` in `HandoffDialog.tsx` so the test targets the intended control across locales.apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts (3)
100-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the inline
import()type to the top-level type import.Line 101 uses an inline
import('@kata-sh/shared/protocol').WorktreeHandoffStatus. The file already imports types from the same module at lines 13-18. AddWorktreeHandoffStatusthere for consistency.♻️ Proposed change
import type { CheckoutMode, WorktreeHandoffDirection, WorktreeHandoffPreview, WorktreeHandoffResult, + WorktreeHandoffStatus, } from '`@kata-sh/shared/protocol`'- status: Extract<import('`@kata-sh/shared/protocol`').WorktreeHandoffStatus, { active: true }>, + status: Extract<WorktreeHandoffStatus, { active: true }>,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts` around lines 100 - 111, Update the top-level type import from `@kata-sh/shared/protocol` to include WorktreeHandoffStatus, then replace the inline import() reference in recoveryResultFromStatus with that imported type while preserving the existing Extract constraint.
49-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
canOfferHandoffalways returnstrue.
handoffDirectionsForCheckoutreturns at least one direction for every input. The test at lines 99-103 ofhandoff-controls.test.tsassertstruein all three cases. The helper therefore provides no gating. The real gate ishandoffCapableinHandoffAction.tsx. Consider removing the helper, or document that it is a placeholder for future direction filtering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts` around lines 49 - 51, Remove the redundant canOfferHandoff helper and update its callers to use the existing handoffCapable gate in HandoffAction.tsx; alternatively, if the helper must remain, document it explicitly as a future direction-filtering placeholder without treating it as a capability check.
76-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isRemoteOwnedPreviewignores itspreviewargument.The function returns
isRemoteWorkspaceonly. The doc comment says the result describes the preview origin. The preview already carriessource.serverIdanddestination.serverId, so the origin is derivable from the payload. Either use those fields or drop the parameter and rename the helper.♻️ Option: drop the unused parameter
-/** True when a preview/result originates from a remote owning server. */ -export function isRemoteOwnedPreview(preview: WorktreeHandoffPreview, isRemoteWorkspace: boolean): boolean { - return isRemoteWorkspace -} +/** True when the session workspace is owned by a remote server. */ +export function isRemoteOwnedPreview(isRemoteWorkspace: boolean): boolean { + return isRemoteWorkspace +}Callers in
HandoffDialog.tsx(line 111) and the test at lines 118-121 need the same update.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts` around lines 76 - 78, Update isRemoteOwnedPreview to derive preview ownership from preview.source.serverId and preview.destination.serverId instead of returning isRemoteWorkspace alone; alternatively remove the unused preview parameter and rename the helper to reflect its actual behavior. Apply the corresponding signature and call-site updates in HandoffDialog.tsx and the affected test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/electron/src/renderer/components/app-shell/handoff/HandoffDialog.tsx`:
- Around line 307-324: Update the HandoffDialog open effect to increment
previewSeqRef.current rather than resetting it to 0, preserving invalidation of
previews from prior runs. Also replace the initialRecovery object dependency
with initialRecovery?.transactionId, or memoize the prop at its call site, so
equivalent recovery data does not reopen and reset dialog state.
- Around line 354-367: Update handleRecover in HandoffDialog so handoffRecover
failures dispatch a dedicated recovery-error action instead of preview-error.
Add or use the corresponding reducer action in handoff-controls to retain phase
recovery-required while storing the failure message, keeping RecoveryBody
mounted and the Recover control available.
- Around line 197-212: Update CommittedBody to render
result.summary.checkout.expectedBranch alongside the committed status details,
using the existing translation or an appropriate branch label. Ensure
data-testid="handoff-committed" includes the mandatory destination branch value
from the committed SessionCheckout result.
In
`@apps/electron/src/renderer/components/right-sidebar/git-changes/ChangesPanel.tsx`:
- Around line 108-115: Reset recoveryDialog in the sessionId change effect
alongside selectedPath, and guard the HandoffDialog render so it only appears
when sessionId is present. Update the recovery flow around HandoffDialog to
prevent a prior session’s transactionId from being used with a new or null
session.
In `@e2e/tests/git/worktree-v2-handoff.spec.ts`:
- Around line 80-82: Update the cleanup in the test’s finally block to also
remove the managed worktree created during handoff and its registry entry, or
configure this spec’s worktree root to use a temporary directory. Preserve
cleanup of the temporary repository and ensure repeated runs leave no managed
worktree state behind.
- Around line 16-18: Move the KATA feature and handoff flag configuration out of
module-scope process.env mutations in the worktree handoff spec, and set them
through the isolated launch environment using the launchEnv/isolatedRun harness
configuration or project env. Ensure the flags are included in baseEnv so the
Electron app receives them without leaking into other tests.
In `@packages/server-core/src/git/__tests__/handoff-runtime-gate.test.ts`:
- Around line 92-95: Rename the local agent variable initialized with
createDeterministicHandoffAdapter({ failVerify: true }) from healthyAgent to
failingAgent, and update its later references in the test. Preserve the failing
adapter configuration and the assertion that an already verified session remains
verified without rerunning the proof.
In `@packages/server-core/src/git/__tests__/worktree-lifecycle.test.ts`:
- Around line 879-898: Rename the test around svc.journal.fail to describe
retention by a failed handoff journal rather than an in-progress one. Do not add
a separate in-progress test unless that guarantee is also required; if added,
omit the fail call in that second test.
In `@packages/server-core/src/git/managed-worktree-service.ts`:
- Around line 397-399: Update the terminal error reporting in the token-attempt
flow around maxAttempts so it reports the actual number of attempts used,
including the single-attempt path when pathToken is supplied, instead of always
referencing MAX_TOKEN_RETRIES. Keep the retry behavior and token generation
unchanged.
- Around line 99-107: Validate pathToken in createWorktree before using it as
the leaf in path.join(destinationRoot, leaf). Reject tokens containing path
separators, "." or ".." components, or any other non-leaf path shape, while
preserving valid internally issued tokens and existing lock/retry behavior.
In `@packages/server-core/src/git/worktree-snapshot-service.ts`:
- Around line 706-775: Update the handoff transaction catch path around
applySnapshotToCheckout so any failure after partial patch or file application
invokes the createWorktree rollback flow: remove the destination checkout and
restore the source using the retained snapshot before completing
recovery-required handling. Preserve recording the retained snapshot ID and
ensure cleanup runs for all incomplete-apply failures.
In `@packages/server-core/src/handlers/rpc/git.ts`:
- Around line 156-158: Localize the shared handoff-fence error and centralize
the SessionManager guard. In
packages/server-core/src/handlers/rpc/git.ts#L156-L158, replace the literal with
i18n.t('git.handoff.pendingFence'); in
packages/server-core/src/sessions/SessionManager.ts#L3337-L3339, `#L6585-L6587`,
and `#L6851-L6853`, extract and reuse assertSessionHandoffNotFenced(sessionId) so
all three sites use the same check and translated message. Add
git.handoff.pendingFence to all seven locale files in alphabetical order.
- Around line 335-344: Update the session leasing loop around
git.repository.getContext so resolution failures are isolated per session: catch
errors inside each iteration, preserve leasing with the original checkoutPath
fallback, and allow lifecycle reconciliation and readiness signaling to
continue. Cache repository-root resolution by checkoutPath and reuse cached
results for shared directories to avoid repeated Git subprocesses.
- Around line 654-667: After the inner resolveMutationContext call in the
mutation-lock callback, assert that ctx.gitCommonDir matches
initialContext.gitCommonDir before invoking op. Reuse the same common-directory
consistency guard used by removeWorktree, and ensure the mutation does not
proceed when the resolved repository differs from the lock key.
In `@packages/server-core/src/sessions/SessionManager.ts`:
- Around line 6585-6587: Update the session-fence check near
assertSessionCheckoutReady so deleteSession remains available when the handoff
transaction is in recovery-required state. Preserve the fence for pending and
other non-recoverable states, and reuse the existing handoff transaction-state
symbol or accessor to identify recovery-required rather than bypassing the fence
unconditionally.
- Around line 1105-1116: Update managedToSession’s handoffCapable calculation to
wrap resolveHandoffCapability(m.agent).supported in try/catch, matching the
existing sharedOwnerCount protection. Preserve the normal capability result when
resolution succeeds, and prevent provider adapter errors from propagating and
breaking session listing.
- Around line 5396-5414: Update verifyHandoffRuntimeBeforeSend to return
immediately when handoffRuntimeState is undefined, before checking
executionCwdRebind, while preserving verification and recovery behavior for
armed states. Add a regression test covering a session with no
handoffRuntimeState and no executionCwdRebind adapter, asserting that sending a
message succeeds.
- Around line 3612-3619: Restrict the deterministic adapter branch in
SessionManager to development or test builds in addition to
KATA_HANDOFF_DETERMINISTIC_ADAPTER === '1', preventing activation in production.
When the guarded branch activates, emit a warning through the existing
session/server logger before assigning createDeterministicHandoffAdapter to
managed.agent.executionCwdRebind.
- Around line 2170-2174: Update loadMessagesFromDisk and the
createManagedSession restoration path to copy storedSession.handoffRuntimeState
into ManagedSession, preserving the persisted state—including
unverified/recovery-required—after restart instead of leaving it undefined.
---
Nitpick comments:
In
`@apps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.ts`:
- Around line 149-168: Extend the recoveryResultFromStatus tests with a case
using an active status that omits retainedSnapshotId, and assert the resulting
recovery-required object does not contain that key. Keep the existing
retainedSnapshotId-present case unchanged.
- Around line 189-253: Extend the “handoff dialog state machine” tests to cover
reducer guard branches: verify confirm leaves state unchanged from a non-preview
phase and for a blocked preview; name-changed is unchanged in loading, error,
and committed phases; recover is unchanged outside recovery-required; and
recovery-from-status is unchanged for a non-recovery-required result. Use deep
state equality assertions against the original state for each case.
In `@apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts`:
- Around line 100-111: Update the top-level type import from
`@kata-sh/shared/protocol` to include WorktreeHandoffStatus, then replace the
inline import() reference in recoveryResultFromStatus with that imported type
while preserving the existing Extract constraint.
- Around line 49-51: Remove the redundant canOfferHandoff helper and update its
callers to use the existing handoffCapable gate in HandoffAction.tsx;
alternatively, if the helper must remain, document it explicitly as a future
direction-filtering placeholder without treating it as a capability check.
- Around line 76-78: Update isRemoteOwnedPreview to derive preview ownership
from preview.source.serverId and preview.destination.serverId instead of
returning isRemoteWorkspace alone; alternatively remove the unused preview
parameter and rename the helper to reflect its actual behavior. Apply the
corresponding signature and call-site updates in HandoffDialog.tsx and the
affected test.
In `@e2e/tests/git/worktree-v2-handoff.spec.ts`:
- Line 75: Replace the localized role-based locator in the worktree handoff test
with the stable `handoff-cancel-button` test id, and add that `data-testid` to
the cancel `Button` in `HandoffDialog.tsx` so the test targets the intended
control across locales.
In `@packages/server-core/src/git/__tests__/handoff-runtime-gate.test.ts`:
- Around line 18-20: Update the test suite’s environment setup around beforeEach
so KATA_FEATURE_GIT_WORKSPACE_V1 is saved before being set and restored after
each test, following the existing save-and-restore pattern in the related
worktree-handoff tests. Ensure restoration handles the variable’s original unset
or defined state.
In `@packages/server-core/src/git/__tests__/worktree-handoff.test.ts`:
- Around line 541-552: Update the test `blocks when the managed worktree is
missing` to assert the exact `WorktreeHandoffBlockerCode` returned for a missing
source checkout, replacing the truthiness check with the service’s actual
missing-worktree blocker code. Keep the existing `p.blocked` presence assertion
and cleanup setup unchanged.
- Line 532: Update the top-level node:fs import to include chmodSync, mkdirSync,
and rmSync, then remove the redundant dynamic imports at the referenced test
locations, reusing the existing static bindings throughout
worktree-handoff.test.ts.
- Line 458: In the managedSession test setup, resolve the expected branch SHA
before the assertion using the guaranteed V2 record branch, then compare
headOf(harness.repo) directly with that resolved value. Remove the inline
ternary, empty-string fallback, and chained await from the expect expression.
- Around line 826-828: Make failingAdapter synchronous because
createDeterministicHandoffAdapter returns immediately; remove async and update
the call sites around lines 872 and 901 to stop awaiting it, or inline the
factory call while preserving the failRebind configuration.
In `@packages/server-core/src/git/managed-worktree-service.ts`:
- Around line 559-578: Extract the shared worktree destination derivation from
createWorktree and resolveWorktreePath into one private helper, including
workspaceId safe-component validation, materialization root resolution, repo-key
computation, display-fragment handling, and leaf construction. Update both
methods to call this helper so preview and creation produce the same path and
validation behavior.
In `@packages/server-core/src/git/worktree-journal.ts`:
- Around line 205-210: Add an operator-visible metric or inventory surface for
unresolved failed handoff entries, including their retained snapshot identifiers
or payload counts. Update the worktree journal/lifecycle reporting around
recover, compact, or gcOrphanedSnapshots without changing retention behavior:
failed entries must remain retained until explicitly recovered.
In `@packages/server-core/src/git/worktree-snapshot-service.ts`:
- Around line 731-734: Update the symlink handling in the snapshot restore flow
around the existing mode check to validate that entry.linkText is defined before
comparing it with readlinkSync(dest). For a missing linkText, raise the same
explicit malformed-manifest error used by restoreFileEntry; only perform the
existing destination-difference check when linkText is present.
In `@packages/server-core/src/handlers/rpc/git.test.ts`:
- Around line 472-509: The handoff RPC tests cover only enabled success paths
and omit disabled-flag and typed-error behavior. Extend the tests around the
shared handoff handlers to assert all four reject when V2_FLAG is unset, and add
coverage for throwTypedWorktreeHandoffError verifying WorktreeHandoffError
becomes a CodedError with WORKTREE_HANDOFF_ERROR_CODE while unrelated errors are
rethrown unchanged.
In
`@packages/shared/src/agent/backend/__tests__/deterministic-handoff-adapter.test.ts`:
- Around line 42-53: Rename the test around createDeterministicHandoffAdapter to
state that missing categories pass the capability handoff gate while the live
proof is incomplete, matching resolution.supported being true. Replace the
dynamic import of resolveHandoffCapability with a static top-level import,
consistent with handoff-capability.test.ts.
In `@packages/shared/src/agent/backend/__tests__/handoff-capability.test.ts`:
- Around line 117-126: Make the transcript-identity test exercise a real
mutation-capable path instead of relying on RecordingHandoffAdapter.sdkCwdWrites
being empty by construction. Update RecordingHandoffAdapter and its setup in
makeBackend so the adapter can reference the backend, then have
rebindExecutionCwd invoke the production update path and assert sdkCwd remains
unchanged; otherwise remove this test and add the assertion at the production
adapter/backend integration point.
In `@packages/shared/src/agent/backend/deterministic-handoff-adapter.ts`:
- Around line 20-26: Derive the missingChecks type from PROOF_CATEGORIES instead
of duplicating the string-union list in the options declaration. Update the
relevant type annotation near PROOF_CATEGORIES so adding a category
automatically updates accepted missingChecks values.
In `@packages/shared/src/agent/backend/handoff-capability.ts`:
- Around line 39-42: Update the capability check around handoffCapability() to
catch exceptions thrown by degraded adapters and return the documented {
supported: false, blocker: 'unsupported-provider' } result. Keep the existing
executionCwdRebindable validation unchanged for successfully returned
capabilities.
In `@packages/shared/src/agent/backend/types.ts`:
- Around line 338-344: Replace the unrestricted checks: string[] field in
ExecutionCwdProof with a union derived from a shared exported proof-category
constant containing file, shell, mcp, and provider. Follow the existing
WORKTREE_HANDOFF_BLOCKER_CODES and WorktreeHandoffBlockerCode pattern so
malformed check values are rejected at compile time.
In `@packages/shared/src/protocol/__tests__/worktree-handoff-contracts.test.ts`:
- Around line 20-23: Update the test covering WORKTREE_HANDOFF_DIRECTIONS to
assert the exact expected wire-value set in addition to its length and
uniqueness checks. Pin all three supported direction strings so renames or
substitutions fail the contract test.
- Around line 40-56: The test in the confirmation contract case currently
validates only omissions from a hand-written literal, not the runtime contract.
Replace the `'in'` checks and local key set around `WorktreeHandoffConfirmInput`
with assertions against the exported allowed-key source or sanitizer used by the
confirm handler, ensuring path, patch, and client-nominated path fields remain
rejected.
In `@packages/shared/src/protocol/worktree-handoff.ts`:
- Around line 66-107: Update the block comment above
WORKTREE_HANDOFF_BLOCKER_CODES to clarify that the tuple includes post-recovery
outcomes in addition to precondition blockers, while preserving the existing
handoff-rolled-back entry.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7220b6c5-6fff-4a50-b687-618d54a67753
⛔ Files ignored due to path filters (6)
apps/electron/resources/release-notes/next.mdis excluded by!**/*.mddocs/adrs/2026-08-07-conflict-safe-checkout-handoff.mdis excluded by!**/*.mddocs/adrs/index.mdis excluded by!**/*.mddocs/adrs/log.mdis excluded by!**/*.mddocs/index.mdis excluded by!**/*.mddocs/log.mdis excluded by!**/*.md
📒 Files selected for processing (42)
apps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.tsapps/electron/src/renderer/components/app-shell/handoff/HandoffAction.tsxapps/electron/src/renderer/components/app-shell/handoff/HandoffDialog.tsxapps/electron/src/renderer/components/app-shell/input/handoff-controls.tsapps/electron/src/renderer/components/right-sidebar/git-changes/ChangesPanel.tsxapps/electron/src/shared/types.tsapps/electron/src/transport/channel-map.tsapps/online-docs/core-concepts/git-worktrees.mdxe2e/tests/git/worktree-v2-handoff.spec.tspackages/server-core/src/git/__tests__/handoff-runtime-gate.test.tspackages/server-core/src/git/__tests__/worktree-handoff.test.tspackages/server-core/src/git/__tests__/worktree-lifecycle.test.tspackages/server-core/src/git/index.tspackages/server-core/src/git/managed-worktree-service.tspackages/server-core/src/git/worktree-handoff-service.tspackages/server-core/src/git/worktree-journal.tspackages/server-core/src/git/worktree-lifecycle-service.tspackages/server-core/src/git/worktree-snapshot-service.tspackages/server-core/src/handlers/rpc/git.test.tspackages/server-core/src/handlers/rpc/git.tspackages/server-core/src/sessions/SessionManager.tspackages/shared/src/agent/backend/__tests__/deterministic-handoff-adapter.test.tspackages/shared/src/agent/backend/__tests__/handoff-capability.test.tspackages/shared/src/agent/backend/deterministic-handoff-adapter.tspackages/shared/src/agent/backend/handoff-capability.tspackages/shared/src/agent/backend/index.tspackages/shared/src/agent/backend/types.tspackages/shared/src/i18n/locales/de.jsonpackages/shared/src/i18n/locales/en.jsonpackages/shared/src/i18n/locales/es.jsonpackages/shared/src/i18n/locales/hu.jsonpackages/shared/src/i18n/locales/ja.jsonpackages/shared/src/i18n/locales/pl.jsonpackages/shared/src/i18n/locales/zh-Hans.jsonpackages/shared/src/protocol/__tests__/worktree-handoff-contracts.test.tspackages/shared/src/protocol/channels.tspackages/shared/src/protocol/dto.tspackages/shared/src/protocol/index.tspackages/shared/src/protocol/routing.tspackages/shared/src/protocol/types.tspackages/shared/src/protocol/worktree-handoff.tspackages/shared/src/sessions/types.ts
Three product fixes surfaced by the credential-backed E2E: the renderer now receives a session_updated push when the agent is created (handoffCapable was never visible without a later refresh), a fresh preview supersedes a stale pending preview (React StrictMode double-mounts stranded the session with an unconfirmed transaction), and confirm stays disabled until the preview's branch matches the edited name (the debounced re-preview could otherwise confirm the previous name). The E2E spec now drives preview/confirm through the real provider-backed app and asserts the durable binding.\n\nRefs #42
Owner
Author
|
Updated with the E2E UI UAT evidence: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/tests/git/worktree-v2-handoff.spec.ts`:
- Around line 82-86: Update the onboarding path around completeApiKeyOnboarding
so raw API-key retrieval and transport are delegated to the shared credential
service under packages/shared/src/credentials/. Keep the E2E flow responsible
only for invoking the credential-service-backed onboarding behavior, without
reading, storing, or passing raw credential values in the test or renderer flow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: db50144d-7fc5-40ba-9b3d-0818ec4bdac2
📒 Files selected for processing (7)
apps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.tsapps/electron/src/renderer/components/app-shell/handoff/HandoffDialog.tsxapps/electron/src/renderer/components/app-shell/input/handoff-controls.tse2e/tests/git/worktree-v2-handoff.spec.tspackages/server-core/src/git/__tests__/worktree-handoff.test.tspackages/server-core/src/git/worktree-handoff-service.tspackages/server-core/src/sessions/SessionManager.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts
- packages/server-core/src/git/tests/worktree-handoff.test.ts
- apps/electron/src/renderer/components/app-shell/tests/handoff-controls.test.ts
- packages/server-core/src/sessions/SessionManager.ts
- apps/electron/src/renderer/components/app-shell/handoff/HandoffDialog.tsx
added 3 commits
August 7, 2026 12:08
CodeRabbit/Codex/greptile review fixes for the Phase 3 handoff PR: - Send proof gate: sessions that never performed a handoff are no longer blocked or marked recovery-required (handoffRuntimeState undefined short- circuits the gate); regression test added. - Preview fence: dismissing the handoff dialog cancels a pending preview transaction via the new git:handoffCancel RPC, so the session is never stranded fenced; re-preview works immediately. - Delete escape hatch: deleteSession allows a recovery-required handoff, rolling it back best-effort (cancelForSessionDeletion) before releasing the fence; pending handoffs still block deletion. - Rollback rebind: an interrupted confirm that already rebound the provider runtime is rebound back to the source checkout during recovery. - Deterministic adapter: gated on non-production NODE_ENV with a warning log. - Handoff fence message: localized as git.handoff.pendingFence (7 locales) and centralized in assertSessionHandoffNotFenced. - Leasing loop: per-session try/catch with repository-root cache so one unreadable directory cannot fence all lifecycle work. - runMutation: asserts gitCommonDir stability under the mutation lock. - managedToSession: guards resolveHandoffCapability against adapter throws. - pathToken shape validation, actual attempt count in the terminal creation error, and a shared leaf-derivation helper for createWorktree and resolveWorktreePath. - Renderer: recover-error keeps the recovery surface mounted, previewSeqRef increments instead of resetting, recoveryDialog resets on session change, committed body renders the destination branch, stable cancel test id. - Test hygiene: failingAdapter rename, failed-journal GC test name, pinned blocker code, static fs imports, env save/restore, channel routing + registration coverage for the new RPC, reducer guard-branch coverage. Refs #48, #42
…eset removeCapturedState runs `git reset --hard HEAD` before the source-cleaned marker was recorded. A failure mid-cleanup left the rollback with no restoration marker while the retained snapshot was still deleted, losing the captured tracked/staged state. The durable intent is now journaled before cleanup starts in both the current-to-managed and hand-back flows; rollback restores the source from the snapshot whenever cleanup was attempted (conditional on the source no longer matching the snapshot, so recording intent early is harmless when cleanup never mutated anything). Regression test injects a cleanup failure after the destructive reset and asserts recovery restores the captured state. Refs #48, #42
The previous assertion polled for >= 2 DOM matches of the deterministic token. The token is echoed in the user's own message bubble and session list previews, so two matches could occur without any model reply — a failed send passed the test. expectAssistantReply now: - reads the last assistant turn's response content (new stable assistant-turn test id + the existing response data attribute) and requires an exact match with the token by default; extra text, wrapping, or an error echo fails the test - waits for the app's processing flag to clear before judging, so a mid-stream pause is not mistaken for a settled wrong reply - fails fast with the error text when a chat error bubble appears (new chat-error-message test id) - keeps a 'contains' mode for flows that only need the turn to complete (worktree handoff quiescence) Refs #48
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/server-core/src/sessions/SessionManager.ts (2)
7231-7235: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftHandle handoff preflight failures before leaving the session processing.
Line 7235 can throw before the
tryblock at Line 7263. At that point, the user message is persisted andmanaged.isProcessingis alreadytrue. The rejection skipsonProcessingStopped, so later sends enter the queue path with no active chat to drain.Start the existing error-handling scope before
getOrCreateAgent()andverifyHandoffRuntimeBeforeSend(), or explicitly emit an error and callonProcessingStopped()on preflight failure. Add a regression test for a failed execution-CWD proof and assert that the session returns to idle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/sessions/SessionManager.ts` around lines 7231 - 7235, The session preflight path around getOrCreateAgent() and verifyHandoffRuntimeBeforeSend() must handle failures after processing begins. Move the existing error-handling scope to cover both calls, or explicitly log the failure and invoke onProcessingStopped() before propagating it; add a regression test for failed execution-CWD proof that verifies the session returns to idle.
5381-5383: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize handoff error messages.
These errors can reach the renderer through the send and handoff RPC flows. Replace the hard-coded strings with
i18n.t()keys. Add each key to all seven locale files in alphabetical order.As per coding guidelines, “All user-facing strings must go through
t()ori18n.t(). Translation keys must exist in all seven locale files and be alphabetically sorted.”Also applies to: 5432-5438, 5452-5458
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/sessions/SessionManager.ts` around lines 5381 - 5383, Replace the hard-coded handoff errors in the session lookup, execution-CWD validation, and corresponding flows around the referenced symbols with i18n.t() calls using new translation keys. Add every key to all seven locale files in alphabetical order, preserving the existing error behavior and messages.Source: Coding guidelines
apps/electron/src/renderer/components/right-sidebar/git-changes/ChangesPanel.tsx (1)
387-398: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the
HandoffDialogrender onsessionId.The reset effect at Line 130 clears
recoveryDialogwhensessionIdchanges, but effects run after the render commits. On the render pass wheresessionIdbecomesnull,recoveryDialogstill holds the previous session's transaction, soHandoffDialogmounts once withsessionId=''. The dialog's open effect bails on the empty ID, so no RPC is issued, but the empty-string fallback is unnecessary.🛠️ Proposed fix
- {recoveryDialog && ( + {recoveryDialog && sessionId && ( <HandoffDialog open - sessionId={sessionId ?? ''} + sessionId={sessionId}This repeats a point from an earlier review; the effect half was applied, the render guard was not.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/right-sidebar/git-changes/ChangesPanel.tsx` around lines 387 - 398, Guard the conditional HandoffDialog render with a valid sessionId in addition to recoveryDialog, so it cannot mount during the render where sessionId becomes null. Preserve the existing recoveryDialog reset effect and pass the actual sessionId without relying on the empty-string fallback.
🧹 Nitpick comments (4)
packages/server-core/src/handlers/rpc/git.ts (1)
523-529: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMap typed handoff errors on the status channel too.
HANDOFF_STATUSis the only handoff handler withoutthrowTypedWorktreeHandoffError. Ifgit.handoff.status()ever throws aWorktreeHandoffError(for example after journal-based restoration rejects malformed durable metadata), the client receives an untyped error while the other four channels receiveWORKTREE_HANDOFF_ERROR_CODE.♻️ Proposed change
assertWorktreeV2Enabled() - return git.handoff.status(input.sessionId) + try { + return await git.handoff.status(input.sessionId) + } catch (error) { + throwTypedWorktreeHandoffError(error) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/handlers/rpc/git.ts` around lines 523 - 529, Update the HANDOFF_STATUS handler to wrap git.handoff.status(input.sessionId) with throwTypedWorktreeHandoffError, matching the error mapping used by the other handoff RPC handlers while preserving its existing status result and Worktree V2 assertion.e2e/src/flows/agentChat.ts (1)
119-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the redundant
assistant-turncount.
responseis derived fromgetByTestId("assistant-turn").last().response.count()already returns0when no assistant turn exists, so the firstcount()call adds one extra round trip per poll without changing the outcome.♻️ Proposed change
- if ((await page.getByTestId("assistant-turn").count()) > 0 && (await response.count()) > 0) { + if ((await response.count()) > 0) { text = ((await response.textContent().catch(() => null)) ?? "").trim(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/src/flows/agentChat.ts` around lines 119 - 121, In the polling logic around the response locator, remove the redundant assistant-turn count check and rely solely on response.count() before reading textContent. Preserve the existing fallback and trimming behavior for empty or unavailable responses.packages/server-core/src/handlers/rpc/git.test.ts (1)
192-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
typedoverride apply to every handoff mock.The doc comment at Line 47 states that the override makes every handoff method throw typed or plain. Only
previewhonours'typed'.confirm,status,recover, andcancelthrow a plainError('boom')for both override values. As a result, the typed-mapping test at Line 562 covers onlyHANDOFF_PREVIEW, and a regression that dropsthrowTypedWorktreeHandoffErrorfromHANDOFF_CONFIRMorHANDOFF_RECOVERwould not fail.Extract a single throw helper and call it from all five mocks.
♻️ Proposed change
+ const maybeThrow = () => { + if (overrides?.handoffError === 'typed') { + throw new WorktreeHandoffError('HANDOFF_TRANSACTION_UNKNOWN', 'Unknown handoff transaction.') + } + if (overrides?.handoffError === 'plain') throw new Error('boom') + }Then replace each per-method conditional with
maybeThrow().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server-core/src/handlers/rpc/git.test.ts` around lines 192 - 218, In the handoff mock definitions, extract a shared maybeThrow helper that throws WorktreeHandoffError for the typed override and Error('boom') for the plain override, then invoke it from preview, confirm, status, recover, and cancel before each method’s normal return. Remove the individual per-method error conditionals while preserving each method’s call recording and response.apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts (1)
71-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isRemoteOwnedPreviewreturns its argument unchanged.The function body is
return isRemoteWorkspace. It adds an exported symbol, a test, and an import inHandoffDialog.tsxwithout any logic. If no further ownership rule is planned, use theisRemoteWorkspaceflag directly inPreviewBody. If a rule is planned, add a short comment stating what it will encode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts` around lines 71 - 74, Remove the redundant isRemoteOwnedPreview helper and its associated import/test, then use the existing isRemoteWorkspace flag directly in PreviewBody. If retaining the helper for a planned ownership rule, add a concise comment describing that rule instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/src/flows/agentChat.ts`:
- Around line 126-138: Update expectAssistantReply to track whether readTurnIdle
has observed an active processing state at least once, and only start idleSince
and evaluate the reply after that observation. Reset or maintain the tracking
state appropriately while polling so an initial idle result before processing
cannot trigger the mismatch error.
In `@packages/server-core/src/git/__tests__/lifecycle.test.ts`:
- Line 218: Add await to the services.handoff.status('stuck') resolves assertion
so the test waits for the promise and reports failures correctly, matching the
surrounding awaited assertions.
In `@packages/shared/src/protocol/worktree-handoff.ts`:
- Around line 67-72: Update the typed handoff outcome definitions around
WorktreeHandoffBlocked so the post-recovery success code handoff-rolled-back
cannot appear in preview or confirmation blocker payloads with outcome:
'blocked'. Keep it represented separately as a recoverable success/code
discriminant, while preserving the tuple as the single source of truth for
deriving valid codes.
---
Outside diff comments:
In
`@apps/electron/src/renderer/components/right-sidebar/git-changes/ChangesPanel.tsx`:
- Around line 387-398: Guard the conditional HandoffDialog render with a valid
sessionId in addition to recoveryDialog, so it cannot mount during the render
where sessionId becomes null. Preserve the existing recoveryDialog reset effect
and pass the actual sessionId without relying on the empty-string fallback.
In `@packages/server-core/src/sessions/SessionManager.ts`:
- Around line 7231-7235: The session preflight path around getOrCreateAgent()
and verifyHandoffRuntimeBeforeSend() must handle failures after processing
begins. Move the existing error-handling scope to cover both calls, or
explicitly log the failure and invoke onProcessingStopped() before propagating
it; add a regression test for failed execution-CWD proof that verifies the
session returns to idle.
- Around line 5381-5383: Replace the hard-coded handoff errors in the session
lookup, execution-CWD validation, and corresponding flows around the referenced
symbols with i18n.t() calls using new translation keys. Add every key to all
seven locale files in alphabetical order, preserving the existing error behavior
and messages.
---
Nitpick comments:
In `@apps/electron/src/renderer/components/app-shell/input/handoff-controls.ts`:
- Around line 71-74: Remove the redundant isRemoteOwnedPreview helper and its
associated import/test, then use the existing isRemoteWorkspace flag directly in
PreviewBody. If retaining the helper for a planned ownership rule, add a concise
comment describing that rule instead.
In `@e2e/src/flows/agentChat.ts`:
- Around line 119-121: In the polling logic around the response locator, remove
the redundant assistant-turn count check and rely solely on response.count()
before reading textContent. Preserve the existing fallback and trimming behavior
for empty or unavailable responses.
In `@packages/server-core/src/handlers/rpc/git.test.ts`:
- Around line 192-218: In the handoff mock definitions, extract a shared
maybeThrow helper that throws WorktreeHandoffError for the typed override and
Error('boom') for the plain override, then invoke it from preview, confirm,
status, recover, and cancel before each method’s normal return. Remove the
individual per-method error conditionals while preserving each method’s call
recording and response.
In `@packages/server-core/src/handlers/rpc/git.ts`:
- Around line 523-529: Update the HANDOFF_STATUS handler to wrap
git.handoff.status(input.sessionId) with throwTypedWorktreeHandoffError,
matching the error mapping used by the other handoff RPC handlers while
preserving its existing status result and Worktree V2 assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fab2e4b-bcf3-4164-ba0d-4d7bfd016093
⛔ Files ignored due to path filters (1)
apps/electron/resources/release-notes/next.mdis excluded by!**/*.md
📒 Files selected for processing (36)
apps/electron/src/renderer/components/app-shell/ChatDisplay.tsxapps/electron/src/renderer/components/app-shell/__tests__/handoff-controls.test.tsapps/electron/src/renderer/components/app-shell/handoff/HandoffDialog.tsxapps/electron/src/renderer/components/app-shell/input/handoff-controls.tsapps/electron/src/renderer/components/right-sidebar/git-changes/ChangesPanel.tsxapps/electron/src/shared/__tests__/ipc-channels.test.tsapps/electron/src/shared/types.tsapps/electron/src/transport/channel-map.tse2e/src/flows/agentChat.tse2e/tests/git/worktree-v2-handoff.spec.tspackages/server-core/src/git/__tests__/handoff-runtime-gate.test.tspackages/server-core/src/git/__tests__/lifecycle.test.tspackages/server-core/src/git/__tests__/worktree-handoff.test.tspackages/server-core/src/git/__tests__/worktree-lifecycle.test.tspackages/server-core/src/git/managed-worktree-service.tspackages/server-core/src/git/worktree-handoff-service.tspackages/server-core/src/git/worktree-snapshot-service.tspackages/server-core/src/handlers/rpc/git.test.tspackages/server-core/src/handlers/rpc/git.tspackages/server-core/src/sessions/SessionManager.tspackages/shared/src/agent/backend/__tests__/deterministic-handoff-adapter.test.tspackages/shared/src/agent/backend/__tests__/handoff-capability.test.tspackages/shared/src/agent/backend/deterministic-handoff-adapter.tspackages/shared/src/agent/backend/handoff-capability.tspackages/shared/src/agent/backend/types.tspackages/shared/src/i18n/locales/de.jsonpackages/shared/src/i18n/locales/en.jsonpackages/shared/src/i18n/locales/es.jsonpackages/shared/src/i18n/locales/hu.jsonpackages/shared/src/i18n/locales/ja.jsonpackages/shared/src/i18n/locales/pl.jsonpackages/shared/src/i18n/locales/zh-Hans.jsonpackages/shared/src/protocol/__tests__/worktree-handoff-contracts.test.tspackages/shared/src/protocol/channels.tspackages/shared/src/protocol/routing.tspackages/shared/src/protocol/worktree-handoff.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/shared/src/i18n/locales/zh-Hans.json
- packages/shared/src/protocol/routing.ts
- packages/shared/src/i18n/locales/en.json
- packages/shared/src/i18n/locales/de.json
- packages/server-core/src/git/tests/worktree-lifecycle.test.ts
- packages/shared/src/i18n/locales/hu.json
- packages/shared/src/i18n/locales/ja.json
- packages/shared/src/i18n/locales/pl.json
- e2e/tests/git/worktree-v2-handoff.spec.ts
- packages/shared/src/agent/backend/tests/deterministic-handoff-adapter.test.ts
- packages/server-core/src/git/tests/handoff-runtime-gate.test.ts
- apps/electron/src/renderer/components/app-shell/tests/handoff-controls.test.ts
- apps/electron/src/transport/channel-map.ts
- packages/shared/src/agent/backend/tests/handoff-capability.test.ts
- packages/server-core/src/git/worktree-snapshot-service.ts
- packages/shared/src/protocol/tests/worktree-handoff-contracts.test.ts
- apps/electron/src/renderer/components/app-shell/handoff/HandoffDialog.tsx
- packages/shared/src/agent/backend/deterministic-handoff-adapter.ts
- packages/server-core/src/git/managed-worktree-service.ts
- packages/shared/src/i18n/locales/es.json
- Send preflight failures (agent build, execution-CWD proof, source build) now run inside the error-handling scope, so a failed proof stops processing and the session returns to idle instead of staying stuck with no active chat to drain; regression test covers the failed-proof path (PR #48 review) - Localize the three handoff error messages that reach the renderer (executionCwdRequired, runtimeReconstructionUnavailable, runtimeVerificationFailed) with i18n keys in all seven locales - Split post-recovery outcome codes (handoff-rolled-back) out of the precondition blocker tuple so they cannot appear in preview or confirmation blocker payloads; contracts test updated - Map typed WorktreeHandoffError on the HANDOFF_STATUS channel too, and make the typed override apply to every handoff mock so the mapping is covered for all five channels - Guard the ChangesPanel HandoffDialog render on sessionId instead of the empty-string fallback; drop the pass-through isRemoteOwnedPreview helper - E2E: expectAssistantReply only arms the idle mismatch after observing active processing (fixes flake on slow sends); drop the redundant assistant-turn count poll - Await the handoff status assertion in the deleteSession lifecycle test
Owner
Author
|
Addressed the remaining findings from the 2026-08-07 CodeRabbit review in commit 4e61fc3: Outside-diff comments
Nitpicks
All affected suites pass (shared protocol contracts, server-core git/handlers/sessions, electron handoff-controls), typechecks green for shared/server-core/electron, and i18n lints pass. |
gannonh
deleted the
feat/worktree-v2-phase-3-conflict-safe-checkout-hando
branch
August 7, 2026 23:08
16 tasks
13 tasks
This was referenced Aug 8, 2026
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
1 participant
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.
Build complete
Branch:
feat/worktree-v2-phase-3-conflict-safe-checkout-handoBase SHA:
18fa6867e419c5c8cd194b4f0c8dab2ca62c2a66(main)Head SHA:
07f2491(13 commits, e7529c6..07f2491)Tasks delivered (each with spec-compliance + code-quality review gates)
ExecutionCwdRebindCapability,resolveHandoffCapability)snapshottedfor hand-backrecover()), idempotent across crashes,handoff-rolled-backtyped blocker, failed-journal re-fence fix, orphan-GC protectionrecovery-requiredblocks Send), persistenthandoffRuntimeStatedocs/adrs/2026-08-07-conflict-safe-checkout-handoff.md), docs index/log updates, online-docs git-worktrees handoff section, release notes@worktree-v2 handoffspec +KATA_HANDOFF_DETERMINISTIC_ADAPTERseam; AC-15 blocker/state-transfer coverage testsVerification evidence
packages/shared:bun test src/git59 pass;tsc --noEmitclean; handoff contracts/capability/deterministic-adapter 21 pass.packages/server-core: 9-file suite (handoff 41, snapshot 23, lifecycle 52, managed-worktree, mutation-lock, reconcile, turn-status-refresh, rpc git, headless-server-flow) 188 pass; runtime-gate 8 pass;bun run typecheckclean.apps/electron: app-shell + right-sidebar 204 pass;bun run typecheckclean.bun run lint:i18n:parity(7 locales, 1748 keys) +lint:i18n:sortedclean.bun run e2e --list --grep '@worktree-v2.*handoff'lists the spec; the live run requires a provider credential (see Deferred: handoff UI UAT and credentialed provider UAT for Worktree V2 Phase 3 #47).git diff --checkclean; working tree clean.Reviews
Every task passed spec-compliance + code-quality review (subagent reviewer); Important findings were fixed and re-reviewed: tracked-
.worktreeincludedata-loss edge, fail-open binding hook, phantom restart re-fence, m2c duplicate-state residue, client capability gating, dead spinner/name-edit dialog states. Final whole-branch review: PASS_WITH_FINDINGS — findings addressed (E2E test 2 rewritten out, AC-15 coverage closed).Approved deviations / notes
binding-committedwithout a journal commit marker stays explicitrecovery-required(ambiguous by design).status:verifiedlabel (reported earlier as hygiene; non-blocking).PR: https://github.com/gannonh/kata-agents/pull/NN (replace with actual)
Summary by CodeRabbit
New Features
Documentation
Tests
Greptile Summary
The checkout-handoff changes now preserve source edits when cleanup is interrupted and avoid applying handoff runtime requirements to sessions that have never moved between checkouts.
Focused execution checks disproved the previously reported normal-send failure: a session without handoff state completed the pre-send path without an execution-CWD adapter. They also disproved the cleanup recovery failure: after a destructive reset and injected cleanup failure, recovery restored both modified tracked content and untracked files from the retained snapshot.
Confidence Score: 5/5
Safe to merge based on the exercised runtime-gate and interrupted-cleanup recovery paths.
No blocking failure remains.
What T-Rex did
Reviews (5): Last reviewed commit: "fix(worktree): address CodeRabbit findin..." | Re-trigger Greptile