feat(rook): complete operator surfaces, security baseline, and audit trail#630
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 51 seconds. ⌛ 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (146)
📝 WalkthroughWalkthroughThis PR implements comprehensive additions to the Rook gateway API with streaming support, idempotency, admin management, and transport infrastructure, while expanding agent-runtime with richer orchestration lifecycle tracking and memory consolidation. It includes a new Vue dashboard, GitHub branch protection rulesets, and extensive backend services across auth, database, and HTTP middleware layers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as Rook Gateway<br/>(Middleware Stack)
participant Upstream as Upstream Provider<br/>(OpenAI/Anthropic)
Note over Client,Upstream: Chat Completions with Idempotency
Client->>Gateway: POST /v1/chat/completions<br/>{"stream": false,<br/>"idempotency-key": "..."}
Gateway->>Gateway: 1. Extract bearer token<br/>2. Validate idempotency key<br/>3. Canonicalize JSON body<br/>4. Hash canonical bytes
Gateway->>Gateway: Check idempotency store<br/>for (principal, key, method, path)
alt Replay Completed
Gateway->>Gateway: Load stored response
Gateway-->>Client: 200 OK<br/>+ Idempotency-Replayed: true
else In Progress
Gateway-->>Client: 409 Conflict<br/>idempotency_request_in_progress
else Key Reused Mismatch
Gateway-->>Client: 409 Conflict<br/>idempotency_key_reused
else New Request
Gateway->>Gateway: Reserve idempotency slot<br/>status = InProgress
Gateway->>Upstream: POST /v1/chat/completions<br/>(buffered)
Upstream-->>Gateway: 200 + JSON response
Gateway->>Gateway: Store response + status<br/>= Completed
Gateway-->>Client: 200 OK<br/>+ Idempotency-Replayed: false
end
sequenceDiagram
participant Client
participant Gateway as Rook Gateway<br/>(Middleware Stack)
participant Upstream as Upstream Provider
Note over Client,Upstream: Streaming Chat Completions (stream=true)
Client->>Gateway: POST /v1/chat/completions<br/>{"stream": true}
Gateway->>Gateway: 1. Auth validation<br/>2. Rate limit check<br/>3. Route to streaming handler
Gateway->>Upstream: POST /v1/chat/completions<br/>(stream request)
Upstream-->>Gateway: 200 OK<br/>(streaming response)
Gateway->>Gateway: Open byte stream<br/>from upstream
Gateway->>Gateway: Parse SSE frames<br/>("data: {...}\n\n")
loop For each upstream chunk
Gateway->>Gateway: Validate frame format<br/>Reject duplicate [DONE]
Gateway->>Client: SSE Event<br/>(event:, id:, data:)
end
Upstream-->>Gateway: [DONE]
Gateway-->>Client: Final SSE Event<br/>([DONE])
Gateway->>Gateway: Update account health<br/>(async)
sequenceDiagram
participant Parent as Parent Runtime<br/>(Coordinator)
participant Mailbox
participant Child as Child Process
Note over Parent,Child: Child Launch with Execution Contract
Parent->>Parent: validate child execution spec<br/>(transport, isolation, broker)
alt Unsupported Contract
Parent-->>Parent: CoordinatorError<br/>LaunchContractRejected
else Contract Valid
Parent->>Parent: Normalize execution metadata<br/>(requested vs enforced)
Parent->>Parent: Emit CoordinatorEvent<br/>(ChildStateChanged → Queued)
Parent->>Mailbox: Enqueue launch message<br/>(with execution spec)
Mailbox-->>Child: Deliver launch
Child->>Child: Process launch
Parent->>Parent: Emit CoordinatorEvent<br/>(ChildStateChanged → Starting)
Parent->>Parent: Persist event to log
Note over Parent: Inspect returns child state + events
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Rationale: This PR introduces substantial, interconnected changes across multiple technical domains (Rust backend services, web frontend, database persistence, authentication, transport middleware, streaming protocols, and state machine logic). Key factors:
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-24 to 2026-04-24 |
Deploying corvus with
|
| Latest commit: |
28df7ff
|
| Status: | ⚡️ Build in progress... |
github-actions
Bot
added
size/xl
yacosta738
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 68
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
clients/agent-runtime/src/tools/delegate.rs (1)
274-283:⚠️ Potential issue | 🟠 MajorReject unsupported execution fields instead of silently dropping them.
Session-mode
delegatebuildsexecution: None, whileexecute()ignores any extra request fields. A caller that passesremote_bridge, transport, isolation, or escalation metadata can be silently run locally, which violates the fail-closed contract.🛡️ Proposed fix
async fn execute(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> { - let agent_name = args - .get("agent") + let Some(args_obj) = args.as_object() else { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some("delegate arguments must be an object".into()), + structured: None, + }); + }; + + if let Some(unsupported) = args_obj + .keys() + .find(|key| !matches!(key.as_str(), "agent" | "prompt" | "context")) + { + return Ok(ToolResult { + success: false, + output: String::new(), + error: Some(format!( + "Unsupported delegate parameter '{unsupported}'; remote transport, isolation, and escalation fields are not accepted by delegate" + )), + structured: None, + }); + } + + let agent_name = args_obj + .get("agent") .and_then(|v| v.as_str()) .map(str::trim) .ok_or_else(|| anyhow::anyhow!("Missing 'agent' parameter"))?; if agent_name.is_empty() { @@ - let prompt = args - .get("prompt") + let prompt = args_obj + .get("prompt") .and_then(|v| v.as_str()) .map(str::trim) .ok_or_else(|| anyhow::anyhow!("Missing 'prompt' parameter"))?; @@ - let context = args - .get("context") + let context = args_obj + .get("context") .and_then(|v| v.as_str()) .map(str::trim) .unwrap_or("");As per coding guidelines,
clients/agent-runtime/src/tools/**/*.rs: ImplementTooltrait insrc/tools/with strict parameter schema, validate and sanitize all inputs, and return structuredToolResultwithout panics in runtime path.Also applies to: 348-438
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/tools/delegate.rs` around lines 274 - 283, The delegate session builder currently constructs CoordinatorLaunchRequest/ChildLaunchRequest with execution: None and silently drops execution-related fields (remote_bridge, transport, isolation, escalation), so update the delegate handling in clients/agent-runtime/src/tools/delegate.rs to validate incoming request parameters against the Tool trait schema and fail-fast: if any execution-specific fields are present in the incoming request, return a structured ToolResult error (not a panic) indicating unsupported execution metadata for session-mode delegates; specifically add checks around the code that constructs CoordinatorLaunchRequest and ChildLaunchRequest to inspect execution/remote_bridge/transport/isolation/escalation and reject with a clear ToolResult error, and ensure execute() is updated to propagate/validate these errors rather than ignoring extra fields.clients/agent-runtime/src/agent/coordinator.rs (2)
1034-1081:⚠️ Potential issue | 🟠 MajorLifecycle gap:
Cancellingcan be overwritten by a late child message.
ChildStarted,ChildProgress,RequestApproval, andResolveApprovalonly reject onis_terminal(), butCancellingis non-terminal. A message that races withapply_cancellation_visibility(or a slow in-process runner) will flip the record back toRunning/WaitingOnParent, silently erasing the "cancellation in progress" state that the spec guarantees callers can observe, and re-emitting aChildStateChangedevent that contradicts prior cancellation visibility.Guard these transitions against
Cancellingtoo (either ignore or fold into the cancelling summary without state regression):🛡️ Proposed fix
CoordinatorMessage::ChildStarted { session_id } => { - if record.state.is_terminal() { + if record.state.is_terminal() || record.state == ChildState::Cancelling { return Err(CoordinatorError::AlreadyTerminalState); } record.state = ChildState::Running; record.approval = ApprovalStatus::None; record.session_id = session_id.clone(); } CoordinatorMessage::ChildProgress { summary } => { - if record.state.is_terminal() { + if record.state.is_terminal() || record.state == ChildState::Cancelling { return Err(CoordinatorError::AlreadyTerminalState); } - record.state = ChildState::Running; - record.summary = Some(summary.clone()); + record.summary = Some(summary.clone()); } CoordinatorMessage::RequestApproval { request } => { - if record.state.is_terminal() { + if record.state.is_terminal() || record.state == ChildState::Cancelling { return Err(CoordinatorError::AlreadyTerminalState); } record.state = ChildState::WaitingOnParent;As per coding guidelines: "Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable" — cancellation visibility counts as a policy constraint the parent relies on.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/agent/coordinator.rs` around lines 1034 - 1081, The handlers for CoordinatorMessage::ChildStarted, ::ChildProgress, ::RequestApproval, and ::ResolveApproval currently only reject terminal states but will overwrite a non-terminal ChildState::Cancelling; update each handler in the apply_message/apply_envelope path so that before mutating record.state or emitting events (e.g., via self.log_event(CoordinatorEvent::...)) you check for record.state == ChildState::Cancelling and either ignore the incoming message or merge it into the cancelling state (e.g., update summary/approval without changing state or emitting a ChildStateChanged), ensuring apply_cancellation_visibility cannot be reverted by late child messages; keep the check colocated with the existing is_terminal() guard so all four message arms (ChildStarted, ChildProgress, RequestApproval, ResolveApproval) behave consistently.
1838-1926: 🧹 Nitpick | 🔵 Trivial
admit_childruns three times per child on the launch path.
validate_launch_request(via a throwawayCoordinator), thensnapshot_seed.admit_child, thencoordinator.run_with_cancellation→admit_childagain inside the spawned task. Thesnapshot_seedcoordinator is immediately dropped after producing the initial snapshot, so it contributes nothing beyond duplicating validation. Consider collapsing to one: validate once, build the initial snapshot from the validator (or from the real coordinator after a pre-admit that the spawned task then skips).Not a correctness bug —
admit_childis deterministic for valid inputs — but it's extra allocation and a third place any future admission invariant needs to stay in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/agent/coordinator.rs` around lines 1838 - 1926, The validator flow currently admits children three times; change validate_launch_request to build and return a pre-admitted Coordinator (or accept a Coordinator to admit into) instead of creating a throwaway inside the function, then in launch reuse that returned Coordinator as both the snapshot_seed and the coordinator passed into tokio::spawn (so snapshot_from_coordinator uses it and run_with_cancellation is started on the same pre-admitted Coordinator), ensuring only one admit_child sequence; update references: validate_launch_request, snapshot_from_coordinator, launch, Coordinator::admit_child, and run_with_cancellation to use the shared pre-admitted Coordinator.clients/rook/src/db/settings.rs (1)
62-68:⚠️ Potential issue | 🟠 MajorUse checked conversions for persisted numeric settings.
The
row_to_settings()function retrieves signedi64values from SQLite and casts them to unsigned types (u16,u32,u64) with uncheckedasoperators at lines 63, 66–67. Corrupted or manually edited database rows containing out-of-range values (negative numbers, values exceeding type limits) will silently wrap or truncate into valid-looking configuration. Replace these withtry_from()checks:Ok(RookSettings { - gateway_port: gateway_port as u16, + gateway_port: u16::try_from(gateway_port) + .map_err(|_| RookError::Registry(format!("gateway_port out of range: {gateway_port}")))?, default_routing_policy: RoutingPolicy { strategy, - max_retries: max_retries as u32, - cooldown_seconds: cooldown_seconds as u64, + max_retries: u32::try_from(max_retries) + .map_err(|_| RookError::Registry(format!("max_retries out of range: {max_retries}")))?, + cooldown_seconds: u64::try_from(cooldown_seconds).map_err(|_| { + RookError::Registry(format!("cooldown_seconds out of range: {cooldown_seconds}")) + })?, },This pattern is already used elsewhere in
db/(e.g.,idempotency.rs:38,account.rs:84–86) and aligns with the existing error handling convention in this function.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/db/settings.rs` around lines 62 - 68, Replace the unchecked casts in row_to_settings() that convert the retrieved i64 values into unsigned types—specifically gateway_port, max_retries, and cooldown_seconds used to populate RookSettings and its default_routing_policy (RoutingPolicy)—with checked conversions (e.g., u16::try_from / u32::try_from / u64::try_from) and map any conversion failures to the function's existing error return (consistent with other db modules like idempotency.rs/account.rs). Ensure each failed try_from returns a clear DB error (include the field name and offending value) so out-of-range or negative persisted values are rejected instead of silently wrapping.clients/rook/src/db/route.rs (1)
174-225:⚠️ Potential issue | 🟡 MinorDoc comment now lies; also TOCTOU between the three queries.
Two issues in the rewritten
delete_route:
- The doc comment still claims the function only returns
true/false, but it now also returnsErr(RookError::Registry(...))when the route is referenced. Update the contract so callers know to expect the 409-style conflict branch.- The guarded
DELETE ... AND NOT EXISTS (...), the existenceSELECT, and the referencingSELECTrun as three independent statements on a pooled connection. If a concurrent caller removes the referencing route between steps 1 and 3, you fall through to the finalOk(false)on line 224 even though the route still exists and could have been deleted. It's a narrow race, but the admin API will then lie to the operator ("not found") for a row that's still in the DB.Consider wrapping the three statements in a single transaction (or re-attempting the guarded delete once when the reference check comes back empty) and refreshing the doc.
📝 Proposed doc tweak
- /// Delete a [`ModelRoute`] by ID. - /// - /// Returns `true` if a row was deleted, `false` if not found. + /// Delete a [`ModelRoute`] by ID. + /// + /// Returns `Ok(true)` if the row was deleted, `Ok(false)` if it did not exist, + /// or `Err(RookError::Registry(_))` if another route still references it as + /// `fallback_route_id`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/db/route.rs` around lines 174 - 225, The doc comment for delete_route is stale and the function has a TOCTOU race across three separate queries; update the doc to state it can return Ok(true) if deleted, Ok(false) if not found, or Err(RookError::Registry(...)) for a conflict when referenced, and fix the race by executing the guarded DELETE, existence check, and reference check inside a single database transaction (use a transaction obtained from the pool and run query/execute on that transaction) so the checks and delete are atomic; alternatively you may re-attempt the guarded DELETE once after the reference check returns empty, but prefer the transaction approach and ensure you commit/rollback the transaction and preserve the same error messages from delete_route.clients/rook/src/services/account.rs (1)
143-147:⚠️ Potential issue | 🔴 Critical
updateas delete+insert without atomicity is a data loss footgun.The schema uses
ON DELETE CASCADEforpool_members.account_id, so deleting a provider account cascades to remove every pool membership that references it. The current implementation executesdelete_account+insert_accountas two separate, non-atomic statements—a crash or error between them leaves the account gone entirely.Concretely: updating an account's
display_name/weight/etc. silently deletes all pool memberships that reference it on successful completion. A network hiccup or transient database error mid-operation loses the account entirely.
SqlitePoolService::updatedemonstrates the correct pattern: wrap the delete+insert insqlx::Transactionand commit atomically (seedb/pool.rslines 147–226). Apply the same approach here, or implement a realUPDATE provider_accounts SET ...statement in the db layer (preservingcreated_at, updatingupdated_at). Also noteSqliteRouteService::updatehas the identical issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/services/account.rs` around lines 143 - 147, The current async fn update in account.rs performs self.db.delete_account(...).await followed by self.db.insert_account(...).await without atomicity, which can cascade-delete pool_members on failure; wrap the delete+insert inside a sqlx::Transaction (begin, perform delete and insert via the transaction, then commit) following the pattern used by SqlitePoolService::update so the operation is atomic, or alternatively implement a proper UPDATE statement in the db layer that preserves created_at and updates updated_at; also apply the same transactional fix to SqliteRouteService::update which has the identical delete+insert issue.clients/rook/src/db/pool.rs (1)
147-229:⚠️ Potential issue | 🟠 MajorThe DELETE+INSERT pattern will fail if any model_route references this pool.
When deleting and re-inserting a row in
provider_pools, themodel_routestable's foreign key constraint(target_pool_id) REFERENCES provider_pools(id) ON DELETE RESTRICTprevents deletion. If any route targets this pool, the DELETE fails and the transaction rolls back. Thepool_memberstable avoids this via itsON DELETE CASCADE, but the code still repopulates them unnecessarily.Replace with
UPDATE provider_pools SET name=?, strategy=?, fallback_pool_id=?, updated_at=? WHERE id=?, then diff members (DELETE missing ones, INSERT new ones). This keeps the row ID stable, avoids FK triggers, and prevents thecreated_atmutation problem.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/rook/src/db/pool.rs` around lines 147 - 229, In replace_pool, the current DELETE+INSERT of provider_pools breaks foreign keys (model_routes ON DELETE RESTRICT) and mutates created_at; instead update the existing row: perform an UPDATE provider_pools SET name=?, strategy=?, fallback_pool_id=?, updated_at=? WHERE id=? (use strategy_to_db_str(&pool.strategy) and now for updated_at) rather than deleting and reinserting, preserving created_at and the same id; then diff pool.members against existing pool_members (select existing account_id for pool_id) and issue DELETEs for removed members and INSERT OR IGNORE for new members (binding pool_id strings), keeping the fallback existence check and transaction/commit logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7565c78-d181-4e22-bb70-7c23370b5495
⛔ Files ignored due to path filters (3)
.opencode/package-lock.jsonis excluded by!**/package-lock.jsonclients/rook/Cargo.lockis excluded by!**/*.lockclients/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (146)
.github/rulesets/README.md.github/rulesets/develop-protection.json.github/rulesets/minor-protection.jsonclients/agent-runtime/src/agent/coordinator.rsclients/agent-runtime/src/agent/mailbox.rsclients/agent-runtime/src/bridge/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/lib.rsclients/agent-runtime/src/memory/dream.rsclients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/agent-runtime/src/tools/delegate_launch.rsclients/rook/Cargo.tomlclients/rook/assets/assets/index-BGLvRAEp.cssclients/rook/assets/assets/index-BOcxyaY5.jsclients/rook/assets/assets/index-Bzqbvx-K.jsclients/rook/assets/assets/index-Cjl9wA4y.jsclients/rook/assets/assets/index-Dq_oV3yI.jsclients/rook/assets/assets/index.jsclients/rook/assets/index.htmlclients/rook/migrations/0004_chat_completions_idempotency.sqlclients/rook/src/admin/handlers.rsclients/rook/src/admin/mod.rsclients/rook/src/admin/types.rsclients/rook/src/auth/bearer.rsclients/rook/src/auth/middleware.rsclients/rook/src/auth/mod.rsclients/rook/src/auth/types.rsclients/rook/src/config/mod.rsclients/rook/src/dashboard/mod.rsclients/rook/src/db/account.rsclients/rook/src/db/idempotency.rsclients/rook/src/db/mod.rsclients/rook/src/db/pool.rsclients/rook/src/db/route.rsclients/rook/src/db/settings.rsclients/rook/src/domain/mod.rsclients/rook/src/gateway/handlers.rsclients/rook/src/gateway/mod.rsclients/rook/src/gateway/streaming.rsclients/rook/src/gateway/types.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/vendor.rsclients/rook/src/idempotency/canonical.rsclients/rook/src/idempotency/middleware.rsclients/rook/src/idempotency/mod.rsclients/rook/src/idempotency/types.rsclients/rook/src/lib.rsclients/rook/src/main.rsclients/rook/src/registry/mod.rsclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rsclients/rook/src/services/account.rsclients/rook/src/services/health.rsclients/rook/src/services/idempotency.rsclients/rook/src/services/mod.rsclients/rook/src/services/pool.rsclients/rook/src/services/route.rsclients/rook/src/services/settings.rsclients/rook/src/transport/context.rsclients/rook/src/transport/forwarded.rsclients/rook/src/transport/middleware.rsclients/rook/src/transport/mod.rsclients/rook/src/transport/rate_limit.rsclients/rook/src/transport/request_id.rsclients/web/apps/rook-dashboard/README.mdclients/web/apps/rook-dashboard/e2e/rook-dashboard.spec.tsclients/web/apps/rook-dashboard/index.htmlclients/web/apps/rook-dashboard/package.jsonclients/web/apps/rook-dashboard/playwright.config.tsclients/web/apps/rook-dashboard/src/App.vueclients/web/apps/rook-dashboard/src/features/accounts/AccountsPage.spec.tsclients/web/apps/rook-dashboard/src/features/accounts/AccountsPage.vueclients/web/apps/rook-dashboard/src/features/accounts/useAccounts.spec.tsclients/web/apps/rook-dashboard/src/features/accounts/useAccounts.tsclients/web/apps/rook-dashboard/src/features/overview/OverviewPage.spec.tsclients/web/apps/rook-dashboard/src/features/overview/OverviewPage.vueclients/web/apps/rook-dashboard/src/features/overview/useOverview.spec.tsclients/web/apps/rook-dashboard/src/features/overview/useOverview.tsclients/web/apps/rook-dashboard/src/lib/api/client.tsclients/web/apps/rook-dashboard/src/lib/api/types.tsclients/web/apps/rook-dashboard/src/lib/navigation/routes.spec.tsclients/web/apps/rook-dashboard/src/lib/navigation/routes.tsclients/web/apps/rook-dashboard/src/lib/session/useRookSession.tsclients/web/apps/rook-dashboard/src/main.tsclients/web/apps/rook-dashboard/src/style.cssclients/web/apps/rook-dashboard/src/vite-env.d.tsclients/web/apps/rook-dashboard/tsconfig.app.jsonclients/web/apps/rook-dashboard/tsconfig.jsonclients/web/apps/rook-dashboard/tsconfig.node.jsonclients/web/apps/rook-dashboard/vite.config.tsclients/web/package.jsonopenspec/changes/archive/2026-04-21-rook-589-gateway-api/state.yamlopenspec/changes/archive/2026-04-21-rook-590-admin-api/design.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/proposal.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/spec.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/state.yamlopenspec/changes/archive/2026-04-21-rook-590-admin-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/verify-report.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/design.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/proposal.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/specs/multi-agent-orchestration/spec.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/state.yamlopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/design.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/state.yamlopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/design.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/state.yamlopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-global-surface-rate-limits/design.mdopenspec/changes/archive/2026-04-22-rook-591-global-surface-rate-limits/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-global-surface-rate-limits/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-rook-591-global-surface-rate-limits/state.yamlopenspec/changes/archive/2026-04-22-rook-591-global-surface-rate-limits/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-global-surface-rate-limits/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-inbound-auth-boundary/design.mdopenspec/changes/archive/2026-04-22-rook-591-inbound-auth-boundary/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-inbound-auth-boundary/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-rook-591-inbound-auth-boundary/state.yamlopenspec/changes/archive/2026-04-22-rook-591-inbound-auth-boundary/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-inbound-auth-boundary/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-transport-middleware-baseline/design.mdopenspec/changes/archive/2026-04-22-rook-591-transport-middleware-baseline/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-transport-middleware-baseline/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-rook-591-transport-middleware-baseline/state.yamlopenspec/changes/archive/2026-04-22-rook-591-transport-middleware-baseline/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-transport-middleware-baseline/verify-report.mdopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/design.mdopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/proposal.mdopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/specs/dashboard/spec.mdopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/state.yamlopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/tasks.mdopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/verify-report.mdopenspec/specs/dashboard/spec.mdopenspec/specs/gateway/spec.mdopenspec/specs/multi-agent-orchestration/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cloudflare Pages
- GitHub Check: sonar
- GitHub Check: pr-checks
🧰 Additional context used
📓 Path-based instructions (9)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/rook-dashboard/src/vite-env.d.tsclients/rook/src/auth/mod.rsopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/state.yamlclients/agent-runtime/src/lib.rsclients/rook/assets/assets/index-BGLvRAEp.cssopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdclients/rook/src/services/mod.rsclients/web/apps/rook-dashboard/tsconfig.jsonclients/web/apps/rook-dashboard/src/lib/navigation/routes.spec.tsclients/web/apps/rook-dashboard/src/main.tsclients/web/apps/rook-dashboard/tsconfig.node.jsonclients/web/apps/rook-dashboard/index.htmlclients/rook/src/lib.rsclients/rook/assets/assets/index.jsopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/state.yamlclients/rook/assets/index.htmlopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/tasks.mdclients/agent-runtime/src/agent/mailbox.rsopenspec/changes/archive/2026-04-21-rook-589-gateway-api/state.yamlclients/web/apps/rook-dashboard/README.mdclients/web/apps/rook-dashboard/src/lib/session/useRookSession.tsclients/rook/src/dashboard/mod.rsclients/agent-runtime/src/tools/delegate_cancel.rsclients/web/apps/rook-dashboard/src/features/accounts/useAccounts.spec.tsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/web/apps/rook-dashboard/src/features/overview/useOverview.spec.tsopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/proposal.mdclients/web/apps/rook-dashboard/playwright.config.tsopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/design.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/proposal.mdclients/rook/src/transport/mod.rsclients/rook/src/services/health.rsclients/agent-runtime/src/gateway/mod.rsopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/design.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/proposal.mdclients/web/apps/rook-dashboard/tsconfig.app.jsonclients/rook/assets/assets/index-Bzqbvx-K.jsclients/rook/src/auth/types.rsopenspec/changes/archive/2026-04-21-rook-590-admin-api/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/proposal.mdclients/web/apps/rook-dashboard/src/App.vueclients/rook/assets/assets/index-BOcxyaY5.jsclients/rook/src/idempotency/mod.rsclients/rook/src/auth/bearer.rsopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/verify-report.mdclients/web/apps/rook-dashboard/vite.config.tsclients/rook/src/db/settings.rsopenspec/changes/archive/2026-04-21-rook-590-admin-api/state.yamlclients/web/apps/rook-dashboard/src/features/accounts/AccountsPage.vueclients/web/apps/rook-dashboard/src/features/overview/OverviewPage.vueclients/rook/src/gateway/vendor.rsclients/agent-runtime/src/memory/mod.rsclients/rook/src/transport/middleware.rsopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/design.mdclients/web/apps/rook-dashboard/src/lib/api/client.tsclients/web/apps/rook-dashboard/package.jsonclients/rook/src/domain/mod.rsclients/web/apps/rook-dashboard/src/lib/api/types.tsclients/agent-runtime/src/tools/delegate_launch.rsclients/web/apps/rook-dashboard/e2e/rook-dashboard.spec.tsclients/agent-runtime/src/bridge/mod.rsopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/verify-report.mdclients/web/apps/rook-dashboard/src/lib/navigation/routes.tsclients/rook/src/services/account.rsclients/rook/src/admin/mod.rsclients/rook/src/auth/middleware.rsclients/web/apps/rook-dashboard/src/features/overview/OverviewPage.spec.tsopenspec/changes/archive/2026-04-21-rook-590-admin-api/spec.mdclients/rook/assets/assets/index-Cjl9wA4y.jsclients/rook/src/services/settings.rsclients/rook/src/gateway/mod.rsclients/rook/src/transport/request_id.rsclients/rook/migrations/0004_chat_completions_idempotency.sqlopenspec/changes/archive/2026-04-21-rook-590-admin-api/tasks.mdclients/rook/src/idempotency/canonical.rsclients/rook/src/db/account.rsclients/web/apps/rook-dashboard/src/features/accounts/AccountsPage.spec.tsclients/rook/Cargo.tomlclients/rook/src/idempotency/types.rsclients/web/package.jsonclients/web/apps/rook-dashboard/src/features/accounts/useAccounts.tsclients/rook/src/main.rsclients/rook/src/registry/mod.rsopenspec/changes/archive/2026-04-21-rook-590-admin-api/design.mdclients/rook/src/transport/context.rsclients/web/apps/rook-dashboard/src/features/overview/useOverview.tsclients/rook/assets/assets/index-Dq_oV3yI.jsopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/specs/gateway/spec.mdclients/rook/src/db/idempotency.rsclients/rook/src/services/route.rsclients/rook/src/db/route.rsclients/rook/src/idempotency/middleware.rsclients/rook/src/transport/forwarded.rsopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/specs/multi-agent-orchestration/spec.mdclients/rook/src/gateway/types.rsclients/rook/src/gateway/streaming.rsclients/rook/src/db/pool.rsclients/rook/src/admin/types.rsclients/rook/src/transport/rate_limit.rsclients/rook/src/services/pool.rsclients/rook/src/services/idempotency.rsclients/rook/src/db/mod.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/handlers.rsclients/agent-runtime/src/memory/dream.rsclients/web/apps/rook-dashboard/src/style.cssclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rsclients/rook/src/config/mod.rsclients/rook/src/admin/handlers.rsclients/agent-runtime/src/agent/coordinator.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/rook/src/auth/mod.rsclients/agent-runtime/src/lib.rsclients/rook/src/services/mod.rsclients/rook/src/lib.rsclients/agent-runtime/src/agent/mailbox.rsclients/rook/src/dashboard/mod.rsclients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/rook/src/transport/mod.rsclients/rook/src/services/health.rsclients/agent-runtime/src/gateway/mod.rsclients/rook/src/auth/types.rsclients/rook/src/idempotency/mod.rsclients/rook/src/auth/bearer.rsclients/rook/src/db/settings.rsclients/rook/src/gateway/vendor.rsclients/agent-runtime/src/memory/mod.rsclients/rook/src/transport/middleware.rsclients/rook/src/domain/mod.rsclients/agent-runtime/src/tools/delegate_launch.rsclients/agent-runtime/src/bridge/mod.rsclients/rook/src/services/account.rsclients/rook/src/admin/mod.rsclients/rook/src/auth/middleware.rsclients/rook/src/services/settings.rsclients/rook/src/gateway/mod.rsclients/rook/src/transport/request_id.rsclients/rook/src/idempotency/canonical.rsclients/rook/src/db/account.rsclients/rook/src/idempotency/types.rsclients/rook/src/main.rsclients/rook/src/registry/mod.rsclients/rook/src/transport/context.rsclients/rook/src/db/idempotency.rsclients/rook/src/services/route.rsclients/rook/src/db/route.rsclients/rook/src/idempotency/middleware.rsclients/rook/src/transport/forwarded.rsclients/rook/src/gateway/types.rsclients/rook/src/gateway/streaming.rsclients/rook/src/db/pool.rsclients/rook/src/admin/types.rsclients/rook/src/transport/rate_limit.rsclients/rook/src/services/pool.rsclients/rook/src/services/idempotency.rsclients/rook/src/db/mod.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/handlers.rsclients/agent-runtime/src/memory/dream.rsclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rsclients/rook/src/config/mod.rsclients/rook/src/admin/handlers.rsclients/agent-runtime/src/agent/coordinator.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/lib.rsclients/agent-runtime/src/agent/mailbox.rsclients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/tools/delegate_launch.rsclients/agent-runtime/src/bridge/mod.rsclients/agent-runtime/src/memory/dream.rsclients/agent-runtime/src/agent/coordinator.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/lib.rsclients/agent-runtime/src/agent/mailbox.rsclients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/tools/delegate_launch.rsclients/agent-runtime/src/bridge/mod.rsclients/agent-runtime/src/memory/dream.rsclients/agent-runtime/src/agent/coordinator.rs
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/tasks.mdclients/web/apps/rook-dashboard/README.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/design.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/proposal.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/design.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/proposal.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/verify-report.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/proposal.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/verify-report.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/design.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/verify-report.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/spec.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/tasks.mdopenspec/changes/archive/2026-04-21-rook-590-admin-api/design.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/specs/multi-agent-orchestration/spec.md
clients/agent-runtime/src/tools/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Tooltrait insrc/tools/with strict parameter schema, validate and sanitize all inputs, and return structuredToolResultwithout panics in runtime path
Files:
clients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/agent-runtime/src/tools/delegate_launch.rs
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Files:
clients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/tools/delegate_launch.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/agent-runtime/src/tools/delegate_inspect.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/tools/delegate_launch.rs
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/rook-dashboard/src/App.vueclients/web/apps/rook-dashboard/src/features/accounts/AccountsPage.vueclients/web/apps/rook-dashboard/src/features/overview/OverviewPage.vue
🧠 Learnings (13)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/rook/src/auth/mod.rsclients/agent-runtime/src/lib.rsclients/rook/src/auth/middleware.rsclients/rook/src/transport/context.rsclients/rook/src/server/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/rook/src/auth/mod.rsclients/agent-runtime/src/lib.rsclients/rook/src/lib.rsclients/rook/src/gateway/vendor.rsclients/rook/src/auth/middleware.rsclients/rook/src/main.rsclients/rook/src/transport/context.rsclients/rook/src/gateway/types.rsclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rsclients/rook/src/config/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/rook/src/auth/mod.rsclients/agent-runtime/src/lib.rsclients/rook/src/lib.rsclients/rook/src/transport/mod.rsclients/rook/src/domain/mod.rsclients/agent-runtime/src/bridge/mod.rsclients/rook/src/registry/mod.rsclients/rook/src/routing/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
clients/agent-runtime/src/lib.rsclients/rook/src/lib.rsclients/rook/Cargo.toml
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/src/lib.rsclients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate.rsclients/rook/src/services/health.rsclients/agent-runtime/src/tools/delegate_launch.rsclients/rook/src/main.rsclients/rook/src/db/route.rsclients/rook/src/gateway/types.rsclients/rook/src/routing/mod.rsclients/agent-runtime/src/agent/coordinator.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/src/lib.rsopenspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdclients/rook/src/idempotency/mod.rsclients/rook/src/gateway/vendor.rsclients/rook/src/domain/mod.rsclients/rook/src/main.rsclients/agent-runtime/src/memory/dream.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdopenspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/tasks.mdclients/rook/src/transport/mod.rsclients/agent-runtime/src/bridge/mod.rsclients/rook/src/gateway/mod.rsclients/rook/src/main.rsclients/rook/src/gateway/upstream.rsclients/rook/src/gateway/handlers.rsclients/rook/src/routing/mod.rsclients/rook/src/server/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes
Applied to files:
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
clients/agent-runtime/src/tools/delegate_cancel.rsclients/agent-runtime/src/tools/delegate_launch.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
clients/rook/Cargo.toml
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/rook/src/main.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Follow `.github/pull_request_template.md` and keep PR descriptions concrete with problem, change, non-goals, risk, and rollback information
Applied to files:
.github/rulesets/README.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/rook/src/routing/mod.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/tasks.md
[grammar] ~23-~23: Ensure spelling is correct
Context: ...he chat-completions route. ## Phase 4: Config and Apply Readiness - [x] 4.1 Update `...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
openspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/proposal.md
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e SSE for streaming chat completions. - Define the minimum SSE event framing, media ty...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...eaming behavior for GET /v1/models. - Any streaming behavior for routes outside `...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/proposal.md
[grammar] ~26-~26: Ensure spelling is correct
Context: ...le tools so launch, inspect, and cancel form one explicit, parent-owned runtime co...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
openspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/design.md
[style] ~59-~59: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...raw incoming bytes as-is - Canonicalize only known OpenAI fields and ignore passthro...
(ADVERB_REPETITION_PREMIUM)
openspec/changes/archive/2026-04-21-rook-590-admin-api/proposal.md
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., get, update, and delete operations. - Expose pool membership endpoints for adding an...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~29-~29: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...emoving provider accounts from pools. - Expose route management endpoints for create, ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..., get, update, and delete operations. - Expose health read endpoints for account-level...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... records and aggregate summary views. - Expose settings read and update endpoints. - A...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/proposal.md
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...etions create attempt for this slice. - Define replay behavior for equivalent repeated...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...otency behavior for GET /v1/models. - Any idempotency behavior for routes outside...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/design.md
[style] ~6-~6: Consider using “the surrounding contract”.
Context: ...ed orchestration authority and finishes the contract around it instead of introducing a second orchest...
(NOUN_AROUND_IT)
[style] ~226-~226: This wording could be more concise.
Context: ...ify | Keep shared service/runner wiring unchanged in shape, but ensure the same contract types are...
(ADJECTIVE_IN_ATTRIBUTE)
openspec/changes/archive/2026-04-21-rook-590-admin-api/spec.md
[style] ~32-~32: Consider removing “of” to be more concise
Context: ...ok. The composed server MUST preserve all of the following at the same time: - GET /ap...
(ALL_OF_THE)
openspec/changes/archive/2026-04-21-rook-590-admin-api/design.md
[style] ~245-~245: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...| clients/rook/src/services/pool.rs | Maybe modify | Only if needed to tighten not-...
(REP_MAYBE)
[style] ~246-~246: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... clients/rook/src/services/route.rs | Maybe modify | Only if needed to preserve sta...
(REP_MAYBE)
[style] ~246-~246: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ices/route.rs` | Maybe modify | Only if needed to preserve stable not-found/conflict mapp...
(REP_NEED_TO_VB)
openspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/specs/gateway/spec.md
[style] ~81-~81: Consider removing “of” to be more concise
Context: ...system MUST determine equivalence using all of the following together: authenticated princ...
(ALL_OF_THE)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/proposal.md
[warning] 24-24: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
openspec/changes/archive/2026-04-22-rook-591-chat-completions-idempotency/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 53-53: Ordered list item prefix
Expected: 1; Actual: 13; Style: 1/1/1
(MD029, ol-prefix)
🪛 Stylelint (17.7.0)
clients/rook/assets/assets/index-BGLvRAEp.css
[error] 1-1: Expected quotes around "Segoe UI" (font-family-name-quotes)
(font-family-name-quotes)
clients/web/apps/rook-dashboard/src/style.css
[error] 1-1: Expected "url("@corvus/shared/base.css")" to be ""@corvus/shared/base.css"" (import-notation)
(import-notation)
Comment on lines
+225
to
+280
| fn normalize_execution_metadata( | ||
| spec: Option<&ChildExecutionSpec>, | ||
| ) -> Result<Option<ChildExecutionMetadataView>, LaunchContractRejection> { | ||
| let Some(spec) = spec else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| let requested = NormalizedExecutionRequest::from(spec); | ||
|
|
||
| if requested.transport == CoordinatorTransport::RemoteBridge { | ||
| return Err(LaunchContractRejection::UnsupportedTransport { | ||
| requested: CoordinatorTransport::RemoteBridge, | ||
| }); | ||
| } | ||
|
|
||
| if let Some(repository_id) = &requested.repository_id { | ||
| return Err(LaunchContractRejection::UnsupportedIsolation { | ||
| field: "repository_id".to_string(), | ||
| requested: repository_id.clone(), | ||
| }); | ||
| } | ||
|
|
||
| if let Some(worktree_id) = &requested.worktree_id { | ||
| return Err(LaunchContractRejection::UnsupportedIsolation { | ||
| field: "worktree_id".to_string(), | ||
| requested: worktree_id.clone(), | ||
| }); | ||
| } | ||
|
|
||
| if let Some(sandbox_mode) = &requested.sandbox_mode { | ||
| let normalized = sandbox_mode.trim().to_ascii_lowercase(); | ||
| if matches!( | ||
| normalized.as_str(), | ||
| "clone" | "cloned" | "isolated" | "workspace_clone" | ||
| ) { | ||
| return Err(LaunchContractRejection::UnsupportedIsolation { | ||
| field: "sandbox_mode".to_string(), | ||
| requested: sandbox_mode.clone(), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if let Some(permission_broker) = &requested.permission_broker { | ||
| let normalized = permission_broker.trim().to_ascii_lowercase(); | ||
| if normalized != "parent_owned_only" { | ||
| return Err(LaunchContractRejection::UnsupportedPermissionBroker { | ||
| reason: permission_broker.clone(), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| Ok(Some(ChildExecutionMetadataView { | ||
| enforced: enforced_execution_guarantees(&requested), | ||
| requested, | ||
| })) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "coordinator.rs" | grep -E "agent-runtime"Repository: dallay/corvus
Length of output: 105
🏁 Script executed:
rg "fn normalize_execution_metadata" --type rs -A 70Repository: dallay/corvus
Length of output: 83
🏁 Script executed:
rg "struct ChildExecutionSpec" --type rs -B 2 -A 15Repository: dallay/corvus
Length of output: 83
🏁 Script executed:
rg "fn build_effective_config" --type rs -A 20Repository: dallay/corvus
Length of output: 83
🏁 Script executed:
rg "delegate_launch" --type rs -B 3 -A 15 | head -100Repository: dallay/corvus
Length of output: 83
🏁 Script executed:
cat -n clients/agent-runtime/src/agent/coordinator.rs | sed -n '225,285p'Repository: dallay/corvus
Length of output: 2524
🏁 Script executed:
cat -n clients/agent-runtime/src/agent/coordinator.rs | sed -n '550,580p'Repository: dallay/corvus
Length of output: 1537
🏁 Script executed:
rg "struct ChildExecutionSpec" --lang rust -B 2 -A 20Repository: dallay/corvus
Length of output: 465
🏁 Script executed:
rg "fn build_effective_config" --lang rust -A 25Repository: dallay/corvus
Length of output: 465
🏁 Script executed:
rg "delegate_launch" --lang rust -B 3 -A 20Repository: dallay/corvus
Length of output: 465
🏁 Script executed:
rg "struct ChildExecutionSpec" -B 2 -A 20 clients/agent-runtime/src/Repository: dallay/corvus
Length of output: 1864
🏁 Script executed:
rg "delegate_launch" -B 5 -A 25 clients/agent-runtime/src/Repository: dallay/corvus
Length of output: 29824
🏁 Script executed:
rg "working_directory" clients/agent-runtime/src/ | head -30Repository: dallay/corvus
Length of output: 1074
🏁 Script executed:
cat -n clients/agent-runtime/src/tools/delegate_launch.rs | head -150Repository: dallay/corvus
Length of output: 7295
🏁 Script executed:
rg "ChildExecutionSpec" clients/agent-runtime/src/tools/delegate_launch.rs -B 3 -A 10Repository: dallay/corvus
Length of output: 650
🏁 Script executed:
rg "working_directory" clients/agent-runtime/src/tools/delegate_launch.rs -B 3 -A 3Repository: dallay/corvus
Length of output: 907
🏁 Script executed:
rg "workspace_dir" clients/agent-runtime/src/ -B 2 -A 2 | head -80Repository: dallay/corvus
Length of output: 5609
🏁 Script executed:
cat -n clients/agent-runtime/src/agent/coordinator.rs | sed -n '170,220p'Repository: dallay/corvus
Length of output: 2429
🏁 Script executed:
rg "workspace_dir" clients/agent-runtime/src/agent/coordinator.rs -B 3 -A 3Repository: dallay/corvus
Length of output: 324
🏁 Script executed:
rg "NormalizedExecutionRequest" clients/agent-runtime/src/agent/coordinator.rs -B 5 -A 20 | head -100Repository: dallay/corvus
Length of output: 3718
🏁 Script executed:
rg "canonicalize|absolute|normalize_path" clients/agent-runtime/src/agent/coordinator.rs -B 2 -A 2Repository: dallay/corvus
Length of output: 39
🏁 Script executed:
rg "workspace_dir.*from|PathBuf::from" clients/agent-runtime/src/ -B 2 -A 2 | head -50Repository: dallay/corvus
Length of output: 3929
Missing validation on working_directory allows arbitrary filesystem scope bypass.
normalize_execution_metadata rejects repository_id, worktree_id, and clone-ish sandbox_mode values, but allows any working_directory through without normalization or validation. The field is deserialized directly from untrusted delegate_launch input (src/tools/, a high-risk surface) and written straight into config.workspace_dir at line 565 via std::path::PathBuf::from() with no canonicalization or bounds checking. A child request carrying "working_directory": "/etc" or "../.." will silently reparent the delegated session's filesystem scope.
Either add canonicalization + allow-list validation in normalize_execution_metadata (consistent with how you handle other isolation fields), or reject working_directory entirely if it is intended as parent-orchestration-only and should never flow from LLM input.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/agent/coordinator.rs` around lines 225 - 280,
normalize_execution_metadata currently accepts an unvalidated working_directory
from ChildExecutionSpec which later becomes config.workspace_dir; either
canonicalize and validate it here or reject it outright: update
normalize_execution_metadata (working with ChildExecutionSpec,
ChildExecutionMetadataView, and enforced_execution_guarantees) to (a) if you
intend to allow a working_directory, canonicalize the path (resolve .. and
symlinks), enforce it is within an allow-list or sandbox root (reject absolute
paths like "/etc" and paths that escape the allowed workspace), and return an
UnsupportedIsolation/validation rejection on failure, or (b) simpler: refuse any
incoming working_directory by returning
LaunchContractRejection::UnsupportedIsolation for the "working_directory" field
so no untrusted path flows into config.workspace_dir. Ensure checks run before
building ChildExecutionMetadataView and reference the working_directory field on
the NormalizedExecutionRequest/ChildExecutionSpec.
Comment on lines
+1876
to
1891
| let events = coordinator | ||
| .event_log() | ||
| .map_err(OrchestrationServiceError::CoordinatorError)? | ||
| .iter() | ||
| .map(LifecycleEventView::from) | ||
| .collect(); | ||
|
|
||
| Ok(OrchestrationSnapshot { | ||
| handle: handle.clone(), | ||
| parent_session_id: request.parent_session_id.clone(), | ||
| state, | ||
| children, | ||
| events, | ||
| outcome, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
event_log() clones the full Vec<CoordinatorEvent> on every inspect.
For runs with many lifecycle transitions (or frequent polling of inspect), this is O(events) clone per snapshot plus an internal allocation for LifecycleEventView. Fine at current scale; worth a since_sequence filter or bounded ring buffer before this becomes a hot path on the dashboard.
As per coding guidelines: "Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/agent/coordinator.rs` around lines 1876 - 1891, The
current code clones the entire Vec<CoordinatorEvent> via coordinator.event_log()
on every inspect; change the handling to avoid cloning and to filter events by
sequence before building views: call coordinator.event_log().map_err(...)? to
get a reference to the events, then use .iter().filter(|e| e.sequence >
request.since_sequence.unwrap_or(0)) (or an equivalent since_sequence field) and
map LifecycleEventView::from over the filtered iterator, collecting only the
needed subset (optionally cap the number returned). Update the mapping code
around LifecycleEventView::from and the events binding so it operates on
&CoordinatorEvent (no full Vec clone) and respects a since_sequence/bounded
limit to prevent O(n) cloning on every inspect.
Comment on lines
+41
to
+47
| pub fn to_sse_frame(&self) -> anyhow::Result<String> { | ||
| Ok(format!( | ||
| "event: {}\nid: {}\ndata: {}\n\n", | ||
| self.kind, | ||
| self.sequence, | ||
| serde_json::to_string(self)? | ||
| )) |
Contributor
There was a problem hiding this comment.
Validate kind before writing it into the SSE frame.
self.kind is inserted into the event: header verbatim. A value containing \n/\r can inject additional SSE fields or split frames. Reject control characters or restrict event names to a safe allowlist before formatting.
🛡️ Proposed fix
impl BridgeEnvelope {
+ fn validate_sse_event_name(name: &str) -> anyhow::Result<()> {
+ let valid = !name.is_empty()
+ && name
+ .bytes()
+ .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-' | b'.'));
+ anyhow::ensure!(valid, "invalid SSE event name");
+ Ok(())
+ }
+
pub fn to_sse_frame(&self) -> anyhow::Result<String> {
+ Self::validate_sse_event_name(&self.kind)?;
+ let data = serde_json::to_string(self)?;
Ok(format!(
"event: {}\nid: {}\ndata: {}\n\n",
self.kind,
self.sequence,
- serde_json::to_string(self)?
+ data
))
}
}As per coding guidelines, **/*: Security first, performance second.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/bridge/mod.rs` around lines 41 - 47, The
to_sse_frame method currently interpolates self.kind directly into the "event:"
header allowing newline/control injection; update to_sse_frame to validate
self.kind before formatting by rejecting any characters like '\n' or '\r' or
other control chars (or enforce a safe allowlist such as /^[A-Za-z0-9_-]+$/) and
return an Err (anyhow::Error) when validation fails so only safe event names are
written into the SSE frame.
Comment on lines
+1685
to
+1710
| async fn finalize_generated_session_if_needed( | ||
| state: &AppState, | ||
| session_id: &str, | ||
| session_source: webhook_dispatch::WebhookSessionSource, | ||
| ) { | ||
| if !matches!( | ||
| session_source, | ||
| webhook_dispatch::WebhookSessionSource::Generated | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| if let Err(error) = state.mem.end_session(session_id).await { | ||
| tracing::debug!("generated session end best-effort failed: {error}"); | ||
| return; | ||
| } | ||
|
|
||
| let workspace_dir = state.config.lock().workspace_dir.clone(); | ||
| if let Err(error) = crate::memory::record_session_completion(&workspace_dir) { | ||
| tracing::debug!("dream session counter update failed: {error}"); | ||
| return; | ||
| } | ||
| if let Err(error) = crate::memory::run_dream_if_triggered(&workspace_dir) { | ||
| tracing::debug!("dream consolidation after generated session skipped: {error}"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites for finalize_generated_session_if_needed:"
rg -n -C3 '\bfinalize_generated_session_if_needed\s*\('
echo
echo "Gateway session lifecycle touchpoints:"
rg -n -C5 '\b(end_session|record_session_completion|run_dream_if_triggered|update_session_activity_if_persisted)\b' clients/agent-runtime/src/gateway/mod.rsRepository: dallay/corvus
Length of output: 4037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full-repo search for finalize_generated_session_if_needed ==="
rg 'finalize_generated_session_if_needed' --type rust
echo
echo "=== Check function visibility ==="
head -n 1690 clients/agent-runtime/src/gateway/mod.rs | tail -n 10
echo
echo "=== Verify if function would be flagged as dead code ==="
echo "Checking for 'async fn finalize_generated_session_if_needed' with no callers..."
rg -A 25 'async fn finalize_generated_session_if_needed' clients/agent-runtime/src/gateway/mod.rs | head -n 30Repository: dallay/corvus
Length of output: 1516
Wire this cleanup helper into the webhook completion path or remove it.
finalize_generated_session_if_needed is completely unused—the entire repository contains zero call sites. None of the /webhook completion paths invoke it, so generated sessions never finalize and the completion/dream hooks never execute. This function also fails cargo clippy --all-targets -- -D warnings as dead code.
Either integrate this into the webhook lifecycle (likely before the response in the completion handlers at lines ~2120–2210) or remove the function entirely. If removed, ensure the session cleanup logic belongs elsewhere or confirm generated sessions can safely remain open.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/gateway/mod.rs` around lines 1685 - 1710,
finalize_generated_session_if_needed is dead and never called; either invoke it
from the webhook completion flow or delete it. To wire it in: call
finalize_generated_session_if_needed(&state, &session_id, session_source).await
in each webhook completion handler (the handlers that finalize
responses/completions) before sending the HTTP response, ensuring you pass the
same session_id and webhook_dispatch::WebhookSessionSource used for that request
and await the future; handle errors only via internal logging (no panic). To
remove it: delete the finalize_generated_session_if_needed function and ensure
the session cleanup logic (end_session, record_session_completion,
run_dream_if_triggered) is moved into an existing completion path so generated
sessions are finalized and the clippy dead-code warning is resolved.
Comment on lines
+142
to
+143
| for file in &recent_signal_files { | ||
| let content = fs::read_to_string(file)?; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the dream.rs file
find . -name "dream.rs" -type fRepository: dallay/corvus
Length of output: 100
🏁 Script executed:
# Get the file size and line count
wc -l clients/agent-runtime/src/memory/dream.rsRepository: dallay/corvus
Length of output: 102
🏁 Script executed:
# Examine the recent_memory_files function around lines 142-143 and 297-304
sed -n '130,160p' clients/agent-runtime/src/memory/dream.rsRepository: dallay/corvus
Length of output: 1122
🏁 Script executed:
# Also check the 297-304 range
sed -n '290,315p' clients/agent-runtime/src/memory/dream.rsRepository: dallay/corvus
Length of output: 801
🏁 Script executed:
# Search for all fs::read functions in the file to understand the full context
rg 'fs::read' clients/agent-runtime/src/memory/dream.rs -n -A 2 -B 2Repository: dallay/corvus
Length of output: 1166
🏁 Script executed:
# Check if there are any symlink-related tests
rg 'symlink|hardlink' clients/agent-runtime/src/memory/dream.rs -nRepository: dallay/corvus
Length of output: 39
🏁 Script executed:
# Examine the full recent_memory_files function and context
sed -n '296,315p' clients/agent-runtime/src/memory/dream.rsRepository: dallay/corvus
Length of output: 704
🏁 Script executed:
# Check what get passed to fs::read_to_string - trace recent_signal_files origin
rg 'recent_signal_files' clients/agent-runtime/src/memory/dream.rs -n -B 3 -A 3Repository: dallay/corvus
Length of output: 933
🏁 Script executed:
# Look for any platform-specific or symlink handling code
rg 'symlink|canonical|realpath' clients/agent-runtime/src/memory/ -nRepository: dallay/corvus
Length of output: 279
🏁 Script executed:
# Check the entire test module to see if symlink handling is tested
sed -n '380,435p' clients/agent-runtime/src/memory/dream.rsRepository: dallay/corvus
Length of output: 1948
🏁 Script executed:
# Let's verify fs::read_to_string actually follows symlinks in Rust
# by checking the Rust standard library behavior through web search is more reliable
echo "Checking Rust std fs behavior - fs::read_to_string follows symlinks per Rust docs"Repository: dallay/corvus
Length of output: 138
Reject symlinked memory files before reading them.
recent_memory_files() enumerates .md files without filtering for regular files. Since fs::read_to_string() follows symlinks, a crafted memory/*.md symlink can read files outside memory/, which then get merged into MEMORY.md. This violates input boundary validation.
🛡️ Proposed hardening
fn recent_memory_files(memory_dir: &Path) -> Result<Vec<PathBuf>> {
if !memory_dir.is_dir() {
return Ok(Vec::new());
}
- let mut files: Vec<PathBuf> = fs::read_dir(memory_dir)?
- .filter_map(|entry| entry.ok().map(|value| value.path()))
- .filter(|path| path.extension().and_then(|value| value.to_str()) == Some("md"))
- .collect();
+ let mut files = Vec::new();
+ for entry in fs::read_dir(memory_dir)? {
+ let entry = entry?;
+ if !entry.file_type()?.is_file() {
+ continue;
+ }
+ let path = entry.path();
+ if path.extension().and_then(|value| value.to_str()) == Some("md") {
+ files.push(path);
+ }
+ }
files.sort();Add a regression test creating a symlinked memory/*.md entry and verify it is skipped during consolidation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/memory/dream.rs` around lines 142 - 143, The loop
that reads recent memory files (iterating recent_signal_files in
clients/agent-runtime/src/memory/dream.rs) currently calls
fs::read_to_string(file) without rejecting symlinks; fix by checking file
metadata before reading: use std::fs::symlink_metadata(file) and skip the entry
if metadata.file_type().is_symlink() (or otherwise ensure canonicalized path
stays inside the memory directory), then only call fs::read_to_string for
regular files (metadata.is_file()). Also add a regression test that creates a
symlinked memory/*.md and asserts that the symlink is skipped during
consolidation.
Comment on lines
+138
to
+170
| ### Requirement: Account responses MUST redact credentials | ||
|
|
||
| The system MUST accept `api_key` in create and update requests as a write-only field. | ||
|
|
||
| The system MUST NOT return raw `api_key` values in any admin response body. | ||
|
|
||
| Every account response MUST expose `has_api_key: boolean` instead of `api_key`. | ||
|
|
||
| This redaction rule MUST apply to list responses, get responses, create responses, update | ||
| responses, nested pool member views if any are ever returned, and any error payload metadata. | ||
|
|
||
| #### Scenario: account create response redacts api_key | ||
|
|
||
| - GIVEN a `CreateAccountRequest` with `api_key = "sk-secret"` | ||
| - WHEN the request succeeds | ||
| - THEN the response body MUST include `has_api_key: true` | ||
| - AND the response body MUST NOT include an `api_key` field | ||
| - AND the raw submitted credential MUST NOT be echoed back | ||
|
|
||
| #### Scenario: account update response redacts api_key | ||
|
|
||
| - GIVEN an existing account without a key | ||
| - WHEN a client submits `PUT /api/accounts/{account_id}` with a new `api_key` | ||
| - THEN the response body MUST include `has_api_key: true` | ||
| - AND the response body MUST NOT include an `api_key` field | ||
|
|
||
| #### Scenario: account list response remains redacted | ||
|
|
||
| - GIVEN one or more stored accounts with API keys | ||
| - WHEN a client requests `GET /api/accounts` | ||
| - THEN every item in the response MUST include `has_api_key` | ||
| - AND no item in the response MUST expose raw credential material | ||
|
|
Contributor
There was a problem hiding this comment.
Spec is silent on logging/telemetry redaction.
The redaction rule covers response bodies and error payload metadata, but not logs, traces, metrics labels, or request echo in validation errors. Recommend adding: api_key MUST NOT appear in server logs, tracing spans, metrics labels, or error details derived from the raw request body.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-04-21-rook-590-admin-api/spec.md` around lines
138 - 170, Add a clear logging/telemetry redaction rule to the "Requirement:
Account responses MUST redact credentials" section: state that the `api_key`
field (and any raw credential material) MUST NOT be written to server logs,
tracing spans, metrics labels, or included in error `details` or any
request-echo in validation errors; ensure this requirement is explicitly applied
alongside the existing requirements for `CreateAccountRequest`, `PUT
/api/accounts/{account_id}`, `GET /api/accounts` and list/get/create/update
responses so implementers know to strip or redact `api_key` from logs, traces,
metrics, and error metadata as well as response bodies.
Comment on lines
+537
to
+554
| ### Requirement: Loopback-first and no-auth M1 safety posture | ||
|
|
||
| This change MUST preserve the current M1 safety posture. | ||
|
|
||
| The admin API defined here MUST NOT expand exposure beyond the existing loopback/local-admin | ||
| assumption. | ||
|
|
||
| Authentication and authorization are explicitly out of scope for this spec and belong to #591. | ||
|
|
||
| The admin API contract MUST therefore be specified without bearer-token, pairing, or role-based | ||
| authorization requirements. | ||
|
|
||
| #### Scenario: admin API remains unauthenticated in M1 contract | ||
|
|
||
| - GIVEN the M1 admin API defined by this spec | ||
| - WHEN a client interacts with `/api/*` | ||
| - THEN the contract MUST NOT require auth features from #591 | ||
| - AND the spec MUST continue to describe this surface as local-admin only |
Contributor
There was a problem hiding this comment.
Make the loopback-only posture an enforced contract, not an assumption.
This section states the admin API "MUST NOT expand exposure beyond the existing loopback/local-admin assumption" and defers auth to #591, but nothing here requires the server to actually bind to loopback. An unauthenticated CRUD surface that can write api_key and delete accounts/pools/routes is dangerous if a misconfiguration (e.g. 0.0.0.0 bind, reverse proxy, container port publish) exposes it on a routable interface.
Consider adding a requirement such as:
- Admin routes MUST only be served on a loopback listener in M1, OR
- Binding to a non-loopback address MUST refuse to mount
/api/*admin routes (or MUST log a prominent warning and require an explicit opt-in flag).
Otherwise the "local-admin only" guarantee is documentation, not behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-04-21-rook-590-admin-api/spec.md` around lines
537 - 554, Make the loopback-only posture an enforced contract: update the spec
text around the "Loopback-first and no-auth M1 safety posture" and the
"Scenario: admin API remains unauthenticated in M1 contract" to require that
admin routes under "/api/*" are only served on a loopback listener in M1, and
specify behavior if the server binds to a non-loopback address (e.g., the server
MUST refuse to mount "/api/*" admin routes or MUST require an explicit
documented opt-in flag such as an "--admin-bind" override and log a prominent
warning). Reference the M1 admin API and "/api/*" admin routes when adding this
requirement and describe the required runtime checks (bind address validation or
opt-in gating) and the expected failure/warning behavior.
Comment on lines
+3
to
+26
| ## Phase 1: Contract and Validation Foundation | ||
|
|
||
| - [x] 1.1 RED: Add coordinator/service tests in `clients/agent-runtime/src/agent/coordinator.rs` for normalized execution metadata, `remote_bridge` rejection, unsupported isolation rejection, and unsupported permission-broker rejection. | ||
| - [x] 1.2 GREEN: In `clients/agent-runtime/src/agent/coordinator.rs`, add normalized request/enforced-guarantee types, launch rejection types, and centralized fail-closed admission validation used by supervised orchestration. | ||
| - [x] 1.3 REFACTOR: Tighten `clients/agent-runtime/src/bridge/mod.rs` to metadata-only seam types consumed by the orchestration contract, without active runner wiring. | ||
|
|
||
| ## Phase 2: Lifecycle Read Model and Mailbox Semantics | ||
|
|
||
| - [x] 2.1 RED: Extend coordinator/mailbox tests in `clients/agent-runtime/src/agent/coordinator.rs` and `clients/agent-runtime/src/agent/mailbox.rs` for `cancelling` state, immutable terminal states, deterministic event ordering, and redelivery dedupe. | ||
| - [x] 2.2 GREEN: Update `clients/agent-runtime/src/agent/coordinator.rs` to expose child execution metadata views, bounded lifecycle events, explicit `Cancelling` state, and terminal snapshot handling from coordinator-owned authority. | ||
| - [x] 2.3 GREEN: Update `clients/agent-runtime/src/agent/mailbox.rs` so mailbox-backed delivery remains internal lifecycle/control transport aligned with coordinator sequencing, dedupe, and no cross-run visibility. | ||
|
|
||
| ## Phase 3: Tool Surface Parity Wiring | ||
|
|
||
| - [x] 3.1 RED: Add tool/service tests in `clients/agent-runtime/src/tools/delegate_launch.rs`, `delegate_inspect.rs`, and `delegate_cancel.rs` covering shared handle usage, requested vs enforced metadata visibility, idempotent cancel, and stale-handle fail-closed behavior. | ||
| - [x] 3.2 GREEN: Update `clients/agent-runtime/src/tools/delegate_launch.rs` to normalize launch metadata, reject unsupported transport/isolation/approval requests before admission, and return the initial orchestration snapshot. | ||
| - [x] 3.3 GREEN: Update `clients/agent-runtime/src/tools/delegate_inspect.rs` and `clients/agent-runtime/src/tools/delegate_cancel.rs` to read only supervised orchestration authority, expose lifecycle/event views, and preserve parent-owned cancel semantics. | ||
| - [x] 3.4 GREEN: Update `clients/agent-runtime/src/tools/delegate.rs` and `clients/agent-runtime/src/tools/mod.rs` to keep single-child compatibility while routing through the same supervised orchestration contract. | ||
|
|
||
| ## Phase 4: Focused Compatibility and Boundary Coverage | ||
|
|
||
| - [x] 4.1 RED: Add integration tests around shared `SupervisedOrchestrationService` in `clients/agent-runtime/src/tools/` for mailbox-backed launch→inspect→cancel flow and single-child `delegate` compatibility. | ||
| - [x] 4.2 GREEN: Adjust `clients/agent-runtime/src/lib.rs` exports only as needed so the finalized orchestration and bridge seam types are consistently available to the runtime crate surface. | ||
| - [x] 4.3 VERIFY: Update/mark `openspec/changes/2026-04-22-track-4-orchestration-parity-seam/tasks.md` during apply as tasks land, keeping Track 6 transport, reconnect/resume, JWT auth, and full remote sessions explicitly out of scope. |
Contributor
There was a problem hiding this comment.
Add explicit risk/threat and rollback tasks before archiving as fully complete.
The checklist tracks RED/GREEN/VERIFY well, but it omits an explicit threat/risk note and rollback strategy task for these runtime/gateway changes. Please add a final checklist item capturing security/runtime failure modes and rollback steps.
Proposed doc patch
## Phase 4: Focused Compatibility and Boundary Coverage
@@
- [x] 4.3 VERIFY: Update/mark `openspec/changes/2026-04-22-track-4-orchestration-parity-seam/tasks.md` during apply as tasks land, keeping Track 6 transport, reconnect/resume, JWT auth, and full remote sessions explicitly out of scope.
+
+## Phase 5: Risk and Rollback Readiness
+
+- [ ] 5.1 VERIFY: Document threat/risk notes for fail-closed admission, mailbox ordering/dedupe, and cancellation semantics regressions.
+- [ ] 5.2 VERIFY: Document rollback strategy (feature flags/revert points/data-contract safety) for coordinator, mailbox, and delegate tool surfaces.Based on learnings: "Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes".
📝 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
| ## Phase 1: Contract and Validation Foundation | |
| - [x] 1.1 RED: Add coordinator/service tests in `clients/agent-runtime/src/agent/coordinator.rs` for normalized execution metadata, `remote_bridge` rejection, unsupported isolation rejection, and unsupported permission-broker rejection. | |
| - [x] 1.2 GREEN: In `clients/agent-runtime/src/agent/coordinator.rs`, add normalized request/enforced-guarantee types, launch rejection types, and centralized fail-closed admission validation used by supervised orchestration. | |
| - [x] 1.3 REFACTOR: Tighten `clients/agent-runtime/src/bridge/mod.rs` to metadata-only seam types consumed by the orchestration contract, without active runner wiring. | |
| ## Phase 2: Lifecycle Read Model and Mailbox Semantics | |
| - [x] 2.1 RED: Extend coordinator/mailbox tests in `clients/agent-runtime/src/agent/coordinator.rs` and `clients/agent-runtime/src/agent/mailbox.rs` for `cancelling` state, immutable terminal states, deterministic event ordering, and redelivery dedupe. | |
| - [x] 2.2 GREEN: Update `clients/agent-runtime/src/agent/coordinator.rs` to expose child execution metadata views, bounded lifecycle events, explicit `Cancelling` state, and terminal snapshot handling from coordinator-owned authority. | |
| - [x] 2.3 GREEN: Update `clients/agent-runtime/src/agent/mailbox.rs` so mailbox-backed delivery remains internal lifecycle/control transport aligned with coordinator sequencing, dedupe, and no cross-run visibility. | |
| ## Phase 3: Tool Surface Parity Wiring | |
| - [x] 3.1 RED: Add tool/service tests in `clients/agent-runtime/src/tools/delegate_launch.rs`, `delegate_inspect.rs`, and `delegate_cancel.rs` covering shared handle usage, requested vs enforced metadata visibility, idempotent cancel, and stale-handle fail-closed behavior. | |
| - [x] 3.2 GREEN: Update `clients/agent-runtime/src/tools/delegate_launch.rs` to normalize launch metadata, reject unsupported transport/isolation/approval requests before admission, and return the initial orchestration snapshot. | |
| - [x] 3.3 GREEN: Update `clients/agent-runtime/src/tools/delegate_inspect.rs` and `clients/agent-runtime/src/tools/delegate_cancel.rs` to read only supervised orchestration authority, expose lifecycle/event views, and preserve parent-owned cancel semantics. | |
| - [x] 3.4 GREEN: Update `clients/agent-runtime/src/tools/delegate.rs` and `clients/agent-runtime/src/tools/mod.rs` to keep single-child compatibility while routing through the same supervised orchestration contract. | |
| ## Phase 4: Focused Compatibility and Boundary Coverage | |
| - [x] 4.1 RED: Add integration tests around shared `SupervisedOrchestrationService` in `clients/agent-runtime/src/tools/` for mailbox-backed launch→inspect→cancel flow and single-child `delegate` compatibility. | |
| - [x] 4.2 GREEN: Adjust `clients/agent-runtime/src/lib.rs` exports only as needed so the finalized orchestration and bridge seam types are consistently available to the runtime crate surface. | |
| - [x] 4.3 VERIFY: Update/mark `openspec/changes/2026-04-22-track-4-orchestration-parity-seam/tasks.md` during apply as tasks land, keeping Track 6 transport, reconnect/resume, JWT auth, and full remote sessions explicitly out of scope. | |
| ## Phase 1: Contract and Validation Foundation | |
| - [x] 1.1 RED: Add coordinator/service tests in `clients/agent-runtime/src/agent/coordinator.rs` for normalized execution metadata, `remote_bridge` rejection, unsupported isolation rejection, and unsupported permission-broker rejection. | |
| - [x] 1.2 GREEN: In `clients/agent-runtime/src/agent/coordinator.rs`, add normalized request/enforced-guarantee types, launch rejection types, and centralized fail-closed admission validation used by supervised orchestration. | |
| - [x] 1.3 REFACTOR: Tighten `clients/agent-runtime/src/bridge/mod.rs` to metadata-only seam types consumed by the orchestration contract, without active runner wiring. | |
| ## Phase 2: Lifecycle Read Model and Mailbox Semantics | |
| - [x] 2.1 RED: Extend coordinator/mailbox tests in `clients/agent-runtime/src/agent/coordinator.rs` and `clients/agent-runtime/src/agent/mailbox.rs` for `cancelling` state, immutable terminal states, deterministic event ordering, and redelivery dedupe. | |
| - [x] 2.2 GREEN: Update `clients/agent-runtime/src/agent/coordinator.rs` to expose child execution metadata views, bounded lifecycle events, explicit `Cancelling` state, and terminal snapshot handling from coordinator-owned authority. | |
| - [x] 2.3 GREEN: Update `clients/agent-runtime/src/agent/mailbox.rs` so mailbox-backed delivery remains internal lifecycle/control transport aligned with coordinator sequencing, dedupe, and no cross-run visibility. | |
| ## Phase 3: Tool Surface Parity Wiring | |
| - [x] 3.1 RED: Add tool/service tests in `clients/agent-runtime/src/tools/delegate_launch.rs`, `delegate_inspect.rs`, and `delegate_cancel.rs` covering shared handle usage, requested vs enforced metadata visibility, idempotent cancel, and stale-handle fail-closed behavior. | |
| - [x] 3.2 GREEN: Update `clients/agent-runtime/src/tools/delegate_launch.rs` to normalize launch metadata, reject unsupported transport/isolation/approval requests before admission, and return the initial orchestration snapshot. | |
| - [x] 3.3 GREEN: Update `clients/agent-runtime/src/tools/delegate_inspect.rs` and `clients/agent-runtime/src/tools/delegate_cancel.rs` to read only supervised orchestration authority, expose lifecycle/event views, and preserve parent-owned cancel semantics. | |
| - [x] 3.4 GREEN: Update `clients/agent-runtime/src/tools/delegate.rs` and `clients/agent-runtime/src/tools/mod.rs` to keep single-child compatibility while routing through the same supervised orchestration contract. | |
| ## Phase 4: Focused Compatibility and Boundary Coverage | |
| - [x] 4.1 RED: Add integration tests around shared `SupervisedOrchestrationService` in `clients/agent-runtime/src/tools/` for mailbox-backed launch→inspect→cancel flow and single-child `delegate` compatibility. | |
| - [x] 4.2 GREEN: Adjust `clients/agent-runtime/src/lib.rs` exports only as needed so the finalized orchestration and bridge seam types are consistently available to the runtime crate surface. | |
| - [x] 4.3 VERIFY: Update/mark `openspec/changes/2026-04-22-track-4-orchestration-parity-seam/tasks.md` during apply as tasks land, keeping Track 6 transport, reconnect/resume, JWT auth, and full remote sessions explicitly out of scope. | |
| ## Phase 5: Risk and Rollback Readiness | |
| - [ ] 5.1 VERIFY: Document threat/risk notes for fail-closed admission, mailbox ordering/dedupe, and cancellation semantics regressions. | |
| - [ ] 5.2 VERIFY: Document rollback strategy (feature flags/revert points/data-contract safety) for coordinator, mailbox, and delegate tool surfaces. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.md`
around lines 3 - 26, Add a final checklist item to the tasks doc
(openspec/changes/.../tasks.md) that explicitly captures threat/risk notes and
rollback strategy: insert under Phase 4 (after 4.3 VERIFY) a bullet describing
security/runtime/gateway failure modes to monitor (e.g., auth/token, transport
resume, unauthorized isolation/permission denial, mailbox redelivery), automated
and manual rollback steps (feature flags, config toggles, service-side rejects,
DB/schema rollback, and test harness replay), and a requirement to add
boundary/failure-mode tests; reference the doc's Track and supervised
orchestration artifacts (SupervisedOrchestrationService, coordinator/mailbox,
delegate_* tools) so reviewers know which components the risk/rollback item
covers.
Comment on lines
+5
to
+26
| - [x] 1.1 RED: Add coordinator/service tests in `clients/agent-runtime/src/agent/coordinator.rs` for normalized execution metadata, `remote_bridge` rejection, unsupported isolation rejection, and unsupported permission-broker rejection. | ||
| - [x] 1.2 GREEN: In `clients/agent-runtime/src/agent/coordinator.rs`, add normalized request/enforced-guarantee types, launch rejection types, and centralized fail-closed admission validation used by supervised orchestration. | ||
| - [x] 1.3 REFACTOR: Tighten `clients/agent-runtime/src/bridge/mod.rs` to metadata-only seam types consumed by the orchestration contract, without active runner wiring. | ||
|
|
||
| ## Phase 2: Lifecycle Read Model and Mailbox Semantics | ||
|
|
||
| - [x] 2.1 RED: Extend coordinator/mailbox tests in `clients/agent-runtime/src/agent/coordinator.rs` and `clients/agent-runtime/src/agent/mailbox.rs` for `cancelling` state, immutable terminal states, deterministic event ordering, and redelivery dedupe. | ||
| - [x] 2.2 GREEN: Update `clients/agent-runtime/src/agent/coordinator.rs` to expose child execution metadata views, bounded lifecycle events, explicit `Cancelling` state, and terminal snapshot handling from coordinator-owned authority. | ||
| - [x] 2.3 GREEN: Update `clients/agent-runtime/src/agent/mailbox.rs` so mailbox-backed delivery remains internal lifecycle/control transport aligned with coordinator sequencing, dedupe, and no cross-run visibility. | ||
|
|
||
| ## Phase 3: Tool Surface Parity Wiring | ||
|
|
||
| - [x] 3.1 RED: Add tool/service tests in `clients/agent-runtime/src/tools/delegate_launch.rs`, `delegate_inspect.rs`, and `delegate_cancel.rs` covering shared handle usage, requested vs enforced metadata visibility, idempotent cancel, and stale-handle fail-closed behavior. | ||
| - [x] 3.2 GREEN: Update `clients/agent-runtime/src/tools/delegate_launch.rs` to normalize launch metadata, reject unsupported transport/isolation/approval requests before admission, and return the initial orchestration snapshot. | ||
| - [x] 3.3 GREEN: Update `clients/agent-runtime/src/tools/delegate_inspect.rs` and `clients/agent-runtime/src/tools/delegate_cancel.rs` to read only supervised orchestration authority, expose lifecycle/event views, and preserve parent-owned cancel semantics. | ||
| - [x] 3.4 GREEN: Update `clients/agent-runtime/src/tools/delegate.rs` and `clients/agent-runtime/src/tools/mod.rs` to keep single-child compatibility while routing through the same supervised orchestration contract. | ||
|
|
||
| ## Phase 4: Focused Compatibility and Boundary Coverage | ||
|
|
||
| - [x] 4.1 RED: Add integration tests around shared `SupervisedOrchestrationService` in `clients/agent-runtime/src/tools/` for mailbox-backed launch→inspect→cancel flow and single-child `delegate` compatibility. | ||
| - [x] 4.2 GREEN: Adjust `clients/agent-runtime/src/lib.rs` exports only as needed so the finalized orchestration and bridge seam types are consistently available to the runtime crate surface. | ||
| - [x] 4.3 VERIFY: Update/mark `openspec/changes/2026-04-22-track-4-orchestration-parity-seam/tasks.md` during apply as tasks land, keeping Track 6 transport, reconnect/resume, JWT auth, and full remote sessions explicitly out of scope. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Confirm referenced runtime files exist:"
fd -a 'coordinator.rs|mailbox.rs|delegate_launch.rs|delegate_inspect.rs|delegate_cancel.rs|delegate.rs|mod.rs|lib.rs'
echo
echo "2) Spot-check key behaviors claimed as complete:"
rg -n -C2 'remote_bridge|unsupported isolation|permission-broker|Cancelling|dedupe|idempotent cancel|stale-handle|SupervisedOrchestrationService' --type rust
echo
echo "3) Confirm integration tests under tools mention launch->inspect->cancel flow:"
rg -n -C2 'launch.*inspect.*cancel|inspect.*cancel|SupervisedOrchestrationService' --type rust clients/agent-runtime/src/tools
echo
echo "4) Validate task-file path references (archived vs non-archived):"
rg -n 'openspec/changes/.*/tasks.md' openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdRepository: dallay/corvus
Length of output: 50369
🏁 Script executed:
#!/bin/bash
# Check if the archived tasks.md explicitly documents Track 6 out-of-scope constraints
head -50 openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.mdRepository: dallay/corvus
Length of output: 3265
Task 4.3 references an outdated non-archived path; update the documented path reference or clarify intent.
All claimed implementations, tests, and integration flows verified as complete. However, task 4.3 is marked [x] VERIFY but it references openspec/changes/2026-04-22-track-4-orchestration-parity-seam/tasks.md, which doesn't match the actual archived file location at openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.md.
Track 6 scope exclusion (transport, reconnect/resume, JWT auth, full remote sessions) is explicitly documented. Update 4.3 to reference the correct archived path, or document why the reference should remain non-archived.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-22-2026-04-22-track-4-orchestration-parity-seam/tasks.md`
around lines 5 - 26, Task 4.3's documentation references an
incorrect/non-archived path; update the line for "4.3 VERIFY" in tasks.md so it
either points to the actual archived location or explicitly states why the
non-archived path is intentional (e.g., keep as a top-level spec pointer vs
archived copy). Locate the "4.3 VERIFY" entry under "Phase 4: Focused
Compatibility and Boundary Coverage" and replace the outdated path string with
the correct archived reference or add a short clarifying note indicating
retention of the non-archived reference and reasoning (mention Track 6
exclusions remain out of scope).
Comment on lines
+191
to
+192
| | `openspec/changes/rook-591-chat-completions-streaming-transport/design.md` | Create | This technical design artifact. | | ||
| | `openspec/changes/rook-591-chat-completions-streaming-transport/state.yaml` | Modify | Mark design phase complete and advance to tasks. | |
Contributor
There was a problem hiding this comment.
Stale self-reference to the non-archived change directory.
The File Changes table still lists openspec/changes/rook-591-chat-completions-streaming-transport/design.md and .../state.yaml even though this file itself is now under openspec/changes/archive/2026-04-22-.... Either refresh the paths to the archived location or remove the rows — otherwise the "Create/Modify" entries are pointing at ghosts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/design.md`
around lines 191 - 192, The File Changes table contains stale paths pointing to
openspec/changes/rook-591-chat-completions-streaming-transport/design.md and
openspec/changes/rook-591-chat-completions-streaming-transport/state.yaml while
the current document lives under
openspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport;
update the table rows to reference the archived paths
(openspec/changes/archive/2026-04-22-rook-591-chat-completions-streaming-transport/design.md
and .../state.yaml) or remove those rows entirely so the "Create/Modify" entries
no longer point to nonexistent locations.
Expose stable /api endpoints for accounts, pools, routes, health, settings, and usage status so the dashboard and automation can manage Rook through supported contracts instead of internal runtime state.
Wrap admin extractor failures in the documented error envelope, avoid leaking internal error details, make account and route deletes safer under references, validate pool fallbacks transactionally, and align Rook admin docs with the verified contract.
Sync the admin API delta spec into the gateway source of truth and archive the completed rook-590-admin-api change after PASS verification.
Add inbound auth, transport middleware, global surface rate limits, chat idempotency, and streaming support so Rook can protect and serve OpenAI-compatible chat traffic more reliably.
Finalize durable local orchestration, parent-owned lifecycle controls, and a fail-closed remote bridge seam so multi-agent delegation can be inspected and cancelled through a stable runtime contract.
Implements the dedicated Rook dashboard surface for overview and provider account administration. - add embedded rook-dashboard Vue app and tests - preserve stored api keys on metadata-only account edits - wire built assets into clients/rook assets - archive OpenSpec change for issue #592 - update dashboard spec with Rook operator flows
Remove unnecessary font-family quotes so the web pre-push checks pass for the new embedded Rook dashboard.
Align the new embedded Rook dashboard sources with Biome import, unused binding, and formatting rules so the web pre-push checks pass.
Resolve the remaining Biome/style formatting and template-binding warnings in the embedded Rook dashboard so pre-push validation can complete successfully.
Apply Biome formatting to the embedded Rook dashboard sources so the web pre-push checks pass cleanly.
|
yacosta738
changed the title
yacosta738
changed the title
yacosta738
changed the title
yacosta738
changed the title
yacosta738
changed the title
8 tasks
8 tasks
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.
Related Issues
closes #592
closes #593
closes #594
closes #595
closes #596
closes #597
closes #598
closes #599
Summary
This PR completes the first M2 Corvus Rook operator surfaces and lands the first meaningful M3 hardening and observability slices.
It keeps the Rook experience separate from the legacy Corvus dashboard and includes:
clients/web/apps/rook-dashboardserved byclients/rookapi_keypreserves the stored secret on metadata-only editsrook tuiandrook serve --tuiWith #597, the Rook TUI is intentionally finalized as a fast read-only observability surface, while the web dashboard remains the primary operator surface for setup and mutations. With #598 and #599, the gateway security and admin observability baselines are strengthened without inventing unsupported pairing integration, fake usage analytics, or durable health history.
Tested Information
I verified the shipped slices with targeted dashboard and Rook runtime validation recorded in archived verification reports, plus a full repository build check.
Dashboard validation
pnpm --dir "clients/web" --filter @corvus/rook-dashboard checkpnpm --dir "clients/web" --filter @corvus/rook-dashboard test -- --run src/features/accounts/AccountsPage.spec.tspnpm --dir "clients/web" --filter @corvus/rook-dashboard test -- src/features/pools/usePools.spec.ts src/features/pools/PoolsPage.spec.ts src/features/routes/useRoutes.spec.ts src/features/routes/RoutesPage.spec.tspnpm --filter @corvus/rook-dashboard run testpnpm --filter @corvus/rook-dashboard run test:e2epnpm --filter @corvus/rook-dashboard run buildpnpm --dir "clients/web" --filter @corvus/dashboard checkObserved evidence from archived verification:
pnpm checkPASS, focused pools/routes tests PASS, Playwright PASScheckPASS, unit/component tests71passed, E2E tests2passed, build emitted embedded assets successfullyRook TUI validation
cargo fmt --manifest-path "clients/rook/Cargo.toml" --checkcargo test --manifest-path "clients/rook/Cargo.toml"cargo test --manifest-path "clients/rook/Cargo.toml" tui::cargo test --manifest-path "clients/rook/Cargo.toml" tui_command_launches_real_runner_with_effective_db_pathcargo test --manifest-path "clients/rook/Cargo.toml" enable_tui_runs_embedded_tui_with_shared_shutdowncargo clippy --manifest-path "clients/rook/Cargo.toml" --all-targets -- -D warningsObserved evidence from archived verification (#595, #596, #597):
cargo testPASS from prior TUI slice verification (src/lib.rs: 262 passed,src/main.rs: 3 passed, doc tests: 1 passed)cargo fmt --checkPASScargo clippy ... -D warningsPASSGateway hardening and audit validation
cargo test --manifest-path "clients/rook/Cargo.toml" serve_cli_defaults_to_loopback_first_bind_posturecargo test --manifest-path "clients/rook/Cargo.toml" server_config_defaults_to_loopback_first_bind_targetcargo test --manifest-path "clients/rook/Cargo.toml" explicit_non_loopback_override_remains_honoredcargo test --manifest-path "clients/rook/Cargo.toml" proxy_chat_completion_never_reuses_inbound_bearer_token_as_provider_authcargo test --manifest-path "clients/rook/Cargo.toml" inbound_auth_operator_state_reports_enabled_and_configured_without_exposing_tokencargo test --manifest-path "clients/rook/Cargo.toml" account_view_redacts_api_key_and_sets_has_api_keycargo test --manifest-path "clients/rook/Cargo.toml" middleware_completion_log_fields_remain_structured_and_secret_freecargo test --manifest-path "clients/rook/Cargo.toml" open_in_memory_applies_admin_audit_events_migrationcargo test --manifest-path "clients/rook/Cargo.toml" open_in_memory_records_admin_audit_events_migration_versioncargo test --manifest-path "clients/rook/Cargo.toml" append_and_list_admin_audit_events_newest_firstcargo test --manifest-path "clients/rook/Cargo.toml" sqlite_audit_service_appends_and_lists_recent_eventscargo test --manifest-path "clients/rook/Cargo.toml" registry_audit_append_and_list_round_tripcargo test --manifest-path "clients/rook/Cargo.toml" handle_create_account_appends_audit_event_on_successcargo clippy --manifest-path "clients/rook/Cargo.toml" --all-targets -- -D warningsObserved evidence from archived verification (#598, #599):
cargo clippy ... -D warningsPASSFull repository check
make buildObserved evidence:
make buildexit code:0Documentation Impact
openspec/specs/dashboard/spec.mdopenspec/specs/rook-tui/spec.mdopenspec/specs/gateway/spec.mdopenspec/changes/archive/2026-04-22-rook-592-dashboard-overview-providers-accounts/openspec/changes/archive/2026-04-22-rook-593-dashboard-pools-routes-health-ops/openspec/changes/archive/2026-04-22-rook-594-dashboard-usage-settings/openspec/changes/archive/2026-04-23-rook-595-tui-status-providers-pools-health/openspec/changes/archive/2026-04-23-rook-596-tui-route-inspection-recent-logs/openspec/changes/archive/2026-04-23-rook-597-tui-setup-troubleshooting/openspec/changes/archive/2026-04-23-rook-598-security-defaults-and-secret-protection/openspec/changes/archive/2026-04-23-rook-599-observability-usage-health-audit/Breaking Changes
None.
Checklist