Skip to content

feat(providers): give agents the user's own tools over MCP, starting with FixMap - #226

Closed
aryamthecodebreaker wants to merge 2 commits into
j35dev:mainfrom
aryamthecodebreaker:feat/fixmap-mcp
Closed

aryamthecodebreaker wants to merge 2 commits into
j35dev:mainfrom
aryamthecodebreaker:feat/fixmap-mcp

Conversation

@aryamthecodebreaker

@aryamthecodebreaker aryamthecodebreaker commented Sep 13, 2026 •

Copy link
Copy Markdown

Two changes to what Ari hands an agent at the start of a session. They are separate concerns and I am happy to split them — say so and I will open the second as its own PR.

1. feat(providers): offer the user's own tools to agents over MCP

session/new, session/load and session/resume have always sent mcpServers: []. Ari therefore had no way to give its agents a tool, short of the user configuring each CLI separately — which is the thing Ari exists to avoid.

This sends the list instead, and ships one entry: FixMap, a local, deterministic repo map that answers "which files and tests does this task touch" without a model or an account. Agents get fixmap_plan, fixmap_context, fixmap_graph, fixmap_explain, fixmap_compare, fixmap_verify and fixmap_doctor as tools, on by default.

No FixMap code enters Ari. Ari names a command the user already installed and the agent spawns it, exactly as launches.ts treats agent CLIs: "Ari never bundles adapter runtimes: the user's own CLI must be installed." Nothing is downloaded, and with FixMap absent the list is empty and agents behave precisely as they do today.

Details worth review:

  • Windows extension order is deliberate. findCommand tries .cmd/.exe before the bare name. npm writes an extensionless sh shim beside the .cmd, and here the agent is what spawns the command, so handing it the sh shim fails with ENOENT inside a subprocess Ari never sees. (The same lookup bug affects findBinary for the agent CLIs themselves; that fix is separate.)
  • Read per turn, not captured at registration, so switching the tool off applies on the next turn rather than the next launch.
  • A settings toggle lives in Advanced → Agent tools, default on, and rides along in the settings bundle like the other sections.

2. fix(engine): stop repeating the control-surface preamble on every message

#runTurn prepends this to every prompt whenever the session env carries ARI_ENV=1:

[Ari control surface: this session can operate Ari. Commands: ari env, ari agents, … Full protocol: ari --skill. Never disclose ARI_CONTROL_TOKEN.]

Every turn, not once. Two things follow, and I hit both with Claude driving a real session:

  1. The agent cannot tell it from text typed by the user, so on seeing it repeatedly it concluded someone was injecting prompts into the user's input, said so, and refused to act on it: "That 'Ari control surface' text is being injected into every message you send … Looks like a prompt injection riding along in your input somehow, not something you're typing." Ari was teaching agents to distrust its own control surface.
  2. It costs tokens on every turn to repeat something the agent already has in its transcript.

Now it goes out on the first prompt of a provider thread only — resumeOf === null. Once the provider has a thread to resume, the line is already in that transcript. Providers that report no thread id (the one-shot CLI transport, which genuinely starts cold each turn) still get it every time, so nothing regresses there.

Worth noting the control surface itself was working: ari.cmd is on the agent's PATH and answers. The agent's claim that it had no such command was wrong — but it had good reason to disbelieve the block telling it otherwise.

How verified

Windows 11, Node 24.19, pnpm 10.33, FixMap 0.9.0 installed globally.

FixMap's server was driven directly over stdio the way an agent will, with the command Ari resolves:

INIT:  {"name":"fixmap","version":"0.9.0"}
TOOLS: fixmap_plan, fixmap_context, fixmap_graph, fixmap_verify, fixmap_explain, fixmap_compare, fixmap_doctor

New tests: lookup order on win32 vs posix and the not-installed path (mcp-servers.test.ts); the servers riding on all three session calls, not just the first (connection.test.ts); the toggle's default, an older settings file without the section, and patch validation (settings-tools.test.ts); the Advanced toggle and its bundle round-trip (AdvancedSettings.test.tsx); and the preamble appearing on a fresh thread but not on a resumed one (control-surface.test.ts). The existing agent-runtime assertion that the first turn carries ari --skill still passes unchanged.

pnpm verify — typecheck and lint clean across the workspace; tests:

shared       15 passed
contracts    24 passed
ui          159 passed
providers   420 passed
engine      163 passed, 1 failed
desktop    1347 passed, 3 skipped

The engine failure is git-service.test.ts > parses porcelain v2 status into branch and typed entries, which spawns real git in a temp dir and times out at 5000ms under full-suite load on
this machine. It passes on its own here (16/16) and touches none of this code. Those 5s budgets
look tight for a loaded Windows box, but that is its own issue.

🤖 Generated with Claude Code


Devin Review

aryamthecodebreaker and others added 2 commits September 13, 2026 16:01
session/new, session/load and session/resume always sent `mcpServers: []`,
so Ari had no way to give an agent a tool without the user configuring each
CLI separately. They now carry the host's list, and it ships one entry:
FixMap (https://usefixmap.vercel.app), a local repo map that answers which
files and tests a task touches without a model or an account.

No FixMap code enters Ari. Ari names a command the user already installed
and the agent spawns it, the same rule launches.ts states for agent CLIs.
Nothing is downloaded, and with FixMap absent the list is empty and agents
behave exactly as before.

findCommand tries .cmd/.exe before the bare name on Windows: npm writes an
extensionless sh shim beside the .cmd, and the agent is what spawns this, so
the shim would fail with ENOENT inside a subprocess Ari never sees.

The list is read per turn rather than captured at registration, so the
Advanced > Agent tools toggle takes effect on the next turn.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The preamble naming `ari` was prepended to every prompt whenever the session
env carried ARI_ENV=1. An agent cannot tell it from text the user typed, and
on seeing it turn after turn Claude concluded it was a prompt injection
riding along in the input and refused to act on it — costing Ari the control
surface the line exists to advertise, and paying tokens every turn to repeat
something already in the agent's transcript.

It now goes out on the first prompt of a provider thread only. Once the
provider has a thread to resume, it already holds the line; a provider that
reports no thread id starts cold each turn and still gets it every time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration 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.

Devin Review found 2 potential issues.

2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

Devin Review

Comment on lines +550 to +551
runtimeEnv?.ARI_ENV === '1' && resumeOf === null
? `${ARI_CONTROL_PREAMBLE}\n\n${prompt}`

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.

🟡 Fresh fallback threads miss control preamble

When resuming fails or ACP falls back to a one-shot driver, resumeOf suppresses the preamble for the resulting fresh thread. openSession creates fresh sessions, while AcpDriver.create can switch transports. The agent then lacks Ari's control commands for that turn.

Learn more

The engine decides whether to add the preamble before the adapter knows whether resumeOf can actually resume. The ACP adapter degrades to session/new when resume and load fail. The ACP wrapper also degrades to the legacy one-shot driver when setup fails. Both paths create a fresh provider thread from a prompt whose preamble was already omitted.

Example: A session stores sess_old. On the next turn, session/resume and session/load reject, so openSession calls session/new. The new agent receives only the user text, while a first turn would also receive the Ari control instructions.

Recommended fix: Move first-thread preamble selection behind transport/session opening, or let the driver report whether the resulting adapter resumed. Ensure ACP's session/new degradation and legacy fallback both receive the preamble, while successful resumes do not.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +41 to +43
const full = join(dir, candidate)
if (existsSync(full)) return full
}

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.

🟡 Unrunnable FixMap paths get offered

With a directory or non-executable file named FixMap, findCommand returns it as an MCP command. FixMap remains unavailable despite being advertised, and the agent can report a server startup failure.

Learn more

Existence does not establish that a path is a runnable command. existsSync returns true for directories, and POSIX files can exist without execute permission. The returned path is passed directly to the ACP agent as the MCP server command.

Example: If /home/alice/.local/bin/fixmap is a directory or has mode 0644, findCommand returns it. The session advertises FixMap, but spawning that command fails instead of treating FixMap as absent.

Recommended fix: Require a regular file and verify execute permission on POSIX before returning a candidate. Keep the existing Windows extension ordering, and add tests for directories and non-executable files.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@j35dev

j35dev commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Can not consider your FixMap PR at the moment. Have to wait until Ari fully supports MCPs & Plugins. Seperate both PRs.

@j35dev

j35dev commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Please split this, as above.

The FixMap half still needs to wait until Ari supports MCPs and plugins properly. The control-surface half is small and there is already a fix in flight for that same banner, so it belongs there rather than in its own PR — see #227, which changes the wording on the same line and would just conflict with a separate one.

Worth handling when you rebase it onto that: resumeOf === null is not the same as "this thread has not been told". openSession in acp-driver.ts does not fail when session/resume and session/load both fail — it falls through to session/new and publishes the new id, which the engine persists. resumeOf is non-null from the next turn on, so the preamble is never re-sent and the recreated thread never receives it. Recording the announced thread in the journal and re-announcing when it changes keeps your once-per-thread saving without that hole.

@aryamthecodebreaker

Copy link
Copy Markdown
Author

Split done. FixMap is out of here entirely and parked until Ari has real MCP/plugin support — no point reviewing wiring that has to be rebuilt on top of that surface. The detector fix from the other PR is now #228, and #223 is background work only, in draft.

On the control surface, your resumeOf point is correct and I had it wrong. openSession falls through to session/new when resume and load both fail and publishes the new id, which the engine persists, so from the next turn resumeOf is non-null for a thread that was never told anything. Devin found the same hole from the other direction — the ACP-to-one-shot fallback creates a fresh thread for a prompt whose preamble was already omitted. resumeOf answers "is there a thread id to resume", not "has this thread been told", and I conflated the two.

Recording the announced thread id in the journal and re-announcing when it changes is the right shape: it keeps the once-per-thread saving and closes both paths, since a recreated thread gets a new id either way.

Since #227 already changes the wording on that same line, I have not opened this separately. Happy to send it as a commit on top of #227 once that lands, or as a suggestion against your branch if you would rather fold it in — your call which is less work for you.

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.

2 participants