Skip to content

Rename inbound delivery modes#898

Merged
willwashburn merged 3 commits into
mainfrom
codex/rename-inbound-delivery-mode
May 18, 2026
Merged

Rename inbound delivery modes#898
willwashburn merged 3 commits into
mainfrom
codex/rename-inbound-delivery-mode

Conversation

@willwashburn
Copy link
Copy Markdown
Member

@willwashburn willwashburn commented May 18, 2026

Summary

  • Rename broker inbound delivery modes from human/passthrough to manual_flush/auto_inject
  • Rename the broker/API/SDK route and helpers to inbound delivery terminology, without keeping the old /mode compatibility route
  • Keep CLI attach verbs as view, drive, and passthrough; remove the agent-relay relay attach alias/command
  • Update broker API, CLI, and TypeScript SDK docs for the route, mode, and CLI naming

Tests

  • cargo test
  • cargo test types::inbound_delivery_tests
  • ./node_modules/.bin/vitest run src/cli/bootstrap.test.ts src/cli/commands/drive.test.ts src/cli/commands/passthrough.test.ts src/cli/commands/new.test.ts
  • ./node_modules/.bin/vitest run src/cli/commands/passthrough.test.ts
  • (cd packages/sdk && ../../node_modules/.bin/vitest run src/__tests__/orchestration-upgrades.test.ts -t "exposes inbound delivery mode")
  • git diff HEAD --check
  • npm run build

Note: the full SDK orchestration-upgrades file was not used as the verification gate because unfiltered tests try to reach api.relaycast.dev and fail under the sandboxed network.

@willwashburn willwashburn requested a review from khaliqgant as a code owner May 18, 2026 17:34
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7546990b-3ef9-4ae3-ba18-b5f72e342d81

📥 Commits

Reviewing files that changed from the base of the PR and between de77083 and 850c8e0.

📒 Files selected for processing (12)
  • src/cli/bootstrap.test.ts
  • src/cli/bootstrap.ts
  • src/cli/commands/new.test.ts
  • src/cli/commands/new.ts
  • src/cli/commands/passthrough.test.ts
  • src/cli/commands/passthrough.ts
  • src/cli/commands/rm.ts
  • src/cli/lib/broker-connection.ts
  • src/cli/lib/spawn-and-attach.ts
  • web/content/docs/reference-broker-api.mdx
  • web/content/docs/reference-cli.mdx
  • web/content/docs/typescript-sdk.mdx
✅ Files skipped from review due to trivial changes (4)
  • src/cli/commands/rm.ts
  • src/cli/lib/broker-connection.ts
  • src/cli/commands/new.ts
  • src/cli/lib/spawn-and-attach.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/content/docs/reference-broker-api.mdx

📝 Walkthrough

Walkthrough

This PR refactors the broker and SDK from per-worker "session mode" (human/passthrough) to "inbound delivery mode" (auto_inject/manual_flush). Type definitions and state tracking are replaced throughout the broker, the SDK client API methods are updated, and CLI commands (drive and passthrough) are modified to use the new delivery-mode endpoints. Documentation is updated across broker API, CLI reference, and TypeScript SDK guides.

Changes

Session Mode to Inbound Delivery Mode Refactoring

