[Spec 1273] Builder context reset: save-state → /clear → re-orient - #1305
Merged
Conversation
afx reset (designed), afx interrupt (minimal), builder wait-discipline docs. Records the three reset invariants (assemble-before-destroy, nonce receipt, complete frame) that make the issue's two failure modes impossible by construction, plus the A/B/C mechanism exploration.
Codex REQUEST_CHANGES (both gaps closed): - Add invariant R4 (quiescence-or-abort): bounded wait, exactly one ESC escalation permitted only after the R2 receipt, then abort without clearing. - Define the re-orientation payload contract: role frame = identity block, not the full role document; fixed inline-vs-referenced division; separate --file (caller filesystem) from the state-file path override (worktree-bound). Claude/Gemini comments: default path assumes an addressable builder, double-reset test, --resume wording, content-quality trade-off, wedged-builder integration test, state file named .builder-state.md, hybrid delivery resolved.
Six phases: afx interrupt + ESC delivery path; lastDataAt observability; reset receipt gate (R2); re-orientation assembly (R3); reset orchestrator (R1/R4); docs. Phases 1-4 independent, phase 5 the only integration point. Invariants enforced by module boundaries and asserted over an ordered step log, so 'impossible by construction' is checkable. Resolves the spec's two numeric open questions (min-bytes, quiet window, timeouts).
Codex REQUEST_CHANGES (4 issues, all verified against code first): - Add packages/core/src/tower-client.ts (escape option) to phase 1 — the client surface afx interrupt actually rides on. - New phase 4 (context resolution). The registry does not carry what the plan assumed: builders.protocol_name is NULL for spec-type builders (spawn.ts:488-492 never passes it), and mode is not persisted at all. Resolve protocol/phase/mode/harness from status.yaml, .builder-prompt.txt and .builder-start.sh, each chain ending in a loud abort. Declined a mode DB column — NULL for every running builder, and duplicates the worktree. - Anchor re-orientation long form to buildPromptFromTemplate output and reuse buildResumeNotice verbatim, instead of 'registry -> payload'. - Update both .claude and .codex afx skill trees. Claude non-blocking: add supportsContextReset to HarnessProvider; extend the existing pty-last-data-at.test.ts rather than adding a parallel file. Phase count 6 -> 7.
… builder's PTY Makes the verified mid-turn recovery a first-class command. Until now `afx send <b> --raw "$(printf '\x1b')"" lived only in architect lore and had to be rediscovered under pressure. - core tower-client: escape option on sendMessage, forwarded in the body. - message-write: writeEscapeToSession + ESC/ESCAPE_ENTER_DELAY_MS. ESC is a control byte, not text, so it bypasses line pacing. The trailing Enter is load-bearing (it lets messages queued during the wedge process), not incidental — --no-enter opts out. - tower-routes: escape branch placed before formatting AND before the send buffer. An interrupt that can be deferred because someone recently typed is not an interrupt. - commands/interrupt.ts: reuses afx send's resolver, workspace detection and #1094 identity check verbatim — one address resolver, not two. Tests: byte sequence, --no-enter, buffer bypass with isUserIdle()=false, non-writable terminal fails loudly, normal sends unaffected, and a regression pinning that a lone \x1b survives handleSend's trim()/non-empty guard — an accidental invariant the whole recovery rests on.
… terminal info Spec 467 has tracked _lastDataAt since it landed, but only as an in-process getter — it never reached `info`, which is what GET /api/terminals/:id serialises. Without it on the wire a client cannot measure output quiescence, and afx reset would have to ASSUME a builder's turn had ended before typing /clear into its terminal. R4 requires measuring, not assuming. - PtySessionInfo.lastDataAt (required) + the info getter. - TowerTerminal.lastDataAt (optional — an older Tower omits it, and a consumer that needs it must say so rather than read a missing field as 0). Rejected alternative, recorded in the plan: polling /output and diffing tails. It ships output bytes on every poll and cannot tell 'no new output' from 'new output identical to the last tail' — a repeating spinner frame. A monotonic timestamp has neither problem. Tests appended to the existing Spec 467 file rather than a parallel one: field present and typed on info, advances on output, holds steady while silent (the actual quiescence signal), and stays in sync with the getter.
…erminals/:id wire contract
Phase 2 CMAP: Codex REQUEST_CHANGES — the new tests exercised session.info
directly and never called the endpoint, leaving the phase's own acceptance
criterion ('GET /api/terminals/:id includes lastDataAt as an epoch-ms number')
unverified. Testing the getter pins the field on the class; afx reset reads it
over HTTP.
Claude reviewed the same gap and argued the opposite — the handler is a pure
JSON.stringify(session.info) passthrough, so the unit test suffices. Sided with
Codex: 'pure passthrough' is a property of today's handler, not of the contract,
and a future projection or envelope at the route would break the wire while
every session.info test stayed green. Reasoning recorded in the rebuttal.
… stability
Replaces the manual flow's eyeball check ('ours was 203 lines') with evidence.
A state file is accepted only if it carries THIS run's nonce, meets a minimum
size, and has stopped growing between two observations a real interval apart.
Freshness comes from a nonce INSIDE the file, not mtime: timestamp granularity
and clock skew make mtime fragile, and it cannot tell 'rewritten in response to
this request' from 'touched'. A nonce can only appear in a file written after
the request carrying it — which is what kills the stale-file case.
Checks run existence -> freshness -> substance -> stability so the reported
reason is the most specific one true. A stale stub is both wrong-nonce and
too-small; reporting 'too-small' would send the architect chasing --min-bytes
for a staleness problem.
Pure module over an injected fs port: every rejection path is testable with no
builder, no worktree, no clock. 24 tests, including that a first observation is
never accepted however substantive the file, and that whitespace drift in the
marker does not discard a genuine save.
…join Phase 3 CMAP: Codex REQUEST_CHANGES — stateFilePath concatenated a hardcoded '/' and stripped only forward slashes, so a Windows worktree yielded C:\repo\wt\/.builder-state.md. Not cosmetic: this string is interpolated into the save request and handed to the builder verbatim as 'write your state here'. A malformed path means the builder writes to one location while the gate stats another — the file never appears, R2 times out, and the failure presents as 'the builder ignored the request', sending the architect to debug builder behaviour rather than a path bug. Also (Claude, minor): the 'every item on the cold-reader checklist' test asserted 6 of 7, omitting 'position in the protocol'. The gate is structural by design — it never scores prose — so the request wording is the only mechanism driving state-file quality, and these assertions are the only thing pinning it. 26 tests pass.
…from the worktree This phase exists because the plan CMAP caught the first draft assuming the builder registry held these facts. It does not: - builders.protocol_name is NULL for spec-type builders — spawn.ts never passes protocolName on that path, only the protocol-type spawn does. Every SPIR/ASPIR lane, the exact target of this feature, has NULL there. db/schema.ts shows the column and looks fine; the persistence path is where the truth is. - Mode is not persisted at all. resolveMode computes it at spawn from flags plus protocol defaults and discards it, so a spawn-time --soft cannot be recovered by recomputation. - Harness comes from workspace config, which can change while a builder runs. So resolution reads the worktree, which holds all of it authoritatively: status.yaml (protocol, phase), .builder-prompt.txt (the literal '## Mode:' line the builder was given, still correct after --resume), .builder-start.sh (the launch line of the process actually running). Every chain ends in a loud abort, never a default — a guessed protocol or mode yields a plausible re-orientation that quietly reframes the builder, the exact drift R3 prevents. A non-porch lane resolves with porch: null; that is a branch, not a gate failure. Adds supportsContextReset to HarnessProvider (optional, absence means no — the safe direction). Claude declares it; the others abort by name. 24 tests, including resolution succeeding with a NULL protocol_name.
…ustom harnesses, match on command position Phase 4 CMAP: Codex REQUEST_CHANGES (2), Claude non-blocking (3). All accepted; the first defect was found independently by both reviewers. 1. Resolved context was missing specName/specPath/planPath and forwarded rather than resolved the issue number. Phase 5 builds its long form from buildPromptFromTemplate, whose TemplateContext needs those paths; leaving them out would push worktree-reading into the phase whose job is to fail loudly on a missing field, not to go hunting for one. Paths are returned only when the file exists — a pointer to a nonexistent plan would send a freshly-reset builder, with no memory to cross-check against, chasing a ghost. 2. Harness lookup ignored custom providers from .codev/config.json, so a custom-harness builder failed as "no recognisable launch command" — sending the project to debug a config that is correct. Now recognised, mapped via buildCustomHarnessProvider, then capability-checked, so the refusal names the real reason. Custom harnesses stay unsupported by default (the safe default). 3. Harness detection searched the whole line, so a future script containing a conditional that mentions another harness name would return the wrong one. That is not a harmless misread: it either refuses a resettable builder or approves typing /clear into one that cannot reset, where the keystrokes land as literal text in a live agent's prompt. Now matches command position via commandNameOf(), stripping shell keywords and VAR=value prefixes, then basenaming. 4. protocolFromStatus and readPorchContext each scanned the project dirs. Unified — two scans could disagree about which status.yaml is authoritative. 38 tests (was 24).
…reference
Phase 7. Moves wait discipline out of architect lore into the builder-facing
role document, and makes both commands discoverable where they are looked up.
Role doc (skeleton primary, codev mirror kept byte-identical — the four-tier
resolver means codev/ SHADOWS the skeleton for this workspace, so
skeleton-only would leave our own builders without the guidance and
codev-only would leave every adopter without it). Three rules, each with the
reasoning that makes it stick:
- A wait is a CLAIM THAT A PRODUCER EXISTS. Verify the producing process is
alive first. In the incident (2026-07-27) the producer had already died,
so the wait was not slow — it was unsatisfiable.
- Run waits as tracked background tasks that END THE TURN. A turn that ends
is a turn someone can interrupt.
- NEVER CHAIN FOREGROUND POLL LOOPS. Not for efficiency: every afx send —
including the order to stop, including a reset request — queues unread
until the turn ends. A turn that never ends is a builder nobody can
reach, and from inside the turn everything looks fine.
- Plus the escape hatch: afx interrupt and afx reset both reach you anyway,
worth knowing so you can suggest them when you notice trouble.
Command reference: full afx reset and afx interrupt entries in BOTH trees,
including every flag and the fail-safe property (every gate aborts without
clearing). That last part is load-bearing for adoption — without it the
command reads as far more dangerous than it is and goes unused.
Both skill trees updated: .claude/skills/afx/SKILL.md AND
.codex/skills/afx/SKILL.md. Updating only the Claude one would leave
Codex-driven agents with no reason to believe reset exists.
18 docs tests: the three rules and their reasoning are present in both role
docs, all eight reset flags are documented in both command docs, the
fail-safe property is stated, both skill trees carry both commands, and the
role-doc and skill-doc pairs are asserted byte-identical. Docs tests are
usually a smell; they earn their place because four parallel trees is exactly
where "updated one, forgot the twin" hides — it is invisible in review since
the file you are reading looks right. This is the standing "grep BOTH trees"
lesson, executed.
Noted, not fixed: codev/ and codev-skeleton/ copies of agent-farm.md carry
~375 lines of PRE-EXISTING drift unrelated to this work (wording of shellper
descriptions etc). My added sections are byte-identical across both; the
existing drift is out of scope and left for a reconciliation pass.
Suite 3958 -> 3976, build clean.
… own echo PR CMAP iter 2: Gemini APPROVE, Claude APPROVE, Codex REQUEST_CHANGES. Accepted — a genuinely new finding. confirmClear matched /context (?:cleared|reset)|conversation cleared|\/clear/i. The final alternative matches the ECHO of the keystroke we just typed. A PTY echoes its input, so `/clear` appears in recent output on every run: the check was self-fulfilling and reported clear-confirmed unconditionally. Same defect as phase 6 iter 3 wearing the opposite mask. There readRecentOutput was unbound, so the report always said unconfirmed — a check that never looked. Here it always said confirmed — a check that looked only at its own reflection. I fixed the first by binding the reader and introduced the second in the same breath, by never asking what the output would contain GIVEN that we had just typed into it. This direction is strictly worse. A false "unconfirmed" is conservative. A false "confirmed" is trusted — it is the one line of the report that speaks to whether the destructive step landed, and it would have said yes regardless. On a harness where /clear silently no-ops, reset would have reported complete success while the builder kept its whole context. Pattern now matches only what the harness emits after clearing, never the input echo. Test feeds `> /clear\n> ` (exactly what a PTY echo looks like) and requires clear-unconfirmed, with an explicit negative assertion on clear-confirmed. Confirmation stays advisory — report-only, and the re-orientation is correct either way. The point is that the report has to be believable, which is the same argument that motivated the step log. Suite 4014 -> 4015, build clean.
…put only
PR CMAP iter 3: Gemini APPROVE, Claude APPROVE, Codex REQUEST_CHANGES.
Accepted. Third consecutive finding on the same three lines, third distinct
false-report mechanism.
buildSaveRequest opens with "CONTEXT RESET INCOMING — save your working state
now.", which matches /context (?:cleared|reset)/i. Reset sends that into the
same terminal moments before clearing, so it is in the buffer on every run.
Last round I removed the `/clear` alternative without asking the obvious
follow-up: what else in this buffer did I put there?
The progression is the point:
1. readRecentOutput unbound -> always unconfirmed (never looked)
2. pattern matched /clear -> always confirmed (own keystroke echo)
3. pattern matched the request -> always confirmed (own message)
Each fix was a better regex and each time the regex was the wrong layer. The
defect was scanning a buffer that CONTAINS reset's own writes and hoping the
pattern would not collide. Reset writes into that terminal three times per
run; collision is the default, not a risk.
Fixed structurally: readRecentOutput becomes readOutput(): { lines, total }.
The orchestrator snapshots `total` immediately before sending /clear and
confirms against only the lines produced after it. Everything reset wrote sits
at or below the snapshot and is excluded by construction, so a future pattern
change cannot reintroduce the class.
Test harness models it honestly — PRE_CLEAR_BUFFER carries the real
save-request header, so a windowing regression leaks reset's own words into
the check and fails the negatives. Two tests pin the closed cases (echoed
/clear, echoed save request), one confirms the check still fires on genuine
post-clear output (a guard that only ever returns false is no better than the
unbound version).
Named in the review as a remaining gap: the PATTERN is still unvalidated. I
have never observed what a real /clear emits, so the matched strings remain an
educated guess. The window closes the false-positive class; it cannot prove
the pattern ever matches in production. clear-confirmed means "the harness
said something clear-like", never proof. Settled by the live e2e. This is why
confirmation is advisory and report-only — no invariant depends on it.
Suite 4015 -> 4016, build clean.
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
afx reset <builder>makes the hand-run context-reset procedure a supported command: request a save-state, verify it, wait for the builder's turn to end,/clear, then re-orient with the same protocol framing a fresh spawn delivers.afx interrupt <builder>wraps the ESC-into-PTY recovery that previously lived only in architect lore. Wait discipline moves into the builder role doc so the wedge that motivated this is less likely to recur.The problem:
afx spawn --resumereattaches the same conversation, so a builder that has exhausted its context resumes exhausted. There was no supported way to give a long-running builder a fresh window without losing what it knew.Closes #1273
Changes
Seven phases, 48 files, 3894 → 3976 tests.
afx interrupt— ESC delivery via a new Towerescaperoute (the only thing that reaches a builder mid-turn).lastDataAton serialised terminal info — makes output quiescence measurable by a client.builders.protocol_nameis NULL for every SPIR/ASPIR lane and mode is in no column at all).buildPromptFromTemplate's output, not a paraphrase of it.Safety invariants
Every gate fails toward not clearing, exits non-zero, and names the gate that failed. A refused reset leaves the builder with its context and a saved state file; a wrong clear is unrecoverable.
Reset's safety properties are all ordering properties, and a run that clears before saving and one that clears after both end
completed— only the sequence distinguishes them. Hence the step log, and hence tests that assert absence (noclearin any aborted run;escalate-escnever beforereceipt-accepted).Testing
afx reset --helpand the invalid-flag rejections verified against the realafxbinary, not just the build.pnpm -w run local-install, which restarts Tower and affects every builder in the workspace. Per the architect this happens in the post-merge verify window. 3976 passing tests is not "it works" and this PR does not claim otherwise.Review notes
Two findings worth the reviewer's attention, both documented in the review:
/clearnearly shipped as a no-op. Tower'sescaperoute writes a hardcoded ESC and discards the message body, so binding the clear to it would have sent an interrupt — a clean-looking run in which the builder kept its entire context. Fixed structurally with three distinct port methods.--quiet-window -1made quiescence pass instantly;--timeout nopeproduced a NaN deadline that never fires. Now rejected at both the CLI and library boundaries.Both are the same class: silent success. Neither was visible from the state machine's own tests, which is why a separate command-surface test file exists and is carrying four regressions.
Known gaps
node-ptyand cannot model an agent's turn, which is what a wedge consists of. Declared under the plan's explicit escape hatch.codev/andcodev-skeleton/copies ofagent-farm.mdcarry ~375 lines of pre-existing, unrelated drift. The sections added here are byte-identical across both trees.Spec
codev/specs/1273-builder-context-reset-should-b.md
Plan
codev/plans/1273-builder-context-reset-should-b.md
Review
codev/reviews/1273-builder-context-reset-should-b.md