Skip to content

[Spec 1273][Verify] Fix afx reset: own-project resolution + task lane - #1308

Merged
waleedkadous merged 7 commits into
mainfrom
builder/1273-verify-fixes
Jul 31, 2026
Merged

[Spec 1273][Verify] Fix afx reset: own-project resolution + task lane#1308
waleedkadous merged 7 commits into
mainfrom
builder/1273-verify-fixes

Conversation

@waleedkadous

Copy link
Copy Markdown
Contributor

Summary

The verify-phase e2e found afx reset non-functional in production on every lane. This fixes it. All three fixes are confirmed against live builders with --dry-run (zero writes).

Follow-up to #1305 under the same pre-grant terms.

Refs #1273

F1 (BLOCKER, all lanes) — wrong-winner project selection

readPorchContext returned the first directory under codev/projects with a parsable status.yaml. Correct for a repo with one project dir; wrong here, because porch history is committed to main and every worktree inherits every project ever run — 203 of them. The alphabetically-first is 0087-porch-timeout-termination-retries, protocol spider, so every builder resolved protocol spider and died on Protocol "spider" has no builder-prompt.md.

The comment I had written at context.ts:163"when a worktree somehow holds more than one project directory" — named the assumption that broke. It was not "somehow"; it is the normal state of every worktree in this repo. This is the #1235 wrong-winner family: pick-the-first over a set assumed to be a singleton.

Now selects by identity, never by position. Matching ignores case and leading zeros — the registry lowercases ids while directories preserve case (builder-task-re_v vs .builders/task-RE_V, which macOS hides and Linux would not), and dirs use 0087 where ids use 87.

No match returns null — the honest answer for a non-porch builder, and the safe one for a porch builder: a missing porch block is visible in the re-orientation, whereas another project's protocol is a confident lie.

F1b — the bare --task lane has no protocol template

With F1 fixed, the probe resolved protocol task and died on Protocol "task" has no builder-prompt.md. afx spawn --task without --protocol builds its prompt straight from the task text (spawn.ts:551); there is no template to render. The long form now reproduces that shape, selected on a positive fact (task text present, no porch) rather than by catching the error. The --task --protocol variant does get a porch project, so it still renders its real template.

F2 — the task lane could never auto-detect its mode

Mode detection requires a ## Mode: line that task spawns never render, so afx reset <task> hard-errored without --mode. A no-porch builder now defaults to soft with modeSource: 'task-default'. Not a guess dressed as a fact: strict means porch orchestrates, and a builder with no porch project cannot be strict. Scoped to the no-porch case — a porch builder missing its Mode line is a genuine ambiguity and still aborts.

F3 — reported, but not a defect in reset

The path printed (.builders/task-RE_V) is correct and comes verbatim from the registry, which stores id builder-task-re_v alongside worktree .builders/task-RE_V. The id/directory case divergence originates at spawn, not here. Reset already used registry paths verbatim; the new id matching is case-insensitive so a case-sensitive filesystem behaves like macOS. Happy to file the spawn-side normalization separately if wanted.

Verification

Live --dry-run against real builders:

Target Before After
task-re_v Protocol "spider" has no builder-prompt.md TASK / soft, spawn's task prompt reproduced verbatim
1286 Protocol "spider" … ASPIR / strict, project 1286-consult-configurable-per-lane-
1273 Protocol "spider" … ASPIR

Suite 4016 → 4027, build clean.

New tests: a multi-project-dir fixture reproducing the 203-dir shape (own project wins over three foreign spider projects; null when the builder owns none; issue-number match; leading-zero and case tolerance), the task-lane mode default plus the porch-builder abort it must not swallow, and two reorient tests pinning that the template port is never called for a bare task and still is for --task --protocol.

What the e2e vindicated

Every failure was a refusal, not a corruption — the R1–R4 invariants did their job, --dry-run wrote nothing as promised, and no builder ever received a wrong re-orientation. The bugs were all in resolution, upstream of the destructive path.

Still outstanding: the live reset against the planted-context probe, for the architect to re-run after this installs.

… support the task lane

Verify-phase e2e found afx reset non-functional in production on every lane.
Three fixes, all confirmed against live builders with --dry-run.

F1 (BLOCKER, all lanes) — wrong-winner project selection.
readPorchContext returned the FIRST directory under codev/projects with a
parsable status.yaml. Correct for a repo with one project dir; wrong here,
because porch history is committed to main and every worktree inherits every
project ever run — 203 of them. The alphabetically-first is
0087-porch-timeout-termination-retries, protocol `spider`, so EVERY builder
resolved protocol spider and died on
`Protocol "spider" has no builder-prompt.md`.

