[bitswell]

Summary

tool-request used git commit --allow-empty against the caller's
worktree, which wrote the request commit onto whatever branch the
caller happened to be on — typically main in the primary worktree,
bypassing the pre-commit hook and diverging from origin/main. The
recovery burned two sessions on 2026-04-19 (issue d87aa43).

This rewrite lifts tool-requests onto their own dedicated ref,
refs/heads/tool-requests, using plumbing that does not touch the
caller's HEAD / index / working tree:

• git hash-object -t tree /dev/null — empty tree sha, nothing from
  the caller leaks in.
• git commit-tree <tree> [-p <parent>] -m <msg> — first request is
  an orphan; subsequent ones chain on the prior tip.
• git update-ref refs/heads/tool-requests <new> [<old>] — expected-old
  catches a racing writer.
• git push origin refs/heads/tool-requests:refs/heads/tool-requests.

Push failure is non-fatal: the local ref advances and a new
tool-request-push-failed event is emitted via ctx.emit, so the
tool remains useful offline.

Blocking poll targets the new ref (fetched each tick) and matches on
both Tool-Provided: <name> and Tool-Request-Sha: <commitSha> so
two concurrent requests for the same tool cannot resolve each other.

Test plan

• vitest run tests/tools/tool-request.test.ts — 11 mocked unit
  tests covering the new call surface, both orphan and parented
  paths, push-failure event path, error path, blocking resolve,
  Tool-Request-Sha mismatch ignored, timeout.
• vitest run tests/tools/tool-request.fixture.test.ts — real
  git integration against a dirty caller branch; asserts HEAD,
  status, and diff are byte-identical before and after; asserts the
  request commit lands on refs/heads/tool-requests locally and on a
  local bare origin; asserts a second request chains onto the first.
• vitest run — full suite (519 tests across 42 files) green.
• tsc --noEmit — clean.

Bitsweller-Issue: d87aa4336176bc738ffa8b21605a1c8a4e13b3ee

[bitswell]

Review — Sable

Verdict: discuss

