Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3588bde
chore(porch): 1018 init pir
amrmelsayed Jun 23, 2026
0e85a8e
[PIR #1018] Plan draft
amrmelsayed Jun 23, 2026
7b44c3d
chore(porch): 1018 plan-approval gate-requested
amrmelsayed Jun 23, 2026
d09862f
chore(porch): 1018 plan-approval gate-approved
amrmelsayed Jun 23, 2026
fa65ec7
chore(porch): 1018 implement phase-transition
amrmelsayed Jun 23, 2026
93f455d
[PIR #1018] Add worktree write-guard hook for Claude builders
amrmelsayed Jun 23, 2026
d5ec715
[PIR #1018] Test the write-guard script and CLAUDE_HARNESS wiring
amrmelsayed Jun 23, 2026
84b2f7b
[PIR #1018] Document worktree path-discipline backstop in builder role
amrmelsayed Jun 23, 2026
10aa609
[PIR #1018] Update builder thread for implement phase
amrmelsayed Jun 23, 2026
636a2b8
chore(porch): 1018 dev-approval gate-requested
amrmelsayed Jun 23, 2026
396ced7
chore(porch): 1018 dev-approval gate-approved
amrmelsayed Jun 23, 2026
6c0b363
chore(porch): 1018 review phase-transition
amrmelsayed Jun 23, 2026
622d895
[PIR #1018] Review + retrospective
amrmelsayed Jun 23, 2026
b736b9c
chore(porch): 1018 record PR #1098
amrmelsayed Jun 23, 2026
c3b3d2b
chore(porch): 1018 review build-complete
amrmelsayed Jun 23, 2026
d395fc4
[PIR #1018] Install write-guard for all Claude spawn modes (addresses…
amrmelsayed Jun 23, 2026
764a75e
[PIR #1018] Record 3-way consultation outcome in review
amrmelsayed Jun 23, 2026
cbae10b
chore(porch): 1018 pr gate-requested
amrmelsayed Jun 23, 2026
86cac57
[PIR #1018] Thread: record consultation outcome + no-role fix
amrmelsayed Jun 23, 2026
dfd0f26
[PIR #1018] Fix Linux CI: anchor guard test fixtures outside /tmp (al…
amrmelsayed Jun 23, 2026
b0f23ff
[PIR #1018] Record Linux CI fix + CMAP-3 re-run (iter2) in review/thread
amrmelsayed Jun 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions codev-skeleton/roles/builder.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,27 @@ Builders may submit multiple sequential PRs within a single worktree session. Th
- **Worktree cleanup is architect-driven** -- the architect decides when to run `afx cleanup`, not the builder
- If a builder session is interrupted, use `afx spawn XXXX --resume` to reconnect to the existing worktree

## Worktree isolation: filesystem path discipline

Your worktree (`.builders/<id>/`) is **nested inside the main checkout**, and at the
branch base the two trees are **byte-identical**. This creates a silent failure mode:

- The `Write`/`Edit` tools require **absolute** paths. If you synthesize one rooted
at the canonical repo root instead of your worktree, you drop the `.builders/<id>/`
segment and write into the **main checkout** — a real, writable directory. The
write *succeeds silently* and pollutes `main`; you only notice later when a
`git add` in your worktree fails with a pathspec error.
- Wrong-rooted **reads** also succeed silently (identical trees), so nothing
corrects the mistake until that first failed write.

Rules:
- **Absolute paths for Write/Edit must be rooted at your worktree.** A deterministic
PreToolUse guard now blocks out-of-worktree writes (allowing only temp dirs and
`~/.claude`); if you see that denial, re-root the path under your worktree.
- **Bash `cwd` is your worktree — prefer relative paths there.** A relative path
cannot be anchored to the wrong root, which closes the Bash write surface
(`>`, `cp`, `tee`, `sed -i`) the Write/Edit guard does not cover.

## Constraints

- **Stay in scope** - Only implement what's in the spec
Expand Down
207 changes: 207 additions & 0 deletions codev/plans/1018-builder-worktree-write-guard-p.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
# PIR Plan: Builder worktree write-guard (deterministic Write/Edit boundary)

## Understanding

A strict-mode builder runs in an isolated git worktree at `.builders/<id>/`, which
is nested inside the main checkout. The `Write`/`Edit` tools require **absolute**
paths, so the builder model must *synthesize* one. The current model/CLI runtime
sometimes anchors that path at the inferred **canonical repo root** instead of its
actual worktree `cwd`, dropping the `.builders/<id>/` segment. Because the worktree
is nested in the main checkout, the wrong path is a real writable directory, so the
mis-write **succeeds silently** — the file lands in the main checkout's working
tree. Byte-identical trees at branch base mean wrong-rooted *reads* also succeed
silently, so nothing corrects the model until a later `git add` in the worktree
fails with a pathspec error.

Per the issue, this is **not a codev code regression** — it is intrinsic
path-synthesis behavior of the builder runtime that drifts across model/CLI
upgrades. Instructions, per-agent memory, and bisects do not hold against a moving
runtime. **Only a deterministic guard holds across versions.** That is the
deliverable.

This plan implements the issue's two-part fix:
1. A **PreToolUse hook on Write/Edit** that rejects absolute paths resolving
outside the worktree root (the real fix — model-proof, deterministic).
2. A **role-doc backstop** in `roles/builder.md` naming the failure mode (the
cheap, drift-fragile secondary control).

### Where the relevant code lives (verified)
- `packages/codev/src/agent-farm/commands/spawn-worktree.ts`
- `startBuilderSession()` (≈724) — builds `.builder-start.sh`, which `cd`s into
the worktree and launches the bare builder command. **No `--settings` flag.**
- `writeWorktreeFiles(files, worktreePath)` (≈679) — the existing mechanism that
writes per-worktree files (used today only for OpenCode's `opencode.json`).
Uses `writeFileSync` with **no `mkdir`**, and `git update-index --skip-worktree`
so generated files are not committed.
- `packages/codev/src/agent-farm/utils/harness.ts`
- `HarnessProvider.getWorktreeFiles?(roleContent, roleFilePath)` (≈44) — optional
hook for file-based worktree config. **Does not currently receive the worktree
path.** `CLAUDE_HARNESS` (≈62) does not implement it today.
- `codev/roles/builder.md` and `codev-skeleton/roles/builder.md` — byte-identical
(8623 bytes); both must be edited (dual-tree mirror rule).
- No `.claude/settings.json` is committed anywhere in the repo; a fresh worktree's
`.claude/` contains only `skills/`.

## Proposed Change

### Part 1 — Deterministic PreToolUse guard (the real fix)

**Mechanism chosen:** generate a per-worktree `.claude/settings.local.json` plus a
self-contained Node guard script `.claude/hooks/worktree-write-guard.cjs` at spawn
time, via the existing `getWorktreeFiles` → `writeWorktreeFiles` path, routed
through `CLAUDE_HARNESS` (so it respects the harness abstraction and only applies
to Claude builders — `PreToolUse` is a Claude Code concept).

**Why a Node `.cjs` and not bash+jq:** Node is guaranteed present (codev/claude run
on it); `jq` is not. A `.cjs` is portable, has zero runtime deps, and is directly
testable by spawning it with fixture stdin.

**Why generated-into-the-worktree (not symlinked / not `$CLAUDE_PROJECT_DIR`):**
the published npm package must carry the guard so every adopter gets it
automatically. Emitting it from package code (a TS string constant) ships it for
free with no asset-copy build step. The settings file references the script by an
**absolute** path baked at spawn time, removing all ambiguity about the hook's cwd
or `$CLAUDE_PROJECT_DIR` semantics.

**Worktree-root resolution (deterministic, with self-healing fallback):** the
spawn-time generator knows the worktree absolute path, so it bakes it into the hook
command as `CODEV_WORKTREE_ROOT=<abs>`. The guard reads that env var as its primary
source and falls back to `git rev-parse --show-toplevel` (which returns the
worktree, not the main checkout) if the env var is ever absent. The issue suggests
`git rev-parse`; baking the value in is strictly more deterministic, with
`git rev-parse` retained as the documented fallback.

**Guard decision logic:**
1. Read the PreToolUse JSON from stdin; act only on `tool_name` ∈ {`Write`,
`Edit`}; for any other tool, exit 0 (allow).
2. Read `tool_input.file_path`; if absent, exit 0.
3. Resolve the target to a canonical absolute path **without requiring it to
exist**: make absolute against the hook's `cwd`, then resolve the longest
existing ancestor with `fs.realpathSync` and re-append the non-existent tail.
This handles new files/dirs *and* symlink normalization (macOS `/tmp` →
`/private/tmp`).
4. Resolve the worktree root the same way (canonicalized).
5. **Allow** if the target is the worktree root or inside it (`root` or
`root + sep` prefix).
6. **Allow** if the target is inside an allowlisted dir (see below).
7. Otherwise **deny** via the JSON protocol: exit 0 with
`{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny",
"permissionDecisionReason":"<msg naming the worktree root>"}}`. The reason names
the worktree root and the offending path so the model re-roots immediately.

**Allowlist (deliberate):**
- Temp dirs: `/tmp`, `/private/tmp`, and `$TMPDIR` (canonicalized) — legitimate
scratch writes (incl. the session scratchpad under `/private/tmp/...`).
- `$HOME/.claude` — so builder **memory** writes (`~/.claude/projects/<slug>/memory/`)
and Claude config writes are not blocked. This is the one out-of-worktree write
builders legitimately perform; it is user config, never the main checkout.

Everything else outside the worktree — crucially the main checkout's `codev/...`,
sibling worktrees, and shared root config reached via symlink — is denied.

### Part 2 — Role-doc backstop

In **both** `codev/roles/builder.md` and `codev-skeleton/roles/builder.md`, add an
invariant that names the *failure mode* (not just "cwd is the worktree"):
- A byte-identical sibling main checkout exists; wrong-rooted reads succeed
silently and mask the error until a write fails, so absolute paths for Write/Edit
must be rooted at the worktree.
- Bash `cwd` is the worktree — **prefer relative paths there** (a relative path
cannot be anchored to the wrong root, closing the Bash surface the Write/Edit
hook does not cover).

## Files to Change

**New:**
- `packages/codev/src/agent-farm/utils/worktree-write-guard.ts` — exports
`WORKTREE_WRITE_GUARD_SCRIPT` (the self-contained `.cjs` source as a string,
single source of truth) and a small `buildWorktreeGuardFiles(worktreeAbsPath)`
helper returning the `{relativePath, content}[]` for the settings + script.
- `packages/codev/src/__tests__/worktree-write-guard.test.ts` — unit tests that
write the script to a tmp file and spawn `node` against it with fixture stdin.

**Modified:**
- `packages/codev/src/agent-farm/utils/harness.ts`
- Extend `getWorktreeFiles?(roleContent, roleFilePath, worktreePath?)` (add the
worktree path; backward compatible — OpenCode ignores it).
- Implement `CLAUDE_HARNESS.getWorktreeFiles` to return the guard files via
`buildWorktreeGuardFiles(worktreePath)`.
- `packages/codev/src/agent-farm/commands/spawn-worktree.ts`
- `writeWorktreeFiles`: add `mkdirSync(dirname(targetPath), {recursive:true})`
before writing (needed for `.claude/hooks/`).
- Pass `worktreePath` to the `harness.getWorktreeFiles(...)` call (≈775).
- `codev/roles/builder.md` — add the backstop invariant.
- `codev-skeleton/roles/builder.md` — identical edit (mirror).

## Risks & Alternatives Considered

- **Risk: false-positive blocks a legitimate out-of-worktree write.** Mitigated by
the allowlist (`$HOME/.claude` for memory/config; temp dirs for scratch). Reads
are untouched, so codev's intentional cross-checkout reads (architect reads
builder threads, sibling-thread reads) keep working. Symlinked shared root config
(`.codev/config.json`, `.env`) resolves outside the worktree and is **denied** —
this is judged correct (a builder should not mutate shared root config), but it
is a behavior change worth a human's eyes at the gate.
- **Risk: non-existent target path breaks realpath.** Mitigated by the
longest-existing-prefix resolution (resolve the existing ancestor, append the
tail) rather than realpath on the full path.
- **Risk: `.claude/hooks/` doesn't exist → `writeFileSync` throws.** Mitigated by
adding `mkdir -p` to `writeWorktreeFiles`.
- **Risk: generated files committed into the builder PR.** Mitigated by
`git update-index --skip-worktree` (already in `writeWorktreeFiles`) and the
builder's explicit-staging discipline. They sit as untracked infra files
alongside the existing `.builder-*.{sh,txt,md}` files.
- **Scope limitation: non-Claude harnesses (codex/gemini) get no guard.** Accepted
— `PreToolUse` is Claude-specific and the bug is observed on Claude. Noted, not
fixed here.
- **Out of scope: full per-builder filesystem isolation.** Per the issue, this
fights codev's shared-filesystem model (worktrees share the object DB; codev
relies on cross-checkout reads + symlinked root config). The right answer for a
future cloud/sandbox model, not local builders today.
- **Related but out of scope: #1092** (consult `impl-review` sub-agent reads from
the outer checkout). Same root cause, **read** surface, lower severity
(self-heals via the SDK's not-found cwd note). A sibling-architect comment
proposes a unified guard covering both via two call sites; an earlier comment
argues for keeping them scoped separately. **This plan stays scoped to #1018's
Write/Edit surface** but factors the guard into `worktree-write-guard.ts` so the
same logic could later be wired as a `PreToolUse` hook on the consult SDK
`query()` for #1092. **Decision deferred to the human at the plan-approval gate.**
- **Alternative rejected — bash+jq guard:** `jq` is not guaranteed present.
- **Alternative rejected — symlink a static settings.json from root:** the hook
command needs the per-builder worktree root; a static file cannot carry it, and
it would not ship to adopters.
- **Alternative rejected — `$CLAUDE_PROJECT_DIR`-relative command:** its resolution
vs `cwd` is not guaranteed; the baked absolute path is unambiguous.

## Test Plan

**Unit (`vitest`, run from the worktree):** write `WORKTREE_WRITE_GUARD_SCRIPT` to a
tmp `.cjs`, create a fake worktree dir, spawn `node tmp.cjs` with fixture stdin and
assert exit/stdout:
- Write **inside** the worktree → allow (exit 0, no deny JSON).
- Write to a **main-checkout path outside** the worktree (e.g. the worktree's
parent `/codev/plans/x.md`) → **deny** (deny JSON, reason names the worktree root).
- Write to a **new nested** file whose parent dir does not yet exist, inside the
worktree → allow (longest-prefix resolution).
- Write to `/tmp/...` and `/private/tmp/...` → allow (temp allowlist + macOS
symlink normalization).
- Write to `$HOME/.claude/projects/.../memory/x.md` → allow.
- `Edit` tool → same as Write.
- Non-Write/Edit tool (e.g. `Bash`) → allow (untouched).
- `CODEV_WORKTREE_ROOT` unset → falls back to `git rev-parse` in a real git
worktree fixture.
- `buildWorktreeGuardFiles(abs)` returns two files with the expected
`relativePath`s and a settings file whose hook command references the absolute
script path + bakes `CODEV_WORKTREE_ROOT`.

**Build/typecheck:** `pnpm --filter @cluesmith/codev build` is clean.

**Manual at the `dev-approval` gate (the real teeth — reviewer runs this):**
spawn a throwaway builder and confirm in its session:
1. A `Write` to a main-checkout path (e.g. `<repo>/codev/plans/zzz.md`) is
**blocked** with a clear reason.
2. A legitimate `/tmp` (or scratchpad) write **passes**.
3. A genuine sibling-thread / cross-checkout **read** still works (reads unaffected).
4. A memory write under `~/.claude/...` **passes**.
5. A correctly worktree-rooted Write **passes** and the file lands in the worktree
(`git status` in the worktree shows it; the main checkout is untouched).
29 changes: 29 additions & 0 deletions codev/projects/1018-builder-worktree-write-guard-p/status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
id: '1018'
title: builder-worktree-write-guard-p
protocol: pir
phase: review
plan_phases: []
current_plan_phase: null
gates:
plan-approval:
status: approved
requested_at: '2026-06-23T00:48:54.491Z'
approved_at: '2026-06-23T03:18:28.862Z'
dev-approval:
status: approved
requested_at: '2026-06-23T03:30:26.139Z'
approved_at: '2026-06-23T23:21:39.771Z'
pr:
status: pending
requested_at: '2026-06-23T23:36:35.993Z'
iteration: 1
build_complete: true
history: []
started_at: '2026-06-23T00:38:54.889Z'
updated_at: '2026-06-23T23:36:35.994Z'
pr_history:
- phase: review
pr_number: 1098
branch: builder/pir-1018
created_at: '2026-06-23T23:26:33.365Z'
pr_ready_for_human: true
11 changes: 11 additions & 0 deletions codev/resources/arch.md
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,17 @@ When spawning a builder (`afx spawn 3 --protocol spir`):
- `.builder-role.md`: Role definition (from `codev/roles/builder.md`)
- `.builder-start.sh`: Launch script for builder session

#### Worktree Write-Guard (Issue #1018)

A builder worktree is **nested inside the main checkout** and byte-identical to it at the branch base. The `Write`/`Edit` tools require absolute paths, so the builder model synthesizes one; when it anchors at the inferred canonical repo root instead of its worktree cwd, the `.builders/<id>/` segment is dropped and the write silently lands in the main checkout. This is intrinsic model/CLI path-synthesis behavior that drifts across upgrades, so instructions/memory don't hold — only a deterministic guard does.

The guard is a Claude **PreToolUse hook**, installed per-worktree at spawn time through the existing `HarnessProvider.getWorktreeFiles()` → `writeWorktreeFiles()` path (`CLAUDE_HARNESS` only — `PreToolUse` is Claude-specific). `buildWorktreeGuardFiles()` (`packages/codev/src/agent-farm/utils/worktree-write-guard.ts`, the single source of truth) emits two files:

- `.claude/hooks/worktree-write-guard.cjs` — a self-contained Node hook (no project imports; ships as a TS string constant since `tsc` copies no assets). It denies any `Write`/`Edit` whose `file_path` resolves outside the worktree root, allowlisting temp dirs (`/tmp`, `/private/tmp`, `$TMPDIR`) and `$HOME/.claude` (builder memory/config). Paths are canonicalized via realpath-of-longest-existing-ancestor (handles non-existent new files and macOS `/tmp`→`/private/tmp`). **Fail-open** on any error so it never bricks a session.
- `.claude/settings.local.json` — wires the hook on `Write|Edit|MultiEdit`, baking the worktree root in as `CODEV_WORKTREE_ROOT` (deterministic) with `git rev-parse --show-toplevel` as a runtime fallback.

Scope: write surface only. Reads are untouched, preserving codev's intentional cross-checkout reads (architect reads builder threads, sibling-thread reads). The complementary control for the Bash write surface (`>`, `cp`, `tee`) is relative-path discipline (cwd = worktree), documented in `roles/builder.md`. The architect session is unaffected — it launches via `buildRoleInjection` in the main checkout and never receives the hook (and modifying `main` is its job by design). The consult sub-agent read-surface sibling (#1092) is a separate fix.

#### Directory Structure

```
Expand Down
Loading
Loading