The comment I wrote at context.ts:163 — "when a worktree somehow holds more
than one project directory" — named the assumption that broke. It was not
"somehow"; it is the normal state of every worktree in this repo. This is the
#1235 wrong-winner family: pick-the-first over a set assumed to be a singleton.

Now selects by identity, never by position. readPorchContext takes a
PorchProjectIdentity and matches status.yaml's `id` (or the directory prefix)
against the issue number, the builder id, or the builder id minus its protocol
prefix. Comparison ignores case and leading zeros — the registry lowercases ids
while directories preserve case (builder-task-re_v vs .builders/task-RE_V,
which macOS hides and Linux would not), and dirs use 0087 where ids use 87.

No match returns null. That is the honest answer for a genuinely non-porch
builder, and it is also the safe one for a porch builder: a missing porch block
is visible in the re-orientation, whereas another project's protocol is a
confident lie. porch is now resolved ONCE and reused for both the protocol
chain and the result, so two scans cannot disagree.

F1b — the bare --task lane has no protocol template.
With F1 fixed, the probe resolved protocol `task` from its builder id and died
on `Protocol "task" has no builder-prompt.md`. `afx spawn --task` without
--protocol takes spawn.ts's else-branch (:551) and builds the prompt straight
from the task text; there is no protocols/task/builder-prompt.md to render.
buildLongForm now reproduces that shape for the bare-task lane, selected on a
POSITIVE fact (taskText present and no porch) rather than by catching the
error. The --task --protocol variant does get a porch project (:548), so it is
unaffected and still renders its real template. Header, state pointer, addendum
and porch re-entry moved into a shared assembleLongFormDocument so the framing
cannot drift between lanes.

F2 — the task lane could never auto-detect its mode.
Mode detection requires a `## Mode:` line, which task spawns never render, so
afx reset <task> hard-errored without --mode. A no-porch builder now defaults
to soft with modeSource 'task-default'. Not a guess dressed as a fact: "strict"
means porch orchestrates, and a builder with no porch project cannot be strict.
The source is recorded so the report says where it came from. Scoped to the
no-porch case — a porch builder missing its Mode line is a genuine ambiguity
and still aborts, now naming the project that proves it is not the task lane.

F3 — reported, not a defect in reset. The path printed
(.builders/task-RE_V) is CORRECT and comes verbatim from the registry, which
stores id `builder-task-re_v` alongside worktree `.builders/task-RE_V`. The
id/directory case divergence originates at spawn, not here. Reset was already
using registry paths verbatim; the new id matching is case-insensitive so a
case-sensitive filesystem behaves like macOS.

Verified live with --dry-run (zero writes): task-re_v -> TASK/soft with spawn's
task prompt reproduced verbatim; 1286 -> ASPIR/strict on project
1286-consult-configurable-per-lane-; 1273 -> ASPIR. No spider anywhere.

Tests: multi-project-dir fixture reproducing the 203-dir shape (own project
wins over three foreign spider projects; null when the builder owns none;
issue-number match; leading-zero and case tolerance), task-lane mode default
plus the porch-builder abort it must not swallow, and two reorient tests
pinning that the template port is never called for a bare task and still is for
--task --protocol.

Suite 4016 -> 4027, build clean.
Verify-PR CMAP iter 1: Gemini APPROVE, Claude REQUEST_CHANGES, Gemini/Codex
lane missing. Claude's finding accepted — a real regression introduced by my
own F1 fix.

candidateProjectIds stripped the `builder-` prefix and offered only the
stripped forms. But `spawn --task --protocol X` passes the FULL builderId to
porch init (spawn.ts:548), and initPorchInWorktree keeps dashes when sanitising
(spawn-worktree.ts:472), so that lane's porch project id really is
`builder-task-<id>`. No candidate matched it, so readPorchContext returned null
and the builder was re-oriented as protocol TASK with no porch re-entry.

That failure mode is worse than the one F1 fixed: `spider` aborted loudly and
touched nothing, whereas this silently produced a DEGRADED frame — a
porch-driven builder told it had no porch. Exactly the silent-drift class R3
exists to prevent, reintroduced by the fix for a loud bug. Narrowing a
match is as dangerous as widening it.

Now offers the raw registry id as a candidate as well as the stripped form.

Tests: the porch id with a raw `builder-` prefix resolves, and the lane
end-to-end keeps protocolSource 'status.yaml' and modeSource 'builder-prompt'
(NOT the task default — this lane has porch, so the F2 default must not fire
for it).

