Repository navigation
After #871: the waiting set is not closed transitively, the safety gate's word can stick, and the seams the lane did not take #921
Description
Base: dev@e67f1827b (#871, nothing auxiliary blocks the person). Everything
here was found by the architecture re-review of that PR or reported by the lane
and deliberately not taken. Two are defects in the laws #871 landed — a law that
can be walked around is worse than no law, because it is read as proof — one is a
real race a person can see with Guardian on, one is doc rot, and the rest are
the named follow-ups.
#896 item 4a is CLOSED by #871. postPhaseNews and postLaneNews both hand
the news to a desk (internal/session/sidecar.go) instead of calling the
reader down the caller's stack, so the engine goroutine no longer pays a Bubble
Tea Update+View cycle at the exit of every model call.
TestASlowListenerNeverHoldsTheTurnThatIsTellingIt is the runtime guard and
TestAListenerIsAlwaysToldFromADesk the structural one. internal/provider
needed no change: its own header already states the division, and the reader it
had installed was this package's.
1 — the waiting set is not closed transitively, so one helper reopens the gate
internal/session/sidecar_law_test.go derives the set of readings from the
tree — every selector a readBeside ask calls — and then refuses either a
reading performed, or the waiting verb takeAtTheEnd awaited, anywhere the turn
is not already committed to ending. Both halves are right and the set is one hop
deep. A function that does the waiting for you is not in it, and neither is
anything that calls it.
Replication (deterministic, no model)
Add this to internal/session — nothing else, no edit to the law:
// settleTheMark waits for the drawing and hands the turn over.
func (a *Agent) settleTheMark(ctx context.Context, aside *markAside, …) bool {
landed, _ := aside.takeAtTheEnd()
return a.handOverRunningTurn(ctx, landed, …).moved
}and call it from the step boundary in runTurn, where the turn carries on:
if a.settleTheMark(ctx, marked, …) {
return
}go test ./internal/session -run TestEveryReadingBesideTheWorkGoesThroughTheOneDoor
is green. The helper itself is honest by the law's own property — it reaches
an ending door with nothing between — and its caller is not watched at all,
because settleTheMark is neither a readBeside ask nor the waiting verb. The
whole 8.1-second mid-turn gate the census measured is back, behind one wrapper,
with the gate that exists to stop it reporting success.
The same hole read from the other end is 1b: TestAListenerIsAlwaysToldFromADesk
matches a listener call only as a bare ident(args) (calledOutsideADesk tests
call.Fun.(*ast.Ident)). A listener reached as a field or through a slice —
a.reader(news), readers[i](news) — is a SelectorExpr or an IndexExpr and
the law never sees it. Plant func post(a *Agent, news PhaseNews) { a.reader(news) }
and the desk law stays green.
Where
internal/session/sidecar_law_test.go — readingsFrom, besideLaw,
calledOutsideADesk. Searchable: the law is reading the wrong tree.
The fix
Close both watched sets to a fixed point rather than one hop. A function is
watched if it performs a watched call or awaits the waiting verb anywhere the
turn is not committed to ending; iterate until the set stops growing, then judge
callers by the same property. That is the honest reading of the sentence already
in the file — whatever a readBeside ask calls IS a reading — carried one
level further, and it is roughly fifteen lines: a worklist over the package's
own FuncDecls. For 1b, match a call whose Fun resolves to a listener through
Ident, SelectorExpr or IndexExpr rather than Ident alone.
Acceptance
- Unit (the law, on the laws gate): a fourth planted shape — the wrapper
above, called from a branch that carries on — is named by
TestTheLawNamesAWaitPutBackInFrontOfTheWork, and the complaint names
the caller, not the helper, because the helper is honest. - Unit: a fifth plant for 1b —
a.reader(news)through a struct field — is
named byTestTheLawNamesAListenerCalledOnTheTurnsOwnGoroutine. - Unit:
TestTheLawLetsAnEndingReadInLinestill passes unchanged, so the
closure did not swallow the three honest shapes. A fixed point that fails
everything is not a fix. - e2e: none is owed. This is a structural law about the tree and the
behaviour it guards already has its runtime door
(TestTheOnlyWaitAPersonExperiencesIsTheModelGenerating).
2 — two guarded calls in one batch can leave checking whether this is safe to run stuck on the line
Agent.interruptPhase says a word over the top of whatever the turn is saying
and hands back the way to put the old one back. It captures under one lock and
posts under another:
a.phase.mu.Lock()
held := a.phase.held
a.phase.mu.Unlock()
a.tellPhase(phase, detail, since) // takes the same lock againA tool batch runs its calls concurrently — loop.go starts one goroutine per
call and waits on a WaitGroup — and every one of them goes through the consent
door, so with Config.Guardian on two of them reach this function at once:
A (go test) |
B (read) |
|
|---|---|---|
| 1 | captures running go test |
|
| 2 | tells checking whether this is safe to run |
|
| 3 | captures checking whether this is safe to run |
|
| 4 | tells checking whether this is safe to run |
|
| 5 | restores running go test |
|
| 6 | restores checking whether this is safe to run |
The batch is still running and the line now says the gate's word until something
else replaces it — on a go test that is minutes. It is the mirror of the defect
#871 fixed (the gate clearing the batch's word); the fix took the capture out of
the same critical section as the post, and the concurrent case was missed.
Replication (deterministic, no model)
internal/session, no provider and no key: build an agent with
Config.Guardian on and a stubbed guardian that answers slowly, drive one round
whose batch has two calls both landing on PROMPT, collect PhaseNews through
OnPhaseNews, and assert the last phase before the batch ends. Today it is
PhaseChecking with guardianPhaseWho as its detail; it should be
PhaseRunning naming the batch's first tool. The interleaving is forced without
sleeps by releasing the two stubbed guardian answers in the order above.
Where
internal/session/phasenews.go — Agent.interruptPhase. Searchable:
THE OLD STAGE KEEPS ITS OWN START. The caller is
internal/session/guardian.go, searchable restorePhase.
The fix
One critical section for capture-and-post, and an identity check in the
restore: put the old word back only if what is held is still the word this
interrupt installed. A restore that finds a stranger's word does nothing, which
is right — somebody else owns the line now and will restore it themselves. The
slot stays one slot; what changes is that a nested interrupt cannot capture a
word that is itself an interrupt.
Only reachable with Config.Guardian on, which is off by default (see §6),
so nobody has seen it yet — that is a reason to fix it before somebody does, not
a reason to leave it.
Acceptance
- e2e: the tmux suite with
Guardianon, a two-call batch on a long tool,
asserting the status line readsrunning go testwhile the build runs and
never sits onchecking whether this is safe to runafter both gates answer. - Unit: the deterministic replication above, asserting the phase sequence
throughOnPhaseNews. - Unit: a restore whose word has since been replaced by a third party is a
no-op.
3 — readRemains never goes through the door, so the law cannot see it
The mark's other reading — the one asked when the model stops without a tool
call — is called straight:
decision := a.decideRemains(ctx, a.readRemains(ctx), said)from two places in checkpoint.go (the turn's end, and the handover road). It
never passes through readBeside, so readingsFrom never learns the name and
§1's law does not watch it. It is awaited on a road that really does end the
turn, so it is not a live defect — it is a reading the law is blind to, and
the next person to move that call has no gate.
This is the same seam as the lane report's §6.2: making the end-of-turn readers
land after EventTurnDone needs an announcement lane that outlives the turn,
because launchRouteTask announces through the hub and the hub closes when
runTurn returns. Both want the same decision from whoever owns eventHub, so
they are one item.
Replication (deterministic, no model)
go test ./internal/session -run TestEveryReadingBesideTheWorkGoesThroughTheOneDoor -v
and print the derived set: readRemains is not in it. Move the
a.readRemains(ctx) call to the top of runTurn and the law is still green.
Where
internal/session/checkpoint.go — Agent.readRemains, and its two callers.
Searchable: readRemains asks the mark's own reader.
The fix
Decide the announcement question first, because it sets the shape: either
eventHub gains a post-turn announcement lane that survives runTurn's return
— in which case both readRemains and the route judge become ordinary readings
started beside the work and taken at the end — or it does not, and readRemains
is converted to readBeside anyway so the law can see it, awaited through
takeAtTheEnd on the ending road it already sits on. The second is small and
buys the gate; the first is the actual speed win and is a design call.
Acceptance
- Unit:
readRemainsappears in the setreadingsFromderives, and moving
its call to a road that carries on fails the law by name. - e2e:
go test ./internal/session -run 'Remains|Reopen|Handover'green — the
reading still decides the same three outcomes (re-open, let the turn end, hand
over), asserted on the decision rather than on the call count.
4 — doc rot: closeMarkAside is gone and checkpoint.go still names it as the caller
// is for an ENDING and nowhere else ([Agent.closeMarkAside] is the one caller).Agent.closeMarkAside was deleted answering the review — the settle is inlined
into runTurn's own exit defer, which is where the tree can see the ending — and
this doc link now points at nothing. A dead [Agent.…] link is worse than a
plain sentence: it reads as a claim about where to look.
Where
internal/session/checkpoint.go, on markAside.takeAtTheEnd. Searchable:
is for an ENDING and nowhere else.
The fix
Name the real caller: runTurn's exit defer (loop.go). One line.
Acceptance
- Unit:
go vet ./internal/session/and the doc-link check pass; a grep for
closeMarkAsideacross the tree returns nothing.
5 — Agent.PrefetchRecall(draft): typing time is free latency
The largest pre-turn win left, and it is not in internal/session's gift alone.
#871 took the recall off the critical path — it starts when the turn starts and
is applied wherever it lands — but it still starts at submit. A person types
for seconds before they press enter, and the recall's own cue is in the draft.
The fix
Agent.PrefetchRecall(draft string) starts the reading against the current draft
after a short dwell, and startTurnLocked adopts a prefetch whose cue still
matches what was actually sent rather than starting a second one. The surface
(internal/tui3) calls it from the composer. A prefetch that does not match is
let go, which costs one cheap call on a person who rewrites their message.
Acceptance
- e2e: the tmux suite types a message with a clear cue, waits past the dwell,
presses enter, and the turn'spacejournal row shows the recall already
landed at assembly (recallin the decomposition, norecall:latenote). - Unit: a draft that is edited after the prefetch fires starts at most one
more reading, and the turn never carries memory read against a cue the person
deleted.
6 — the smaller seams, each with its door
checkpointBriefhas no window at all. The handover's first model call
takes the whole transcript on the conversation model with no bound; measured
15–30 s between the model's last word and the task appearing. It is an ENDING
so nothing can run beside it, andbriefing a workercovers it — but no bound
is still no bound. Recommendlane.RoleJudge.GiveUp()(90 s), matching
writeHandoff's own order of magnitude rather than a flat wall. Acceptance:
a stubbed conversation model that never answers ends the handover inside the
give-up with the brief's fall-through, asserted through
go test ./internal/session -run Handover.- The caption and the two namers are still hand-rolled goroutines.
maybeCaption,taskname.goandjobname.goeach have their own channel
discipline and each is a reading beside the work by every part of the
definition. Converting them toreadBesideis mechanical and deletes three
shapes. Acceptance: they appear in the setreadingsFromderives, and
TestThereIsOneDoorForAReadingBesideTheWorkstill finds exactly one door. internal/tui3/phase_test.goposts aPhasePreparingwith"saved context".
Nothing emits that phase any more — Nothing auxiliary blocks the person: one reading beside the work, never in front of it #871 deleted it, because it named a wait
that is gone. The test is green either way (it renders news it makes itself),
which is exactly why it will outlive everyone who remembers what it was for.
Acceptance: the case is dropped or repointed at a phase the engine still
posts, asserted by the phase word appearing ininternal/session.Config.Guardianis off by default, sochecking whether this is safe to run— and §2's defect — are invisible on a stock install. Somebody should say
whether that default is deliberate. Acceptance: whichever way it goes, the
manual page that describes the safety gate
(internal/manual/chat/permissions.md) states the default in the product's own
words, andinternal/manual/chat_test.go's probe table reaches it from "is
anything checking what tools run".
Why one issue and not seven
§1 and §2 are the ones that should not sit: a law with a fifteen-line hole in it
is read by the next lane as proof, and §2 is a person-visible wedge waiting for
whoever turns the guardian on. §3 and §6's first two are the same conversation
about where a reading lives. §4 is a minute. §5 is the only one that is a new
capability rather than a correction, and it is the biggest remaining win.
🤖 Generated with Claude Code
Drafted with CodeAF · reviewed and owned by the author
Activity