Skip to content

test(runtime-image): probe gateway first-turn memory (diagnostic, do not merge) - #3820

Closed
freeqaz wants to merge 2 commits into
mainfrom
diag-2692
Closed

freeqaz wants to merge 2 commits into
mainfrom
diag-2692

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Problem

On a remote test VM, Image Runtime Startup 2 fails all three runtime-image-gateway-unclean-restart tests since #2982: the first Gateway's first chat turn ends with "fetch failed: other side closed" because Docker kills the Gateway container at its 2 GiB memory limit (OOMKilled: true). GitHub CI passes on the same commits. We need the same numbers from a CI runner to explain the difference.

Change

Diagnostic only. The first Gateway's turn records the container's memory.current, CPU throttling, memory limit and heap limit every 2 s, then the test fails with a one-line DIAG2692 summary in the job log.

Verification

None needed beyond the job log of Image Runtime Startup 2 on this PR.

Risks

None. Draft; it is closed without merging.

…not merge)

Temporary probe for a first-turn OOM kill seen only on a remote test VM:
record the Gateway container's memory, CPU throttling and heap limit around
the first chat turn and fail with a one-line summary, so the CI job log shows
the same numbers. Close without merging.
@clawsweeper

clawsweeper Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

The first probe showed the Gateway at its 2 GiB limit on CI without an OOM
kill; record host swap and the container's swap use to see where the
overflow goes.
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 27, 2026
@clawsweeper

clawsweeper Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

What this changes

This PR samples Gateway memory and swap during the first chat turn, then deliberately fails the test with a diagnostic summary.

Example: The Docker recovery case sends “Reply with READY.” to a Gateway limited to 2 GiB.

  • Before: The test checks the answer and proceeds to kill and restart the Gateway.
  • After: The test samples memory and swap every two seconds, reports container OOM status, and stops with DIAG2692.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful temporary instrumentation, with incomplete CPU accounting and no claim to resolve the underlying OOM.
Proof confidence 🦐 gold shrimp (3/6) This maintainer-authored, test-only diagnostic changes no production behavior; CI metadata confirms execution, but raw diagnostic logs could not be inspected because their storage host was blocked. The inspected managed-mode screenshot belongs to the linked earlier PR and does not demonstrate memory sampling; its video preparation failed because ffprobe was unavailable.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: A remote test VM loses the Gateway during its first chat turn while CI reportedly succeeds.
Reason: The collaborator has authorized a narrow diagnostic comparison; the PR deliberately offers no production fix and explicitly prohibits merging.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep this PR open for maintainer triage and preserve its explicit do-not-merge hold; it is a useful temporary investigation, with one missing diagnostic measurement.

Priority: P2
Reviewed head: f4b09a650a9385c150dba67d40fe34f92903794f

Before merge

  • Capture CPU throttling counters around the first turn (P3) - The diagnostic promises CPU-throttling measurements, but cpu.max contains only quota and period, and the sampler reads only memory and swap. A throttled and an unthrottled turn can therefore produce indistinguishable CPU evidence. Read cpu.stat before and after the turn, or include its throttling counters in the samples, so the CI-versus-VM comparison contains the advertised measurement.

Findings

  • [P3] Capture CPU throttling counters around the first turn — tests/integration/runtime-image-gateway-unclean-restart.test.mjs:224-227
Agent review details

How this fits together

The runtime-image integration suite starts the production Gateway wrapper against a stand-in model provider; this probe reads container accounting around its first HTTP chat request.

flowchart LR
  A[Image Runtime Startup 2] --> B[Startup scenario helper]
  B --> C[Gateway runtime container]
  C --> D[Stand-in model provider]
  E[Memory and swap sampler] --> C
  C --> F[DIAG2692 failure summary]
  E --> F
Loading

Technical review

Best possible solution:

Use the temporary probe to compare resource accounting, then let the maintainer retire it and carry any confirmed runtime repair in a separate PR.

Do we have a high-confidence way to reproduce the issue?

The remote-VM OOM is reported with a concrete first-turn trigger and OOMKilled result, but this review did not reproduce it or verify raw CI resource measurements.

Is this the best way to solve the issue?

Extending the existing container scenario is a proportionate diagnostic approach, provided it captures the claimed throttling data and remains outside main.

Full review comments:

  • [P3] Capture CPU throttling counters around the first turn — tests/integration/runtime-image-gateway-unclean-restart.test.mjs:224-227
    The diagnostic promises CPU-throttling measurements, but cpu.max contains only quota and period, and the sampler reads only memory and swap. A throttled and an unthrottled turn can therefore produce indistinguishable CPU evidence. Read cpu.stat before and after the turn, or include its throttling counters in the samples, so the CI-versus-VM comparison contains the advertised measurement.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 34d3848633a1.

Provenance checked

Testing

Proof path: none.

Security

None.

Evidence

What I checked:

Labels

Label changes:

  • add P2: This is a bounded runtime-test investigation without evidence of an emergency in deployed installations.
  • add merge-risk: 🚨 automation: Merging the unconditional diagnostic exception would permanently fail three selected runtime cases; the author explicitly accepts this only on a branch that must not merge.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

  • P2: This is a bounded runtime-test investigation without evidence of an emergency in deployed installations.
  • merge-risk: 🚨 automation: Merging the unconditional diagnostic exception would permanently fail three selected runtime cases; the author explicitly accepts this only on a branch that must not merge.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 27, 2026, 1:00 AM ET / 05:00 UTC.

@freeqaz

freeqaz commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Diagnostic probe done: CI runners have 11 GiB swap, and this test's Gateway container sits at its 2 GiB limit and swaps 0.25-1 GiB during its first turn. A test VM without swap OOM-kills it instead. Closing without merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant