[Spec 1273] Harden reset project resolution: retroactive codex review findings

packages/codev/src/agent-farm/__tests__/spec-1273-reset-context.test.ts

@@ -19,6 +19,7 @@ import {
   resolveBuilderContext,
   readPorchContext,
   protocolFromStatus,
+  issueNumberFromPorchId,
   modeFromBuilderPrompt,
   harnessFromLaunchScript,
   ContextResolutionError,
@@ -627,6 +628,7 @@ describe('mode resolution for the ad-hoc task lane (Spec 1273 F2)', () => {
       builderId: 'builder-task-re_v',
       worktree: WORKTREE,
       branch: 'builder/task-RE_V',
+      taskText: 'Be a probe.',
     });
 
     expect(ctx.mode).toBe('soft');
@@ -657,6 +659,7 @@ describe('mode resolution for the ad-hoc task lane (Spec 1273 F2)', () => {
       builderId: 'builder-task-re_v',
       worktree: WORKTREE,
       branch: 'builder/task-RE_V',
+      taskText: 'Be a probe.',
       modeOverride: 'strict',
     });
     expect(ctx.mode).toBe('strict');
@@ -812,3 +815,117 @@ describe('a bare project NUMBER is not enough to claim a builder (Spec 1273 veri
     expect(() => readPorchContext(fs, WORKTREE, { builderId: BUILDER_ID })).toThrow(/Ambiguous/);
   });
 });
+
+// ============================================================================
+// Retroactive codex review of the merged #1308 (2026-07-31)
+// ============================================================================
+
+describe('post-merge codex findings (Spec 1273)', () => {
+  it('a status.yaml that states a DIFFERENT id is not overruled by its directory name', () => {
+    // `codev/projects/1273-old/` holding `id: '999'` belongs to 999, whatever
+    // the directory is called. The dir-name fallback previously overruled that,
+    // letting a renamed or recycled directory claim a builder — and
+    // manufacturing false ambiguities beside the real project.
+    const fs = makeFs(
+      {
+        [join(WORKTREE, 'codev', 'projects', '1273-old', 'status.yaml')]:
+          "id: '999'\nprotocol: aspir\nphase: implement\n",
+        [join(WORKTREE, '.builder-prompt.txt')]: BUILDER_PROMPT,
+        [join(WORKTREE, '.builder-start.sh')]: LAUNCH_SCRIPT,
+      },
+      { [join(WORKTREE, 'codev', 'projects')]: ['1273-old'] },
+    );
+
+    expect(readPorchContext(fs, WORKTREE, { builderId: BUILDER_ID })).toBeNull();
+  });
+
+  it('still falls back to the directory name when the file states no id at all', () => {
+    const fs = makeFs(
+      {
+        [join(WORKTREE, 'codev', 'projects', '1273-x', 'status.yaml')]:
+          'protocol: aspir\nphase: implement\n',
+        [join(WORKTREE, '.builder-prompt.txt')]: BUILDER_PROMPT,
+        [join(WORKTREE, '.builder-start.sh')]: LAUNCH_SCRIPT,
+      },
+      { [join(WORKTREE, 'codev', 'projects')]: ['1273-x'] },
+    );
+
+    expect(readPorchContext(fs, WORKTREE, { builderId: BUILDER_ID })?.projectName).toBe('1273-x');
+  });
+
+  it('rejects a weak claim when the builder id yields no protocol to corroborate with', () => {
+    // The previous code's own comment said a noncanonical id "cannot be
+    // corroborated and is not trusted" while `if (expectedProtocol && mismatch)`
+    // let EVERY weak claim through when expectedProtocol was null. A legacy
+    // builder could adopt any historical project sharing its tail.
+    const fs = makeFs(
+      {
+        [join(WORKTREE, 'codev', 'projects', '1273-x', 'status.yaml')]: STATUS_YAML,
+        [join(WORKTREE, '.builder-prompt.txt')]: BUILDER_PROMPT,
+        [join(WORKTREE, '.builder-start.sh')]: LAUNCH_SCRIPT,
+      },
+      { [join(WORKTREE, 'codev', 'projects')]: ['1273-x'] },
+    );
+
+    expect(readPorchContext(fs, WORKTREE, { builderId: 'some-legacy-name-1273' })).toBeNull();
+  });
+
+  it('recovers a BUGFIX issue number from its <prefix>-<N> porch id', () => {
+    // BUGFIX deliberately stores `bugfix-<N>` (spawn.ts:817). The strict /^\d+$/
+    // guard threw that identity away when the registry row had none — and on
+    // BUGFIX the issue body IS the spec, so the re-orientation lost the
+    // requirements it exists to carry.
+    expect(issueNumberFromPorchId('bugfix-887')).toBe('887');
+    expect(issueNumberFromPorchId('1273')).toBe('1273');
+    // But an ad-hoc task id still cannot masquerade as an issue.
+    expect(issueNumberFromPorchId('builder-task-abc')).toBeUndefined();
+    expect(issueNumberFromPorchId(undefined)).toBeUndefined();
+  });
+
+  it('does not treat a --task --protocol builder as bare when porch init failed', () => {
+    // initPorchInWorktree is deliberately non-fatal, so that lane can have task
+    // text AND no porch. Inferring "bare" from that stripped its real protocol
+    // template. The prompt is written BEFORE porch init, so its rendered
+    // template — and its `## Mode:` line — is the positive evidence.
+    const fs = makeFs(
+      {
+        [join(WORKTREE, '.builder-prompt.txt')]: BUILDER_PROMPT, // rendered template, has Mode
+        [join(WORKTREE, '.builder-start.sh')]: LAUNCH_SCRIPT,
+      },
+      { [join(WORKTREE, 'codev', 'projects')]: [] },
+    );
+
+    const ctx = resolveBuilderContext({
+      fs,
+      builderId: 'builder-task-abc',
+      worktree: WORKTREE,
+      branch: 'builder/task-abc',
+      taskText: 'ad-hoc work',
+    });
+
+    expect(ctx.isBareTask).toBe(false);
+    // And its real mode survives, rather than being defaulted to soft.
+    expect(ctx.mode).toBe('strict');
+    expect(ctx.modeSource).toBe('builder-prompt');
+  });
+
+  it('is bare only with task text AND no rendered template', () => {
+    const bare = makeFs(
+      {
+        [join(WORKTREE, '.builder-prompt.txt')]: 'You are a Builder.\n\n# Task\n\nBe a probe.',
+        [join(WORKTREE, '.builder-start.sh')]: LAUNCH_SCRIPT,
+      },
+      { [join(WORKTREE, 'codev', 'projects')]: [] },
+    );
+
+    const ctx = resolveBuilderContext({
+      fs: bare,
+      builderId: 'builder-task-re_v',
+      worktree: WORKTREE,
+      branch: 'builder/task-RE_V',
+      taskText: 'Be a probe.',
+    });
+
+    expect(ctx.isBareTask).toBe(true);
+  });
+});

packages/codev/src/agent-farm/__tests__/spec-1273-reset-reorient.test.ts

@@ -63,6 +63,7 @@ function makeContext(overrides: Partial<ResolvedBuilderContext> = {}): ResolvedB
     specPath: 'codev/specs/1273-builder-context-reset-should-b.md',
     planPath: 'codev/plans/1273-builder-context-reset-should-b.md',
     issueNumber: '1273',
+    isBareTask: false,
     ...overrides,
   };
 }
@@ -466,6 +467,7 @@ describe('long-form re-orientation (Spec 1273)', () => {
         planPath: null,
         issueNumber: undefined,
         taskText: 'You are a reset-test probe.',
+        isBareTask: true,
       }),
       statePath: STATE_PATH,
       buildSpawnPrompt: port,

packages/codev/src/agent-farm/commands/reset/context.ts

