Skip to content

fix(chat-path): classify signal exits before auth fallback (#906)#909

Merged
oleksandr-korin merged 1 commit into
devfrom
feature/906-sigkill-signal-classifier-chat-path
May 21, 2026
Merged

fix(chat-path): classify signal exits before auth fallback (#906)#909
oleksandr-korin merged 1 commit into
devfrom
feature/906-sigkill-signal-classifier-chat-path

Conversation

@oleksandr-korin

Copy link
Copy Markdown
Contributor

Summary

Background

Issue #516 introduced _classify_signal_exit to fix the SIGKILL → "token expired" misclassification in the headless path (/api/task, scheduled tasks). The fix never landed on the parallel chat path (/api/chat, interactive chats), so the same misclassification persists there.

Two-codepath analysis from the issue's diagnosis:

Codepath File Called _classify_signal_exit first?
Scheduled tasks (/api/task) headless_executor.py:683 ✅ Yes (Issue #516)
Interactive chat (/api/chat) claude_code.py:453 No — bug

The misclassification has two compounding effects beyond bad operator-facing text:

  1. Operator misdirection — operators chase token regeneration when the actual cause is OOM / timeout / external kill (the issue cites a 72-hour wrong-fix cycle)
  2. SUB-003 auto-switch state corruption — the auto-switch trigger pattern matcher reads the error string and rotates subscriptions on phantom auth failures, burning the 2-hour skip-list slot

Changes

docker/base-image/agent_server/services/claude_code.py (+12)

  • Add _classify_signal_exit to the imports from error_classifier
  • Insert the signal-exit check before _diagnose_exit_failure in the chat handler's if return_code != 0 block. Pattern matches headless_executor.py:683-687 exactly.

tests/unit/test_signal_exit_classification.py (+52)

  • Add a structural regression test parametrized over both files (claude_code.py, headless_executor.py)
  • Test asserts _classify_signal_exit( appears before _diagnose_exit_failure( in each file's source. Order reversal in either file would re-introduce the misclassification — verified by manually flipping the order and confirming the test fails.

Out of scope (deferred to follow-ups)

  • Fix 2 (gate auto-switch on observed wire signal) — adding a structured error_class field to executions and refactoring the SUB-003 matcher to read it instead of the freeform error text. Larger refactor; filed for follow-up.
  • Fix 3 (cgroup OOM event reading) — upgrading SIGKILL classification to "container OOM observed" when memory.events.oom_kill incremented. Optional per the issue body.

Deployment

This change touches docker/base-image/agent_server/. Per CLAUDE.md note #7, base image rebuild + agent restart is required for the fix to take effect on running agents:

./scripts/deploy/build-base-image.sh
# Then restart agents (e.g. via /agent stop+start in UI, or fleet-wide restart)

Until the rebuild + restart, agents already running will continue to emit the old misclassified error.

Test plan

  • Unit tests: python3.11 -m pytest tests/unit/test_signal_exit_classification.py --confcutdir=tests/unit -q — 23 passed (21 existing + 2 new parametrized cases)
  • Regression test verified by temporarily reversing the call ordering in claude_code.py — test fails as expected, then restores
  • Manual verification post-deploy: trigger a SIGKILL on an agent's chat session (e.g. via cgroup OOM with small memory limit + memory-allocating skill) and observe the recorded error is "Execution terminated by SIGKILL after 0 tool calls / 0 turns…" instead of "Subscription token may be expired…"
  • Verify SUB-003 auto-switch no longer fires on the SIGKILL repro (subscription should remain healthy / unrotated)

Fixes #906

🤖 Generated with Claude Code

The chat path (claude_code.py) was missing the _classify_signal_exit call
that was added to the headless path (headless_executor.py) for Issue #516.
When a SIGKILL terminates the claude subprocess at 0 turns (cgroup OOM,
host SIGKILL, watchdog cancel), the chat handler would fall straight
into _diagnose_exit_failure, which returns "Subscription token may be
expired or revoked. Generate a new one with 'claude setup-token'." even
when no auth signal was observed.

This misclassification:
- Misleads operators into chasing token regeneration when the actual
  cause is OOM / timeout / external kill
- Pollutes the SUB-003 auto-switch trigger pattern matcher (which reads
  the error string), causing spurious subscription rotations on agents
  whose subscriptions are provably healthy
- Burns the auto-switch 2-hour skip-list slot on phantom auth failures

Fix mirrors the existing pattern in headless_executor.py:683 — call
_classify_signal_exit first, fall through to _diagnose_exit_failure
only for non-signal exits. No new logic; the classifier already
produces the honest "Execution terminated by SIGKILL after N tool
calls / N turns" message.

Adds a structural regression test (parametrized over both files) that
pins the call ordering — _classify_signal_exit must appear before
_diagnose_exit_failure in both call sites, otherwise the auth-fallback
heuristic re-introduces the misclassification.

Deployment: requires base image rebuild + agent restart for the fix
to take effect on running agents (per CLAUDE.md note #7).

Out of scope: Fix 2 (gate auto-switch on observed wire 401/403/429)
and Fix 3 (cgroup OOM event reading) — both flagged in #906 as
follow-up improvements.

Fixes #906

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@oleksandr-korin
oleksandr-korin merged commit f78c505 into dev May 21, 2026
12 checks passed
@oleksandr-korin
oleksandr-korin requested a review from vybe May 21, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant