feat(cli): configure MCP natively across coding agents - #187
Open
Max17190 wants to merge 39 commits into
Open
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Max17190
force-pushed
the
feat/native-mcp-setup
branch
from
August 12, 2026 21:55
cc7e59b to
9162131
Compare
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
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="src/utils/run-client-command.ts">
<violation number="1" location="src/utils/run-client-command.ts:59">
P2: When an earlier PATH entry contains a directory named `<command><PATHEXT>`, `resolveWindowsCommand` selects it because `existsSync` accepts directories, then launcher setup fails instead of continuing to the real executable. Require a regular file before returning a PATH candidate.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| for (const directory of pathEntries) { | ||
| for (const candidate of candidates) { | ||
| const resolved = directory ? path.join(directory, candidate) : candidate; | ||
| if (existsSync(resolved)) return resolved; |
There was a problem hiding this comment.
P2: When an earlier PATH entry contains a directory named <command><PATHEXT>, resolveWindowsCommand selects it because existsSync accepts directories, then launcher setup fails instead of continuing to the real executable. Require a regular file before returning a PATH candidate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/run-client-command.ts, line 59:
<comment>When an earlier PATH entry contains a directory named `<command><PATHEXT>`, `resolveWindowsCommand` selects it because `existsSync` accepts directories, then launcher setup fails instead of continuing to the real executable. Require a regular file before returning a PATH candidate.</comment>
<file context>
@@ -0,0 +1,94 @@
+ for (const directory of pathEntries) {
+ for (const candidate of candidates) {
+ const resolved = directory ? path.join(directory, candidate) : candidate;
+ if (existsSync(resolved)) return resolved;
+ }
+ }
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Replace the subprocess installer in `setup mcp` with a built-in one that detects installed agents, pre-selects them in a picker, and offers to install rules telling those agents to prefer Firecrawl for web search and scraping. Covers Claude Code, Cursor, VS Code, Codex, OpenCode, Windsurf, Zed, Hermes Agent, and OpenClaw through per-agent flags, `--project` for project scope, and `--rules` / `--no-rules` for scripted runs. `-y` stays MCP-only. The two launchers were previously reachable only by flag, so a plain `setup mcp` never offered them; they now sit in the picker alongside the editors, and because a launcher shells out to a CLI, a missing binary is reported against that one agent instead of ending the run. Credential handling is unchanged in principle and stricter in reach: an API key is never written as a literal. Knowing which agents were selected means each one receives a reference to FIRECRAWL_API_KEY in the syntax it expands, so the setup no longer has to refuse a run that omits --agent. Agents with no verified syntax fall back to the keyless endpoint and say so rather than persisting a secret. Config edits are surgical. JSON is patched through a JSONC-aware editor so commented settings files parse at all and keep their comments, and TOML tables are replaced along with any stale sub-tables left by a previous stdio entry. Reruns are byte-identical. Also gives every setup test a throwaway HOME and resets spawn mocks between tests, since MCP setup now writes real config files and would otherwise rewrite the developer's own agent settings; and teaches doctor about the `servers` and `context_servers` keys so those registrations are recognized.
The PATH lookup matched any executable named `hermes`, including an unrelated JavaScript engine that ships with common toolchains, so the picker pre-selected an agent the user did not have. Detection now prefers a false negative to a false positive: every agent is listed either way, so missing one costs a keystroke while pre-selecting a missing one is misleading. Also pins HOME and PATH for setup tests. Both feed agent detection, so leaving the real ones visible made results depend on what happened to be installed on the machine running the suite.
Zed's native remote MCP support is version-gated and its handling of request headers is inconsistent across releases, so a written entry can report success while the agent never connects. That reads as Firecrawl being broken, which is worse than not offering the agent at all. Removing it until the shape can be confirmed against a live install. Also pins the picker page size to the number of agents. The default was smaller than the list, so the last agent scrolled out of view.
Windsurf's remote entry shape is not settled: sources disagree on whether a transport field is required and what its value should be, and one reports streamable HTTP working only through a local proxy. A wrong entry does not error, it reports success and then exposes no tools, so this stays out until the shape can be confirmed against a live install. With every supported agent now carrying a verified environment-reference syntax and project-level config, the keyless-fallback and global-fallback branches no longer have a case. Removing them rather than leaving unreachable logic behind; they come back with the agent that needs them.
Six defects, four of them silent: * Quiet mode returned before the total-failure check, so a run in which nothing was written resolved successfully. `firecrawl init` and `firecrawl launch` both use quiet mode and reported success regardless. * The TOML writer split on "\n" only, so a config.toml with CRLF endings never matched its existing table and gained a duplicate one, leaving the file invalid and taking the rest of the user's Codex config with it. * The TOML writer absorbed comment and blank lines directly above the next table into the replaced range and deleted them. * A leading byte order mark was reported as a parse error even though the document parses, so a config written by a Windows editor was refused. * The rule fence required "\n" after its marker, so a file converted to CRLF gained a second copy of the section instead of an updated one. * `--agent all` reached only detected clients. It means every client, which is what the installer it replaced did. The fence replacement now uses a function so nothing in the rule body can be read as a replacement pattern. Line endings and byte order marks are preserved on write rather than normalised away.
…scope flag `firecrawl setup --yes --agent windsurf` installed skills and then aborted, because MCP setup rejected a name it writes no config for. An agent we support for skills but not for MCP is not an error: the run now finishes, skips the MCP step, and prints the server URL so the user can wire it up themselves. A name nothing supports is still rejected, so a typo does not silently do nothing. Scope: global is the intended default, so that one command reaches every agent surface rather than the current checkout alone. `--project` is the only scope flag that means anything on setup. `-g` is accepted for existing scripts but hidden from help and reported as deprecated when used, and the mutually exclusive scope error it existed for is gone. `-g` is untouched on init and launch. Tests also pin USERPROFILE and APPDATA alongside HOME. os.homedir() reads USERPROFILE on Windows, so the sandbox that keeps a test run away from the developer's own agent config was doing nothing there.
…keys Project scope fought the one-command-every-agent goal, and --agent hermes/openclaw aborted on a stored key while the boolean flags wrote keyless config.
… artifacts Three Firecrawl MCP configs were tracked at the repository root. Tests wrote them into the working directory and a broad `git add` swept them in, so anyone opening this checkout inherited MCP servers from the repo. Removed, and every setup test now runs in its own working directory so project-relative writes cannot reach the repository again. A stored key behaved three different ways depending on how the target was named: keyless for the boolean flags, keyless for `--agent hermes`, and a hard abort for `--agent all`. The README documents keyless. `--agent all` now agrees with the rest, and the launchers report through the same summary so the keyless fallback is stated rather than implied by a bare installer log line. `--agent launchers` was a synonym for every agent plus both launchers, which is not what the name says. It now selects the launchers. Launcher dispatch is exhaustive rather than treating anything that is not Hermes as OpenClaw, and doctor recognises OpenCode's top-level `mcp` map, which it previously reported as unregistered right after setup wrote it.
Setup could carry a key or run anonymously, but not sign in, so the one path the docs present to a person at a terminal had no command behind it. `--oauth` writes the sign-in endpoint instead of a credential, and each agent starts the browser flow itself the first time it connects. Sign-in is a different server URL rather than a different header, so it replaces the credential rather than travelling beside it: an exported key is ignored under --oauth and no Authorization header is written. The two are separate endpoints, so combining --oauth with --keyless is rejected instead of silently preferring one. A URL alone is not enough for every agent. Hermes starts the flow only when the entry carries `auth: oauth`, and OpenClaw ignores a static Authorization header unless `auth: "oauth"` is set, which is also what gates its login command. Those fields live beside the agent in the registry, next to the environment syntax, and an agent without a verified sign-in shape gets no entry rather than one that reports success and then exposes no tools. No agent signs in during setup, and each one starts the flow differently, so the summary prints the step per agent rather than one footer: `/mcp` in Claude Code, `codex mcp login firecrawl`, Cursor Settings, and a browser on first use for the rest.
Four defects reported against the sign-in commit, each confirmed against the code before changing it: * Replacing an empty `mcp_servers:` key dropped an inline comment sitting on it. The comment belongs to the null value being replaced; a block map has no inline slot on its key, so it now moves to the head of the section instead of disappearing with the node it was attached to. Preserved and relocated beats silently deleted. * A marker-fenced rule section was written with LF regardless of the file it joined, so updating a CRLF AGENTS.md left mixed endings. The section now adopts the line endings of the file it is written into, which is how the JSON, TOML, and YAML writers already behave. * The OpenClaw workspace was resolved from the environment and the documented defaults alone, so a workspace moved through config took the rule to a path the launcher never reads. OpenClaw is now asked where its workspace is, since its config is JSON5 and out of reach; the previous resolution stays as the fallback for when the CLI cannot answer. * Declining rules reported OpenClaw as not supporting them, which stopped being true when it gained a rule. It reports skipped now, and unsupported is left for agents that genuinely have nowhere to put one.
installOpenClawMcp read the stored key whenever keyless was not also set, so calling it with sign-in alone produced an OAuth entry carrying an Authorization header. OpenClaw ignores a static header once auth is oauth, so the result was inert rather than harmful, but writing credential configuration into a sign-in entry is wrong either way. The CLI never reached this: setup derives keyless from the auth mode, and sign-in is not env, so the header was already dropped. The helper is exported though, and the credential helper beside it is written to be safe in isolation for the same reason, so sign-in now drops the key in the config builder where every caller passes through.
setup mcp promises a global entry in a file we can parse, never a literal key, plus an optional rule file we own. Claude Code, Cursor, VS Code, and Codex meet that as registry data: the only difference is the file format. OpenCode is the same kind of write with its own map key and header form, so it stays. Hermes and OpenClaw were each a second product. Hermes needed a YAML writer, an owner-only create mode, an extra entry field for sign-in, and had no global rule file to own. OpenClaw needed a subprocess, a JSON5 config we cannot parse, a workspace probe, a duplicate credential builder, and a rule that could only be written if its AGENTS.md already existed. Neither shaped the product; both shaped the code around them. They stay supported and stop being written. `--hermes`, `--openclaw`, and their names on --agent print the server URL and succeed, the way an agent we install skills for but write no MCP config for already did. Skills and firecrawl launch are untouched, and the URL still works for both. Removing the two takes the whole launcher concept with them: the subprocess runner and its Windows argv escaping, the second credential path, launcher detection, the YAML writer, and the target/client split that existed only because launchers were not clients. Five agents, one contract, one writer.
…ed yaml dep Two defects reported against the scope change: * The URL-only branch returns before the writers run, and the mutual-exclusion check lived with the writers, so `--hermes --oauth --keyless` printed a sign-in URL and exited zero while `--cursor --oauth --keyless` was rejected. The check now runs at the top of installMcp, ahead of everything that reports or returns. * Removing the Hermes writer took the last import of the yaml package with it. Nothing under src/ imports it now, so it is dropped from the manifest and the lockfile rather than shipped unused. A third finding, that a test leaks FIRECRAWL_API_KEY into later tests, does not hold: beforeEach deletes that variable and afterEach restores the original, so each test starts without it. The assignment is also load-bearing where it is, since the assertion it supports is that sign-in drops a key that was present.
The sign-in mode had its own lead-in, code block, and paragraph, and the two agents setup no longer configures had a paragraph explaining why. Both restate structure the section already has: one paragraph names the supported agents, and one paragraph covers how the credential is handled. Sign-in now sits in that credential paragraph beside keyless, and the two unwritten agents sit in the sentence that already lists the supported set. Same facts, no new sections, nine fewer lines. The harness table above is scoped to skills and stays accurate as written.
A rule tells an agent to prefer firecrawl_search and firecrawl_scrape. It was written even when the server entry for that agent had failed, so an agent could be instructed to reach for tools it had no way to call. The rule now depends on the entry landing. The dependency runs one way only: a failed rule still leaves a working MCP server, which is what keeps the two writes separate. A run that configured some of the chosen agents also exited zero, so a script could not tell a partial install from a complete one. The summary already named what failed; the exit code now agrees with it. Quiet mode is unchanged, since it runs inside init and launch, which report their own outcome and continue. Also ignores .cursor/, alongside the editor directories already listed. Nothing from it was ever tracked, but it is the same kind of workspace artifact a broad `git add` swept into the repo once already.
Failing the run on a partial install swallowed the tail of `--agent all`: the throw left before the Hermes and OpenClaw lines printed, and their URL is the whole answer for those two, so the run that most needed to mention them was the one that did not. The report now runs whether or not the writers threw. The exit code is unchanged. Configuring what it can and then reporting failure is the same contract as before, since per-agent isolation is about the other agents still being configured, not about the command claiming success.
… did installMcp is shared, so making Hermes and OpenClaw URL-only reached callers this branch never edited. Three consequences, all in that seam: * `firecrawl launch hermes --install` printed the URL for an agent we do not configure and then said it was configured with Firecrawl MCP. Launch now claims MCP only when the run actually wrote it, which also covers --skip-mcp, where the same line was already claiming a write that never happened. * Naming an agent we do not configure returned from the whole run, so `--agent hermes --cursor` printed the URL and left Cursor unconfigured. The URL is now reported for whichever flag named it and the rest of the run continues; it ends early only when nothing else was named. * Detecting no agents at all is still an error, since nothing was configured and the run should not report success. It now names the server URL alongside the agent flags, so a machine we cannot detect still learns how to connect. The launch target table keeps its `mcpAgent` entries for those two agents: the call is what surfaces the URL, and dropping it would trade a false claim for silence.
init and launch render an indented block, and these lines sat flush against it.
This reverts the scope reduction and everything built on it. The PR keeps the harnesses main already supports and changes only how MCP gets installed: Claude Code, Cursor, VS Code, Codex, OpenCode, Hermes Agent, and OpenClaw, with native writers in place of a third-party installer subprocess. Narrowing the set to the agents that share one contract is a real argument, but it is a different change: it drops harnesses, so it has to move `firecrawl launch` and the docs with it, and it should be judged on its own rather than riding along with the writer swap. It comes back as "stop auto-configuring Hermes and OpenClaw" if we want it. So the Hermes YAML writer and the OpenClaw launcher are back, `launch` configures both exactly as it does on main, and the URL-only path invented here is gone rather than left as a second way an agent can be supported. Kept from the reverted range, since none of it depends on the smaller set: the auth-mode check ahead of every branch, rules not being written for an agent whose server entry failed, a partial run failing the command, and .cursor/ in .gitignore. The launcher isolation test now asserts both halves of that last one: OpenClaw failing is reported, and Cursor beside it is still configured.
Those three branches passed `yes: true` into installMcpClients to skip the picker, but naming the targets already does that: the picker only runs when no targets were selected, and each of these arrives with a list. What `yes` still reached was the rules prompt, so `--agent cursor` asked whether to install rules and `--agent all` silently decided no. Dropping the flag leaves the picker skipped as before and lets the question be asked. Nothing else moves: without a TTY the run is still non-interactive, `-y` still declines rules, and `--rules` still forces them.
…her gaps The Hermes/OpenClaw revert brought back unguarded launcher rule writes, PATH false positives, a Windows workspace probe that swallowed .cmd failures, and YAML setIn crashes on non-mapping mcp_servers.
The oauth report prefixed every step with "Sign in" and then repeated the verb inside the step itself, so VS Code read "Sign in sign in from the MCP view in VS Code". Checking each agent against its own docs showed the duplication was hiding worse problems: VS Code has no MCP view and runs client registration itself, Cursor surfaces a needs-login control of its own, OpenCode prompts on first use, and Claude Code shows a startup notice for any server that answers 401, which the sign-in endpoint does. Those four now print nothing. The three that do need a command keep one, and the strings are corrected: Codex leads with its own command but names the Authenticate action, since the desktop app and the IDE extension share the config file this writes and only the CLI needs the command. Hermes reloads config on a 30s timer that cannot outlast an interactive flow, so it names the login command and says to run it from a new terminal rather than claiming the browser opens by itself. The footer no longer promises that every agent prompts you.
Hermes has no global rules file to own, so every run ended its block with "Rules not supported by this agent" — a line about something the user never asked for and cannot act on. The skipped case still prints, because that one means a rule was requested and did not land.
The integrations checkbox named three editors, but MCP setup writes to every agent it detects, so the list both undersold it and went stale as agents were added. It now reads like the skills entry above it and names no agents. printNextSteps also offered "Add MCP: firecrawl setup mcp" unconditionally, including right below its own "✓ MCP server installed". stepIntegrations now reports whether the install succeeded, and the line prints only for someone who does not have it: skipping the integration or a failed install both keep it.
Quiet mode fails only when nothing lands, so a run where one agent failed resolved normally and init printed "✓ MCP server installed" directly under its own ✗ line, then hid the "Add MCP" next step that would have fixed it. installMcp now returns whether every targeted agent was configured, and init claims success only on that; a partial run points at "firecrawl setup mcp" and keeps the next step. The OpenClaw workspace lookup also expanded any leading tilde against home, so a `~other/ws` workspace, which names another account to a shell, wrote the rule into $HOME/other/ws instead. Expansion is now limited to `~` and a `~/` prefix, and reads the resolved home from the context the writers already carry rather than calling os.homedir() again.
Hermes has no global rule file, so --rules was going silent: clients only reach unsupported after rules were requested, and the dropped line hid that.
Parse Codex TOML by AST so quoted and BOM-prefixed tables are updated in place, honor CODEX_HOME and HERMES_HOME, write VS Code rules to the documented instructions path, and reject empty install-mode runs.
Do not fail the default setup bundle after skills when no agents are installed, and treat a recovered JSONC tree as registered when it still contains Firecrawl.
upsertTomlServer replaced everything between the first Firecrawl table and the last, so an unrelated [mcp_servers.other] sitting between the entry and its env sub-table was deleted along with them. Each table is now removed on its own, in reverse order so the earlier offsets stay valid in the mutated string, and the replacement goes in where the first one started. The first table keeps its terminating newline. The replacement block is written without one, so a config whose next table follows immediately, which is the documented stdio layout of [mcp_servers.firecrawl] above [mcp_servers.firecrawl.env], came back as `url = "..."[mcp_servers.other]` and failed to reparse. The user saw "could not parse existing config" about a file that was valid TOML. runClientCommand moves to its own module. Doctor's OpenClaw probe called execFileSync directly, which cannot launch a .cmd shim, so on Windows it reported the server unregistered for the exact reason the wrapper exists. OpenClaw also counts as installed when the launcher is on PATH rather than only when ~/.openclaw exists, which is what setup already detects.
init passed `--agent all` whenever FIRECRAWL_API_KEY was exported. That was a workaround for a guard this branch removed, back when environment-backed setup refused to run without a named agent, and `all` now means every supported agent whether or not it is installed. On a machine with only Claude Code it wrote six config files, including ~/.cursor/mcp.json and VS Code's, then failed on OpenClaw and ended the run pointing at "firecrawl setup mcp". With no agent named, detection runs instead and resolves the exported key on its own, so the same machine gets one file and a clean finish. The rule fence now pairs the last two markers rather than the first two. A file carrying an odd marker, from a half-written run or a hand edit, paired that stray one with our opening marker, so the next run replaced the span between them and took the user's own text with it. Also: `openclaw config get` gets the 8s timeout doctor's probe already has, since its stderr is discarded and a wedged launcher would otherwise hang setup with nothing on screen; a launcher result records the mode the run configured instead of folding oauth in with keyless; and the `--agent` help no longer advertises the environment-backed requirement that is gone.
…e writing setup defaults matched web_search by line and rewrote whatever it found. A user who had deliberately set web_search = "enabled" got it flipped to "disabled", and the inline comment explaining why went with it. The already-disabled check compared the whole line as a string, so web_search="disabled" without spaces was rewritten too and reported as a change. A test asserted the overwrite, so it was locked in rather than accidental. web_search is now read from the TOML AST that this branch already parses for Codex MCP config. A value the user set is reported and left alone; only an absent key is written, and only the value this command writes is removed on undo. A config.toml that does not parse is skipped rather than rewritten. Claude settings are patched through the same JSONC editor used for MCP config, so comments survive. Previously they were stripped to parse and lost on write. Both writers can run as a dry run, so an interactive run now prints the exact line each agent will receive and asks before touching anything. --yes and non-interactive runs are unchanged. The closing line no longer claims an agent was configured when its existing value was kept.
…rules setup mcp wrote instruction files telling each agent to prefer Firecrawl for web work. A rule is only advice: the agent can still call its own web search, and it cost a rule spec per agent, a marker-fenced writer for shared files, a launcher workspace probe, and a five-state reporting surface. Registering the server is now all setup mcp writes. In its place the command offers a second step: hand its native web search and fetch over to Firecrawl. Claude Code and Codex each document a switch for that, so those are the two agents offered, and only when their server entry landed in the same run. An agent left without a working Firecrawl server never has its own tools taken away. Turning off a built-in tool is a real behavior change, so it is confirmed: the exact line for each config is printed before anything is written, a value already set is reported and kept rather than replaced, and declining writes nothing. -y stays server-only; automation opts in with --defaults and out with --no-defaults. setup defaults still runs the same step on its own. Removes the rule spec, the rule writers, the marker fence, the launcher workspace lookup, and the dead local left over from resolving auth in installMcpClients.
Max17190
force-pushed
the
feat/native-mcp-setup
branch
from
August 18, 2026 22:18
79f2aa6 to
b7c9ab8
Compare
Contributor
Author
|
@cubic-dev-ai review this PR |
@Max17190 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
When one selected agent failed and Claude Code or Codex succeeded, reporting threw before the handover ran, so --defaults never reached the agent that was configured. The step is already scoped to agents whose entry landed, so the throw was acting as a safety gate it did not need to be. Reporting now only reports. The aggregate failure moves to its own step that runs after the handover, keeping both existing outcomes: quiet callers still fail only when nothing landed, and an interactive run still fails when any requested agent did not.
Four defects in the defaults writer, all reproduced before the fix.
The Codex insert anchored on the first line of the last root key rather than the
end of its value, so a multi-line root value had web_search spliced into the
middle of it. An inline array left the file unparseable while the run reported
success; a triple-quoted string silently corrupted the user's value and never
set web_search. The offset now comes from the end of the value, and the edited
document is re-parsed before it is written, matching how the MCP TOML writer
already guards itself.
Claude settings guarded the read of `permissions` against a non-object but not
the write, so `{"permissions": null}` parsed fine, previewed fine, was confirmed
by the user, and then threw out of jsonc-parser. A permissions value of the
wrong shape now has the whole key replaced, the same way the MCP JSON writer
handles a malformed servers map.
Both files ignored CODEX_HOME and CLAUDE_CONFIG_DIR, which this branch honours
everywhere else, so on a relocated config tree the server entry and the switch
landed in different places and the switch went to a file the agent never reads.
The step is an offer on top of the work the command was asked to do, so an
unreadable file or a permissions error no longer fails a run whose MCP write
already succeeded and was reported.
Also drops an em dash from an init picker label.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
setup mcpshelled out to a third-party installer, putting a package we do notcontrol on the critical path of a first-run command and capping which agents we
support. It also only ever registered the server, and registering a server does
not stop an agent reaching for its own web search first.
Summary
setup mcpnow configures agents directly: it detects what is installed, liststhose in a picker (already selected), writes the server into each one chosen,
and then offers to hand over native web search where the agent documents a way
to turn it off.
OpenClaw. Every config shape and environment-reference syntax is verified
against vendor documentation; an agent stays out until it is.
document a switch, so they are the two offered, and only when their server
entry landed in the same run. An agent left without a working Firecrawl
server never has its own tools taken away. The exact line for each config is
printed before anything is written, and declining writes nothing.
web_searchis read from the TOML AST rather than matchedby line, so a value you set yourself is reported and kept rather than
replaced, and only the value this command writes is removed on undo. Claude
settings are patched through the JSONC editor, so comments survive.
-ystays server-only;automation opts into the handover with
--defaultsand out with--no-defaults.setup defaultsstill runs the same step on its own, withthe same preview and the same no-clobber guarantees.
-gis still accepted forexisting scripts, hidden from help, and reported as deprecated when used.
--projectis no longer accepted onsetup.FIRECRAWL_API_KEYisexported where the agent will run, each agent gets a reference to that
variable in the syntax it expands. Otherwise setup stays keyless and says so.
--oauthpoints agents at the sign-in endpoint instead.supported agents ship by default. TOML tables are replaced cleanly, including
stale sub-tables from an earlier stdio entry, and CRLF files keep their line
endings. Reruns are byte-identical.
against that agent rather than ending the run. A malformed config fails only
its own agent and is never overwritten.
Test Plan
pnpm test(518),type-check,format:check,buildsetup mcp -ywrites server entries only and changes no web-tool settingssetup mcp -y --defaultsdenies ClaudeWebSearch/WebFetchand sets Codexweb_search = "disabled"web_searchvalue is preserved, inline comment included, and reported~/.claude/settings.jsonsurvive; unrelated deny entries preserved