Simplify Agent Relay core surfaces#1010
Open
willwashburn wants to merge 53 commits into
Open
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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: 86fe101dd6
ℹ️ 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".
| "@agent-relay/config": "7.1.1", | ||
| "@agent-relay/hooks": "7.1.1", | ||
| "@agent-relay/memory": "7.1.1", | ||
| "@agent-relay/driver": "7.1.1", |
There was a problem hiding this comment.
Adding @agent-relay/driver as a runtime dependency means the agent-relay npm package cannot be installed until that exact driver version exists on npm. I checked the publish workflow matrices (.github/workflows/publish.yml publish-sdk-internal-deps lines 801-807 and publish-packages lines 1017-1035), and none of them publish driver, so a release of the CLI will point at an unpublished @agent-relay/driver@7.1.1 and fresh installs fail with an npm not-found/dependency resolution error.
Useful? React with 👍 / 👎.
Comment on lines
+28
to
32
| "./capabilities": { | ||
| "types": "./dist/capabilities.d.ts", | ||
| "import": "./dist/capabilities.js", | ||
| "default": "./dist/capabilities.js" | ||
| } |
There was a problem hiding this comment.
This shortened SDK export map removes the broker-path/driver APIs, but the SDK release smoke still imports them: .github/workflows/publish.yml uses @agent-relay/sdk/broker-path at line 717 and AgentRelayClient from @agent-relay/sdk at line 737, and scripts/post-publish-verify/verify-install.sh does the same at line 232. Any SDK publish/smoke run will now fail before validating the package unless those checks are moved to @agent-relay/driver or the compatibility exports are kept.
Useful? React with 👍 / 👎.
| @@ -1,59 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
Deleting this package directory leaves the package=all release workflow pointing at paths that no longer exist: I checked .github/workflows/publish.yml and the publish-packages matrix still includes acp-bridge and openclaw at lines 1027-1028. Those matrix jobs run with working-directory: packages/${{ matrix.package }}, so the next all-package release will fail as soon as it tries to publish either deleted package.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 269 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="packages/sdk/src/delivery/runner.ts">
<violation number="1" location="packages/sdk/src/delivery/runner.ts:38">
P1: `stop()` does not cancel the inbox subscription, so the runner can hang indefinitely while waiting for the next message.</violation>
<violation number="2" location="packages/sdk/src/delivery/runner.ts:81">
P1: Exceptions from `onError` prevent `inbox.fail(...)` from running, which can leave errored items without delivery-state updates and terminate processing.</violation>
</file>
<file name="packages/driver/package.json">
<violation number="1" location="packages/driver/package.json:43">
P1: Add a `prepack` hook so this package builds before `npm pack`/`npm publish`; otherwise it can be published without the `dist` files that `main`/`exports` reference.</violation>
</file>
<file name="packages/driver/src/broker-driver.ts">
<violation number="1" location="packages/driver/src/broker-driver.ts:53">
P1: Don't auto-spawn a broker in the shared client initializer; status/release should require an existing client or separate initialization path.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
| private async run(): Promise<void> { | ||
| await this.options.delivery.connect?.(); | ||
| try { | ||
| for await (const item of this.options.messaging.inbox.subscribe({ |
There was a problem hiding this comment.
P1: stop() does not cancel the inbox subscription, so the runner can hang indefinitely while waiting for the next message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/delivery/runner.ts, line 38:
<comment>`stop()` does not cancel the inbox subscription, so the runner can hang indefinitely while waiting for the next message.</comment>
<file context>
@@ -0,0 +1,89 @@
+ private async run(): Promise<void> {
+ await this.options.delivery.connect?.();
+ try {
+ for await (const item of this.options.messaging.inbox.subscribe({
+ agentName: this.options.agentName,
+ })) {
</file context>
| metadata: result.metadata, | ||
| }); | ||
| } catch (error) { | ||
| await Promise.resolve(this.options.onError?.(item, error)); |
There was a problem hiding this comment.
P1: Exceptions from onError prevent inbox.fail(...) from running, which can leave errored items without delivery-state updates and terminate processing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/delivery/runner.ts, line 81:
<comment>Exceptions from `onError` prevent `inbox.fail(...)` from running, which can leave errored items without delivery-state updates and terminate processing.</comment>
<file context>
@@ -0,0 +1,89 @@
+ metadata: result.metadata,
+ });
+ } catch (error) {
+ await Promise.resolve(this.options.onError?.(item, error));
+ await this.options.messaging.inbox.fail({
+ inboxItemId: item.id,
</file context>
| "package.json" | ||
| ], | ||
| "scripts": { | ||
| "build": "node -e \"require('node:fs').rmSync('dist',{recursive:true,force:true})\" && tsc -p tsconfig.json", |
There was a problem hiding this comment.
P1: Add a prepack hook so this package builds before npm pack/npm publish; otherwise it can be published without the dist files that main/exports reference.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/driver/package.json, line 43:
<comment>Add a `prepack` hook so this package builds before `npm pack`/`npm publish`; otherwise it can be published without the `dist` files that `main`/`exports` reference.</comment>
<file context>
@@ -0,0 +1,67 @@
+ "package.json"
+ ],
+ "scripts": {
+ "build": "node -e \"require('node:fs').rmSync('dist',{recursive:true,force:true})\" && tsc -p tsconfig.json",
+ "check": "tsc -p tsconfig.json --noEmit"
+ },
</file context>
|
|
||
| private async ensureClient(): Promise<AgentRelayClient> { | ||
| if (!this.client) { | ||
| this.client = await AgentRelayClient.spawn(this.options); |
There was a problem hiding this comment.
P1: Don't auto-spawn a broker in the shared client initializer; status/release should require an existing client or separate initialization path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/driver/src/broker-driver.ts, line 53:
<comment>Don't auto-spawn a broker in the shared client initializer; status/release should require an existing client or separate initialization path.</comment>
<file context>
@@ -0,0 +1,79 @@
+
+ private async ensureClient(): Promise<AgentRelayClient> {
+ if (!this.client) {
+ this.client = await AgentRelayClient.spawn(this.options);
+ }
+ return this.client;
</file context>
There was a problem hiding this comment.
2 issues found across 4 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="packages/sdk/src/delivery/runner.ts">
<violation number="1" location="packages/sdk/src/delivery/runner.ts:38">
P1: `stop()` does not cancel the inbox subscription, so the runner can hang indefinitely while waiting for the next message.</violation>
<violation number="2" location="packages/sdk/src/delivery/runner.ts:81">
P1: Exceptions from `onError` prevent `inbox.fail(...)` from running, which can leave errored items without delivery-state updates and terminate processing.</violation>
</file>
<file name="packages/driver/package.json">
<violation number="1" location="packages/driver/package.json:43">
P1: Add a `prepack` hook so this package builds before `npm pack`/`npm publish`; otherwise it can be published without the `dist` files that `main`/`exports` reference.</violation>
</file>
<file name="packages/driver/src/broker-driver.ts">
<violation number="1" location="packages/driver/src/broker-driver.ts:53">
P1: Don't auto-spawn a broker in the shared client initializer; status/release should require an existing client or separate initialization path.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
3 issues found across 1 file (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="packages/sdk/src/delivery/runner.ts">
<violation number="1" location="packages/sdk/src/delivery/runner.ts:38">
P1: `stop()` does not cancel the inbox subscription, so the runner can hang indefinitely while waiting for the next message.</violation>
<violation number="2" location="packages/sdk/src/delivery/runner.ts:81">
P1: Exceptions from `onError` prevent `inbox.fail(...)` from running, which can leave errored items without delivery-state updates and terminate processing.</violation>
</file>
<file name="packages/driver/package.json">
<violation number="1" location="packages/driver/package.json:43">
P1: Add a `prepack` hook so this package builds before `npm pack`/`npm publish`; otherwise it can be published without the `dist` files that `main`/`exports` reference.</violation>
</file>
<file name="packages/driver/src/broker-driver.ts">
<violation number="1" location="packages/driver/src/broker-driver.ts:53">
P1: Don't auto-spawn a broker in the shared client initializer; status/release should require an existing client or separate initialization path.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 1 file (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="README.md">
<violation number="1" location="README.md:67">
P2: Use the SDK handler signature here; the payload is passed directly as the first argument, not as `{ input }`.</violation>
<violation number="2" location="README.md:93">
P2: Use the action context here instead of destructuring a non-existent `agent` field.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Comment on lines
+93
to
+94
| handler: async ({ agent, input }) => { | ||
| await writeToDb(agent.name, input.vote); |
There was a problem hiding this comment.
P2: Use the action context here instead of destructuring a non-existent agent field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 93:
<comment>Use the action context here instead of destructuring a non-existent `agent` field.</comment>
<file context>
@@ -43,49 +43,60 @@ await relay.workspace.register([complaintTriager, engineer, taskManager]);
- handler: async (input, ctx) => {
- writeToDb(ctx.agent.name, input.vote);
- if (allVotesAreIn() === true) {
+ handler: async ({ agent, input }) => {
+ await writeToDb(agent.name, input.vote);
+ if (await allVotesAreIn()) {
</file context>
Suggested change
| handler: async ({ agent, input }) => { | |
| await writeToDb(agent.name, input.vote); | |
| handler: async (input, ctx) => { | |
| await writeToDb(ctx.caller.name, input.vote); |
| model: z.enum(['opus', 'sonnet']), | ||
| }), | ||
| availableTo: [taskManager, engineer], // leave this out if you want to make it available to all agents! | ||
| handler: async ({ input }) => { |
There was a problem hiding this comment.
P2: Use the SDK handler signature here; the payload is passed directly as the first argument, not as { input }.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 67:
<comment>Use the SDK handler signature here; the payload is passed directly as the first argument, not as `{ input }`.</comment>
<file context>
@@ -43,49 +43,60 @@ await relay.workspace.register([complaintTriager, engineer, taskManager]);
- await claude.new({ model: input.model });
- },
availableTo: [taskManager, engineer], // leave this out if you want to make it available to all agents!
+ handler: async ({ input }) => {
+ const agent = claude.new({ model: input.model });
+ await relay.workspace.register(agent);
</file context>
Suggested change
| handler: async ({ input }) => { | |
| handler: async (input) => { |
…codex/core-simplification
…codex/core-simplification
First we write a lot of words, then we remove them, then we add more
…codex/core-simplification
…odex/core-simplification # Conflicts: # package-lock.json # package.json # packages/acp-bridge/README.md # packages/acp-bridge/src/acp-agent.ts # packages/acp-bridge/src/cli.ts # packages/acp-bridge/src/index.ts # packages/acp-bridge/src/types.ts # packages/cli/package.json # packages/cli/scripts/build-cjs.mjs # packages/gateway/package.json # packages/gateway/src/types.ts # packages/hooks/package.json # packages/hooks/src/types.ts # packages/openclaw/README.md # packages/openclaw/bridge/bridge.mjs # packages/openclaw/package.json # packages/openclaw/skill/SKILL.md # packages/openclaw/src/cli.ts # packages/openclaw/src/config.ts # packages/openclaw/src/gateway.ts # packages/openclaw/src/inject.ts # packages/openclaw/src/setup.ts # packages/openclaw/src/spawn/docker.ts # packages/openclaw/src/spawn/process.ts # packages/openclaw/src/spawn/types.ts # packages/openclaw/src/types.ts # packages/sdk/package.json # packages/sdk/src/index.ts # packages/sdk/src/models.ts
Contributor
|
Preview deployed!
This preview will be cleaned up when the PR is merged or closed. |
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
workspaceKey/RELAY_WORKSPACE_KEYand keepapiKey/RELAY_API_KEYonly as compatibility aliases@agent-relay/driverto@agent-relay/runtimeand add@agent-relay/harnessesas the CLI harness catalogVerification
npm run build:corenpm --prefix packages/sdk test(35 tests)npm --prefix packages/openclaw test(83 tests, run with loopback access)npm run knipgit diff --cached --check