Skip to content

runtime: a panicking slog handler on the supersede log loses queued events #143

Closed

Description

@ThomasK33

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.

What happens

  1. displaceLocked logs chat <label> superseded before it closes the displaced waiter's channel (runtime.go#L1422-L1425, at the fix(runtime): release what the prelude took when State or the logger panics #142 head).
  2. Under debounce, if the handler panics on that line, the newest event is lost as well as the superseded one.
  3. 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

Activity

  1. added
    needs-triageMaintainer needs to evaluate this issue
    bugSomething isn't working
    on Oct 30, 2026
  2. ThomasK33 commented on Oct 30, 2026

    @ThomasK33
    MemberAuthor

    Two related notes from the final review of #142 (merged), for the same triage:

    1. 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.
    2. A delivery whose prelude panicked leaves its dispatch span open, as main did before fix(runtime): release what the prelude took when State or the logger panics #142. So a lost event is also invisible in tracing. The fix(runtime): release what the prelude took when State or the logger panics #142 owner reported this, and the merge advisor read it as a gap against ADR 0015 invariant 3 ("No silent loss"). I did not reproduce it myself.

    Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh

  3. ThomasK33 commented on Oct 30, 2026

    @ThomasK33
    MemberAuthor

    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:

    1. 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.
    2. 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.
    3. 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

  4. added
    ready-for-agentFully specified, ready for an AFK agent
    and removed
    needs-triageMaintainer needs to evaluate this issue
    on Oct 30, 2026
  5. ThomasK33 commented on Oct 30, 2026

    @ThomasK33
    MemberAuthor

    Triage: I decided the contract question. Yes: a panicking logger must not lose events.

    Reasons:

    1. 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.
    2. 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

  6. ThomasK33 commented on Oct 30, 2026

    @ThomasK33
    MemberAuthor

    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

  7. added a commit that references this issue on Oct 30, 2026
    f437dee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingready-for-agentFully specified, ready for an AFK agent

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions