From 98f3821f91d3a95f91be20844e67f8e84b2db132 Mon Sep 17 00:00:00 2001 From: Gandalf the Greybeard Date: Thu, 23 Apr 2026 23:34:59 +0000 Subject: [PATCH] fix: address remaining minor code review findings (FAR-15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #9: match Paperclip container by name in k8s-client instead of trusting spec.containers[0], which could be a service-mesh sidecar - #11: key assistant-text dedup by (message.id, index) so legitimate duplicate content across turns isn't collapsed in the summary - #16: trim trailing hyphens from sanitized K8s names so truncation doesn't produce names ending in "-" Findings #5 (keepalive re-verify) and #6 (one-shot log dedup) were already addressed in the current code — verified during this review. #8 (orphan reattach behavior) requires a product decision on whether "new session wins" is intentional, so deferring. Co-Authored-By: Paperclip --- src/server/job-manifest.ts | 4 +++- src/server/k8s-client.ts | 7 ++++++- src/server/parse.ts | 21 +++++++++++++++------ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/server/job-manifest.ts b/src/server/job-manifest.ts index cc59383..f56c8b8 100644 --- a/src/server/job-manifest.ts +++ b/src/server/job-manifest.ts @@ -203,7 +203,9 @@ export interface JobBuildResult { } function sanitizeForK8sName(value: string, maxLen = 16): string { - return value.toLowerCase().replace(/[^a-z0-9-]/g, "").slice(0, maxLen); + // Trim trailing hyphens after slicing so names don't end with `-` when + // truncation lands on a hyphen boundary (finding #16, FAR-15). + return value.toLowerCase().replace(/[^a-z0-9-]/g, "").slice(0, maxLen).replace(/-+$/, ""); } /** diff --git a/src/server/k8s-client.ts b/src/server/k8s-client.ts index f2ea4ef..03c906d 100644 --- a/src/server/k8s-client.ts +++ b/src/server/k8s-client.ts @@ -106,7 +106,12 @@ export async function getSelfPodInfo(kubeconfigPath?: string): Promise c.name === "paperclip") ?? spec.containers[0]; if (!mainContainer?.image) { throw new Error(`claude_k8s: pod ${hostname} has no container image`); } diff --git a/src/server/parse.ts b/src/server/parse.ts index 12fed75..bd641ff 100644 --- a/src/server/parse.ts +++ b/src/server/parse.ts @@ -9,9 +9,12 @@ export function parseClaudeStreamJson(stdout: string) { let model = ""; let finalResult: Record | null = null; const assistantTexts: string[] = []; - // Belt-and-braces dedup: track seen text blocks to filter duplicates - // caused by log stream reconnects replaying overlapping windows. - const seenTexts = new Set(); + // Belt-and-braces dedup: key by (message.id, textIndex) so a session that + // legitimately emits the same text twice in different turns isn't collapsed + // (finding #11, FAR-15). The log-dedup filter handles reconnect overlaps + // at the line level; this guard only needs to protect against the same + // message block being parsed twice. + const seenBlocks = new Set(); for (const rawLine of stdout.split(/\r?\n/)) { const line = rawLine.trim(); @@ -29,14 +32,20 @@ export function parseClaudeStreamJson(stdout: string) { if (type === "assistant") { sessionId = asString(event.session_id, sessionId ?? "") || sessionId; const message = parseObject(event.message); + const messageId = asString(message.id, ""); const content = Array.isArray(message.content) ? message.content : []; - for (const entry of content) { + for (let i = 0; i < content.length; i++) { + const entry = content[i]; if (typeof entry !== "object" || entry === null || Array.isArray(entry)) continue; const block = entry as Record; if (asString(block.type, "") === "text") { const text = asString(block.text, ""); - if (text && !seenTexts.has(text)) { - seenTexts.add(text); + if (!text) continue; + // Prefer (messageId, index) when the message has an id; fall back + // to text content when it doesn't (legacy/partial events). + const key = messageId ? `${messageId}:${i}` : `text:${text}`; + if (!seenBlocks.has(key)) { + seenBlocks.add(key); assistantTexts.push(text); } }