codex/core-simplification: workspace setup and core surface simplification#1
Open
willwashburn wants to merge 5 commits into
Open
codex/core-simplification: workspace setup and core surface simplification#1willwashburn wants to merge 5 commits into
willwashburn wants to merge 5 commits into
Conversation
- Expand tsconfig.json from monorepo extends to self-contained compiler options - Update package.json repository URL to standalone repo - Fix test scripts to run vitest directly (no longer in monorepo) - Add typescript and vitest to devDependencies - Add .gitignore Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThe PR consolidates authentication terminology and architecture across the OpenClaw bridge. It introduces a new driver-managed ChangesWorkspace Key Consolidation and Spawn Architecture Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
bridge/spawn-from-env.mjs (1)
41-47: ⚡ Quick winConsider adding a timeout for the agentExited event wait.
The script waits indefinitely for an
agentExitedevent matching the agent name. If the agent fails to exit cleanly (crashes, hangs, etc.), this could cause the spawn script to hang forever without any feedback.🔄 Suggested improvement with timeout
- await new Promise((resolve) => { - client.addListener('agentExited', (event) => { - if (event.name === agent.name) { - resolve(); - } - }); - }); + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error(`Agent ${agent.name} did not exit within timeout`)); + }, 300_000); // 5 minutes + + client.addListener('agentExited', (event) => { + if (event.name === agent.name) { + clearTimeout(timeout); + resolve(); + } + }); + });🤖 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 `@bridge/spawn-from-env.mjs` around lines 41 - 47, The current await new Promise waiting on client.addListener('agentExited', ...) can hang indefinitely; update the promise in spawn-from-env.mjs to implement a timeout: register the listener for 'agentExited' (matching agent.name) and also start a timer (e.g., setTimeout) that rejects or resolves with an error after a configurable interval, and ensure you clear the timer and remove the listener in both the success and timeout paths so resources are cleaned up; use the existing symbols client.addListener, 'agentExited', agent.name, resolve/reject, and clearTimeout/clearListener to locate and modify the code.src/spawn/types.ts (1)
4-7: ⚡ Quick winFix the concern: spawn providers already validate key presence
BothDockerSpawnProviderandProcessSpawnProviderderiveworkspaceKeyasoptions.workspaceKey ?? options.relayApiKeyand immediatelythrowif it’s missing (src/spawn/docker.tslines ~163-169,src/spawn/process.tslines ~63-66), so the “neither key set” case won’t fail later unpredictably. Optional: encode “at least one ofworkspaceKeyor deprecatedrelayApiKey” inSpawnOptions’ type to catch this earlier.🤖 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 `@src/spawn/types.ts` around lines 4 - 7, SpawnOptions currently marks workspaceKey and deprecated relayApiKey as optional but runtime code in DockerSpawnProvider and ProcessSpawnProvider already derives and throws if neither is provided; update the type to enforce "at least one present" at compile time by replacing the two optional fields with a union that requires one key (e.g. { workspaceKey: string; relayApiKey?: never } | { workspaceKey?: never; relayApiKey: string }) and keep the relayApiKey marked deprecated in the comment; reference the SpawnOptions type and the DockerSpawnProvider and ProcessSpawnProvider derivation logic so future callers get a TS error instead of relying solely on runtime throws.src/spawn/docker.ts (1)
133-133: 💤 Low valueRemove unused variable
r.Line 133 creates a require function via
m.createRequire()but it's never used in the subsequent code. The script continues to use the globalrequireobject.♻️ Proposed cleanup
'node -e "' + "const p = require('path');" + "const m = require('module');" + - "const r = m.createRequire(require.resolve('`@agent-relay/openclaw/package.json`'));" + "const dir = p.dirname(require.resolve('`@agent-relay/openclaw/package.json`'));" +🤖 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 `@src/spawn/docker.ts` at line 133, Remove the unused local variable by deleting the unused assignment "const r = m.createRequire(require.resolve('`@agent-relay/openclaw/package.json`'));" so the module.createRequire call isn't stored to an unused symbol; if you intended to use an alternate require, replace subsequent uses of the global require with "r" (or rename it to a meaningful identifier) and update calls accordingly, otherwise simply remove the "r" declaration to avoid the unused-variable warning.
🤖 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.
Nitpick comments:
In `@bridge/spawn-from-env.mjs`:
- Around line 41-47: The current await new Promise waiting on
client.addListener('agentExited', ...) can hang indefinitely; update the promise
in spawn-from-env.mjs to implement a timeout: register the listener for
'agentExited' (matching agent.name) and also start a timer (e.g., setTimeout)
that rejects or resolves with an error after a configurable interval, and ensure
you clear the timer and remove the listener in both the success and timeout
paths so resources are cleaned up; use the existing symbols client.addListener,
'agentExited', agent.name, resolve/reject, and clearTimeout/clearListener to
locate and modify the code.
In `@src/spawn/docker.ts`:
- Line 133: Remove the unused local variable by deleting the unused assignment
"const r =
m.createRequire(require.resolve('`@agent-relay/openclaw/package.json`'));" so the
module.createRequire call isn't stored to an unused symbol; if you intended to
use an alternate require, replace subsequent uses of the global require with "r"
(or rename it to a meaningful identifier) and update calls accordingly,
otherwise simply remove the "r" declaration to avoid the unused-variable
warning.
In `@src/spawn/types.ts`:
- Around line 4-7: SpawnOptions currently marks workspaceKey and deprecated
relayApiKey as optional but runtime code in DockerSpawnProvider and
ProcessSpawnProvider already derives and throws if neither is provided; update
the type to enforce "at least one present" at compile time by replacing the two
optional fields with a union that requires one key (e.g. { workspaceKey: string;
relayApiKey?: never } | { workspaceKey?: never; relayApiKey: string }) and keep
the relayApiKey marked deprecated in the comment; reference the SpawnOptions
type and the DockerSpawnProvider and ProcessSpawnProvider derivation logic so
future callers get a TS error instead of relying solely on runtime throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 252ad118-1653-41b1-b49c-45a719fd8a31
📒 Files selected for processing (16)
.gitignoreREADME.mdbridge/bridge.mjsbridge/spawn-from-env.mjspackage.jsonskill/SKILL.mdsrc/cli.tssrc/config.tssrc/gateway.tssrc/inject.tssrc/setup.tssrc/spawn/docker.tssrc/spawn/process.tssrc/spawn/types.tssrc/types.tstsconfig.json
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="bridge/spawn-from-env.mjs">
<violation number="1" location="bridge/spawn-from-env.mjs:32">
P2: `agentExited` listener is registered too late, creating a race that can hang the script if the agent exits quickly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+32
to
+49
| const agent = await client.spawnPty({ | ||
| name, | ||
| cli, | ||
| args, | ||
| channels, | ||
| task: process.env.AGENT_TASK, | ||
| cwd: process.env.AGENT_CWD, | ||
| }); | ||
|
|
||
| await new Promise((resolve) => { | ||
| client.addListener('agentExited', (event) => { | ||
| if (event.name === agent.name) { | ||
| resolve(); | ||
| } | ||
| }); | ||
| }); | ||
| } finally { | ||
| await client.shutdown().catch(() => {}); |
There was a problem hiding this comment.
P2: agentExited listener is registered too late, creating a race that can hang the script if the agent exits quickly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bridge/spawn-from-env.mjs, line 32:
<comment>`agentExited` listener is registered too late, creating a race that can hang the script if the agent exits quickly.</comment>
<file context>
@@ -0,0 +1,56 @@
+ });
+
+ try {
+ const agent = await client.spawnPty({
+ name,
+ cli,
</file context>
Suggested change
| const agent = await client.spawnPty({ | |
| name, | |
| cli, | |
| args, | |
| channels, | |
| task: process.env.AGENT_TASK, | |
| cwd: process.env.AGENT_CWD, | |
| }); | |
| await new Promise((resolve) => { | |
| client.addListener('agentExited', (event) => { | |
| if (event.name === agent.name) { | |
| resolve(); | |
| } | |
| }); | |
| }); | |
| } finally { | |
| await client.shutdown().catch(() => {}); | |
| const waitForExit = new Promise((resolve) => { | |
| const onAgentExited = (event) => { | |
| if (event.name === name) { | |
| client.removeListener('agentExited', onAgentExited); | |
| resolve(); | |
| } | |
| }; | |
| client.addListener('agentExited', onAgentExited); | |
| }); | |
| await client.spawnPty({ | |
| name, | |
| cli, | |
| args, | |
| channels, | |
| task: process.env.AGENT_TASK, | |
| cwd: process.env.AGENT_CWD, | |
| }); | |
| await waitForExit; |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="bridge/spawn-from-env.mjs">
<violation number="1" location="bridge/spawn-from-env.mjs:6">
P2: Workspace key precedence is inverted: when both env vars are set, this bridge still uses `RELAY_API_KEY` instead of preferring `RELAY_WORKSPACE_KEY`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+6
to
+8
| if (!process.env.RELAY_API_KEY && process.env.RELAY_WORKSPACE_KEY) { | ||
| process.env.RELAY_API_KEY = process.env.RELAY_WORKSPACE_KEY; | ||
| } |
There was a problem hiding this comment.
P2: Workspace key precedence is inverted: when both env vars are set, this bridge still uses RELAY_API_KEY instead of preferring RELAY_WORKSPACE_KEY.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bridge/spawn-from-env.mjs, line 6:
<comment>Workspace key precedence is inverted: when both env vars are set, this bridge still uses `RELAY_API_KEY` instead of preferring `RELAY_WORKSPACE_KEY`.</comment>
<file context>
@@ -1,53 +1,13 @@
-
- if (!name) {
- throw new Error('AGENT_NAME is required');
+ if (!process.env.RELAY_API_KEY && process.env.RELAY_WORKSPACE_KEY) {
+ process.env.RELAY_API_KEY = process.env.RELAY_WORKSPACE_KEY;
}
</file context>
Suggested change
| if (!process.env.RELAY_API_KEY && process.env.RELAY_WORKSPACE_KEY) { | |
| process.env.RELAY_API_KEY = process.env.RELAY_WORKSPACE_KEY; | |
| } | |
| if (process.env.RELAY_WORKSPACE_KEY) { | |
| process.env.RELAY_API_KEY = process.env.RELAY_WORKSPACE_KEY; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Carries over the
codex/core-simplificationwork from the relay monorepo:Notes
The
chore: extract to standalone repoandci: add publish and test workflowscommits on this branch overlap with what was applied directly tomainduring the repo extraction — expect a merge conflict on those two commits that will need to be resolved on merge.🤖 Generated with Claude Code
Summary by cubic
Switches the OpenClaw adapter to a workspace‑first Agent Relay flow and updates naming from “Relaycast” to “Agent Relay,” with spawning now managed via
@agent-relay/sdk. CLI/docs default to creating a workspace; config prefersRELAY_WORKSPACE_KEYwhile keepingRELAY_API_KEYas a fallback.New Features
relay-openclaw setupcreates or joins a workspace and starts the gateway; README/SKILL updated. Setup reuses saved config when re-run with the same name.bridge/spawn-from-env.mjs; Docker/Process spawns useAgentRelayClient.spawn*and setRELAY_WORKSPACE_KEY(also mirrored toRELAY_API_KEY).RELAY_WORKSPACE_KEYfirst and falls back toRELAY_API_KEY.Migration
RELAY_API_KEYkeeps working; setup now writesRELAY_WORKSPACE_KEYand retainsRELAY_API_KEYas an alias.npx -y @agent-relay/openclaw setup --name <claw>; pass a sharedrk_live_...only to join an existing workspace.@agent-relay/sdk(AgentRelayClient.spawn*orspawnFromEnv);SpawnOptions.relayApiKeyis deprecated in favor ofworkspaceKey.Written for commit 7435523. Summary will update on new commits.
Review in cubic