fix(runtimes): reach a working agent CLI past a broken shim - #7153
fix(runtimes): reach a working agent CLI past a broken shim#7153lefarcen wants to merge 3 commits into
Conversation
Resolving an agent's name on PATH only proves a file exists there, never that it runs. Detection stopped at the first hit, so a wrapper orphaned by a half-finished `npm i -g` shadowed a perfectly good CLI of the same name in a later search directory — and because the not-invocable branch dropped the resolved path, the picker (which renders an unavailable agent only when it carries one) hid the agent entirely. The user saw "scan found nothing" with no way to act. Detection now walks past candidates that cannot be executed, and reports the path it tried when every candidate fails, so the row stays actionable. Also on this chain: - `@open-design/dsh-runtime` pinned its DeepSeek Harness peers to one exact release candidate, so it became uninstallable the moment upstream shipped the next one. Peers now take ranges. `scripts/guard.ts` gains a narrow, logged exemption for external-host plugin manifests — their peers describe a host this repo neither installs nor controls, which is the one case where an exact pin is wrong rather than reproducible. - The DSH installers pinned only the entry package while its ~190 siblings used caret ranges over prerelease floors. Once a newer release candidate existed, the transitive tree floated onto it and its mutually exclusive peer sets sent npm into an ERESOLVE backtrack that never converged. Both installers now freeze the resolution window alongside the pin. - The rescan notice counted agents the list never renders, so it announced "3 available" above two rows.
lefarcen
added
size/L
|
🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable). |
🚀 Landing page previewThis PR is deployed to a Cloudflare Pages preview — not staging or production:
|
Visual regression reviewHead: 0 changed · 49 unchanged · 0 new without baseline · 0 failed Unchanged cases
Visual diff is advisory only and does not block merging. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
Thanks @lefarcen — the Settings visibility fix, the installer generation freeze, and the narrow guard exemption for external-host peers are all thoughtfully done.
One launch-path gap still blocks the stated goal: detection can walk past a broken shim and mark the agent available, but chat/run still resolve the first PATH hit. Details are inline.
🔁 Powered by <a href="https://github.com/nexu-io/looper\">Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
… use The candidate walk taught detection to skip a binary it cannot execute, but every other resolution — chat, the connection test, the memory summariser, companion install — called `resolveAgentLaunch` with no skip list and got back the first file that merely exists on PATH: the broken shim detection had just rejected. Settings advertised the agent as installed while each turn exec'd the wrapper, so the fix only ever reached the picker. Detection is the only stage that spawns anything, so it is the only one that learns which candidate works. It now publishes that winner, and resolution prefers it over a fresh first-hit walk. An explicit `*_BIN` override and a packaged built-in still outrank it, a winner whose file has since vanished is ignored, and detection clears the entry before each pass so a rescan after a repair never resolves against a stale one. This restores the invariant the file already documented — detection probes the exact path the runtime will spawn — which the walk had broken. Reported by @mrcfps in review.
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@lefarcen thank you for the careful follow-up — this is a really solid bug-fix chain.
I re-reviewed fd5716d against the earlier launch-path split. Detection still walks past a not-invocable PATH hit, then rememberDetectedExecutable publishes the winner and inspectAgentExecutableResolution prefers it, so resolveAgentLaunch (chat, connection test, memory, companion setup) now lands on the same binary Settings reported. Overrides and packaged built-ins still outrank the cache, a vanished winner falls through to a live walk, and forgetDetectedExecutable at the top of probe() keeps a rescan honest. The new fallback test asserting both detectAgent() and a later resolveAgentLaunch(def) closes the hole that went red on the previous head.
On the sandbox-keying question: agent-id-only matches detectedRuntimeVersions and is the right call. OD_AGENT_HOME is process-wide, so a sandboxed detect cannot populate a winner that a non-sandboxed spawn in the same daemon would reuse.
The rest of the PR holds up too: unavailable agents keep their attempted path so the Settings row stays actionable, the rescan count ignores hidden CLIs, the installer freeze (version + shared --before cutoff) is tested on both scripts, and the guard exemption is peer-only, allowlisted, and visible in the pass line.
Really nice work tracing one Windows field report into four real defects and then closing the review gap without widening the design. 🙏
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
…te set The winner map made resolution stop being a pure function of the current environment. It only checked that the remembered file still existed, so a path learned in a richer environment survived an emptied PATH, a sandboxed OD_AGENT_HOME, and any other narrowing of the search — resurrecting a binary the caller could no longer see. That is not just a test artifact: `GET /api/integrations/vela/status` must answer `amr-runtime-unavailable` when the vela binary cannot be resolved, and it started answering "ready" instead, because an earlier detection in the same process had remembered a path. The memory now reorders candidates rather than introducing them: it applies only when the live search already offers the remembered path. An emptied PATH, a sandbox, and an uninstalled CLI all keep meaning "not found", while a broken shim shadowing a working binary is still resolved to the winner. This also settles the scope question raised in review — no separate cache key is needed, because the candidate set already carries the resolution scope.
|
Pushed 3b4c2f7 — CI caught a regression I introduced in the previous commit, so recording what happened here. What broke. The winner map from the last commit made binary resolution stop being a pure function of the current environment. It only verified that the remembered file still existed, so a path learned earlier in the process survived an emptied
That route must answer Fix. The memory now reorders candidates instead of introducing one — it applies only when the live search already offers the remembered path. An emptied PATH, a sandbox, and an uninstalled CLI all keep meaning "not found", while a broken shim shadowing a working binary still resolves to the winner. A regression test pins the invariant: detect against a populated PATH, then empty PATH and assert This also answers the scope question I raised in the thread above. No separate cache key is needed for sandboxed passes — the candidate set already carries the resolution scope, so a winner from a richer environment can never leak into a narrower one. Disregard that open question. Validation on this head
Re-reviewing is welcome but not required from my side — the approval predates this commit, so treat it as your call whether the fix changes anything for you. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@lefarcen thank you for the careful CI follow-up — this is a really solid close-out of the winner-map regression.
I re-reviewed 3b4c2f7 against the earlier existence-only cache. preferRememberedExecutable now only reorders paths that the live search already offers, so an emptied PATH, a sandboxed OD_AGENT_HOME, or a vanished CLI stays “not found” instead of resurrecting a file that happens to still be on disk. That matches the vela status failure you reproduced (503 / amr-runtime-unavailable vs a stale ready), and the new fallback test pins it: detect against a populated PATH, empty PATH, assert resolveAgentLaunch returns null even though the binary is still there.
The rest of the chain still holds. Detection walks past a not-invocable PATH hit, publishes the winner, and later resolveAgentLaunch (chat, connection test, memory, companion setup) lands on the same binary Settings reported. Overrides and packaged built-ins still outrank the memory, forgetDetectedExecutable at the top of probe() keeps a rescan honest, unavailable agents keep the attempted path so the Settings row stays actionable, the rescan count ignores hidden CLIs, both installers freeze the same version + --before cutoff, and the guard exemption is peer-only, allowlisted, and visible in the pass line.
Really nice work turning one Windows field report into four real defects and then tightening the cache so resolution stays a function of the current environment. 🙏
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
2 participants
Why
A user installed DeepSeek Harness on Windows and OpenDesign still reported "scan found nothing". Chasing it turned up four separate defects on one chain, three of which affect every agent, not just DSH.
The root cause: their machine had two
dshbinaries — a stale wrapper left in%APPDATA%\npmby an earlier failednpm i -g, and the working one the official installer wrote to~/.local/bin.%APPDATA%\npmranks earlier in our search order (packages/platform/src/toolchain.ts:115vs:118), so detection resolved the broken one, failed to execute it, and gave up — never reaching the healthy CLI sitting one directory later.It then made the failure invisible: the
not-invocablebranch calledunavailableAgent(def, [diagnostic])without a path, and the picker only renders an unavailable agent when it carries one (deepSeekHarnessNeedsSetup). So the agent vanished from Settings with no diagnostic, no fix action, nothing to click.Two more defects surfaced while verifying:
@open-design/dsh-runtimewas uninstallable against any newer host. Its peers were pinned to an exact release candidate, so when upstream shipped the next one, peer resolution failed and our connection component could no longer be installed at all.@deepseek-ai/dsh, while its ~190 siblings declare each other with caret ranges over prerelease floors. npm reads^0.1.0-rc.6as "this prerelease or anything newer", so the transitive tree floated onto the newest release candidate while the entry package stayed pinned. Those generations peer-require their own siblings, so the mixed tree has no solution — npm backtracks across a combinatorial space forever. To the user it looks like an endless scroll of ERESOLVE warnings that never completes.What users will see
dsh— OpenDesign no longer requires one exact release candidate.Surface area
0.1.0-rc.8instead of0.1.0-rc.6Screenshots
Not attached — the UI change here is not a new surface but a row that stops disappearing, which needs a machine with a shadowed binary to photograph. Verified instead at the daemon HTTP boundary against a real
dsh, which is what drives that row:Before (baseline
mainbuild), same machine, same PATH:After:
Happy to add UI captures if a reviewer wants them.
Bug fix verification
apps/daemon/tests/runtimes/executable-fallback.test.tsexpected true, received false(working CLI unreachable behind the broken one)expected "<path>", received undefined(path dropped, so the row is hidden)apps/landing-page/tests/install-dsh-static.test.tsgains a case asserting both installers freeze the resolution window and agree on version + cutoff, so a future version bump that forgets one half fails the build.apps/web/tests/utils/visibleAgents.test.tscovers the count/render mismatch and the path requirement that made the row disappear.Validation
pnpm guard— exit 0 (dependency spec check now reports7 external-host peer ranges, so the exemption is visible, not silent)pnpm typecheck— exit 0 across all packagespnpm --filter @open-design/daemon test— 52 files, 756 passed / 2 skippedpnpm --filter @open-design/landing-page test— 148 passedpnpm --filter @open-design/dsh-runtime typecheck && build && test— exit 0, 12 passed, with devDependencies raised to0.1.0-rc.8, so cross-version compatibility is compiler-verified rather than assertedapps/web—tests/utils/(111 passed) plusSettingsDialog.execution/AgentDiagnosticRow/App.onboarding-agent-autoselect(151 passed)End-to-end, beyond the unit layer:
install-dsh.shfor real against the live npm registry:added 452 packages in 8m, exit 0, zero ERESOLVE lines, tree uniformly0.1.0-rc.8, launcher reports0.1.0-rc.8. The same install pre-fix does not converge.dshwith a deliberately broken shim earlier on PATH, and queried/api/agents— the before/after above. The baseline run used amain-equivalent build (source changes stashed and rebuilt), so the comparison is against actual pre-fix behavior, not a recollection of it.open-designprofile installed by Open Design Beta on 2026-08-14, carrying the rc.6-era@open-design/dsh-runtime@0.1.0. It completed the--probehandshake against the rc.8dshbinary, so runtime compatibility across release candidates is demonstrated, not just compile-time.Note for reviewers
scripts/guard.tsis the one change that touches a repo-wide convention, and it deserves a deliberate look. The rule "all dependency specs must be exact" is right for everything we install. It is wrong for one narrow case: a manifest published for an external host to load as a plugin, whosepeerDependenciesdescribe packages the user's host supplies and this repo cannot pin. Pinning those exactly is precisely what madedsh-runtimeuninstallable.The exemption is a one-entry allowlist covering
peerDependenciesonly —dependenciesanddevDependenciesin the same manifest still must be exact — and guard prints the count so it can't rot silently. If you'd rather solve this another way, the alternative is keeping exact peers and accepting that DSH support breaks on every upstream release candidate.