feat(cli-surface): expose @relayfile/sdk/relay-cli for agent-relay file - #507
Conversation
Mount relayfile into the agent-relay CLI as `agent-relay file` without copying a single command implementation into relay. Go CLI: - New cmd/relayfile-cli/commandspec.go holds one declarative command table. run()'s 60-line top-level `switch` is replaced by a lookup into that table, so a top-level command cannot be declared without being routable, or routed without being declared. - Hidden `relayfile __command-spec --json` emits the table in the RelayCliCommandSpec[] shape @agent-relay/cli-surface defines. relayfile does not use cobra, so there is no command-object tree to walk; the table is the substitute, and commandspec_test.go parses each group's dispatch switch and each leaf's flag.FlagSet out of the source AST to assert the declared subcommands and options match the implementation exactly. - `writeback retry` gains `--op-id` as a kebab-case alias for `--opId`, which the contract's flag grammar cannot express. `--opId` still works. SDK: - New ./relay-cli subpath export. createRelayCliSurface() returns the surface (id 'relayfile', contract 1, 23 top-level commands); commands comes from a checked-in snapshot of the Go table, regenerated by `npm run gen:command-spec` and diffed by a test so it cannot drift; run(argv, io) spawns the same Go binary, pipes its stdout/stderr into io, forwards stdin, installs no signal handlers, calls no process.exit, and returns the child's real exit code. Unknown command exits 2. - Binary resolution and the Cloud sign-in preflight moved here from packages/cli/scripts, so each exists exactly once in the repo. - @agent-relay/cli-surface is a devDependency only: the surface is structurally typed, so the published package gains no runtime dep on relay. - relay-cli stays a subpath export, asserted by import-safety.test.ts, so the default entry never pulls node:child_process into memory. CLI package: - scripts/run.js and scripts/install.js now call the SDK instead of carrying their own copies of the platform map, binary lookup, and preflight. `relayfile --version` still short-circuits before any binary lookup. - cloud-preflight.js and its tests are replaced by cloud-auth.test.js (the vendored bundle) and run.test.js (the shim), with the preflight logic tests ported to the SDK. Verified: go test ./... and go vet ./... clean; SDK typecheck clean; 45 relay-cli tests pass against a real built binary and via `go run`; the surface imports and reports 23 commands from a packed tarball install. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 57ec71cd-46c6-41cf-8fb5-952f0cd36dab
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 57ec71cd-46c6-41cf-8fb5-952f0cd36dab
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 4 potential issues.
4 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| child.stdout?.setEncoding("utf8") | ||
| child.stderr?.setEncoding("utf8") | ||
| child.stdout?.on("data", (chunk: string) => io.stdout(chunk)) | ||
| child.stderr?.on("data", (chunk: string) => io.stderr(chunk)) |
There was a problem hiding this comment.
🔴 Binary stdout is corrupted
When read or export emits binary stdout, setEncoding("utf8") replaces invalid byte sequences. Mounted commands return corrupted file content.
Learn more
Relayfile writes raw bytes to stdout for binary files in runRead and for tar exports in runExport. Setting a stream encoding makes Node decode those bytes into JavaScript strings. Invalid UTF-8 sequences become replacement characters before io.stdout receives them, so the original bytes cannot be reconstructed.
Example: A remote PNG containing byte sequence 89 50 4e 47 is read with agent-relay file read image.png. The leading 0x89 is invalid UTF-8 and reaches the host as �, changing the downloaded image.
Recommended fix: Preserve child stdout as Buffer data and extend or use the CLI-surface I/O contract's binary-safe sink. If the contract only accepts strings, commands capable of binary stdout need a binary transport or must require --output rather than piping decoded content.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const resolution = resolveRelayfileBinary(options.resolve) | ||
| const command = resolution.command | ||
| const childArgs = [...resolution.args, ...args] | ||
| const cwd = resolution.kind === "go-run" ? resolution.cwd : options.cwd |
There was a problem hiding this comment.
🟡 Source fallback ignores requested directory
When resolveRelayfileBinary returns go-run, options.cwd is discarded for the checkout root. Relative inputs and outputs then target the repository.
Learn more
The surface documents options.cwd as the child command's working directory. The Go fallback needs the checkout root to resolve ./cmd/relayfile-cli, so this branch silently gives repository resolution precedence over the public working-directory option. Commands using relative paths or current-directory defaults then observe a different directory than binary-backed invocations.
Example: A source-checkout consumer creates a surface with cwd: "/work/customer" and runs seed without a directory. The binary path uploads /work/customer, but the go-run path uploads the relayfile checkout's . instead.
Recommended fix: Launch a built temporary binary for the source fallback, or otherwise separate Go package resolution from the spawned CLI process's working directory. Add a real fallback test that runs a current-directory-sensitive command with options.cwd outside the checkout.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Name: "listen", | ||
| Description: "Stream workspace file events, optionally running a command per event", | ||
| Aliases: []string{"watch"}, | ||
| flagSource: "runListen", | ||
| Options: listenOptions(), | ||
| dispatch: func(inv cliInvocation) error { | ||
| return runListen(inv.args, inv.stdout) |
There was a problem hiding this comment.
🟡 Listen workspace argument disappears
The spec omits listen's optional workspace argument, and dev copies the same omission. The mounted host can reject valid workspace-qualified invocations.
Learn more
The native runListen consumes its first positional value as the workspace. runDev forwards the same argument list to runListen. The mounted host builds its parser and help from this command specification, so omitting the positional argument changes the exposed grammar even though direct binary dispatch still accepts it.
Example: relayfile listen acme selects workspace acme. The generated agent-relay file listen command declares zero positional arguments, so agent-relay file listen acme can fail host argument validation instead of starting the listener.
Recommended fix: Add Args: []cliArgSpec{workspaceArg} to both listen and dev, regenerate command-spec.json, and add a mounted-host test using an explicit workspace positional.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Options: []cliOptionSpec{ | ||
| // supervisor install forwards its argv to | ||
| // `relayfile listen`, which owns these flags. | ||
| {Flags: "--interval <duration>", Description: "sync interval passed through to the supervised listen process"}, | ||
| }, |
There was a problem hiding this comment.
🟡 Supervisor installs a failing listener
The spec advertises --interval for supervisor install, but runListen rejects that forwarded flag. The installed service exits and repeatedly restarts.
Learn more
supervisor install forwards all remaining arguments into the generated relayfile listen command through supervisorInstall. The listener's flag set does not register interval. The service manager starts the generated command with restart-on-failure enabled, so the advertised option produces a permanently failing service.
Example: agent-relay file supervisor install acme --interval 30s writes an ExecStart ending in listen acme --interval 30s. Relayfile exits with flag provided but not defined: -interval, and systemd retries it every five seconds.
Recommended fix: Either remove --interval from the command specification and existing supervisor help, or implement a real listener interval option with defined behavior. Add an installation test that executes the generated listener argv.
Was this helpful? React with 👍 or 👎 to provide feedback.
`agent-relay file <cmd>` resolved no binary on a clean install. The
relayfile CLI binary was only ever fetched by the `relayfile` package's
postinstall, and `agent-relay` depends on `@relayfile/sdk`, not on
`relayfile` — so nothing in its dependency chain installed one. The
existing tests passed only because they ran inside this checkout, where a
built binary and a Go toolchain are both already present.
Ship the binary the way this repo already ships `relayfile-mount`:
- Add `@relayfile/cli-{darwin,linux}-{arm64,x64}` and
`@relayfile/cli-win32-{arm64,x64}`, mirroring `packages/mount-*` —
same manifest shape, `os`/`cpu`, `files`, and `.gitignore` treatment.
win32 is included because `packages/cli/scripts/build-binaries.js`
already cross-compiles both Windows targets and the release attaches
them; mount has no Windows build, the CLI does.
- Make them `optionalDependencies` of `@relayfile/sdk`, pinned exactly.
npm installs only the matching one: no postinstall, no install-time
network, works offline and in CI, integrity from the registry.
- `scripts/build-cli-npm-packages.mjs` fills them, mirroring
`build-mount-npm-packages.mjs`; `publish.yml` releases them alongside
the mount packages.
`resolveRelayfileBinary` now searches: `RELAYFILE_CLI_BIN`, the platform
package, the previous `binDirs` chain, `make build`/`make release`
outputs in a checkout, `go run`, then `PATH`. The order is commented.
The PATH step matches `relayfile-cli` only, never the generic
`relayfile`, which on PATH is the npm bin shim that resolves through
this module — scanning for it would recurse forever.
When nothing resolves, the error names the `@relayfile/cli-*` package
for the current platform and how the optional dependency goes missing,
instead of a bare ENOENT. Mounted as a CLI surface that is reported
through the host's `io` as exit 127 rather than thrown, since the
contract says `run()` resolves to an exit code.
Also pass the child's output through as raw bytes rather than
UTF-8-decoded strings, so `export --format tar --output -` survives
being mounted. `@agent-relay/cli-surface`'s `RelayCliIo` was widened to
`string | Uint8Array` for exactly this case.
Tests, none of which can pass on this checkout's built binary alone:
- `clean-install.test.ts` assembles a directory shaped like a real npm
install under the OS temp dir — the SDK and the platform package under
`node_modules`, no `relayfile` package, no `go.mod` above it — and
runs a probe with plain `node` and an empty PATH, so the SDK loads
through its real `exports` map, the platform package is found by the
real `require.resolve`, and the real Go binary is spawned. Covers the
resolving case, the `--omit=optional` case, and the env override.
- `platform-packages.test.ts` fails if the package directories, the SDK's
optionalDependencies, the resolver's target table, either build
script, or any of the eight places `publish.yml` needs them drift
apart — so a package can neither be published without being buildable
nor exist without being published.
- `resolve-binary.test.ts` pins the full search order with every ambient
input sealed off.
- `mount-routing.test.ts` proves `mount` routes identically through the
surface and a direct spawn for twelve argv shapes — both positionals,
all four hidden subcommands, and both aliases — including that its
exit 2 is the binary's, not the surface's unknown-command 2.
- `binary-output.test.ts` pins byte-exact stdout, including bytes no
UTF-8 decode survives.
Verified from a packed tarball install (`npm pack` of both, installed
into an empty directory, run with an empty PATH and no checkout above
it): 23 commands, the binary resolved from
`node_modules/@relayfile/cli-linux-x64/bin/relayfile-cli`, `--version`
exited 0 printing 0.10.56. Removing that package yields exit 127 and the
actionable message.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Session-Id: 4e63354c-d2b2-48a1-82e8-21a328d10f6b
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 4e63354c-d2b2-48a1-82e8-21a328d10f6b
The devDependency pointed at a `file:` path in a sibling checkout, so it resolved only on the machine that wrote it. Every CI runner failed the surface and command-spec tests with ERR_MODULE_NOT_FOUND. The contract package is now on npm at 12.2.2; this pins it there. It stays a devDependency: the published package gains no runtime dependency on relay, and the surface is still satisfied structurally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916
…ock postinstall Three behavioural findings from the PR review on the relay CLI surface. `agent-relay file version` exited 2 without spawning: the binary routes a lone `version` through wantsVersion(), the same path as --version, but routableTopLevelNames() listed only `help` and `__command-spec`. It stays out of the declared command tree — that tree is a generated snapshot of the Go command table, which handles `version` outside it and does not advertise it in `relayfile --help` either. Checkout resolution looked for `bin/relayfile-cli` and `dist/relayfile-cli-<goos>-<goarch>` with no `.exe`, so on Windows a successful `make build` was invisible and resolution fell through to `go run`. packages/cli postinstall imported @relayfile/sdk/relay-cli before it could detect a source checkout and skip the download. A fresh clone has no SDK dist/, so `npm install` died in postinstall and never reached the skip. The two checkout markers are now tested locally, ahead of any SDK load; install.test.js pins that predicate against the SDK's findSourceCheckoutRoot. Tests: 5 new in packages/cli/scripts/install.test.js (3 fail without the install.js fix), plus `version` routing and Windows checkout cases in the SDK suite (4 fail without the resolver and surface fixes). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 7377ec91-f877-4cd3-863f-29d31e4de625
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 7377ec91-f877-4cd3-863f-29d31e4de625
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
The devDependency resolved through a link to a sibling checkout, so CI could not load the surface at all. Relay's 12.2.4 release publishes a working tarball — the earlier 12.2.2 shipped only package.json because `files` is ["dist","README.md"] and the publish ran with ignore-scripts — so this pins ^12.2.4 and records a registry tarball with an integrity hash. This clears the SDK Typecheck and Client Typecheck failures. It does NOT make `npm ci` pass here: the six @relayfile/cli-* optionalDependencies are still unpublished, and npm refuses a lockfile it cannot resolve regardless of optionality. That needs the relayfile release, which is its own ordering problem — the packages only exist on this branch, so the release that publishes them has to run after this merges. 97 SDK tests passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916
…ec drift Three of Devin's four findings on #507; the fourth was already fixed. `go run` launches the program with the go command's own working directory, and go only finds the module from the checkout — so the source fallback ran relayfile from the repository, and every relative path in argv (`--output report.json`) resolved there instead of where the caller actually was. An absolute package path fails outside a module and `go -C <dir> run` moves the child too, so the fix is to build first and spawn the result: buildGoRunBinary builds into a temp path (never the working tree) and both entry points — the mounted surface and the `relayfile` bin shim — then spawn it with the caller's cwd inherited. The command table also drifted from what the binary parses, in the two ways the table was supposed to make impossible: - `listen` reads its first positional as the workspace and `dev` forwards argv to it, but neither declared an arg, so a host routing from the emitted spec refused `agent-relay file listen my-workspace`. Declared on both, and on `workspace status`, which the new guard caught doing the same. - `supervisor install` advertised `--interval`, which runListen has never registered. It embeds its argv into the unit's ExecStart as `relayfile listen ...` under Restart=on-failure, so that flag installed a service that exited on every start, forever. It now declares runListen's flags. Both had a blind spot in the AST drift test rather than bad luck: TestOptionsMatchSourceFlagSets exempted `supervisor install` outright (isPassThroughCommand), and nothing checked positionals at all. The exemption is gone and TestDeclaredArgsCoverSourcePositionals asserts that a command whose parser reads fs.Arg/fs.Args declares a positional. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 6a85a22c-13e0-4844-a610-fabfeb3fc126
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: 6a85a22c-13e0-4844-a610-fabfeb3fc126
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c0c7c9f. Configure here.
The six platform packages are on the registry now, so npm can finally record them and `npm ci` resolves a complete tree. That was the last thing failing Release Tooling on this PR. The published 0.10.56 tarballs are empty — `bin/` holds only .gitkeep, no binary — so they satisfy resolution but cannot serve a run yet. The next release bumps past them and this workflow's 'Prepare cli platform package' step stages the cross-compiled binary and `test -f`s it first, so a release-built package cannot ship hollow the way a hand publish did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916
…d attestation The six platform packages were added to the workflow's PACKAGE_PATHS_JSON and to the release commit's `git add`, but not to the two constants the release trust chain is built from. resolve-release-baseline.mjs derives RELEASE_COMMIT_PATHS from RELEASE_PACKAGE_PATHS. A release commit that touches a package.json the resolver does not list fails the changed-path allowlist, so its annotated tag stops being a trusted baseline: the next dispatch falls back to the lagging source version, bumps onto the version that is already tagged, and aborts on the tag-collision guard. The resolver also never checked that the cli-* packages carried the tag version. create-release-attestation.mjs rejects any package record whose name is not in RELEASE_PACKAGE_NAMES, while the release job requires one record per published package (17). The cli-* packages are published and reconciled, so the release attestation step would have failed outright and the executable half of the release would otherwise ship unattested. Add a guard test tying RELEASE_PACKAGE_PATHS, RELEASE_PACKAGE_NAMES, and the workflow's attestation-count gate to the shared package list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916
…-run builds `supervisor install` copied all of listenOptions() into its spec, which swept in --background and --daemonized. Both parse, so the drift tests were happy, but runListen acts on them before it connects: --background re-execs a detached child and returns, leaving launchd's KeepAlive relaunching a process that exits every start and systemd killing the orphan with the unit's cgroup. listenOptions() now splits into filters and process-model flags; the unit gets the filters, and supervisorInstall refuses the rest so argv that reaches the binary directly is rejected too. buildGoRunBinary built straight to the shared path keyed by the checkout, which a `listen` or `mount` launched from it holds open for hours — impossible to replace on Windows, and a torn-read race between two builders on Unix. The build now writes a private sibling and publishes it with one rename, falls back to the staged path when the shared name cannot be replaced, and sweeps artifacts older than a day. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916
1 participant
Part of a cross-repo change mounting every Relay product into the
agent-relayCLI. This repo's half:
@relayfile/sdkgains a./relay-cliexport thatagent-relaymounts asagent-relay file, without relay reimplementing asingle relayfile command.
What this does
@relayfile/sdk/relay-cliexportscreateRelayCliSurface()returning acontract-v1 surface: a declared command tree plus a dispatcher that resolves
and spawns the real Go binary and returns its exit code.
cloud preflight moved out of
packages/cli/scripts/run.jsinto the SDK; thenpm launcher calls into it. There is one implementation of "find the relayfile
binary" in the repo.
relayfile __command-spec --jsonemitsthe tree from a declarative table that drives the top-level dispatch, so the
spec and the dispatcher cannot disagree at that level. A Go test parses the
per-group
switchcases out of the source AST to catch drift in the nestedlevels, and a TS snapshot test regenerates and diffs the checked-in JSON.
The Go CLI is a hand-rolled
flagdispatcher, not cobra, so there was nocommand-object tree to walk — hence the table plus AST approach rather than
hand-writing a second tree that would rot.
__command-specis deliberately excluded from the emitted tree: it is aninternal introspection hook, not something a user should discover.
Scope
The refactor is confined to top-level dispatch. The per-group switches inside
main.goare untouched — a 14.7k-line file is not something to restructure as aside effect of a CLI mount.
Verification
go test ./cmd/relayfile-cli/...— ok (37s)src/client.test.tsassertingtypeof ErrorEvent === "undefined", which is false on Node 26. That file isuntouched by this branch and fails identically on
main.src/relay-cli/surface.test.ts(12 tests) including a real-binary runthrough
surface.run()asserting a genuine non-zero exit.agent-relay file --helplists all 64 commands, andagent-relay file statusreaches the real binary.Release
Nothing is published.
agent-relaypins the published version and currentlyreports "upgrade @relayfile/sdk" until this ships.
🤖 Generated with Claude Code
Note
Medium Risk
Touches release publishing, CLI dispatch, and binary resolution on the agent-relay critical path; mistakes could break installs or ship broken supervisor units, though drift tests and clean-install E2E mitigate spec/binary mismatch.
Overview
Adds
@relayfile/sdk/relay-clisoagent-relay filecan mount relayfile without reimplementing commands: a checked-in command tree (fromrelayfile __command-spec --json) plus a dispatcher that resolves and spawns the Go binary with byte-exact stdout/stderr.The Go CLI’s top-level routing now goes through a declarative command table in
commandspec.go(same table drives dispatch and JSON emission). Go AST tests and an SDK snapshot test guard nested subcommands, flags, and positionals against drift—closing gaps where undeclared args (e.g.listenworkspace) or wrongsupervisor installflags would break host parsers or install broken systemd/launchd units.Binary resolution moves into the SDK (shared by
packages/clishims). Six@relayfile/cli-<platform>-<arch>optional packages mirror@relayfile/mount-*, withpublish.yml,.gitignore, and attestation count updates to ship them. Source-checkout fallback builds then executes (preserving caller cwd) and publishes via atomic rename to avoid races with long-running processes.Follow-up fixes on the surface:
supervisor installadvertises listen filters only and rejects--background/--daemonizedat runtime;--op-idkebab alias; listen/dev/workspace-status workspace positional; go-run build path hardening; postinstall source-checkout skip;versionrouted without polluting the spec.Reviewed by Cursor Bugbot for commit e45c194. Bugbot is set up for automated code reviews on this repo. Configure here.