From 7db553b9a58ea6670ba6e660a800b4ff0d2c0648 Mon Sep 17 00:00:00 2001 From: pseudo Date: Mon, 31 Aug 2026 09:42:56 -0600 Subject: [PATCH 1/5] chore(porch): 271 init air --- .../271-safety-adjacent-an-architect-c/status.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 codev/projects/271-safety-adjacent-an-architect-c/status.yaml diff --git a/codev/projects/271-safety-adjacent-an-architect-c/status.yaml b/codev/projects/271-safety-adjacent-an-architect-c/status.yaml new file mode 100644 index 000000000..101fbb403 --- /dev/null +++ b/codev/projects/271-safety-adjacent-an-architect-c/status.yaml @@ -0,0 +1,14 @@ +id: '271' +title: safety-adjacent-an-architect-c +protocol: air +phase: implement +plan_phases: [] +current_plan_phase: null +gates: + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-08-31T15:42:55.843Z' +updated_at: '2026-08-31T15:42:55.844Z' From 4f56783245c6d98af9a251a027a14183801b0fc9 Mon Sep 17 00:00:00 2001 From: pseudo Date: Mon, 31 Aug 2026 10:23:48 -0600 Subject: [PATCH 2/5] [Air #271] fix: the architect that arrived, hung, and could not be seen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The role is not lost anywhere in the merged tree. The client running on hardware was a pre-spec-250 global install: its porch-thread-engine forwards roleContent/roleFilePath and never input.role, and its porch-driver puts no role on the thread.create payload at all. Both carry version 3.3.1, the same as the source, so nothing about the version says they are behind. The thread.created event in ~/.t3/dev records role:null, which means the decider was handed a command with no role — the server, the event payload and the projection writer all carry it and all three were read. So the deliverable for the role is the test that would have caught it: createArchitectThread against a live fork server, then codev_role read back out of the server's own projection_threads. Nothing in it writes role on a payload. Removing the forwarding reproduces role:null exactly. Two faults in this tree turned up alongside it, and neither is the role. The command hung past two minutes having already created the thread AND written the row: the thread branch returned without closeThreadBackend, so an open WebSocket kept the event loop alive. afx interrupt already carries this fix with a comment describing the same symptom. And afx status could not see the result. The Architects section was built entirely from Tower's terminal list, and a thread-backed architect has no terminal — registered and invisible, which reads from outside exactly like a command that did nothing. It now lists them from state, de-duplicated against what Tower already showed, with threadId on the --json payload as nullable-not-optional so the two backings are distinguishable. Every new assertion was confirmed to fail with its fix reverted. Co-Authored-By: Claude Opus 5 (1M context) --- codev/state/air-271_thread.md | 82 +++++++ .../issue-271-architect-role-live.e2e.test.ts | 152 +++++++++++++ .../issue-271-status-thread-architect.test.ts | 208 ++++++++++++++++++ ...-phase-9-add-architect-thread-path.test.ts | 36 +++ .../codev/src/agent-farm/commands/status.ts | 98 ++++++--- .../commands/workspace-add-architect.ts | 44 ++-- 6 files changed, 568 insertions(+), 52 deletions(-) create mode 100644 codev/state/air-271_thread.md create mode 100644 packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts create mode 100644 packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts diff --git a/codev/state/air-271_thread.md b/codev/state/air-271_thread.md new file mode 100644 index 000000000..dcd0ef71d --- /dev/null +++ b/codev/state/air-271_thread.md @@ -0,0 +1,82 @@ +# air-271 — architect created by afx lands with codev_role empty + +Issue #271. AIR, strict mode. + +## What the investigation actually found + +The role is **not** lost anywhere in the merged tree. Every layer the architect named +forwards it, and I verified the whole chain end to end against a live fork server. + +The thing running on hardware was a **stale global install**: + +``` +/Users/chris/.nvm/versions/node/v20.19.2/lib/node_modules/@cluesmith/codev + dist/agent-farm/porch-thread-engine.js forwards roleContent/roleFilePath only, never input.role +/Users/chris/.nvm/.../@cluesmith/porch-driver + dist/thread.js no role on the thread.create payload at all +``` + +Both are pre-spec-250 builds carrying version 3.3.1, the same version as the source, so +nothing about the version string says they are behind. `pnpm -w run local-install` is the +step that was missed after #266 merged. + +The decisive evidence is the event, not the projection: + +``` +sqlite3 ~/.t3/dev/state.sqlite "select payload_json from orchestration_events where event_type='thread.created'" +{"threadId":"2e2bd2c7-...","title":"architect-lan",...,"role":null,"parentThreadId":null,...} +``` + +`role: null` in the event means the decider was handed a command with no role — the client +never sent one. The server, the event payload and the projection writer are all innocent; +all three were read and all three carry it. + +## Two corrections to the report + +**The architect WAS registered.** `global.db` has it, written 24 ms after the thread: + +``` +/Users/chris/dev/codev-1455|lan|0|0||2026-08-31T15:37:09.219Z|||2e2bd2c7-...|claude|claude-haiku-4-5 +``` + +So `createArchitectThread` returned and `setArchitectByName` ran. What could not see it was +`afx status`, which built its Architects section entirely from Tower's terminal list — +and a thread-backed architect has no terminal, by definition. Registered and invisible, +which reads from outside exactly like a command that did nothing. + +**The 2-minute hang is a third fault, not the same one.** The thread branch of +`workspace-add-architect` returned without `closeThreadBackend`, so the open WebSocket kept +the event loop alive and the process never exited. `afx interrupt` already carries this fix +with a comment describing the identical symptom. + +## What shipped + +1. `issue-271-architect-role-live.e2e.test.ts` — drives `createArchitectThread` against a + `start-fork` server and reads `codev_role` out of the server's own `projection_threads`. + Nothing in it writes `role` on a payload. Verified failable: removing the forwarding in + `porch-thread-engine.ts` reproduces `role: null` exactly, the hardware symptom. +2. `closeThreadBackend` in a `finally` in `workspace-add-architect`, so the command exits. + Ordering asserted (register, then close) — closing first would exit a process whose + registration had not landed. +3. `afx status` lists thread-backed architects from state, de-duplicated against the ones + Tower already listed, with `threadId` added to the `--json` payload as + nullable-not-optional. + +All six new assertions were confirmed to fail with their fix reverted. + +## Notes for whoever runs this next + +- The fork checkout `/Users/chris/dev/t3code-codev` had an uncommitted `tools/lan-serve.mjs`, + and `t3-server.mjs start-fork` refuses a dirty checkout. I parked it, ran, and restored it + byte-identical. It is untracked work someone will want; do not delete it. A run of the live + e2e test will skip with a reason while it sits there. +- The live test needs `T3_NODE` pointing at a Node 22 binary and `T3_HARNESS_PORT`. It skips + with a stated reason otherwise; it never passes for want of a server. +- `~/.t3/dev` is the fork's `vp dev` server, not the harness's. The harness uses + `tools/t3-server/.runtime/data/userdata/state.sqlite` and starts on empty data. + +## Left out, deliberately + +Nothing in a new build can stop an OLD build from dropping a field silently — the old code +is what runs. Making a stale install detectable (a build-provenance check on `afx`) is real +and is a different issue; raised with the architect rather than grown into this one. diff --git a/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts b/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts new file mode 100644 index 000000000..63e0a935b --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts @@ -0,0 +1,152 @@ +/** + * Issue #271 — the role an architect is created with, read back out of the + * server's own projection. + * + * ## Why this test exists at all + * + * Every hierarchy assertion spec 250 shipped used SEEDED threads: a fixture + * dispatched `thread.create` with `role` on it, or wrote `codev_role` into + * `projection_threads` directly. Both prove the projector stores what it is + * handed. Neither proves that the command a human actually runs — + * `afx workspace add-architect` — hands it anything. + * + * It does not. On real hardware the thread was created and `codev_role` came back + * empty, so t3code's sidebar drew an ordinary thread and nothing could ever nest + * under it. + * + * So this test drives the PRODUCTION entry point, `createArchitectThread`, against + * a live fork server, and then reads the column out of the server's own SQLite + * file the way the issue's reproducer does. Nothing here writes `role` on a + * payload; if a layer between `createArchitectThread` and `projection_threads` + * drops it, this fails. + * + * ## Why it reads SQLite rather than an RPC + * + * The projection row is what the sidebar renders from, and it is the thing that + * was observed empty. A query that went back through the server's read path could + * be satisfied by a value the server still had in memory. The file is the end of + * the line. + * + * ## Unavailable is a SKIP, never a pass + * + * The fork server needs `T3_NODE` and a clean fork checkout. When it cannot + * start, this run has learned nothing about the fix — and "I could not tell" must + * not be spelled like "no". Each bail-out names its own reason. + */ + +import { describe, it, expect, afterAll } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { + mintPairingCredential, + startForkServer, + stopForkStack, + type ForkStackReady, +} from '../../__tests__/e2e/spec-250-fork-stack.js'; +import { closeThreadBackend, ensureThreadBackendReady } from '../thread-backend.js'; +import { createArchitectThread } from '../thread-runtime.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const repoRoot = resolve(here, '..', '..', '..', '..', '..'); + +/** + * The server's projection database. + * + * `t3-server.mjs` starts the fork with `--base-dir /data`, and the + * server puts its state under `userdata/`. Named here rather than discovered so a + * moved file fails loudly instead of matching some other database. + */ +function projectionDbPath(): string { + const runtimeDir = process.env.T3_HARNESS_DIR ?? join(repoRoot, 'tools/t3-server/.runtime'); + return resolve(runtimeDir, 'data/userdata/state.sqlite'); +} + +/** + * One column of one row, read out of the running server's file. + * + * Read-only and through `sqlite3`, so nothing here can write to a database a live + * server owns, and the WAL the server is holding open is read the same way the + * issue's reproducer read it. + */ +function projectionRole(threadId: string): { present: boolean; role: string | null } { + const out = execFileSync( + 'sqlite3', + [ + `file:${projectionDbPath()}?mode=ro`, + '-cmd', + '.timeout 5000', + // Two markers, because "no row" and "a row whose role is NULL" are + // different failures and a single string could not tell them apart. + `select 'ROW', coalesce(codev_role, 'NULL-ROLE') from projection_threads ` + + `where thread_id = '${threadId}';`, + ], + { encoding: 'utf8' }, + ).trim(); + if (out === '') return { present: false, role: null }; + const [, role] = out.split('|'); + return { present: true, role: role === 'NULL-ROLE' ? null : (role ?? null) }; +} + +/** A workspace that is a real git repository, because the backend registers one. */ +function scratchWorkspace(): string { + const ws = mkdtempSync(join(tmpdir(), 'issue-271-')); + execFileSync('git', ['init', '-q'], { cwd: ws }); + mkdirSync(join(ws, '.codev'), { recursive: true }); + writeFileSync(join(ws, '.codev', 'config.json'), '{}\n'); + return ws; +} + +let workspace: string | undefined; + +afterAll(() => { + if (workspace !== undefined) closeThreadBackend(workspace); + stopForkStack(); +}); + +describe('issue 271: an architect created by the production path carries its role', () => { + it( + 'writes codev_role="architect" into projection_threads', + { timeout: 300_000 }, + async () => { + const server = await startForkServer(); + if (!server.available) { + // A skip that says why. See the header: this is not a pass. + console.warn(`[issue-271] skipped: ${server.reason}`); + return; + } + const stack: ForkStackReady = { ...server, webUrl: '' }; + + // A FRESH single-use pairing credential, which is what the backend's + // `bootstrapToken` is in production against this server — the harness's own + // start token was already spent on `stack.accessToken`. + const bootstrapToken = await mintPairingCredential(stack); + + workspace = scratchWorkspace(); + process.env.CODEV_T3_URL = stack.serverBase; + process.env.CODEV_T3_TOKEN = bootstrapToken; + // `thread.create` requires `modelSelection`, so a workspace with no + // `threads.model` cannot create a thread at all. Production reads this from + // config; the env override is the same field by another layer. + process.env.CODEV_T3_MODEL = 'gpt-5.6-luna'; + + const installed = await ensureThreadBackendReady(workspace); + expect(installed, 'the thread backend did not install, so nothing below was exercised') + .toBe('installed'); + + const threadId = await createArchitectThread({ name: 'lan', workspaceRoot: workspace }); + expect(typeof threadId, 'createArchitectThread returned no thread id').toBe('string'); + + const row = projectionRole(threadId); + expect(row.present, `no projection row for thread ${threadId}`).toBe(true); + expect( + row.role, + 'the architect reached projection_threads with no role — issue #271. The sidebar ' + + 'renders this as an ordinary thread and nothing can nest under it.', + ).toBe('architect'); + }, + ); +}); diff --git a/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts b/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts new file mode 100644 index 000000000..bd64dc0fb --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts @@ -0,0 +1,208 @@ +/** + * Issue #271 — `afx status` can see a thread-backed architect. + * + * `afx workspace add-architect --name lan` created the thread, wrote the row, and + * printed its success line. `afx status` then listed only the terminal-backed + * `main`, which read as "the command did nothing" — the report on the issue says + * exactly that, and it was wrong: the architect was registered the whole time. + * + * The cause is that the Architects section was built entirely from Tower's + * terminal list, and a thread-backed architect has no terminal. Nothing was + * missing from state; nothing looked there. + * + * The assertions below are on the ARCHITECTS section, not on the fact that some + * line somewhere mentions the name. A run whose builder table happened to name + * `lan` would satisfy a looser check while the section stayed empty. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +const mockLoadState = vi.fn(); +const mockIsRunning = vi.fn(); +const mockGetHealth = vi.fn(); +const mockGetWorkspaceStatus = vi.fn(); +const mockLoggerInfo = vi.fn(); +const mockLog = vi.fn(); + +vi.mock('../utils/config.js', () => ({ + getConfig: vi.fn(() => ({ workspaceRoot: '/fake/workspace' })), +})); + +vi.mock('../state.js', () => ({ + loadState: (...args: any[]) => mockLoadState(...args), +})); + +vi.mock('../lib/tower-client.js', () => ({ + getTowerClient: () => ({ + isRunning: (...a: any[]) => mockIsRunning(...a), + getHealth: (...a: any[]) => mockGetHealth(...a), + getWorkspaceStatus: (...a: any[]) => mockGetWorkspaceStatus(...a), + getOverview: async () => null, + }), +})); + +vi.mock('../../lib/config.js', () => ({ + loadConfig: vi.fn(() => ({})), +})); + +vi.mock('../utils/logger.js', () => ({ + logger: { + header: vi.fn(), + success: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + info: (...args: any[]) => mockLoggerInfo(...args), + kv: vi.fn(), + blank: vi.fn(), + row: vi.fn(), + }, + fatal: vi.fn((msg: string) => { throw new Error(msg); }), +})); + +import { status } from '../commands/status.js'; + +// eslint-disable-next-line no-control-regex +const stripAnsi = (s: string) => s.replace(/\[[0-9;]*m/g, ''); + +/** + * The lines under the `Architects:` heading, up to the next heading. + * + * Scoped deliberately. Reading the whole log would let a name printed anywhere + * else stand in for a section that never rendered. + */ +function architectSection(): string[] { + const lines = mockLoggerInfo.mock.calls.map((c: any[]) => stripAnsi(String(c[0]))); + const start = lines.indexOf('Architects:'); + if (start === -1) return []; + const rest = lines.slice(start + 1); + const end = rest.findIndex((line) => !line.startsWith(' ')); + return end === -1 ? rest : rest.slice(0, end); +} + +function architectTerminal(name: string) { + return { + id: `architect:${name}`, + terminalId: `term-${name}`, + type: 'architect', + label: name, + architectName: name, + pid: 4242, + active: true, + }; +} + +describe('issue 271: afx status shows thread-backed architects', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockIsRunning.mockResolvedValue(true); + mockGetHealth.mockResolvedValue({ ok: true }); + }); + + it('lists an architect that has a thread and no terminal', async () => { + mockGetWorkspaceStatus.mockResolvedValue({ + name: 'codev-1455', + active: true, + terminals: [architectTerminal('main')], + }); + mockLoadState.mockReturnValue({ + architect: null, + architects: [ + { name: 'main', cmd: 'claude', startedAt: 'now', terminalId: 'term-main' }, + { + name: 'lan', + cmd: '', + startedAt: 'now', + threadId: '2e2bd2c7-3ae7-4582-9d60-673da525a93f', + harness: 'claude', + model: 'claude-haiku-4-5', + }, + ], + builders: [], + utils: [], + annotations: [], + }); + + await status(); + + const section = architectSection(); + expect(section.some((line) => line.includes('main'))).toBe(true); + const lan = section.find((line) => line.includes('lan')); + expect(lan, 'the thread-backed architect is absent from the Architects section').toBeDefined(); + expect(lan).toContain('thread=2e2bd2c7-3ae7-4582-9d60-673da525a93f'); + expect(lan).toContain('model=claude-haiku-4-5'); + // A thread has no pid and no port. Printing `pid=?` would report a value this + // command failed to read, when there was never one to read. + expect(lan).not.toContain('pid='); + }); + + /** + * The workspace whose ONLY architect is thread-backed. The section used to be + * nested inside a check on the terminal list, so this case printed nothing at + * all — the emptier the workspace, the more complete the silence. + */ + it('renders the section when there are no architect terminals at all', async () => { + mockGetWorkspaceStatus.mockResolvedValue({ name: 'codev-1455', active: true, terminals: [] }); + mockLoadState.mockReturnValue({ + architect: null, + architects: [{ name: 'lan', cmd: '', startedAt: 'now', threadId: 'thr-1' }], + builders: [], + utils: [], + annotations: [], + }); + + await status(); + + expect(architectSection()).toEqual([expect.stringContaining('lan')]); + }); + + /** + * An architect Tower already listed must not appear twice. Tower is the source + * for a terminal-backed one; state carries a row for it too. + */ + it('does not print an architect twice when Tower already listed it', async () => { + mockGetWorkspaceStatus.mockResolvedValue({ + name: 'codev-1455', + active: true, + terminals: [architectTerminal('main')], + }); + mockLoadState.mockReturnValue({ + architect: null, + // A row carrying BOTH is the dual-identity state the codebase forbids; it is + // used here because a duplicate can only be produced by one, and this test + // is about the de-duplication rather than about how the row got that way. + architects: [{ name: 'main', cmd: 'claude', startedAt: 'now', terminalId: 'term-main', threadId: 'thr-1' }], + builders: [], + utils: [], + annotations: [], + }); + + await status(); + + expect(architectSection().filter((line) => line.includes('main'))).toHaveLength(1); + }); + + it('carries threadId into the --json payload, null for a PTY-backed architect', async () => { + mockGetWorkspaceStatus.mockResolvedValue({ name: 'codev-1455', active: true, terminals: [] }); + mockLoadState.mockReturnValue({ + architect: null, + architects: [ + { name: 'main', cmd: 'claude', startedAt: 'now', terminalId: 'term-main' }, + { name: 'lan', cmd: '', startedAt: 'now', threadId: 'thr-1' }, + ], + builders: [], + utils: [], + annotations: [], + }); + const spy = vi.spyOn(console, 'log').mockImplementation((...args: any[]) => { mockLog(...args); }); + + await status({ json: true }); + + spy.mockRestore(); + const payload = JSON.parse(String(mockLog.mock.calls[0]?.[0])); + expect(payload.architects).toEqual([ + { name: 'main', threadId: null }, + { name: 'lan', threadId: 'thr-1' }, + ]); + }); +}); diff --git a/packages/codev/src/agent-farm/__tests__/spec-146-phase-9-add-architect-thread-path.test.ts b/packages/codev/src/agent-farm/__tests__/spec-146-phase-9-add-architect-thread-path.test.ts index b201340a7..7444002b2 100644 --- a/packages/codev/src/agent-farm/__tests__/spec-146-phase-9-add-architect-thread-path.test.ts +++ b/packages/codev/src/agent-farm/__tests__/spec-146-phase-9-add-architect-thread-path.test.ts @@ -16,6 +16,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; const ensureThreadBackendReady = vi.fn(); +const closeThreadBackend = vi.fn(); const createArchitectThread = vi.fn(); const tryGetThreadEngine = vi.fn(); const architectThreadDefaults = vi.fn(); @@ -24,6 +25,7 @@ const addArchitect = vi.fn(); vi.mock('../thread-backend.js', () => ({ ensureThreadBackendReady: (...args: unknown[]) => ensureThreadBackendReady(...args), + closeThreadBackend: (...args: unknown[]) => closeThreadBackend(...args), })); vi.mock('../thread-runtime.js', () => ({ @@ -158,6 +160,40 @@ describe('workspace add-architect — the thread path is reachable in a fresh pr expect(addArchitect).toHaveBeenCalledWith('/ws', 'uiv2'); }); + /** + * Issue #271. The command hung past two minutes on real hardware, having + * already created the thread AND written the row: an open WebSocket keeps the + * event loop alive, and nothing closed it. + * + * Asserted AFTER `setArchitectByName`, not merely "was called". Closing the + * socket before the row is written would exit a process whose registration had + * not landed, which is a worse bug than the hang it replaces. + */ + it('closes the thread backend after registering, so the process can exit', async () => { + threadEngineInstalled(); + const order: string[] = []; + setArchitectByName.mockImplementation(() => { order.push('register'); }); + closeThreadBackend.mockImplementation(() => { order.push('close'); }); + + await workspaceAddArchitect({ name: 'uiv2' }); + + expect(closeThreadBackend).toHaveBeenCalledWith('/ws'); + expect(order).toEqual(['register', 'close']); + }); + + /** + * A create that throws must still close. Otherwise the failure path is the + * hang: an error printed, and a process that never returns to print it from. + */ + it('closes the thread backend even when the create fails', async () => { + threadEngineInstalled(); + createArchitectThread.mockRejectedValue(new Error('server refused')); + + await expect(workspaceAddArchitect({ name: 'uiv2' })).rejects.toThrow('server refused'); + + expect(closeThreadBackend).toHaveBeenCalledWith('/ws'); + }); + /** * A server that was named and could not be reached must not fall through to * Tower. `ensureThreadBackendReady` throws for exactly this reason, and diff --git a/packages/codev/src/agent-farm/commands/status.ts b/packages/codev/src/agent-farm/commands/status.ts index 8222970ff..e0486bc36 100644 --- a/packages/codev/src/agent-farm/commands/status.ts +++ b/packages/codev/src/agent-farm/commands/status.ts @@ -221,7 +221,7 @@ function emitStatusJson(params: { // must still emit `"name": null` so the machine-readable contract is stable // for tooling — `JSON.stringify` would otherwise drop an `undefined` key. workspace: { path: string; name: string | null; active: boolean }; - architects: Array<{ name: string }>; + architects: Array<{ name: string; threadId?: string | null }>; builders: Builder[]; ownerFilter: string | undefined; // Issue #1227: null (not omitted) when Tower is down or the running Tower @@ -245,7 +245,11 @@ function emitStatusJson(params: { orphans, ptyDrain, ownerFilter: ownerFilter ?? null, - architects: architects.map((a) => ({ name: a.name ?? 'main' })), + // Issue #271: `threadId` is nullable-not-optional, the same contract as + // `workspace.name` above — a PTY-backed architect emits `"threadId": null` + // rather than dropping the key, so tooling can tell the two backings apart + // instead of inferring one from a missing field. + architects: architects.map((a) => ({ name: a.name ?? 'main', threadId: a.threadId ?? null })), builders: visible.map((b) => ({ id: b.id, name: b.name, @@ -374,42 +378,64 @@ export async function status(options: StatusOptions = {}): Promise { logger.kv('PTY drain', ptyDrain === 0 ? chalk.gray('0') : String(ptyDrain)); renderOrphans(orphans, sized); - if (workspaceStatus.terminals.length > 0) { - // Spec 786 Phase 5: enumerate architects explicitly first, so users see - // ALL registered architects (not just one collapsed "Architect" row). - // Each architect entry's `architectName`, `pid`, and optional `port` - // come from the Tower API (per Spec 786 Phase 5's TowerWorkspaceStatus - // extension). Spec 1057: builders move to their own owner-aware section - // below; shells/dev remain in the general Terminals list. - const architectTerminals = workspaceStatus.terminals.filter(t => t.type === 'architect'); - const otherTerminals = workspaceStatus.terminals.filter( - t => t.type !== 'architect' && t.type !== 'builder', - ); - - if (architectTerminals.length > 0) { - logger.blank(); - logger.info('Architects:'); - for (const term of architectTerminals) { - const name = term.architectName || term.label; - const pid = term.pid ? `pid=${term.pid}` : 'pid=?'; - const port = term.port ? ` port=${term.port}` : ''; - // Spec 786 Phase 5: prefer `terminalId` (the actual PtySession id) - // over `id` (the Spec 761 tab identifier, e.g. `architect` or - // `architect:`). Falls back to `id` for older Tower versions - // that haven't shipped the Phase 5 extension yet. - const termIdValue = term.terminalId ?? term.id; - const termId = ` terminal=${termIdValue}`; - logger.info(` ${chalk.cyan(name)} (${pid}${port}${termId})`); - } + // Spec 786 Phase 5: enumerate architects explicitly first, so users see + // ALL registered architects (not just one collapsed "Architect" row). + // Each architect entry's `architectName`, `pid`, and optional `port` + // come from the Tower API (per Spec 786 Phase 5's TowerWorkspaceStatus + // extension). Spec 1057: builders move to their own owner-aware section + // below; shells/dev remain in the general Terminals list. + const architectTerminals = workspaceStatus.terminals.filter(t => t.type === 'architect'); + const otherTerminals = workspaceStatus.terminals.filter( + t => t.type !== 'architect' && t.type !== 'builder', + ); + + // Issue #271. A thread-backed architect has NO Tower terminal — that is + // what being thread-backed means — so a section built only from the + // terminal list could never show one. `afx workspace add-architect` wrote + // its row, `add-architect` printed its success line, and `afx status` then + // reported only the terminal-backed `main`: registered and invisible, which + // reads exactly like a command that did nothing. + // + // Sourced from state, the same place the row was written, and matched on + // NAME rather than on the absence of a terminalId: a name Tower already + // listed above must not print twice. + const shownNames = new Set( + architectTerminals.map(t => (t.architectName || t.label || '').toLowerCase()), + ); + const threadArchitects = architects.filter( + (a) => a.threadId !== undefined && !shownNames.has((a.name ?? 'main').toLowerCase()), + ); + + if (architectTerminals.length > 0 || threadArchitects.length > 0) { + logger.blank(); + logger.info('Architects:'); + for (const term of architectTerminals) { + const name = term.architectName || term.label; + const pid = term.pid ? `pid=${term.pid}` : 'pid=?'; + const port = term.port ? ` port=${term.port}` : ''; + // Spec 786 Phase 5: prefer `terminalId` (the actual PtySession id) + // over `id` (the Spec 761 tab identifier, e.g. `architect` or + // `architect:`). Falls back to `id` for older Tower versions + // that haven't shipped the Phase 5 extension yet. + const termIdValue = term.terminalId ?? term.id; + const termId = ` terminal=${termIdValue}`; + logger.info(` ${chalk.cyan(name)} (${pid}${port}${termId})`); } + for (const arch of threadArchitects) { + // No pid and no port, and those are not printed as unknowns: a thread + // has neither, so `pid=?` would report a value that could not exist as + // one this command failed to read. + const model = arch.model ? ` model=${arch.model}` : ''; + logger.info(` ${chalk.cyan(arch.name ?? 'main')} (thread=${arch.threadId}${model})`); + } + } - if (otherTerminals.length > 0) { - logger.blank(); - logger.info('Terminals:'); - for (const term of otherTerminals) { - const typeColor = term.type === 'dev' ? chalk.green : chalk.gray; - logger.info(` ${typeColor(term.type)} - ${term.label} (${term.active ? 'active' : 'stopped'})`); - } + if (otherTerminals.length > 0) { + logger.blank(); + logger.info('Terminals:'); + for (const term of otherTerminals) { + const typeColor = term.type === 'dev' ? chalk.green : chalk.gray; + logger.info(` ${typeColor(term.type)} - ${term.label} (${term.active ? 'active' : 'stopped'})`); } } diff --git a/packages/codev/src/agent-farm/commands/workspace-add-architect.ts b/packages/codev/src/agent-farm/commands/workspace-add-architect.ts index d05aa6b4c..30ed3fdfd 100644 --- a/packages/codev/src/agent-farm/commands/workspace-add-architect.ts +++ b/packages/codev/src/agent-farm/commands/workspace-add-architect.ts @@ -22,7 +22,7 @@ import { } from '../utils/architect-name.js'; import { getArchitects, setArchitectByName } from '../state.js'; import { architectThreadDefaults, createArchitectThread, tryGetThreadEngine } from '../thread-runtime.js'; -import { ensureThreadBackendReady } from '../thread-backend.js'; +import { closeThreadBackend, ensureThreadBackendReady } from '../thread-backend.js'; export interface WorkspaceAddArchitectOptions { name?: string; @@ -95,21 +95,33 @@ export async function workspaceAddArchitect( // Read BEFORE the create, so the pair recorded is the one this create resolves — // not a re-read of configuration that a concurrent edit could have moved. const defaults = architectThreadDefaults(workspacePath); - const threadId = await createArchitectThread({ name, workspaceRoot: workspacePath }); - setArchitectByName(workspacePath, name, { - name, - cmd: '', - startedAt: new Date().toISOString(), - threadId, - // Issue #227 item 3: the pair this thread was created with, pinned on the row the - // way a builder's is. Without it a later `attach` — which is where Tower resumes - // this thread — carries no harness or model and falls back to whatever - // `.codev/config.json` says at THAT moment, so editing `threads.model` between a - // spawn and a delivery silently moved a live architect onto a different model. - harness: defaults?.harness, - model: defaults?.model, - }); - logger.success(`Started architect '${name}' (thread ${threadId}).`); + try { + const threadId = await createArchitectThread({ name, workspaceRoot: workspacePath }); + setArchitectByName(workspacePath, name, { + name, + cmd: '', + startedAt: new Date().toISOString(), + threadId, + // Issue #227 item 3: the pair this thread was created with, pinned on the row the + // way a builder's is. Without it a later `attach` — which is where Tower resumes + // this thread — carries no harness or model and falls back to whatever + // `.codev/config.json` says at THAT moment, so editing `threads.model` between a + // spawn and a delivery silently moved a live architect onto a different model. + harness: defaults?.harness, + model: defaults?.model, + }); + logger.success(`Started architect '${name}' (thread ${threadId}).`); + } finally { + // Issue #271. An open WebSocket keeps the event loop alive, and this command is + // expected to exit. Without it the live run printed nothing and hung past two + // minutes until the caller killed it — the architect having already been created + // AND registered. Working, and hung, which reads from outside exactly like a + // command that failed. + // + // The same fix `afx interrupt` carries, for the same reason. Every one-shot + // command that reaches a thread owes it. + closeThreadBackend(workspacePath); + } return; } From c5bec635a69f4fd6436e9a2271adf5498629ac4a Mon Sep 17 00:00:00 2001 From: pseudo Date: Mon, 31 Aug 2026 10:24:29 -0600 Subject: [PATCH 3/5] chore(porch): 271 record PR #274 --- .../271-safety-adjacent-an-architect-c/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/271-safety-adjacent-an-architect-c/status.yaml b/codev/projects/271-safety-adjacent-an-architect-c/status.yaml index 101fbb403..4d5835074 100644 --- a/codev/projects/271-safety-adjacent-an-architect-c/status.yaml +++ b/codev/projects/271-safety-adjacent-an-architect-c/status.yaml @@ -11,4 +11,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-31T15:42:55.843Z' -updated_at: '2026-08-31T15:42:55.844Z' +updated_at: '2026-08-31T16:24:29.279Z' +pr_history: + - phase: implement + pr_number: 274 + branch: builder/air-271 + created_at: '2026-08-31T16:24:29.278Z' From 43007dd052b35e59b5c227891d5a199ad89f8025 Mon Sep 17 00:00:00 2001 From: pseudo Date: Mon, 31 Aug 2026 10:45:53 -0600 Subject: [PATCH 4/5] =?UTF-8?q?[Air=20#271]=20fix:=20act=20on=20the=20inte?= =?UTF-8?q?gration=20review=20=E2=80=94=20a=20guard=20that=20runs,=20and?= =?UTF-8?q?=20one=20spelling=20per=20fact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The role guard lived only in a suite that skips. `issue-271-architect-role-live` needs T3_NODE and a clean fork checkout, so CI never ran it and neither does this machine by default — and a test that cannot run reads exactly like a test that passed, which is the failure this issue is about wearing a second costume. So the seam is pinned by a unit test as well: a fake engine registered the way production registers one, asserting `createArchitectThread` hands `role: 'architect'` to `engine.create`. Neither test substitutes for the other. The live one cannot say WHERE a lost role was lost; this one says nothing about whether the server stores it. Three things the review raised and left to me, all taken: The live test restores CODEV_T3_URL / CODEV_T3_TOKEN / CODEV_T3_MODEL and removes its scratch workspace. Those variables are read for every workspace, so leaving them set pointed the rest of the run at a server this test had already stopped — harmless only because it happened to run last, which is a property of the schedule and not a guarantee. The projection query refuses a thread id that is not a UUID rather than escaping it. `sqlite3` takes one SQL string and no bound parameters, so the id is interpolated either way; what changes is that the shape someone copies now rejects the input that would matter. And the Tower-down fallback names the thread. It printed `cmd= started=` for a thread-backed architect — `cmd` is empty because there is no process — so the two render paths disagreed about the same row: one named the thread, the other named nothing. Co-Authored-By: Claude Opus 5 (1M context) --- .../issue-271-architect-role-live.e2e.test.ts | 32 ++++++- .../issue-271-architect-role-seam.test.ts | 88 +++++++++++++++++++ .../issue-271-status-thread-architect.test.ts | 30 +++++++ .../codev/src/agent-farm/commands/status.ts | 10 ++- 4 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 packages/codev/src/agent-farm/__tests__/issue-271-architect-role-seam.test.ts diff --git a/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts b/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts index 63e0a935b..699232d3e 100644 --- a/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts +++ b/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-live.e2e.test.ts @@ -36,7 +36,7 @@ import { describe, it, expect, afterAll } from 'vitest'; import { execFileSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -73,6 +73,14 @@ function projectionDbPath(): string { * issue's reproducer read it. */ function projectionRole(threadId: string): { present: boolean; role: string | null } { + // Refused rather than escaped. `sqlite3` takes one SQL string and no bound + // parameters, so the id is interpolated — and an interpolation that quietly + // accepts anything is the shape someone copies into a query where the value is + // not a server-minted UUID. A thread id that is not one is a bug in the caller, + // and it stops here. + if (!/^[0-9a-fA-F-]{36}$/.test(threadId)) { + throw new Error(`refusing to query for a thread id that is not a UUID: ${JSON.stringify(threadId)}`); + } const out = execFileSync( 'sqlite3', [ @@ -102,8 +110,28 @@ function scratchWorkspace(): string { let workspace: string | undefined; +/** + * The env this test sets, captured so it can be put back. + * + * `CODEV_T3_URL` and its siblings are read by `readThreadBackendConfig` for EVERY + * workspace, so leaving them set points the rest of the run at a server this test + * has already stopped. The suite is sequential and this file happened to run + * last; that is a property of the schedule, not a guarantee. + */ +const CODEV_T3_KEYS = ['CODEV_T3_URL', 'CODEV_T3_TOKEN', 'CODEV_T3_MODEL'] as const; +const savedEnv = new Map( + CODEV_T3_KEYS.map((key) => [key, process.env[key]]), +); + afterAll(() => { - if (workspace !== undefined) closeThreadBackend(workspace); + if (workspace !== undefined) { + closeThreadBackend(workspace); + rmSync(workspace, { recursive: true, force: true }); + } + for (const [key, value] of savedEnv) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } stopForkStack(); }); diff --git a/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-seam.test.ts b/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-seam.test.ts new file mode 100644 index 000000000..064ce3ef6 --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/issue-271-architect-role-seam.test.ts @@ -0,0 +1,88 @@ +/** + * Issue #271 — `createArchitectThread` names the role at the seam. + * + * The live test in `issue-271-architect-role-live.e2e.test.ts` proves the whole + * chain, and it skips without `T3_NODE` and a clean fork checkout — so CI never + * runs it. A guard that cannot run reads exactly like a guard that passed, which + * is the shape of the failure this issue is about, so the same regression is + * pinned here as well by a test that runs everywhere. + * + * This is the ONE seam neither test can substitute for the other on. The live + * test cannot say WHERE a lost role was lost; this one says nothing about + * whether the server stores it. Both, or neither is enough. + * + * The engine is a fake registered through `setThreadEngine` — the same + * registration production uses — rather than a hand-built call to `engine.create`. + * A test that called the engine itself would be asserting its own argument. + */ + +import { describe, it, expect, afterEach } from 'vitest'; + +import { + clearThreadEngines, + createArchitectThread, + setThreadEngine, + type ThreadEngine, +} from '../thread-runtime.js'; + +type CreateInput = Parameters[0]; + +function recordingEngine(): { engine: ThreadEngine; creates: CreateInput[] } { + const creates: CreateInput[] = []; + const engine = { + defaults: { harness: 'claude', model: 'claude-opus-5' }, + async create(input: CreateInput) { + creates.push(input); + return 'thr-architect-1'; + }, + async attach() { throw new Error('attach is not part of this seam'); }, + async startTurn() { throw new Error('startTurn is not part of this seam'); }, + async recoverTurn() { throw new Error('recoverTurn is not part of this seam'); }, + async interrupt() { throw new Error('interrupt is not part of this seam'); }, + } as unknown as ThreadEngine; + return { engine, creates }; +} + +afterEach(() => { + clearThreadEngines(); +}); + +describe('issue 271: createArchitectThread hands the engine a role', () => { + it("passes role: 'architect' through to engine.create", async () => { + const { engine, creates } = recordingEngine(); + setThreadEngine(engine, '/ws'); + + const threadId = await createArchitectThread({ name: 'lan', workspaceRoot: '/ws' }); + + expect(threadId).toBe('thr-architect-1'); + expect(creates).toHaveLength(1); + // The assertion the issue is about. Without it the thread is created, the row + // is written, the command succeeds — and t3code's sidebar draws an ordinary + // thread that nothing can nest under. + expect(creates[0]?.role).toBe('architect'); + }); + + /** + * `parentThreadId` stays ABSENT, not null. An architect has no parent, and the + * server refuses a parent on a non-builder — so a null here would be a value + * the create path cannot carry, not a tidier way to say "none". + */ + it('names no parent for an architect', async () => { + const { engine, creates } = recordingEngine(); + setThreadEngine(engine, '/ws'); + + await createArchitectThread({ name: 'lan', workspaceRoot: '/ws' }); + + expect(creates[0]).not.toHaveProperty('parentThreadId'); + }); + + it('creates the thread at the workspace root, which is what makes it an architect', async () => { + const { engine, creates } = recordingEngine(); + setThreadEngine(engine, '/ws'); + + await createArchitectThread({ name: 'lan', workspaceRoot: '/ws' }); + + expect(creates[0]?.worktreePath).toBe('/ws'); + expect(creates[0]?.builderId).toBe('architect-lan'); + }); +}); diff --git a/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts b/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts index bd64dc0fb..5a9e58f5f 100644 --- a/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts +++ b/packages/codev/src/agent-farm/__tests__/issue-271-status-thread-architect.test.ts @@ -182,6 +182,36 @@ describe('issue 271: afx status shows thread-backed architects', () => { expect(architectSection().filter((line) => line.includes('main'))).toHaveLength(1); }); + /** + * The Tower-down fallback renders the same rows through different code. It + * printed `cmd= started=…` for a thread-backed architect — `cmd` is empty + * because there is no process — so the two paths disagreed about the same row: + * one named the thread, the other named nothing. + */ + it('names the thread in the Tower-down fallback too, instead of an empty cmd', async () => { + mockIsRunning.mockResolvedValue(false); + mockLoadState.mockReturnValue({ + architect: null, + architects: [ + { name: 'main', cmd: 'claude', startedAt: 'T0', terminalId: 'term-main' }, + { name: 'lan', cmd: '', startedAt: 'T0', threadId: 'thr-1' }, + ], + builders: [], + utils: [], + annotations: [], + }); + + await status(); + + const lines = mockLoggerInfo.mock.calls.map((c: any[]) => stripAnsi(String(c[0]))); + const lan = lines.find((line) => line.includes('lan:')); + expect(lan, 'the thread-backed architect is absent from the Tower-down listing').toBeDefined(); + expect(lan).toContain('thread=thr-1'); + expect(lan).not.toContain('cmd='); + // The PTY-backed row is unchanged: it has a command, and that is what it says. + expect(lines.find((line) => line.includes('main:'))).toContain('cmd=claude'); + }); + it('carries threadId into the --json payload, null for a PTY-backed architect', async () => { mockGetWorkspaceStatus.mockResolvedValue({ name: 'codev-1455', active: true, terminals: [] }); mockLoadState.mockReturnValue({ diff --git a/packages/codev/src/agent-farm/commands/status.ts b/packages/codev/src/agent-farm/commands/status.ts index e0486bc36..65abc2d9a 100644 --- a/packages/codev/src/agent-farm/commands/status.ts +++ b/packages/codev/src/agent-farm/commands/status.ts @@ -482,7 +482,15 @@ export async function status(options: StatusOptions = {}): Promise { logger.kv('Architects', chalk.green(`${architects.length} registered`)); logger.info(` (Tower not running — PID/port not available)`); for (const a of architects) { - logger.info(` ${chalk.cyan(a.name ?? 'main')}: cmd=${a.cmd} started=${a.startedAt}`); + // Issue #271. A thread-backed architect has an empty `cmd` — there is no + // process, which is the point — so this line rendered as `cmd= started=…` + // and named nothing that identified the thread. The two render paths then + // disagreed about the same row: Tower-up showed a thread id, Tower-down + // showed a blank. Same fact, one spelling. + const backing = a.threadId !== undefined + ? `thread=${a.threadId}` + : `cmd=${a.cmd}`; + logger.info(` ${chalk.cyan(a.name ?? 'main')}: ${backing} started=${a.startedAt}`); } } else { logger.kv('Architects', chalk.gray('none registered')); From 50c8c7afabbeb92d04293a4dd88f6e878241aac8 Mon Sep 17 00:00:00 2001 From: pseudo Date: Mon, 31 Aug 2026 10:51:31 -0600 Subject: [PATCH 5/5] chore(porch): 271 pr phase-transition --- codev/projects/271-safety-adjacent-an-architect-c/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/271-safety-adjacent-an-architect-c/status.yaml b/codev/projects/271-safety-adjacent-an-architect-c/status.yaml index 4d5835074..30baab20f 100644 --- a/codev/projects/271-safety-adjacent-an-architect-c/status.yaml +++ b/codev/projects/271-safety-adjacent-an-architect-c/status.yaml @@ -1,7 +1,7 @@ id: '271' title: safety-adjacent-an-architect-c protocol: air -phase: implement +phase: pr plan_phases: [] current_plan_phase: null gates: @@ -11,7 +11,7 @@ iteration: 1 build_complete: false history: [] started_at: '2026-08-31T15:42:55.843Z' -updated_at: '2026-08-31T16:24:29.279Z' +updated_at: '2026-08-31T16:51:31.821Z' pr_history: - phase: implement pr_number: 274