Live re-verified, unchanged: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK.
Suite 4027 -> 4029, build clean.
… project id

Verify-PR CMAP iter 2: Gemini APPROVE, Claude COMMENT (one item flagged
worth fixing before merge), Codex lane unavailable. Accepted.

`issueNumber: issueNumber ?? porch?.projectId` assumed every porch project id
is an issue number. For the `--task --protocol` lane the previous commit just
un-orphaned, that id is `builder-task-<id>`, so the re-orientation rendered
`- Issue: #builder-task-abc` and an unfollowable
`gh issue view builder-task-abc`.

A fabricated issue reference is worse than no issue line: on issue-driven
protocols the issue body IS the spec, so this sends a freshly-reset builder —
one with no history to notice the oddity — to look up requirements that do not
exist. Now guarded with /^\d+$/; a non-numeric project id yields undefined and
the frame simply omits the issue.

Pinned in the existing --task --protocol end-to-end test rather than a new one,
so the lane's expectations live together.

Live re-verified, unchanged: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK.
Suite 4029, build clean.
Verify-PR CMAP iter 3: Gemini APPROVE, Claude REQUEST_CHANGES (verified live).
Accepted — the wrong-winner class SURVIVED my first fix.

F1 stopped alphabetically-first from winning, but a bare-NUMBER match still let
one project claim any builder sharing the digits, across protocols. Confirmed
in this repo: issue 799 has a PIR project
(799-vscode-builder-changed-file-ro, protocol pir) and a bugfix, so
`builder-bugfix-799` silently adopted the PIR project's protocol and porch id.
Silent, not loud — the degraded-frame failure mode, worse than the `spider`
abort it replaced.

Claims are now graded:

  strong — the project id equals the raw registry builder id and is NON-numeric
           (`builder-task-abc`). Globally unique, so it needs no corroboration
           — which matters because that lane's protocol legitimately differs
           from its id prefix (task vs air).
  weak   — matched on the bare number or a directory prefix. Requires the
           project's protocol to AGREE with the protocol the builder id
           declares.
  none   — not this builder's.

Two or more surviving claims abort loudly rather than picking one. Choosing the
least-wrong of several is the original bug with better manners.

DELIBERATE SEMANTIC REVERSAL, called out because it changes phase-4 behaviour
and breaks a test I wrote there: "prefers status.yaml over the builder id when
they disagree" no longer holds for the PROTOCOL field. status.yaml remains
authoritative for a builder's own project (phase, plan phase, identity), but it
can no longer DECIDE which project is its own. The two principles cannot both
hold — the signal is identical in each case — and the live harm settles it: a
hypothetical id/status protocol disagreement within one project is the cost;
adopting a stranger's project is what is actually happening here. The test now
encodes the new rule with that reasoning.

Tests: the 799 collision (bugfix builder gets null, PIR builder gets the
project, and the bugfix builder still resolves protocol from its id with no
porch block — visible absence, not a confident lie), the non-numeric id
crossing a protocol mismatch, and the two-equal-claims abort. Three earlier
fixtures paired a builder with a foreign-protocol project and were unrealistic;
rebuilt protocol-consistent so they still exercise zero/case tolerance.

Live re-verified: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK.
Suite 4029 -> 4035, build clean.
@waleedkadous
waleedkadous merged commit eb5333d into main Jul 31, 2026
6 checks passed
waleedkadous added a commit that referenced this pull request Jul 31, 2026
…iew of #1308

#1308 merged on 2-way CMAP during the codex outage. Retroactive advisory review
(architect-directed) returned REQUEST_CHANGES with real findings. Four fixed
here; two recorded as pre-existing and not fixed.

1. A status.yaml stating a DIFFERENT id was overruled by its directory name.
   `codev/projects/1273-old/` holding `id: '999'` belongs to 999, whatever the
   directory is called. The dir-name fallback let a renamed or recycled
   directory claim a builder, and manufactured false ambiguities beside the
   real project. The fallback now applies ONLY when the file states no id.

2. Weak claims survived when corroboration was impossible. The code read
   `if (expectedProtocol && mismatch) continue`, so a noncanonical builder id —
   which yields no protocol — let EVERY weak claim through. My own comment two
   lines above said such a claim "cannot be corroborated and is not trusted".
   The comment was right and the code did the opposite; now it rejects.
   (Third time this project that a docstring asserted what the code did not do.)

