Harden Codex GPT-5.5 worker spawning#856
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughPromotes GPT-5.5 across registries and SDKs, implements local Codex CLI catalog probing and model-arg rewrite in worker spawn (spawn now returns enriched AgentSpec), updates CI/workflows/dependency pins/pricing/tests, adds CLI spawn flags and broker readiness polling, and records trajectory artifacts. ChangesGPT-5.5 Model Promotion and Codex Fallback Hardening
Sequence DiagramsequenceDiagram
participant Client
participant WorkerRegistry
participant Resolver
participant CodexCLI
participant BrokerMain
Client->>WorkerRegistry: spawn(spec, args)
WorkerRegistry->>Resolver: resolve_model_flag_for_cli(provider, args)
Resolver->>CodexCLI: `codex debug models` (1.5s timeout)
CodexCLI-->>Resolver: JSON models list / timeout
Resolver-->>WorkerRegistry: Some(model) or None
WorkerRegistry->>WorkerRegistry: rewrite args if unsupported, set spec.model
WorkerRegistry-->>Client: Ok(effective_spec)
Client->>BrokerMain: persist effective_spec, emit telemetry/event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.trajectories/completed/2026-05/traj_2yicjxgajt0a.json:
- Line 47: Replace the absolute local path stored in the JSON key "projectId"
with a repo-stable identifier (e.g., a repo slug, relative path, or remove the
key entirely) to avoid leaking user-specific filesystem details; update the
"projectId" value in this trajectory JSON and apply the same scrub to all other
new trajectory JSON files added in this PR so none contain absolute /Users/...
paths.
In @.trajectories/index.json:
- Around line 406-433: The index entries (e.g., objects
"traj_n8duofq5vq1a","traj_7zu7et53ph3l","traj_2yicjxgajt0a","traj_hrsndfzk0qay")
currently store machine-local absolute paths in their "path" fields; change them
to repository-relative paths (for example
".trajectories/completed/2026-05/traj_*.json") so the index is portable across
machines/CI, and update whatever writer that emits these entries to build paths
relative to the repo root (use the same relative format used elsewhere in the
repo) instead of using process-specific absolute paths.
In `@packages/shared/cli-registry.yaml`:
- Around line 33-45: The CLI registry YAML changes (new/edited model entries
gpt_5_5 and gpt_5_4 in packages/shared/cli-registry.yaml) are not reflected in
the generated artifacts causing CI failures; run the model codegen task (npm run
codegen:models), review the generated outputs for the models referenced
(gpt_5_5, gpt_5_4), and commit all updated generated files produced by that
command so the generated-model artifacts stay in sync with the registry edits.
🪄 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: 43f1fad9-6620-41e0-9329-9452e7d8084c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
.github/workflows/build-broker-binary.yml.github/workflows/publish.yml.github/workflows/rust-ci.yml.github/workflows/test.yml.trajectories/completed/2026-05/traj_2yicjxgajt0a.json.trajectories/completed/2026-05/traj_2yicjxgajt0a.md.trajectories/completed/2026-05/traj_7zu7et53ph3l.json.trajectories/completed/2026-05/traj_7zu7et53ph3l.md.trajectories/completed/2026-05/traj_hrsndfzk0qay.json.trajectories/completed/2026-05/traj_hrsndfzk0qay.md.trajectories/completed/2026-05/traj_n8duofq5vq1a.json.trajectories/completed/2026-05/traj_n8duofq5vq1a.md.trajectories/index.jsonCargo.tomlpackages/config/src/cli-registry.generated.tspackages/sdk-py/src/agent_relay/models.pypackages/sdk/src/__tests__/models.test.tspackages/shared/cli-registry.yamlsrc/cost/pricing.tssrc/cost/tracker.test.tssrc/worker.rsworkflows/cloud-connect/fix-agent-relay-utils-bundling.tsworkflows/cloud-connect/fix-cloud-connect-claude-hang.tsworkflows/cloud-connect/validate-cloud-connect-e2e.tsworkflows/relay-e2e-meta-workflow.ts
| }, | ||
| "commits": [], | ||
| "filesChanged": [], | ||
| "projectId": "/Users/khaliqgant/Projects/AgentWorkforce/relay", |
There was a problem hiding this comment.
Remove absolute local path from committed trajectory metadata.
Line 47 stores an absolute machine path (/Users/...) including a local username. That leaks environment-specific identifier data into VCS. Use a repo-stable identifier (or omit projectId) and apply the same scrub to the other new trajectory JSON files in this PR.
Suggested change
- "projectId": "/Users/khaliqgant/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.
Suggested change
| "projectId": "/Users/khaliqgant/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 @.trajectories/completed/2026-05/traj_2yicjxgajt0a.json at line 47, Replace
the absolute local path stored in the JSON key "projectId" with a repo-stable
identifier (e.g., a repo slug, relative path, or remove the key entirely) to
avoid leaking user-specific filesystem details; update the "projectId" value in
this trajectory JSON and apply the same scrub to all other new trajectory JSON
files added in this PR so none contain absolute /Users/... paths.
Comment on lines
+33
to
45
| version: '0.130.0' | ||
| install: 'npm install -g @openai/codex' | ||
| models: | ||
| gpt_5_5: | ||
| id: 'gpt-5.5' | ||
| label: 'GPT-5.5 — Frontier model for complex coding, research, and real-world work.' | ||
| default: true | ||
| reasoning_efforts: ['low', 'medium', 'high', 'xhigh'] | ||
| default_reasoning_effort: 'xhigh' | ||
| gpt_5_4: | ||
| id: 'gpt-5.4' | ||
| label: 'GPT-5.4 — Frontier model for complex coding, research, and real-world work.' | ||
| default: true | ||
| label: 'GPT-5.4 — More affordable model for complex coding and professional work.' | ||
| reasoning_efforts: ['low', 'medium', 'high', 'xhigh'] |
There was a problem hiding this comment.
Codegen outputs are still out of sync with this registry source.
CI is currently failing on Codegen Models / codegen with generated-model drift after these YAML edits. Please run npm run codegen:models and commit all regenerated artifacts before merge.
Also applies to: 534-536
🤖 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/shared/cli-registry.yaml` around lines 33 - 45, The CLI registry
YAML changes (new/edited model entries gpt_5_5 and gpt_5_4 in
packages/shared/cli-registry.yaml) are not reflected in the generated artifacts
causing CI failures; run the model codegen task (npm run codegen:models), review
the generated outputs for the models referenced (gpt_5_5, gpt_5_4), and commit
all updated generated files produced by that command so the generated-model
artifacts stay in sync with the registry edits.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.trajectories/index.json (1)
406-488:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse repository-relative paths for trajectory records.
At Line 411, Line 418, Line 425, Line 432, and throughout Lines 439-488,
pathvalues are machine-local absolute paths. This makes the index non-portable and leaks local environment details.💡 Proposed fix
- "path": "/Users/khaliqgant/Projects/AgentWorkforce/relay/.trajectories/completed/2026-05/traj_n8duofq5vq1a.json" + "path": ".trajectories/completed/2026-05/traj_n8duofq5vq1a.json"Apply the same normalization to all added entries in this block.
🤖 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 @.trajectories/index.json around lines 406 - 488, The trajectory index entries (e.g., keys "traj_n8duofq5vq1a", "traj_7zu7et53ph3l", "traj_2yicjxgajt0a", "traj_hrsndfzk0qay", "traj_4t07itef99ug" and the other traj_* entries in this block) contain machine-local absolute paths in their "path" fields; change each "path" value to the repository-relative path (for example ".trajectories/completed/2026-05/<filename>.json") so all records are portable and do not leak local environment details, applying this normalization consistently across the whole block shown.
🤖 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 @.trajectories/completed/2026-05/traj_fh8oosbijpwc.md:
- Around line 20-23: Replace all machine-local absolute paths in the trajectory
doc entries with repo-relative or generic references: remove occurrences of
"/Users/..." and instead say something like "sibling relaycast repo" or "sibling
repo containing `@relaycast/sdk` source"; update the lines that contain the
sentence starting "Relaycast Track A implementation belongs in sibling repo ..."
and any other occurrences (e.g., the mentions at lines where the absolute path
is embedded) so no user-home paths remain and references point to the sibling
repo or package name instead.
---
Duplicate comments:
In @.trajectories/index.json:
- Around line 406-488: The trajectory index entries (e.g., keys
"traj_n8duofq5vq1a", "traj_7zu7et53ph3l", "traj_2yicjxgajt0a",
"traj_hrsndfzk0qay", "traj_4t07itef99ug" and the other traj_* entries in this
block) contain machine-local absolute paths in their "path" fields; change each
"path" value to the repository-relative path (for example
".trajectories/completed/2026-05/<filename>.json") so all records are portable
and do not leak local environment details, applying this normalization
consistently across the whole block shown.
🪄 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: 8df0e7df-bf59-4163-9115-c2724ff94afc
📒 Files selected for processing (20)
.trajectories/completed/2026-05/traj_4t07itef99ug.json.trajectories/completed/2026-05/traj_4t07itef99ug.md.trajectories/completed/2026-05/traj_6sjeohtm3php.json.trajectories/completed/2026-05/traj_6sjeohtm3php.md.trajectories/completed/2026-05/traj_947wzpddsg9j.json.trajectories/completed/2026-05/traj_947wzpddsg9j.md.trajectories/completed/2026-05/traj_fh8oosbijpwc.json.trajectories/completed/2026-05/traj_fh8oosbijpwc.md.trajectories/completed/2026-05/traj_fh8oosbijpwc.trace.json.trajectories/completed/2026-05/traj_gh05rj5gwsap.json.trajectories/completed/2026-05/traj_gh05rj5gwsap.md.trajectories/completed/2026-05/traj_gh05rj5gwsap.trace.json.trajectories/completed/2026-05/traj_tgism98me5na.json.trajectories/completed/2026-05/traj_tgism98me5na.md.trajectories/completed/2026-05/traj_v1wexlfur5zr.json.trajectories/completed/2026-05/traj_v1wexlfur5zr.md.trajectories/completed/2026-05/traj_whd40oxptlhn.trace.json.trajectories/completed/2026-05/traj_zfa6skfr32vy.json.trajectories/completed/2026-05/traj_zfa6skfr32vy.md.trajectories/index.json
✅ Files skipped from review due to trivial changes (16)
- .trajectories/completed/2026-05/traj_gh05rj5gwsap.json
- .trajectories/completed/2026-05/traj_4t07itef99ug.json
- .trajectories/completed/2026-05/traj_6sjeohtm3php.md
- .trajectories/completed/2026-05/traj_6sjeohtm3php.json
- .trajectories/completed/2026-05/traj_v1wexlfur5zr.json
- .trajectories/completed/2026-05/traj_gh05rj5gwsap.trace.json
- .trajectories/completed/2026-05/traj_tgism98me5na.md
- .trajectories/completed/2026-05/traj_fh8oosbijpwc.trace.json
- .trajectories/completed/2026-05/traj_947wzpddsg9j.json
- .trajectories/completed/2026-05/traj_zfa6skfr32vy.json
- .trajectories/completed/2026-05/traj_tgism98me5na.json
- .trajectories/completed/2026-05/traj_v1wexlfur5zr.md
- .trajectories/completed/2026-05/traj_gh05rj5gwsap.md
- .trajectories/completed/2026-05/traj_947wzpddsg9j.md
- .trajectories/completed/2026-05/traj_fh8oosbijpwc.json
- .trajectories/completed/2026-05/traj_whd40oxptlhn.trace.json
Comment on lines
+20
to
+23
| ### Relaycast Track A implementation belongs in sibling repo /Users/khaliqgant/Projects/AgentWorkforce/relaycast because @relaycast/sdk source lives there | ||
|
|
||
| - **Chose:** Relaycast Track A implementation belongs in sibling repo /Users/khaliqgant/Projects/AgentWorkforce/relaycast because @relaycast/sdk source lives there | ||
| - **Reasoning:** The relay repo only re-exports RelayCast from @relaycast/sdk; package source is in the sibling repo. |
There was a problem hiding this comment.
Remove machine-local absolute paths from committed trajectory docs.
Line 20, Line 22, and Line 43 embed /Users/... paths. Please replace with repo-relative or generic references (e.g., “sibling relaycast repo”) to avoid leaking local environment details.
💡 Proposed fix
- ### Relaycast Track A implementation belongs in sibling repo /Users/khaliqgant/Projects/AgentWorkforce/relaycast because `@relaycast/sdk` source lives there
+ ### Relaycast Track A implementation belongs in sibling relaycast repo because `@relaycast/sdk` source lives there
- - **Chose:** Relaycast Track A implementation belongs in sibling repo /Users/khaliqgant/Projects/AgentWorkforce/relaycast because `@relaycast/sdk` source lives there
+ - **Chose:** Relaycast Track A implementation belongs in sibling relaycast repo because `@relaycast/sdk` source lives there
- - Relaycast Track A implementation belongs in sibling repo /Users/khaliqgant/Projects/AgentWorkforce/relaycast because `@relaycast/sdk` source lives there: Relaycast Track A implementation belongs in sibling repo /Users/khaliqgant/Projects/AgentWorkforce/relaycast because `@relaycast/sdk` source lives there
+ - Relaycast Track A implementation belongs in sibling relaycast repo because `@relaycast/sdk` source lives there: Relaycast Track A implementation belongs in sibling relaycast repo because `@relaycast/sdk` source lives thereAlso applies to: 43-43
🤖 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 @.trajectories/completed/2026-05/traj_fh8oosbijpwc.md around lines 20 - 23,
Replace all machine-local absolute paths in the trajectory doc entries with
repo-relative or generic references: remove occurrences of "/Users/..." and
instead say something like "sibling relaycast repo" or "sibling repo containing
`@relaycast/sdk` source"; update the lines that contain the sentence starting
"Relaycast Track A implementation belongs in sibling repo ..." and any other
occurrences (e.g., the mentions at lines where the absolute path is embedded) so
no user-home paths remain and references point to the sibling repo or package
name instead.
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 `@src/cli/commands/agent-management.ts`:
- Around line 93-108: When waitForReadyBrokerClient(client) throws a non-warmup
error while autoStart is true, the existing client connection is left open;
modify the inner catch (the one that currently checks !autoStart ||
isBrokerWarmupError(err)) to explicitly shut down the partially-initialized
client before falling through to auto-start logic: call and await the client's
close/shutdown method (e.g., await client.close()) and swallow/log any close
errors, then allow the flow to continue so a new broker/client can be created;
reference connect(), waitForReadyBrokerClient, isBrokerWarmupError, autoStart
and the client/AgentManagementClient variable when making the change.
🪄 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: d2bf7729-9362-4a9c-b670-23b2fee08b28
📒 Files selected for processing (14)
.prettierignore.trajectories/completed/2026-05/traj_3gjtcykvybt5.json.trajectories/completed/2026-05/traj_3gjtcykvybt5.md.trajectories/completed/2026-05/traj_o251whkvy9rl.json.trajectories/completed/2026-05/traj_o251whkvy9rl.md.trajectories/index.jsonpackages/config/src/cli-registry.generated.tssrc/auth.rssrc/cli/commands/agent-management.test.tssrc/cli/commands/agent-management.tssrc/main.rssrc/routing.rssrc/swarm.rssrc/worker.rs
✅ Files skipped from review due to trivial changes (9)
- .trajectories/completed/2026-05/traj_3gjtcykvybt5.md
- .prettierignore
- .trajectories/completed/2026-05/traj_3gjtcykvybt5.json
- .trajectories/completed/2026-05/traj_o251whkvy9rl.json
- src/auth.rs
- src/routing.rs
- .trajectories/index.json
- src/swarm.rs
- packages/config/src/cli-registry.generated.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/worker.rs
Comment on lines
93
to
108
| try { | ||
| await waitForReadyBrokerClient(client); | ||
| return client as unknown as AgentManagementClient; | ||
| } catch (err) { | ||
| if (!autoStart || isBrokerWarmupError(err)) { | ||
| throw err; | ||
| } | ||
| } | ||
| } catch (err) { | ||
| if (!autoStart) { | ||
| if (err instanceof Error && err.message !== '') { | ||
| throw err; | ||
| } | ||
| throw new Error('No running broker found. Start one with: agent-relay up'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential resource leak: connected client not cleaned up on non-warmup error.
When autoStart=true, connect() succeeds, but waitForReadyBrokerClient throws a non-warmup error (e.g., auth failure), the inner condition !autoStart || isBrokerWarmupError(err) evaluates to false. Control falls through without shutting down the first client, then proceeds to start a new broker and create another client—leaking the original connection.
🛡️ Suggested fix: shut down client before falling through
try {
const client = AgentRelayClient.connect({ cwd });
try {
await waitForReadyBrokerClient(client);
return client as unknown as AgentManagementClient;
} catch (err) {
if (!autoStart || isBrokerWarmupError(err)) {
throw err;
}
+ // Non-warmup error with autoStart: clean up before falling through to start new broker
+ await client.shutdown?.().catch(() => undefined);
}
} catch (err) {🤖 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/agent-management.ts` around lines 93 - 108, When
waitForReadyBrokerClient(client) throws a non-warmup error while autoStart is
true, the existing client connection is left open; modify the inner catch (the
one that currently checks !autoStart || isBrokerWarmupError(err)) to explicitly
shut down the partially-initialized client before falling through to auto-start
logic: call and await the client's close/shutdown method (e.g., await
client.close()) and swallow/log any close errors, then allow the flow to
continue so a new broker/client can be created; reference connect(),
waitForReadyBrokerClient, isBrokerWarmupError, autoStart and the
client/AgentManagementClient variable when making the change.
khaliqgant
force-pushed
the
skill/running-headless-orchestrator-nohup
branch
from
1b917eb to
5e1e108
Compare
khaliqgant
force-pushed
the
skill/running-headless-orchestrator-nohup
branch
from
5e1e108 to
b15fcdd
Compare
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
codex debug modelscatalog and falling back to GPT-5.4 when GPT-5.5 requires a newer CLI--model/-margs and update workflow launchers to use the registry constantValidation
cargo check --bin agent-relay-brokercargo test worker::testsnpx vitest run src/cost/tracker.test.tscd packages/sdk && npx vitest run src/__tests__/models.test.ts