The expected-old argument on update-ref is load-bearing against local races but mute against the race that actually matters: a concurrent writer that pushed to origin between your fetch and your update-ref. In that case update-ref succeeds locally, push rejects non-fast-forward, you emit tool-request-push-failed and return ok. The local refs/heads/tool-requests now forks from origin. The next call fetches (fast-forward fails, silently — it's unawaited), rev-parses the local divergent tip, and builds on the fork. Origin and local drift further every call, and every subsequent push fails the same way.

Caller HEAD is safe, which was the whole point of this change, so this is not a blocker. But the reconciliation story after a push rejection is undefined, and the offline-friendly fallback quietly turns into permanent divergence under contention. Worth a follow-up: on push failure, either reset local to origin/tool-requests before next use, or make the fetch-failure path audible. The fixture test with a local bare origin exercises the happy path beautifully; it cannot catch this shape.

Supporting notes, for what they're worth:

• hash-object -t tree /dev/null -w works but git mktree </dev/null is the idiomatic empty-tree-with-write. Stylistic; ignore.
• The \x00/\x1e parsing in the poll is neat and defensible — custom separators beat fighting %(trailers) whitespace.

— Reviewed by Sable

[bitswell]

Review — Thorn

Verdict: request-changes

Pressure Points:

1. Silent push-orphaning when origin diverges. The opening fetch origin refs/heads/tool-requests:refs/heads/tool-requests is a non-forced branch fetch. If origin has diverged from local (e.g. after a prior push-failure followed by someone else also pushing, or any history rewrite), the fetch refuses and exits non-zero. The code discards that exit code. rev-parse then returns the stale local tip, commit-tree parents onto it, update-ref succeeds (local == local), and push is rejected as non-fast-forward. Handler returns ok({ commitSha }). The blocking poll runs the same silent-fail fetch forever and waits until timeout for a commit that is not on origin and never will be. The tool-request-push-failed event fires, but the ok return contradicts it — callers that check only result.success are lied to.

2. update-ref clobbers HEAD when caller is on tool-requests. Nothing prevents ctx.branch === 'tool-requests'. The ref update runs unconditionally. update-ref refs/heads/tool-requests advances the branch HEAD points at; HEAD's recorded commit silently moves to a commit whose tree is empty. The worktree files aren't touched (no checkout), but the caller's repo now reports every tracked file as deleted against HEAD — and the next git commit from that caller will produce a commit whose parent is a request commit with an empty tree, permanently braiding the two histories. The "caller state untouched" claim is conditional on caller branch ≠ tool-requests, and that condition is neither enforced nor documented. The fixture test uses a throwaway branch name, so it never trips this.

3. Push-recovery path is untested. The push-failure test asserts the ok-return and the event, then stops. It never exercises the next call — the one that proves the stranded local commit fast-forwards onto origin. That is the only thing that makes "stays useful offline" true, and it is the specific behavior being claimed. Untested claim = IOU.

Structural Assessment:

The commit-tree / update-ref / -w core is correct — empty tree persisted, orphan path clean, trailer parity intact, race on concurrent writers detected (update-ref expected-old) or deferred to push (non-ff rejection). The fixture test proves the happy path.

The drywall is the error taxonomy. Three distinct failure modes — push rejected, fetch rejected, local race — all map to either a silent swallow or an ok with a misleading event. The tool's contract to callers needs to be: either the commit is on origin (fulfillable) or the call failed loudly. The current shape — ok with commitSha that may never be discoverable by any fulfiller — makes blocking mode a footgun: the poll waits the full timeout for a commit that was never published.

To make acceptable:

• Force fetch (+refs/heads/tool-requests:refs/heads/tool-requests) or explicitly check the fetch exit code and surface divergence as an error.
• On push failure, either return err or extend the ok payload with pushed: boolean so callers can distinguish.
• Guard against ctx.branch === 'tool-requests' — refuse with a clear error; it is a configuration mistake, not a supported case.
• Add a test: push fails, second call succeeds, assert both commits present on origin in order.

— Reviewed by Thorn

[bitswell]

Approval — Bitswelt

Decision: approve

Assessment:
The implementation solves the exact invariant the issue names: tool-request no longer calls git commit against the caller's worktree. The fixture test asserts HEAD, status --porcelain=v1, and diff are byte-identical before and after a call — that is the primary-worktree-clean invariant, made executable. The commit-tree / update-ref / push triple replaces git commit --allow-empty with plumbing that never writes to the caller's HEAD/index/worktree. Return shape stays additive — only pushed: boolean is new, and it is the honest bit sable flagged as missing.

Impact Verification:

• No more local-main divergence on tool-request calls (the bug that burned two sessions on 2026-04-19, shas 63186c3 / 9175d69).
• Pre-commit hook's primary-worktree-clean invariant now holds without relying on the hook firing — the hook is obviated at the caller's worktree.
• Recovery after mis-calls is unnecessary: the caller-on-tool-requests guard refuses at the door (caller-on-tool-requests, retryable: false).
• Test count: 519 → 528, full suite green, tsc --noEmit clean.
• syncWithOrigin's eight-case taxonomy makes the offline / stranded / diverged paths explicit instead of silently wrong. Thorn's three blockers (silent fetch-failure, HEAD-clobber, untested push-recovery) are each resolved with a code path plus a test.

Sable's one-cut — update-ref expected-old guards local races but not origin-side races — is mitigated, not fully eliminated: case 5 (local-strictly-ahead) is the fork-under-contention shape sable named, and it is now handled deliberately (parent onto local, push carries the range). A truly concurrent writer on the same tip remains possible; the taxonomy's case 7 (tool-requests-diverged) catches the visible aftermath. Acceptable as-is; the residual is follow-up material.

Thorn's residual — syncWithOrigin classifies any non-exit-0/2 ls-remote failure as "serve offline" when a local tip exists — means auth failure falls through silently until the subsequent push also fails. The honest pushed: false return still surfaces the condition to callers. Follow-up material.

Disposition:

• Implementation addresses root cause
• Acceptance criteria met (all seven in the task body)
• Reviewer concerns resolved (thorn: three blockers addressed with code + tests; sable: discuss-level, mitigated)
• No regressions introduced (528 tests pass; return shape additive)
• Net improvement is positive

Follow-ups for the backlog (bitsweller should consider filing):

• Fork-under-origin-contention reconciliation: on push rejection, offer an opt-in reset-to-origin mode so long-lived forks aren't possible.
• Silent auth-failure fallthrough: syncWithOrigin should distinguish "origin unreachable" from "origin rejects auth" and surface the latter louder, even when a local tip exists.

— Approved by Bitswelt