chore(runner): bump all dependencies, upgrade to UBI 10 / Python 3.12#845
chore(runner): bump all dependencies, upgrade to UBI 10 / Python 3.12#845jeremyeder wants to merge 12 commits intomainfrom
Conversation
|
Test comment from Claude Code review |
|
Claude Code Review Summary This PR bumps all 17 ambient-runner Python dependencies to their latest PyPI releases and migrates dev-dependencies from the deprecated [tool.uv] dev-dependencies format to the PEP 735-standard [dependency-groups] table. The main risk is the major version jump in pytest-asyncio (0.21 to 1.3) without explicit asyncio mode configuration. The PR test plan correctly flags this for verification but does not add the necessary configuration. Issues by Severity Blocker Issues: None Critical Issues: None Major Issues
File: components/runners/ambient-runner/pyproject.toml Problem: pytest-asyncio bumped from >=0.21.0 to >=1.3.0 (major version) with no [tool.pytest.ini_options] added. pytest-asyncio 1.x dropped backward-compatible legacy behavior and changed event loop scope management. The PR test plan itself calls out Verify pytest-asyncio 1.x config compatibility as an outstanding TODO without providing the fix. Evidence: All async tests use @pytest.mark.asyncio markers; no asyncio_mode exists anywhere in runner config files (confirmed by search). Suggested fix - add to pyproject.toml: [tool.pytest.ini_options] Use strict since every async test already has an explicit @pytest.mark.asyncio marker. Minor Issues
File: components/runners/ambient-runner/.mcp.json The workspace-mcp bump (1.6.1 to 1.14.2) duplicates commit 406bcbb already merged to main. No code change needed - just confirm no merge conflict is expected. Positive Highlights
Recommendations
Reviewed by Claude Code (claude-sonnet-4-6) |
|
CLAUDE CODE REVIEW - PR 845 Summary: PR 845 bumps all 17 ambient-runner dependencies to latest PyPI releases and migrates dev-dependencies from deprecated Blocker Issues: None. Critical Issues: None. MAJOR ISSUE 1: Stale lock file was out of sync with pyproject.toml before this PR
MAJOR ISSUE 2: pytest-asyncio 0.21.0 to 1.3.0 is a major-version breaking change with unchecked test plan
MAJOR ISSUE 3: mcp-atlassian 0.13.0 to 0.21.0 introduces
MINOR ISSUE 1: claude-agent-sdk floor version does not match the resolved lock version
MINOR ISSUE 2: fastapi 0.135.1 adds
Positive Highlights
Recommendations
Reviewed against: backend-development.md, security-standards.md, k8s-client-usage.md, error-handling.md, react-query-usage.md. No backend or frontend application code changes in scope. Review generated by Claude Code (claude-sonnet-4-6) |
|
See review below. |
|
Full Claude Code review posted as a reply. |
7164ba7 to
2646476
Compare
|
test comment from review script |
This comment has been minimized.
This comment has been minimized.
|
Claude Code Review - PR 845 chore(runner): bump all dependencies, upgrade to UBI 10 / Python 3.12 SUMMARY: Comprehensive dependency maintenance pass on the ambient-runner (17 Python packages bumped to latest, UBI 9 to UBI 10, Python 3.11 to 3.12, type hints modernized across 21 files, dead code removed). Changes are well-scoped and cleanly executed. No security issues or architecture violations found. BLOCKER ISSUES: None MINOR ISSUES:
POSITIVE HIGHLIGHTS:
RECOMMENDATIONS:
Review generated by Claude Code using repository security standards and architectural conventions. |
Comprehensive update of the ambient-runner: Dependencies (pyproject.toml): - Bump all 17 Python deps to latest PyPI releases - Migrate [tool.uv] dev-dependencies to [dependency-groups] dev - Add pytest-asyncio asyncio_mode = "auto" for v1.x compatibility - Update requires-python from >=3.11 to >=3.12 MCP servers (.mcp.json): - Bump workspace-mcp from 1.6.1 to 1.14.2 Container images (Dockerfile): - Upgrade base from ubi9/python-311 to ubi10/ubi (pinned by digest) - Python 3.11 to 3.12 (RHEL 10 system default) - Node.js 20 to 22 LTS (explicit module stream) - Add --break-system-packages for PEP 668 on RHEL 10 - Remove 37 lines of commented-out Playwright code State-sync (state-sync/Dockerfile): - Bump Alpine from 3.19 to 3.21 Housekeeping: - Modernize type hints across 21 files (Optional -> X | None, etc.) - Fix test_google_drive_tool_invocation to use @pytest.mark.skip - Add docs/UPDATE_PROCEDURE.md for repeatable maintenance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2646476 to
739ea79
Compare
- Add Zod schema + React form matching claude-agent-sdk 0.1.48 types - Integrate into create-session-dialog behind `advanced-agent-options` flag - Add agentOptions field to CreateAgenticSessionRequest type - Refine UPDATE_PROCEDURE.md with form sync steps and automation strategy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude Code Review Summary PR 845 covers: dependency bumps (ambient-runner Python deps to latest, UBI 9 to 10, Python 3.11 to 3.12, Node 20 to 22); a new ClaudeAgentOptions Zod schema and React form (1,087 lines) matching claude-agent-sdk 0.1.48; integration behind the The dependency bumps and type-hint cleanup are clean. Two frontend issues are worth addressing before this ships. Blocker Issues: None. Critical Issues: None. Major Issue 1 —
Major Issue 2 —
Minor Issue 3 — Raw
Minor Issue 4 — Array index used as
Positive Highlights
Recommendations
|
Session SummaryWhat was done
Key decisions
|
Address code review findings: - Split 1087-line options-form.tsx into 11 sub-components under _components/ - Replace bare <label> elements with Shadcn <Label> throughout - Use stable IDs instead of array index keys in list editors - Add TODO comment documenting backend agentOptions handler gap Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add agentOptions to CreateAgenticSessionRequest in agentic-session.ts (dialog imports from this file, not api/sessions.ts) - Use z.input for ClaudeAgentOptionsForm type so zodResolver input type matches the form type parameter (fields with .default() are optional) - Bump CI Python version from 3.11 to 3.12 to match requires-python Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
- Add httpx to CI test dependencies (moved to dependency-groups in pyproject.toml, not installed by pip install -e .) - Fix UBI 10 base image digest to use OCI manifest index format (buildx needs the multi-arch index digest, not the single manifest) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
- Install git before gh CLI (gh RPM depends on git package) - Update observability tests to use standalone setup_bridge_observability() (method was refactored from bridge instance method to module function) - Add httpx to CI test dependencies Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove dnf module enable for nodejs (RHEL 10 ships Node.js directly in AppStream, not as a module stream) - Install all optional deps in CI (pip install -e ".[all]") so tests requiring claude-agent-sdk can run Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
test |
The feedback endpoint now returns {type: "RAW", event: {...}} instead
of flat response. Update assertions to access metaType via event key.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Claude Code ReviewSummaryThis PR bundles three changes: UBI9 to UBI10 / Python 3.11 to 3.12 / Node 20 to 22 runtime upgrades, dependency bumps across the ambient-runner, and a new ClaudeAgentOptions form (12 collapsible sections, Zod schema, Unleash flag gate). The infrastructure upgrades are clean. The frontend form work is architecturally sound and well-guarded behind a feature flag, but has a few correctness bugs and one guideline violation worth fixing before merge. Issues by SeverityBLOCKERS: None. CRITICAL: None. MAJOR ISSUES 1. options-form.tsx exceeds the 200-line component limit (482 lines) File: components/frontend/src/components/claude-agent-options/options-form.tsx Problem: Single AgentOptionsFields render function at 482 lines. The frontend guidelines require components under 200 lines (frontend-development.md pre-commit checklist). The prior commit cbddef9 already split sub-components out, but the orchestrating form is still well over the limit. Fix: Split the 12 sections into individual section components (CoreSection, ToolsSection, McpSection, SandboxSection, etc.) under _components/. The existing Section wrapper makes this a clean refactor. 2. KeyValueEditor: silent key collision when user clicks Add multiple times File: components/frontend/src/components/claude-agent-options/_components/key-value-editor.tsx:191 Problem: addEntry always inserts with an empty string key. Clicking Add twice without changing the first key silently overwrites the first entry -- items are lost with no warning. Current code: const addEntry = () => onChange({ ...value, empty-string-key: empty-string-value }); Fix: Use a unique placeholder key driven by a useRef counter -- the same pattern already used in StringListEditor. 3. AgentsEditor: key collision after remove-then-add cycle File: components/frontend/src/components/claude-agent-options/_components/agents-editor.tsx:52 Problem: New agents are keyed agent-(entries.length + 1). If a user adds agent-1 and agent-2 (length=2), removes agent-1 (length=1), then adds again, the key becomes agent-2 -- silently overwriting the existing agent-2. Fix: Use a useRef counter (like PluginsEditor does) and find the next unused agent-N key instead of deriving from entries.length. MINOR ISSUES 4. Unsafe type cast in McpServersEditor silences a null/string mismatch File: components/frontend/src/components/claude-agent-options/_components/mcp-servers-editor.tsx (env and headers onChange callbacks) Problem: KeyValueEditor onChange returns Record<string, string | null> but the cast to Record<string, string> discards null. A user who clears a value sends null silently cast away, which could produce an invalid MCP server config. Fix: Filter null entries via Object.entries(e).filter(([, v]) => v !== null) before spreading into updateServer. 5. SandboxField: update helper typed as Record<string, unknown> bypasses type safety File: components/frontend/src/components/claude-agent-options/_components/sandbox-field.tsx:481 Problem: The update function accepts patch: Record<string, unknown> and merges blindly into field value, bypassing the sandboxSettingsSchema shape at the TypeScript level. Zod catches issues at submit, but errors are hidden earlier. Fix: Type patch as Partial<z.infer>. 6. agentOptionsForm.getValues() skips Zod output defaults File: components/frontend/src/components/create-session-dialog.tsx:162 Problem: getValues() returns raw z.input form state, not z.output after .default() transforms. Fields with .default([]) or .default({}) may arrive as undefined in the payload rather than their defaults. Low-risk today since the backend drops the field (per the TODO comment). When backend support lands, use claudeAgentOptionsSchema.parse(agentOptionsForm.getValues()) to get the fully-defaulted output. 7. Unnecessary export on McpFormServer type File: components/frontend/src/components/claude-agent-options/_components/mcp-servers-editor.tsx:268 Problem: McpFormServer is exported but appears to only be used internally within the claude-agent-options feature folder. Remove export unless there are other consumers. Positive Highlights
Recommendations
🤖 Reviewed with Claude Code |
|
test |
The interrupt endpoint returns "No active session manager" instead of including the thread_id in the error. Relax assertion to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
|
Claude Code Review Summary PR 845 combines three distinct things: (1) a Python/Node/base-image upgrade with type-hint modernization across the runner, (2) a net-new ClaudeAgentOptions form backed by a comprehensive Zod schema, and (3) integration of that form into the create-session dialog behind an Unleash flag. The runner changes are clean and low-risk. The frontend work follows project patterns well (Shadcn UI, type over interface, colocated components, no any types) but contains one confirmed data-loss bug in KeyValueEditor and two other issues worth fixing before the flag is enabled. Blocker Issues: None Critical Issues 1. KeyValueEditor — adding multiple entries produces only one entry File: components/frontend/src/components/claude-agent-options/_components/key-value-editor.tsx, line 191 Every new entry is keyed with an empty string. Because value is Record, calling addEntry twice overwrites the first entry — the second call is a no-op. The user sees two rows appear (via local refs) but the actual record holds only one entry. MCP server env, extra_args, and custom headers are silently truncated. Fix: generate a unique placeholder key per call, same as the agent/server name pattern used in other editors in the same PR (e.g. agent-1, server-1). Major Issues 1. Unsafe casts in McpServersEditor File: components/frontend/src/components/claude-agent-options/_components/mcp-servers-editor.tsx, lines 325 and 333 KeyValueEditor types its value as Record<string, string|null>. The two cast sites (env and headers) discard null safety. A null value serialised to JSON becomes the string null or is silently omitted — neither is correct for MCP server config. Fix: filter null values via Object.fromEntries with a type-guard before assigning to env/headers. 2. agentOptions field is silently dropped by the backend Files: components/frontend/src/types/agentic-session.ts line 1678, types/api/sessions.ts line 1693 The existing TODO comments acknowledge: Go encoding/json silently drops it. Safe while the flag defaults to off. Every submission from an enabled flag sends data that is immediately discarded with no error. Acceptable only while the flag stays off, but the tech debt needs explicit tracking. Recommendation: add a GitHub issue link in the TODO comment, and confirm the flag default-off state is enforced at the Unleash level (not just in code). 3. permission_prompt_tool_name in schema has no form field File: components/frontend/src/components/claude-agent-options/schema.ts, line 1526 This field appears in claudeAgentOptionsSchema but is not rendered anywhere in the form. It will always be undefined. Either add a form field in the Advanced section, or remove it from the schema with a comment explaining the exclusion (as done for non-serialisable fields in UPDATE_PROCEDURE.md). Minor Issues
Positive Highlights
Recommendations
Reviewed by Claude Code (amber.review) |
validate_vertex_credentials_file now checks that the credentials file exists. Provide a temporary file in tests that use Vertex mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude Code Review Summary This PR bundles three distinct changes: a runtime dependency + base-image upgrade (UBI 9 to UBI 10, Python 3.11 to 3.12), a large new ClaudeAgentOptions form behind an Unleash feature flag, and Python modernisation. The dependency upgrades are straightforward. The feature-flag gate is the right call for incomplete work. The main concerns are a single oversized component, a half-wired backend field, and a type-duplication issue. Blocker Issues: None. Critical Issues: None. Major Issues M1 — options-form.tsx exceeds the 200-line limit by 2.4x File: components/frontend/src/components/claude-agent-options/options-form.tsx (482 lines) Frontend standards require components under 200 lines (.claude/context/frontend-development.md pre-commit checklist). AgentOptionsFields at 482 lines is more than double the limit. Each Section block is already extracted as its own sub-component; the form orchestrator should follow the same pattern, trimmed to ~100 lines that wire the 12 Section blocks together and delegate all field rendering to those sub-components. M2 — agentOptions field is sent to the backend but not consumed (silent data loss) Files: components/frontend/src/types/agentic-session.ts:199, components/frontend/src/types/api/sessions.ts:133 Both files carry a TODO acknowledging that the backend Go handler does not yet unmarshal or persist agentOptions. Users who enable the advanced-agent-options flag will see their settings silently dropped on every session create. The feature flag keeps users safe for now, but this should either (a) include the backend CR spec update and handler wiring in this PR, or (b) be tracked as a concrete follow-up issue so the work does not slip. M3 — Duplicate CreateAgenticSessionRequest type definition Files: components/frontend/src/types/agentic-session.ts:196, components/frontend/src/types/api/sessions.ts:130 Both files define CreateAgenticSessionRequest (including the newly added agentOptions field) and both were updated in this PR. Extending both simultaneously is the right moment to consolidate. A future divergence will cause silent request-shape mismatches. Suggested fix: re-export one from the other, or pick a single canonical location. Minor Issues m4 — OutputFormatField local state does not sync on external form reset File: components/frontend/src/components/claude-agent-options/_components/output-format-field.tsx:24 rawJson is initialised from value once at mount via useState. When agentOptionsForm.reset() is called on dialog close, field.value changes but rawJson does not, leaving stale JSON visible when the dialog reopens next time. Fix: add a useEffect keyed on value to sync rawJson. m5 — MCP server name key can collide after delete-then-add File: components/frontend/src/components/claude-agent-options/_components/mcp-servers-editor.tsx:33 addServer uses server-(entries.length+1) as the key. With [server-1, server-2], deleting server-1 then clicking Add produces key server-2, silently overwriting the existing entry. Fix: use a useRef counter or Date.now() instead of entries.length. m6 — workspace-mcp version jump (1.6.1 to 1.14.2) should be verified Files: .mcp.json line 20 (1.6.1 to 1.14.2), pyproject.toml (>=0.10.0 to >=0.11.0). An 8 minor-version jump in the pinned reference — the changelog should be reviewed for breaking changes to the drive tool interface before merging, as this underpins the Google Drive integration. Positive Highlights
Recommendations
Reviewed by Claude Code |
…dule) The test imports from a top-level `auth` module that doesn't exist. Skip the entire file with a clear reason until migrated to the new ambient_runner.platform.auth module path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
test |
|
test comment from review bot |
|
Claude Code Review Summary This PR delivers a well-scoped dependency maintenance release: bumping all Python runner dependencies to latest, upgrading the base image from UBI 9 to UBI 10 (Python 3.11 to 3.12, Node.js 20 to 22), modernizing Python type annotations to 3.10+ built-in style, and adding a new ClaudeAgentOptions form gated safely behind an Unleash feature flag. Security posture and architectural conventions are respected throughout. No blockers or critical issues found. Blocker Issues: None Critical Issues: None Major Issue 1 — Agent options validation errors invisible when sections are collapsed File:
Violates: frontend-development.md — 'Loading and error states handled.' Suggested fix: Propagate a Major Issue 2 — Test assertion weakened without explanation File: The assertion Violates: error-handling.md — 'Errors logged with context'; Testing axis — 'Adequate coverage.' Minor Issues
Positive Highlights
Recommendations
Reviewed by Claude Code (claude-sonnet-4-6) |
|
@Gkrumbach07 this is really nice - some options we might want to turn off. the CLI flags and using the claude code binary inside a session. that defeats the purpose of any controls |
|
Randomly picking this pull request for some procedural feedback. IMO it's rather silly to go from having AI generate code (and hopefully pre-review it locally) to posting a PR to have the same model using the same rules re-review it again and post a gigantic markdown doc as a single comment. A big immediate win here would be to change your review workflow to use the Github API which allows associating comments with particular lines (as other tools do like Gemini Code Review, Rabbit etc.). I have this "skill" over here https://github.com/bootc-dev/agent-skills/tree/main/perform-forge-review which gives agents enough information to do this. But even then, I kind of doubt any human reads all of the review here - it's just too much. Personally the workflow I want to get to is where humans do review, assisted by AI. And in this case literally it could just boil down to a human seeing the automated review, they do some quick sanity checking and click approve on their own. They can add as much or as little context into the PR as they want. |
Review Queue — Blockers Found
Action needed: Author needs to resolve merge conflicts
|

Summary
ClaudeAgentOptionsform (Zod schema + React form) matching claude-agent-sdk 0.1.48advanced-agent-optionsUnleash flag (default: off)Version changes
New files
components/frontend/src/components/claude-agent-options/schema.ts— Zod schema for all ClaudeAgentOptions fieldscomponents/frontend/src/components/claude-agent-options/options-form.tsx— React form with 12 collapsible sectionscomponents/frontend/src/components/claude-agent-options/index.ts— barrel exportsModified files
components/frontend/src/components/create-session-dialog.tsx— dual-form architecture, Unleash flag gatecomponents/frontend/src/types/api/sessions.ts—agentOptionsfield on CreateAgenticSessionRequestcomponents/runners/ambient-runner/docs/UPDATE_PROCEDURE.md— form sync step, automation strategyUnleash flag
Create
advanced-agent-optionsin Unleash with tagscope: workspaceto enable the form in the create-session dialog.Test plan
advanced-agent-optionsflag is off (default)🤖 Generated with Claude Code