You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 readBesideask 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()
returna.handOverRunningTurn(ctx, landed, …).moved
}
and call it from the step boundary in runTurn, where the turn carries on:
ifa.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 by TestTheLawNamesAListenerCalledOnTheTurnsOwnGoroutine.
Unit:TestTheLawLetsAnEndingReadInLine still 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.helda.phase.mu.Unlock()
a.tellPhase(phase, detail, since) // takes the same lock again
A 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 Guardian on, a two-call batch on a long tool,
asserting the status line reads running go test while the build runs and
never sits on checking whether this is safe to run after both gates answer.
Unit: the deterministic replication above, asserting the phase sequence
through OnPhaseNews.
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:
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:readRemains appears in the set readingsFrom derives, 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 closeMarkAside across 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's pace journal row shows the recall already
landed at assembly (recall in the decomposition, no recall:late note).
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
checkpointBrief has 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, and briefing a worker covers it — but no bound
is still no bound. Recommend lane.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.go and jobname.go each have their own channel
discipline and each is a reading beside the work by every part of the
definition. Converting them to readBeside is mechanical and deletes three
shapes. Acceptance: they appear in the set readingsFrom derives, and TestThereIsOneDoorForAReadingBesideTheWork still finds exactly one door.
internal/tui3/phase_test.go posts a PhasePreparing with "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 in internal/session.
Config.Guardian is off by default, so checking 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, and internal/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.
Base:
dev@e67f1827b(#871, nothing auxiliary blocks the person). Everythinghere 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
Guardianon, one is doc rot, and the rest arethe named follow-ups.
#896 item 4a is CLOSED by #871.
postPhaseNewsandpostLaneNewsboth handthe news to a
desk(internal/session/sidecar.go) instead of calling thereader 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.
TestASlowListenerNeverHoldsTheTurnThatIsTellingItis the runtime guard andTestAListenerIsAlwaysToldFromADeskthe structural one.internal/providerneeded 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.goderives the set of readings from thetree — every selector a
readBesideaskcalls — and then refuses either areading performed, or the waiting verb
takeAtTheEndawaited, anywhere the turnis 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:and call it from the step boundary in
runTurn, where the turn carries on:go test ./internal/session -run TestEveryReadingBesideTheWorkGoesThroughTheOneDooris 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
settleTheMarkis neither areadBesideask nor the waiting verb. Thewhole 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:
TestAListenerIsAlwaysToldFromADeskmatches a listener call only as a bare
ident(args)(calledOutsideADesktestscall.Fun.(*ast.Ident)). A listener reached as a field or through a slice —a.reader(news),readers[i](news)— is aSelectorExpror anIndexExprandthe 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
readBesideask calls IS a reading — carried onelevel further, and it is roughly fifteen lines: a worklist over the package's
own
FuncDecls. For 1b, match a call whoseFunresolves to a listener throughIdent,SelectorExprorIndexExprrather thanIdentalone.Acceptance
above, called from a branch that carries on — is named by
TestTheLawNamesAWaitPutBackInFrontOfTheWork, and the complaint namesthe caller, not the helper, because the helper is honest.
a.reader(news)through a struct field — isnamed by
TestTheLawNamesAListenerCalledOnTheTurnsOwnGoroutine.TestTheLawLetsAnEndingReadInLinestill passes unchanged, so theclosure did not swallow the three honest shapes. A fixed point that fails
everything is not a fix.
behaviour it guards already has its runtime door
(
TestTheOnlyWaitAPersonExperiencesIsTheModelGenerating).2 — two guarded calls in one batch can leave
checking whether this is safe to runstuck on the lineAgent.interruptPhasesays a word over the top of whatever the turn is sayingand hands back the way to put the old one back. It captures under one lock and
posts under another:
A tool batch runs its calls concurrently —
loop.gostarts one goroutine percall and waits on a
WaitGroup— and every one of them goes through the consentdoor, so with
Config.Guardianon two of them reach this function at once:go test)read)running go testchecking whether this is safe to runchecking whether this is safe to runchecking whether this is safe to runrunning go testchecking whether this is safe to runThe batch is still running and the line now says the gate's word until something
else replaces it — on a
go testthat 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 withConfig.Guardianon and a stubbed guardian that answers slowly, drive one roundwhose batch has two calls both landing on PROMPT, collect
PhaseNewsthroughOnPhaseNews, and assert the last phase before the batch ends. Today it isPhaseCheckingwithguardianPhaseWhoas its detail; it should bePhaseRunningnaming the batch's first tool. The interleaving is forced withoutsleeps 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 isinternal/session/guardian.go, searchablerestorePhase.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.Guardianon, 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
Guardianon, a two-call batch on a long tool,asserting the status line reads
running go testwhile the build runs andnever sits on
checking whether this is safe to runafter both gates answer.through
OnPhaseNews.no-op.
3 —
readRemainsnever goes through the door, so the law cannot see itThe mark's other reading — the one asked when the model stops without a tool
call — is called straight:
from two places in
checkpoint.go(the turn's end, and the handover road). Itnever passes through
readBeside, soreadingsFromnever 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
EventTurnDoneneeds an announcement lane that outlives the turn,because
launchRouteTaskannounces through the hub and the hub closes whenrunTurnreturns. Both want the same decision from whoever ownseventHub, sothey are one item.
Replication (deterministic, no model)
go test ./internal/session -run TestEveryReadingBesideTheWorkGoesThroughTheOneDoor -vand print the derived set:
readRemainsis not in it. Move thea.readRemains(ctx)call to the top ofrunTurnand 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
eventHubgains a post-turn announcement lane that survivesrunTurn's return— in which case both
readRemainsand the route judge become ordinary readingsstarted beside the work and taken at the end — or it does not, and
readRemainsis converted to
readBesideanyway so the law can see it, awaited throughtakeAtTheEndon the ending road it already sits on. The second is small andbuys the gate; the first is the actual speed win and is a design call.
Acceptance
readRemainsappears in the setreadingsFromderives, and movingits call to a road that carries on fails the law by name.
go test ./internal/session -run 'Remains|Reopen|Handover'green — thereading 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:
closeMarkAsideis gone andcheckpoint.gostill names it as the caller// is for an ENDING and nowhere else ([Agent.closeMarkAside] is the one caller).Agent.closeMarkAsidewas deleted answering the review — the settle is inlinedinto
runTurn's own exit defer, which is where the tree can see the ending — andthis doc link now points at nothing. A dead
[Agent.…]link is worse than aplain sentence: it reads as a claim about where to look.
Where
internal/session/checkpoint.go, onmarkAside.takeAtTheEnd. Searchable:is for an ENDING and nowhere else.The fix
Name the real caller:
runTurn's exit defer (loop.go). One line.Acceptance
go vet ./internal/session/and the doc-link check pass; a grep forcloseMarkAsideacross the tree returns nothing.5 —
Agent.PrefetchRecall(draft): typing time is free latencyThe 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 draftafter a short dwell, and
startTurnLockedadopts a prefetch whose cue stillmatches 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 islet go, which costs one cheap call on a person who rewrites their message.
Acceptance
presses enter, and the turn's
pacejournal row shows the recall alreadylanded at assembly (
recallin the decomposition, norecall:latenote).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 calltakes 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, and
briefing a workercovers it — but no boundis still no bound. Recommend
lane.RoleJudge.GiveUp()(90 s), matchingwriteHandoff'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.maybeCaption,taskname.goandjobname.goeach have their own channeldiscipline and each is a reading beside the work by every part of the
definition. Converting them to
readBesideis mechanical and deletes threeshapes. Acceptance: they appear in the set
readingsFromderives, andTestThereIsOneDoorForAReadingBesideTheWorkstill 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 in
internal/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 saywhether 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 ownwords, and
internal/manual/chat_test.go's probe table reaches it from "isanything 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