3. The numeric issueNumber guard dropped BUGFIX issue identity. BUGFIX stores
   porch ids as `bugfix-<N>` deliberately (spawn.ts:817, "historical, kept
   untouched"), so /^\d+$/ discarded it whenever 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. This was a regression I introduced in
   #1308's own issueNumber fix. New issueNumberFromPorchId accepts a bare number
   or a canonical `<word>-<number>`, still rejecting ad-hoc task ids.

4. Bare-task detection was a negative inference. initPorchInWorktree is
   deliberately non-fatal, so a `--task --protocol X` builder whose porch init
   FAILED also has task text and no porch — and `taskText && !porch` stripped
   its real protocol template and handed it raw task text instead. Replaced
   with positive evidence: `isBareTask` on the resolved context, true only when
   task text is present AND the prompt carries no `## Mode:` heading. The prompt
   is written BEFORE porch init (spawn.ts:545-548), so a --task --protocol
   builder's rendered template survives a failed init and is reliable evidence.
   This also correctly re-scopes the F2 mode default.

RECORDED, NOT FIXED — both pre-existing spawn properties rather than
regressions from this work, and neither is reachable in this repo today:

- Hyphenated protocol names: `builder-my-protocol-1273` parses as protocol `my`
  via parseAgentName's first-dash split. Needs a custom protocol containing a
  hyphen; none exists. Fixing it means constraining protocol names at spawn or
  changing the agent-name encoding — outside a reset fix.
- Task short ids are 24-bit random and not checked against committed porch
  history, so a reused `builder-task-xxxx` could strong-match a stale project.
  Probabilistic and spawn-side; belongs with the id-generation issue (#1312).

Live re-verified: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK.
Suite 4035 -> 4041, build clean.
waleedkadous added a commit that referenced this pull request Jul 31, 2026
…iew of #1308

#1308 merged on 2-way CMAP during the codex outage. Retroactive advisory review
(architect-directed) returned REQUEST_CHANGES with real findings. Four fixed
here; two recorded as pre-existing and not fixed.

1. A status.yaml stating a DIFFERENT id was overruled by its directory name.
   `codev/projects/1273-old/` holding `id: '999'` belongs to 999, whatever the
   directory is called. The dir-name fallback let a renamed or recycled
   directory claim a builder, and manufactured false ambiguities beside the
   real project. The fallback now applies ONLY when the file states no id.

2. Weak claims survived when corroboration was impossible. The code read
   `if (expectedProtocol && mismatch) continue`, so a noncanonical builder id —
   which yields no protocol — let EVERY weak claim through. My own comment two
   lines above said such a claim "cannot be corroborated and is not trusted".
   The comment was right and the code did the opposite; now it rejects.
   (Third time this project that a docstring asserted what the code did not do.)

3. The numeric issueNumber guard dropped BUGFIX issue identity. BUGFIX stores
   porch ids as `bugfix-<N>` deliberately (spawn.ts:817, "historical, kept
   untouched"), so /^\d+$/ discarded it whenever 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. This was a regression I introduced in
   #1308's own issueNumber fix. New issueNumberFromPorchId accepts a bare number
   or a canonical `<word>-<number>`, still rejecting ad-hoc task ids.

4. Bare-task detection was a negative inference. initPorchInWorktree is
   deliberately non-fatal, so a `--task --protocol X` builder whose porch init
   FAILED also has task text and no porch — and `taskText && !porch` stripped
   its real protocol template and handed it raw task text instead. Replaced
   with positive evidence: `isBareTask` on the resolved context, true only when
   task text is present AND the prompt carries no `## Mode:` heading. The prompt
   is written BEFORE porch init (spawn.ts:545-548), so a --task --protocol
   builder's rendered template survives a failed init and is reliable evidence.
   This also correctly re-scopes the F2 mode default.

RECORDED, NOT FIXED — both pre-existing spawn properties rather than
regressions from this work, and neither is reachable in this repo today:

- Hyphenated protocol names: `builder-my-protocol-1273` parses as protocol `my`
  via parseAgentName's first-dash split. Needs a custom protocol containing a
  hyphen; none exists. Fixing it means constraining protocol names at spawn or
  changing the agent-name encoding — outside a reset fix.
- Task short ids are 24-bit random and not checked against committed porch
  history, so a reused `builder-task-xxxx` could strong-match a stale project.
  Probabilistic and spawn-side; belongs with the id-generation issue (#1312).

Live re-verified: 1273 -> ASPIR, 1286 -> ASPIR, task-re_v -> TASK.
Suite 4035 -> 4041, build clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant