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
{{ message }}
Repository navigation
runtime: a panicking slog handler on the supersede log loses queued events #143
runtime: a panic in the app's slog handler on the supersede log line loses queued events.
#142 (for #139) makes the prelude release everything it took when State or the slog handler panics. Its independent verifier found one more panic effect that is not a leak. Some events are lost.
Under debounce, if the handler panics on that line, the newest event is lost as well as the superseded one.
Under the queue strategy, a panic on that line in takeQueueTurn (runtime.go#L1432) loses both the newer event and the waiter it replaced. The verifier's panic sweep counted A=0 and W=0 handler runs.
After #142, nothing leaks on this path. Locks, turn records and slots are all released, and the next delivery runs. Only these events are dropped.
Possible fix
Finish the state change (signal the displaced waiter, record the new waiter) before the log line, so a panicking logger cannot cut the handoff short. Add a test per strategy that panics on the supersede log and asserts which events still run.
Owner and trigger
The coder/chat maintainer triages this. Decide whether a panicking logger must not lose events, then fix it with the next change to queue or debounce coordination.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh
Two related notes from the final review of #142 (merged), for the same triage:
The queue strategy's conflicted registerPending path also loses the newest event when the logger panics there. The body above names only debounce for that path.
Triage: confirmed bug, ready-for-agent. I decided the open question.
Decision: yes. A panicking slog handler on a supersede log line must not lose events beyond the documented supersession. ADR 0015 invariant 3 ("No silent loss") already binds this: every admitted delivery reaches an observable terminal outcome. Supersession itself stays allowed, because ADR 0015 documents it. What is not allowed is that the newer event, or the waiter it replaced, silently never runs.
Work items, each with its own failing test first, behind a real httptest.Server:
Debounce: in displaceLocked, finish the handoff (signal the displaced waiter, record the new one) before the log line. Then show that the newest event runs when the log panics.
Queue: do the same in takeQueueTurn and on the conflicted registerPending path. Then show that the newer event runs and that the replaced waiter ends superseded, not lost.
Dispatch span: a delivery whose prelude panicked must end its dispatch span with a terminal outcome. Show the span ends after the panic.
Keep the #142 rule: release everything, and add no recover that hides the panic from net/http.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh
Triage: I decided the contract question. Yes: a panicking logger must not lose events.
Reasons:
This extends the runtime: a panic during the prelude still leaks the acceptance entry, the Thread Lock, and a burst slot #139 rule. That rule says a panic in State or the slog handler during the prelude must not leak what the prelude took. An event that the runtime already accepted, and under deferred dispatch already acknowledged, is part of that state. Slack and Linear do not redeliver an acknowledged event, so a lost event is gone for good.
The fix is cheap. Finish the handoff (signal the displaced waiter, record the new waiter) before the log line. A log line must never sit between two halves of a state change.
The fix in the issue is the plan, with one failing test per strategy (queue and debounce) first. Add one sentence to the ADR 0015 rule from #142: a panic in the slog handler cannot drop an event that a strategy keeps.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: medium
I corrected the cause in #145. Finishing the handoff before the log line is not enough. The supersede log ran on the newer delivery's own goroutine, after MarkEvent and before that delivery's run started. A panic there unwinds the only goroutine that would run the newer event, so a reorder inside displaceLocked or takeQueueTurn still loses it. With that reorder alone, debounce runs neither event, and under queue the orphan pending waiter trips the leaveQueueScope assert.
#145 moves the log line out of queueMu and into the dispatcher. Under DispatchDeferred it runs after the newer tail starts. Under DispatchSync it runs after the newer handler returns. The panic still aborts the request, but the newer event runs and the displaced waiter ends superseded. #145 also ends the dispatch span of a prelude that panicked with the error outcome.
Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh
Activity
fix(runtime): release what the prelude took when State or the logger …
ThomasK33 commentedon Oct 30, 2026
Two related notes from the final review of #142 (merged), for the same triage:
registerPendingpath also loses the newest event when the logger panics there. The body above names only debounce for that path.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhighThomasK33 commentedon Oct 30, 2026
Triage: confirmed bug,
ready-for-agent. I decided the open question.Decision: yes. A panicking
sloghandler on a supersede log line must not lose events beyond the documented supersession. ADR 0015 invariant 3 ("No silent loss") already binds this: every admitted delivery reaches an observable terminal outcome. Supersession itself stays allowed, because ADR 0015 documents it. What is not allowed is that the newer event, or the waiter it replaced, silently never runs.Work items, each with its own failing test first, behind a real
httptest.Server:displaceLocked, finish the handoff (signal the displaced waiter, record the new one) before the log line. Then show that the newest event runs when the log panics.takeQueueTurnand on the conflictedregisterPendingpath. Then show that the newer event runs and that the replaced waiter ends superseded, not lost.Keep the #142 rule: release everything, and add no
recoverthat hides the panic fromnet/http.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhighThomasK33 commentedon Oct 30, 2026
Triage: I decided the contract question. Yes: a panicking logger must not lose events.
Reasons:
Stateor thesloghandler during the prelude must not leak what the prelude took. An event that the runtime already accepted, and under deferred dispatch already acknowledged, is part of that state. Slack and Linear do not redeliver an acknowledged event, so a lost event is gone for good.The fix in the issue is the plan, with one failing test per strategy (queue and debounce) first. Add one sentence to the ADR 0015 rule from #142: a panic in the
sloghandler cannot drop an event that a strategy keeps.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:mediumThomasK33 commentedon Oct 30, 2026
I corrected the cause in #145. Finishing the handoff before the log line is not enough. The supersede log ran on the newer delivery's own goroutine, after
MarkEventand before that delivery's run started. A panic there unwinds the only goroutine that would run the newer event, so a reorder insidedisplaceLockedortakeQueueTurnstill loses it. With that reorder alone, debounce runs neither event, and under queue the orphan pending waiter trips theleaveQueueScopeassert.#145 moves the log line out of
queueMuand into the dispatcher. UnderDispatchDeferredit runs after the newer tail starts. UnderDispatchSyncit runs after the newer handler returns. The panic still aborts the request, but the newer event runs and the displaced waiter ends superseded. #145 also ends the dispatch span of a prelude that panicked with theerroroutcome.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:xhighfix(runtime): keep the newer event when the supersede log panics (#145)