Layer / File(s) Summary
Type definitions and broker-side inbound delivery model
src/types.rs, packages/sdk/src/protocol.ts
Define InboundDeliveryMode with auto_inject/manual_flush variants and broker event agent_inbound_delivery_mode_changed; replace session-state tracking with InboundDeliveryState/InboundDeliveryDispatch for pending-message queueing, FIFO eviction, and dispatch outcome tracking.
Broker HTTP API layer for delivery-mode routes and handlers
src/listen_api.rs
Add GetInboundDeliveryMode/SetInboundDeliveryMode request variants and DeliveryRouteError error type; register /api/spawned/{name}/delivery-mode GET/PUT endpoints; implement request parsing, validation (400 invalid_mode), and error mapping; update pending/flush routes to use new error type.
Broker state management and inbound message gating
src/main.rs
Rename session_states to delivery_states map and update all cleanup paths; replace session-mode gate with gate_inbound_for_delivery_mode; update inbound routing in HTTP Send and Relaycast WS to use delivery-mode outcomes; update debug logs and queued-message reason strings.
SDK client API methods and exports
packages/sdk/src/client.ts, packages/sdk/src/index.ts, packages/sdk/src/__tests__/orchestration-upgrades.test.ts
Replace getSessionMode/setSessionMode with getInboundDeliveryMode/setInboundDeliveryMode; update result interface to SetInboundDeliveryModeResult; validate mode values and throw invalid_response on broker errors; update orchestration tests.
Drive command with manual-flush delivery-mode attachment
src/cli/commands/drive.ts, src/cli/commands/drive.test.ts
Read prior delivery mode, flip to manual_flush on attach, restore prior mode on detach; export getInboundDeliveryMode/setInboundDeliveryMode helpers; update renderStatusLine to label as delivery=manual_flush; update error recovery and test assertions for /delivery-mode endpoints.
Passthrough command with auto-inject delivery mode
src/cli/commands/passthrough.ts, src/cli/commands/passthrough.test.ts
Read prior delivery mode, force auto_inject on attach, restore prior mode on detach; import helpers from drive.ts; update renderStatusLine to label as delivery=auto_inject; update error recovery and test assertions for /delivery-mode endpoints.
CLI command registration, bootstrap, and dispatch
src/cli/bootstrap.ts, src/cli/bootstrap.test.ts, src/cli/lib/spawn-and-attach.ts, src/cli/commands/rm.ts, src/cli/lib/broker-connection.ts, src/cli/commands/new.ts, src/cli/commands/new.test.ts
Extend collectTopLevelVerbs to include command aliases; update spawn-and-attach WebSocket casting; update documentation comments to reference passthrough instead of relay; clarify new --mode supported session verbs and payload shapes.
Documentation updates for inbound delivery mode and commands
web/content/docs/reference-broker-api.mdx, web/content/docs/reference-cli.mdx, web/content/docs/typescript-sdk.mdx
Replace session-mode with inbound-delivery-mode documentation; update /api/spawned/{name}/delivery-mode endpoints and auto_inject/manual_flush semantics; clarify drive (queue) vs passthrough (auto-inject) behavior; add worked example and update TypeScript SDK example.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • AgentWorkforce/relay#884: Introduces per-worker session-mode control with /mode//pending//flush endpoints; this PR replaces them with inbound-delivery-mode endpoints.
  • AgentWorkforce/relay#889: Modifies CLI bootstrap verb/alias detection and -n NAME dispatch logic; this PR extends the same plumbing to include command aliases.
  • AgentWorkforce/relay#896: Adds getSessionMode/setSessionMode to AgentRelayClient; this PR replaces them with inbound-delivery-mode equivalents in the same code area.

Suggested reviewers

  • khaliqgant

Poem

🐰 From session modes to delivery streams so bright,
Auto-inject flows or manual queues take flight,
Drive in manual_flush, passthrough stays quick,
The broker's new abstraction—a well-orchestrated trick!
Pending messages queue when you tell them to wait,
State transitions draining at just the right rate. 📨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: renaming inbound delivery modes from session mode terminology to delivery mode terminology.
Description check ✅ Passed The description provides a clear summary of the changes, lists the testing approach, and includes verification steps, though the test checklist items are not marked.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/rename-inbound-delivery-mode

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

web/content/docs/reference-broker-api.mdx

Parsing error: Expression expected.

web/content/docs/reference-cli.mdx

Parsing error: Expression expected.

web/content/docs/typescript-sdk.mdx

Parsing error: Expression expected.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88b12e6d48

ℹ️ 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".

