Skip to content

feat(chat-runtime): Claude harness adapter with recorded wire fixtures - #6193

Merged
saddlepaddle merged 2 commits into
mainfrom
chat-claude-adapter
Aug 5, 2026
Merged

saddlepaddle merged 2 commits into
mainfrom
chat-claude-adapter

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented Aug 5, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #6167. ClaudeTranslator (pure SDKMessage→AdapterEvent core) + ClaudeAdapter wiring @anthropic-ai/claude-agent-sdk 0.3.201 (pinned exact, injected query — tests never spawn a process). Six real recorded scenarios as fixtures (text, bash, edit approved/declined, abort mid-tool, subagent); 69 tests + a live smoke (passed 14.3s, kept skipped in CI). Includes 11 wire-truth findings vs the mapping doc — notably allowedTools silently bypassing canUseTool, updatedInput required-at-runtime on accept, abort as thrown iterator error, and the Agent/Task tool naming — doc amendments to chat-harness-adapters.md follow separately.

https://claude.ai/code/session_01XRFCEPsfmJHu2SenDKYJ2h


Summary by cubic

Adds a Claude harness adapter and translator for chat-runtime, wired to @anthropic-ai/claude-agent-sdk@0.3.201 and validated with recorded wire fixtures. Fixes cancel/interrupt behavior, delta targeting, and tool mapping to match the SDK stream.

  • New Features

    • Adapter streams SDK messages into AdapterEvents via ClaudeTranslator using an injected query (no external process in tests/CI).
    • Tool-use rendering via mapToolUse with kind, titles, locations, and concise outcome text.
    • Six recorded JSONL fixtures and a recording script; 69 tests + a live smoke test (skipped in CI).
  • Bug Fixes

    • Cancel/interrupt now aborts the session and settles the turn cleanly.
    • Corrected delta targeting and tool mapping (e.g., Write→edit, BashOutput→execute) to align with real SDK behavior.

Written for commit 5256c33. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added Claude integration for streaming responses, tool usage, approvals, cancellations, interruptions, edits, Bash commands, and subagent activity.
    • Added translated events with tool details, file locations, usage, costs, errors, and turn lifecycle updates.
    • Added support for resuming Claude sessions and configurable session behavior.
  • Tests

    • Added comprehensive coverage for Claude conversations, tool mapping, streaming, cancellation, approvals, interruptions, and subagent workflows.
    • Added representative scenarios for automated integration testing.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 384bf5e2-abb9-488d-811e-846b96a434ee

📥 Commits

Reviewing files that changed from the base of the PR and between b9f103e and 5256c33.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (20)
  • packages/chat-runtime/package.json
  • packages/chat-runtime/scripts/recordClaudeFixtures.ts
  • packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.live.test.ts
  • packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.test.ts
  • packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts
  • packages/chat-runtime/src/harness/claude/claudeAdapter/index.ts
  • packages/chat-runtime/src/harness/claude/fixtures/abort-mid-tool.jsonl
  • packages/chat-runtime/src/harness/claude/fixtures/bash-run.jsonl
  • packages/chat-runtime/src/harness/claude/fixtures/edit-approved.jsonl
  • packages/chat-runtime/src/harness/claude/fixtures/edit-declined.jsonl
  • packages/chat-runtime/src/harness/claude/fixtures/index.ts
  • packages/chat-runtime/src/harness/claude/fixtures/subagent-task.jsonl
  • packages/chat-runtime/src/harness/claude/fixtures/text-reply.jsonl
  • packages/chat-runtime/src/harness/claude/index.ts
  • packages/chat-runtime/src/harness/claude/mapToolUse/index.ts
  • packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.test.ts
  • packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts
  • packages/chat-runtime/src/harness/claude/translateStream/index.ts
  • packages/chat-runtime/src/harness/claude/translateStream/translateStream.test.ts
  • packages/chat-runtime/src/harness/claude/translateStream/translateStream.ts

📝 Walkthrough

Walkthrough

Adds Claude Agent SDK integration for chat runtime sessions. The change introduces tool-use mapping, stream translation, prompt and approval handling, cancellation and disposal, fixture recording, JSONL fixtures, and adapter tests.

