feat(server): package and apply a thread's git work#16734

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the repository root when creating the snapshot.

pack runs git add --all -- . with input.cwd. If input.cwd is a subdirectory, Git stages only changes under that directory. Changes elsewhere remain outside the snapshot without an error.

✅ Confirmed as addressed by @juliusmarminge

Fixed in b074ad6bf9 with the :/ pathspec, so the snapshot covers the whole working tree wherever the thread's checkout sits. A new test packs from a subdirectory and checks edits above it arrive; it fails with ..

@juliusmarminge Thanks for the fix. The :/ pathspec addresses the snapshot scope issue, and the reported regression test covers packing from a subdirectory.

✅ Review thread resolved.

You are interacting with an AI system.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Keep the underlying failure as cause and do not build the message from it.

HandoffGitError has no cause field. Line 108 also builds the message from error.message. The service guidelines require that a wrapping error keeps the immediate underlying error as cause. They also require that the message is never built from cause.message. Add a required or optional cause: Schema.Defect field and use a fixed message with the git subcommand.

As per coding guidelines: "The message is fixed or built from those attributes, never from cause, cause.message" and "An error that wraps a failure keeps the immediate underlying error as cause."

Source: Coding guidelines

✅ Confirmed as addressed by @juliusmarminge

Fixed in b074ad6bf9. HandoffGitStepError already had cause; the message no longer copies cause.message and is just git <step> failed.. Git's own reason goes to the server log (handoff-git.step-failed).

@juliusmarminge Thanks for the update. The fixed message addresses the remaining concern. Keeping cause on HandoffGitStepError and logging Git’s reason separately preserves the diagnostic details.

✅ Review thread resolved.

You are interacting with an AI system.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Sanitize inherited Git environment variables before running handoff commands.

When the server starts with repository-scoped variables such as GIT_DIR, GIT_WORK_TREE, or GIT_INDEX_FILE, HandoffGit can run Git against the wrong repository. Calls without env inherit the server environment. Calls with env also retain it because ProcessRunner enables extendEnv.

Run handoff Git commands with a sanitized environment and disable environment extension for that process, or add an equivalent VcsProcess option. Removing keys only from the object merged at line 98 is not sufficient because ProcessRunner re-adds the parent environment.

Source: Learnings

✅ Confirmed as addressed by @juliusmarminge

Fixed in b074ad6bf9: every handoff git command now runs with GIT_DIR, GIT_WORK_TREE, GIT_COMMON_DIR, GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_NAMESPACE and GIT_CEILING_DIRECTORIES unset. The spawner merges the env over process.env with these keys set to undefined, so they're dropped. A new test sets GIT_DIR to another clone and packs; it fails without the fix.

@juliusmarminge Thanks for clarifying. I verified the environment overrides and the new GIT_DIR regression test in the current code. The shared Git helper clears the inherited variables while preserving the explicit private-index override.

Setting these keys to undefined is sufficient with the spawner’s merge behavior; disabling extendEnv is not required. I did not run the test.

✅ Review thread resolved.

You are interacting with an AI system.

🟠 High handoff/HandoffGit.ts:319

If a later apply step fails after creating the branch, rollback can delete a branch another process has advanced, removing reachability of its commits. The deletion omits an expected old OID, so git update-ref does not verify that the branch still points to input.tip; pass input.tip as the expected OID.

Fixed in b074ad6bf9: the rollback deletes the branch only if it still points at the tip. A new test has a post-checkout hook move the new branch and fail the checkout; the branch keeps the hook's commit. It fails without the expected OID.

🟠 High handoff/HandoffGit.ts:201

When cwd is a repository subdirectory, pack omits edits, deletions, and new files outside that directory, so the successful handoff restores an incomplete working tree. Because the private index starts at HEAD, git add --all -- . updates only the . pathspec; remove the pathspec so git add --all stages the entire working tree.

Fixed in b074ad6bf9, same root cause as the thread above. I used :/ rather than dropping the pathspec, so it stays explicit. A new test packs from a subdirectory; it fails without the fix.