Comment thread src/listen_api.rs
Comment on lines +1229 to +1234
let Some(mode) = InboundDeliveryMode::parse(&body.mode) else {
return api_error(
axum::http::StatusCode::BAD_REQUEST,
"invalid_mode",
format!(
"unsupported session mode '{}' (expected 'passthrough' or 'human')",
"unsupported inbound delivery mode '{}' (expected 'auto_inject' or 'manual_flush')",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve legacy /mode payload values for alias clients

The /api/spawned/{name}/mode route is kept as a compatibility alias, but this handler now parses only auto_inject|manual_flush; older SDK/CLI clients that still send { "mode": "passthrough" } or { "mode": "human" } to /mode will get 400 invalid_mode and fail to attach/restore mode. That breaks the stated back-compat path for existing clients while they migrate.

Useful? React with 👍 / 👎.

Comment thread src/cli/lib/spawn-and-attach.ts Outdated
Comment on lines +260 to +261
if (mode !== 'view' && mode !== 'drive' && mode !== 'relay') {
deps.error(`Error: --mode must be one of view|drive|relay (got '${String(options.mode)}')`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow --mode passthrough in spawn-and-attach path

This validation now rejects new --attach --mode passthrough, even though the interactive command still exposes passthrough as an alias of relay. Existing automation that uses the old attach-mode flag will now fail early with a usage error, creating an inconsistent migration experience across CLI entry points.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main.rs (1)

4314-4339: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't collapse queue eviction into a plain queued result.

InboundDeliveryDispatch::QueuedEvicted means the oldest pending message was dropped, but this helper returns the same GateOutcome::Queued as a normal enqueue. That makes overflow-induced message loss invisible to the HTTP/API callers and any downstream eventing that currently only sees a successful queue.

🤖 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/main.rs` around lines 4314 - 4339, The match arm for
InboundDeliveryDispatch::QueuedEvicted must not map to the same
GateOutcome::Queued as a normal enqueue; change it to return a distinct outcome
(e.g., GateOutcome::QueuedEvicted or GateOutcome::Queued { evicted: true }) so
callers can detect overflow and dropped messages; update the match arm handling
InboundDeliveryDispatch::QueuedEvicted to return that new GateOutcome and
include eviction metadata (dropped_from, queue_len) so state.accept_inbound and
any HTTP/API or eventing paths that consume GateOutcome can surface/log the
eviction to clients and downstream systems (adjust GateOutcome enum/consumers
accordingly).
🧹 Nitpick comments (1)
src/listen_api.rs (1)

3205-3212: ⚡ Quick win

Add auth-coverage for the /mode back-compat alias.

The auth matrix covers /delivery-mode but skips /api/spawned/{name}/mode, which is still intentionally supported. Add GET/PUT alias cases so this compatibility path is protected from regressions.

Suggested test delta
         for (method, path) in [
             ("GET", "/api/spawned/worker-a/delivery-mode"),
             ("PUT", "/api/spawned/worker-a/delivery-mode"),
+            ("GET", "/api/spawned/worker-a/mode"),
+            ("PUT", "/api/spawned/worker-a/mode"),
             ("GET", "/api/spawned/worker-a/pending"),
             ("POST", "/api/spawned/worker-a/flush"),
         ] {
🤖 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/listen_api.rs` around lines 3205 - 3212, The test
inbound_delivery_routes_require_auth currently enumerates routes for auth checks
but omits the backward-compatible alias paths /api/spawned/{name}/mode; update
the route list inside inbound_delivery_routes_require_auth (where test_router is
used) to also include the GET and PUT cases for "/api/spawned/worker-a/mode" so
the legacy aliases are exercised (i.e., add ("GET",
"/api/spawned/worker-a/mode") and ("PUT", "/api/spawned/worker-a/mode") to the
array of (method, path) tuples).
🤖 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 `@src/cli/commands/drive.ts`:
- Around line 496-500: The restore-on-exit broker call to setInboundDeliveryMode
(invoked in the error and 'no_pty' paths and again around lines 673-675) is
awaited with no timeout, so a stalled network request can leave
runDriveSession() unresolved; update those call sites (the
setInboundDeliveryMode(...) invocations) to use a bounded request by adding an
AbortController/timeout or using a provided timeout-capable fetch helper (e.g.,
wrap deps.fetch with a timeout signal or call a timeoutFetch helper) and pass
the controller.signal into setInboundDeliveryMode so the restore is aborted
after a short deadline, ensuring detach/error paths cannot hang indefinitely.

In `@src/cli/commands/relay.ts`:
- Around line 320-325: The restore calls to setInboundDeliveryMode (invoked with
connection, name, previousMode ?? 'auto_inject', deps.fetch) can hang if broker
I/O stalls; wrap those calls in a time-bounded helper that races the original
Promise against a timeout (e.g., Promise.race with a timeout rejection or
resolved sentinel) and ensure the timeout path logs an error via deps.error and
returns/continues gracefully; replace the direct calls in the error branches
shown (the cases that call setInboundDeliveryMode around snapshot handling and
the later calls at the other restore location) with this timed wrapper so
teardown never waits indefinitely.

In `@src/main.rs`:
- Around line 1413-1419: Persist the delivery_states HashMap (type
HashMap<String, InboundDeliveryState>) together with pending_deliveries so
worker inbound delivery modes and queued manual_flush messages survive --persist
restarts: update the same save/load persistence routines that currently
serialize pending_deliveries to also include delivery_states, add
delivery_states to the persisted struct/schema, include it when writing and
restore it when reading, and ensure restored InboundDeliveryState entries are
inserted back into the delivery_states variable (leaving lazy-creation behavior
intact for missing entries).

---

Outside diff comments:
In `@src/main.rs`:
- Around line 4314-4339: The match arm for
InboundDeliveryDispatch::QueuedEvicted must not map to the same
GateOutcome::Queued as a normal enqueue; change it to return a distinct outcome
(e.g., GateOutcome::QueuedEvicted or GateOutcome::Queued { evicted: true }) so
callers can detect overflow and dropped messages; update the match arm handling
InboundDeliveryDispatch::QueuedEvicted to return that new GateOutcome and
include eviction metadata (dropped_from, queue_len) so state.accept_inbound and
any HTTP/API or eventing paths that consume GateOutcome can surface/log the
eviction to clients and downstream systems (adjust GateOutcome enum/consumers
accordingly).

---

Nitpick comments:
In `@src/listen_api.rs`:
- Around line 3205-3212: The test inbound_delivery_routes_require_auth currently
enumerates routes for auth checks but omits the backward-compatible alias paths
/api/spawned/{name}/mode; update the route list inside
inbound_delivery_routes_require_auth (where test_router is used) to also include
the GET and PUT cases for "/api/spawned/worker-a/mode" so the legacy aliases are
exercised (i.e., add ("GET", "/api/spawned/worker-a/mode") and ("PUT",
"/api/spawned/worker-a/mode") to the array of (method, path) tuples).
🪄 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: b3977aba-3539-4b30-a7ae-6e569b66af7e

📥 Commits

Reviewing files that changed from the base of the PR and between da9e912 and 88b12e6.

📒 Files selected for processing (19)
  • packages/sdk/src/__tests__/orchestration-upgrades.test.ts
  • packages/sdk/src/client.ts
  • packages/sdk/src/index.ts
  • packages/sdk/src/protocol.ts
  • src/cli/bootstrap.test.ts
  • src/cli/bootstrap.ts
  • src/cli/commands/drive.test.ts
  • src/cli/commands/drive.ts
  • src/cli/commands/new.test.ts
  • src/cli/commands/new.ts
  • src/cli/commands/relay.test.ts
  • src/cli/commands/relay.ts
  • src/cli/lib/spawn-and-attach.ts
  • src/listen_api.rs
  • src/main.rs
  • src/types.rs
  • web/content/docs/reference-broker-api.mdx
  • web/content/docs/reference-cli.mdx
  • web/content/docs/typescript-sdk.mdx

Comment thread src/cli/commands/drive.ts
Comment on lines +496 to +500
await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
deps.error(`Error: ${snapshot.message ?? `no agent named '${name}'`}`);
return 1;
case 'no_pty':
await setSessionMode(connection, name, previousMode ?? 'passthrough', deps.fetch);
await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Bound restore-on-exit broker calls so detach/error paths can’t hang indefinitely.

Line 496, Line 500, and Line 673 currently wait on a network restore call with no timeout budget. If that request stalls, runDriveSession() can remain unresolved after detach/error.

💡 Suggested fix
+  const restoreModeBestEffort = async (): Promise<void> => {
+    const target = previousMode ?? 'auto_inject';
+    await Promise.race([
+      setInboundDeliveryMode(connection, name, target, deps.fetch).then(() => undefined),
+      new Promise<void>((resolve) => setTimeout(resolve, 1500)),
+    ]);
+  };

   switch (snapshot.status) {
@@
     case 'not_found':
-      await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
+      await restoreModeBestEffort();
       deps.error(`Error: ${snapshot.message ?? `no agent named '${name}'`}`);
       return 1;
     case 'no_pty':
-      await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
+      await restoreModeBestEffort();
       deps.error(`Error: ${snapshot.message ?? `agent '${name}' has no PTY to drive`}`);
       return 1;
@@
-      void setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch).finally(() => {
+      void restoreModeBestEffort().finally(() => {
         resolve(code);
       });

Also applies to: 673-675

🤖 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/cli/commands/drive.ts` around lines 496 - 500, The restore-on-exit broker
call to setInboundDeliveryMode (invoked in the error and 'no_pty' paths and
again around lines 673-675) is awaited with no timeout, so a stalled network
request can leave runDriveSession() unresolved; update those call sites (the
setInboundDeliveryMode(...) invocations) to use a bounded request by adding an
AbortController/timeout or using a provided timeout-capable fetch helper (e.g.,
wrap deps.fetch with a timeout signal or call a timeoutFetch helper) and pass
the controller.signal into setInboundDeliveryMode so the restore is aborted
after a short deadline, ensuring detach/error paths cannot hang indefinitely.

Comment on lines +320 to 325
await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
deps.error(`Error: ${snapshot.message ?? `no agent named '${name}'`}`);
return 1;
case 'no_pty':
await setSessionMode(connection, name, previousMode ?? 'passthrough', deps.fetch);
await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
deps.error(`Error: ${snapshot.message ?? `agent '${name}' has no PTY to attach to`}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid indefinite waits during relay teardown by time-bounding mode restore.

Line 320, Line 324, and Line 452 run restore calls without a timeout. If broker I/O hangs, detach/error completion can stall and never resolve the session promise.

💡 Suggested fix
+  const restoreModeBestEffort = async (): Promise<void> => {
+    const target = previousMode ?? 'auto_inject';
+    await Promise.race([
+      setInboundDeliveryMode(connection, name, target, deps.fetch).then(() => undefined),
+      new Promise<void>((resolve) => setTimeout(resolve, 1500)),
+    ]);
+  };
@@
     case 'not_found':
-      await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
+      await restoreModeBestEffort();
       deps.error(`Error: ${snapshot.message ?? `no agent named '${name}'`}`);
       return 1;
     case 'no_pty':
-      await setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch);
+      await restoreModeBestEffort();
       deps.error(`Error: ${snapshot.message ?? `agent '${name}' has no PTY to attach to`}`);
       return 1;
@@
-      void setInboundDeliveryMode(connection, name, previousMode ?? 'auto_inject', deps.fetch).finally(() => {
+      void restoreModeBestEffort().finally(() => {
         resolve(code);
       });

Also applies to: 452-454

🤖 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/cli/commands/relay.ts` around lines 320 - 325, The restore calls to
setInboundDeliveryMode (invoked with connection, name, previousMode ??
'auto_inject', deps.fetch) can hang if broker I/O stalls; wrap those calls in a
time-bounded helper that races the original Promise against a timeout (e.g.,
Promise.race with a timeout rejection or resolved sentinel) and ensure the
timeout path logs an error via deps.error and returns/continues gracefully;
replace the direct calls in the error branches shown (the cases that call
setInboundDeliveryMode around snapshot handling and the later calls at the other
restore location) with this timed wrapper so teardown never waits indefinitely.

Comment thread src/main.rs
Comment on lines +1413 to +1419
// Per-worker inbound-delivery-mode + pending-relay-message queue. Lives
// parallel to `workers.workers` so we can swap modes / inspect /
// drain without touching `WorkerHandle` (which holds OS-level
// process state). See `relay_broker::types::SessionState`. Entries
// process state). See `relay_broker::types::InboundDeliveryState`. Entries
// are created lazily on first lookup and removed wherever workers
// exit (`Release` arm, `worker_exited` frame, `reap_exited` sweep).
let mut session_states: HashMap<String, SessionState> = HashMap::new();
let mut delivery_states: HashMap<String, InboundDeliveryState> = HashMap::new();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Persist delivery_states alongside pending_deliveries.

delivery_states is now the only place that stores a worker's inbound delivery mode and its queued manual_flush messages, but this file still only saves/loads pending_deliveries. In --persist mode, a broker restart will silently reset workers back to the default mode and drop any queued inbound messages that were waiting for a flush.

🤖 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/main.rs` around lines 1413 - 1419, Persist the delivery_states HashMap
(type HashMap<String, InboundDeliveryState>) together with pending_deliveries so
worker inbound delivery modes and queued manual_flush messages survive --persist
restarts: update the same save/load persistence routines that currently
serialize pending_deliveries to also include delivery_states, add
delivery_states to the persisted struct/schema, include it when writing and
restore it when reading, and ensure restored InboundDeliveryState entries are
inserted back into the delivery_states variable (leaving lazy-creation behavior
intact for missing entries).

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 19 files

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Re-trigger cubic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/listen_api.rs (1)

1204-1230: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize malformed delivery-mode bodies to the documented 400 response.

Right now, only syntactically valid { "mode": "..." } payloads reach InboundDeliveryMode::parse(). Missing fields, wrong types, invalid JSON, or a missing JSON content type are rejected by Axum's Json extractor first, which returns 422 Unprocessable Entity by default—conflicting with the documented invalid_mode contract that promises a 400 response.

Accept Result<axum::Json<SetInboundDeliveryModePayload>, _> to intercept JsonRejection and map those cases through the same api_error(..., "invalid_mode", ...) path. Add regression tests for {} and { "mode": 1 } to lock in the fix.

🤖 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/listen_api.rs` around lines 1204 - 1230, The handler
listen_api_set_inbound_delivery_mode currently uses axum::Json extractor which
yields 422 on malformed/missing fields; change the signature to accept
Result<axum::Json<SetInboundDeliveryModePayload>,
axum::extract::rejection::JsonRejection> so you can match the Err case and
convert it into the same api_error(axum::http::StatusCode::BAD_REQUEST,
"invalid_mode", ...) response used for invalid_mode, then proceed to call
InboundDeliveryMode::parse on the Ok payload as before; also add regression
tests that POST/PUT with {} and with { "mode": 1 } produce the documented 400
"invalid_mode" response to lock behavior.
🤖 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 `@web/content/docs/reference-broker-api.mdx`:
- Around line 467-469: The curl examples for the PUT
/api/spawned/{id}/delivery-mode endpoint are missing the required Content-Type:
application/json header, causing Axum's Json extractor to reject the request;
update the examples that call the PUT /delivery-mode (shown as curl -sX PUT
localhost:3888/api/spawned/Alice/delivery-mode ...) to include -H "Content-Type:
application/json" (and keep the existing X-API-Key header) so the JSON body is
accepted by the handler.

---

Outside diff comments:
In `@src/listen_api.rs`:
- Around line 1204-1230: The handler listen_api_set_inbound_delivery_mode
currently uses axum::Json extractor which yields 422 on malformed/missing
fields; change the signature to accept
Result<axum::Json<SetInboundDeliveryModePayload>,
axum::extract::rejection::JsonRejection> so you can match the Err case and
convert it into the same api_error(axum::http::StatusCode::BAD_REQUEST,
"invalid_mode", ...) response used for invalid_mode, then proceed to call
InboundDeliveryMode::parse on the Ok payload as before; also add regression
tests that POST/PUT with {} and with { "mode": 1 } produce the documented 400
"invalid_mode" response to lock behavior.
🪄 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: d7968b2f-d335-407f-8aae-046f2c6e6243

📥 Commits

Reviewing files that changed from the base of the PR and between 88b12e6 and de77083.

📒 Files selected for processing (2)
  • src/listen_api.rs
  • web/content/docs/reference-broker-api.mdx

Comment on lines +467 to +469
curl -sX PUT localhost:3888/api/spawned/Alice/delivery-mode \
-H "X-API-Key: $KEY" \
-d '{"mode":"human"}'
-d '{"mode":"manual_flush"}'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In axum 0.8, does the Jsonextractor requireContent-Type: application/json, and what response status does it return when that header is missing?

💡 Result:

In Axum 0.8, the Json extractor does require the request to have a Content-Type: application/json (or compatible) header [1][2]. When this header is missing, the extractor rejects the request with a JsonRejection::MissingJsonContentType error [3][4]. By default, this rejection is converted into an HTTP 400 Bad Request response [3][5]. You can customize this behavior by handling the JsonRejection explicitly in your handler or by using middleware to map the rejection to a different status code if desired [3][6].

Citations:


🏁 Script executed:

# Find and examine the listen_api.rs handler for delivery-mode
find . -name "listen_api.rs" -type f

Repository: AgentWorkforce/relay

Length of output: 84


🏁 Script executed:

# Read the documentation file around the affected lines
cat -n web/content/docs/reference-broker-api.mdx | sed -n '460,500p'

Repository: AgentWorkforce/relay

Length of output: 1593


🏁 Script executed:

# Search for the delivery-mode handler in listen_api.rs
rg -A 10 "delivery-mode" src/listen_api.rs

Repository: AgentWorkforce/relay

Length of output: 6305


Add Content-Type: application/json to the new PUT /delivery-mode examples.

These two curl examples send JSON with -d, but the handler uses Axum's Json extractor, which requires the Content-Type header. Without it, requests fail with HTTP 400 Bad Request before reaching the route logic. The test suite confirms this requirement by explicitly setting the header on all PUT requests to this endpoint.

Suggested doc fix
 curl -sX PUT localhost:3888/api/spawned/Alice/delivery-mode \
   -H "X-API-Key: $KEY" \
+  -H "Content-Type: application/json" \
   -d '{"mode":"manual_flush"}'
@@
 curl -sX PUT localhost:3888/api/spawned/Alice/delivery-mode \
   -H "X-API-Key: $KEY" \
+  -H "Content-Type: application/json" \
   -d '{"mode":"auto_inject"}'

Also applies to: 490-492

🤖 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 `@web/content/docs/reference-broker-api.mdx` around lines 467 - 469, The curl
examples for the PUT /api/spawned/{id}/delivery-mode endpoint are missing the
required Content-Type: application/json header, causing Axum's Json extractor to
reject the request; update the examples that call the PUT /delivery-mode (shown
as curl -sX PUT localhost:3888/api/spawned/Alice/delivery-mode ...) to include
-H "Content-Type: application/json" (and keep the existing X-API-Key header) so
the JSON body is accepted by the handler.

@willwashburn willwashburn merged commit e8502cb into main May 18, 2026
49 of 51 checks passed
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@willwashburn willwashburn deleted the codex/rename-inbound-delivery-mode branch May 18, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant