fix: partial log stream + better error messages for fast-exit containers (FAR-122 follow-up) #6

Merged
farhoodliquor-paperclip[bot] merged 2 commits from fix/far-122-partial-log-stream-and-error-message into master 2026-04-22 19:51:05 +00:00
farhoodliquor-paperclip[bot] commented 2026-04-22 19:33:46 +00:00 (Migrated from github.com)

Summary

Follow-up to #5 — two more issues surfaced after the 404 fix:

1. Partial log capture on fast container exit

When Claude exits very quickly (unsupported model, missing API key, etc.), the K8s follow-log stream may only capture the {"type":"system","subtype":"init",...} line before the connection drops. The follow stream is unreliable for already-terminated containers.

Fix: after the existing empty-stdout fallback, add a second fallback: if we have stdout but no result event, attempt a readPodLogs one-shot read. The one-shot path is more reliable for terminated containers and returns the full log from the beginning. If it returns more content, we use it.

2. Unreadable error message

The !parsed path used stdout.split(…).find(Boolean) which returns the first non-empty line — the giant {"type":"system","subtype":"init",…} JSON blob. This produced errors like:

Claude exited with code -1: {"type":"system","subtype":"init","cwd":"/paperclip/…","model":"MiniMax-M2.7",…}

Fix: skip lines whose type is "system" when building the error string. If only the init event was captured, emit a targeted message:

Claude started but did not produce a result (model: MiniMax-M2.7) — check API credentials, model support, and adapter config

Test plan

  • TypeScript typechecks pass
  • All 209 tests pass
  • Reproduce: agent with unsupported model or no API key — should now get a clear error instead of code -1: {init JSON}

🤖 Generated with Claude Code

## Summary Follow-up to #5 — two more issues surfaced after the 404 fix: **1. Partial log capture on fast container exit** When Claude exits very quickly (unsupported model, missing API key, etc.), the K8s follow-log stream may only capture the `{"type":"system","subtype":"init",...}` line before the connection drops. The follow stream is unreliable for already-terminated containers. Fix: after the existing empty-stdout fallback, add a *second* fallback: if we have stdout but no result event, attempt a `readPodLogs` one-shot read. The one-shot path is more reliable for terminated containers and returns the full log from the beginning. If it returns more content, we use it. **2. Unreadable error message** The `!parsed` path used `stdout.split(…).find(Boolean)` which returns the first non-empty line — the giant `{"type":"system","subtype":"init",…}` JSON blob. This produced errors like: > `Claude exited with code -1: {"type":"system","subtype":"init","cwd":"/paperclip/…","model":"MiniMax-M2.7",…}` Fix: skip lines whose `type` is `"system"` when building the error string. If only the init event was captured, emit a targeted message: > `Claude started but did not produce a result (model: MiniMax-M2.7) — check API credentials, model support, and adapter config` ## Test plan - [x] TypeScript typechecks pass - [x] All 209 tests pass - [ ] Reproduce: agent with unsupported model or no API key — should now get a clear error instead of `code -1: {init JSON}` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
farhoodliquor-paperclip[bot] (Migrated from github.com) reviewed 2026-04-22 19:39:56 +00:00
farhoodliquor-paperclip[bot] (Migrated from github.com) left a comment

Review — PR #6

Verdict: LGTM with minor nits. Both fixes are well-scoped and the logic is correct. Tests (209) pass, typecheck passes, CI green.

What's good

  • Partial-log fallbackif (stdout.trim() && !parseClaudeStreamJson(stdout).resultJson) is the right gate: only pay the extra K8s API call when output exists but is unterminated. The follow-stream race on fast container exit is real.
  • Error-message skip of type:"system" events — correct fix for the "giant init JSON as error" case. initOnlyOutput reusing parsedStream.model (already populated by the init event) is elegant.
  • Contained to one file, 40/-4, additive only — low risk.

Nits (non-blocking)

  1. Double parse of stream JSON. parseClaudeStreamJson(stdout) runs at execute.ts:658 just to read .resultJson, then again at line 737. Negligible in practice, but you could hoist it: const partialParsed = parseClaudeStreamJson(stdout); if (stdout.trim() && !partialParsed.resultJson) {...} and reuse below.
  2. Second readPodLogs call can re-fetch on the empty path. If the first fallback (!stdout.trim() at L645) already read full logs and they contain only the init event, the second fallback will call readPodLogs again and compare lengths. Harmless but an extra API round-trip. A boolean flag (alreadyDidOneShot) would skip it.
  3. No unit tests for the new branches. Test plan has one unchecked box (manual repro). A small execute.ts test covering (a) stdout with init-only → initOnlyOutput message, (b) stdout with both init and a non-system error line → uses content line, would lock in the regression.
  4. firstContentLine filter only excludes type:"system". If a future event type (e.g. assistant with a huge content block) ends up first without a result event, you'll still emit a big JSON error. Probably fine for now — the init-only case is the common one and the current filter handles it.

Merge recommendation

Ship it. The nits are pure polish and don't need to block. If you want to revisit later, #1 and #3 are the highest-leverage cleanups.

## Review — PR #6 **Verdict:** LGTM with minor nits. Both fixes are well-scoped and the logic is correct. Tests (209) pass, typecheck passes, CI green. ### What's good - **Partial-log fallback** — `if (stdout.trim() && !parseClaudeStreamJson(stdout).resultJson)` is the right gate: only pay the extra K8s API call when output exists but is unterminated. The follow-stream race on fast container exit is real. - **Error-message skip of `type:"system"` events** — correct fix for the "giant init JSON as error" case. `initOnlyOutput` reusing `parsedStream.model` (already populated by the init event) is elegant. - Contained to one file, 40/-4, additive only — low risk. ### Nits (non-blocking) 1. **Double parse of stream JSON.** `parseClaudeStreamJson(stdout)` runs at `execute.ts:658` just to read `.resultJson`, then again at line 737. Negligible in practice, but you could hoist it: `const partialParsed = parseClaudeStreamJson(stdout); if (stdout.trim() && !partialParsed.resultJson) {...}` and reuse below. 2. **Second `readPodLogs` call can re-fetch on the empty path.** If the first fallback (`!stdout.trim()` at L645) already read full logs and they contain only the init event, the second fallback will call `readPodLogs` again and compare lengths. Harmless but an extra API round-trip. A boolean flag (`alreadyDidOneShot`) would skip it. 3. **No unit tests for the new branches.** Test plan has one unchecked box (manual repro). A small execute.ts test covering (a) stdout with init-only → `initOnlyOutput` message, (b) stdout with both init and a non-system error line → uses content line, would lock in the regression. 4. **`firstContentLine` filter only excludes `type:"system"`.** If a future event type (e.g. `assistant` with a huge content block) ends up first without a result event, you'll still emit a big JSON error. Probably fine for now — the init-only case is the common one and the current filter handles it. ### Merge recommendation Ship it. The nits are pure polish and don't need to block. If you want to revisit later, #1 and #3 are the highest-leverage cleanups.
Sign in to join this conversation.