@@ -87,6 +87,21 @@ export interface ResolvedBuilderContext {
    * this field the task lane is indistinguishable from an issue-driven one.
    */
   taskText?: string;
+  /**
+   * True only for a BARE `afx spawn --task` builder (no `--protocol`).
+   *
+   * Established from positive evidence — task text present AND the prompt file
+   * carries no `## Mode:` heading — rather than inferred from the absence of a
+   * porch project. `initPorchInWorktree` is deliberately non-fatal, so a
+   * `--task --protocol X` builder whose porch init failed also has task text and
+   * no porch, and inferring from that would strip its real protocol template and
+   * hand it the raw task text instead.
+   *
+   * The prompt is built BEFORE porch init (`spawn.ts:545-548`), so a
+   * `--task --protocol` builder's prompt carries the rendered template — and its
+   * `## Mode:` line — regardless of whether porch init later succeeded.
+   */
+  isBareTask: boolean;
 }
 
 export interface PorchContext {
@@ -219,8 +234,16 @@ function claimStrength(
   if (statusId && normalizeId(statusId) === rawId && !/^\d+$/.test(rawId)) return 'strong';
 
   const wanted = candidateProjectIds(identity).map(normalizeId);
-  if (statusId && wanted.includes(normalizeId(statusId))) return 'weak';
 
+  // A status.yaml that STATES its id is authoritative about what it is. If that
+  // id does not claim this builder, the directory name cannot overrule it —
+  // `codev/projects/1273-old/` holding `id: '999'` belongs to 999, whatever the
+  // directory is called. Falling through to the name here let a renamed or
+  // recycled directory claim a builder, and manufactured false ambiguities
+  // alongside the real project.
+  if (statusId) return wanted.includes(normalizeId(statusId)) ? 'weak' : 'none';
+
+  // Directory-name fallback applies ONLY when the file states no id at all.
   const dirNorm = normalizeId(dir);
   return wanted.some(c => dirNorm === c || dirNorm.startsWith(`${c}-`)) ? 'weak' : 'none';
 }
@@ -273,7 +296,15 @@ export function readPorchContext(
     // A weak match must agree with the protocol the builder id declares.
     // Without this, `builder-bugfix-799` adopts the PIR project that happens to
     // share the number — wrong protocol, wrong porch id, and silently so.
-    if (expectedProtocol && protocol.toLowerCase() !== expectedProtocol.toLowerCase()) continue;
+    //
+    // And when the id is not in canonical form there IS no protocol to
+    // corroborate against, so the claim cannot be trusted at all. The previous
+    // version's comment said exactly that while the code did the opposite —
+    // `if (expectedProtocol && mismatch) continue` let every weak claim through
+    // whenever `expectedProtocol` was null. A legacy or noncanonical builder
+    // could adopt any historical project sharing its tail.
+    if (!expectedProtocol) continue;
+    if (protocol.toLowerCase() !== expectedProtocol.toLowerCase()) continue;
     weak.push(ctx);
   }
 
@@ -338,6 +369,25 @@ export function modeFromBuilderPrompt(fs: ContextFsPort, worktree: string): 'str
   return m[1].toLowerCase() as 'strict' | 'soft';
 }
 
+/**
+ * Recover an issue number from a porch project id.
+ *
+ * Porch ids are not uniformly numeric. PIR and SPIR use the bare issue number;
+ * **BUGFIX deliberately uses `<prefix>-<N>`** (`spawn.ts:817`, "historical, kept
+ * untouched"). A strict `/^\d+$/` guard therefore threw away BUGFIX's issue
+ * identity whenever the registry row lacked one — and on BUGFIX the issue body
+ * IS the spec, so the re-orientation lost the requirements it was meant to carry.
+ *
+ * Accepts a bare number or a canonical `<word>-<number>`; rejects anything else,
+ * so an ad-hoc task id (`builder-task-abc`) still cannot masquerade as an issue.
+ */
+export function issueNumberFromPorchId(projectId?: string): string | undefined {
+  if (!projectId) return undefined;
+  if (/^\d+$/.test(projectId)) return projectId;
+  const m = projectId.match(/^[a-z]+-(\d+)$/i);
+  return m ? m[1] : undefined;
+}
+
 // ============================================================================
 // Harness
 // ============================================================================
@@ -508,13 +558,20 @@ export function resolveBuilderContext(options: ResolveContextOptions): ResolvedB
   }
 
   // --- Mode: flag → .builder-prompt.txt → abort ---------------------------
+  const promptMode = modeFromBuilderPrompt(fs, worktree);
+
+  // Positive evidence, not a negative inference: a bare `--task` spawn writes a
+  // prompt with no `## Mode:` heading, while `--task --protocol X` renders the
+  // full template (built BEFORE porch init, so it survives a failed init).
+  const isBareTask = Boolean(taskText) && promptMode === null;
+
   let mode = modeOverride ?? null;
   let modeSource: ResolvedBuilderContext['modeSource'] = 'flag';
   if (!mode) {
-    mode = modeFromBuilderPrompt(fs, worktree);
+    mode = promptMode;
     modeSource = 'builder-prompt';
   }
-  if (!mode && !porch) {
+  if (!mode && isBareTask) {
     // A `--task` spawn writes a bare prompt with no `## Mode:` heading, so this
     // lane could never auto-detect and every `afx reset <task>` hard-errored.
     //
@@ -531,8 +588,8 @@ export function resolveBuilderContext(options: ResolveContextOptions): ResolvedB
   if (!mode) {
     throw new ContextResolutionError(
       `Cannot determine the mode (strict/soft) for '${builderId}': no '## Mode:' line in ` +
-        `${join(worktree, '.builder-prompt.txt')}, and this builder HAS a porch project ` +
-        `(${porch?.projectName}), so it is not the ad-hoc-task lane that defaults to soft. ` +
+        `${join(worktree, '.builder-prompt.txt')}, and this builder is not the bare ad-hoc-task ` +
+        `lane that defaults to soft (no task text, or a rendered protocol template). ` +
         `Mode is not persisted anywhere else — pass --mode strict or --mode soft explicitly.`,
     );
   }
@@ -583,8 +640,8 @@ export function resolveBuilderContext(options: ResolveContextOptions): ResolvedB
     // and an unfollowable `gh issue view builder-task-abc` in the
     // re-orientation. A fabricated issue reference is worse than none: it sends
     // a freshly-reset builder to look up requirements that do not exist.
-    issueNumber:
-      issueNumber ?? (porch && /^\d+$/.test(porch.projectId) ? porch.projectId : undefined),
+    issueNumber: issueNumber ?? issueNumberFromPorchId(porch?.projectId),
     taskText,
+    isBareTask,
   };
 }

packages/codev/src/agent-farm/commands/reset/reorient.ts

@@ -322,11 +322,12 @@ function buildLongForm(options: AssembleOptions): string {
   // render, so the live e2e died on
   // `Protocol "task" has no builder-prompt.md`.
   //
-  // Identified by a POSITIVE fact rather than by catching that error: task text
-  // present and no porch project. The `--task --protocol X` variant does get a
-  // porch project (`spawn.ts:548`), so this cannot swallow it — that lane still
-  // renders its real template below.
-  if (c.taskText && !c.porch) {
+  // Identified by a POSITIVE fact rather than by catching that error, and
+  // rather than by `taskText && !porch`: `initPorchInWorktree` is non-fatal, so
+  // a `--task --protocol X` builder whose porch init FAILED also has task text
+  // and no porch. `isBareTask` is established from the prompt file itself (see
+  // context.ts), which carries the rendered template for that lane regardless.
+  if (c.isBareTask) {
     const longFormTask = [
       'You are a Builder. Read codev/roles/builder.md for your full role definition.',
       '',