Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions codev/projects/271-safety-adjacent-an-architect-c/status.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
id: '271'
title: safety-adjacent-an-architect-c
protocol: air
phase: pr
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-31T16:51:31.821Z'
pr_history:
- phase: implement
pr_number: 274
branch: builder/air-271
created_at: '2026-08-31T16:24:29.278Z'
82 changes: 82 additions & 0 deletions codev/state/air-271_thread.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/**
* 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, rmSync, 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 <runtime>/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 } {
// 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',
[
`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;

/**
* 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<string, string | undefined>(
CODEV_T3_KEYS.map((key) => [key, process.env[key]]),
);

afterAll(() => {
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();
});

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');
},
);
});
Original file line number Diff line number Diff line change
@@ -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<ThreadEngine['create']>[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');
});
});
Loading
Loading