Changes

Claude harness

Layer / File(s) Summary
Fixture recording and corpus
packages/chat-runtime/package.json, packages/chat-runtime/scripts/recordClaudeFixtures.ts, packages/chat-runtime/src/harness/claude/fixtures/*
Adds the Claude SDK dependency, configurable fixture recording, fixture loading, path redaction, and JSONL scenarios for text, Bash, edits, interruption, and subagents.
Tool-use mapping
packages/chat-runtime/src/harness/claude/mapToolUse/*
Maps Claude tools to protocol kinds, titles, content, diffs, terminal output, subagent reports, and file locations.
Claude stream translation
packages/chat-runtime/src/harness/claude/translateStream/*
Translates Claude messages and streaming events into session, turn, item, delta, notice, and usage events. It tracks tools, interruptions, declines, parent relationships, and costs.
Claude session adapter
packages/chat-runtime/src/harness/claude/claudeAdapter/*, packages/chat-runtime/src/harness/claude/index.ts
Adds prompt streaming, event buffering, tool approvals, cancellation, session modes, disposal, public exports, simulated-session tests, and a skipped live integration test.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ClaudeAdapter
  participant ClaudeAgentSDK
  participant ClaudeTranslator
  participant HarnessEventStream
  ClaudeAdapter->>ClaudeAgentSDK: Start query with prompt stream and approval callback
  ClaudeAgentSDK-->>ClaudeAdapter: Emit Claude session messages
  ClaudeAdapter->>ClaudeTranslator: Translate each message
  ClaudeTranslator-->>HarnessEventStream: Emit translated adapter events
  HarnessEventStream-->>ClaudeAdapter: Deliver events to consumers
  ClaudeAdapter->>ClaudeAgentSDK: Interrupt or abort the session
Loading

Possibly related PRs

✨ 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 chat-claude-adapter

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

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) ✅ View Branch
Vercel API (Vercel) ✅ Open Preview
Vercel Web (Vercel) ✅ Open Preview
Vercel Marketing (Vercel) ✅ Open Preview
Vercel Admin (Vercel) ✅ Open Preview
Vercel Docs (Vercel) ✅ Open Preview

Preview updates automatically with new commits

@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR introduces a Claude Agent SDK harness adapter, translating Claude wire messages and tool activity into chat-runtime protocol events using recorded fixtures.

  • Adds adapter lifecycle, approval, cancellation, and streaming-query integration.
  • Adds translation and tool-rendering helpers for messages, reasoning, tool calls, results, usage, and diffs.
  • Pins the Claude Agent SDK and adds fixture recording, recorded scenarios, unit coverage, and a skipped live smoke test.

Confidence Score: 4/5

The PR should not merge until cancellation can preserve subsequent turns and attachment prompts reach Claude with usable content.

The adapter permanently aborts its only query stream when one turn is canceled even though LiveSession can immediately reuse it for queued work, and its prompt conversion silently strips all usable attachment information.

Files Needing Attention: packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts

Important Files Changed

Filename Overview
packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts Adds the SDK lifecycle and approval bridge, but cancellation permanently disables later turns and prompt conversion drops attachment data.
packages/chat-runtime/src/harness/claude/translateStream/translateStream.ts Adds stateful conversion of Claude stream envelopes into session, turn, item, delta, and tool-call events with broad recorded-fixture coverage.
packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts Adds tool categorization, titles, locations, terminal output, and edit/write diff rendering.
packages/chat-runtime/scripts/recordClaudeFixtures.ts Adds an isolated recorder for representative Claude wire scenarios, including approval, denial, abort, and subagent activity.
packages/chat-runtime/package.json Adds an exactly pinned Claude Agent SDK runtime dependency.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Session as LiveSession
  participant Adapter as ClaudeAdapter
  participant SDK as Claude Agent SDK
  participant Translator as ClaudeTranslator

  Client->>Session: prompt(UserContent)
  Session->>Adapter: prompt(content)
  Adapter->>SDK: streamed user message
  SDK-->>Adapter: SDKMessage stream
  Adapter->>Translator: translate(message)
  Translator-->>Session: AdapterEvents
  SDK->>Adapter: canUseTool(...)
  Adapter-->>Session: approval_request
  Client->>Session: approval decision
  Session->>Adapter: respondToApproval(...)
  Adapter-->>SDK: allow or deny
  Client->>Session: cancelTurn()
  Session->>Adapter: cancelTurn()
  Adapter->>SDK: abort()
Loading
Prompt To Fix All With AI
### Issue 1
packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts:186-188
**Cancellation permanently stops the adapter**

When a prompt is queued and the current turn is canceled, `cancelTurn()` permanently aborts the adapter's only SDK stream. `LiveSession` then delivers the queued prompt to the same adapter, but its pump has already ended, so the new turn never starts and the session stalls.

### Issue 2
packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts:127-131
**Attachment content is discarded**

When a prompt contains an attachment, `promptText()` reduces it to a name such as `[report.pdf]` and drops its ID and contents. Claude therefore cannot inspect the attachment or answer questions about it despite `HarnessAdapter.prompt()` accepting attachment content.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat(chat-runtime): Claude adapter with ..." | Re-trigger Greptile

Comment on lines +186 to +188
cancelTurn(): void {
this.abortController.abort();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cancellation permanently stops the adapter

When a prompt is queued and the current turn is canceled, cancelTurn() permanently aborts the adapter's only SDK stream. LiveSession then delivers the queued prompt to the same adapter, but its pump has already ended, so the new turn never starts and the session stalls.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts
Line: 186-188

Comment:
**Cancellation permanently stops the adapter**

When a prompt is queued and the current turn is canceled, `cancelTurn()` permanently aborts the adapter's only SDK stream. `LiveSession` then delivers the queued prompt to the same adapter, but its pump has already ended, so the new turn never starts and the session stalls.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment on lines +127 to +131

function promptText(content: UserContent[]): string {
return content
.map((part) => (part.type === "text" ? part.text : `[${part.name}]`))
.join("\n");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Attachment content is discarded

When a prompt contains an attachment, promptText() reduces it to a name such as [report.pdf] and drops its ID and contents. Claude therefore cannot inspect the attachment or answer questions about it despite HarnessAdapter.prompt() accepting attachment content.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts
Line: 127-131

Comment:
**Attachment content is discarded**

When a prompt contains an attachment, `promptText()` reduces it to a name such as `[report.pdf]` and drops its ID and contents. Claude therefore cannot inspect the attachment or answer questions about it despite `HarnessAdapter.prompt()` accepting attachment content.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 issues found across 21 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts">

<violation number="1" location="packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts:44">
P3: Titles lose directory context for workspace paths whose first segment starts with `..`, because this containment check rejects `..cache/...` as if it were parent traversal. Check for `..` or a `..` path segment using the platform separator instead of testing the prefix alone.</violation>

<violation number="2" location="packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts:94">
P2: NotebookRead, BashOutput, and KillShell produce raw SDK names as their UI titles instead of human-readable summaries because they fall through this default. Add explicit title cases for every recognized tool so the tool row does not expose internal names.</violation>

<violation number="3" location="packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts:163">
P2: A failed Bash call can render with an empty terminal output even when Claude returned an error message; when the raw result object has no stdout/stderr, this branch discards `modelFacingText`. Fall back to the model-facing text when the stream output is empty so the settled tool row preserves the failure reason.</violation>
</file>

<file name="packages/chat-runtime/scripts/recordClaudeFixtures.ts">

<violation number="1" location="packages/chat-runtime/scripts/recordClaudeFixtures.ts:106">
P3: Each recording run leaks one temporary workspace per scenario; clean the workspace in a `finally` block after the query has stopped, including failure paths.</violation>

<violation number="2" location="packages/chat-runtime/scripts/recordClaudeFixtures.ts:132">
P1: The abort scenario hides unrelated SDK failures and can leave a corrupt fixture while reporting success; only accept the exception when this recorder's `AbortController` is actually aborted.</violation>

<violation number="3" location="packages/chat-runtime/scripts/recordClaudeFixtures.ts:140">
P2: The abort fixture can be recorded without an abort when Claude finishes before the first Bash tool use; validate that the controller was aborted before writing this scenario's fixture.</violation>

<violation number="4" location="packages/chat-runtime/scripts/recordClaudeFixtures.ts:157">
P3: A misspelled scenario name silently does nothing and leaves existing fixtures unchanged; reject unknown names before starting the recording loop.</violation>
</file>

<file name="packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts">

<violation number="1" location="packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.ts:130">
P2: Non-text `UserContent` entries are converted to `[name]` here, which silently strips attachment payloads before sending the prompt. That prevents Claude from using attachment contents; either pass attachment data through or fail fast for unsupported attachment types instead of degrading silently.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Host as Host Application
    participant Adapter as ClaudeAdapter
    participant Queue as PromptQueue
    participant SDK as @anthropic-ai/claude-agent-sdk
    participant Translator as ClaudeTranslator
    participant Mapper as mapToolUse
    participant EventQ as EventQueue
    participant Test as Test Fixture (JSONL)

    Note over Host,EventQ: NEW: Claude Harness Adapter Flow

    Host->>Adapter: start({ cwd, modelId })
    Adapter->>Translator: new ClaudeTranslator({ cwd, now, mintId })
    Adapter->>SDK: query({ prompt: Queue.stream(), options })
    Note over Adapter,SDK: NEW: Injected query function—no process spawn
    SDK-->>Adapter: AsyncIterable<SDKMessage>

    par Host sends prompt
        Host->>Adapter: prompt([UserContent])
        Adapter->>Queue: push(user message)
        Queue-->>SDK: yield SDK user message
    and SDK streams messages
        loop for each SDK message
            SDK-->>Adapter: SDKMessage (system, assistant, stream_event, result)
            Adapter->>Translator: translate(message)
            Translator->>Translator: dispatch by type/subtype
            alt system init
                Translator-->>Adapter: { kind: session, session: { harness, modelId } }
            else assistant content blocks
                Translator->>Mapper: toolKindFor(name) / titleFor(use, cwd)
                Mapper-->>Translator: ToolKind / title
                alt text block
                    Translator-->>Adapter: { kind: item, item: agent_message }
                else thinking block
                    Translator-->>Adapter: { kind: item, item: reasoning }
                else tool_use block
                    Translator->>Translator: track tool (pending)
                    Translator-->>Adapter: { kind: item, item: tool_call (running) }
                end
            else stream_event content_block_delta
                alt text/thinking delta
                    Translator-->>Adapter: { kind: delta, delta: { type: text, append } }
                else input_json_delta
                    Translator-->>Adapter: { kind: delta, delta: { type: tool_input, append } }
                end
            else result
                alt is_error
                    Translator-->>Adapter: { kind: turn, turn: { status: running } } (initial)
                    Note over Translator,EventQ: NEW: abort as thrown iterator error
                    Adapter->>Translator: interrupt(String(error))
                    Translator->>Mapper: contentFor(tool, null)
                    Translator-->>Adapter: { kind: item, item: tool_call (canceled) } for each pending
                    Translator-->>Adapter: { kind: turn, turn: { status: interrupted } }
                else success
                    Translator->>Translator: build usage + cost
                    Translator-->>Adapter: { kind: turn, turn: { status: completed } }
                end
                Note over Translator,EventQ: NEW: updatedInput required at runtime on accept
            end
            Adapter->>EventQ: push(AdapterEvent)
        end
    and SDK asks for permission
        SDK->>Adapter: canUseTool(toolName, input, { toolUseID })
        Note over Adapter,SDK: NEW: allowedTools bypasses canUseTool (wire finding)
        Adapter->>Adapter: requestApproval() – create pending approval
        Adapter->>EventQ: push({ kind: item, item: approval_request (pending) })
        Host->>Adapter: respondToApproval(id, decision)
        alt accept / accept_for_session
            Adapter->>Adapter: settle({ behavior: allow, updatedInput: input })
            Adapter-->>SDK: PermissionResult (allow)
        else decline
            Adapter->>Translator: markDeclined(toolUseId)
            Adapter->>Adapter: settle({ behavior: deny, ... })
            Adapter-->>SDK: PermissionResult (deny)
        else cancel
            Adapter->>Adapter: cancelTurn() -> abortController.abort()
            SDK-->>Adapter: throws – caught in run()
        end
        Adapter->>EventQ: push({ kind: item, item: approval_request (answered) })
    end

    Note over EventQ,Test: Test Mode (injected fixture replay)
    Test->>Translator: translate(fixture lines)
    Translator->>Mapper: contentFor, titleFor, etc.
    Translator-->>Test: AdapterEvent[]

    Note over Translator,Mapper: NEW: Agent/Task tool naming (wire finding)
    Note over Translator,EventQ: NEW: 11 wire-truth findings logged for doc updates
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

}
}
} catch (error) {
if (!scenario.abortAfterToolUse) throw error;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The abort scenario hides unrelated SDK failures and can leave a corrupt fixture while reporting success; only accept the exception when this recorder's AbortController is actually aborted.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/scripts/recordClaudeFixtures.ts, line 132:

<comment>The abort scenario hides unrelated SDK failures and can leave a corrupt fixture while reporting success; only accept the exception when this recorder's `AbortController` is actually aborted.</comment>

<file context>
@@ -0,0 +1,166 @@
+			}
+		}
+	} catch (error) {
+		if (!scenario.abortAfterToolUse) throw error;
+		messages.push({
+			type: "system",
</file context>

Comment thread packages/chat-runtime/package.json Resolved
text(use.input.subject) ??
"Subagent task"
);
default:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: NotebookRead, BashOutput, and KillShell produce raw SDK names as their UI titles instead of human-readable summaries because they fall through this default. Add explicit title cases for every recognized tool so the tool row does not expose internal names.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts, line 94:

<comment>NotebookRead, BashOutput, and KillShell produce raw SDK names as their UI titles instead of human-readable summaries because they fall through this default. Add explicit title cases for every recognized tool so the tool row does not expose internal names.</comment>

<file context>
@@ -0,0 +1,194 @@
+				text(use.input.subject) ??
+				"Subagent task"
+			);
+		default:
+			return use.name;
+	}
</file context>


const stdout = text(output?.stdout) ?? "";
const stderr = text(output?.stderr) ?? "";
const combined = output

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A failed Bash call can render with an empty terminal output even when Claude returned an error message; when the raw result object has no stdout/stderr, this branch discards modelFacingText. Fall back to the model-facing text when the stream output is empty so the settled tool row preserves the failure reason.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts, line 163:

<comment>A failed Bash call can render with an empty terminal output even when Claude returned an error message; when the raw result object has no stdout/stderr, this branch discards `modelFacingText`. Fall back to the model-facing text when the stream output is empty so the settled tool row preserves the failure reason.</comment>

<file context>
@@ -0,0 +1,194 @@
+
+	const stdout = text(output?.stdout) ?? "";
+	const stderr = text(output?.stderr) ?? "";
+	const combined = output
+		? [stdout, stderr].filter((part) => part.length > 0).join("\n")
+		: (outcome?.modelFacingText ?? "");
</file context>

if (!raw) return null;
if (!isAbsolute(raw)) return raw;
const inside = relative(cwd, raw);
return inside && !inside.startsWith("..") ? inside : basename(raw);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Titles lose directory context for workspace paths whose first segment starts with .., because this containment check rejects ..cache/... as if it were parent traversal. Check for .. or a .. path segment using the platform separator instead of testing the prefix alone.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts, line 44:

<comment>Titles lose directory context for workspace paths whose first segment starts with `..`, because this containment check rejects `..cache/...` as if it were parent traversal. Check for `..` or a `..` path segment using the platform separator instead of testing the prefix alone.</comment>

<file context>
@@ -0,0 +1,194 @@
+	if (!raw) return null;
+	if (!isAbsolute(raw)) return raw;
+	const inside = relative(cwd, raw);
+	return inside && !inside.startsWith("..") ? inside : basename(raw);
+}
+
</file context>

Comment on lines +157 to +159
const selected = only.length
? SCENARIOS.filter((scenario) => only.includes(scenario.name))
: SCENARIOS;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: A misspelled scenario name silently does nothing and leaves existing fixtures unchanged; reject unknown names before starting the recording loop.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/scripts/recordClaudeFixtures.ts, line 157:

<comment>A misspelled scenario name silently does nothing and leaves existing fixtures unchanged; reject unknown names before starting the recording loop.</comment>

<file context>
@@ -0,0 +1,166 @@
+async function main(): Promise<void> {
+	mkdirSync(FIXTURES_DIR, { recursive: true });
+	const only = process.argv.slice(2);
+	const selected = only.length
+		? SCENARIOS.filter((scenario) => only.includes(scenario.name))
+		: SCENARIOS;
</file context>
Suggested change
const selected = only.length
? SCENARIOS.filter((scenario) => only.includes(scenario.name))
: SCENARIOS;
const unknown = only.filter(
(name) => !SCENARIOS.some((scenario) => scenario.name === name),
);
if (unknown.length > 0) {
throw new Error(`Unknown scenario(s): ${unknown.join(", ")}`);
}
const selected = only.length
? SCENARIOS.filter((scenario) => only.includes(scenario.name))
: SCENARIOS;

}

async function record(scenario: Scenario): Promise<void> {
const cwd = mkdtempSync(join(tmpdir(), `claude-fixture-${scenario.name}-`));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Each recording run leaks one temporary workspace per scenario; clean the workspace in a finally block after the query has stopped, including failure paths.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/scripts/recordClaudeFixtures.ts, line 106:

<comment>Each recording run leaks one temporary workspace per scenario; clean the workspace in a `finally` block after the query has stopped, including failure paths.</comment>

<file context>
@@ -0,0 +1,166 @@
+}
+
+async function record(scenario: Scenario): Promise<void> {
+	const cwd = mkdtempSync(join(tmpdir(), `claude-fixture-${scenario.name}-`));
+	scenario.seed?.(cwd);
+
</file context>

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 15 files (changes from recent commits).

Not reviewed (too large): packages/chat-runtime/src/harness/claude/fixtures/subagent-task.jsonl (~331 lines), packages/chat-runtime/src/harness/claude/fixtures/edit-declined.jsonl (~287 lines), packages/chat-runtime/src/harness/claude/fixtures/edit-approved.jsonl (~258 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.test.ts">

<violation number="1" location="packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.test.ts:131">
P2: The dispose test doesn't verify that dispose() actually resolves. Because abort fires synchronously inside dispose(), the `aborted()` assertion holds even if dispose hangs at `await this.pump`; the Promise.race with a 50ms timeout silently passes either way. Assert on the race outcome (e.g. both settled in Promise.all and that disposal resolved) so a regression that leaves dispose pending is caught rather than missed.</violation>
</file>

<file name="packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts">

<violation number="1" location="packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts:169">
P3: subagentContent re-implements the same `rawOutput` extraction block (ducked through `typeof ... object` and cast to `Record<string, unknown>`/`null`) that `editContent` and `terminalContent` already contain. That's now three copies of the identical idiom in one file, so a future change to how outcomes are parsed (or how rawOutput is structured) has to be made in three places and can drift. A small shared helper like `function rawOutputOf(outcome): Record<string, unknown> | null` that centralizes this extraction would keep the three call sites consistent without changing behavior.</violation>
</file>

<file name="packages/chat-runtime/scripts/recordClaudeFixtures.ts">

<violation number="1" location="packages/chat-runtime/scripts/recordClaudeFixtures.ts:107">
P2: The committed fixtures still expose recording-machine path data and are not reproducible: `system.init.cwd` remains `/private/workspace`, while `memory_paths.auto` retains random `mkdtempSync` names such as `...-3urBTa`. `redact` only replaces the raw temp `cwd`/home strings and a macOS-specific path pattern, so it misses SDK-derived path fields (and other platform temp paths). Normalizing the parsed message path fields—including the SDK's derived `memory_paths` and the process working directory—before serializing the fixture would keep reruns portable and stable.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

const disposal = harness.adapter.dispose();
expect(harness.aborted()).toBe(true);
harness.emit(result());
await Promise.race([

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The dispose test doesn't verify that dispose() actually resolves. Because abort fires synchronously inside dispose(), the aborted() assertion holds even if dispose hangs at await this.pump; the Promise.race with a 50ms timeout silently passes either way. Assert on the race outcome (e.g. both settled in Promise.all and that disposal resolved) so a regression that leaves dispose pending is caught rather than missed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/src/harness/claude/claudeAdapter/claudeAdapter.test.ts, line 131:

<comment>The dispose test doesn't verify that dispose() actually resolves. Because abort fires synchronously inside dispose(), the `aborted()` assertion holds even if dispose hangs at `await this.pump`; the Promise.race with a 50ms timeout silently passes either way. Assert on the race outcome (e.g. both settled in Promise.all and that disposal resolved) so a regression that leaves dispose pending is caught rather than missed.</comment>

<file context>
@@ -0,0 +1,136 @@
+		const disposal = harness.adapter.dispose();
+		expect(harness.aborted()).toBe(true);
+		harness.emit(result());
+		await Promise.race([
+			disposal,
+			new Promise((resolve) => setTimeout(resolve, 50)),
</file context>

}

// Fixtures are committed and public: never ship the recording machine's paths.
function redact(line: string, cwd: string): string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The committed fixtures still expose recording-machine path data and are not reproducible: system.init.cwd remains /private/workspace, while memory_paths.auto retains random mkdtempSync names such as ...-3urBTa. redact only replaces the raw temp cwd/home strings and a macOS-specific path pattern, so it misses SDK-derived path fields (and other platform temp paths). Normalizing the parsed message path fields—including the SDK's derived memory_paths and the process working directory—before serializing the fixture would keep reruns portable and stable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/scripts/recordClaudeFixtures.ts, line 107:

<comment>The committed fixtures still expose recording-machine path data and are not reproducible: `system.init.cwd` remains `/private/workspace`, while `memory_paths.auto` retains random `mkdtempSync` names such as `...-3urBTa`. `redact` only replaces the raw temp `cwd`/home strings and a macOS-specific path pattern, so it misses SDK-derived path fields (and other platform temp paths). Normalizing the parsed message path fields—including the SDK's derived `memory_paths` and the process working directory—before serializing the fixture would keep reruns portable and stable.</comment>

<file context>
@@ -102,6 +103,16 @@ function optionsFor(
 }
 
+// Fixtures are committed and public: never ship the recording machine's paths.
+function redact(line: string, cwd: string): string {
+	return line
+		.split(JSON.stringify(cwd).slice(1, -1))
</file context>

}

function subagentContent(outcome: ToolOutcome | null): ToolContent[] {
const output =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: subagentContent re-implements the same rawOutput extraction block (ducked through typeof ... object and cast to Record<string, unknown>/null) that editContent and terminalContent already contain. That's now three copies of the identical idiom in one file, so a future change to how outcomes are parsed (or how rawOutput is structured) has to be made in three places and can drift. A small shared helper like function rawOutputOf(outcome): Record<string, unknown> | null that centralizes this extraction would keep the three call sites consistent without changing behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-runtime/src/harness/claude/mapToolUse/mapToolUse.ts, line 169:

<comment>subagentContent re-implements the same `rawOutput` extraction block (ducked through `typeof ... object` and cast to `Record<string, unknown>`/`null`) that `editContent` and `terminalContent` already contain. That's now three copies of the identical idiom in one file, so a future change to how outcomes are parsed (or how rawOutput is structured) has to be made in three places and can drift. A small shared helper like `function rawOutputOf(outcome): Record<string, unknown> | null` that centralizes this extraction would keep the three call sites consistent without changing behavior.</comment>

<file context>
@@ -116,23 +153,27 @@ function editContent(use: ToolUse, outcome: ToolOutcome | null): ToolContent[] {
+}
+
+function subagentContent(outcome: ToolOutcome | null): ToolContent[] {
+	const output =
+		outcome && typeof outcome.rawOutput === "object" && outcome.rawOutput
+			? (outcome.rawOutput as Record<string, unknown>)
</file context>

This branch was previously deployed

1 inactive deployment
preview — 5256c335 Deployed Aug 5, 2026 by saddlepaddle via Deploy Web #7154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant