ClaudeSession: cancelled turn leaves type=result in stdout buffer, next caller reads stale result

[FidoCanCode]

Symptom

When a webhook preempts the worker's in-flight session turn, the webhook's reply can include the worker's in-progress output instead of (or concatenated with) its own.

Observed on 2026-04-14: PR #498 comment — rhencke posted "Test comment", fido's reply was:

Yep — already done and pushed. This task is complete. Nothing more to do here! 🐕

---

And for the PR comment reply:

*wags tail* Woof! Just testing things out too? I'm here and ready to fetch some type errors. 🐾

First paragraph is the worker's task-1 turn output ("task already complete"). Second paragraph is the webhook's actual reply-gen output. Both got posted as the reply.

Root cause

ClaudeSession.iter_events (kennel/claude.py:1139) polls _cancel.is_set() each 50ms and breaks early without consuming the rest of the stream:

while True:
    if self._cancel.is_set():
        log.debug("ClaudeSession: cancelled — exiting turn early")
        self._last_turn_cancelled = True
        break
    ...

The worker's turn may have already emitted assistant text (and possibly the type=result itself) to stdout before the poll sees _cancel. Those bytes stay in the stdout buffer. When the webhook thread then acquires the lock and calls consume_until_result, iter_events resumes reading from where the pipe is — picking up the worker's buffered type=result and returning its result field as the webhook's own turn output.

PR #494 removed the control_request + drain step from ClaudeSession.prompt (it hung on fresh subprocesses). That drain was what cleaned stdout between turns. Without it, leaked events now flow into the next caller.

Fix direction

On cancel, drain to the next turn boundary before releasing the lock. Inside iter_events, after seeing _cancel.is_set(), keep reading and discarding events until type=result / type=error / EOF — so the pipe is clean for the next caller. The _last_turn_cancelled flag is already set, so the worker still sees "cancelled, retry"; we're just making sure the stream is at a clean boundary by the time the lock releases.

Pseudocode:

if self._cancel.is_set():
    self._last_turn_cancelled = True
    # drain to turn boundary so next caller starts clean
    while True:
        ready, _, _ = self._selector([self._proc.stdout], [], [], _DRAIN_POLL)
        if not ready:
            break  # nothing pending — safe
        line = self._proc.stdout.readline()
        if not line:
            break  # EOF
        obj = json.loads(line.strip()) if line.strip() else None
        if obj and obj.get("type") in ("result", "error"):
            break  # turn boundary reached
    break

With a short drain deadline so we don't spin forever if claude never emits a boundary (fall back to sending a real control_request in that case, once we have one to handle fresh subprocesses safely).

[FidoCanCode]

Bidirectional

Buffer leak corrupts both callers, not just the webhook:

• Webhook → worker leftover: webhook's consume_until_result returns the worker's cancelled-but-already-emitted type=result. Webhook posts wrong body. (Observed on PR Add pyright to dev deps, gate CI + pre-commit on clean run (closes #466) #498.)
• Worker retry → webhook leftover: after the webhook releases the lock, the worker re-sends its prompt but its consume_until_result reads the webhook's type=result still sitting on stdout. Worker treats the webhook's reply text as its task answer — can lead to garbage commits, bogus task done claims, etc.

Drain-to-boundary-on-cancel fixes both: by the time the lock releases, stdout is at a clean turn boundary, and each caller's next turn reads only its own result.

[FidoCanCode]

Another concrete reproduction

PR #512 issue-comment 4247666607 — Fido posted the literal string:

Use originating PR comment for task titles and edit replies instead of reposting

That's not a reply to anything. It's the title-candidate output from _summarize_as_action_item leaking into the next session turn's consume_until_result — same stream-buffer mechanism documented above. The in-voice reply Fido actually composed ("Good catch — you're right, I was sniffing up the wrong tree…") went into the void.

Now happening across multiple session-turn sequences (reaction → triage → title → reply), not just the two-paragraph bleed. Every additional nudge multiplies the chance of leaking a prior turn's result into a later caller.

Drain-to-boundary on cancel (and on turn completion) is looking increasingly urgent.

[FidoCanCode]

Third repro, new direction: status nudge → title-gen

PR #512 work queue renders:

Build schema loader with type validation in confusio

…on a home PR. That string came from an earlier status nudge ({"status": "Building a schema loader with type validation over in confusio", "emoji": ":dog:"}) whose result was still buffered on stdout when _summarize_as_action_item fired its nudge for the new thread task's title. The title call's consume_until_result returned the status nudge's leftover instead of its own output.

So the leak pattern now spans status nudge → title-gen, not just triage → reply. With the session-nudge refactor in #506, set_status shares the same session as triage/title/reply, so every nudge is a new opportunity for a prior turn's result to bleed into a later caller.

Drain-to-boundary on cancel and on turn completion is necessary — the cancel path is one side, but a normal completed turn that returns without the consumer fully draining the stream is the other. Both paths need to leave stdout at a clean turn boundary before releasing the lock.