test(cli): stop the registry plan check depending on the developer's machine

[CaptainMittens]

The problem

cli_agent_client_registry_routes_plan_install_and_uninstall builds a fixture with its own HOME and PATH, then asserts the whole install plan contains no path with /plugins/ in it.

Agent detection does not stay inside HOME and PATH. cbm_find_cli also looks in /usr/local/bin and, on macOS, /opt/homebrew/bin. A developer with OpenCode installed there has it detected inside the fixture, and OpenCode ships a real plugin file — so the plan legitimately carries a /plugins/ path and the assertion fails. On a machine without OpenCode the same assertion passes, which is why CI has never seen it.

The change

Name the two directories the check actually meant — the fixture's Qoder and Pi directories, neither of which has a plugin directory, so a planned path under one would be invented:

char qoder_plugins[700];
char pi_plugins[700];
snprintf(qoder_plugins, sizeof(qoder_plugins), "%s/plugins/", qoder_dir);
snprintf(pi_plugins, sizeof(pi_plugins), "%s/plugins/", pi_dir);

The check now tests the same thing regardless of what the developer has installed.

One file, tests/test_cli.c, 12 insertions and 1 deletion. No production behaviour changes. Searching well-known install directories is deliberate, so that install finds an agent whose command is not on the current PATH — that is not what this touches.

Evidence

Run on a machine that has OpenCode installed, so the trigger condition is present.

Before, at c38aa35c:

FAIL tests/test_cli.c:6725: CLI install/plan/uninstall must route the agent-client
registry, preserve foreign entries, and keep Pi free of invented MCP configuration
  cli_registry_installs_kimi_rovo_amp_durable_context
  Detected agents: OpenCode Kimi Code CLI Rovo Dev CLI Amp

After, on this branch:

cli_agent_client_registry_routes_plan_install_and_uninstall
  Detected agents: OpenCode Qoder CLI Roo Code Amazon Q Developer IDE Pi Sourcegraph Cody

OpenCode is detected in the fixture in both runs, so the condition that provokes the failure is present either way. The assertion fires in the first and not in the second.

One thing worth stating plainly

The cli suite has other failures on this machine that this change does not address and does not claim to. They vary between runs — the agent-client install and uninstall assertions cannot drain a cohort while a permanent daemon is running, so the set differs each time. origin/main itself reports 536 passed / 6 failed here, and three of those six print the same Detected agents: OpenCode line, at tests/test_cli.c:9390 and :9491 alongside the one fixed here.

So this fixes one instance of a wider pattern rather than the whole pattern. I did not extend it to the other two, because the right fix for each depends on what its own assertion means and I did not want to guess that on your behalf. Happy to follow up on those if the approach here is the one you want.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ERsNp8UNLaixL4uUz7hRTX

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Approved. I verified the mechanism rather than taking it: src/cli/cli.c:977 and :982 hardcode /usr/local/bin/%s and /opt/homebrew/bin/%s, so agent detection genuinely escapes the fixture's HOME and PATH, exactly as you describe.

This is the more valuable half of the fix, and worth stating plainly: the assertion's verdict depended on what the developer happened to have installed. A machine with OpenCode in /opt/homebrew/bin fails; one without passes. A test like that does not report on the code — it reports on the workstation, and a green from it proves nothing.

That is the second finding of this shape we have seen in as many days. #2003 (filed by another contributor) is the same class from the other direction: git exports GIT_DIR into hook environments, where it overrides git -C, so the git-shelling tests in test_pipeline.c assert against whatever repository the developer is standing in. Both are tests whose result is a function of ambient environment rather than of the change under test.

And you fixed it the right way round. The tempting repair is to harden the fixture — sandbox PATH harder, stub the search directories — which fights the production code and tends to drift. Instead you narrowed the assertion to what it actually meant: the fixture's own Qoder and Pi directories, neither of which has a plugin directory, so a planned path under either would be invented. The check now tests one thing, deterministically, and stays true as detection changes.

Your one red is ours

test / test-msan fails at the "Build MSan image (cached layers)" step, with the "MSan suite" step skipped — so no test of yours ever ran. That is a known image-build failure on our side, unrelated to a twelve-line test change.

Merging once the remaining checks report. Thank you — this is the sort of fix that pays for itself the next time someone trusts a green.

[DeusData]

Merged as bac8edc5 — thank you!

This is the second machine-dependent assertion you have caught this week, and the fix is the right way round again: the test now names the two fixture directories it actually cares about instead of searching the whole plan for /plugins/, so a developer with OpenCode in /opt/homebrew/bin and one without get the same verdict. Exactly the kind of test that stops the suite lying to whoever runs it next.

Small note for the record, not a change request: the retained !strstr(plan, "plugin_files") clause guards a key the plan builder never emits, so it is inert — harmless, just not load-bearing.