Skip to content

Treat Docker OOMKilled as an observation, not a status verdict (#151) - #152

Merged
konard merged 3 commits into
mainfrom
issue-151-50e985780da1
Aug 4, 2026
Merged

Treat Docker OOMKilled as an observation, not a status verdict (#151)#152
konard merged 3 commits into
mainfrom
issue-151-50e985780da1

Conversation

@konard

@konard konard commented Aug 4, 2026

Copy link
Copy Markdown
Member

Problem

Since oomKilled became terminal (fix for #148), $ --status <id> reported a still-running detached Docker session as status executed, exitCode 137, while docker inspect — in the very same polling round — reported State.Running: true. The container then kept working and exited 0.

Root cause: State.OOMKilled is a container-cgroup flag that is set when any process inside the cgroup is OOM-killed and is never cleared (moby/moby#47618). enrichDetachedStatus / enrich_detached_status had an early branch that treated record.oomKilled === true || dockerState.oomKilled === true as terminal evidence and synthesized exit code 137, ignoring State.Running and the container's real State.ExitCode.

Solution

OOMKilled is now an observation exposed alongside the status, never a verdict:

  • While docker inspect reports the container as running, the status stays executing, exitCode stays null, endTime stays null, and oomKilled: true is reported next to it.
  • Once the container has stopped, exitCode is the container's real .State.ExitCode (0 in the reported real-world case), never a synthesized 137.
  • 137 remains a last-resort fallback only when the container is gone (liveness unknown) and neither a stored exit code nor an Exit Code: log footer can be recovered — this preserves the $ --status can remain executing while oomKilled=true for detached docker sessions #148 invariant.
  • A log footer (Exit Code: N) still wins over every fallback.

Applied identically to both implementations:

  • js/src/lib/status-formatter.jsbackendExitCode() + resolveOomObservation() replace readBackendExitCode()/resolveOomExitCode(); exit ladder is footerExit ?? backendExitCode(dockerState) ?? (oomKilled ? 137 : -1).
  • rust/src/lib/status_formatter.rs — mirrored backend_exit_code() / resolve_oom_observation() and the same ladder.

Documentation for the new semantics added to README.md.

How to reproduce

Detached Docker session whose container had one process OOM-killed but is still alive; docker inspect -f '{{.State.Running}} {{.State.ExitCode}} {{.State.OOMKilled}}' returns true 0 true.

  • Before: --statusstatus executed, exitCode 137, endTime set.
  • After: --statusstatus executing, exitCode null, endTime null, oomKilled true, currentTime set.

The tests use a fake docker binary injected via PATH / START_DOCKER_BIN to script the inspect output, so no real Docker daemon is required.

Tests

js/test/session-name-status.js — new describe "Issue #151: OOMKilled is an observation, not a verdict":

  1. running container stays executing with oomKilled true (true 0 true);
  2. never synthesizes 137 for a running container (true 137 true);
  3. stopped container reports its real exit code 0 (false 0 true);
  4. log footer Exit Code: 0 beats the 137 fallback when the container is gone;
  5. listExecutions keeps executing for the same record.

The #148 test was rewritten as "makes an OOM-killed session terminal once its container is gone" plus "uses the container exit code, not 137, for a stopped OOM-flagged container", so the #148 behaviour stays covered.

rust/tests/status_formatter.rs mirrors all of these (docker_oom_killed_keeps_running_container_executing, docker_oom_killed_never_synthesizes_137_for_a_running_container, docker_oom_killed_uses_the_container_exit_code_when_it_stops, docker_oom_killed_prefers_the_log_footer_over_the_137_fallback, docker_oom_killed_is_terminal_once_the_container_is_gone).

Reproduction verified: stashing js/src/lib/status-formatter.js makes 3 of the 5 new JS tests fail (Expected: "executing" / Received: "executed"); stashing rust/src/lib/status_formatter.rs makes 2 of the new Rust tests fail. With the fix all pass.

Local checks: bun run check, bun run test (696 pass / 0 fail), check-test-parity (92.1%), validate-changeset, cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, cargo test --all-features, cargo test --doc — all green.

Unrelated CI fix included

js/test/isolation.js"runInDocker > should require image option" failed on the windows-latest runner (twice, including a clean re-run): the runner ships the docker CLI without a running daemon, so runInDocker returns "Docker is installed but not running…" instead of the missing-image message. The test now also skips when isDockerAvailable() is false, matching the existing "docker not installed" skip.

Out of scope

The issue's optional item ("Ideally also expose which process the kernel killed — memory.events oom_kill count, dmesg lines") is intentionally not implemented: reading dmesg needs host privileges and reading the cgroup's memory.events would require an extra docker exec on every status query. Happy to add it in a follow-up if wanted.

Release

  • js/.changeset/issue-151-oom-observation-not-verdict.md (patch)
  • rust/changelog.d/issue-151-oom-observation-not-verdict.md (bump: patch)

Fixes #151

Adding .gitkeep for PR creation (default mode).
This file will be removed when the task is complete.

Issue: #151
@konard konard self-assigned this Aug 4, 2026
A running container keeps status 'executing' with 'oomKilled true' exposed
alongside; a stopped container reports its real .State.ExitCode; 137 is only
used when the container is gone and neither a log footer nor an exit code can
be recovered (preserving the #148 invariant).
@konard konard changed the title [WIP] $ --status reports executed / exitCode 137 from the container OOM flag while docker inspect still says running (session later exits 0) Treat Docker OOMKilled as an observation, not a status verdict (#151) Aug 4, 2026
@konard
konard marked this pull request as ready for review August 4, 2026 04:00
Windows CI runners ship the docker CLI without a running daemon, so
runInDocker returns the 'installed but not running' message instead of the
missing-image message and the assertion failed spuriously.
@konard

konard commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Working session summary

No comments or reviews pending. Work is complete.

PR #152#152 (ready for review, both CI workflows green on 1b9f920)

What was done:

  • Root cause: enrichDetachedStatus / enrich_detached_status (added in the $ --status can remain executing while oomKilled=true for detached docker sessions #148 fix) had an early branch treating oomKilled === true as terminal evidence and synthesizing exit code 137, ignoring State.Running and the container's real State.ExitCode. State.OOMKilled is a cgroup flag set when any process in the cgroup is OOM-killed and never cleared.
  • Fix (identical in js/src/lib/status-formatter.js and rust/src/lib/status_formatter.rs): oomKilled is now an observation exposed alongside the status, never a verdict. Running container → stays executing (exitCode/endTime null, oomKilled true); stopped container → real .State.ExitCode; 137 only when the container is gone and neither an exit code nor an Exit Code: log footer is recoverable — preserving the $ --status can remain executing while oomKilled=true for detached docker sessions #148 invariant.
  • Tests: 5 new JS tests (Issue #151: OOMKilled is an observation, not a verdict) + 4 new Rust tests, plus rewritten $ --status can remain executing while oomKilled=true for detached docker sessions #148 coverage, all driven by a fake docker binary. Verified as reproductions: stashing the JS fix fails 3 of them (Expected: "executing" / Received: "executed"), stashing the Rust fix fails 2.
  • Docs + release: README section on --status semantics; changeset (patch) and Rust changelog fragment (bump: patch).
  • Included CI fix: js/test/isolation.js "should require image option" failed on windows-latest (twice, including a clean re-run) because that runner has the docker CLI without a running daemon; the test now also skips when isDockerAvailable() is false.
  • Deliberately out of scope: the issue's optional "expose which process the kernel killed" (memory.events oom_kill, dmesg) — it needs host privileges or an extra docker exec per status query; noted in the PR description as a possible follow-up.

This summary was automatically extracted from the AI working session output.

@konard

konard commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost: $5.128238

📊 Context and tokens usage:

Claude Opus 5: (2 sub-sessions)

  1. 116.3K / 1M (12%) input tokens, 26.1K / 128K (20%) output tokens
  2. 55.1K / 1M (6%) input tokens, 12.0K / 128K (9%) output tokens

Total: (2.2K new + 136.6K cache writes + 5.3M cache reads) input tokens, 45.0K output tokens, $5.128238 cost

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: opus (claude-opus-5)
  • Thinking level: medium (~15999 tokens)
  • Model: Claude Opus 5 (claude-opus-5)

📎 Log file uploaded as Gist (2625KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard
konard merged commit c2a86c7 into main Aug 4, 2026
25 checks passed
@konard

konard commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

🎉 Auto-merged

This pull request has been automatically merged by hive-mind.

  • All CI checks have passed

Auto-merged by hive-mind with --auto-merge flag

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.

$ --status reports executed / exitCode 137 from the container OOM flag while docker inspect still says running (session later exits 0)

1 participant