From 0cd7c8008e0d82e80c9fa28673fdb30cf005cf49 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 8 Jul 2026 13:30:47 +0700 Subject: [PATCH 01/23] chore(porch): 1145 init pir --- .../status.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml new file mode 100644 index 000000000..389995cf2 --- /dev/null +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -0,0 +1,18 @@ +id: '1145' +title: codev-adopt-launchinstance-mai +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-07-08T06:30:47.411Z' +updated_at: '2026-07-08T06:30:47.412Z' From 83d0723b011cecc8683900060a4cf53fbfc9fcdd Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 8 Jul 2026 13:35:54 +0700 Subject: [PATCH 02/23] [PIR #1145] Plan draft --- .../1145-codev-adopt-launchinstance-mai.md | 88 +++++++++++++++++++ codev/state/pir-1145_thread.md | 13 +++ 2 files changed, 101 insertions(+) create mode 100644 codev/plans/1145-codev-adopt-launchinstance-mai.md create mode 100644 codev/state/pir-1145_thread.md diff --git a/codev/plans/1145-codev-adopt-launchinstance-mai.md b/codev/plans/1145-codev-adopt-launchinstance-mai.md new file mode 100644 index 000000000..46f70fd25 --- /dev/null +++ b/codev/plans/1145-codev-adopt-launchinstance-mai.md @@ -0,0 +1,88 @@ +# PIR Plan: Stop launchInstance from hijacking unrelated Claude sessions on fresh workspaces + +## Understanding + +When Tower's `launchInstance` creates the `main` architect for a workspace, it decides between resuming a persisted conversation and spawning fresh. The decision code at `packages/codev/src/agent-farm/servers/tower-instances.ts:489-499` is: + +1. Read the stored session id from the `main` architect row in `~/.agent-farm/global.db` (keyed by `workspace_path`). Fresh workspace: no row, so `storedSessionId = null`. +2. If no stored id, run the "legacy bridge" fallback: when `getArchitects(resolvedPath).length <= 1`, call `harness.buildResume(workspacePath)` which is mtime-based jsonl discovery (`findLatestSessionId` in `packages/codev/src/agent-farm/utils/claude-session-discovery.ts`). + +The fallback was introduced (#832) to bridge pre-#832 architect rows that exist but carry no `session_id`. But the gate `length <= 1` is satisfied by **zero** rows, so the fallback also fires for a workspace that has *never had an architect at all*: exactly the `codev adopt` / first-touch `launchInstance` case in issue #1145. + +`findLatestSessionId` scans `~/.claude/projects//` for the newest `.jsonl`. Claude Code persists *every* interactive session there, including the user's personal (non-Codev) conversations. So if the user ever chatted with Claude Code in that project directory before adopting Codev, the brand-new "main architect" launches with `--resume `. Two compounding effects: + +- The architect inherits the personal conversation's full context (the "hijack"). +- The resume branch of `resolveArchitectLaunch` (`packages/codev/src/agent-farm/servers/tower-utils.ts:223-230`) deliberately skips role injection (a genuinely-resumed architect already has its role in-conversation) — so the hijacked session also never receives the architect role prompt. + +There is a second, cross-project vector: `encodeClaudeProjectDir` (`claude-session-discovery.ts:28-30`) replaces both `/` and `.` with `-`, which is lossy. Distinct paths can collide into the same store directory (e.g. `/x/foo.bar` and `/x/foo/bar` both encode to `-x-foo-bar`), so discovery can surface a session that genuinely belongs to a *different project*. The session jsonl records the true `cwd` on its user/assistant lines (verified against a real session file), which gives us an ownership tag to check. + +Note: we never pass a bare `--resume` (the "most recent global session" form hypothesized in the issue) — every resume carries an explicit uuid. The hijack mechanism is the discovery fallback firing on fresh workspaces, plus the lossy dir encoding. + +## Proposed Change + +Two surgical changes, matching the issue's fix sketch: + +### 1. Gate the discovery fallback on a pre-existing legacy architect row (tower-instances.ts) + +The fallback exists solely to bridge legacy rows (pre-#832: row present, `session_id` NULL). Make the gate express that: + +- Read the `main` row once. If it has a `sessionId`, use it (unchanged). +- Run jsonl discovery **only when the `main` row exists** (legacy row with no stored id) **and** it is the sole architect (the existing ambiguity guard). +- No row at all (fresh workspace, the `codev adopt` case) → always spawn fresh with a newly minted session id and role injection. + +Degradation note: today the two reads have independent `try` blocks so a `global.db` read failure still allows discovery. With row-existence gating, an unreadable DB means we cannot prove a legacy row exists, so we spawn fresh. That trades a rare resume-miss (context loss on a corrupted DB) for never hijacking — the safe direction. + +This does not regress normal stop/start resume: `afx workspace stop` preserves architect rows (`commands/stop.ts:42,98`), so a restarted workspace resumes via the stored-UUID path, not via discovery. + +### 2. Verify session ownership before attach (claude-session-discovery.ts + harness.ts + tower-utils.ts) + +**Discovery side** — teach `findLatestSessionId` to verify ownership: iterate candidate jsonls newest-first and return the first whose recorded `cwd` matches the requested path. Implementation: read the file's first ~64KB, scan lines for the first record carrying a `cwd` field, compare after `realpath`-canonicalizing both sides. Candidates with a mismatched `cwd` are skipped (encoding collision → not ours); candidates with *no* `cwd` record in the sample (e.g. a session that never got a user message) are skipped too — there is nothing worth resuming in them and skipping is the safe default. This hardens every `buildResume` consumer: the architect legacy fallback and builder resume (#831/#929) alike. + +**Stored-id side** — the issue also asks that a passed session id be cross-checked. Add an optional, harness-gated capability alongside the existing `session` block in `HarnessProvider`: + +```ts +session?: { + newSessionArgs(sessionId: string): string[]; + resumeArgs(sessionId: string): string[]; + /** Optional: return false if the session on disk does not belong to cwd. */ + verifyOwnership?(sessionId: string, cwd: string, opts?: { homeDir?: string }): boolean; +} +``` + +Only Claude implements it: the session file must exist at `~/.claude/projects//.jsonl` *and* its recorded `cwd` must match (same helper as above). `resolveArchitectLaunch` (tower-utils.ts) calls it before taking the resume branch; on `false` it falls through to the fresh branch (new id, role injection). Because `resolveArchitectLaunch` is the single choke point, this covers `launchInstance`, `add-architect` sibling re-spawn, and both shellper restart-bake sites (via `resolveArchitectRestart`) in one place. Harnesses without `verifyOwnership` keep today's behavior (trust the stored id), preserving harness neutrality. + +Side benefit: a stored id whose jsonl was deleted (stale-id class, cousin of #929's crash-loop) now degrades to a fresh spawn instead of a broken `--resume`. + +## Files to Change + +- `packages/codev/src/agent-farm/utils/claude-session-discovery.ts` — add `sessionFileCwd`-style helper (scan first lines for `cwd`), make `findLatestSessionId` filter candidates by ownership (newest-first, first match wins); export a `verifySessionOwnership(absolutePath, sessionId, opts?)` for the harness. +- `packages/codev/src/agent-farm/utils/harness.ts:68-73,132-135` — add optional `session.verifyOwnership` to the `HarnessProvider` interface; implement it on `CLAUDE_HARNESS` using the discovery helper. +- `packages/codev/src/agent-farm/servers/tower-utils.ts:208-236` — in `resolveArchitectLaunch`, verify `storedSessionId` ownership (when the harness offers it) before the resume branch; on failure log-worthy fall-through to fresh (return `resumed: false`, fresh minted id). +- `packages/codev/src/agent-farm/servers/tower-instances.ts:489-499` — restructure the fallback gate: discovery only when a legacy `main` row exists without a session id and is the sole architect; update the long #832 comment block to document the new gate and the fresh-workspace guarantee. +- `packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts` — new cases: mismatched-cwd jsonl skipped; falls back to next-newest matching; no matching candidate → null; cwd-less jsonl skipped; `verifySessionOwnership` true/false paths. +- `packages/codev/src/agent-farm/__tests__/tower-utils.test.ts` — `resolveArchitectLaunch`: stored id failing ownership → fresh spawn with role injection and a *new* session id. +- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` — `launchInstance` on a fresh workspace (no architect row) with a decoy jsonl in the encoded project dir → spawn args contain no `--resume`; legacy row without session id + owned jsonl → still resumes (bridge preserved). + +No `codev/` ↔ `codev-skeleton/` mirroring needed — all changes are package source (`packages/codev/src`), not framework files. + +## Risks & Alternatives Considered + +- **Risk: breaking the legacy bridge.** Pre-#832 rows must still self-migrate to stored-UUID resume. Mitigated by keeping discovery for the row-exists-without-id case and adding a regression test for it. +- **Risk: over-strict ownership check drops valid resumes.** A jsonl whose first 64KB lacks a `cwd` line would be skipped. Sampled real session files show `cwd` appears on the first user message (line ~5); sessions with no user message carry no resumable value. Accepted. +- **Risk: realpath comparison surprises with symlinked workspace paths.** Canonicalize both sides with `fs.realpathSync` (falling back to the raw string if realpath throws) so symlink vs. resolved-path launches compare equal. +- **Alternative: only fix the gate (change `<= 1` to row-exists), skip ownership verification.** Rejected: leaves the encoding-collision cross-project vector and the issue explicitly asks for the ownership check. +- **Alternative: verify ownership inside Tower instead of the harness.** Rejected: jsonl layout is Claude-specific; per the harness abstraction rule, agent-specific session mechanics live behind `HarnessProvider`. +- **Alternative: stop skipping role injection on resume.** Rejected: out of scope; genuine resumes already have the role in-conversation, and double-injection would bloat context. + +## Test Plan + +- **Unit** (vitest, run from the worktree: `pnpm --filter @cluesmith/codev test`): + - discovery: ownership filtering, next-newest fallback, cwd-less skip, collision skip, `verifySessionOwnership` behavior with a fake `homeDir`. + - `resolveArchitectLaunch`: stored id failing verification → fresh (new uuid, role args present, `resumed: false`); passing verification → resume unchanged; harness without `verifyOwnership` → resume unchanged. + - `launchInstance`: fresh workspace + decoy personal jsonl → no `--resume` in spawn args; legacy row bridge still resumes. +- **Manual** (for the dev-approval gate): + 1. Create a scratch project dir (no `codev/`), run `claude` in it briefly so a personal session jsonl exists for that cwd, and exit. + 2. From the scratch dir, trigger the launch path (`codev adopt` then workspace start, or VS Code auto-adopt) against the locally installed build (`pnpm build && pnpm -w run local-install`). + 3. Observe the main architect terminal: it must open a **fresh** conversation (architect role prompt active, no prior chat context), and Tower's log must not print "Resuming architect 'main' …". + 4. Restart the workspace (`afx workspace stop` / `start`): the architect must now resume its *own* stored session (log shows "Resuming architect 'main' session …" with the minted id). +- **Cross-platform**: n/a (server-side Node path handling only; path canonicalization uses `fs.realpathSync` which is platform-neutral). diff --git a/codev/state/pir-1145_thread.md b/codev/state/pir-1145_thread.md new file mode 100644 index 000000000..8a5ddf08a --- /dev/null +++ b/codev/state/pir-1145_thread.md @@ -0,0 +1,13 @@ +# pir-1145 — builder thread + +## 2026-07-08 — Plan phase + +Investigated issue #1145 (main architect hijacks unrelated Claude session on fresh workspaces). + +**Root cause**: the #832 legacy jsonl-discovery fallback in `launchInstance` (tower-instances.ts:493-499) is gated on `getArchitects().length <= 1`, which a *fresh* workspace (0 rows) satisfies. So first-ever launch runs mtime discovery over `~/.claude/projects//` and resumes the user's personal Claude conversation if one exists for that directory — with role injection skipped (resume branch of `resolveArchitectLaunch`). Second vector: the project-dir encoding maps both `/` and `.` to `-` (lossy), so distinct paths can collide into one store dir → genuine cross-project pickup. + +**Plan** (codev/plans/1145-codev-adopt-launchinstance-mai.md): +1. Gate discovery on a legacy `main` row *existing* without a session id (the bridge case it was built for). No row → always fresh. +2. Ownership verification: jsonl candidates must record a `cwd` matching the requested path (discovery side), plus an optional harness-gated `session.verifyOwnership` checked in `resolveArchitectLaunch` before any stored-id resume. + +Confirmed `afx workspace stop` preserves architect rows, so normal stop/start resume is unaffected. Sitting at plan-approval gate. From 2ac751e4ed2f42afb16e08a0f23ba01624fee081 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 8 Jul 2026 13:36:03 +0700 Subject: [PATCH 03/23] chore(porch): 1145 plan-approval gate-requested --- codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 389995cf2..ce83d74e2 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -7,6 +7,7 @@ current_plan_phase: null gates: plan-approval: status: pending + requested_at: '2026-07-08T06:36:03.156Z' dev-approval: status: pending pr: @@ -15,4 +16,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-08T06:30:47.412Z' +updated_at: '2026-07-08T06:36:03.157Z' From 3e0382dcba82c182f0be3c32ba589537ba9abca4 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:24:08 +0700 Subject: [PATCH 04/23] [PIR #1145] Plan revised: drop architect discovery fallback entirely --- .../1145-codev-adopt-launchinstance-mai.md | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/codev/plans/1145-codev-adopt-launchinstance-mai.md b/codev/plans/1145-codev-adopt-launchinstance-mai.md index 46f70fd25..7166ca205 100644 --- a/codev/plans/1145-codev-adopt-launchinstance-mai.md +++ b/codev/plans/1145-codev-adopt-launchinstance-mai.md @@ -22,21 +22,23 @@ Note: we never pass a bare `--resume` (the "most recent global session" form hyp Two surgical changes, matching the issue's fix sketch: -### 1. Gate the discovery fallback on a pre-existing legacy architect row (tower-instances.ts) +### 1. Remove the architect discovery fallback entirely (tower-instances.ts) -The fallback exists solely to bridge legacy rows (pre-#832: row present, `session_id` NULL). Make the gate express that: +The fallback exists to bridge legacy rows (pre-#832: row present, `session_id` NULL). But even gated on row existence it cannot be made safe: mtime discovery picks *the newest session in the cwd*, and a row only proves an architect once ran there — not that the newest jsonl is the architect's. A personal Claude conversation held in the workspace directory after the architect's last run would still be hijacked, and cwd ownership verification (change 2) cannot catch it because the personal session's cwd genuinely matches. -- Read the `main` row once. If it has a `sessionId`, use it (unchanged). -- Run jsonl discovery **only when the `main` row exists** (legacy row with no stored id) **and** it is the sole architect (the existing ambiguity guard). -- No row at all (fresh workspace, the `codev adopt` case) → always spawn fresh with a newly minted session id and role injection. +So `launchInstance`'s session resolution becomes: read `storedSessionId` from the `main` row; resume it if present (after ownership verification, change 2); otherwise spawn fresh with a newly minted session id and role injection. No jsonl discovery on the architect path, ever. -Degradation note: today the two reads have independent `try` blocks so a `global.db` read failure still allows discovery. With row-existence gating, an unreadable DB means we cannot prove a legacy row exists, so we spawn fresh. That trades a rare resume-miss (context loss on a corrupted DB) for never hijacking — the safe direction. +Consequences: -This does not regress normal stop/start resume: `afx workspace stop` preserves architect rows (`commands/stop.ts:42,98`), so a restarted workspace resumes via the stored-UUID path, not via discovery. +- Fresh workspace (`codev adopt` / first touch): always fresh — the issue's fix. +- Legacy pre-#832 row without a stored id: spawns fresh, loses at most one conversation's context, mints and persists an id, and is on the stored-UUID path forever after. Exposure is limited to workspaces untouched since #832 shipped. +- `global.db` read failure: fresh spawn (unchanged in spirit — never hijack on uncertainty). +- Normal stop/start resume is unaffected: `afx workspace stop` preserves architect rows (`commands/stop.ts:42,98`), so restarts resume via the stored-UUID path, which never depended on discovery. +- `buildResume` / `findLatestSessionId` remain in place for **builder** resume (#831/#929), where the worktree cwd is effectively private to the builder so mtime discovery stays sound. The `buildResume` doc comment in `harness.ts:75-93` (which names the architect legacy fallback as a consumer) is updated to reflect builders as the sole remaining consumer. ### 2. Verify session ownership before attach (claude-session-discovery.ts + harness.ts + tower-utils.ts) -**Discovery side** — teach `findLatestSessionId` to verify ownership: iterate candidate jsonls newest-first and return the first whose recorded `cwd` matches the requested path. Implementation: read the file's first ~64KB, scan lines for the first record carrying a `cwd` field, compare after `realpath`-canonicalizing both sides. Candidates with a mismatched `cwd` are skipped (encoding collision → not ours); candidates with *no* `cwd` record in the sample (e.g. a session that never got a user message) are skipped too — there is nothing worth resuming in them and skipping is the safe default. This hardens every `buildResume` consumer: the architect legacy fallback and builder resume (#831/#929) alike. +**Discovery side** — teach `findLatestSessionId` to verify ownership: iterate candidate jsonls newest-first and return the first whose recorded `cwd` matches the requested path. Implementation: read the file's first ~64KB, scan lines for the first record carrying a `cwd` field, compare after `realpath`-canonicalizing both sides. Candidates with a mismatched `cwd` are skipped (encoding collision → not ours); candidates with *no* `cwd` record in the sample (e.g. a session that never got a user message) are skipped too — there is nothing worth resuming in them and skipping is the safe default. With the architect fallback removed (change 1), builder resume (#831/#929) is `buildResume`'s sole consumer, and this protects it from encoding-collision pickups. **Stored-id side** — the issue also asks that a passed session id be cross-checked. Add an optional, harness-gated capability alongside the existing `session` block in `HarnessProvider`: @@ -56,18 +58,19 @@ Side benefit: a stored id whose jsonl was deleted (stale-id class, cousin of #92 ## Files to Change - `packages/codev/src/agent-farm/utils/claude-session-discovery.ts` — add `sessionFileCwd`-style helper (scan first lines for `cwd`), make `findLatestSessionId` filter candidates by ownership (newest-first, first match wins); export a `verifySessionOwnership(absolutePath, sessionId, opts?)` for the harness. -- `packages/codev/src/agent-farm/utils/harness.ts:68-73,132-135` — add optional `session.verifyOwnership` to the `HarnessProvider` interface; implement it on `CLAUDE_HARNESS` using the discovery helper. +- `packages/codev/src/agent-farm/utils/harness.ts:68-73,132-135` — add optional `session.verifyOwnership` to the `HarnessProvider` interface; implement it on `CLAUDE_HARNESS` using the discovery helper. Update the `buildResume` doc comment (`harness.ts:75-93`) to name builder resume as its sole remaining consumer. - `packages/codev/src/agent-farm/servers/tower-utils.ts:208-236` — in `resolveArchitectLaunch`, verify `storedSessionId` ownership (when the harness offers it) before the resume branch; on failure log-worthy fall-through to fresh (return `resumed: false`, fresh minted id). -- `packages/codev/src/agent-farm/servers/tower-instances.ts:489-499` — restructure the fallback gate: discovery only when a legacy `main` row exists without a session id and is the sole architect; update the long #832 comment block to document the new gate and the fresh-workspace guarantee. +- `packages/codev/src/agent-farm/servers/tower-instances.ts:469-499` — delete the discovery fallback block (`getArchitects().length <= 1` + `buildResume`); keep only the stored-id read. Replace the long #832 comment block with the new invariant: architect resume comes exclusively from the stored session id on the workspace-scoped row; no row → fresh. - `packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts` — new cases: mismatched-cwd jsonl skipped; falls back to next-newest matching; no matching candidate → null; cwd-less jsonl skipped; `verifySessionOwnership` true/false paths. - `packages/codev/src/agent-farm/__tests__/tower-utils.test.ts` — `resolveArchitectLaunch`: stored id failing ownership → fresh spawn with role injection and a *new* session id. -- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` — `launchInstance` on a fresh workspace (no architect row) with a decoy jsonl in the encoded project dir → spawn args contain no `--resume`; legacy row without session id + owned jsonl → still resumes (bridge preserved). +- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` — `launchInstance` on a fresh workspace (no architect row) with a decoy jsonl in the encoded project dir → spawn args contain no `--resume`; legacy row without session id → also spawns fresh (fallback removed); row with stored id → resumes. No `codev/` ↔ `codev-skeleton/` mirroring needed — all changes are package source (`packages/codev/src`), not framework files. ## Risks & Alternatives Considered -- **Risk: breaking the legacy bridge.** Pre-#832 rows must still self-migrate to stored-UUID resume. Mitigated by keeping discovery for the row-exists-without-id case and adding a regression test for it. +- **Risk: context loss for dormant pre-#832 workspaces.** A legacy `main` row without a stored id now spawns fresh instead of resuming via discovery — a one-time loss of at most one conversation, after which the workspace self-heals onto the stored-UUID path. Accepted: fresh-not-hijacked is the right default, and even a row-gated fallback would still hijack when a personal session in the same cwd is newer than the architect's last one (mtime cannot distinguish them, and cwd verification passes trivially in the same directory). +- **Alternative: keep the discovery fallback but gate it on a legacy row existing.** Rejected per the above — it narrows but does not close the hijack window in single-architect workspaces. - **Risk: over-strict ownership check drops valid resumes.** A jsonl whose first 64KB lacks a `cwd` line would be skipped. Sampled real session files show `cwd` appears on the first user message (line ~5); sessions with no user message carry no resumable value. Accepted. - **Risk: realpath comparison surprises with symlinked workspace paths.** Canonicalize both sides with `fs.realpathSync` (falling back to the raw string if realpath throws) so symlink vs. resolved-path launches compare equal. - **Alternative: only fix the gate (change `<= 1` to row-exists), skip ownership verification.** Rejected: leaves the encoding-collision cross-project vector and the issue explicitly asks for the ownership check. From 9b44a963a89268c4349e47ffe8f47bc4d5509a3e Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:26:17 +0700 Subject: [PATCH 05/23] chore(porch): 1145 plan-approval gate-approved --- .../projects/1145-codev-adopt-launchinstance-mai/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index ce83d74e2..7690b3987 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -6,8 +6,9 @@ plan_phases: [] current_plan_phase: null gates: plan-approval: - status: pending + status: approved requested_at: '2026-07-08T06:36:03.156Z' + approved_at: '2026-07-09T04:26:17.754Z' dev-approval: status: pending pr: @@ -16,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-08T06:36:03.157Z' +updated_at: '2026-07-09T04:26:17.755Z' From 83aec9469794004994cb32758de86152286db314 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:26:31 +0700 Subject: [PATCH 06/23] chore(porch): 1145 implement phase-transition --- .../projects/1145-codev-adopt-launchinstance-mai/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 7690b3987..7af48f7a2 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -1,7 +1,7 @@ id: '1145' title: codev-adopt-launchinstance-mai 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-07-08T06:30:47.411Z' -updated_at: '2026-07-09T04:26:17.755Z' +updated_at: '2026-07-09T04:26:31.899Z' From f5910e677cefd2f3c0423716759fed71fb851b92 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:40:28 +0700 Subject: [PATCH 07/23] [PIR #1145] Verify Claude session ownership before any resume --- .../src/agent-farm/servers/tower-utils.ts | 49 ++++++- .../utils/claude-session-discovery.ts | 136 +++++++++++++++--- .../codev/src/agent-farm/utils/harness.ts | 22 ++- 3 files changed, 179 insertions(+), 28 deletions(-) diff --git a/packages/codev/src/agent-farm/servers/tower-utils.ts b/packages/codev/src/agent-farm/servers/tower-utils.ts index b5b73e64c..8d254b7e4 100644 --- a/packages/codev/src/agent-farm/servers/tower-utils.ts +++ b/packages/codev/src/agent-farm/servers/tower-utils.ts @@ -14,6 +14,7 @@ import type { RateLimitEntry } from './tower-types.js'; import crypto from 'node:crypto'; import { loadRolePrompt, type RoleConfig } from '../utils/roles.js'; import { getArchitectHarness } from '../utils/config.js'; +import type { HarnessProvider } from '../utils/harness.js'; import { getArchitectByName } from '../state.js'; // ============================================================================ @@ -188,6 +189,27 @@ export function buildArchitectArgs(baseArgs: string[], workspacePath: string): { }; } +/** + * Issue #1145: true when the stored session may be resumed. Harnesses that + * expose `session.verifyOwnership` get the final say; ones that don't are + * trusted (their stored ids are minted by us and have no on-disk store to + * cross-check). A throwing check counts as "not ours". + */ +function sessionIsOwned( + harness: HarnessProvider, + sessionId: string, + workspacePath: string, + homeDir?: string, +): boolean { + const verify = harness.session?.verifyOwnership; + if (!verify) return true; + try { + return verify(sessionId, workspacePath, { homeDir }); + } catch { + return false; + } +} + /** * Issue #832: resolve the args/env to launch (or revive) an architect, choosing * between resuming its persisted conversation and starting a fresh one. The @@ -197,10 +219,15 @@ export function buildArchitectArgs(baseArgs: string[], workspacePath: string): { * Decision order: * 1. Harness has no `session` capability (e.g. Codex/Gemini today) → plain fresh * spawn, `sessionId: null` (nothing to resume next time). - * 2. `storedSessionId` present → resume it (no role injection — the saved - * conversation already holds the role/system prompt); echo the same id back. + * 2. `storedSessionId` present AND the harness confirms the session on disk + * belongs to this workspace (Issue #1145: harnesses without a + * `verifyOwnership` capability are trusted as-is) → resume it (no role + * injection — the saved conversation already holds the role/system + * prompt); echo the same id back. * 3. Else fresh → generate a new id, pin the session to it (with role injection), - * and return it for the caller to persist. + * and return it for the caller to persist. A stored id that fails + * ownership verification (stale jsonl, foreign project scope) lands here, + * and the fresh id the caller persists replaces it. * * The returned `sessionId` is the value the caller writes onto the architect row, * so the column is populated correctly on every spawn. @@ -210,8 +237,10 @@ export function resolveArchitectLaunch(opts: { name: string; baseArgs: string[]; storedSessionId?: string | null; + /** Test seam: pins the home dir the ownership check resolves the session store under. */ + homeDir?: string; }): { args: string[]; env: Record; sessionId: string | null; resumed: boolean } { - const { workspacePath, baseArgs, storedSessionId } = opts; + const { workspacePath, baseArgs, storedSessionId, homeDir } = opts; const harness = getArchitectHarness(workspacePath); // 1. No resumable-session support → plain fresh, nothing to persist. @@ -219,8 +248,11 @@ export function resolveArchitectLaunch(opts: { return { ...buildArchitectArgs(baseArgs, workspacePath), sessionId: null, resumed: false }; } - // 2. Resume the persisted conversation (role injection skipped). - if (storedSessionId) { + // 2. Resume the persisted conversation (role injection skipped) — but only + // when the session on disk verifiably belongs to this workspace. A failed or + // throwing check falls through to a fresh spawn (Issue #1145: never attach + // to a conversation we cannot prove is ours). + if (storedSessionId && sessionIsOwned(harness, storedSessionId, workspacePath, homeDir)) { return { args: [...baseArgs, ...harness.session.resumeArgs(storedSessionId)], env: {}, @@ -253,9 +285,12 @@ export function resolveArchitectRestart( workspacePath: string, architectName: string, baseArgs: string[], + opts?: { homeDir?: string }, ): { args: string[]; env: Record; sessionId: string | null; resumed: boolean; storedSessionId: string | null } { const storedSessionId = getArchitectByName(workspacePath, architectName)?.sessionId ?? null; - const resolved = resolveArchitectLaunch({ workspacePath, name: architectName, baseArgs, storedSessionId }); + const resolved = resolveArchitectLaunch({ + workspacePath, name: architectName, baseArgs, storedSessionId, homeDir: opts?.homeDir, + }); return { ...resolved, storedSessionId }; } diff --git a/packages/codev/src/agent-farm/utils/claude-session-discovery.ts b/packages/codev/src/agent-farm/utils/claude-session-discovery.ts index b8d505aaf..549fb9606 100644 --- a/packages/codev/src/agent-farm/utils/claude-session-discovery.ts +++ b/packages/codev/src/agent-farm/utils/claude-session-discovery.ts @@ -5,20 +5,33 @@ // ~/.claude/projects//.jsonl // where the encoding replaces both '/' and '.' in the absolute path with '-'. // We use the newest jsonl by mtime as a stand-in for "the last conversation -// that ran in this directory" so reviving a dead builder (or architect) can -// resume via `claude --resume ` without any spawn-time bookkeeping. +// that ran in this directory" so reviving a dead builder can resume via +// `claude --resume ` without any spawn-time bookkeeping. // -// This is intentionally a heuristic — multiple jsonl files in the same -// directory mean multiple past sessions, and we pick the most recent. For -// builder worktrees that almost always means the right one; for shared cwds -// the caller should be aware of the ambiguity. +// Because the encoding is lossy ('/' and '.' collapse to the same character), +// two different paths can collide into one store directory. Every candidate is +// therefore verified against the `cwd` the session itself recorded (Issue +// #1145): a jsonl whose recorded cwd does not match the requested path is +// skipped, as is one that never recorded a cwd at all (a session with no user +// message has nothing worth resuming). +// +// This remains a heuristic for shared cwds — multiple matching jsonls mean +// multiple past sessions in the same directory, and we pick the most recent. +// For builder worktrees that almost always means the right one; architect +// launch no longer uses discovery at all (Issue #1145). -import { existsSync, readdirSync, statSync } from 'node:fs'; +import { closeSync, existsSync, openSync, readdirSync, readSync, statSync } from 'node:fs'; +import { realpathSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; const JSONL_EXT = '.jsonl'; +// How much of a session file we are willing to read while looking for the +// first record that carries a `cwd`. Real sessions record it on the first +// user message, within the first handful of lines. +const CWD_SCAN_BYTES = 64 * 1024; + /** * Encode an absolute path to the directory name Claude uses under * ~/.claude/projects/. The scheme is: replace every '/' and '.' with '-'. @@ -33,9 +46,68 @@ export function getClaudeProjectDir(absolutePath: string): string { return join(homedir(), '.claude', 'projects', encodeClaudeProjectDir(absolutePath)); } +/** Canonicalize a path for comparison; fall back to the input when realpath fails. */ +function realpathOrSelf(p: string): string { + try { + return realpathSync(p); + } catch { + return p; + } +} + +/** + * Read the `cwd` a session jsonl recorded, or null if none is found within the + * first CWD_SCAN_BYTES. Session files interleave metadata records (mode, + * file-history-snapshot, ...) with message records; the first user/assistant + * record carries the launch cwd. + */ +export function readSessionCwd(filePath: string): string | null { + let fd: number; + try { + fd = openSync(filePath, 'r'); + } catch { + return null; + } + + let text: string; + try { + const buf = Buffer.alloc(CWD_SCAN_BYTES); + const bytesRead = readSync(fd, buf, 0, CWD_SCAN_BYTES, 0); + text = buf.toString('utf8', 0, bytesRead); + } catch { + return null; + } finally { + closeSync(fd); + } + + for (const line of text.split('\n')) { + if (!line.includes('"cwd"')) continue; + try { + const record = JSON.parse(line) as { cwd?: unknown }; + if (typeof record.cwd === 'string' && record.cwd.length > 0) { + return record.cwd; + } + } catch { + // Partial or malformed line (e.g. truncated by the scan cap) — skip. + } + } + return null; +} + +/** + * True when the session jsonl's recorded cwd matches `absolutePath` (both + * sides realpath-canonicalized, so symlinked launch paths compare equal). + */ +function sessionFileOwnedBy(filePath: string, absolutePath: string): boolean { + const recordedCwd = readSessionCwd(filePath); + if (!recordedCwd) return false; + return realpathOrSelf(recordedCwd) === realpathOrSelf(absolutePath); +} + /** * Return the session UUID of the most-recently-modified jsonl in the Claude - * project dir for the given cwd, or null if none exists. + * project dir for the given cwd whose recorded cwd actually matches that path + * (Issue #1145 ownership check), or null if none qualifies. * * `opts.homeDir` lets tests pin the home directory; otherwise resolves via * `os.homedir()`. @@ -48,23 +120,53 @@ export function findLatestSessionId( const dir = join(home, '.claude', 'projects', encodeClaudeProjectDir(absolutePath)); if (!existsSync(dir)) return null; - let bestName: string | null = null; - let bestMtime = -Infinity; + const candidates: Array<{ name: string; mtime: number }> = []; for (const entry of readdirSync(dir, { withFileTypes: true })) { if (!entry.isFile() || !entry.name.endsWith(JSONL_EXT)) continue; const fullPath = join(dir, entry.name); try { - const mtime = statSync(fullPath).mtimeMs; - if (mtime > bestMtime) { - bestMtime = mtime; - bestName = entry.name; - } + candidates.push({ name: entry.name, mtime: statSync(fullPath).mtimeMs }); } catch { // stat failed (race with deletion, permissions) — skip } } - if (!bestName) return null; - return bestName.slice(0, -JSONL_EXT.length); + candidates.sort((a, b) => b.mtime - a.mtime); + + for (const candidate of candidates) { + if (sessionFileOwnedBy(join(dir, candidate.name), absolutePath)) { + return candidate.name.slice(0, -JSONL_EXT.length); + } + } + return null; +} + +/** + * Verify that a specific session id belongs to `absolutePath` (Issue #1145): + * its jsonl must exist in the cwd-encoded project dir AND its recorded cwd + * must match. Used before resuming a *stored* session id, so a stale or + * foreign id degrades to a fresh spawn instead of attaching to someone + * else's conversation. + */ +export function verifySessionOwnership( + absolutePath: string, + sessionId: string, + opts?: { homeDir?: string }, +): boolean { + const home = opts?.homeDir ?? homedir(); + // Claude keys the store dir by its process cwd, which the OS may report in + // physical (symlink-resolved) form — e.g. /tmp/ws vs /private/tmp/ws on + // macOS. Accept the session under either encoding of the same directory. + const candidateDirs = new Set([ + encodeClaudeProjectDir(absolutePath), + encodeClaudeProjectDir(realpathOrSelf(absolutePath)), + ]); + for (const dir of candidateDirs) { + const filePath = join(home, '.claude', 'projects', dir, `${sessionId}${JSONL_EXT}`); + if (existsSync(filePath) && sessionFileOwnedBy(filePath, absolutePath)) { + return true; + } + } + return false; } diff --git a/packages/codev/src/agent-farm/utils/harness.ts b/packages/codev/src/agent-farm/utils/harness.ts index 7e379a4b1..f5a4211ce 100644 --- a/packages/codev/src/agent-farm/utils/harness.ts +++ b/packages/codev/src/agent-farm/utils/harness.ts @@ -12,7 +12,7 @@ * @see codev/specs/591-af-workspace-failure-with-code.md */ -import { findLatestSessionId } from './claude-session-discovery.js'; +import { findLatestSessionId, verifySessionOwnership } from './claude-session-discovery.js'; import { buildWorktreeGuardFiles } from './worktree-write-guard.js'; // ============================================================================= @@ -70,6 +70,14 @@ export interface HarnessProvider { newSessionArgs(sessionId: string): string[]; /** Args to RESUME an existing session by id (caller skips role injection). */ resumeArgs(sessionId: string): string[]; + /** + * Optional: verify that `sessionId`'s on-disk session actually belongs to + * `cwd` before the caller resumes it (Issue #1145). Returns false for a + * missing session file or one whose recorded project scope points at a + * different directory; callers then spawn fresh instead of attaching to a + * foreign conversation. Harnesses that omit this are trusted as-is. + */ + verifyOwnership?(sessionId: string, cwd: string, opts?: { homeDir?: string }): boolean; }; /** @@ -82,9 +90,12 @@ export interface HarnessProvider { * cwd-keyed session store → callers fall back to a fresh launch. Only Claude * implements it (store: ~/.claude/projects//.jsonl). * - * Discovery-based (newest jsonl by mtime): used for builder resume (#831/#929) and, - * for architects (#832), only as the harness-gated legacy fallback when `main` has - * no stored session id yet (codex/gemini omit it → no stale-jsonl crash-loop). + * Discovery-based (newest owned jsonl by mtime): used for builder resume + * (#831/#929) ONLY. Architect launch never discovers — it resumes solely from + * the stored session id on the workspace-scoped architect row, else spawns + * fresh (Issue #1145: discovery on a fresh workspace hijacked whatever Claude + * conversation the user last held in that directory). Candidates are verified + * against the cwd recorded inside the session file before being offered. */ buildResume?(absolutePath: string, opts?: { homeDir?: string }): { sessionId: string; @@ -132,6 +143,9 @@ export const CLAUDE_HARNESS: HarnessProvider = { session: { newSessionArgs: (sessionId) => ['--session-id', sessionId], resumeArgs: (sessionId) => ['--resume', sessionId], + // Issue #1145: a stored id is only resumed when its jsonl exists under this + // cwd's project dir and records this cwd as its project scope. + verifyOwnership: (sessionId, cwd, opts) => verifySessionOwnership(cwd, sessionId, opts), }, }; From 0ee53387e805cff56c3dbaec2957b37348842067 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:40:39 +0700 Subject: [PATCH 08/23] [PIR #1145] Remove architect jsonl-discovery fallback in launchInstance --- .../src/agent-farm/servers/tower-instances.ts | 46 ++++++++----------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/packages/codev/src/agent-farm/servers/tower-instances.ts b/packages/codev/src/agent-farm/servers/tower-instances.ts index d834a5619..308cffebb 100644 --- a/packages/codev/src/agent-farm/servers/tower-instances.ts +++ b/packages/codev/src/agent-farm/servers/tower-instances.ts @@ -13,7 +13,6 @@ import { exec } from 'node:child_process'; import { promisify } from 'node:util'; import { homedir } from 'node:os'; import { encodeWorkspacePath } from '../lib/tower-client.js'; -import { getArchitectHarness } from '../utils/config.js'; import { loadConfig } from '../../lib/config.js'; const execAsync = promisify(exec); @@ -466,37 +465,28 @@ export async function launchInstance(workspacePath: string): Promise<{ success: const cmdParts = architectCmd.split(/\s+/); const cmd = cmdParts[0]; - // Issue #832: resume main's persisted conversation when its row carries a - // session id, else spawn fresh and mint one. The returned `sessionId` is - // stored on the architect row below so the next restart resumes it. A - // state.db read failure degrades to a fresh spawn rather than aborting. + // Issue #832 / #1145: resume main's persisted conversation ONLY when its + // workspace-scoped row carries a session id (which resolveArchitectLaunch + // additionally ownership-verifies against the on-disk session store); + // anything else spawns fresh with a newly minted id, persisted on the + // architect row below so the next restart resumes it. // - // Legacy bridge (#830/#929): a row from before #832 has no stored id. For it, - // fall back to harness-gated jsonl-discovery — but ONLY when main is the sole - // architect, since a cwd shared with siblings makes newest-by-mtime ambiguous - // (the old `safeToResume` guard, now scoped to just this fallback rather than - // gating resume wholesale). Going through `harness.buildResume` keeps discovery - // harness-gated: only Claude has a jsonl store, so a codex/gemini architect - // returns null → fresh, avoiding the stale-jsonl `--resume` crash-loop (#929). - // Stored-UUID resume applies regardless of architect count, so main resumes in - // multi-architect workspaces once it has an id. resolveArchitectLaunch persists - // whatever it resolves, so a discovered id self-migrates into the stored-UUID - // path on this same revival — no separate backfill step. - // The stored id and the discovery fallback are independent recovery - // sources, so a failed read of one must not disable the other (e.g. a - // state.db hiccup reading the row shouldn't suppress sole-architect - // discovery). Each gets its own try. + // The mtime-based jsonl-discovery fallback that used to run here for + // legacy pre-#832 rows was removed by #1145: on a fresh workspace + // (`codev adopt` / first touch) it resumed whatever Claude conversation + // the user last held in this directory — hijacking personal sessions, + // and roleless too, since the resume path skips role injection. Even + // row-gated, mtime cannot distinguish the architect's last session from + // a newer personal one in the same cwd, so the fallback is gone rather + // than re-gated. A legacy row without an id costs one fresh spawn, then + // self-heals onto the stored-UUID path. Discovery (`buildResume`) + // survives for builder resume only, where the worktree cwd is private. + // A global.db read failure also degrades to a fresh spawn — never + // resume on uncertainty. let storedSessionId: string | null = null; try { storedSessionId = getArchitectByName(resolvedPath, DEFAULT_ARCHITECT_NAME)?.sessionId ?? null; - } catch { /* state.db unreadable — fall through to discovery */ } - if (!storedSessionId) { - try { - if (getArchitects(resolvedPath).length <= 1) { - storedSessionId = getArchitectHarness(workspacePath).buildResume?.(workspacePath)?.sessionId ?? null; - } - } catch { /* discovery unavailable — spawn fresh */ } - } + } catch { /* global.db unreadable — spawn fresh */ } const { args: cmdArgs, env: harnessEnv, sessionId: mainSessionId, resumed } = resolveArchitectLaunch({ workspacePath, name: DEFAULT_ARCHITECT_NAME, From b10e71d6b0286407e57643a459aa0ff8a2f6a026 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:40:39 +0700 Subject: [PATCH 09/23] [PIR #1145] Update tests: ownership verification, fresh-boot regression --- .../claude-session-discovery.test.ts | 168 ++++++++++++++---- .../__tests__/discover-resume-session.test.ts | 8 +- .../__tests__/tower-instances.test.ts | 80 +++++++-- .../agent-farm/__tests__/tower-utils.test.ts | 91 ++++++++-- 4 files changed, 281 insertions(+), 66 deletions(-) diff --git a/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts b/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts index c8f2a8557..693a9b2a8 100644 --- a/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts +++ b/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts @@ -2,6 +2,8 @@ * Tests for Claude session discovery via on-disk jsonl introspection. * * Issue #829 — conversation resume. + * Issue #1145 — ownership verification: a candidate jsonl must record a cwd + * matching the requested path before it may be offered for resume. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; @@ -12,6 +14,8 @@ import { join } from 'node:path'; import { encodeClaudeProjectDir, findLatestSessionId, + readSessionCwd, + verifySessionOwnership, } from '../utils/claude-session-discovery.js'; describe('encodeClaudeProjectDir', () => { @@ -36,7 +40,7 @@ describe('encodeClaudeProjectDir', () => { }); }); -describe('findLatestSessionId', () => { +describe('claude session discovery', () => { let fakeHome: string; let projectsRoot: string; @@ -50,53 +54,143 @@ describe('findLatestSessionId', () => { rmSync(fakeHome, { recursive: true, force: true }); }); - function writeSession(absPath: string, uuid: string, mtime: number): void { + /** + * Write a session jsonl shaped like the real store: leading metadata records + * without a cwd, then a user record carrying the launch cwd. `recordedCwd` + * defaults to the path the store dir is keyed by; pass a different value to + * simulate an encoding-collision foreign session, or null to simulate a + * session that never got a user message. + */ + function writeSession( + absPath: string, + uuid: string, + mtime: number, + recordedCwd: string | null = absPath, + ): void { const dir = join(projectsRoot, encodeClaudeProjectDir(absPath)); mkdirSync(dir, { recursive: true }); const file = join(dir, `${uuid}.jsonl`); - writeFileSync(file, `{"sessionId":"${uuid}"}\n`, 'utf-8'); + const lines = [ + `{"type":"mode","mode":"default","sessionId":"${uuid}"}`, + '{"type":"file-history-snapshot"}', + ]; + if (recordedCwd !== null) { + lines.push(`{"type":"user","cwd":"${recordedCwd}","sessionId":"${uuid}"}`); + } + writeFileSync(file, lines.join('\n') + '\n', 'utf-8'); const t = mtime / 1000; utimesSync(file, t, t); } - it('returns the newest session UUID by mtime', () => { - const worktree = '/Users/x/repo/.builders/pir-1'; - writeSession(worktree, 'old-uuid', 1_000_000_000_000); - writeSession(worktree, 'newest-uuid', 1_700_000_000_000); - writeSession(worktree, 'middle-uuid', 1_400_000_000_000); - expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('newest-uuid'); + describe('findLatestSessionId', () => { + it('returns the newest session UUID by mtime', () => { + const worktree = '/Users/x/repo/.builders/pir-1'; + writeSession(worktree, 'old-uuid', 1_000_000_000_000); + writeSession(worktree, 'newest-uuid', 1_700_000_000_000); + writeSession(worktree, 'middle-uuid', 1_400_000_000_000); + expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('newest-uuid'); + }); + + it('returns null when the project dir does not exist', () => { + expect(findLatestSessionId('/nonexistent/path', { homeDir: fakeHome })).toBeNull(); + }); + + it('returns null when the project dir exists but contains no jsonl files', () => { + const worktree = '/Users/x/repo/.builders/pir-2'; + const dir = join(projectsRoot, encodeClaudeProjectDir(worktree)); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'memory'), 'not a jsonl', 'utf-8'); + expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBeNull(); + }); + + it('ignores non-jsonl files and subdirectories', () => { + const worktree = '/Users/x/repo/.builders/pir-3'; + const dir = join(projectsRoot, encodeClaudeProjectDir(worktree)); + mkdirSync(dir, { recursive: true }); + mkdirSync(join(dir, 'some-uuid'), { recursive: true }); + writeFileSync(join(dir, 'history.txt'), 'text', 'utf-8'); + writeSession(worktree, 'the-uuid', 1_500_000_000_000); + expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('the-uuid'); + }); + + it('returns the single jsonl when only one exists', () => { + const worktree = '/Users/x/repo/.builders/pir-4'; + writeSession(worktree, 'only-uuid', 1_500_000_000_000); + expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('only-uuid'); + }); + + // Issue #1145: ownership verification. + + it('skips a newer jsonl whose recorded cwd is a different path (encoding collision)', () => { + // '/x/foo.bar' and '/x/foo/bar' both encode to '-x-foo-bar'. + const requested = '/x/foo/bar'; + const collider = '/x/foo.bar'; + writeSession(requested, 'ours-uuid', 1_400_000_000_000); + writeSession(requested, 'foreign-uuid', 1_700_000_000_000, collider); + expect(findLatestSessionId(requested, { homeDir: fakeHome })).toBe('ours-uuid'); + }); + + it('returns null when the only candidates belong to a different path', () => { + const requested = '/x/foo/bar'; + writeSession(requested, 'foreign-uuid', 1_700_000_000_000, '/x/foo.bar'); + expect(findLatestSessionId(requested, { homeDir: fakeHome })).toBeNull(); + }); + + it('skips a jsonl that never recorded a cwd (no user message)', () => { + const worktree = '/Users/x/repo/.builders/pir-5'; + writeSession(worktree, 'empty-uuid', 1_700_000_000_000, null); + writeSession(worktree, 'real-uuid', 1_400_000_000_000); + expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('real-uuid'); + }); }); - it('returns null when the project dir does not exist', () => { - expect(findLatestSessionId('/nonexistent/path', { homeDir: fakeHome })).toBeNull(); + describe('readSessionCwd', () => { + it('returns the cwd from the first record that carries one', () => { + const worktree = '/Users/x/repo/.builders/pir-6'; + writeSession(worktree, 'uuid-6', 1_500_000_000_000); + const file = join(projectsRoot, encodeClaudeProjectDir(worktree), 'uuid-6.jsonl'); + expect(readSessionCwd(file)).toBe(worktree); + }); + + it('returns null for a cwd-less session', () => { + const worktree = '/Users/x/repo/.builders/pir-7'; + writeSession(worktree, 'uuid-7', 1_500_000_000_000, null); + const file = join(projectsRoot, encodeClaudeProjectDir(worktree), 'uuid-7.jsonl'); + expect(readSessionCwd(file)).toBeNull(); + }); + + it('returns null for a missing file', () => { + expect(readSessionCwd(join(fakeHome, 'nope.jsonl'))).toBeNull(); + }); }); - it('returns null when the project dir exists but contains no jsonl files', () => { - const worktree = '/Users/x/repo/.builders/pir-2'; - const dir = join(projectsRoot, encodeClaudeProjectDir(worktree)); - mkdirSync(dir, { recursive: true }); - writeFileSync(join(dir, 'memory'), 'not a jsonl', 'utf-8'); - expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBeNull(); - }); - - it('ignores non-jsonl files and subdirectories', () => { - const worktree = '/Users/x/repo/.builders/pir-3'; - const dir = join(projectsRoot, encodeClaudeProjectDir(worktree)); - mkdirSync(dir, { recursive: true }); - mkdirSync(join(dir, 'some-uuid'), { recursive: true }); - writeFileSync(join(dir, 'history.txt'), 'text', 'utf-8'); - writeSession(worktree, 'the-uuid', 1_500_000_000_000); - expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('the-uuid'); - }); - - it('returns the single jsonl when only one exists', () => { - const worktree = '/Users/x/repo/.builders/pir-4'; - writeSession(worktree, 'only-uuid', 1_500_000_000_000); - expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('only-uuid'); + describe('verifySessionOwnership', () => { + it('accepts a session whose jsonl exists and records the same cwd', () => { + const worktree = '/Users/x/repo/ws-1'; + writeSession(worktree, 'owned-uuid', 1_500_000_000_000); + expect(verifySessionOwnership(worktree, 'owned-uuid', { homeDir: fakeHome })).toBe(true); + }); + + it('rejects a session id with no jsonl on disk (stale stored id)', () => { + const worktree = '/Users/x/repo/ws-2'; + mkdirSync(join(projectsRoot, encodeClaudeProjectDir(worktree)), { recursive: true }); + expect(verifySessionOwnership(worktree, 'gone-uuid', { homeDir: fakeHome })).toBe(false); + }); + + it('rejects a session whose recorded cwd is a different path', () => { + const requested = '/x/foo/bar'; + writeSession(requested, 'foreign-uuid', 1_500_000_000_000, '/x/foo.bar'); + expect(verifySessionOwnership(requested, 'foreign-uuid', { homeDir: fakeHome })).toBe(false); + }); + + it('rejects a session that never recorded a cwd', () => { + const worktree = '/Users/x/repo/ws-3'; + writeSession(worktree, 'empty-uuid', 1_500_000_000_000, null); + expect(verifySessionOwnership(worktree, 'empty-uuid', { homeDir: fakeHome })).toBe(false); + }); }); }); -// Issue #832: the live-process session-id capture (cmdline-reading backfill) was -// dropped in favour of the sole-architect jsonl-discovery fallback in -// launchInstance + the stored-UUID spawn/revive path. findLatestSessionId (above) -// is the shared discovery helper that remains. +// Issue #832 introduced the stored-UUID spawn/revive path for architects; +// Issue #1145 removed the architect-side jsonl-discovery fallback entirely. +// findLatestSessionId (above) now serves builder resume only. diff --git a/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts b/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts index e44c47e72..628c996f5 100644 --- a/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts +++ b/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts @@ -36,7 +36,13 @@ function writeSession(projectsRoot: string, absPath: string, uuid: string, mtime const dir = join(projectsRoot, encodeClaudeProjectDir(absPath)); mkdirSync(dir, { recursive: true }); const file = join(dir, `${uuid}.jsonl`); - writeFileSync(file, `{"sessionId":"${uuid}"}\n`, 'utf-8'); + // Shaped like the real store: a user record carrying the launch cwd, which + // discovery verifies ownership against (Issue #1145). + writeFileSync( + file, + `{"type":"mode","sessionId":"${uuid}"}\n{"type":"user","cwd":"${absPath}","sessionId":"${uuid}"}\n`, + 'utf-8', + ); const t = mtimeMs / 1000; utimesSync(file, t, t); } diff --git a/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts b/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts index 597a23a13..745ee5b27 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts @@ -49,9 +49,10 @@ const { vi.mock('../db/index.js', () => ({ getGlobalDb: () => ({ prepare: mockDbPrepare }), - // state.getArchitects() (used by launchInstance's safeToResume guard) calls - // getDb(), not getGlobalDb — mock it too so the guard doesn't throw and - // default to skip-resume. (Issue #929 resume regression tests depend on it.) + // state.getArchitectByName() (launchInstance's stored-session lookup) calls + // getDb(), not getGlobalDb — mock it too. The default mock has no `.get`, + // so the lookup throws and launchInstance treats the workspace as having no + // stored session (the fresh-boot cases below rely on that). getDb: () => ({ prepare: mockDbPrepare }), closeDb: () => {}, })); @@ -600,19 +601,26 @@ describe('tower-instances', () => { }); // ========================================================================= - // Issue #929 — architect resume is gated on the configured harness + // Issue #929 / #1145 — architect launch never discovery-resumes // - // Regression guard for the crash-loop: a codex/gemini architect with a stale - // Claude jsonl in ~/.claude/projects// must NOT launch - // ` --resume `. Only the Claude harness implements - // buildResume, so codex/gemini relaunch fresh (role-injected) instead. + // #929's crash-loop guard (codex/gemini + stale Claude jsonl must not launch + // ` --resume`) is subsumed by #1145: launchInstance no longer consults + // the jsonl store at all. Its ONLY resume source is the session id stored on + // this workspace's `main` architect row. A fresh workspace with a personal + // Claude conversation in the same cwd (the hijack in #1145) must boot fresh. // ========================================================================= - describe('Issue #929 — architect resume gated on harness', () => { - function writeStaleClaudeSession(fakeHome: string, cwd: string, uuid: string): void { + describe('Issue #929/#1145 — architect resume comes only from the stored row', () => { + // Shaped like the real store: metadata line, then a user line whose cwd + // matches the workspace. Discovery WOULD have accepted this session — the + // fresh boot below proves the fallback is gone, not merely cwd-filtered. + function writePersonalClaudeSession(fakeHome: string, cwd: string, uuid: string): void { const dir = path.join(fakeHome, '.claude', 'projects', encodeClaudeProjectDir(cwd)); fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, `${uuid}.jsonl`), `{"sessionId":"${uuid}"}\n`); + fs.writeFileSync( + path.join(dir, `${uuid}.jsonl`), + `{"type":"mode","sessionId":"${uuid}"}\n{"type":"user","cwd":"${cwd}","sessionId":"${uuid}"}\n`, + ); } function makeCapturingDeps(): { deps: InstanceDeps; createSession: ReturnType } { @@ -649,7 +657,7 @@ describe('tower-instances', () => { JSON.stringify(configJson), ); } - writeStaleClaudeSession(fakeHome, tmpDir, uuid); + writePersonalClaudeSession(fakeHome, tmpDir, uuid); process.env.HOME = fakeHome; await fn(tmpDir, fakeHome, uuid); } finally { @@ -661,7 +669,7 @@ describe('tower-instances', () => { }; } - it('codex architect + stale Claude jsonl → launches fresh, no --resume', withSetup( + it('codex architect + stale Claude jsonl → launches fresh, no --resume (#929)', withSetup( { shell: { architect: 'codex' } }, async (tmpDir, _fakeHome, uuid) => { const { deps, createSession } = makeCapturingDeps(); @@ -677,7 +685,7 @@ describe('tower-instances', () => { }, )); - it('claude architect (default) + stale Claude jsonl → resumes with --resume ', withSetup( + it('fresh workspace + personal Claude session in the same cwd → boots fresh, no --resume (#1145 regression)', withSetup( null, async (tmpDir, _fakeHome, uuid) => { const { deps, createSession } = makeCapturingDeps(); @@ -687,9 +695,47 @@ describe('tower-instances', () => { expect(result.success).toBe(true); expect(createSession).toHaveBeenCalled(); - const callStr = JSON.stringify(createSession.mock.calls[0]); - expect(callStr).toContain('--resume'); - expect(callStr).toContain(uuid); + // Assert on the argv tokens, not the stringified call: the injected + // role prompt legitimately mentions "--resume" in its CLI examples. + const callArgs = (createSession.mock.calls[0][0] as { args: string[] }).args; + expect(callArgs).not.toContain('--resume'); + expect(callArgs).not.toContain(uuid); + // Fresh spawn pins a newly minted session id instead. + expect(callArgs).toContain('--session-id'); + }, + )); + + it('main row with a stored session id + owned jsonl → resumes that id', withSetup( + null, + async (tmpDir, _fakeHome, _uuid) => { + const storedId = 'stored-main-uuid-5678'; + writePersonalClaudeSession(_fakeHome, tmpDir, storedId); + // getArchitectByName reads via db.prepare(...).get — return a main row + // carrying the stored conversation id. + const resolvedTmpDir = fs.realpathSync(tmpDir); + const mockGet = vi.fn().mockReturnValue({ + workspace_path: resolvedTmpDir, id: 'main', pid: 0, port: 0, + cmd: 'claude', started_at: 'x', terminal_id: null, session_id: storedId, + }); + mockDbPrepare.mockReturnValue({ run: mockDbRun, all: mockDbAll, get: mockGet }); + try { + // The stored session must exist under BOTH path forms the launch code + // touches (tmpdir is symlinked on macOS: /var → /private/var). + writePersonalClaudeSession(_fakeHome, resolvedTmpDir, storedId); + + const { deps, createSession } = makeCapturingDeps(); + initInstances(deps); + + const result = await launchInstance(tmpDir); + expect(result.success).toBe(true); + expect(createSession).toHaveBeenCalled(); + + const callArgs = (createSession.mock.calls[0][0] as { args: string[] }).args; + expect(callArgs).toContain('--resume'); + expect(callArgs).toContain(storedId); + } finally { + mockDbPrepare.mockReturnValue({ run: mockDbRun, all: mockDbAll }); + } }, )); diff --git a/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts b/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts index 17d178bef..d43976283 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts @@ -29,6 +29,28 @@ vi.mock('../state.js', async (importOriginal) => { return { ...actual, getArchitectByName: vi.fn() }; }); import { getArchitectByName } from '../state.js'; +import { encodeClaudeProjectDir } from '../utils/claude-session-discovery.js'; + +/** + * Issue #1145: stored-id resume now requires the session jsonl to exist under + * the (test-pinned) home dir and to record the launching cwd. This writes a + * minimally-real session file so ownership verification passes — or, with a + * divergent `recordedCwd`, deliberately fails. + */ +function writeSessionFixture( + homeDir: string, + cwdPath: string, + uuid: string, + recordedCwd: string = cwdPath, +): void { + const dir = path.join(homeDir, '.claude', 'projects', encodeClaudeProjectDir(cwdPath)); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, `${uuid}.jsonl`), + `{"type":"mode","sessionId":"${uuid}"}\n{"type":"user","cwd":"${recordedCwd}","sessionId":"${uuid}"}\n`, + 'utf-8', + ); +} describe('tower-utils', () => { describe('isRateLimited', () => { @@ -212,22 +234,27 @@ describe('tower-utils', () => { describe('resolveArchitectLaunch (Issue #832)', () => { let workspace: string; + let fakeHome: string; const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/; beforeEach(() => { // A bare temp dir: no codev/roles, so buildArchitectArgs returns baseArgs // unchanged (loadRolePrompt → null). Default harness resolves to Claude - // (which has the session capability). + // (which has the session capability). fakeHome pins the session store the + // ownership check (Issue #1145) reads. workspace = fs.mkdtempSync(path.join(tmpdir(), 'ral-ws-')); + fakeHome = fs.mkdtempSync(path.join(tmpdir(), 'ral-home-')); }); afterEach(() => { fs.rmSync(workspace, { recursive: true, force: true }); + fs.rmSync(fakeHome, { recursive: true, force: true }); }); it('resumes the stored id (no role injection) and echoes it back', () => { + writeSessionFixture(fakeHome, workspace, 'stored-abc'); const { args, env, sessionId, resumed } = resolveArchitectLaunch({ - workspacePath: workspace, name: 'reviewer', baseArgs: [], storedSessionId: 'stored-abc', + workspacePath: workspace, name: 'reviewer', baseArgs: [], storedSessionId: 'stored-abc', homeDir: fakeHome, }); expect(args).toEqual(['--resume', 'stored-abc']); expect(env).toEqual({}); @@ -237,7 +264,7 @@ describe('resolveArchitectLaunch (Issue #832)', () => { it('mints a fresh --session-id when there is no stored id, and returns it', () => { const { args, sessionId, resumed } = resolveArchitectLaunch({ - workspacePath: workspace, name: 'reviewer', baseArgs: [], storedSessionId: null, + workspacePath: workspace, name: 'reviewer', baseArgs: [], storedSessionId: null, homeDir: fakeHome, }); expect(args).not.toContain('--resume'); expect(args).toContain('--session-id'); @@ -248,25 +275,51 @@ describe('resolveArchitectLaunch (Issue #832)', () => { }); it('mints distinct ids across fresh spawns', () => { - const a = resolveArchitectLaunch({ workspacePath: workspace, name: 'a', baseArgs: [] }).sessionId; - const b = resolveArchitectLaunch({ workspacePath: workspace, name: 'b', baseArgs: [] }).sessionId; + const a = resolveArchitectLaunch({ workspacePath: workspace, name: 'a', baseArgs: [], homeDir: fakeHome }).sessionId; + const b = resolveArchitectLaunch({ workspacePath: workspace, name: 'b', baseArgs: [], homeDir: fakeHome }).sessionId; expect(a).not.toBe(b); }); it('preserves baseArgs ahead of the session flags', () => { + writeSessionFixture(fakeHome, workspace, 'x'); const { args } = resolveArchitectLaunch({ - workspacePath: workspace, name: 'main', baseArgs: ['--foo'], storedSessionId: 'x', + workspacePath: workspace, name: 'main', baseArgs: ['--foo'], storedSessionId: 'x', homeDir: fakeHome, }); expect(args).toEqual(['--foo', '--resume', 'x']); }); it('two siblings with distinct stored ids resume independently (no cross-attachment)', () => { - const reviewer = resolveArchitectLaunch({ workspacePath: workspace, name: 'reviewer', baseArgs: [], storedSessionId: 'rev-1' }); - const casa = resolveArchitectLaunch({ workspacePath: workspace, name: 'casa', baseArgs: [], storedSessionId: 'casa-1' }); + writeSessionFixture(fakeHome, workspace, 'rev-1'); + writeSessionFixture(fakeHome, workspace, 'casa-1'); + const reviewer = resolveArchitectLaunch({ workspacePath: workspace, name: 'reviewer', baseArgs: [], storedSessionId: 'rev-1', homeDir: fakeHome }); + const casa = resolveArchitectLaunch({ workspacePath: workspace, name: 'casa', baseArgs: [], storedSessionId: 'casa-1', homeDir: fakeHome }); expect(reviewer.args).toEqual(['--resume', 'rev-1']); expect(casa.args).toEqual(['--resume', 'casa-1']); }); + // Issue #1145: ownership verification ahead of the resume branch. + + it('spawns fresh when the stored id has no jsonl on disk (stale stored id)', () => { + const { args, sessionId, resumed } = resolveArchitectLaunch({ + workspacePath: workspace, name: 'main', baseArgs: [], storedSessionId: 'ghost-id', homeDir: fakeHome, + }); + expect(args).not.toContain('--resume'); + expect(args).toContain('--session-id'); + expect(resumed).toBe(false); + expect(sessionId).toMatch(UUID_RE); // replacement id for the caller to persist + expect(sessionId).not.toBe('ghost-id'); + }); + + it('spawns fresh when the stored session records a different project cwd', () => { + writeSessionFixture(fakeHome, workspace, 'foreign-id', '/somewhere/else/entirely'); + const { args, resumed, sessionId } = resolveArchitectLaunch({ + workspacePath: workspace, name: 'main', baseArgs: [], storedSessionId: 'foreign-id', homeDir: fakeHome, + }); + expect(args).not.toContain('--resume'); + expect(resumed).toBe(false); + expect(sessionId).not.toBe('foreign-id'); + }); + it('no-session harness → plain fresh, returns null sessionId', () => { // Force a Codex architect harness (no `session` capability) via config. fs.mkdirSync(path.join(workspace, '.codev'), { recursive: true }); @@ -286,21 +339,25 @@ describe('resolveArchitectLaunch (Issue #832)', () => { describe('resolveArchitectRestart (Issue #832 — shellper auto-restart bake)', () => { let workspace: string; + let fakeHome: string; const mockGet = vi.mocked(getArchitectByName); beforeEach(() => { // Bare temp dir → default Claude harness (has the session capability). workspace = fs.mkdtempSync(path.join(tmpdir(), 'rar-ws-')); + fakeHome = fs.mkdtempSync(path.join(tmpdir(), 'rar-home-')); mockGet.mockReset(); }); afterEach(() => { fs.rmSync(workspace, { recursive: true, force: true }); + fs.rmSync(fakeHome, { recursive: true, force: true }); }); it('revives the architect\'s stored session id on restart (--resume, no role injection)', () => { + writeSessionFixture(fakeHome, workspace, 'stored-xyz'); mockGet.mockReturnValue({ name: 'reviewer', cmd: 'claude', startedAt: 'x', sessionId: 'stored-xyz' } as never); - const { args, env, resumed, storedSessionId } = resolveArchitectRestart(workspace, 'reviewer', []); + const { args, env, resumed, storedSessionId } = resolveArchitectRestart(workspace, 'reviewer', [], { homeDir: fakeHome }); expect(mockGet).toHaveBeenCalledWith(workspace, 'reviewer'); expect(args).toEqual(['--resume', 'stored-xyz']); expect(env).toEqual({}); @@ -308,6 +365,16 @@ describe('resolveArchitectRestart (Issue #832 — shellper auto-restart bake)', expect(storedSessionId).toBe('stored-xyz'); }); + it('spawns fresh on restart when the stored session fails ownership verification (#1145)', () => { + // No jsonl fixture written → stale stored id. + mockGet.mockReturnValue({ name: 'reviewer', cmd: 'claude', startedAt: 'x', sessionId: 'stale-xyz' } as never); + const { args, resumed, storedSessionId } = resolveArchitectRestart(workspace, 'reviewer', [], { homeDir: fakeHome }); + expect(args).not.toContain('--resume'); + expect(args).toContain('--session-id'); + expect(resumed).toBe(false); + expect(storedSessionId).toBe('stale-xyz'); // still reported for the caller's log line + }); + it('falls back to a fresh session when the row has no stored id (legacy / self-heal)', () => { mockGet.mockReturnValue({ name: 'reviewer', cmd: 'claude', startedAt: 'x' } as never); // no sessionId const { args, resumed, storedSessionId } = resolveArchitectRestart(workspace, 'reviewer', []); @@ -326,9 +393,11 @@ describe('resolveArchitectRestart (Issue #832 — shellper auto-restart bake)', }); it('looks each architect up by its own name — no cross-attachment between siblings', () => { + writeSessionFixture(fakeHome, workspace, 'rev-1'); + writeSessionFixture(fakeHome, workspace, 'casa-1'); mockGet.mockImplementation((_ws: string, name: string) => (name === 'reviewer' ? { sessionId: 'rev-1' } : { sessionId: 'casa-1' }) as never); - expect(resolveArchitectRestart(workspace, 'reviewer', []).args).toEqual(['--resume', 'rev-1']); - expect(resolveArchitectRestart(workspace, 'casa', []).args).toEqual(['--resume', 'casa-1']); + expect(resolveArchitectRestart(workspace, 'reviewer', [], { homeDir: fakeHome }).args).toEqual(['--resume', 'rev-1']); + expect(resolveArchitectRestart(workspace, 'casa', [], { homeDir: fakeHome }).args).toEqual(['--resume', 'casa-1']); }); }); From f07c045a04d86026984f7b9cdbcd2580069e03d9 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:41:14 +0700 Subject: [PATCH 10/23] [PIR #1145] Thread: implement phase notes --- codev/state/pir-1145_thread.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/codev/state/pir-1145_thread.md b/codev/state/pir-1145_thread.md index 8a5ddf08a..8081c0025 100644 --- a/codev/state/pir-1145_thread.md +++ b/codev/state/pir-1145_thread.md @@ -11,3 +11,17 @@ Investigated issue #1145 (main architect hijacks unrelated Claude session on fre 2. Ownership verification: jsonl candidates must record a `cwd` matching the requested path (discovery side), plus an optional harness-gated `session.verifyOwnership` checked in `resolveArchitectLaunch` before any stored-id resume. Confirmed `afx workspace stop` preserves architect rows, so normal stop/start resume is unaffected. Sitting at plan-approval gate. + +## 2026-07-09 — Plan revised, approved; implement phase + +Reviewer challenged the row-gated fallback: even gated, mtime discovery can't distinguish the architect's last session from a *newer personal session in the same cwd* (and the cwd ownership check passes trivially there). Revised plan to drop the architect discovery fallback entirely; approved. + +Implementation: +- `claude-session-discovery.ts`: candidates now verified against the cwd recorded inside the jsonl (realpath-canonicalized both sides); new `readSessionCwd` + `verifySessionOwnership` (checks both logical and physical path encodings — macOS /tmp symlinks). +- `harness.ts`: optional `session.verifyOwnership` capability; Claude implements it. `buildResume` doc updated: builder resume is its sole consumer now. +- `tower-utils.ts`: `resolveArchitectLaunch` verifies stored-id ownership before the resume branch; failure falls through to fresh (new minted id replaces the stale one on persist). `homeDir` test seam threaded through. +- `tower-instances.ts`: discovery fallback deleted; stored row id is the only architect resume source. + +Test gotcha worth remembering: asserting `JSON.stringify(spawnCall).not.toContain('--resume')` false-fails because the injected architect role prompt *text* contains `--resume` in its CLI examples. Assert on argv tokens instead. + +115 tests green on the four touched files; full suite + push next, then dev-approval gate. From 908c0095db794c8f01cb6f155cae6431ac6aafcb Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Thu, 9 Jul 2026 11:41:45 +0700 Subject: [PATCH 11/23] chore(porch): 1145 dev-approval gate-requested --- codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 7af48f7a2..baff30f72 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -11,10 +11,11 @@ gates: approved_at: '2026-07-09T04:26:17.754Z' dev-approval: status: pending + requested_at: '2026-07-09T04:41:45.899Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-09T04:26:31.899Z' +updated_at: '2026-07-09T04:41:45.899Z' From 6fc3636c3cc3ac4838812589cc51bf2e964f7f06 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Fri, 10 Jul 2026 12:52:16 +0700 Subject: [PATCH 12/23] [PIR #1145] Make the cwd scan streaming: semantic, not byte-offset-bound --- .../claude-session-discovery.test.ts | 19 +++++ .../utils/claude-session-discovery.ts | 84 +++++++++++++------ 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts b/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts index 693a9b2a8..e60fb8c66 100644 --- a/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts +++ b/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts @@ -162,6 +162,25 @@ describe('claude session discovery', () => { it('returns null for a missing file', () => { expect(readSessionCwd(join(fakeHome, 'nope.jsonl'))).toBeNull(); }); + + it('finds a cwd record sitting beyond the first read chunk (large metadata prefix)', () => { + // The scan is semantic, not positional: a session whose first user + // record is pushed past 64KB by e.g. a fat file-history-snapshot must + // still verify. Build the file by hand with a ~200KB cwd-less record + // ahead of the user line. + const worktree = '/Users/x/repo/.builders/pir-8'; + const dir = join(projectsRoot, encodeClaudeProjectDir(worktree)); + mkdirSync(dir, { recursive: true }); + const bigSnapshot = JSON.stringify({ type: 'file-history-snapshot', blob: 'x'.repeat(200 * 1024) }); + const file = join(dir, 'uuid-8.jsonl'); + writeFileSync( + file, + `${bigSnapshot}\n{"type":"user","cwd":"${worktree}","sessionId":"uuid-8"}\n`, + 'utf-8', + ); + expect(readSessionCwd(file)).toBe(worktree); + expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('uuid-8'); + }); }); describe('verifySessionOwnership', () => { diff --git a/packages/codev/src/agent-farm/utils/claude-session-discovery.ts b/packages/codev/src/agent-farm/utils/claude-session-discovery.ts index 549fb9606..4c414100c 100644 --- a/packages/codev/src/agent-farm/utils/claude-session-discovery.ts +++ b/packages/codev/src/agent-farm/utils/claude-session-discovery.ts @@ -24,13 +24,20 @@ import { closeSync, existsSync, openSync, readdirSync, readSync, statSync } from import { realpathSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; +import { StringDecoder } from 'node:string_decoder'; const JSONL_EXT = '.jsonl'; -// How much of a session file we are willing to read while looking for the -// first record that carries a `cwd`. Real sessions record it on the first -// user message, within the first handful of lines. -const CWD_SCAN_BYTES = 64 * 1024; +// Chunk size for the streaming cwd scan. The scan is semantic, not positional: +// it reads chunk by chunk until the first record carrying a `cwd` (or EOF), so +// ownership never depends on the byte offset the record happens to sit at. +// Real sessions record cwd on the first user message, so one chunk suffices. +const CWD_SCAN_CHUNK_BYTES = 64 * 1024; + +// A single buffered line larger than this is dropped un-parsed (scanning +// continues on later records). Only guards runaway memory on pathological +// files; every user record carries a cwd, so a later one still qualifies. +const CWD_SCAN_MAX_LINE_BYTES = 8 * 1024 * 1024; /** * Encode an absolute path to the directory name Claude uses under @@ -55,11 +62,26 @@ function realpathOrSelf(p: string): string { } } +/** Parse one jsonl line and return its top-level `cwd`, or null. */ +function parseCwdLine(line: string): string | null { + if (!line.includes('"cwd"')) return null; + try { + const record = JSON.parse(line) as { cwd?: unknown }; + if (typeof record.cwd === 'string' && record.cwd.length > 0) { + return record.cwd; + } + } catch { + // Malformed or fragmentary line — skip. + } + return null; +} + /** - * Read the `cwd` a session jsonl recorded, or null if none is found within the - * first CWD_SCAN_BYTES. Session files interleave metadata records (mode, - * file-history-snapshot, ...) with message records; the first user/assistant - * record carries the launch cwd. + * Read the `cwd` a session jsonl recorded, or null if the session never + * recorded one. Session files interleave metadata records (mode, + * file-history-snapshot, ...) with message records; the first user record + * carries the launch cwd. Streams the file chunk by chunk with early exit, + * so the result depends only on the file's records, never on their offsets. */ export function readSessionCwd(filePath: string): string | null { let fd: number; @@ -69,29 +91,41 @@ export function readSessionCwd(filePath: string): string | null { return null; } - let text: string; try { - const buf = Buffer.alloc(CWD_SCAN_BYTES); - const bytesRead = readSync(fd, buf, 0, CWD_SCAN_BYTES, 0); - text = buf.toString('utf8', 0, bytesRead); + const chunk = Buffer.alloc(CWD_SCAN_CHUNK_BYTES); + const decoder = new StringDecoder('utf8'); + let carry = ''; + let position = 0; + + for (;;) { + const bytesRead = readSync(fd, chunk, 0, CWD_SCAN_CHUNK_BYTES, position); + if (bytesRead <= 0) break; + position += bytesRead; + carry += decoder.write(chunk.subarray(0, bytesRead)); + + let newlineIdx = carry.indexOf('\n'); + while (newlineIdx !== -1) { + const cwd = parseCwdLine(carry.slice(0, newlineIdx)); + if (cwd) return cwd; + carry = carry.slice(newlineIdx + 1); + newlineIdx = carry.indexOf('\n'); + } + + if (carry.length > CWD_SCAN_MAX_LINE_BYTES) { + // Oversized record: drop the buffered prefix and keep scanning. The + // remainder of this line arrives in later chunks and fails JSON.parse + // as a fragment, which parseCwdLine skips harmlessly. + carry = ''; + } + } + + // Final line without a trailing newline. + return parseCwdLine(carry + decoder.end()); } catch { return null; } finally { closeSync(fd); } - - for (const line of text.split('\n')) { - if (!line.includes('"cwd"')) continue; - try { - const record = JSON.parse(line) as { cwd?: unknown }; - if (typeof record.cwd === 'string' && record.cwd.length > 0) { - return record.cwd; - } - } catch { - // Partial or malformed line (e.g. truncated by the scan cap) — skip. - } - } - return null; } /** From 00284f3f6a3f2055af7eb0fa6a83b1c56512841c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Fri, 10 Jul 2026 12:58:13 +0700 Subject: [PATCH 13/23] [PIR #1145] Narrow ownership verification to file existence; drop content scanning --- codev/state/pir-1145_thread.md | 8 + .../claude-session-discovery.test.ts | 123 +++------------ .../__tests__/discover-resume-session.test.ts | 8 +- .../__tests__/tower-instances.test.ts | 11 +- .../agent-farm/__tests__/tower-utils.test.ts | 22 +-- .../src/agent-farm/servers/tower-utils.ts | 10 +- .../utils/claude-session-discovery.ts | 147 ++++-------------- .../codev/src/agent-farm/utils/harness.ts | 19 ++- 8 files changed, 84 insertions(+), 264 deletions(-) diff --git a/codev/state/pir-1145_thread.md b/codev/state/pir-1145_thread.md index 8081c0025..026bdd926 100644 --- a/codev/state/pir-1145_thread.md +++ b/codev/state/pir-1145_thread.md @@ -25,3 +25,11 @@ Implementation: Test gotcha worth remembering: asserting `JSON.stringify(spawnCall).not.toContain('--resume')` false-fails because the injected architect role prompt *text* contains `--resume` in its CLI examples. Assert on argv tokens instead. 115 tests green on the four touched files; full suite + push next, then dev-approval gate. + +## 2026-07-10 — dev-approval feedback: content scanning removed + +Reviewer walked the design back in three steps at the gate: (1) why scan file contents at all → only real justification was lossy-encoding collisions; (2) the fixed 64KB scan window made resume content-dependent → made it streaming/semantic; (3) collisions are contrived for both architects (row-gated) and builders (Agent-Farm-managed worktree paths) → dropped content scanning entirely. + +Final shape: architect resume = stored row id + file-*existence* check (both logical/physical path encodings, macOS symlink case); builder discovery = plain newest-by-mtime (pre-#1145 behavior). The #1145 fix proper (no discovery fallback in launchInstance) is untouched. Deviation from the approved plan (which specified cwd-content verification) to be recorded in the review file. + +Lesson: when a reviewer asks "why is this here at all", enumerate what each layer actually defends against before defending the layer — two of three protections were against a threat the system's own path conventions already preclude. diff --git a/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts b/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts index e60fb8c66..89c48176a 100644 --- a/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts +++ b/packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts @@ -2,19 +2,18 @@ * Tests for Claude session discovery via on-disk jsonl introspection. * * Issue #829 — conversation resume. - * Issue #1145 — ownership verification: a candidate jsonl must record a cwd - * matching the requested path before it may be offered for resume. + * Issue #1145 — verifySessionOwnership: a stored session id is only resumable + * while its jsonl still exists for the workspace's cwd. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { mkdtempSync, rmSync, mkdirSync, writeFileSync, utimesSync } from 'node:fs'; +import { mkdtempSync, rmSync, mkdirSync, writeFileSync, utimesSync, realpathSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { encodeClaudeProjectDir, findLatestSessionId, - readSessionCwd, verifySessionOwnership, } from '../utils/claude-session-discovery.js'; @@ -54,30 +53,11 @@ describe('claude session discovery', () => { rmSync(fakeHome, { recursive: true, force: true }); }); - /** - * Write a session jsonl shaped like the real store: leading metadata records - * without a cwd, then a user record carrying the launch cwd. `recordedCwd` - * defaults to the path the store dir is keyed by; pass a different value to - * simulate an encoding-collision foreign session, or null to simulate a - * session that never got a user message. - */ - function writeSession( - absPath: string, - uuid: string, - mtime: number, - recordedCwd: string | null = absPath, - ): void { + function writeSession(absPath: string, uuid: string, mtime: number): void { const dir = join(projectsRoot, encodeClaudeProjectDir(absPath)); mkdirSync(dir, { recursive: true }); const file = join(dir, `${uuid}.jsonl`); - const lines = [ - `{"type":"mode","mode":"default","sessionId":"${uuid}"}`, - '{"type":"file-history-snapshot"}', - ]; - if (recordedCwd !== null) { - lines.push(`{"type":"user","cwd":"${recordedCwd}","sessionId":"${uuid}"}`); - } - writeFileSync(file, lines.join('\n') + '\n', 'utf-8'); + writeFileSync(file, `{"sessionId":"${uuid}"}\n`, 'utf-8'); const t = mtime / 1000; utimesSync(file, t, t); } @@ -118,73 +98,10 @@ describe('claude session discovery', () => { writeSession(worktree, 'only-uuid', 1_500_000_000_000); expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('only-uuid'); }); - - // Issue #1145: ownership verification. - - it('skips a newer jsonl whose recorded cwd is a different path (encoding collision)', () => { - // '/x/foo.bar' and '/x/foo/bar' both encode to '-x-foo-bar'. - const requested = '/x/foo/bar'; - const collider = '/x/foo.bar'; - writeSession(requested, 'ours-uuid', 1_400_000_000_000); - writeSession(requested, 'foreign-uuid', 1_700_000_000_000, collider); - expect(findLatestSessionId(requested, { homeDir: fakeHome })).toBe('ours-uuid'); - }); - - it('returns null when the only candidates belong to a different path', () => { - const requested = '/x/foo/bar'; - writeSession(requested, 'foreign-uuid', 1_700_000_000_000, '/x/foo.bar'); - expect(findLatestSessionId(requested, { homeDir: fakeHome })).toBeNull(); - }); - - it('skips a jsonl that never recorded a cwd (no user message)', () => { - const worktree = '/Users/x/repo/.builders/pir-5'; - writeSession(worktree, 'empty-uuid', 1_700_000_000_000, null); - writeSession(worktree, 'real-uuid', 1_400_000_000_000); - expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('real-uuid'); - }); - }); - - describe('readSessionCwd', () => { - it('returns the cwd from the first record that carries one', () => { - const worktree = '/Users/x/repo/.builders/pir-6'; - writeSession(worktree, 'uuid-6', 1_500_000_000_000); - const file = join(projectsRoot, encodeClaudeProjectDir(worktree), 'uuid-6.jsonl'); - expect(readSessionCwd(file)).toBe(worktree); - }); - - it('returns null for a cwd-less session', () => { - const worktree = '/Users/x/repo/.builders/pir-7'; - writeSession(worktree, 'uuid-7', 1_500_000_000_000, null); - const file = join(projectsRoot, encodeClaudeProjectDir(worktree), 'uuid-7.jsonl'); - expect(readSessionCwd(file)).toBeNull(); - }); - - it('returns null for a missing file', () => { - expect(readSessionCwd(join(fakeHome, 'nope.jsonl'))).toBeNull(); - }); - - it('finds a cwd record sitting beyond the first read chunk (large metadata prefix)', () => { - // The scan is semantic, not positional: a session whose first user - // record is pushed past 64KB by e.g. a fat file-history-snapshot must - // still verify. Build the file by hand with a ~200KB cwd-less record - // ahead of the user line. - const worktree = '/Users/x/repo/.builders/pir-8'; - const dir = join(projectsRoot, encodeClaudeProjectDir(worktree)); - mkdirSync(dir, { recursive: true }); - const bigSnapshot = JSON.stringify({ type: 'file-history-snapshot', blob: 'x'.repeat(200 * 1024) }); - const file = join(dir, 'uuid-8.jsonl'); - writeFileSync( - file, - `${bigSnapshot}\n{"type":"user","cwd":"${worktree}","sessionId":"uuid-8"}\n`, - 'utf-8', - ); - expect(readSessionCwd(file)).toBe(worktree); - expect(findLatestSessionId(worktree, { homeDir: fakeHome })).toBe('uuid-8'); - }); }); - describe('verifySessionOwnership', () => { - it('accepts a session whose jsonl exists and records the same cwd', () => { + describe('verifySessionOwnership (Issue #1145)', () => { + it('accepts a session whose jsonl exists for the workspace cwd', () => { const worktree = '/Users/x/repo/ws-1'; writeSession(worktree, 'owned-uuid', 1_500_000_000_000); expect(verifySessionOwnership(worktree, 'owned-uuid', { homeDir: fakeHome })).toBe(true); @@ -196,16 +113,24 @@ describe('claude session discovery', () => { expect(verifySessionOwnership(worktree, 'gone-uuid', { homeDir: fakeHome })).toBe(false); }); - it('rejects a session whose recorded cwd is a different path', () => { - const requested = '/x/foo/bar'; - writeSession(requested, 'foreign-uuid', 1_500_000_000_000, '/x/foo.bar'); - expect(verifySessionOwnership(requested, 'foreign-uuid', { homeDir: fakeHome })).toBe(false); - }); - - it('rejects a session that never recorded a cwd', () => { + it('rejects a session id whose jsonl lives under a different cwd', () => { const worktree = '/Users/x/repo/ws-3'; - writeSession(worktree, 'empty-uuid', 1_500_000_000_000, null); - expect(verifySessionOwnership(worktree, 'empty-uuid', { homeDir: fakeHome })).toBe(false); + writeSession('/Users/x/repo/other-ws', 'other-uuid', 1_500_000_000_000); + expect(verifySessionOwnership(worktree, 'other-uuid', { homeDir: fakeHome })).toBe(false); + }); + + it('accepts a session stored under the physical form of a symlinked cwd', () => { + // macOS: os.tmpdir() is /var/... which is a symlink to /private/var/.... + // Claude keys the store by its process cwd (physical form); the caller + // may hold the logical form. Both must verify. + const logicalDir = mkdtempSync(join(tmpdir(), 'csd-sym-')); + try { + const physicalDir = realpathSync(logicalDir); + writeSession(physicalDir, 'sym-uuid', 1_500_000_000_000); + expect(verifySessionOwnership(logicalDir, 'sym-uuid', { homeDir: fakeHome })).toBe(true); + } finally { + rmSync(logicalDir, { recursive: true, force: true }); + } }); }); }); diff --git a/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts b/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts index 628c996f5..e44c47e72 100644 --- a/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts +++ b/packages/codev/src/agent-farm/__tests__/discover-resume-session.test.ts @@ -36,13 +36,7 @@ function writeSession(projectsRoot: string, absPath: string, uuid: string, mtime const dir = join(projectsRoot, encodeClaudeProjectDir(absPath)); mkdirSync(dir, { recursive: true }); const file = join(dir, `${uuid}.jsonl`); - // Shaped like the real store: a user record carrying the launch cwd, which - // discovery verifies ownership against (Issue #1145). - writeFileSync( - file, - `{"type":"mode","sessionId":"${uuid}"}\n{"type":"user","cwd":"${absPath}","sessionId":"${uuid}"}\n`, - 'utf-8', - ); + writeFileSync(file, `{"sessionId":"${uuid}"}\n`, 'utf-8'); const t = mtimeMs / 1000; utimesSync(file, t, t); } diff --git a/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts b/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts index 745ee5b27..f6fb97a96 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-instances.test.ts @@ -611,16 +611,13 @@ describe('tower-instances', () => { // ========================================================================= describe('Issue #929/#1145 — architect resume comes only from the stored row', () => { - // Shaped like the real store: metadata line, then a user line whose cwd - // matches the workspace. Discovery WOULD have accepted this session — the - // fresh boot below proves the fallback is gone, not merely cwd-filtered. + // A session in this workspace's own store dir — exactly what the removed + // discovery fallback would have picked up and resumed. The fresh boot + // below proves the fallback is gone. function writePersonalClaudeSession(fakeHome: string, cwd: string, uuid: string): void { const dir = path.join(fakeHome, '.claude', 'projects', encodeClaudeProjectDir(cwd)); fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync( - path.join(dir, `${uuid}.jsonl`), - `{"type":"mode","sessionId":"${uuid}"}\n{"type":"user","cwd":"${cwd}","sessionId":"${uuid}"}\n`, - ); + fs.writeFileSync(path.join(dir, `${uuid}.jsonl`), `{"sessionId":"${uuid}"}\n`); } function makeCapturingDeps(): { deps: InstanceDeps; createSession: ReturnType } { diff --git a/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts b/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts index d43976283..7db24749d 100644 --- a/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts +++ b/packages/codev/src/agent-farm/__tests__/tower-utils.test.ts @@ -33,23 +33,13 @@ import { encodeClaudeProjectDir } from '../utils/claude-session-discovery.js'; /** * Issue #1145: stored-id resume now requires the session jsonl to exist under - * the (test-pinned) home dir and to record the launching cwd. This writes a - * minimally-real session file so ownership verification passes — or, with a - * divergent `recordedCwd`, deliberately fails. + * the (test-pinned) home dir. This writes a minimal session file so ownership + * verification passes; omit it to simulate a stale stored id. */ -function writeSessionFixture( - homeDir: string, - cwdPath: string, - uuid: string, - recordedCwd: string = cwdPath, -): void { +function writeSessionFixture(homeDir: string, cwdPath: string, uuid: string): void { const dir = path.join(homeDir, '.claude', 'projects', encodeClaudeProjectDir(cwdPath)); fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync( - path.join(dir, `${uuid}.jsonl`), - `{"type":"mode","sessionId":"${uuid}"}\n{"type":"user","cwd":"${recordedCwd}","sessionId":"${uuid}"}\n`, - 'utf-8', - ); + fs.writeFileSync(path.join(dir, `${uuid}.jsonl`), `{"sessionId":"${uuid}"}\n`, 'utf-8'); } describe('tower-utils', () => { @@ -310,8 +300,8 @@ describe('resolveArchitectLaunch (Issue #832)', () => { expect(sessionId).not.toBe('ghost-id'); }); - it('spawns fresh when the stored session records a different project cwd', () => { - writeSessionFixture(fakeHome, workspace, 'foreign-id', '/somewhere/else/entirely'); + it('spawns fresh when the stored session file lives under a different cwd', () => { + writeSessionFixture(fakeHome, '/somewhere/else/entirely', 'foreign-id'); const { args, resumed, sessionId } = resolveArchitectLaunch({ workspacePath: workspace, name: 'main', baseArgs: [], storedSessionId: 'foreign-id', homeDir: fakeHome, }); diff --git a/packages/codev/src/agent-farm/servers/tower-utils.ts b/packages/codev/src/agent-farm/servers/tower-utils.ts index 8d254b7e4..ad5ab9852 100644 --- a/packages/codev/src/agent-farm/servers/tower-utils.ts +++ b/packages/codev/src/agent-farm/servers/tower-utils.ts @@ -226,8 +226,8 @@ function sessionIsOwned( * prompt); echo the same id back. * 3. Else fresh → generate a new id, pin the session to it (with role injection), * and return it for the caller to persist. A stored id that fails - * ownership verification (stale jsonl, foreign project scope) lands here, - * and the fresh id the caller persists replaces it. + * ownership verification (its jsonl no longer exists) lands here, and + * the fresh id the caller persists replaces it. * * The returned `sessionId` is the value the caller writes onto the architect row, * so the column is populated correctly on every spawn. @@ -249,9 +249,9 @@ export function resolveArchitectLaunch(opts: { } // 2. Resume the persisted conversation (role injection skipped) — but only - // when the session on disk verifiably belongs to this workspace. A failed or - // throwing check falls through to a fresh spawn (Issue #1145: never attach - // to a conversation we cannot prove is ours). + // when the session still exists on disk for this workspace. A failed or + // throwing check falls through to a fresh spawn (Issue #1145: a stored id + // can outlive its jsonl, and resuming it would crash-loop the restart). if (storedSessionId && sessionIsOwned(harness, storedSessionId, workspacePath, homeDir)) { return { args: [...baseArgs, ...harness.session.resumeArgs(storedSessionId)], diff --git a/packages/codev/src/agent-farm/utils/claude-session-discovery.ts b/packages/codev/src/agent-farm/utils/claude-session-discovery.ts index 4c414100c..d2c79c857 100644 --- a/packages/codev/src/agent-farm/utils/claude-session-discovery.ts +++ b/packages/codev/src/agent-farm/utils/claude-session-discovery.ts @@ -8,37 +8,21 @@ // that ran in this directory" so reviving a dead builder can resume via // `claude --resume ` without any spawn-time bookkeeping. // -// Because the encoding is lossy ('/' and '.' collapse to the same character), -// two different paths can collide into one store directory. Every candidate is -// therefore verified against the `cwd` the session itself recorded (Issue -// #1145): a jsonl whose recorded cwd does not match the requested path is -// skipped, as is one that never recorded a cwd at all (a session with no user -// message has nothing worth resuming). -// -// This remains a heuristic for shared cwds — multiple matching jsonls mean -// multiple past sessions in the same directory, and we pick the most recent. -// For builder worktrees that almost always means the right one; architect -// launch no longer uses discovery at all (Issue #1145). - -import { closeSync, existsSync, openSync, readdirSync, readSync, statSync } from 'node:fs'; +// This is intentionally a heuristic — multiple jsonl files in the same +// directory mean multiple past sessions, and we pick the most recent. For +// builder worktrees (Agent-Farm-managed paths, effectively private cwds) that +// almost always means the right one. Architect launch no longer uses discovery +// at all (Issue #1145): it resumes solely from the session id stored on the +// workspace-scoped architect row, after verifySessionOwnership (below) +// confirms the session still exists on disk. + +import { existsSync, readdirSync, statSync } from 'node:fs'; import { realpathSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; -import { StringDecoder } from 'node:string_decoder'; const JSONL_EXT = '.jsonl'; -// Chunk size for the streaming cwd scan. The scan is semantic, not positional: -// it reads chunk by chunk until the first record carrying a `cwd` (or EOF), so -// ownership never depends on the byte offset the record happens to sit at. -// Real sessions record cwd on the first user message, so one chunk suffices. -const CWD_SCAN_CHUNK_BYTES = 64 * 1024; - -// A single buffered line larger than this is dropped un-parsed (scanning -// continues on later records). Only guards runaway memory on pathological -// files; every user record carries a cwd, so a later one still qualifies. -const CWD_SCAN_MAX_LINE_BYTES = 8 * 1024 * 1024; - /** * Encode an absolute path to the directory name Claude uses under * ~/.claude/projects/. The scheme is: replace every '/' and '.' with '-'. @@ -62,86 +46,9 @@ function realpathOrSelf(p: string): string { } } -/** Parse one jsonl line and return its top-level `cwd`, or null. */ -function parseCwdLine(line: string): string | null { - if (!line.includes('"cwd"')) return null; - try { - const record = JSON.parse(line) as { cwd?: unknown }; - if (typeof record.cwd === 'string' && record.cwd.length > 0) { - return record.cwd; - } - } catch { - // Malformed or fragmentary line — skip. - } - return null; -} - -/** - * Read the `cwd` a session jsonl recorded, or null if the session never - * recorded one. Session files interleave metadata records (mode, - * file-history-snapshot, ...) with message records; the first user record - * carries the launch cwd. Streams the file chunk by chunk with early exit, - * so the result depends only on the file's records, never on their offsets. - */ -export function readSessionCwd(filePath: string): string | null { - let fd: number; - try { - fd = openSync(filePath, 'r'); - } catch { - return null; - } - - try { - const chunk = Buffer.alloc(CWD_SCAN_CHUNK_BYTES); - const decoder = new StringDecoder('utf8'); - let carry = ''; - let position = 0; - - for (;;) { - const bytesRead = readSync(fd, chunk, 0, CWD_SCAN_CHUNK_BYTES, position); - if (bytesRead <= 0) break; - position += bytesRead; - carry += decoder.write(chunk.subarray(0, bytesRead)); - - let newlineIdx = carry.indexOf('\n'); - while (newlineIdx !== -1) { - const cwd = parseCwdLine(carry.slice(0, newlineIdx)); - if (cwd) return cwd; - carry = carry.slice(newlineIdx + 1); - newlineIdx = carry.indexOf('\n'); - } - - if (carry.length > CWD_SCAN_MAX_LINE_BYTES) { - // Oversized record: drop the buffered prefix and keep scanning. The - // remainder of this line arrives in later chunks and fails JSON.parse - // as a fragment, which parseCwdLine skips harmlessly. - carry = ''; - } - } - - // Final line without a trailing newline. - return parseCwdLine(carry + decoder.end()); - } catch { - return null; - } finally { - closeSync(fd); - } -} - -/** - * True when the session jsonl's recorded cwd matches `absolutePath` (both - * sides realpath-canonicalized, so symlinked launch paths compare equal). - */ -function sessionFileOwnedBy(filePath: string, absolutePath: string): boolean { - const recordedCwd = readSessionCwd(filePath); - if (!recordedCwd) return false; - return realpathOrSelf(recordedCwd) === realpathOrSelf(absolutePath); -} - /** * Return the session UUID of the most-recently-modified jsonl in the Claude - * project dir for the given cwd whose recorded cwd actually matches that path - * (Issue #1145 ownership check), or null if none qualifies. + * project dir for the given cwd, or null if none exists. * * `opts.homeDir` lets tests pin the home directory; otherwise resolves via * `os.homedir()`. @@ -154,34 +61,35 @@ export function findLatestSessionId( const dir = join(home, '.claude', 'projects', encodeClaudeProjectDir(absolutePath)); if (!existsSync(dir)) return null; - const candidates: Array<{ name: string; mtime: number }> = []; + let bestName: string | null = null; + let bestMtime = -Infinity; for (const entry of readdirSync(dir, { withFileTypes: true })) { if (!entry.isFile() || !entry.name.endsWith(JSONL_EXT)) continue; const fullPath = join(dir, entry.name); try { - candidates.push({ name: entry.name, mtime: statSync(fullPath).mtimeMs }); + const mtime = statSync(fullPath).mtimeMs; + if (mtime > bestMtime) { + bestMtime = mtime; + bestName = entry.name; + } } catch { // stat failed (race with deletion, permissions) — skip } } - candidates.sort((a, b) => b.mtime - a.mtime); - - for (const candidate of candidates) { - if (sessionFileOwnedBy(join(dir, candidate.name), absolutePath)) { - return candidate.name.slice(0, -JSONL_EXT.length); - } - } - return null; + if (!bestName) return null; + return bestName.slice(0, -JSONL_EXT.length); } /** - * Verify that a specific session id belongs to `absolutePath` (Issue #1145): - * its jsonl must exist in the cwd-encoded project dir AND its recorded cwd - * must match. Used before resuming a *stored* session id, so a stale or - * foreign id degrades to a fresh spawn instead of attaching to someone - * else's conversation. + * Verify that a stored session id still has a session file on disk for + * `absolutePath` (Issue #1145). Stored ids are minted by us and persisted + * keyed by workspace path, so existence is the meaningful check: a row can + * outlive its jsonl (`workspace stop` preserves rows; ~/.claude gets pruned + * independently), and resuming a deleted session bakes a broken `--resume` + * into a shellper restart loop (the #929 crash-loop class). A failed check + * degrades to a fresh spawn. */ export function verifySessionOwnership( absolutePath: string, @@ -197,8 +105,7 @@ export function verifySessionOwnership( encodeClaudeProjectDir(realpathOrSelf(absolutePath)), ]); for (const dir of candidateDirs) { - const filePath = join(home, '.claude', 'projects', dir, `${sessionId}${JSONL_EXT}`); - if (existsSync(filePath) && sessionFileOwnedBy(filePath, absolutePath)) { + if (existsSync(join(home, '.claude', 'projects', dir, `${sessionId}${JSONL_EXT}`))) { return true; } } diff --git a/packages/codev/src/agent-farm/utils/harness.ts b/packages/codev/src/agent-farm/utils/harness.ts index f5a4211ce..319bb4bdb 100644 --- a/packages/codev/src/agent-farm/utils/harness.ts +++ b/packages/codev/src/agent-farm/utils/harness.ts @@ -71,11 +71,11 @@ export interface HarnessProvider { /** Args to RESUME an existing session by id (caller skips role injection). */ resumeArgs(sessionId: string): string[]; /** - * Optional: verify that `sessionId`'s on-disk session actually belongs to - * `cwd` before the caller resumes it (Issue #1145). Returns false for a - * missing session file or one whose recorded project scope points at a - * different directory; callers then spawn fresh instead of attaching to a - * foreign conversation. Harnesses that omit this are trusted as-is. + * Optional: verify that `sessionId` still has a resumable session on disk + * for `cwd` before the caller resumes it (Issue #1145). Returns false when + * the session file is gone (a stored id can outlive its jsonl); callers + * then spawn fresh instead of baking a broken resume into a restart loop. + * Harnesses that omit this are trusted as-is. */ verifyOwnership?(sessionId: string, cwd: string, opts?: { homeDir?: string }): boolean; }; @@ -90,12 +90,11 @@ export interface HarnessProvider { * cwd-keyed session store → callers fall back to a fresh launch. Only Claude * implements it (store: ~/.claude/projects//.jsonl). * - * Discovery-based (newest owned jsonl by mtime): used for builder resume + * Discovery-based (newest jsonl by mtime): used for builder resume * (#831/#929) ONLY. Architect launch never discovers — it resumes solely from * the stored session id on the workspace-scoped architect row, else spawns * fresh (Issue #1145: discovery on a fresh workspace hijacked whatever Claude - * conversation the user last held in that directory). Candidates are verified - * against the cwd recorded inside the session file before being offered. + * conversation the user last held in that directory). */ buildResume?(absolutePath: string, opts?: { homeDir?: string }): { sessionId: string; @@ -143,8 +142,8 @@ export const CLAUDE_HARNESS: HarnessProvider = { session: { newSessionArgs: (sessionId) => ['--session-id', sessionId], resumeArgs: (sessionId) => ['--resume', sessionId], - // Issue #1145: a stored id is only resumed when its jsonl exists under this - // cwd's project dir and records this cwd as its project scope. + // Issue #1145: a stored id is only resumed when its jsonl still exists + // under this cwd's project dir (stale ids degrade to a fresh spawn). verifyOwnership: (sessionId, cwd, opts) => verifySessionOwnership(cwd, sessionId, opts), }, }; From 7515cbfe7876c52a040ad30ad9479df4f4021bf0 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:35:57 +0700 Subject: [PATCH 14/23] [PIR #1145] Comment: point at sibling resumption via the reconcile loop --- packages/codev/src/agent-farm/servers/tower-instances.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/codev/src/agent-farm/servers/tower-instances.ts b/packages/codev/src/agent-farm/servers/tower-instances.ts index 308cffebb..ded1d85c5 100644 --- a/packages/codev/src/agent-farm/servers/tower-instances.ts +++ b/packages/codev/src/agent-farm/servers/tower-instances.ts @@ -469,7 +469,10 @@ export async function launchInstance(workspacePath: string): Promise<{ success: // workspace-scoped row carries a session id (which resolveArchitectLaunch // additionally ownership-verifies against the on-disk session store); // anything else spawns fresh with a newly minted id, persisted on the - // architect row below so the next restart resumes it. + // architect row below so the next restart resumes it. This block only + // handles `main`; sibling architects resume the same way via the + // reconciliation loop at the end of launchInstance → addArchitect, + // which reads each sibling's own row. // // The mtime-based jsonl-discovery fallback that used to run here for // legacy pre-#832 rows was removed by #1145: on a fresh workspace From e91b9a36208b6a947a847cabffd247fd17a20d4d Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:38:11 +0700 Subject: [PATCH 15/23] chore(porch): 1145 dev-approval gate-approved --- .../projects/1145-codev-adopt-launchinstance-mai/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index baff30f72..40cdea3b6 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -10,12 +10,13 @@ gates: requested_at: '2026-07-08T06:36:03.156Z' approved_at: '2026-07-09T04:26:17.754Z' dev-approval: - status: pending + status: approved requested_at: '2026-07-09T04:41:45.899Z' + approved_at: '2026-07-11T00:38:11.861Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-09T04:41:45.899Z' +updated_at: '2026-07-11T00:38:11.861Z' From 5e8960f5afd45e5f773ea3c3273515801d41dadf Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:38:23 +0700 Subject: [PATCH 16/23] chore(porch): 1145 review phase-transition --- .../projects/1145-codev-adopt-launchinstance-mai/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 40cdea3b6..393c84fca 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -1,7 +1,7 @@ id: '1145' title: codev-adopt-launchinstance-mai 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-07-08T06:30:47.411Z' -updated_at: '2026-07-11T00:38:11.861Z' +updated_at: '2026-07-11T00:38:23.216Z' From 42f0b06c2f6628b6909d3300b2084aad55c9a3e0 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:41:07 +0700 Subject: [PATCH 17/23] [PIR #1145] Review + retrospective --- codev/resources/arch.md | 2 +- codev/resources/lessons-learned.md | 3 +- .../1145-codev-adopt-launchinstance-mai.md | 73 +++++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 codev/reviews/1145-codev-adopt-launchinstance-mai.md diff --git a/codev/resources/arch.md b/codev/resources/arch.md index 54ee62ac5..9f10ceaa6 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -275,7 +275,7 @@ A `codev doctor` audit (`lib/framework-ref-audit.ts`) flags shell-fetch of frame > **Caveat — unrecognized override commands still default to the claude harness (tracked in cluesmith/codev#1062).** `#929`'s override-awareness only covers *recognized* harness commands (claude/codex/gemini/opencode, matched by `detectHarnessFromCommand`). An override command the detector does **not** recognize — e.g. `TOWER_ARCHITECT_CMD=bash`, a wrapper script, or any custom launcher — with **no** explicit `shell.architectHarness` / `shell.builderHarness` falls through `resolveHarness` to the **claude** harness (`harness.ts`, the final `return CLAUDE_HARNESS`). With a stale Claude `.jsonl` present, that can still build ` --resume ` for the unrecognized command. This is **pre-existing and narrow** (not a #929 regression — #929 strictly *improved* the recognized codex case) and separable. Mitigation today: set an explicit `shell.architectHarness` / `shell.builderHarness` when using an unrecognized launcher command. -**Conversation resume is Claude-main-only.** `launchInstance` resumes a prior session only when the configured harness implements `HarnessProvider.buildResume` — currently just claude (its sessions live at `~/.claude/projects//*.jsonl`). Codex architects (and resumed codex/gemini *builders*, via `spawn.ts` `discoverResumeSession`) return `null` from `buildResume` and relaunch fresh with role injection. This gating fixes a latent crash-loop where a non-Claude harness + a stale Claude `.jsonl` built an invalid ` --resume ` invocation and shellper restart-looped to death. +**Architect conversation resume is stored-id-only (#832 / #1145).** Every architect launch path (`launchInstance` main spawn, `add-architect` / sibling reconcile, both shellper restart-bake sites) resolves through `resolveArchitectLaunch`: a session id stored on the workspace-scoped architect row (minted at spawn, pinned via the harness's `session.newSessionArgs`) is resumed only after the harness's optional `session.verifyOwnership` confirms the session file still exists for this cwd (Claude: `~/.claude/projects//.jsonl`, accepted under either the logical or physical form of a symlinked path). Anything else — no row, no stored id, missing file — spawns fresh with role injection and a newly minted, persisted id. `launchInstance`'s mtime-based jsonl-discovery fallback was **removed** by #1145: on a fresh workspace (`codev adopt` / first touch) it resumed whatever Claude conversation the user last held in that directory — hijacking personal sessions, roleless too, since the resume path skips role injection. Do not reintroduce discovery on any architect path; mtime cannot distinguish an architect's session from a newer personal one in the same cwd. Discovery (`HarnessProvider.buildResume`, newest jsonl by mtime) survives for **builder resume only** (`spawn.ts` `discoverResumeSession`), harness-gated to claude — the gating that fixes the latent crash-loop where a non-Claude harness + a stale Claude `.jsonl` built an invalid ` --resume ` invocation and shellper restart-looped to death. **Architect role injection is centralized in `buildArchitectArgs`** (`tower-utils.ts`), the shared helper every architect-launch path routes through — `launchInstance` (fresh), `add-architect` (sibling), shellper reconnect (×2), and the no-Tower `afx architect` (refactored in #929 to call `buildArchitectArgs` instead of duplicating injection). So the architect role is injected on **every** launch path, not just first-activation. (No architect context-file seam exists: claude/codex read project context natively; the gemini-only `getArchitectFiles` seam #1059 introduced was removed when gemini's architect support was dropped.) diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index be3a68e61..95e988292 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -66,7 +66,7 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated ## Architecture -- [From 1144] When activation becomes ambient (`onStartupFinished` runs in every window), any detection heuristic feeding it becomes an **inertness gate** and must be downgraded to an exact, one-sentence-explainable rule. The extension's workspace detector walked ancestors to the filesystem root looking for `codev/`/`.codev/`; harmless while activation implied a codev workspace, but under ambient activation every folder beneath the user's codev-enabled home directory silently inherited that workspace, connected to Tower, and rendered live workspace actions in unrelated projects. The fix was "the opened folder itself is the workspace root, or nothing" (a settings override covers exotic layouts), which also matches the CLI's semantics: afx runs from the workspace root and never marker-walks. Audit convenience heuristics whenever the trigger that constrained them broadens. +- [From #1145] Gate a legacy-bridge fallback on **positive evidence of the legacy state it bridges**, never on absence of conflict. The #832 jsonl-discovery fallback existed to rescue pre-#832 architect rows (row present, no stored session id) but was gated on `getArchitects().length <= 1` — and a *fresh* workspace (zero rows) satisfies "at most one", so the bridge became the default behavior for every first-ever `codev adopt`, resuming whatever personal Claude conversation the user last held in that directory (roleless, too, since resume skips role injection). Second failure mode found at review: even row-gated, the bridge is unsound — newest-by-mtime in a shared cwd cannot distinguish the architect's last session from a *newer personal* one, so the fallback was removed outright rather than re-gated (cost: one fresh spawn for workspaces dormant since #832, which then self-heal onto the stored-UUID path). Companion lesson from the same review: before defending a protective layer, enumerate the concrete threat it guards — the cwd-content ownership scan defended only path-encoding collisions that Agent-Farm's own worktree path conventions already preclude, and it fell to three "why is this here at all?" questions; what survived was the one check with a real threat behind it (session file existence, guarding the stale-stored-id `--resume` crash-loop). When activation becomes ambient (`onStartupFinished` runs in every window), any detection heuristic feeding it becomes an **inertness gate** and must be downgraded to an exact, one-sentence-explainable rule. The extension's workspace detector walked ancestors to the filesystem root looking for `codev/`/`.codev/`; harmless while activation implied a codev workspace, but under ambient activation every folder beneath the user's codev-enabled home directory silently inherited that workspace, connected to Tower, and rendered live workspace actions in unrelated projects. The fix was "the opened folder itself is the workspace root, or nothing" (a settings override covers exotic layouts), which also matches the CLI's semantics: afx runs from the workspace root and never marker-walks. Audit convenience heuristics whenever the trigger that constrained them broadens. - [From #1140] When a process derives identity from an inherited environment variable, every *programmatic* respawn/recovery path must set that variable explicitly from recorded state, never inherit it. `afx spawn` derives the new builder's `spawned_by_architect` from `CODEV_ARCHITECT_NAME` (correct interactively, because Tower injects it per architect terminal), but `afx workspace recover` re-invoked spawn via `child_process.spawn` with no `env` override, so after any reboot every recovered builder was silently reattributed to whichever architect's shell ran the recovery: affinity routing, the `architect:` anti-spoofing check, and Agents-view ownership all broke. The fix reads the recorded value from `global.db` via the *same* helper the message router uses (`lookupBuilderSpawningArchitect`) and forces it into the child env; rows with no recorded value pass the base env through so the interactive default stays owned by `spawn.ts` alone. Env-derived identity + programmatic re-invocation is a recurring bug shape: the env var is a per-terminal fact, and any automation that fans out from one terminal turns it into a wrong global. - [From #1118] When a subsystem's *scope* outgrows its storage *location*, relocate the storage to match — don't patch the scope with a column and leave the location lying. `state.db` was born per-workspace; when Tower became a system-wide singleton its scope became user-global, but the file stayed at a cwd-dependent `/.agent-farm/state.db`. Bugfix #826 patched the *symptom* (a `workspace_path` column so cross-workspace rows could coexist in one file) while leaving the *location* wrong, so which file Tower opened still depended on its start-cwd — architect state "disappeared" after any restart from a different directory. The real fix was to move the tables into the already-user-global `global.db` and make `getDb()` return it. Corollary caught mid-implementation: moving a table into a *shared* DB forces you to re-examine its primary key — `builders` was keyed by `id` alone (safe when each workspace had its own file), but ids are `-`, unique per repo yet **reused across repos**, so the shared table needs a composite `(workspace_path, id)` PK (mirroring what #826 did for `architect`) or same-id builders from two repos collide — and that collision is security-relevant (it feeds the `afx send` spoofing check). The issue's "tables move as-is" framing was true for 3 of 4 tables but not the one whose key assumed per-file isolation. Migration technique that generalized: an **upsert-if-newer** engine (`ON CONFLICT DO UPDATE … WHERE excluded.started_at > existing`) is the same code for the empty-target one-off and the non-empty satellite import, and reading legacy source files **defensively** (`PRAGMA table_info` + synthesizing the new key column) handles heterogeneous historical schema versions without running the old migration ladder. - [From #832] A recovery mechanism that keys off a process holding a resource *open* (a file descriptor, a lock) is only as sound as that assumption — verify it empirically *before* building on it. #832's first design disambiguated sibling architects (which share one cwd) by correlating each one's process subtree to the session jsonl it "holds open" via `lsof`. But Claude appends to its session jsonl and **closes** it between writes: `lsof -p ` against a live architect showed **zero** jsonl fds, so the correlation never matched — every apparent success was the *unrelated* sole-architect newest-by-mtime fallback. One captured `lsof` (raw data, not reasoning) falsified the whole subsystem in minutes; it had passed unit tests because the tests only exercised the fallback. Corollary: don't delete a *working* self-recovery path to rebuild it as a bridge that can't cover the hard case. The branch had removed #830's sole-architect jsonl resume for `main` and replaced it with a backfill that — because pre-existing siblings are fundamentally unrecoverable (no `--session-id` arg, no open fd, only a jsonl filename with no reliable pid bridge) — could only ever rescue `main`, which the original fallback already did losslessly. Restoring the existing fallback (gated to the unambiguous sole-architect case) and deleting the entire bridge was strictly simpler and lost nothing. @@ -258,6 +258,7 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated - [From 0053] Clean up stale node processes from previous tests before running new tests. Lingering processes cause confusing failures. - [From 0056] Path fallback patterns need explicit testing -- mock both paths (new location exists, old location exists, neither exists) to catch regressions. Always run `copy-skeleton` after modifying `codev-skeleton/` to ensure changes propagate. - [From 456] Vitest 4 constructor mocks require class syntax — `vi.fn(() => ({...}))` throws "is not a constructor". Use `vi.mock('module', () => ({ ClassName: class MockClass { ... } }))` with `vi.hoisted()` for shared mock functions. +- [From #1145] Never assert CLI flags via substring over a `JSON.stringify`'d spawn call — assert on the argv token array. The Claude architect launch embeds the whole role prompt as one `--append-system-prompt` argument, and that prose legitimately contains flag strings like `--resume` in its CLI examples, so `expect(callStr).not.toContain('--resume')` false-fails on a correct fresh launch (and the mirror `toContain` can false-pass). `expect(callArgs).not.toContain('--resume')` checks whole-element equality and is immune. - [From 936] `vi.mock('@cluesmith/codev-*')` still requires the mocked workspace package to *resolve* — Vitest resolves the module ID before substituting the factory. Packages whose `exports` point at an unbuilt `dist/` (e.g. `@cluesmith/codev-types`, `@cluesmith/codev-core/*`) fail with "Failed to resolve entry for package" until built. In a fresh worktree, `pnpm --filter build` the workspace deps before `vitest run`, even for deps you intend to mock. - [From 456] Avoid duplicate React effects that fire on the same dependency change. Two `useEffect` hooks both depending on `isActive` will both fire when `isActive` transitions, causing double-fetches. Merge into a single effect. - [From 0058] Debouncing is essential for search inputs to prevent excessive DOM updates. A global Escape key handler adds resilience by ensuring modals/overlays can always be dismissed. diff --git a/codev/reviews/1145-codev-adopt-launchinstance-mai.md b/codev/reviews/1145-codev-adopt-launchinstance-mai.md new file mode 100644 index 000000000..f2ff17dcb --- /dev/null +++ b/codev/reviews/1145-codev-adopt-launchinstance-mai.md @@ -0,0 +1,73 @@ +# PIR Review: launchInstance no longer hijacks unrelated Claude sessions + +Fixes #1145 + +## Summary + +`codev adopt` (or Tower's first-touch `launchInstance`) on a machine where the user had ever chatted with Claude Code in that directory booted the "main architect" *inside the user's personal conversation* — with no architect role, since the resume path skips role injection. The cause was the #832 legacy-bridge fallback: gated on `getArchitects().length <= 1`, which a fresh workspace (zero rows) satisfies, it ran newest-jsonl-by-mtime discovery over `~/.claude/projects//` and resumed whatever it found. The fallback is now removed outright (not re-gated — mtime cannot distinguish the architect's last session from a newer personal one in the same cwd): architect resume comes exclusively from the session id stored on the workspace-scoped architect row, validated by a new harness-gated existence check (`session.verifyOwnership`) so a stale stored id degrades to a fresh spawn instead of a `--resume` crash-loop. Builder resume (mtime discovery in Agent-Farm-managed worktree cwds) is unchanged. + +## Files Changed + +- `codev/plans/1145-codev-adopt-launchinstance-mai.md` (+91 / -0) +- `codev/resources/arch.md` (+1 / -1) +- `codev/resources/lessons-learned.md` (+2 / -1) +- `codev/state/pir-1145_thread.md` (+35 / -0) +- `packages/codev/src/agent-farm/__tests__/claude-session-discovery.test.ts` (+76 / -38) +- `packages/codev/src/agent-farm/__tests__/tower-instances.test.ts` (+59 / -16) +- `packages/codev/src/agent-farm/__tests__/tower-utils.test.ts` (+70 / -11) +- `packages/codev/src/agent-farm/servers/tower-instances.ts` (+21 / -28) +- `packages/codev/src/agent-farm/servers/tower-utils.ts` (+42 / -7) +- `packages/codev/src/agent-farm/utils/claude-session-discovery.ts` (+47 / -4) +- `packages/codev/src/agent-farm/utils/harness.ts` (+17 / -4) + +(plus porch-managed `codev/projects/1145-*/status.yaml`) + +## Commits + +- `f5910e67` [PIR #1145] Verify Claude session ownership before any resume +- `0ee53387` [PIR #1145] Remove architect jsonl-discovery fallback in launchInstance +- `b10e71d6` [PIR #1145] Update tests: ownership verification, fresh-boot regression +- `6fc3636c` [PIR #1145] Make the cwd scan streaming: semantic, not byte-offset-bound +- `00284f3f` [PIR #1145] Narrow ownership verification to file existence; drop content scanning +- `7515cbfe` [PIR #1145] Comment: point at sibling resumption via the reconcile loop + +(`6fc3636c` and `00284f3f` reflect dev-approval-gate feedback; the streaming scanner introduced by the former was removed by the latter — see Things to Look At.) + +## Test Results + +- `pnpm build`: ✓ pass +- `pnpm test` (full `@cluesmith/codev` suite): ✓ 3451 passed, 48 skipped (pre-existing skips, none added) +- Touched test files: 109 tests, ~20 new/updated cases, including the #1145 regression ("fresh workspace + personal Claude session in the same cwd → boots fresh, no `--resume`") and the preserved #929 codex guard +- Manual verification: the reviewer worked the change over at the `dev-approval` gate through iterative design review (fallback scope, multi-architect resumption, verification necessity); the end-to-end fresh-adopt scenario is scripted below for the PR reviewer + +## Architecture Updates + +Routed to **COLD** `codev/resources/arch.md` (Agent Farm Internals): rewrote the stale "Conversation resume is Claude-main-only" paragraph as "Architect conversation resume is stored-id-only (#832 / #1145)" — documenting the new invariant (stored row id + existence verification, no discovery on any architect path, discovery is builder-only) and an explicit "do not reintroduce discovery on architect paths" warning with the reason. Not HOT-tier material: it's an agent-farm-internal invariant, not a cross-cutting fact that changes day-to-day implementation choices, and the hot file is at its cap. + +## Lessons Learned Updates + +Routed to **COLD** `codev/resources/lessons-learned.md`: + +- **Architecture**: gate a legacy-bridge fallback on positive evidence of the legacy state it bridges, never on absence of conflict (`length <= 1` is satisfied by zero rows, turning the bridge into default behavior for fresh installs); and enumerate the concrete threat behind each protective layer before defending it — the cwd-content scan defended only path-encoding collisions that Agent-Farm's own path conventions preclude. +- **Testing**: never assert CLI flags via substring over a stringified spawn call — the injected role prompt legitimately contains flag strings like `--resume` in its CLI examples; assert on argv tokens. + +Not HOT-tier: both are situation-specific recipes, not behavior-changing cross-cutting rules; the hot file is at its cap. + +## Things to Look At During PR Review + +- **Deviation from the approved plan.** The plan specified cwd-*content* ownership verification (reading the `cwd` recorded inside the session jsonl), per the issue's fix sketch. At the `dev-approval` gate the reviewer walked this back in three steps: content scanning defended only lossy-encoding collisions; the fixed scan window made resume content-dependent; and collisions are contrived for both architects (row-gated, ids minted by us) and builders (Agent-Farm-managed worktree paths). Final shape is existence-only verification. The intermediate streaming scanner (`6fc3636c`) was added and then removed (`00284f3f`) as part of that walk-back, so the net diff never contains it. +- **`verifySessionOwnership` checks two directory encodings** (logical and `realpath`'d) because Claude keys its store by process cwd, which the OS reports in physical form for symlinked paths (macOS `/tmp` → `/private/tmp`). Covered by a dedicated symlink test. +- **Degradation semantics**: a `global.db` read failure, a missing session file, or a throwing harness check all resolve to *fresh spawn* — the deliberate direction is "never resume on uncertainty". Fresh spawns mint and persist a new id, so the row self-heals. +- **Legacy pre-#832 rows** (row exists, no stored id) now spawn fresh once instead of discovery-resuming — a one-time context loss for workspaces dormant since #832, accepted in plan review. +- **The name `session.verifyOwnership`** slightly overstates what it now does (existence check). The reviewer was offered a rename (`isResumable`) and did not take it up; flagging in case the PR reviewer feels differently. + +## How to Test Locally + +- **View diff**: VSCode sidebar → right-click builder `pir-1145` → **View Diff** +- **Run locally**: `pnpm build && pnpm -w run local-install` (restarts Tower) +- **What to verify** (maps to the plan's Test Plan): + 1. Create a scratch project dir (no `codev/`), run `claude` in it briefly and exit — this plants the "personal session" jsonl for that cwd. + 2. From the scratch dir, run `codev adopt` and start the workspace (or open in VS Code with auto-adopt). + 3. The main architect terminal must open a **fresh** conversation (architect role active, no prior chat context); Tower's log must not print "Resuming architect 'main' …". + 4. `afx workspace stop` then `start`: the architect must now resume its **own** stored session ("Resuming architect 'main' session …" with the minted id). + 5. Optional multi-architect check: `afx workspace add-architect --name reviewer`, stop/start, confirm both `main` and `reviewer` resume their own conversations. From 1067ea613be9135a929ab2ab508dc68c193eab09 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:41:29 +0700 Subject: [PATCH 18/23] chore(porch): 1145 record PR #1160 --- .../1145-codev-adopt-launchinstance-mai/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 393c84fca..8de7bb8cf 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -19,4 +19,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-11T00:38:23.216Z' +updated_at: '2026-07-11T00:41:29.252Z' +pr_history: + - phase: review + pr_number: 1160 + branch: builder/pir-1145 + created_at: '2026-07-11T00:41:29.251Z' From 1225c5013b08f38236ea91aebf62865e2ed1c2f1 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:41:42 +0700 Subject: [PATCH 19/23] chore(porch): 1145 review build-complete --- .../projects/1145-codev-adopt-launchinstance-mai/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 8de7bb8cf..096dfdca8 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -16,10 +16,10 @@ gates: pr: status: pending iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-11T00:41:29.252Z' +updated_at: '2026-07-11T00:41:42.206Z' pr_history: - phase: review pr_number: 1160 From ff9855b337da4de372525a5d0677d0107bf014e7 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:46:14 +0700 Subject: [PATCH 20/23] [PIR #1145] Address consultation: fix lessons merge, plan frontmatter, record dispositions --- codev/plans/1145-codev-adopt-launchinstance-mai.md | 10 ++++++++++ codev/resources/lessons-learned.md | 3 ++- codev/reviews/1145-codev-adopt-launchinstance-mai.md | 6 ++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/codev/plans/1145-codev-adopt-launchinstance-mai.md b/codev/plans/1145-codev-adopt-launchinstance-mai.md index 7166ca205..25310efcb 100644 --- a/codev/plans/1145-codev-adopt-launchinstance-mai.md +++ b/codev/plans/1145-codev-adopt-launchinstance-mai.md @@ -1,5 +1,15 @@ +--- +approved: 2026-07-09 +validated: [] +--- + # PIR Plan: Stop launchInstance from hijacking unrelated Claude sessions on fresh workspaces +> **Post-approval deviation (dev-approval gate, 2026-07-10)**: the cwd-content +> ownership verification specified below was narrowed to a file-existence check +> at the human reviewer's direction. Rationale and final shape are recorded in +> `codev/reviews/1145-codev-adopt-launchinstance-mai.md` (Things to Look At). + ## Understanding When Tower's `launchInstance` creates the `main` architect for a workspace, it decides between resuming a persisted conversation and spawning fresh. The decision code at `packages/codev/src/agent-farm/servers/tower-instances.ts:489-499` is: diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index 95e988292..db40a2b8e 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -66,7 +66,8 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated ## Architecture -- [From #1145] Gate a legacy-bridge fallback on **positive evidence of the legacy state it bridges**, never on absence of conflict. The #832 jsonl-discovery fallback existed to rescue pre-#832 architect rows (row present, no stored session id) but was gated on `getArchitects().length <= 1` — and a *fresh* workspace (zero rows) satisfies "at most one", so the bridge became the default behavior for every first-ever `codev adopt`, resuming whatever personal Claude conversation the user last held in that directory (roleless, too, since resume skips role injection). Second failure mode found at review: even row-gated, the bridge is unsound — newest-by-mtime in a shared cwd cannot distinguish the architect's last session from a *newer personal* one, so the fallback was removed outright rather than re-gated (cost: one fresh spawn for workspaces dormant since #832, which then self-heal onto the stored-UUID path). Companion lesson from the same review: before defending a protective layer, enumerate the concrete threat it guards — the cwd-content ownership scan defended only path-encoding collisions that Agent-Farm's own worktree path conventions already preclude, and it fell to three "why is this here at all?" questions; what survived was the one check with a real threat behind it (session file existence, guarding the stale-stored-id `--resume` crash-loop). When activation becomes ambient (`onStartupFinished` runs in every window), any detection heuristic feeding it becomes an **inertness gate** and must be downgraded to an exact, one-sentence-explainable rule. The extension's workspace detector walked ancestors to the filesystem root looking for `codev/`/`.codev/`; harmless while activation implied a codev workspace, but under ambient activation every folder beneath the user's codev-enabled home directory silently inherited that workspace, connected to Tower, and rendered live workspace actions in unrelated projects. The fix was "the opened folder itself is the workspace root, or nothing" (a settings override covers exotic layouts), which also matches the CLI's semantics: afx runs from the workspace root and never marker-walks. Audit convenience heuristics whenever the trigger that constrained them broadens. +- [From #1145] Gate a legacy-bridge fallback on **positive evidence of the legacy state it bridges**, never on absence of conflict. The #832 jsonl-discovery fallback existed to rescue pre-#832 architect rows (row present, no stored session id) but was gated on `getArchitects().length <= 1` — and a *fresh* workspace (zero rows) satisfies "at most one", so the bridge became the default behavior for every first-ever `codev adopt`, resuming whatever personal Claude conversation the user last held in that directory (roleless, too, since resume skips role injection). Second failure mode found at review: even row-gated, the bridge is unsound — newest-by-mtime in a shared cwd cannot distinguish the architect's last session from a *newer personal* one, so the fallback was removed outright rather than re-gated (cost: one fresh spawn for workspaces dormant since #832, which then self-heal onto the stored-UUID path). Companion lesson from the same review: before defending a protective layer, enumerate the concrete threat it guards — the cwd-content ownership scan defended only path-encoding collisions that Agent-Farm's own worktree path conventions already preclude, and it fell to three "why is this here at all?" questions; what survived was the one check with a real threat behind it (session file existence, guarding the stale-stored-id `--resume` crash-loop). +- [From 1144] When activation becomes ambient (`onStartupFinished` runs in every window), any detection heuristic feeding it becomes an **inertness gate** and must be downgraded to an exact, one-sentence-explainable rule. The extension's workspace detector walked ancestors to the filesystem root looking for `codev/`/`.codev/`; harmless while activation implied a codev workspace, but under ambient activation every folder beneath the user's codev-enabled home directory silently inherited that workspace, connected to Tower, and rendered live workspace actions in unrelated projects. The fix was "the opened folder itself is the workspace root, or nothing" (a settings override covers exotic layouts), which also matches the CLI's semantics: afx runs from the workspace root and never marker-walks. Audit convenience heuristics whenever the trigger that constrained them broadens. - [From #1140] When a process derives identity from an inherited environment variable, every *programmatic* respawn/recovery path must set that variable explicitly from recorded state, never inherit it. `afx spawn` derives the new builder's `spawned_by_architect` from `CODEV_ARCHITECT_NAME` (correct interactively, because Tower injects it per architect terminal), but `afx workspace recover` re-invoked spawn via `child_process.spawn` with no `env` override, so after any reboot every recovered builder was silently reattributed to whichever architect's shell ran the recovery: affinity routing, the `architect:` anti-spoofing check, and Agents-view ownership all broke. The fix reads the recorded value from `global.db` via the *same* helper the message router uses (`lookupBuilderSpawningArchitect`) and forces it into the child env; rows with no recorded value pass the base env through so the interactive default stays owned by `spawn.ts` alone. Env-derived identity + programmatic re-invocation is a recurring bug shape: the env var is a per-terminal fact, and any automation that fans out from one terminal turns it into a wrong global. - [From #1118] When a subsystem's *scope* outgrows its storage *location*, relocate the storage to match — don't patch the scope with a column and leave the location lying. `state.db` was born per-workspace; when Tower became a system-wide singleton its scope became user-global, but the file stayed at a cwd-dependent `/.agent-farm/state.db`. Bugfix #826 patched the *symptom* (a `workspace_path` column so cross-workspace rows could coexist in one file) while leaving the *location* wrong, so which file Tower opened still depended on its start-cwd — architect state "disappeared" after any restart from a different directory. The real fix was to move the tables into the already-user-global `global.db` and make `getDb()` return it. Corollary caught mid-implementation: moving a table into a *shared* DB forces you to re-examine its primary key — `builders` was keyed by `id` alone (safe when each workspace had its own file), but ids are `-`, unique per repo yet **reused across repos**, so the shared table needs a composite `(workspace_path, id)` PK (mirroring what #826 did for `architect`) or same-id builders from two repos collide — and that collision is security-relevant (it feeds the `afx send` spoofing check). The issue's "tables move as-is" framing was true for 3 of 4 tables but not the one whose key assumed per-file isolation. Migration technique that generalized: an **upsert-if-newer** engine (`ON CONFLICT DO UPDATE … WHERE excluded.started_at > existing`) is the same code for the empty-target one-off and the non-empty satellite import, and reading legacy source files **defensively** (`PRAGMA table_info` + synthesizing the new key column) handles heterogeneous historical schema versions without running the old migration ladder. - [From #832] A recovery mechanism that keys off a process holding a resource *open* (a file descriptor, a lock) is only as sound as that assumption — verify it empirically *before* building on it. #832's first design disambiguated sibling architects (which share one cwd) by correlating each one's process subtree to the session jsonl it "holds open" via `lsof`. But Claude appends to its session jsonl and **closes** it between writes: `lsof -p ` against a live architect showed **zero** jsonl fds, so the correlation never matched — every apparent success was the *unrelated* sole-architect newest-by-mtime fallback. One captured `lsof` (raw data, not reasoning) falsified the whole subsystem in minutes; it had passed unit tests because the tests only exercised the fallback. Corollary: don't delete a *working* self-recovery path to rebuild it as a bridge that can't cover the hard case. The branch had removed #830's sole-architect jsonl resume for `main` and replaced it with a backfill that — because pre-existing siblings are fundamentally unrecoverable (no `--session-id` arg, no open fd, only a jsonl filename with no reliable pid bridge) — could only ever rescue `main`, which the original fallback already did losslessly. Restoring the existing fallback (gated to the unambiguous sole-architect case) and deleting the entire bridge was strictly simpler and lost nothing. diff --git a/codev/reviews/1145-codev-adopt-launchinstance-mai.md b/codev/reviews/1145-codev-adopt-launchinstance-mai.md index f2ff17dcb..22669e42d 100644 --- a/codev/reviews/1145-codev-adopt-launchinstance-mai.md +++ b/codev/reviews/1145-codev-adopt-launchinstance-mai.md @@ -61,6 +61,12 @@ Not HOT-tier: both are situation-specific recipes, not behavior-changing cross-c - **Legacy pre-#832 rows** (row exists, no stored id) now spawn fresh once instead of discovery-resuming — a one-time context loss for workspaces dormant since #832, accepted in plan review. - **The name `session.verifyOwnership`** slightly overstates what it now does (existence check). The reviewer was offered a rename (`isResumable`) and did not take it up; flagging in case the PR reviewer feels differently. +### Consultation Verdicts and Dispositions (single advisory pass — PIR does not re-review) + +- **Codex: REQUEST_CHANGES** — (1) "implementation drops the approved cwd-based ownership verification; the collision vector remains for builder resume" and (2) "tests for the dropped ownership cases are missing". **Rebutted**: this is the deviation documented above, directed explicitly by the human reviewer at the `dev-approval` gate after a three-step design walk-back; the plan text was deliberately not rewritten to retro-fit the outcome (deviations belong in this review file). The collision vector requires two distinct absolute paths that encode identically AND host codev workspaces/worktrees with matching ids — contrived for Agent-Farm-managed paths, and the reviewer judged the content scan complexity not worth that threat. The dropped tests tested the dropped behavior. (3) "plan file missing approved-plan YAML frontmatter" — **fixed**: frontmatter plus a post-approval deviation note added to the plan file. +- **Claude: COMMENT** — implementation solid; one real finding: an editing error in `lessons-learned.md` merged the unrelated `[From 1144]` lesson into the new `[From #1145]` bullet. **Fixed**: split back into two bullets with the `[From 1144]` attribution restored. +- **Gemini: skipped** (non-blocking) — `agy` CLI not installed on this machine. + ## How to Test Locally - **View diff**: VSCode sidebar → right-click builder `pir-1145` → **View Diff** From 205a24ea58a68da625f0e98c43d988aa257230ab Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:47:07 +0700 Subject: [PATCH 21/23] chore(porch): 1145 pr gate-requested --- .../projects/1145-codev-adopt-launchinstance-mai/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index 096dfdca8..dd4d8bb9e 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -15,13 +15,15 @@ gates: approved_at: '2026-07-11T00:38:11.861Z' pr: status: pending + requested_at: '2026-07-11T00:47:07.380Z' iteration: 1 build_complete: true history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-11T00:41:42.206Z' +updated_at: '2026-07-11T00:47:07.382Z' pr_history: - phase: review pr_number: 1160 branch: builder/pir-1145 created_at: '2026-07-11T00:41:29.251Z' +pr_ready_for_human: true From 903b2cea599ec91c9d1668663f7907636d2299c7 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:47:48 +0700 Subject: [PATCH 22/23] [PIR #1145] Thread: review phase notes --- codev/state/pir-1145_thread.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/codev/state/pir-1145_thread.md b/codev/state/pir-1145_thread.md index 026bdd926..2f60c100d 100644 --- a/codev/state/pir-1145_thread.md +++ b/codev/state/pir-1145_thread.md @@ -33,3 +33,7 @@ Reviewer walked the design back in three steps at the gate: (1) why scan file co Final shape: architect resume = stored row id + file-*existence* check (both logical/physical path encodings, macOS symlink case); builder discovery = plain newest-by-mtime (pre-#1145 behavior). The #1145 fix proper (no discovery fallback in launchInstance) is untouched. Deviation from the approved plan (which specified cwd-content verification) to be recorded in the review file. Lesson: when a reviewer asks "why is this here at all", enumerate what each layer actually defends against before defending the layer — two of three protections were against a threat the system's own path conventions already preclude. + +## 2026-07-11 — Review phase: PR #1160, consultation, pr gate + +dev-approval approved. Retrospective written (arch.md resume paragraph rewritten to the stored-id-only invariant; two lessons routed cold). PR #1160 opened with the review as body. Consultation (single pass): codex=REQUEST_CHANGES (flagged the content-verification deviation — rebutted as human-directed at the gate; frontmatter nit fixed), claude=COMMENT (caught a real lessons-learned.md bullet-merge editing error — fixed in ff9855b3), gemini skipped (agy not installed). Rebuttals in codev/projects/1145-*/1145-review-iter1-rebuttals.md. Architect notified with the REQUEST_CHANGES disposition leading. Sitting at the pr gate. From 19c4da69ce9f2df029cd1277779ec7dcf1d02030 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Sat, 11 Jul 2026 07:50:43 +0700 Subject: [PATCH 23/23] chore(porch): 1145 pr gate-approved --- .../1145-codev-adopt-launchinstance-mai/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml index dd4d8bb9e..442c8d8f0 100644 --- a/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml +++ b/codev/projects/1145-codev-adopt-launchinstance-mai/status.yaml @@ -14,16 +14,17 @@ gates: requested_at: '2026-07-09T04:41:45.899Z' approved_at: '2026-07-11T00:38:11.861Z' pr: - status: pending + status: approved requested_at: '2026-07-11T00:47:07.380Z' + approved_at: '2026-07-11T00:50:43.768Z' iteration: 1 build_complete: true history: [] started_at: '2026-07-08T06:30:47.411Z' -updated_at: '2026-07-11T00:47:07.382Z' +updated_at: '2026-07-11T00:50:43.770Z' pr_history: - phase: review pr_number: 1160 branch: builder/pir-1145 created_at: '2026-07-11T00:41:29.251Z' -pr_ready_for_human: true +pr_ready_for_human: false