Skip to content

Commit 3254101

Browse files
Ambient Code Botclaude
andcommitted
fix: address third round of CodeRabbit review feedback
- Remove unused thread_id from build_options() and parent_tool_use_id from handle_tool_result_block() - Fix base PlatformBridge.context property to return self._context - Add NotImplementedError guard for thread_id in LangGraph interrupt() - Use collections.abc.AsyncIterator in Claude bridge - Reuse creds_file fixture in test_gemini_auth (remove leaked tempfile) - Guard against key/name collisions in key-value, MCP, and agents editors - Sanitize null values from KeyValueEditor before passing to MCP env/headers - Add create-session-dialog.tsx to SDD manifest managed paths - Fix runner spec markdown: add language to fenced code blocks, blank lines around tables Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cdf9c57 commit 3254101

11 files changed

Lines changed: 36 additions & 19 deletions

File tree

.specify/sdd-manifest.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ managed-components:
2727
paths:
2828
- components/runners/ambient-runner/**
2929
- components/frontend/src/components/claude-agent-options/**
30+
- components/frontend/src/components/create-session-dialog.tsx
3031
- .github/workflows/runner-tool-versions.yml
3132
constitution: .specify/constitutions/runner.md
3233
spec: .specify/specs/runner.md

.specify/specs/runner.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ The ambient-runner is a Python application that executes AI agent sessions insid
1515

1616
### Managed Paths
1717

18-
```
18+
```text
1919
components/runners/ambient-runner/
2020
├── Dockerfile # Runner container image
2121
├── main.py # FastAPI entry point
@@ -42,7 +42,7 @@ components/runners/ambient-runner/
4242

4343
### Supporting Frontend Paths
4444

45-
```
45+
```text
4646
components/frontend/src/components/claude-agent-options/
4747
├── schema.ts # Zod schema (mirrors SDK types)
4848
├── options-form.tsx # Main form component
@@ -57,6 +57,7 @@ components/frontend/src/components/claude-agent-options/
5757
- Python 3.12 (system default), Node.js (AppStream), Go (go-toolset)
5858

5959
### Pinned Tools
60+
6061
| Tool | Dockerfile ARG | Purpose |
6162
|------|---------------|---------|
6263
| gh | `GH_VERSION` | GitHub CLI for repo operations |
@@ -66,6 +67,7 @@ components/frontend/src/components/claude-agent-options/
6667
| gemini-cli | `GEMINI_CLI_VERSION` | Google Gemini CLI |
6768

6869
### Key Dependencies
70+
6971
| Package | Constraint | Role |
7072
|---------|-----------|------|
7173
| claude-agent-sdk | `>=0.1.50` | Claude Code agent SDK |

components/frontend/src/components/claude-agent-options/_components/agents-editor.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ export function AgentsEditor({ value, onChange }: { value: Record<string, AgentD
4747
onChange(next);
4848
};
4949
const updateAgentName = (index: number, newName: string) => {
50+
const oldName = entries[index][0];
51+
if (newName !== oldName && newName in value) return;
5052
const next: Record<string, AgentDef> = {};
5153
for (let i = 0; i < entries.length; i++) {
5254
next[i === index ? newName : entries[i][0]] = entries[i][1];

components/frontend/src/components/claude-agent-options/_components/key-value-editor.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ export function KeyValueEditor({
4747
onChange(next);
4848
};
4949
const updateEntry = (index: number, newKey: string, newVal: string | null) => {
50+
const oldKey = entries[index][0];
51+
if (newKey !== oldKey && newKey in value) return;
5052
const next: Record<string, string | null> = {};
5153
for (let i = 0; i < entries.length; i++) {
5254
if (i === index) {

components/frontend/src/components/claude-agent-options/_components/mcp-servers-editor.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ export function McpServersEditor({ value, onChange }: { value: Record<string, Mc
5858
onChange(next);
5959
};
6060
const updateServerName = (index: number, newName: string) => {
61+
const oldName = entries[index][0];
62+
if (newName !== oldName && newName in value) return;
6163
const next: Record<string, McpFormServer> = {};
6264
for (let i = 0; i < entries.length; i++) {
6365
next[i === index ? newName : entries[i][0]] = entries[i][1];
@@ -96,15 +98,27 @@ export function McpServersEditor({ value, onChange }: { value: Record<string, Mc
9698
</div>
9799
<div>
98100
<Label className="text-xs text-muted-foreground">Environment</Label>
99-
<KeyValueEditor value={server.env ?? {}} onChange={(e) => updateServer(name, { ...server, env: e as Record<string, string> })} />
101+
<KeyValueEditor value={server.env ?? {}} onChange={(e) => {
102+
const sanitized: Record<string, string> = {};
103+
for (const [ek, ev] of Object.entries(e)) {
104+
if (ev != null) sanitized[ek] = ev;
105+
}
106+
updateServer(name, { ...server, env: sanitized });
107+
}} />
100108
</div>
101109
</>
102110
) : (
103111
<>
104112
<Input className="font-mono text-xs" placeholder={server.type === "sse" ? "https://server.example.com/sse" : "https://server.example.com/mcp"} value={server.url ?? ""} onChange={(e) => updateServer(name, { ...server, url: e.target.value })} />
105113
<div>
106114
<Label className="text-xs text-muted-foreground">Headers</Label>
107-
<KeyValueEditor value={server.headers ?? {}} onChange={(h) => updateServer(name, { ...server, headers: h as Record<string, string> })} keyPlaceholder="Header-Name" valuePlaceholder="Header value" />
115+
<KeyValueEditor value={server.headers ?? {}} onChange={(h) => {
116+
const sanitized: Record<string, string> = {};
117+
for (const [hk, hv] of Object.entries(h)) {
118+
if (hv != null) sanitized[hk] = hv;
119+
}
120+
updateServer(name, { ...server, headers: sanitized });
121+
}} keyPlaceholder="Header-Name" valuePlaceholder="Header value" />
108122
</div>
109123
</>
110124
)}

components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,6 @@ async def run(
428428
def build_options(
429429
self,
430430
input_data: RunAgentInput | None = None,
431-
thread_id: str | None = None,
432431
resume_from: str | None = None,
433432
) -> "ClaudeAgentOptions":
434433
"""
@@ -438,7 +437,6 @@ def build_options(
438437
439438
Args:
440439
input_data: Optional RunAgentInput for extracting dynamic tools
441-
thread_id: Optional thread_id for session resumption lookup
442440
resume_from: Optional CLI session ID to resume (preserves chat history
443441
across adapter rebuilds, e.g. after a repo is added mid-session)
444442
@@ -1157,9 +1155,8 @@ def flush_pending_msg():
11571155
upsert_message(
11581156
build_agui_tool_message(tool_use_id, block_content)
11591157
)
1160-
parent_id = getattr(message, "parent_tool_use_id", None)
11611158
async for event in handle_tool_result_block(
1162-
block, thread_id, run_id, parent_id
1159+
block, thread_id, run_id
11631160
):
11641161
yield event
11651162

components/runners/ambient-runner/ag_ui_claude_sdk/handlers.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,20 +134,16 @@ async def handle_tool_result_block(
134134
block: Any,
135135
thread_id: str,
136136
run_id: str,
137-
parent_tool_use_id: str | None = None,
138137
) -> AsyncIterator[BaseEvent]:
139138
"""
140139
Handle ToolResultBlock from Claude SDK.
141140
142141
Emits TOOL_CALL_END and TOOL_CALL_RESULT events.
143-
Nested tool results (with parent_tool_use_id) are also emitted - they represent
144-
sub-agent calls (e.g., Task calling WebSearch).
145142
146143
Args:
147144
block: ToolResultBlock from Claude SDK
148145
thread_id: Thread identifier
149146
run_id: Run identifier
150-
parent_tool_use_id: Parent tool ID if this is a nested result
151147
152148
Yields:
153149
AG-UI tool result events

components/runners/ambient-runner/ambient_runner/bridge.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ def get_error_context(self) -> str:
234234
@property
235235
def context(self) -> RunnerContext | None:
236236
"""The current ``RunnerContext``, or ``None`` before ``set_context()``."""
237-
return None
237+
return self._context
238238

239239
@property
240240
def configured_model(self) -> str:

components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
import logging
1313
import os
1414
import time
15-
from typing import Any, AsyncIterator
15+
from collections.abc import AsyncIterator
16+
from typing import Any
1617

1718
from ag_ui.core import (
1819
BaseEvent,
@@ -167,7 +168,7 @@ async def run(
167168
thread_id, None
168169
) or self._session_manager.get_session_id(thread_id)
169170
sdk_options = self._adapter.build_options(
170-
input_data, thread_id=thread_id, resume_from=saved_session_id
171+
input_data, resume_from=saved_session_id
171172
)
172173
worker = await self._session_manager.get_or_create(
173174
thread_id, sdk_options, api_key

components/runners/ambient-runner/ambient_runner/bridges/langgraph/bridge.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ async def interrupt(self, thread_id: str | None = None) -> None:
8888
"""Interrupt the current LangGraph execution."""
8989
if self._adapter is None:
9090
raise RuntimeError("LangGraphBridge: no adapter to interrupt")
91+
if thread_id is not None:
92+
raise NotImplementedError(
93+
"LangGraphBridge.interrupt() does not support thread_id"
94+
)
9195

9296
if hasattr(self._adapter, "interrupt"):
9397
await self._adapter.interrupt()

0 commit comments

Comments
 (0)