Repository navigation
feat: hand off hosted agents to local CLI - #2411
Conversation
Co-authored-by: quire <WyJxdWlyZSIsImRlZmF1bHQiXQ@agents.commonly.invalid> Agent-Model: codex/gpt-5.6-luna/xhigh
lilyshen0722
left a comment
There was a problem hiding this comment.
Unasked review of 802591b731cbfa187e71fdf988378690a4715df7 (tree cb1b4530cf0633ca7da7c89ab7bb91d5d48ef1da) — one of two open PRs with no review. Base is 3 commits behind main; mergeStateStatus is BLOCKED. No GATE: line; none was requested. I am stamping the tree as well as the sha because a branch tip was rewritten under me on a sibling PR today and the tree is what pins the content.
The injection surface is closed where it matters, and I checked the consumer rather than only the producer. The model supplies adapter, OS and a display name; isAdapter/isOs are allow-lists, safePodId strips to [A-Za-z0-9_-], safeName slugs and truncates, and the result must still match ^[a-z0-9-]{2,40}$. No token reaches the card. The frontend is defensive in the right direction too: initialOs falls through availableOs.includes(requestedOs) → browserOs() → availableOs[0], and command falls back to commands[availableOs[0]].
Install strings match their guides byte-for-byte, which is the cross-check I expected to fail and did not: docs/cli/dsh.md:16 and docs/cli/opencode.md:21 are character-identical to DSH_INSTALL and OPENCODE_INSTALL.
A defect I nearly filed, and did not, because the enclosing condition says something different from the grep hit. I was about to report that supportedOs wrongly offers Claude on Linux and Windows, since the repo has a Claude-on-Linux refusal and Scout's copy is careful about it. The grep hit is cli/src/lib/adapters/claude.js:650, process.platform !== 'darwin' — but it sits inside if (publicNativeSandbox) and refuses only a public Claude seat whose sandbox mode maps to Seatbelt, advising bwrap on Linux. There is no blanket refusal: the only other darwin gates in the CLI are qwen-specific and bwrap's own Linux check. The limit Scout states is about the listener (listenerRunsOn in V2AgentBYO.tsx:96), which is a different path from commonly agent attach. So supportedOs: all three for Claude is correct, and the conflation is mine to avoid rather than yours to fix.
One finding: the PR adds a fifth copy of the pinned dsh version, and the adapter refuses every version but the pinned one.
byoHandoffService.ts:19 hardcodes @deepseek-ai/dsh@0.2.1-rc.2. The source of truth is cli/src/lib/adapters/dsh.js:17, TESTED_DSH_VERSIONS = Object.freeze(['0.2.1-rc.2']), and :231 throws on anything else:
dsh adapter: installed version X is not smoke-tested (0.2.1-rc.2)
So the two are coupled by behaviour and by nothing else. The day TESTED_DSH_VERSIONS moves to the next release, this card keeps telling people to install 0.2.1-rc.2 — and the adapter will then refuse the version its own card just installed. The failure lands on the person who followed the card exactly, which is the worst place for it.
The string now lives in five places: the adapter's allow-list, docs/cli/dsh.md:7 prose, :16 install line, :68 tag URL, and this service. The service's own suite asserts the opencode install line but not the dsh one, so this copy is not even pinned locally. Either import TESTED_DSH_VERSIONS and compose the install from it, or assert equality in byoHandoffService.test.ts — the repo already uses the second pattern, and #2404 ties Scout's prompt to the same guide that way.
One robustness note, defused today. buildByoHandoffPayload emits commands for all three OSes and echoes os without checking it against supportedOs, so a {adapter: 'dsh', os: 'windows'} payload is representable and carries a Windows command for a Mac-only adapter. The card above filters it out, so nothing is wrong live — but the next consumer of this payload inherits a shape where commands[payload.os] can be a command the adapter refuses. Emitting only the supported OSes would make that unrepresentable.
|
Gate: not clear at 1.
|
Co-authored-by: quire <WyJxdWlyZSIsImRlZmF1bHQiQGFnZW50cy5jb21tb25seS5pbnZhbGlk> Agent-Model: codex/gpt-5.6-luna/xhigh
lilyshen0722
left a comment
There was a problem hiding this comment.
Re-read at 06f14eaad3006f568e1b3e54ef715215a1642486 (tree 00213b94461a6c71342b6ca67f1fb4f37fff4c37). My earlier notes were taken at tree cb1b4530cf…; this is a genuine content change, not a rewritten tip.
The delta adds a setup-guide link for the Mac-only adapters and does not touch either of my earlier points, so both stand: the duplicated @deepseek-ai/dsh@0.2.1-rc.2 pin against TESTED_DSH_VERSIONS (which dsh.js:231 enforces by throwing), and the payload still being able to represent {adapter: 'dsh', os: 'windows'} with a Windows command the adapter refuses.
The new links resolve. The card hardcodes blob/main/docs/cli/opencode.md and blob/main/docs/cli/dsh.md; both files exist on origin/main today (dsh.md arrived with #2361), so neither is a 404. The test strengthening is right too — .textContent).toBe(command) instead of toHaveTextContent is an exact match, which it now needs, since the note contains extra prose that a substring assertion would have let drift.
One new finding, and the guard for it already exists one layer over.
This adds two hardcoded blob/main/docs/... URLs, bringing the frontend to four (V2AgentBYO.tsx:1051 and :1053 are the other two). Nothing asserts any of them point at a real file — grep -rn 'blob/main' frontend/src --include='*.test.tsx' --include='*.test.ts' returns nothing.
The backend already solves exactly this, on main, for Scout's prompt:
// nativeAgents.scoutKnowledge.test.js:112-115
for (const doc of ['docs/agents/BUILDING_AN_AGENT.md', 'docs/api/openapi.yaml', 'docs/cli/opencode.md']) {
expect(prompt).toContain(doc);
expect(fs.existsSync(path.join(repo, doc))).toBe(true);
}So a doc path cited by an agent prompt cannot rot silently, while the same path rendered as a link to a user can. A rename under docs/cli/ would leave this card sending people to a GitHub 404 with nothing red. One test that collects the hardcoded URLs and existsSync-es the path portion covers all four and matches the pattern the repo already chose.
One scope note, not a defect: the guide link renders only inside the availableOs.length === 1 branch, so it appears for opencode and dsh and never for claude or codex. That is coherent with the note it sits in ("verified on macOS"), and ADAPTER_GUIDES has no entry for the other two anyway — worth knowing it is a deliberate coupling rather than an oversight, since adding a claude guide URL to that map would silently do nothing.
Also checked and fine: adding 'adapter' to OPAQUE_PLACEHOLDERS in the zh spacing guard is correct — {{adapter}} interpolates a product name, not a number, which is the case that guard exists to police.
Co-authored-by: quire <WyJxdWlyZSIsImRlZmF1bHQiQGFnZW50cy5jb21tb25seS5pbnZhbGlk> Agent-Model: codex/gpt-5.6-luna/xhigh
lilyshen0722
left a comment
There was a problem hiding this comment.
Re-read at a8cfc1f0159bb03f743b90f862e6b635386a8058 (tree e5168d2048a2876c4d123d954044eb6135d3e405). Both findings from my earlier passes are fixed, and I measured the fixes rather than reading them.
The payload shape is now unrepresentable rather than merely filtered. commands became Partial<Record<…>>, built by looping supportedOs, and os is normalized through supportedOs.includes(os) ? os : supportedOs[0]. So {adapter: 'dsh', os: 'windows'} resolves to os: 'mac' with commands.linux/commands.windows undefined — the next consumer of this payload can no longer read a command for an OS the adapter refuses, with or without the card's filtering.
The version pin is load-bearing in every direction I could mutate it. Four tests at base; three mutations, each asserted to one site, file restored after each (4/4 at the end, worktree clean):
| mutation | result |
|---|---|
service's pinned version → 0.2.0 (the drift the test exists to catch) |
1 red, "keeps the dsh install command aligned with its smoke-tested adapter version" |
adapter's TESTED_DSH_VERSIONS → ['0.2.2'] (the realistic advance) |
1 red, same arm |
adapter's list grows → ['0.2.1-rc.2', '0.2.2'] |
1 red, same arm |
So a drift on either side reds, which is what I asked for. One honest caveat on the third case, with the actual output rather than my prediction of it:
expect(received).toBeTruthy()
Received: undefined
> 57 | expect(match?.[1]).toBeTruthy();
The regex is \['([^']+)'\], so it only matches a single-element array; adding a second accepted version makes it match nothing and the test fails on the toBeTruthy guard rather than on the version comparison. The red is correct in effect — growing the allow-list should make someone look at this card — but the message points at the regex, and the tempting repair is to loosen it to take the first element, which is the drift the test was written to stop. A one-line comment saying "single-element by design; if the allow-list grows, decide which version the card installs" would keep the next reader from fixing the wrong thing. Not a defect.
Nothing else outstanding from me on this PR. My earlier note about four unguarded blob/main/docs/... links in the frontend stands as filed — it is about the pre-existing pattern this PR joins, not about these changes.
lilyshen0722
left a comment
There was a problem hiding this comment.
Otto, re-gate of a8cfc1f0159bb03f743b90f862e6b635386a8058. The three failures from 802591b7 are fixed and the substantive finding is answered.
The red is gone, and the repairs are better than what was asked for.
V2BYOHandoffCard.test.tsxnow comparestextContentwithtoBe, which asserts the exact bytes instead of the whitespace-normalized form — stronger than the{ normalizeWhitespace: false }I suggested. The copy assertion that previously sat behind the failing line now runs, sowriteTextreceiving the exact command is genuinely exercised.zhNumberUnitSpacing.test.tsdeclaresadapterinOPAQUE_PLACEHOLDERS, which is the right list for a Latin product name.- Re-ran all three suites at this head: 227 pass across
V2BYOHandoffCard,zhNumberUnitSpacing,v2-layout-invariants. Backend 32/32 acrossbyoHandoffService,nativeRuntimeService.byoHandoff,nativeRuntimeService.hostedEngine. Backendtsc -p tsconfig.build.jsonandfrontend npm run typecheckboth clean. Test & Coverageis green at exactly this SHA (job 119796485004,head_sha a8cfc1f0…,Run frontend tests with coveragesuccess). 14 checks pass, 3 skip by design. Both new commits carryCo-authored-by+Agent-Modeland are authored as Lily Shen.
The opencode/dsh dead end is closed. The note slot now links the checked-in setup guide for exactly the two adapters that need one, and commands no longer carries an OS the adapter is not verified on — so there is no longer a Linux or Windows string for dsh to hand anyone. Two things I did not expect and will name because they are the better half of the fix: a test asserts the guide URLs resolve to files that exist in the repo, so the link cannot rot silently, and another pins the @deepseek-ai/dsh version in the copied command to TESTED_DSH_VERSIONS in the adapter, so the install line cannot drift from the only smoke-tested release. The repo is public, so blob/main/... resolves for the person reading the card.
Two leftovers, neither blocking:
- The note reads
This adapter is currently verified on macOS. OpenCode setup guide— it links the prerequisite without saying there is one. Someone who copies first and reads second still hitsEnvironment file not found: /Users/you/agents/environment.json. One clause ("needs anenvironment.jsonfirst") would turn a hint into an instruction. - The guide URLs are hardcoded in the component rather than i18n or config. The new test makes this safe rather than fragile, so it is a preference, not a defect.
GATE: PASS
Written by Quire, a Commonly agent — https://commonly.me/v2/pods/6aaee2af1e8446ae11fb1457
Summary
When a hosted native agent needs local code or repository access, it can post a server-composed BYO handoff card instead of shell prose. The card supports Claude Code, Codex, OpenCode, and dsh; selects the person's OS; and copies the exact token-free attach/login/run command from the structured payload.
Checks
Otto is the single gate; staging/browser verification remains for CI/deploy.