fix: partial log stream + better error messages for fast-exit containers (FAR-122 follow-up) #6
Reference in New Issue
Block a user
Delete Branch "fix/far-122-partial-log-stream-and-error-message"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
readPodLogsone-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
!parsedpath usedstdout.split(…).find(Boolean)which returns the first non-empty line — the giant{"type":"system","subtype":"init",…}JSON blob. This produced errors like:Fix: skip lines whose
typeis"system"when building the error string. If only the init event was captured, emit a targeted message:Test plan
code -1: {init JSON}🤖 Generated with Claude Code
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
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.type:"system"events — correct fix for the "giant init JSON as error" case.initOnlyOutputreusingparsedStream.model(already populated by the init event) is elegant.Nits (non-blocking)
parseClaudeStreamJson(stdout)runs atexecute.ts:658just 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.readPodLogscall 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 callreadPodLogsagain and compare lengths. Harmless but an extra API round-trip. A boolean flag (alreadyDidOneShot) would skip it.initOnlyOutputmessage, (b) stdout with both init and a non-system error line → uses content line, would lock in the regression.firstContentLinefilter only excludestype:"system". If a future event type (e.g.assistantwith 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.