Repository navigation
Allow creating agents with Claude Code - #678
salman1993 wants to merge 4 commits into
Conversation
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
One P2 launch blocker; changes requested. Address the inline finding and cover an explicit CLI override with no discoverable CLI.
Reviewed head c790c3f28be09a198ea2a64ce4853c9d11c97b14 against merge-base 04ab197ae95dabe8e109141cebfcc7935ab28472 (target tip fc594b28964a39e401f2c86c3d89bfd14f9c2686). Independent frontend/native review plus a local probe using the unchanged Rust discovery helpers; no local app launch or duplicated broad suites.
Normal PR CI and DCO pass. The separate Windows run failed in archive retention and Git-helper cancellation; its Claude tests passed. This is a separate CI issue, not evidence that this diff caused those failures. Please update the stale “pending” note. Packaged native Create/Edit, live relay replies, mixed-version external CLI use, and actual cross-platform launches remain unverified by this review.
| let cli = if let Some(app_data) = managed_data { | ||
| managed_tool(app_data, "claude").or_else(|| installed_npm_tool("claude")) | ||
| } else { | ||
| installed_npm_tool("claude") | ||
| } | ||
| .ok_or("Install Claude Code in Settings → Agents → Harnesses")?; |
There was a problem hiding this comment.
[P2] Honor the explicit CLI override before requiring autodiscovery
command_checked calls claude_tools before applying/checking the saved CLAUDE_CODE_EXECUTABLE override. Consequently this unconditional error prevents a valid custom CLI from being used when no separate claude is discoverable. For example, enter an absolute adapter through Custom executable / current value, with Node beside it, and set Advanced CLAUDE_CODE_EXECUTABLE to a runnable CLI outside the searched PATH (or move an existing agent’s CLI there). Save succeeds, but Start returns “Install Claude Code” without launching. Disabling the ordinary picker does not prevent this supported custom-entry/saved-agent path.
I reproduced the discovery failure with the unchanged helpers: the complete fixture succeeds; removing only the discoverable CLI fails despite a runnable explicitly selected CLI. The new override test retains the discoverable CLI, so it misses this case.
Pass the effective override into launch preparation and skip the unrelated CLI-discovery requirement when it is explicitly supplied, while retaining Node resolution. Add coverage where only the overridden CLI exists.
There was a problem hiding this comment.
🤖 Fixed in 211cb7e. A saved CLAUDE_CODE_EXECUTABLE now bypasses CLI discovery while retaining Node resolution and PATH setup. Extended the launch fixture with only a runnable custom CLI outside discovery paths; the isolated regression failed before the fix with “Install Claude Code” and passed afterward. All 58 active runtime tests passed. Updated the CI notes to distinguish the unrelated upstream message-composer and Windows failures.
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Prior P2 addressed; no new code blockers found. Re-reviewed 211cb7e2eb7b9cb32dcd5a44c96cab0ddd1749b5 against merge-base 04ab197ae95dabe8e109141cebfcc7935ab28472, focusing on changes since c790c3f2.
- The explicit CLI override now reaches launch preparation before discovery. An isolated probe of the unchanged helpers passed override-only external and managed installs, retained both Node requirements, and retained missing-CLI rejection without an override. The added command-level regression preserves the override in the child environment.
- No blockers in the new local exporter. Independent review exercised the actual exporter and file viewer in Chromium/WebKit: inert text, expandable tool results, no external requests, unchanged source, and owner-only output permissions on macOS.
Current-head CI is still running; DCO passes. No broad suites or native agent launch were rerun here. Packaged lifecycle and cross-platform launch acceptance remain deferred as documented. This is a comment review, not approval.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211cb7e2eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "setupHint": "Install Hermes with ACP support and configure it with `hermes model`. Store provider API keys in `~/.hermes/.env`; Buzz doesn’t inherit shell exports. Verify with `hermes acp --check`." | ||
| }, | ||
| { | ||
| "id": "claude", |
There was a problem hiding this comment.
Commit 0bc0f3a1dd7ad8c99ed86660602494fc879db87d has no Signed-off-by trailer, so it violates the repository’s mandatory DCO policy and cannot satisfy the required hosted DCO check. Recreate the commit with git commit --signoff using the verified effective author identity.
AGENTS.md reference: AGENTS.md:L157-L162
Useful? React with 👍 / 👎.
2 participants
Why
Settings can install and sign in to Claude Code after #654, but Create agent cannot select it. Complete that path using the existing native controller.
What
Offer Claude Code in Create/Edit when its tools are installed. Use Claude's own model and credentials. Preserve the dedicated Settings row and guidance for missing tools or sign-in.
How
Reuse the shared harness preset and native setup discovery. Save the selected adapter's absolute path with empty default arguments. Managed launches use pinned Node and the installed Claude CLI. External launches resolve local tools. Explicit Advanced environment overrides remain supported. A saved
CLAUDE_CODE_EXECUTABLEbypasses CLI discovery while Node resolution remains required.The installed adapter lacked its SDK's optional native binary. It passed
--versionbut failed at session creation. Pointing it at the existing Claude CLI fixes that path without reinstalling. Windows batch login launchers remain excluded from the JavaScript SDK override; those installs need the SDK's native optional dependency.The pinned Buzz runtime already handles Claude's ACP sessions, system-prompt append, permissions and cancellation. Identity creation, Start/Stop/Restart, profile publication and retry keep their existing owners. No runtime dependency update or second lifecycle owner is added.
Risk
Claude launch discovery, its form choice, and local transcript export change. Model browsing, provider controls and device-wide Claude defaults remain outside this slice. Picker availability confirms installed tools, not inference. Existing model/provider values remain visible for explicit recovery to Claude defaults.
Testing
Verified the missing-CLI override case under a macOS sandbox that denied reads from the host’s global CLI fallback directories. The extended launch regression failed before the fix with “Install Claude Code” and passed after it. The fixture supplied only a runnable custom CLI outside discovery paths. No installed tools were changed. Independent review found no blockers.
/usr/bin/sandbox-exec -p '(version 1)(allow default)(deny file-read* (subpath "/opt/homebrew/bin") (subpath "/usr/local/bin"))' target/debug/deps/buzz_agent_controller-23c0c66bb07d8feb --exact runtime::tests::external_claude_launch_resolves_node_and_avoids_windows_batch_cli_overrides --nocaptureAdded Claude session viewing instructions and a non-interactive HTML exporter. Verified transcript lookup using the requested Buzz thread and generated a 35-message local viewer. Chromium and WebKit displayed messages and expanded tool results with zero external requests. The source transcript was unchanged. Independent review found no blockers. No raw transcript data is included.
The human created a Claude agent and showed its live channel replies. The transcript confirmed that the initial empty reply came from a shell alias whose target was outside the runtime PATH. Bypassing the alias delivered the reply. Runtime behavior is unchanged by this documentation follow-up.
On macOS arm64, exercised installed adapter 0.85.1 with the same restricted environment as the launch path. Initialization passed before the fix, but
session/newfailed with a missing native binary. With the selected managed Claude CLI, session creation passed and a prompt returned exactlyBUZZ_CLAUDE_OKwithend_turn.Exercised the actual creation dialog in isolated Chromium and WebKit fixtures: selected Claude, confirmed provider/model controls were absent, and observed Create → Start → profile completion. Fixtures use synthetic identities and fake native host responses. No browser journeys were added or removed. Independent agent review found no blockers.
Normal PR CI and hosted DCO passed at
c790c3f2. The later run at1a82986achecked a synthetic merge into main515554ef; JavaScript and integration jobs failed on the upstream message composer’s undefinedmentionCandidates, also reported by failing browser jobs. That file is outside this PR. Windows native CI failed in archive paging and project-Git helper cancellation tests outside the changed files; its Claude tests passed. Windows validation remains incomplete.Deferred: packaged native Create/Edit, human Stop/Start acceptance, and actual Windows/Linux/Intel macOS launches. A real adapter prompt and browser fixture do not establish those flows. No
buzz-review-completedattestation is claimed.To try locally, run
bin/just desktopfrom this worktree. In Settings → Agents, confirm Claude is signed in. Open Agents → Add agent, choose Claude Code, name it, select a workspace, and create it. Add or mention it in a channel and check its reply, then Stop/Start it.Bigger picture
This is the second Claude Code PR after #654. It learns from old Buzz's launch behavior while reusing the new app's existing owners.
Generated with Codex