fix(sandbox): accumulate canonical stdout from polled deltas, not final re-read (#1850)

[larryro]

Summary

Fixes #1850: sandbox/k8s stdout beyond kubelet log rotation returns a mid-stream window.

Root cause

In KubernetesBackend.execute(), after the polling loop ends, the code performed a final readPodLog() call to get the canonical stdout. When kubelet had rotated the container log (due to output exceeding containerLogMaxSize, default 10Mi), this final read returned only the content of the current rotated file — a window from the middle of the stream rather than the deterministic first-N bytes that the docker backend's drainAndCap produces.

The previous fix PR detected log shrinkage and set truncated.stdout = true, but the content itself was still wrong.

Fix

Accumulate canonical stdout incrementally from polled deltas in pollRunnerStdout, capped at stdoutMaxBytes, instead of re-reading the full log at the end:

• Added canonicalChunks: Buffer[] and canonicalByteCount accumulators inside the closure.
• Each delta (logs.slice(lastLogLen)) is appended to the accumulator up to the cap.
• On rotation detection (logs.length < lastLogLen): set logShrunk = true and reset lastLogLen = 0 so subsequent polls pick up post-rotation content from byte 0 of the new file.
• Replaced the final readPodLog() call with Buffer.concat(canonicalChunks).toString('utf8').

This gives deterministic first-N-bytes semantics regardless of when kubelet rotates the container log.

Tests

Two regression tests added to k8s-backend.test.ts:

1. Pre-rotation head bytes are preserved when rotation is detected in the final poll — truncated.stdout is set.
2. Post-rotation deltas are appended correctly when the canonical cap has not yet been reached.

Test plan

• bun test in services/sandbox — 327 pass, 0 fail
• bunx tsc --noEmit — no type errors
• bunx oxlint --type-aware — 0 warnings, 0 errors

Summary by CodeRabbit

• Bug Fixes

  • Improved runner stdout collection to reliably handle Kubernetes log rotation and truncation scenarios.
  • Enhanced incremental stdout accumulation with better tracking across container lifecycle events.

• Tests

  • Added comprehensive test coverage for stdout behavior during log rotation, truncation, and rotation recovery.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 41764580-557a-4300-bdff-24acc26805ec

📥 Commits

Reviewing files that changed from the base of the PR and between 5946d86 and ed300bb.

📒 Files selected for processing (2)

• services/sandbox/src/backend/kubernetes/k8s-backend.test.ts
• services/sandbox/src/backend/kubernetes/k8s-backend.ts

---

📝 Walkthrough

Walkthrough

KubernetesBackend.execute in k8s-backend.ts is refactored to accumulate canonical stdout incrementally during the polling loop rather than performing a single readPodLog call after the loop ends. New state (canonicalChunks, canonicalByteCount) is initialized before polling; pollRunnerStdout now appends each log delta to this buffer (capped at stdoutMaxBytes) and resets lastLogLen to 0 on kubelet log rotation (setting logShrunk). After the loop, stdout is derived from canonicalChunks and stdoutStreamTruncated from canonicalByteCount/logShrunk. In the test file, formatResultLine is imported, the readLog stub signature is updated to accept an optional container parameter, and a new describe block adds two tests covering head-preservation on rotation and post-rotation delta appending.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing stdout accumulation in Kubernetes backend by switching from final re-read to incremental delta accumulation.
Description check	✅ Passed	The PR description provides a comprehensive summary with root cause analysis, the fix approach, and test coverage. The test plan confirms all checks pass, though the description template requires a checklist that is not fully visible.
Linked Issues check	✅ Passed	The PR fully addresses the primary objective from issue #1850 by implementing incremental canonical stdout accumulation from polled deltas instead of final re-reads, ensuring deterministic first-N-bytes semantics regardless of kubelet log rotation.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the stdout accumulation issue: updates to k8s-backend.ts polling logic and corresponding test additions to k8s-backend.test.ts. No unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch tale/sx7c8tby6w25dt1kfw7abh4b918927pg

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[larryro]

Desk Review: fix(sandbox) — accumulate canonical stdout from polled deltas (#1850)

Verdict: READY TO MERGE

---

CI Status — ALL REQUIRED CHECKS PASS

Check	Result
Analyze (javascript-typescript)	✅ pass
Browser	✅ pass
Build controller / convex / db / sandbox / proxy / sandbox-runtime / sandbox-egress / platform	✅ pass
Smoke test	✅ pass
Scan sandbox	✅ pass
Validate images	✅ pass
Lint commits	✅ pass
Opengrep / Opengrep OSS	✅ pass
Unit / UI / Migrations	✅ pass
CodeRabbit	✅ pass
Smoke test (fork PR) / Docs container / Storybook / Trivy / Web container	skipping (expected for this PR type)

---

Test Run

bun test services/sandbox/src/
327 pass, 0 fail — 22 files [3.29s]

The two new tests in k8s-backend.test.ts ('pre-rotation head bytes are preserved; truncated.stdout is set' and 'post-rotation deltas are appended when cap has not been reached') both pass and correctly pin the fix's two key scenarios.

---

Correctness Review

Root cause correctly targeted. The old code's final readPodLog(limitBytes: stdoutMaxBytes) after the poll loop returned the rotated (mid-stream) file when kubelet had rotated the container log between the last poll and the final read — producing a window of a different position rather than the canonical head. The fix eliminates that re-read entirely and builds the canonical buffer from incremental polled deltas.

Happy path (no rotation): Deltas are computed correctly as logs.slice(lastLogLen) where lastLogLen tracks string-character positions consistently. Accumulation is correctly capped at stdoutMaxBytes via the remaining guard.

Rotation path: When logs.length < lastLogLen is detected (log shrunk), lastLogLen = 0 is now set (previously it was left at the old value and subsequent polls computed a negative or zero delta). This correctly causes the next poll to treat the full new file content as a fresh delta. Pre-rotation bytes in canonicalChunks are preserved. logShrunk = true is a one-way latch, correct even for multiple sequential rotations.

Cap path: Once canonicalByteCount >= stdoutMaxBytes, the remaining <= 0 guard prevents further accumulation. The limitBytes API parameter also caps the raw log read from the START of the log (confirmed in k8s-client.ts: "caps the response from the START (matches our 'truncate the tail' stdout policy)"), so the two caps are complementary.

Truncation flag: canonicalByteCount >= cfg.stdoutMaxBytes || logShrunk is correct — rotation always sets the flag because bytes between the last successful old-file poll and the rotation point may be unobserved.

limitBytes semantics confirmed: Delta computation (logs.slice(lastLogLen)) only makes sense if the API returns the FIRST N bytes (not a tail window). The k8s-client.ts comment explicitly confirms this.

---

Minor Observations (non-blocking)

1. Double allocation: Buffer.from(delta, 'utf8') is created twice per delta — once for scanner.onStdoutChunk?.(...) and once for accumulation as deltaBuf. Trivially fixable, but both buffers are bounded by stdoutMaxBytes and the overhead is negligible.

2. Slightly misleading test comment: In the first new test, "Runner log sequence: empty → pre-rotation → rotation" doesn't match the actual call sequence — call 1 returns 'AAAAA' (pre-rotation), not an empty probe. The behavior tested is correct; only the comment is off.

3. No dedicated test for the canonicalByteCount >= stdoutMaxBytes branch: The cap logic is simple and correct; with stdoutMaxBytes = 5 MiB in the test config, small test data never exercises this branch. Not blocking.

---

Summary

The fix is correct on all paths (happy, rotation, cap, multiple rotation). The new tests pin both primary scenarios. All 327 sandbox tests pass locally and all required CI checks are green. No blocking findings.