From 3588bde362e1fbf36c82c78fcaf983b346c7650e Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 10:38:55 +1000 Subject: [PATCH 01/21] chore(porch): 1018 init pir --- .../status.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 codev/projects/1018-builder-worktree-write-guard-p/status.yaml diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml new file mode 100644 index 000000000..2e0ccb14c --- /dev/null +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -0,0 +1,18 @@ +id: '1018' +title: builder-worktree-write-guard-p +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-06-23T00:38:54.889Z' +updated_at: '2026-06-23T00:38:54.890Z' From 0e85a8ee898510487ce424307a6b84a995cec00f Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 10:48:47 +1000 Subject: [PATCH 02/21] [PIR #1018] Plan draft --- .../1018-builder-worktree-write-guard-p.md | 207 ++++++++++++++++++ codev/state/pir-1018_thread.md | 41 ++++ 2 files changed, 248 insertions(+) create mode 100644 codev/plans/1018-builder-worktree-write-guard-p.md create mode 100644 codev/state/pir-1018_thread.md diff --git a/codev/plans/1018-builder-worktree-write-guard-p.md b/codev/plans/1018-builder-worktree-write-guard-p.md new file mode 100644 index 000000000..890d35261 --- /dev/null +++ b/codev/plans/1018-builder-worktree-write-guard-p.md @@ -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//`, 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//` 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=`. 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":""}}`. 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//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. `/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). diff --git a/codev/state/pir-1018_thread.md b/codev/state/pir-1018_thread.md new file mode 100644 index 000000000..d288f8a62 --- /dev/null +++ b/codev/state/pir-1018_thread.md @@ -0,0 +1,41 @@ +# PIR #1018 — Builder worktree write-guard + +## Phase: plan (iteration 1) + +### What this is +Deterministic PreToolUse guard so a builder's Write/Edit cannot silently land +files in the main checkout (outside its worktree). Root cause is model/CLI +path-synthesis drift, so only a runtime guard holds; instructions are a backstop. + +### Investigation findings +- Builder sessions launch via `startBuilderSession()` (spawn-worktree.ts:724) which + `cd`s into the worktree and runs the bare `claude` command — no `--settings` flag. +- The ONLY existing pattern for injecting per-worktree files is + `HarnessProvider.getWorktreeFiles()` (harness.ts:44) + `writeWorktreeFiles()` + (spawn-worktree.ts:679). OpenCode uses it for `opencode.json`. +- `getWorktreeFiles(roleContent, roleFilePath)` currently does NOT receive the + worktree absolute path → plan extends the signature so the Claude harness can + bake an absolute hook command path + worktree root. +- `writeWorktreeFiles` writes via `writeFileSync` (no mkdir) → must add mkdir -p + because `.claude/hooks/` doesn't exist in a fresh worktree (only `.claude/skills/`). +- No committed `.claude/settings.json` in repo → use `.claude/settings.local.json`. +- `roles/builder.md` is byte-identical in `codev/roles/` and `codev-skeleton/roles/` + (8623 bytes) → backstop edit must mirror both. +- Build is `tsc` only (no asset copy) → emit the guard as a TS string constant + (single source), unit-tested by writing it to a tmp .cjs and spawning it. + +### Key design decisions +- Guard authored as a self-contained Node `.cjs` (portable, no jq dep), emitted + into `.claude/hooks/worktree-write-guard.cjs`. +- Worktree root baked in via env at spawn time (deterministic) with + `git rev-parse --show-toplevel` as runtime fallback. +- Allowlist: temp dirs (`/tmp`, `/private/tmp`, `$TMPDIR`) + `$HOME/.claude` + (so memory + claude config writes still work). realpath both sides (macOS + `/tmp`→`/private/tmp`). +- Scope: Write/Edit only (reads unaffected → cross-checkout reads preserved); + Claude harness only (PreToolUse is Claude-specific). +- #1092 (consult read surface) is a SIBLING issue — note the reuse seam, leave + out of scope, surface to human at gate. + +### Status +Plan drafted → awaiting plan-approval gate. From 7b44c3d027d850a5f01aff642941bf379b2159c4 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 10:48:54 +1000 Subject: [PATCH 03/21] chore(porch): 1018 plan-approval gate-requested --- codev/projects/1018-builder-worktree-write-guard-p/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 2e0ccb14c..b33f8ee03 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -7,6 +7,7 @@ current_plan_phase: null gates: plan-approval: status: pending + requested_at: '2026-06-23T00:48:54.491Z' dev-approval: status: pending pr: @@ -15,4 +16,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T00:38:54.890Z' +updated_at: '2026-06-23T00:48:54.491Z' From d09862fb9ebac0a3b2797be2722eaa0bd1be8297 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:18:28 +1000 Subject: [PATCH 04/21] chore(porch): 1018 plan-approval gate-approved --- .../projects/1018-builder-worktree-write-guard-p/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index b33f8ee03..e3c85642d 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -6,8 +6,9 @@ plan_phases: [] current_plan_phase: null gates: plan-approval: - status: pending + status: approved requested_at: '2026-06-23T00:48:54.491Z' + approved_at: '2026-06-23T03:18:28.862Z' dev-approval: status: pending pr: @@ -16,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T00:48:54.491Z' +updated_at: '2026-06-23T03:18:28.862Z' From fa65ec76800ba4fa62d17092f95aaf5792c0e59d Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:18:40 +1000 Subject: [PATCH 05/21] chore(porch): 1018 implement phase-transition --- .../projects/1018-builder-worktree-write-guard-p/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index e3c85642d..6fd212470 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -1,7 +1,7 @@ id: '1018' title: builder-worktree-write-guard-p protocol: pir -phase: plan +phase: implement plan_phases: [] current_plan_phase: null gates: @@ -17,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T03:18:28.862Z' +updated_at: '2026-06-23T03:18:39.991Z' From 93f455de82fb4e4c8579c17479e0d78dd97523f6 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:28:56 +1000 Subject: [PATCH 06/21] [PIR #1018] Add worktree write-guard hook for Claude builders --- .../src/agent-farm/commands/spawn-worktree.ts | 13 +- .../codev/src/agent-farm/utils/harness.ts | 14 +- .../agent-farm/utils/worktree-write-guard.ts | 233 ++++++++++++++++++ 3 files changed, 254 insertions(+), 6 deletions(-) create mode 100644 packages/codev/src/agent-farm/utils/worktree-write-guard.ts diff --git a/packages/codev/src/agent-farm/commands/spawn-worktree.ts b/packages/codev/src/agent-farm/commands/spawn-worktree.ts index a0eb5e91f..677179aab 100644 --- a/packages/codev/src/agent-farm/commands/spawn-worktree.ts +++ b/packages/codev/src/agent-farm/commands/spawn-worktree.ts @@ -682,6 +682,9 @@ function writeWorktreeFiles( ): void { for (const file of files) { const targetPath = resolve(worktreePath, file.relativePath); + // Generated files may live in a subdir that doesn't exist yet in a fresh + // worktree (e.g. .claude/hooks/ for the write-guard — Issue #1018). + mkdirSync(dirname(targetPath), { recursive: true }); if (file.relativePath.endsWith('.json') && existsSync(targetPath)) { try { const existing = JSON.parse(readFileSync(targetPath, 'utf-8')); @@ -770,9 +773,10 @@ done .join('\n'); const envBlock = envExports ? `${envExports}\n` : ''; - // Write any harness-specific worktree files (e.g., opencode.json for OpenCode) + // Write any harness-specific worktree files (e.g., opencode.json for OpenCode, + // the write-guard hook for Claude — Issue #1018) if (harness.getWorktreeFiles) { - writeWorktreeFiles(harness.getWorktreeFiles(roleWithPort, roleFile), worktreePath); + writeWorktreeFiles(harness.getWorktreeFiles(roleWithPort, roleFile, worktreePath), worktreePath); } scriptContent = `#!/bin/bash @@ -859,9 +863,10 @@ export function buildWorktreeLaunchScript( .join('\n'); const envBlock = envExports ? `${envExports}\n` : ''; - // Write any harness-specific worktree files (e.g., opencode.json for OpenCode) + // Write any harness-specific worktree files (e.g., opencode.json for OpenCode, + // the write-guard hook for Claude — Issue #1018) if (harness.getWorktreeFiles) { - writeWorktreeFiles(harness.getWorktreeFiles(roleWithPort, roleFile), worktreePath); + writeWorktreeFiles(harness.getWorktreeFiles(roleWithPort, roleFile, worktreePath), worktreePath); } return `#!/bin/bash diff --git a/packages/codev/src/agent-farm/utils/harness.ts b/packages/codev/src/agent-farm/utils/harness.ts index bfb7f3b67..eb716cbcb 100644 --- a/packages/codev/src/agent-farm/utils/harness.ts +++ b/packages/codev/src/agent-farm/utils/harness.ts @@ -12,6 +12,8 @@ * @see codev/specs/591-af-workspace-failure-with-code.md */ +import { buildWorktreeGuardFiles } from './worktree-write-guard.js'; + // ============================================================================= // Types // ============================================================================= @@ -39,9 +41,13 @@ export interface HarnessProvider { /** * Optional: files to write in the worktree before launching the agent. * Used by harnesses that rely on file-based configuration (e.g., OpenCode - * uses opencode.json's instructions field for role injection). + * uses opencode.json's instructions field for role injection; Claude uses it + * to install the worktree write-guard hook — Issue #1018). + * + * `worktreePath` is the absolute path to the builder's worktree, needed by + * harnesses that bake worktree-specific values into generated files. */ - getWorktreeFiles?(roleContent: string, roleFilePath: string): Array<{ + getWorktreeFiles?(roleContent: string, roleFilePath: string, worktreePath: string): Array<{ relativePath: string; content: string; }>; @@ -68,6 +74,10 @@ export const CLAUDE_HARNESS: HarnessProvider = { fragment: `--append-system-prompt "$(cat '${shellEscapeSingleQuote(filePath)}')"`, env: {}, }), + // Install the worktree write-guard PreToolUse hook (Issue #1018) so a builder + // cannot silently write outside its worktree (e.g. into the main checkout). + getWorktreeFiles: (_content, _filePath, worktreePath) => + buildWorktreeGuardFiles(worktreePath), }; export const CODEX_HARNESS: HarnessProvider = { diff --git a/packages/codev/src/agent-farm/utils/worktree-write-guard.ts b/packages/codev/src/agent-farm/utils/worktree-write-guard.ts new file mode 100644 index 000000000..526715082 --- /dev/null +++ b/packages/codev/src/agent-farm/utils/worktree-write-guard.ts @@ -0,0 +1,233 @@ +/** + * Builder worktree write-guard (Issue #1018). + * + * A strict-mode builder runs in a git worktree nested inside the main checkout. + * The Write/Edit tools require absolute paths, so the model must *synthesize* + * one. When the runtime anchors that path at the inferred canonical repo root + * instead of the worktree cwd, the `.builders//` segment is dropped and the + * write silently lands in the main checkout (a real, writable, byte-identical + * sibling tree). Instructions/memory don't hold against this because it's + * intrinsic path-synthesis behavior that drifts across model/CLI upgrades — only + * a deterministic guard holds. + * + * This module is the single source of truth for the guard. It emits two files + * into each Claude builder worktree at spawn time (via CLAUDE_HARNESS): + * - `.claude/hooks/worktree-write-guard.cjs` — a self-contained Node hook + * - `.claude/settings.local.json` — a PreToolUse hook wiring on Write/Edit + * + * The hook denies any absolute Write/Edit path that resolves outside the + * worktree root, with an allowlist for temp dirs and `$HOME/.claude` (so builder + * memory/config writes still work). The worktree root is baked into the hook + * command as `CODEV_WORKTREE_ROOT` (deterministic), with `git rev-parse + * --show-toplevel` as a runtime fallback. + * + * @see codev/plans/1018-builder-worktree-write-guard-p.md + */ + +import { isAbsolute, join, resolve } from 'node:path'; + +/** Worktree-relative path of the emitted guard script. */ +export const GUARD_SCRIPT_RELPATH = '.claude/hooks/worktree-write-guard.cjs'; + +/** Worktree-relative path of the emitted settings file. */ +export const GUARD_SETTINGS_RELPATH = '.claude/settings.local.json'; + +/** + * The self-contained Node guard script, emitted verbatim into each worktree. + * + * Kept as a string constant (not a separate asset) so it ships with the + * compiled package automatically — `tsc` does not copy non-TS files to `dist`. + * It is exercised directly by the unit test, which writes it to a temp `.cjs` + * and spawns it with fixture stdin, so the constant is the tested artifact. + * + * Self-contained: no `require` of any project code (worktrees have no access to + * the package's node_modules, and neither do adopter repos). Node core only. + * + * Fail-open by design: any error (bad JSON, missing root, unexpected shape) + * allows the tool call. A safety net must never brick a builder session; the + * worst case of a failure is reverting to today's unguarded behavior. + */ +export const WORKTREE_WRITE_GUARD_SCRIPT = `#!/usr/bin/env node +// AUTO-GENERATED by @cluesmith/codev (Issue #1018). Do not edit in the worktree; +// edit packages/codev/src/agent-farm/utils/worktree-write-guard.ts instead. +'use strict'; + +const fs = require('node:fs'); +const path = require('node:path'); +const { execSync } = require('node:child_process'); + +// Tools whose file_path we guard. NotebookEdit (notebook_path) is out of scope. +const GUARDED_TOOLS = new Set(['Write', 'Edit', 'MultiEdit']); + +function allow() { + process.exit(0); +} + +function deny(target, root) { + const reason = + 'Blocked: ' + target + ' is outside this builder\\'s worktree (' + root + '). ' + + 'Builders are isolated to their worktree — a path anchored at the main checkout ' + + 'root silently pollutes it. Re-root the path under ' + root + + ' (prefer a worktree-relative path). Allowed exceptions: temp dirs and ~/.claude.'; + process.stdout.write(JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + permissionDecision: 'deny', + permissionDecisionReason: reason, + }, + })); + process.exit(0); +} + +// Resolve a path to a canonical absolute form WITHOUT requiring it to exist: +// realpath the longest existing ancestor (resolving symlinks, e.g. macOS +// /tmp -> /private/tmp), then re-append the non-existent tail. +function canonicalize(p, cwd) { + let abs = path.isAbsolute(p) ? p : path.resolve(cwd, p); + abs = path.normalize(abs); + let tail = ''; + let cur = abs; + for (;;) { + try { + const real = fs.realpathSync(cur); + return tail ? path.join(real, tail) : real; + } catch (e) { + const parent = path.dirname(cur); + if (parent === cur) { + return abs; + } + tail = tail ? path.join(path.basename(cur), tail) : path.basename(cur); + cur = parent; + } + } +} + +function isInside(child, parent) { + if (!parent) { + return false; + } + if (child === parent) { + return true; + } + return child.startsWith(parent + path.sep); +} + +function main(raw) { + let input; + try { + input = JSON.parse(raw); + } catch (e) { + return allow(); + } + + const toolName = input && input.tool_name; + if (!GUARDED_TOOLS.has(toolName)) { + return allow(); + } + + const toolInput = (input && input.tool_input) || {}; + const filePath = toolInput.file_path; + if (!filePath || typeof filePath !== 'string') { + return allow(); + } + + const cwd = (input && input.cwd) || process.cwd(); + + // Worktree root: baked-in env (deterministic) first, then git as a fallback. + let root = process.env.CODEV_WORKTREE_ROOT; + if (!root) { + try { + root = execSync('git rev-parse --show-toplevel', { cwd, encoding: 'utf8' }).trim(); + } catch (e) { + root = ''; + } + } + if (!root) { + // Can't determine the boundary — fail open rather than block every write. + return allow(); + } + root = canonicalize(root, cwd); + + const target = canonicalize(filePath, cwd); + + if (isInside(target, root)) { + return allow(); + } + + // Allowlist: temp dirs (scratch) and ~/.claude (builder memory + config). + const allowRoots = []; + const addAllow = (p) => { + if (p) { + allowRoots.push(canonicalize(p, cwd)); + } + }; + addAllow('/tmp'); + addAllow('/private/tmp'); + addAllow(process.env.TMPDIR); + if (process.env.HOME) { + addAllow(path.join(process.env.HOME, '.claude')); + } + + for (const a of allowRoots) { + if (isInside(target, a)) { + return allow(); + } + } + + return deny(target, root); +} + +let buf = ''; +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (d) => { + buf += d; +}); +process.stdin.on('end', () => { + try { + main(buf); + } catch (e) { + // Never brick a session on an unexpected error. + allow(); + } +}); +`; + +/** Single-quote-escape a value for safe embedding in a POSIX shell command. */ +function shellSingleQuote(value: string): string { + return value.replace(/'/g, `'\\''`); +} + +/** + * Build the worktree files that install the write-guard for a Claude builder. + * + * @param worktreePath absolute path to the builder's worktree + * @returns files (relativePath + content) for writeWorktreeFiles() + */ +export function buildWorktreeGuardFiles( + worktreePath: string, +): Array<{ relativePath: string; content: string }> { + const root = isAbsolute(worktreePath) ? worktreePath : resolve(worktreePath); + const scriptAbs = join(root, GUARD_SCRIPT_RELPATH); + + // Bake the worktree root in (deterministic) and run via `node` so the emitted + // script needs no executable bit. + const command = + `CODEV_WORKTREE_ROOT='${shellSingleQuote(root)}' ` + + `node '${shellSingleQuote(scriptAbs)}'`; + + const settings = { + hooks: { + PreToolUse: [ + { + matcher: 'Write|Edit|MultiEdit', + hooks: [{ type: 'command', command }], + }, + ], + }, + }; + + return [ + { relativePath: GUARD_SCRIPT_RELPATH, content: WORKTREE_WRITE_GUARD_SCRIPT }, + { relativePath: GUARD_SETTINGS_RELPATH, content: JSON.stringify(settings, null, 2) + '\n' }, + ]; +} From d5ec7156e99fe9c7a011c0870269284659f2bf39 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:29:06 +1000 Subject: [PATCH 07/21] [PIR #1018] Test the write-guard script and CLAUDE_HARNESS wiring --- .../__tests__/worktree-write-guard.test.ts | 214 ++++++++++++++++++ .../src/agent-farm/__tests__/harness.test.ts | 15 +- 2 files changed, 225 insertions(+), 4 deletions(-) create mode 100644 packages/codev/src/__tests__/worktree-write-guard.test.ts diff --git a/packages/codev/src/__tests__/worktree-write-guard.test.ts b/packages/codev/src/__tests__/worktree-write-guard.test.ts new file mode 100644 index 000000000..2aad15f02 --- /dev/null +++ b/packages/codev/src/__tests__/worktree-write-guard.test.ts @@ -0,0 +1,214 @@ +/** + * Tests for the builder worktree write-guard (Issue #1018). + * + * The guard is emitted into each Claude builder worktree as a self-contained + * Node hook. These tests exercise the EXACT emitted artifact: they write + * WORKTREE_WRITE_GUARD_SCRIPT to a temp .cjs and spawn it with fixture stdin, + * so the tested behavior is the behavior builders get. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync, execSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + WORKTREE_WRITE_GUARD_SCRIPT, + buildWorktreeGuardFiles, + GUARD_SCRIPT_RELPATH, + GUARD_SETTINGS_RELPATH, +} from '../agent-farm/utils/worktree-write-guard.js'; + +let base: string; +let mainCheckout: string; +let worktree: string; +let homeDir: string; +let scriptPath: string; + +beforeAll(() => { + base = fs.mkdtempSync(path.join(os.tmpdir(), 'cguard-')); + mainCheckout = path.join(base, 'main'); + worktree = path.join(mainCheckout, '.builders', 'wt'); + homeDir = path.join(base, 'home'); + fs.mkdirSync(worktree, { recursive: true }); + fs.mkdirSync(path.join(homeDir, '.claude'), { recursive: true }); + scriptPath = path.join(base, 'guard.cjs'); + fs.writeFileSync(scriptPath, WORKTREE_WRITE_GUARD_SCRIPT); +}); + +afterAll(() => { + fs.rmSync(base, { recursive: true, force: true }); +}); + +interface GuardResult { + status: number | null; + denied: boolean; + reason: string; +} + +/** + * Run the guard with a deterministic env. TMPDIR is deliberately omitted so the + * temp-backed fixture dirs are NOT treated as an allowlisted temp dir — only the + * worktree, /tmp, /private/tmp, and $HOME/.claude should pass. + */ +function runGuard( + toolName: string, + filePath: string | undefined, + opts: { root?: string; home?: string; cwd?: string; bakeRoot?: boolean } = {}, +): GuardResult { + const cwd = opts.cwd ?? opts.root ?? worktree; + const env: Record = { + PATH: process.env.PATH ?? '', + HOME: opts.home ?? homeDir, + }; + const bakeRoot = opts.bakeRoot ?? true; + if (bakeRoot && opts.root !== null) { + env.CODEV_WORKTREE_ROOT = opts.root ?? worktree; + } + + const toolInput: Record = {}; + if (filePath !== undefined) { + toolInput.file_path = filePath; + } + + const res = spawnSync('node', [scriptPath], { + input: JSON.stringify({ tool_name: toolName, tool_input: toolInput, cwd }), + env, + encoding: 'utf8', + }); + + let denied = false; + let reason = ''; + const out = (res.stdout ?? '').trim(); + if (out) { + const parsed = JSON.parse(out); + denied = parsed?.hookSpecificOutput?.permissionDecision === 'deny'; + reason = parsed?.hookSpecificOutput?.permissionDecisionReason ?? ''; + } + return { status: res.status, denied, reason }; +} + +describe('worktree-write-guard script', () => { + it('allows a Write inside the worktree', () => { + const r = runGuard('Write', path.join(worktree, 'codev', 'plans', 'x.md')); + expect(r.status).toBe(0); + expect(r.denied).toBe(false); + }); + + it('allows a Write to a new deeply-nested non-existent path inside the worktree', () => { + const r = runGuard('Write', path.join(worktree, 'a', 'b', 'c', 'new.txt')); + expect(r.denied).toBe(false); + }); + + it('DENIES a Write to a main-checkout path outside the worktree (the #1018 bug)', () => { + const r = runGuard('Write', path.join(mainCheckout, 'codev', 'plans', 'x.md')); + expect(r.status).toBe(0); + expect(r.denied).toBe(true); + // The reason names the worktree root so the model can re-root. + expect(r.reason).toContain(fs.realpathSync(worktree)); + }); + + it('DENIES an Edit to a path outside the worktree (Edit is guarded too)', () => { + const r = runGuard('Edit', path.join(mainCheckout, 'src', 'app.ts')); + expect(r.denied).toBe(true); + }); + + it('allows a Write to /tmp (temp allowlist, with macOS symlink normalization)', () => { + const r = runGuard('Write', '/tmp/codev-guard-scratch/out.txt'); + expect(r.denied).toBe(false); + }); + + it('allows a Write to /private/tmp (temp allowlist)', () => { + const r = runGuard('Write', '/private/tmp/codev-guard-scratch/out.txt'); + expect(r.denied).toBe(false); + }); + + it('allows a Write to $HOME/.claude (builder memory / config)', () => { + const r = runGuard( + 'Write', + path.join(homeDir, '.claude', 'projects', 'p', 'memory', 'm.md'), + ); + expect(r.denied).toBe(false); + }); + + it('allows a non-guarded tool (Bash) regardless of path', () => { + const r = runGuard('Bash', undefined); + expect(r.denied).toBe(false); + }); + + it('allows when file_path is missing', () => { + const r = runGuard('Write', undefined); + expect(r.denied).toBe(false); + }); + + it('allows on malformed JSON (fail-open)', () => { + const res = spawnSync('node', [scriptPath], { + input: 'not json', + env: { PATH: process.env.PATH ?? '', HOME: homeDir, CODEV_WORKTREE_ROOT: worktree }, + encoding: 'utf8', + }); + expect(res.status).toBe(0); + expect((res.stdout ?? '').trim()).toBe(''); + }); + + it('falls back to `git rev-parse` when CODEV_WORKTREE_ROOT is unset', () => { + const gitRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'cguard-git-')); + try { + const env = { cwd: gitRepo, stdio: 'pipe' as const }; + execSync('git init -q', env); + execSync('git config user.email t@t.t', env); + execSync('git config user.name t', env); + fs.writeFileSync(path.join(gitRepo, 'seed.txt'), 'seed'); + execSync('git add seed.txt', env); + execSync('git commit -q -m seed', env); + execSync('git worktree add -q .builders/wt -b gtest', env); + const gitWorktree = path.join(gitRepo, '.builders', 'wt'); + + // No CODEV_WORKTREE_ROOT: the guard must resolve the worktree via git and + // still deny a write that lands in the outer checkout. + const r = runGuard('Write', path.join(gitRepo, 'codev', 'x.md'), { + cwd: gitWorktree, + bakeRoot: false, + }); + expect(r.denied).toBe(true); + + const inside = runGuard('Write', path.join(gitWorktree, 'codev', 'x.md'), { + cwd: gitWorktree, + bakeRoot: false, + }); + expect(inside.denied).toBe(false); + } finally { + fs.rmSync(gitRepo, { recursive: true, force: true }); + } + }); +}); + +describe('buildWorktreeGuardFiles', () => { + it('returns the guard script and a settings file at the expected paths', () => { + const files = buildWorktreeGuardFiles('/abs/worktree'); + const relPaths = files.map((f) => f.relativePath).sort(); + expect(relPaths).toEqual([GUARD_SETTINGS_RELPATH, GUARD_SCRIPT_RELPATH].sort()); + + const script = files.find((f) => f.relativePath === GUARD_SCRIPT_RELPATH); + expect(script?.content).toBe(WORKTREE_WRITE_GUARD_SCRIPT); + }); + + it('bakes an absolute CODEV_WORKTREE_ROOT and runs the script via node', () => { + const files = buildWorktreeGuardFiles('/abs/worktree'); + const settings = files.find((f) => f.relativePath === GUARD_SETTINGS_RELPATH); + const parsed = JSON.parse(settings!.content); + const entry = parsed.hooks.PreToolUse[0]; + expect(entry.matcher).toContain('Write'); + expect(entry.matcher).toContain('Edit'); + const command = entry.hooks[0].command; + expect(command).toContain("CODEV_WORKTREE_ROOT='/abs/worktree'"); + expect(command).toContain(`node '/abs/worktree/${GUARD_SCRIPT_RELPATH}'`); + }); + + it('resolves a relative worktree path to absolute before baking', () => { + const files = buildWorktreeGuardFiles('relative/wt'); + const settings = files.find((f) => f.relativePath === GUARD_SETTINGS_RELPATH); + const command = JSON.parse(settings!.content).hooks.PreToolUse[0].hooks[0].command; + expect(command).toContain(`CODEV_WORKTREE_ROOT='${path.resolve('relative/wt')}'`); + }); +}); diff --git a/packages/codev/src/agent-farm/__tests__/harness.test.ts b/packages/codev/src/agent-farm/__tests__/harness.test.ts index 8290eda08..58ad5d244 100644 --- a/packages/codev/src/agent-farm/__tests__/harness.test.ts +++ b/packages/codev/src/agent-farm/__tests__/harness.test.ts @@ -76,7 +76,7 @@ describe('harness', () => { }); it('getWorktreeFiles returns opencode.json with instructions', () => { - const files = OPENCODE_HARNESS.getWorktreeFiles!(ROLE_CONTENT, ROLE_FILE); + const files = OPENCODE_HARNESS.getWorktreeFiles!(ROLE_CONTENT, ROLE_FILE, '/abs/wt'); expect(files).toHaveLength(1); expect(files[0].relativePath).toBe('opencode.json'); const parsed = JSON.parse(files[0].content); @@ -84,9 +84,16 @@ describe('harness', () => { }); }); - describe('getWorktreeFiles backward compatibility', () => { - it('CLAUDE_HARNESS does not have getWorktreeFiles', () => { - expect(CLAUDE_HARNESS.getWorktreeFiles).toBeUndefined(); + describe('getWorktreeFiles', () => { + it('CLAUDE_HARNESS installs the worktree write-guard (Issue #1018)', () => { + const files = CLAUDE_HARNESS.getWorktreeFiles!(ROLE_CONTENT, ROLE_FILE, '/abs/wt'); + const relPaths = files.map((f) => f.relativePath).sort(); + expect(relPaths).toEqual( + ['.claude/hooks/worktree-write-guard.cjs', '.claude/settings.local.json'].sort(), + ); + const settings = files.find((f) => f.relativePath === '.claude/settings.local.json'); + const parsed = JSON.parse(settings!.content); + expect(parsed.hooks.PreToolUse[0].matcher).toContain('Write'); }); it('CODEX_HARNESS does not have getWorktreeFiles', () => { From 84b2f7b1d847e51a91de242edd775dc1a41d4b5e Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:29:06 +1000 Subject: [PATCH 08/21] [PIR #1018] Document worktree path-discipline backstop in builder role --- codev-skeleton/roles/builder.md | 21 +++++++++++++++++++++ codev/roles/builder.md | 21 +++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/codev-skeleton/roles/builder.md b/codev-skeleton/roles/builder.md index 4f49ded80..be5d7cbf5 100644 --- a/codev-skeleton/roles/builder.md +++ b/codev-skeleton/roles/builder.md @@ -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//`) 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//` + 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 diff --git a/codev/roles/builder.md b/codev/roles/builder.md index 4f49ded80..be5d7cbf5 100644 --- a/codev/roles/builder.md +++ b/codev/roles/builder.md @@ -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//`) 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//` + 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 From 10aa609571df196b2a4801cfd04e43bd98247de4 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:29:51 +1000 Subject: [PATCH 09/21] [PIR #1018] Update builder thread for implement phase --- codev/state/pir-1018_thread.md | 39 +++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/codev/state/pir-1018_thread.md b/codev/state/pir-1018_thread.md index d288f8a62..29d72f44e 100644 --- a/codev/state/pir-1018_thread.md +++ b/codev/state/pir-1018_thread.md @@ -37,5 +37,42 @@ path-synthesis drift, so only a runtime guard holds; instructions are a backstop - #1092 (consult read surface) is a SIBLING issue — note the reuse seam, leave out of scope, surface to human at gate. +### Scope decision (at plan-approval gate) +Q: should the guard also protect architect terminals from modifying main? +A (human): keep #1018 builder-scoped. +Rationale to record in review Lessons Learned: +- The hook is generated only into builder worktrees (builder-spawn path); + architect sessions launch via buildRoleInjection in the main checkout and + never receive it → zero impact on architects. +- The guard enforces "writes stay inside the worktree"; an architect's tree IS + main, and modifying main is its job (Pre-Spawn Rule, committed→integrated gate). + Same hook on an architect = structural no-op (root resolves to main checkout). +- Architects are LESS prone to the path-synthesis bug (no .builders/ segment to + drop). The only analog (architect writing INTO .builders/*) is lower severity + and a separate concern, not folded in here. + ### Status -Plan drafted → awaiting plan-approval gate. +Plan drafted → awaiting plan-approval gate. (architect-scope question resolved: builder-only) + +## Phase: implement (iteration 1) — complete, awaiting dev-approval + +Implemented as planned: +- `packages/codev/src/agent-farm/utils/worktree-write-guard.ts` (new) — single + source of truth: `WORKTREE_WRITE_GUARD_SCRIPT` (self-contained Node `.cjs`) + + `buildWorktreeGuardFiles(worktreePath)`. Baked-in `CODEV_WORKTREE_ROOT` env, + `git rev-parse` fallback, fail-open on error. Allowlist: /tmp, /private/tmp, + $TMPDIR, $HOME/.claude. +- `harness.ts` — extended `getWorktreeFiles` signature with `worktreePath`; + `CLAUDE_HARNESS` now emits the guard files. +- `spawn-worktree.ts` — `writeWorktreeFiles` now `mkdir -p`s the target dir + (needed for `.claude/hooks/`); both call sites pass `worktreePath`. +- Both `builder.md` (codev/ + skeleton, mirrored) — path-discipline backstop. + +Tests: new `worktree-write-guard.test.ts` (14 cases incl. git fallback, deny +shape, allowlist, non-existent nested path) + updated `harness.test.ts` contract +(CLAUDE_HARNESS now HAS getWorktreeFiles). + +Checks: `npm run build` ✓ (exit 0), `npm test` ✓ (3358 passed, 0 failed). +Note: full `npm run build` must run before `npm test` — the terminal +session-manager *integration* tests need the built shellper binary; without the +build they fail to connect (environmental, not related to this change). From 636a2b8f73ece2354344c901438dcc414ff40aa5 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Tue, 23 Jun 2026 13:30:26 +1000 Subject: [PATCH 10/21] chore(porch): 1018 dev-approval gate-requested --- codev/projects/1018-builder-worktree-write-guard-p/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 6fd212470..72b96e435 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -11,10 +11,11 @@ gates: approved_at: '2026-06-23T03:18:28.862Z' dev-approval: status: pending + requested_at: '2026-06-23T03:30:26.139Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T03:18:39.991Z' +updated_at: '2026-06-23T03:30:26.139Z' From 396ced743e9ff2448e79cb6e439420a6e17226ad Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:21:39 +1000 Subject: [PATCH 11/21] chore(porch): 1018 dev-approval gate-approved --- .../projects/1018-builder-worktree-write-guard-p/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 72b96e435..857566ff4 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -10,12 +10,13 @@ gates: requested_at: '2026-06-23T00:48:54.491Z' approved_at: '2026-06-23T03:18:28.862Z' dev-approval: - status: pending + status: approved requested_at: '2026-06-23T03:30:26.139Z' + approved_at: '2026-06-23T23:21:39.771Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T03:30:26.139Z' +updated_at: '2026-06-23T23:21:39.772Z' From 6c0b3639c1900a852dbcf05262c02b1804c136c3 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:22:25 +1000 Subject: [PATCH 12/21] chore(porch): 1018 review phase-transition --- .../projects/1018-builder-worktree-write-guard-p/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 857566ff4..8ee5f93a0 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -1,7 +1,7 @@ id: '1018' title: builder-worktree-write-guard-p protocol: pir -phase: implement +phase: review plan_phases: [] current_plan_phase: null gates: @@ -19,4 +19,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T23:21:39.772Z' +updated_at: '2026-06-23T23:22:25.026Z' From 622d895a9af68bec8da705e0e4c2265a8c385a01 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:26:16 +1000 Subject: [PATCH 13/21] [PIR #1018] Review + retrospective --- codev/resources/arch.md | 11 +++ codev/resources/lessons-learned.md | 3 + .../1018-builder-worktree-write-guard-p.md | 69 +++++++++++++++++++ codev/state/pir-1018_thread.md | 7 ++ 4 files changed, 90 insertions(+) create mode 100644 codev/reviews/1018-builder-worktree-write-guard-p.md diff --git a/codev/resources/arch.md b/codev/resources/arch.md index df543ecaf..88c0af722 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -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//` 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 ``` diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index 50b5e802e..c3c43e340 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -75,6 +75,9 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated - [From #971] A "rehydrate the session before declaring it unknown" guard at the WS upgrade handler is **dead code** in this codebase, because both shellper-reconnect paths **reassign the terminal id** (`tower-terminals.ts:646`/`:669-672` startup reconcile; `:832`/`:868-872` on-the-fly) and delete the old SQLite row. After a Tower restart a persistent session returns under a **new** id, so the **old** id in a stale tab's WS URL is permanently dead — `getSession(oldId)` stays null no matter how much you rehydrate. The give-up close code is therefore always truthful, and recovery for the persistent case is a **state re-fetch → successor id → remount** (the `Terminal` effect is keyed on `wsPath`), not a WS retry to the same URL. Before adding a "wait and retry, it might come back" affordance keyed on an identifier, check whether the recovery path even preserves that identifier. (Stale-tab auto-remount tracked as a follow-up: #991.) - [From #920] When a feature needs *extra* data or a *different* query from a shared forge concept (e.g. issue `body`, or `--state closed`), add a **dedicated concept** (`issue-search`) rather than bolting opt-in env flags onto the shared one (`issue-list`). Parameterizing the shared concept couples a feature's needs into a primitive other paths depend on, and — worse — only the GitHub script honored the new flags, so non-GitHub forges would have *silently returned wrong results* (e.g. open issues when "Closed" was requested). A dedicated concept keeps the shared one byte-for-byte unchanged and degrades **honestly**: a forge that hasn't implemented `issue-search` returns null → the UI shows "search unavailable" instead of wrong data. Extends the forge-agnostic-layer lesson [From #909]. - [From #920] Name by layer, not by feature: an HTTP route/concept/lib fn is a *resource* (`/api/issue-search`, `searchIssues`) and should match its sibling resource endpoints (`/api/issue`); only the user-facing UI keeps the *feature* name ("Search Backlog" panel). Mixing the two (a `backlog-search` route feeding an `issue-search` concept) creates a self-inflicted vocabulary seam. +- [From #1018] Against a *moving runtime*, only a deterministic guard holds — instructions, per-agent memory, and `git bisect` do not. The builder write-into-main-checkout bug is intrinsic model/CLI path-synthesis behavior (the model anchors a synthesized absolute path at the inferred repo root, dropping its `.builders//` worktree segment) that drifts across upgrades in both directions. The fix that survives version churn is a `PreToolUse` hook that converts a silent wrong-rooted write into a loud, correctable deny; the role-doc instruction is only a backstop. When a bug's root cause is "the model guessed wrong and got no corrective signal," reach for a runtime invariant, not a better prompt. +- [From #1018] A guard's *surface* and its *blast radius* must match the actual hazard, not the role. The write-guard is builder-only and write-only by design: (a) the architect legitimately owns `main`, so the same hook there is a structural no-op (root resolves to the main checkout) and was deliberately not installed; (b) reads are left unguarded so codev's intentional cross-checkout reads (architect↔builder threads, sibling threads) keep working. Guarding "outside the worktree" symmetrically across roles or across read+write would have broken designed-in behavior. Scope the invariant to where the silent failure actually occurs. +- [From #1018] `fs.writeFileSync` does not create missing parent dirs, and a git worktree only materializes directories that contain *tracked* files — git never checks out an empty dir. A path like `.claude/hooks/` (holding only a generated, intentionally-untracked file) therefore does not exist in a fresh worktree, and even `.claude/` may be absent in an adopter repo that tracks nothing under it. Any code that writes a generated file into a worktree subdir must `mkdir -p` its parent first; don't assume a dir exists just because a sibling tracked dir (e.g. `.claude/skills/`) does. - [From 810] The builder-overview shape is defined twice — the `OverviewBuilder` wire type (`packages/types`) and a structurally-identical local `BuilderOverview` interface in `overview.ts`, kept in sync by hand. Adding a field to only the wire type compiles for clients (vscode/dashboard) but breaks the codev build at the server-side `builders.push({...})` sites. Compounding footgun: the codev package has no `check-types` script, so the mismatch is invisible until a full `pnpm build` runs `tsc` over `codev/src` — vscode/dashboard type-checks pass meanwhile. When touching the overview projection, build the codev package, not just the client type-check. - [From 0395] Prompt-based instructions beat programmatic file manipulation for flexible document generation — the Builder already has context and can write natural responses, while code would need fragile parsing and placeholder logic - [From 0395] Keep specs and plans clean as forward-looking documents — append review history (consultation feedback, lessons learned) to review files, not to the documents being reviewed diff --git a/codev/reviews/1018-builder-worktree-write-guard-p.md b/codev/reviews/1018-builder-worktree-write-guard-p.md new file mode 100644 index 000000000..276d84996 --- /dev/null +++ b/codev/reviews/1018-builder-worktree-write-guard-p.md @@ -0,0 +1,69 @@ +# PIR Review: Builder worktree write-guard + +Fixes #1018 + +## Summary + +Strict-mode builders run in a git worktree nested inside the main checkout; the runtime sometimes synthesizes an absolute `Write`/`Edit` path anchored at the inferred repo root, dropping the `.builders//` segment so the write silently lands in the main checkout. This PR installs a deterministic Claude **PreToolUse hook** into every builder worktree at spawn time that denies `Write`/`Edit` to paths resolving outside the worktree root (allowlisting temp dirs and `~/.claude`), turning a silent main-checkout pollution into a loud, correctable deny. A role-doc backstop documents the failure mode and the relative-path discipline that covers the Bash write surface. + +## Files Changed + +- `packages/codev/src/agent-farm/utils/worktree-write-guard.ts` (+233 / -0) — new; single source of truth (guard script string + `buildWorktreeGuardFiles()`) +- `packages/codev/src/__tests__/worktree-write-guard.test.ts` (+214 / -0) — new; 14 cases exercising the emitted script + the file builder +- `packages/codev/src/agent-farm/utils/harness.ts` (+12 / -2) — `CLAUDE_HARNESS.getWorktreeFiles` emits the guard; signature gains `worktreePath` +- `packages/codev/src/agent-farm/commands/spawn-worktree.ts` (+9 / -4) — `writeWorktreeFiles` `mkdir -p`s the target dir; both call sites pass `worktreePath` +- `packages/codev/src/agent-farm/__tests__/harness.test.ts` (+11 / -4) — contract update (CLAUDE_HARNESS now has `getWorktreeFiles`) +- `codev/roles/builder.md` (+21 / -0) — worktree path-discipline backstop +- `codev-skeleton/roles/builder.md` (+21 / -0) — identical mirror +- `codev/resources/arch.md` (+9 / -0) — COLD: write-guard mechanism under Worktree Management +- `codev/resources/lessons-learned.md` (+3 / -0) — COLD: deterministic-guard / scope-the-hazard / mkdir-before-write lessons + +## Commits + +- `93f455de` [PIR #1018] Add worktree write-guard hook for Claude builders +- `d5ec7156` [PIR #1018] Test the write-guard script and CLAUDE_HARNESS wiring +- `84b2f7b1` [PIR #1018] Document worktree path-discipline backstop in builder role +- `10aa6095` [PIR #1018] Update builder thread for implement phase +- (plus the review/retrospective commit on top) + +## Test Results + +- `npm run build`: ✓ pass (porch check, 6.8s) +- `npm test`: ✓ pass (porch check, 20.2s; 3358 passed, 0 failed, 48 skipped) — 14 new tests in `worktree-write-guard.test.ts` +- Manual verification (human, at the `dev-approval` gate): the running worktree was reviewed and approved. + +Note on running the suite: a full `npm run build` must precede `npm test`. The terminal `session-manager` *integration* tests spawn and connect to the built shellper binary; without a prior full build they fail to connect. This is environmental and unrelated to this change — confirmed by the suite passing cleanly after `npm run build`. + +## Architecture Updates + +**COLD** — added a "Worktree Write-Guard (Issue #1018)" subsection to `codev/resources/arch.md` under Worktree Management: the failure mode, the hook mechanism, where it's wired (`CLAUDE_HARNESS.getWorktreeFiles` → `writeWorktreeFiles`), the allowlist, fail-open behavior, baked-in `CODEV_WORKTREE_ROOT` with `git rev-parse` fallback, and scope (write surface / Claude only / architect unaffected / #1092 separate). + +**HOT** — no change. The hot `arch-critical.md` is capped and full; this fact is reference detail that belongs in the cold archive, and the existing "Worktrees in `.builders/` are Agent-Farm-managed" hot fact already covers the always-on invariant. + +## Lessons Learned Updates + +**COLD** — added three entries to `codev/resources/lessons-learned.md` (Architecture): +1. Against a moving runtime, only a deterministic guard holds — instructions/memory/bisect don't. +2. A guard's surface and blast radius must match the hazard, not the role (builder-only, write-only by design; architect owns `main`; reads stay unguarded to preserve cross-checkout reads). +3. `writeFileSync` doesn't create parent dirs and git only materializes dirs with tracked files — `mkdir -p` before writing a generated file into a worktree subdir. + +**HOT** — no change. `lessons-critical.md` is capped and full; the closest existing hot lesson ("When guessing fails, build a minimal repro") is adjacent but distinct, and these are spec-narrow recipes better kept cold. + +## Things to Look At During PR Review + +- **Allowlist policy** (`worktree-write-guard.ts`): temp dirs + `$HOME/.claude`. The `~/.claude` entry is deliberate so builder *memory* writes are not blocked. A consequence worth a conscious nod: symlinked shared root config (`.codev/config.json`, `.env`) resolves outside the worktree and is **denied** for writes — judged correct (a builder should not mutate shared root config), but it is a behavior boundary. +- **Fail-open philosophy**: any error (bad JSON, unresolvable root) allows the call. A safety net must never brick a builder; worst case reverts to today's unguarded behavior. +- **Path canonicalization**: realpath-of-longest-existing-ancestor + re-append tail, so non-existent new files resolve correctly and macOS `/tmp`→`/private/tmp` is normalized. Covered by tests, but the trickiest part to get right. +- **Determinism vs. fallback**: `CODEV_WORKTREE_ROOT` is baked at spawn (absolute); `git rev-parse --show-toplevel` is the runtime fallback. Both are canonicalized. +- **Scope decision** (recorded at the plan gate): builder-only. The architect/builder asymmetry rationale is in the Lessons Learned. #1092 (consult sub-agent read surface) is intentionally out of scope; the guard module is factored so its boundary logic could later back a consult-side hook. + +## How to Test Locally + +- **View diff**: VSCode sidebar → right-click builder `pir-1018` → **Review Diff** +- **Run dev server**: VSCode sidebar → **Run Dev Server**, or `afx dev pir-1018` +- **What to verify** (spawn a throwaway builder against this branch): + - A `Write` to a main-checkout path (e.g. `/codev/plans/zzz.md`) is **blocked** with a re-root message. + - A `/tmp` (or scratchpad) write **passes**. + - A genuine sibling-thread / cross-checkout **read** still works (reads unaffected). + - A `~/.claude/...` memory write **passes**. + - A correctly worktree-rooted Write **passes** and lands in the worktree; the main checkout is untouched. diff --git a/codev/state/pir-1018_thread.md b/codev/state/pir-1018_thread.md index 29d72f44e..875544c67 100644 --- a/codev/state/pir-1018_thread.md +++ b/codev/state/pir-1018_thread.md @@ -76,3 +76,10 @@ Checks: `npm run build` ✓ (exit 0), `npm test` ✓ (3358 passed, 0 failed). Note: full `npm run build` must run before `npm test` — the terminal session-manager *integration* tests need the built shellper binary; without the build they fail to connect (environmental, not related to this change). + +## Phase: review (iteration 1) + +dev-approval approved by human. Wrote review retrospective; routed COLD governance +updates (arch.md write-guard subsection + 3 lessons-learned entries). HOT tiers +untouched (capped/full). Opening PR next; porch verify runs single-pass 3-way +consult (advisory). Then wait at pr gate for human merge. From b736b9c9072f0c81891e72856c34b546cd26d488 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:26:33 +1000 Subject: [PATCH 14/21] chore(porch): 1018 record PR #1098 --- .../1018-builder-worktree-write-guard-p/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 8ee5f93a0..325e42c09 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -19,4 +19,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T23:22:25.026Z' +updated_at: '2026-06-23T23:26:33.366Z' +pr_history: + - phase: review + pr_number: 1098 + branch: builder/pir-1018 + created_at: '2026-06-23T23:26:33.365Z' From c3b3d2b67ae6367b89c0f235be8a451347e6c1eb Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:26:45 +1000 Subject: [PATCH 15/21] chore(porch): 1018 review build-complete --- .../projects/1018-builder-worktree-write-guard-p/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 325e42c09..5eebe9840 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -16,10 +16,10 @@ gates: pr: status: pending iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-06-23T00:38:54.889Z' -updated_at: '2026-06-23T23:26:33.366Z' +updated_at: '2026-06-23T23:26:45.399Z' pr_history: - phase: review pr_number: 1098 From d395fc45c5f65751ece71da991b6c6cfc5f43c96 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:35:58 +1000 Subject: [PATCH 16/21] [PIR #1018] Install write-guard for all Claude spawn modes (addresses Codex REQUEST_CHANGES) --- .../__tests__/spawn-worktree.test.ts | 21 +++++++++++ .../src/agent-farm/commands/spawn-worktree.ts | 35 +++++++++++++++---- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts b/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts index d042ddc59..35ebe2b51 100644 --- a/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts +++ b/packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts @@ -292,6 +292,27 @@ describe('spawn-worktree', () => { ); expect(opencodeCall).toBeUndefined(); }); + + it('installs the write-guard for claude WITH a role (#1018)', async () => { + getBuilderHarnessMock.mockReturnValueOnce(CLAUDE_HARNESS); + const { writeFileSync } = await import('node:fs'); + const role = { content: 'You are a builder', source: 'codev' }; + buildWorktreeLaunchScript('/tmp/worktree', 'claude', role); + const written = vi.mocked(writeFileSync).mock.calls.map(c => String(c[0])); + expect(written.some(p => p.endsWith('.claude/settings.local.json'))).toBe(true); + expect(written.some(p => p.endsWith('.claude/hooks/worktree-write-guard.cjs'))).toBe(true); + }); + + it('installs the write-guard for claude even WITHOUT a role (#1018 — no-role spawn path)', async () => { + // Regression: the guard must be deterministic across all Claude spawn + // modes. A no-role spawn previously fell through unguarded. + getBuilderHarnessMock.mockReturnValueOnce(CLAUDE_HARNESS); + const { writeFileSync } = await import('node:fs'); + buildWorktreeLaunchScript('/tmp/worktree', 'claude', null); + const written = vi.mocked(writeFileSync).mock.calls.map(c => String(c[0])); + expect(written.some(p => p.endsWith('.claude/settings.local.json'))).toBe(true); + expect(written.some(p => p.endsWith('.claude/hooks/worktree-write-guard.cjs'))).toBe(true); + }); }); // ========================================================================= diff --git a/packages/codev/src/agent-farm/commands/spawn-worktree.ts b/packages/codev/src/agent-farm/commands/spawn-worktree.ts index 677179aab..239c37681 100644 --- a/packages/codev/src/agent-farm/commands/spawn-worktree.ts +++ b/packages/codev/src/agent-farm/commands/spawn-worktree.ts @@ -14,7 +14,7 @@ import { globSync } from 'glob'; import type { Config, ProtocolDefinition } from '../types.js'; import { logger, fatal } from '../utils/logger.js'; import { getBuilderHarness, getWorktreeConfig } from '../utils/config.js'; -import { shellEscapeSingleQuote } from '../utils/harness.js'; +import { shellEscapeSingleQuote, type HarnessProvider } from '../utils/harness.js'; import { defaultSessionOptions } from '../../terminal/index.js'; import { run, runStreaming, commandExists } from '../utils/shell.js'; import { fetchIssueOrThrow, type ForgeIssue } from '../../lib/github.js'; @@ -715,6 +715,23 @@ function writeWorktreeFiles( } } +/** + * Install harness-specific worktree files. Role-independent — keyed on the + * worktree path — so the Claude write-guard hook (Issue #1018) lands for EVERY + * fresh Claude spawn mode, not only role-bearing ones. `roleContent`/`roleFile` + * are '' for no-role spawns; CLAUDE_HARNESS ignores them and uses only the path. + */ +function installHarnessWorktreeFiles( + harness: HarnessProvider, + roleContent: string, + roleFile: string, + worktreePath: string, +): void { + if (harness.getWorktreeFiles) { + writeWorktreeFiles(harness.getWorktreeFiles(roleContent, roleFile, worktreePath), worktreePath); + } +} + /** * Start a terminal session for a builder. * @@ -775,9 +792,7 @@ done // Write any harness-specific worktree files (e.g., opencode.json for OpenCode, // the write-guard hook for Claude — Issue #1018) - if (harness.getWorktreeFiles) { - writeWorktreeFiles(harness.getWorktreeFiles(roleWithPort, roleFile, worktreePath), worktreePath); - } + installHarnessWorktreeFiles(harness, roleWithPort, roleFile, worktreePath); scriptContent = `#!/bin/bash cd "${worktreePath}" @@ -792,6 +807,11 @@ done // Fresh spawn without role injection. const promptFile = resolve(worktreePath, '.builder-prompt.txt'); writeFileSync(promptFile, prompt); + + // Install harness worktree files even without a role, so the write-guard + // (Issue #1018) is deterministic across all Claude spawn modes. + installHarnessWorktreeFiles(getBuilderHarness(config.workspaceRoot), '', '', worktreePath); + scriptContent = `#!/bin/bash cd "${worktreePath}" while true; do @@ -865,9 +885,7 @@ export function buildWorktreeLaunchScript( // Write any harness-specific worktree files (e.g., opencode.json for OpenCode, // the write-guard hook for Claude — Issue #1018) - if (harness.getWorktreeFiles) { - writeWorktreeFiles(harness.getWorktreeFiles(roleWithPort, roleFile, worktreePath), worktreePath); - } + installHarnessWorktreeFiles(harness, roleWithPort, roleFile, worktreePath); return `#!/bin/bash cd "${worktreePath}" @@ -879,6 +897,9 @@ ${envBlock}while true; do done `; } + // Install harness worktree files even without a role, so the write-guard + // (Issue #1018) is deterministic across all Claude spawn modes. + installHarnessWorktreeFiles(getBuilderHarness(workspaceRoot), '', '', worktreePath); return `#!/bin/bash cd "${worktreePath}" while true; do From 764a75e4e715d19ad0ecb3ffb9ef64e73361b250 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:35:58 +1000 Subject: [PATCH 17/21] [PIR #1018] Record 3-way consultation outcome in review --- codev/reviews/1018-builder-worktree-write-guard-p.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/codev/reviews/1018-builder-worktree-write-guard-p.md b/codev/reviews/1018-builder-worktree-write-guard-p.md index 276d84996..f82f1edcd 100644 --- a/codev/reviews/1018-builder-worktree-write-guard-p.md +++ b/codev/reviews/1018-builder-worktree-write-guard-p.md @@ -49,6 +49,14 @@ Note on running the suite: a full `npm run build` must precede `npm test`. The t **HOT** — no change. `lessons-critical.md` is capped and full; the closest existing hot lesson ("When guessing fails, build a minimal repro") is adjacent but distinct, and these are spec-narrow recipes better kept cold. +## 3-Way Consultation Outcome (single advisory pass) + +- **Gemini**: APPROVE (HIGH). **Claude**: APPROVE (HIGH). **Codex**: REQUEST_CHANGES (HIGH). +- **Codex finding (addressed):** the guard was only emitted in the *role-bearing* spawn branches, so a Claude builder spawned **without a role** fell through the unguarded `else` branches in `startBuilderSession` and `buildWorktreeLaunchScript` and never received the guard — not deterministic across all spawn modes. Real gap; fixed. Harness-file installation is now role-independent via a shared `installHarnessWorktreeFiles()` helper called from **all four** fresh-spawn branches (role + no-role, both functions). Added two regression tests in `spawn-worktree.test.ts` asserting the guard files are written for Claude both with and without a role; the no-role one fails without this fix. +- **Claude minor observation (noted, not changed):** if an adopter already has a `.claude/settings.local.json` containing its own `hooks.PreToolUse`, `writeWorktreeFiles`'s shallow merge (`{...existing, ...incoming}`) overwrites the whole `hooks` key. Near-zero risk today (fresh worktree; the file is generated/untracked), so deferred — flagged here for a future deep-merge iteration if needed. +- **Known edge (not a spawn-mode gap):** the `--resume` path reuses an existing worktree and does not re-emit; a worktree first spawned *before* this feature and later resumed won't retroactively get the guard. `afx setup` or a fresh spawn covers it. +- PIR is single-pass — none of the above was independently re-reviewed by the models. The human at the `pr` gate is the remaining check on the fix. + ## Things to Look At During PR Review - **Allowlist policy** (`worktree-write-guard.ts`): temp dirs + `$HOME/.claude`. The `~/.claude` entry is deliberate so builder *memory* writes are not blocked. A consequence worth a conscious nod: symlinked shared root config (`.codev/config.json`, `.env`) resolves outside the worktree and is **denied** for writes — judged correct (a builder should not mutate shared root config), but it is a behavior boundary. From cbae10b8766dae606d91bbd899c308401a834a0e Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:36:36 +1000 Subject: [PATCH 18/21] chore(porch): 1018 pr gate-requested --- .../projects/1018-builder-worktree-write-guard-p/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml index 5eebe9840..4582d4f58 100644 --- a/codev/projects/1018-builder-worktree-write-guard-p/status.yaml +++ b/codev/projects/1018-builder-worktree-write-guard-p/status.yaml @@ -15,13 +15,15 @@ gates: 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:26:45.399Z' +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 From 86cac57796462226dca34d0c8a034a55c566e129 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:37:10 +1000 Subject: [PATCH 19/21] [PIR #1018] Thread: record consultation outcome + no-role fix --- codev/state/pir-1018_thread.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/codev/state/pir-1018_thread.md b/codev/state/pir-1018_thread.md index 875544c67..0596157ef 100644 --- a/codev/state/pir-1018_thread.md +++ b/codev/state/pir-1018_thread.md @@ -83,3 +83,18 @@ dev-approval approved by human. Wrote review retrospective; routed COLD governan updates (arch.md write-guard subsection + 3 lessons-learned entries). HOT tiers untouched (capped/full). Opening PR next; porch verify runs single-pass 3-way consult (advisory). Then wait at pr gate for human merge. + +### PR #1098 + consultation outcome +3-way (single pass): gemini=APPROVE, claude=APPROVE, codex=REQUEST_CHANGES. +Codex caught a real gap: guard only emitted in role-bearing spawn branches, so +no-role Claude builders bypassed it. FIXED — role-independent +`installHarnessWorktreeFiles` helper wired into all 4 fresh-spawn branches +(startBuilderSession + buildWorktreeLaunchScript, role + no-role) + 2 regression +tests in spawn-worktree.test.ts (no-role test fails pre-fix). resume path +intentionally excluded (existing worktree already guarded). Claude's shallow-merge +note deferred (near-zero risk). Build+test green (3360 passed). Dispositions in +codev/projects/1018-*/1018-review-iter1-rebuttals.md. + +### Status: pr gate PENDING +Architect notified (led with REQUEST_CHANGES + fix). Waiting for human to merge + +approve pr gate. Will NOT merge until porch reports gate_status: approved. From dfd0f26476aafc622a21f0bde9b1945c9862c066 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:48:46 +1000 Subject: [PATCH 20/21] [PIR #1018] Fix Linux CI: anchor guard test fixtures outside /tmp (allowlisted) --- .../__tests__/worktree-write-guard.test.ts | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/codev/src/__tests__/worktree-write-guard.test.ts b/packages/codev/src/__tests__/worktree-write-guard.test.ts index 2aad15f02..d1b0078af 100644 --- a/packages/codev/src/__tests__/worktree-write-guard.test.ts +++ b/packages/codev/src/__tests__/worktree-write-guard.test.ts @@ -10,7 +10,6 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { spawnSync, execSync } from 'node:child_process'; import * as fs from 'node:fs'; -import * as os from 'node:os'; import * as path from 'node:path'; import { WORKTREE_WRITE_GUARD_SCRIPT, @@ -19,6 +18,16 @@ import { GUARD_SETTINGS_RELPATH, } from '../agent-farm/utils/worktree-write-guard.js'; +// IMPORTANT (Issue #1018, CI fix): fixtures must NOT live under the OS temp dir. +// The guard allowlists /tmp and /private/tmp, and on Linux `os.tmpdir()` IS /tmp, +// so a fake "outside-worktree" path placed there would be allowlisted and the +// deny-tests would wrongly pass-as-allow (green on macOS where tmpdir is +// /var/folders, red on Linux CI). Anchoring fixtures under the package's +// node_modules (gitignored, never allowlisted, literal non-symlinked path on +// both platforms) makes the deny-tests platform-independent and exercises the +// same literal-path comparison Linux production hits. +const FIXTURE_HOME = path.join(path.resolve(__dirname, '..', '..'), 'node_modules', '.cguard-fixtures'); + let base: string; let mainCheckout: string; let worktree: string; @@ -26,7 +35,8 @@ let homeDir: string; let scriptPath: string; beforeAll(() => { - base = fs.mkdtempSync(path.join(os.tmpdir(), 'cguard-')); + fs.mkdirSync(FIXTURE_HOME, { recursive: true }); + base = fs.mkdtempSync(path.join(FIXTURE_HOME, 'cguard-')); mainCheckout = path.join(base, 'main'); worktree = path.join(mainCheckout, '.builders', 'wt'); homeDir = path.join(base, 'home'); @@ -37,7 +47,7 @@ beforeAll(() => { }); afterAll(() => { - fs.rmSync(base, { recursive: true, force: true }); + fs.rmSync(FIXTURE_HOME, { recursive: true, force: true }); }); interface GuardResult { @@ -47,9 +57,10 @@ interface GuardResult { } /** - * Run the guard with a deterministic env. TMPDIR is deliberately omitted so the - * temp-backed fixture dirs are NOT treated as an allowlisted temp dir — only the - * worktree, /tmp, /private/tmp, and $HOME/.claude should pass. + * Run the guard with a deterministic env. TMPDIR is deliberately omitted (and + * fixtures live outside the OS temp dir, see FIXTURE_HOME) so that only the + * worktree, /tmp, /private/tmp, and $HOME/.claude are allowlisted — nothing else + * should pass. */ function runGuard( toolName: string, @@ -152,7 +163,7 @@ describe('worktree-write-guard script', () => { }); it('falls back to `git rev-parse` when CODEV_WORKTREE_ROOT is unset', () => { - const gitRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'cguard-git-')); + const gitRepo = fs.mkdtempSync(path.join(FIXTURE_HOME, 'cguard-git-')); try { const env = { cwd: gitRepo, stdio: 'pipe' as const }; execSync('git init -q', env); From b0f23ff1874daa0a1585364f210daef14a303dba Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 24 Jun 2026 09:56:49 +1000 Subject: [PATCH 21/21] [PIR #1018] Record Linux CI fix + CMAP-3 re-run (iter2) in review/thread --- .../1018-builder-worktree-write-guard-p.md | 8 ++++++++ codev/state/pir-1018_thread.md | 19 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/codev/reviews/1018-builder-worktree-write-guard-p.md b/codev/reviews/1018-builder-worktree-write-guard-p.md index f82f1edcd..57cb96c65 100644 --- a/codev/reviews/1018-builder-worktree-write-guard-p.md +++ b/codev/reviews/1018-builder-worktree-write-guard-p.md @@ -57,6 +57,14 @@ Note on running the suite: a full `npm run build` must precede `npm test`. The t - **Known edge (not a spawn-mode gap):** the `--resume` path reuses an existing worktree and does not re-emit; a worktree first spawned *before* this feature and later resumed won't retroactively get the guard. `afx setup` or a fresh spawn covers it. - PIR is single-pass — none of the above was independently re-reviewed by the models. The human at the `pr` gate is the remaining check on the fix. +### Iteration 2 (architect-directed re-run, after the Linux CI fix) + +CI on the first push failed on the **Linux** runner: the guard test placed its fake repo under `os.tmpdir()`, which is `/tmp` on Linux — and the guard correctly allowlists `/tmp`, so the "outside-worktree" deny-tests were allowlisted-as-allowed (green on macOS where `tmpdir` is `/var/folders`, red on Linux). **Root cause: test fixture location, not the guard** — the guard already canonicalizes both sides with `realpathSync`, and a literal non-`/tmp` path (verified against the compiled guard) is correctly denied on Linux. Production worktrees never live under `/tmp`, so the guard was always correct in production. + +**Fix:** anchor the guard test fixtures under the package's gitignored `node_modules/.cguard-fixtures` (never allowlisted; literal, non-symlinked on both platforms). This makes the deny-tests platform-independent and actually exercises the Linux-style literal-path comparison. The `/tmp`/`/private/tmp` *allow*-tests stay (they cover the symlink-normalization/allowlist path). + +Re-verified: `npm run build` ✓, `npm test` ✓ (3360 passed); **CI green** on the fix commit (Tests + CLI Integration Tests). Re-ran CMAP-3 on the updated diff: **Codex APPROVE (HIGH)**, **Claude APPROVE (HIGH)**, **Gemini skipped** (agy timed out — non-blocking). No REQUEST_CHANGES remaining. + ## Things to Look At During PR Review - **Allowlist policy** (`worktree-write-guard.ts`): temp dirs + `$HOME/.claude`. The `~/.claude` entry is deliberate so builder *memory* writes are not blocked. A consequence worth a conscious nod: symlinked shared root config (`.codev/config.json`, `.env`) resolves outside the worktree and is **denied** for writes — judged correct (a builder should not mutate shared root config), but it is a behavior boundary. diff --git a/codev/state/pir-1018_thread.md b/codev/state/pir-1018_thread.md index 0596157ef..99c0ba506 100644 --- a/codev/state/pir-1018_thread.md +++ b/codev/state/pir-1018_thread.md @@ -95,6 +95,23 @@ intentionally excluded (existing worktree already guarded). Claude's shallow-mer note deferred (near-zero risk). Build+test green (3360 passed). Dispositions in codev/projects/1018-*/1018-review-iter1-rebuttals.md. -### Status: pr gate PENDING +### Status: pr gate PENDING (iter1) Architect notified (led with REQUEST_CHANGES + fix). Waiting for human to merge + approve pr gate. Will NOT merge until porch reports gate_status: approved. + +### Linux CI failure + fix (architect-flagged) +CI red on Linux: 3 deny-tests in worktree-write-guard.test.ts allowed-as-pass. +Root cause = TEST FIXTURE, not the guard: fixtures lived under os.tmpdir(), which +is /tmp on Linux, and the guard correctly allowlists /tmp → the fake +"outside-worktree" path was allowlisted. Guard already realpaths both sides; +verified the compiled guard DENIES a literal non-/tmp path (Linux-equivalent). +Production worktrees never live under /tmp → guard always correct in prod. +Fix: anchor fixtures under packages/codev/node_modules/.cguard-fixtures +(gitignored, never allowlisted, literal path). Kept /tmp allow-tests. +Re-verified: build ✓, test ✓ (3360), CI GREEN on dfd0f264 (Tests + CLI). +CMAP-3 re-run (iter2, architect-directed): codex=APPROVE, claude=APPROVE, +gemini=skipped (agy timeout, non-blocking). No REQUEST_CHANGES remaining. + +### Status: pr gate PENDING (iter2) — CI green, CMAP-3 clear +Waiting for human to review PR #1098 + approve pr gate. Will NOT merge until +porch reports gate_status: approved.