feat(sdk): expose headless spawn facade method#1003
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces provider-shaped spawn APIs with CLI-shaped inputs, adds AgentRelay.spawnHeadless and shared lifecycle helpers, centralizes headless lifecycle/result handling, refactors AgentRelayClient to spawnCli, updates lifecycle-hook types, and adds tests, docs, changelog, gateway type changes, and trajectory records. ChangesAgentRelay spawn facade expansion
Sequence DiagramsequenceDiagram
participant Caller
participant AgentRelay
participant spawnHeadlessWithLifecycle
participant AgentRelayClient
Caller->>AgentRelay: spawnHeadless(input)
AgentRelay->>spawnHeadlessWithLifecycle: create lifecycle context & onStart
spawnHeadlessWithLifecycle->>AgentRelayClient: resolve harnessConfig & client.spawnHeadless(payload)
AgentRelayClient-->>spawnHeadlessWithLifecycle: spawn response (agent)
spawnHeadlessWithLifecycle->>AgentRelay: register contracts & create Agent handle
spawnHeadlessWithLifecycle->>AgentRelay: invoke onSuccess
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request surfaces high-level AgentRelay.spawnProvider and AgentRelay.spawnHeadless facade methods in the @agent-relay/sdk package, allowing callers to launch provider-backed and headless harness agents with standard lifecycle hooks and handles. It also widens SpawnHeadlessInput to accept additional harness-backed provider metadata and configurations, documents the new APIs in the README and CHANGELOG, and adds comprehensive Vitest unit tests to verify the behavior. I have no feedback to provide as there are no review comments and the implementation is clean and well-tested.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df9e4c5665
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result = await invoke(client, { | ||
| name: input.name, | ||
| provider: input.provider, | ||
| transport: input.transport ?? harnessConfig?.runtime ?? defaultTransport, |
There was a problem hiding this comment.
When spawnProvider() is called without an explicit transport but with a harness config (or a registered harness), this line resolves and stores the transport before AgentRelayClient.spawnProvider() runs beforeAgentSpawn hooks. Those hooks are allowed to replace harnessConfig, but they cannot patch transport; the client then sees this pre-set transport and will not derive it from the patched harness, so a hook that swaps a headless harness for a pty harness (or vice versa) sends mismatched transport/harnessConfig to the broker and the spawn is rejected. Let the client derive transport from the post-hook input unless the caller explicitly supplied one.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agentworkforce/trajectories/completed/2026-05/traj_bvo77swtj1br/trajectory.json:
- Line 47: The committed trajectory.json contains a machine-specific absolute
path in the "projectId" field; replace that value with a neutral, repo-relative
identifier or project slug (e.g., "relay" or "./relay") instead of
"/Users/will/Projects/AgentWorkforce/relay". Update the "projectId" entry in
trajectory.json (look for the "projectId" key) to a non-sensitive identifier,
commit the change, and ensure future generation excludes absolute local paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4af54a17-81b1-472a-9f49-e5fb4627dd22
📒 Files selected for processing (8)
.agentworkforce/trajectories/completed/2026-05/traj_bvo77swtj1br/summary.md.agentworkforce/trajectories/completed/2026-05/traj_bvo77swtj1br/trajectory.jsonCHANGELOG.mdpackages/sdk/README.mdpackages/sdk/src/__tests__/orchestration-upgrades.test.tspackages/sdk/src/__tests__/spawn-token.test.tspackages/sdk/src/relay.tspackages/sdk/src/types.ts
| }, | ||
| "commits": [], | ||
| "filesChanged": [], | ||
| "projectId": "/Users/will/Projects/AgentWorkforce/relay", |
There was a problem hiding this comment.
Remove absolute local path from committed trajectory metadata.
projectId currently embeds a developer-local filesystem path (/Users/will/...), which leaks user/environment-identifying data and makes artifacts machine-specific. Prefer a repo-relative identifier or neutral project slug.
Suggested change
- "projectId": "/Users/will/Projects/AgentWorkforce/relay",
+ "projectId": "AgentWorkforce/relay",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "projectId": "/Users/will/Projects/AgentWorkforce/relay", | |
| "projectId": "AgentWorkforce/relay", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.agentworkforce/trajectories/completed/2026-05/traj_bvo77swtj1br/trajectory.json
at line 47, The committed trajectory.json contains a machine-specific absolute
path in the "projectId" field; replace that value with a neutral, repo-relative
identifier or project slug (e.g., "relay" or "./relay") instead of
"/Users/will/Projects/AgentWorkforce/relay". Update the "projectId" entry in
trajectory.json (look for the "projectId" key) to a non-sensitive identifier,
commit the change, and ensure future generation excludes absolute local paths.
willwashburn
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk/src/relay.ts (1)
928-1017:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't register the headless result contract under the requested name before the broker finalizes it.
This helper already expects
result.name !== input.name. In that path, pre-populatingthis.resultContractswithinput.namecan clobber a live agent that already owns the requested name, and its pendingwaitForResult()state is never reset because onlyresult.nameis lifecycle-reset after the spawn succeeds. Please keep the pre-spawn contract separate from live-agent state, or explicitly tear down the requested-name state before reusing that key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk/src/relay.ts` around lines 928 - 1017, The code pre-populates this.resultContracts with the requested input.name before the broker finalizes the agent name, which can clobber a live agent when result.name !== input.name; modify spawnHeadlessWithLifecycle so you do NOT set this.resultContracts.set(input.name, ...) before the spawn call — instead keep the prepared resultContract in a local variable and only insert it into this.resultContracts after the spawn succeeds (using result.name), and ensure the error path still cleans up any temporary state (remove the pre-population and rely on the existing cleanup that deletes by result.name when necessary); key symbols: spawnHeadlessWithLifecycle, prepareAgentResultContract, resultContract, this.resultContracts, input.name, result.name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/sdk/src/relay.ts`:
- Around line 928-1017: The code pre-populates this.resultContracts with the
requested input.name before the broker finalizes the agent name, which can
clobber a live agent when result.name !== input.name; modify
spawnHeadlessWithLifecycle so you do NOT set
this.resultContracts.set(input.name, ...) before the spawn call — instead keep
the prepared resultContract in a local variable and only insert it into
this.resultContracts after the spawn succeeds (using result.name), and ensure
the error path still cleans up any temporary state (remove the pre-population
and rely on the existing cleanup that deletes by result.name when necessary);
key symbols: spawnHeadlessWithLifecycle, prepareAgentResultContract,
resultContract, this.resultContracts, input.name, result.name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 47e1aa6c-bafe-4a25-b506-874bded77a73
📒 Files selected for processing (7)
.agentworkforce/trajectories/completed/2026-05/traj_33ykjz5a7avh/summary.md.agentworkforce/trajectories/completed/2026-05/traj_33ykjz5a7avh/trajectory.jsonCHANGELOG.mdpackages/sdk/README.mdpackages/sdk/src/__tests__/orchestration-upgrades.test.tspackages/sdk/src/__tests__/spawn-token.test.tspackages/sdk/src/relay.ts
✅ Files skipped from review due to trivial changes (2)
- .agentworkforce/trajectories/completed/2026-05/traj_33ykjz5a7avh/trajectory.json
- .agentworkforce/trajectories/completed/2026-05/traj_33ykjz5a7avh/summary.md
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- packages/sdk/README.md
- packages/sdk/src/tests/spawn-token.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk/src/relay.ts (1)
815-818:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDelay PTY result-contract registration until the spawn resolves.
spawnPty()still overwritesresultContracts[input.name]before the broker confirms the final agent name, and the error path deletes that key again. If a same-named agent is already running, its in-flightagent_resultcan be validated/callbacked against the new contract, and a failed spawn can drop the old contract entirely. The new headless helper below already avoids this by registering only afterresult.nameis known.Proposed fix
await this.invokeLifecycleHook(input.onStart, lifecycleContext, `spawnPty("${input.name}") onStart`); let result: SpawnAgentResult; const resultContract = this.prepareAgentResultContract(input.result); - if (resultContract) { - this.resultContracts.set(input.name, resultContract as InternalAgentResultContract); - } try { const harnessConfig = this.resolveHarnessConfig(input); result = await client.spawnPty({ name: input.name, cli: input.cli, @@ skipRelayPrompt: input.skipRelayPrompt, agentResultSchema: resultContract?.jsonSchema ?? input.agentResultSchema, }); } catch (error) { - if (resultContract) { - this.resultContracts.delete(input.name); - } await this.invokeLifecycleHook( input.onError, { ...lifecycleContext, error, @@ ); throw error; } this.resetAgentLifecycleState(result.name); - if (result.name !== input.name && resultContract) { - this.resultContracts.delete(input.name); + if (resultContract) { this.resultContracts.set(result.name, resultContract as InternalAgentResultContract); }Also applies to: 840-858
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sdk/src/relay.ts` around lines 815 - 818, The current code registers the prepared result contract into this.resultContracts using input.name before spawnPty resolves, which can cause race conditions and accidental overwrites/deletes; change the flow so prepareAgentResultContract is still called but do NOT set this.resultContracts.set(input.name, ...) until after spawnPty (or the spawn promise) resolves and you have the final result.name from the broker; update the spawnPty-related logic (including any code paths in the same block and the similar logic around lines 840-858) to register the contract only when the spawn outcome confirms the final agent name (cast to InternalAgentResultContract when storing), and ensure error/cleanup paths only remove entries that were actually added after successful spawn resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/sdk/src/relay.ts`:
- Around line 815-818: The current code registers the prepared result contract
into this.resultContracts using input.name before spawnPty resolves, which can
cause race conditions and accidental overwrites/deletes; change the flow so
prepareAgentResultContract is still called but do NOT set
this.resultContracts.set(input.name, ...) until after spawnPty (or the spawn
promise) resolves and you have the final result.name from the broker; update the
spawnPty-related logic (including any code paths in the same block and the
similar logic around lines 840-858) to register the contract only when the spawn
outcome confirms the final agent name (cast to InternalAgentResultContract when
storing), and ensure error/cleanup paths only remove entries that were actually
added after successful spawn resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d3e8d96-b4a7-43b2-a64d-02933e0c8b4d
📒 Files selected for processing (2)
packages/sdk/src/__tests__/orchestration-upgrades.test.tspackages/sdk/src/relay.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk/src/tests/orchestration-upgrades.test.ts
|
Preview deployed!
This preview will be cleaned up when the PR is merged or closed. |
1 participant
Summary
AgentRelay.spawnHeadless({ cli, ... })with the same facade lifecycle, result-contract, channel, and handle wiring asspawnPty().AgentRelayClient.spawnProvider()->spawnCli(),SpawnProviderInput->SpawnCliInput, andSpawnHeadlessInput.provider->SpawnHeadlessInput.cli.Verification
npm --prefix packages/sdk run checknpm --prefix packages/sdk run buildnpm --prefix packages/gateway run buildnpx vitest run --config vitest.config.ts src/__tests__/lifecycle-hooks.test.ts src/__tests__/orchestration-upgrades.test.ts src/__tests__/spawn-token.test.ts -t "spawnCli|cli transport|spawnHeadless|SpawnCliInput|SpawnHeadlessInput|SpawnHeadlessAgentInput"npx prettier --check CHANGELOG.md packages/sdk/README.md packages/sdk/src/client.ts packages/sdk/src/lifecycle-hooks.ts packages/sdk/src/relay.ts packages/sdk/src/types.ts packages/sdk/src/__tests__/lifecycle-hooks.test.ts packages/sdk/src/__tests__/orchestration-upgrades.test.ts packages/sdk/src/__tests__/spawn-token.test.ts packages/sdk/src/__tests__/integration.test.ts packages/gateway/src/types.tsNote: a full run of
src/__tests__/orchestration-upgrades.test.tsin this sandbox hits existing tests that auto-create Relaycast workspaces when no API key is set, so it fails on restricted network DNS forapi.relaycast.dev. The added/renamed CLI and headless tests pass in the focused run above.Closes #998