feat(agent-runtime): add typed slash command contract#546
Conversation
…vice.rs issues - gateway/mod.rs: replace MockMemory with SqliteMemory in slash-session tests - canonical_outcome_early_response_intercepts_slash_session_commands - legacy_webhook_preview_intercepts_slash_session_commands - web_chat_stream_returns_deterministic_slash_session_sse_without_provider_execution - update assertions to expect 422 + session_command_failed error - session_commands/service.rs: - FakeMemory::get_session now looks up by session_id - build_resume_context adds truncation limits (2k per entry, 16k global) - visibility check uses get_session instead of paginated list - openspec/specs/sessions/spec.md: add trailing newline
# Conflicts: # clients/agent-runtime/src/gateway/mod.rs # clients/agent-runtime/src/session_commands/service.rs
Deploying corvus with
|
| Latest commit: |
08f33f0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28b16acf.corvus-42x.pages.dev |
| Branch Preview URL: | https://feature-dallay-540-slash-com.corvus-42x.pages.dev |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces a typed, owned Changes
Sequence Diagram(s)sequenceDiagram
participant Transport as Transport (CLI/HTTP/SSE/Channel)
participant PreExec as pre_execution::evaluate_ingress
participant Registry as SlashCommandRegistry / Service
participant Memory as Memory (SqliteMemory)
Transport->>PreExec: build CommandContext (for_*)
PreExec->>Registry: dispatch(context, prompt)
alt SessionCommandOutcome::Success
Registry-->>PreExec: SessionCommandOutcome::Success { message, data }
PreExec-->>Transport: map success.message -> response
else SessionCommandOutcome::Failure
Registry-->>PreExec: SessionCommandOutcome::Failure { kind, message }
alt failure requires scoped resume lookup
Registry->>Memory: get_resumable_session_for_scope(session_id, caller_scope_key)
Memory-->>Registry: Option<ResumableSessionEntry>
end
PreExec-->>Transport: map failure.message -> response (status or error envelope)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-16 to 2026-04-16 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clients/agent-runtime/src/gateway/mod.rs (1)
1975-1998:⚠️ Potential issue | 🟠 MajorReserve idempotency before the preview slash-command short-circuit.
Line 1975 can return before Line 1990 records
X-Idempotency-Key, so preview-mode/webhookslash commands bypass duplicate suppression entirely. A retried request with the same key will execute again instead of returning the duplicate envelope.As per coding guidelines, "`**/*`: Security first, performance second. Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules."♻️ Suggested ordering
- if is_preview && !dispatcher_enabled { - if let Some((response, persist_idempotency)) = canonical_outcome_early_response( - &state, - &session_id, - &scrubbed_message, - token_hash.as_deref(), - ) - .await - { - release_idempotency_key(&state, None, persist_idempotency); - return response; - } - } - let reserved_idempotency_key = if let Some(idempotency_key) = webhook_idempotency_key(&headers) { if !state.idempotency_store.record_if_new(idempotency_key) { return webhook_duplicate_response(idempotency_key); } Some(idempotency_key) } else { None }; + + if is_preview && !dispatcher_enabled { + if let Some((response, persist_idempotency)) = canonical_outcome_early_response( + &state, + &session_id, + &scrubbed_message, + token_hash.as_deref(), + ) + .await + { + release_idempotency_key(&state, reserved_idempotency_key, persist_idempotency); + return response; + } + }🤖 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 1975 - 1998, Move the idempotency reservation before the preview-mode short-circuit so preview slash-command requests still go through duplicate suppression: call webhook_idempotency_key(&headers) and record_if_new(...) on state.idempotency_store before checking is_preview && !dispatcher_enabled; on duplicate return webhook_duplicate_response(idempotency_key) as before; if you reserved a key and then still short-circuit via canonical_outcome_early_response(...).await, ensure you call release_idempotency_key(&state, None, persist_idempotency) or release_idempotency_key(&state, None, reserved_idempotency_key) as appropriate to avoid leaking reservations; preserve the existing persist_idempotency handling from canonical_outcome_early_response.clients/agent-runtime/src/session_commands/service.rs (1)
576-589:⚠️ Potential issue | 🟠 MajorBug: Resume context truncation realignment logic is incorrect.
The realignment check at line 582 will never be true. After
find("\n- ")returns positionpos,skip(pos)produces a string starting with"\n- ", not"- ".// Current: never matches because adjustment starts with "\n" if adjustment.starts_with("- ") { context = adjustment; }🐛 Proposed fix
if context.chars().count() > MAX_CONTEXT_LENGTH { let excess = context.chars().count() - MAX_CONTEXT_LENGTH; context = context.chars().skip(excess).collect::<String>(); - // Ensure we don't start mid-entry if let Some(pos) = context.find("\n- ") { - let adjustment: String = context.chars().skip(pos).collect(); - if adjustment.starts_with("- ") { - context = adjustment; - } + // Skip past the newline to start at the entry + context = context.chars().skip(pos + 1).collect(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/session_commands/service.rs` around lines 576 - 589, The truncation realignment in the function that enforces MAX_CONTEXT_LENGTH uses context.find("\n- ") and then creates adjustment via context.chars().skip(pos) which yields a string starting with "\n- ", so the conditional adjustment.starts_with("- ") never matches; update the logic in that block (where context is reassigned after computing excess and pos) to either skip one more char (pos + 1) or check for adjustment.starts_with("\n- ") and then strip the leading newline before assigning to context so list entries beginning with "- " are correctly realigned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/channels/mod.rs`:
- Around line 1870-1876: The CommandContext is being constructed with a
hardcoded ExecutionMode::Standard; change the third argument in
CommandContext::for_channel to use the actual runtime execution mode (the
existing ExecutionMode value in scope, e.g., execution_mode or runtime_mode)
instead of ExecutionMode::Standard so slash commands inherit the real plan vs
standard mode; if no ExecutionMode variable is available in this function,
thread an ExecutionMode parameter through the caller chain into this site
(affecting ingress_context creation), update references to
crate::session_commands::CommandContext::for_channel accordingly, and add/adjust
tests to cover both plan and standard paths to prevent authz/behavior
regressions.
In `@clients/agent-runtime/src/gateway/mod.rs`:
- Around line 6168-6177: Add a regression test for the failure-path of the new
stream contract: when the session command outcome is
SessionCommandOutcome::Failure, assert that the response status is
StatusCode::UNPROCESSABLE_ENTITY (422) and that the collected response body (the
body_str built from resp.into_body().collect()) contains the SSE error envelope
("event: error") and the expected failure message instead of the success
chunks/done lines; update or add a test near the existing success-case
assertions that sets up a Failure outcome and verifies resp.status() ==
StatusCode::UNPROCESSABLE_ENTITY and body_str.contains("event: error") plus the
failure text.
- Around line 1770-1775: The HTTP command context is always being created with
CommandSessionSource::Explicit, losing the real source resolved by
resolve_session_id; update the two handle_webhook() call sites so they pass the
resolved session_source (the value returned from resolve_session_id) into
crate::session_commands::CommandContext::for_gateway_http instead of the
hardcoded CommandSessionSource::Explicit, ensuring the context uses the actual
session source for /webhook, /resume and other source-sensitive flows.
In `@clients/agent-runtime/src/gateway/webhook_dispatch.rs`:
- Around line 396-410: evaluate_ingress is being called with
request.execution_mode before the server clamps the mode via
resolve_webhook_execution_mode, allowing pre-execution to observe a less
restrictive mode; change the call site so you first compute the clamped mode
using resolve_webhook_execution_mode(...) and pass that clamped value into
CommandContext::for_webhook (or otherwise into evaluate_ingress) instead of
request.execution_mode so evaluate_ingress (and ingress_context) always sees the
final, enforced execution mode; update references to
ingress_context/request.execution_mode/evaluate_ingress accordingly.
In `@clients/agent-runtime/src/main.rs`:
- Around line 1401-1405: The fast-return path that handles slash commands via
maybe_handle_cli_session_command(&config, raw_message).await? executes before
normalizing agent flags (e.g., applying plan to config.agent.execution_mode) and
validating unsupported overrides like --peripheral, causing incorrect
CommandContext construction for inputs like `--plan --message "/resume"` or
silently ignoring invalid --peripheral flags; move or duplicate the
flag-normalization and validation logic (the code that sets
config.agent.execution_mode from plan and rejects unsupported --peripheral
overrides) to run before invoking maybe_handle_cli_session_command so the CLI
contract and error reporting are preserved for the slash-command fast path and
CommandContext is built from the normalized config.
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.md`:
- Around line 166-170: The design doc's CommandCaller enum is missing the
DerivedCliScope variant that exists in the implementation; update the design.md
CommandCaller declaration to include the fourth variant DerivedCliScope (with
the same field shape used in the codebase, e.g., any channel/scope_key or
cli-specific fields used by the implementation) so the design matches the enum
defined in types.rs (ensure the names VerifiedTokenHash, DerivedChannelScope,
DerivedCliScope, and Unavailable are present and described).
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.md`:
- Line 40: The archived proposal references the live spec path
`openspec/changes/slash-command-context-permissions-result-contract/specs/`,
which is now stale; update the reference in the document to the archived specs
path (e.g.,
`openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/`)
or remove the live-directory reference if archives do not include a specs/
subtree so the archived proposal points to an accurate location; edit the single
occurrence of the path string in the proposal.md to reflect the correct archived
path or to indicate the specs are in the live change directory only.
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.md`:
- Line 13: Add a dedicated scenario that tests the rule "If no verifiable or
derivable caller scope can be established for an authorization-sensitive
`/resume` operation, the runtime MUST return an explicit authorization-denied or
unsupported outcome instead of broadening visibility"; construct the scenario to
send a `/resume` request with missing/invalid/unverifiable caller scope (e.g.,
absent claims, tampered token, or unable to derive scope from context), assert
the runtime returns an explicit denial (authorization-denied/unsupported error
or 403-like outcome) rather than returning broader visibility or success, and
place this scenario alongside the existing sessions `/resume` scenarios so it
covers the previously untested path referenced by the rule.
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.md`:
- Line 17: Update the sentence in verify-report.md that reads "All tasks in
`openspec/changes/slash-command-context-permissions-result-contract/tasks.md`
remain marked complete." so it points to the archived tasks list instead of the
pre-archive location; replace the referenced tasks.md path with the
corresponding archived path under changes/archive/.../tasks.md so the link stays
valid after the active change directory is cleaned up.
---
Outside diff comments:
In `@clients/agent-runtime/src/gateway/mod.rs`:
- Around line 1975-1998: Move the idempotency reservation before the
preview-mode short-circuit so preview slash-command requests still go through
duplicate suppression: call webhook_idempotency_key(&headers) and
record_if_new(...) on state.idempotency_store before checking is_preview &&
!dispatcher_enabled; on duplicate return
webhook_duplicate_response(idempotency_key) as before; if you reserved a key and
then still short-circuit via canonical_outcome_early_response(...).await, ensure
you call release_idempotency_key(&state, None, persist_idempotency) or
release_idempotency_key(&state, None, reserved_idempotency_key) as appropriate
to avoid leaking reservations; preserve the existing persist_idempotency
handling from canonical_outcome_early_response.
In `@clients/agent-runtime/src/session_commands/service.rs`:
- Around line 576-589: The truncation realignment in the function that enforces
MAX_CONTEXT_LENGTH uses context.find("\n- ") and then creates adjustment via
context.chars().skip(pos) which yields a string starting with "\n- ", so the
conditional adjustment.starts_with("- ") never matches; update the logic in that
block (where context is reassigned after computing excess and pos) to either
skip one more char (pos + 1) or check for adjustment.starts_with("\n- ") and
then strip the leading newline before assigning to context so list entries
beginning with "- " are correctly realigned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fb42758-5879-486f-bf1b-5fc239dccdf3
📒 Files selected for processing (21)
clients/agent-runtime/crates/corvus-traits/src/memory.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/exploration.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/state.yamlopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/tasks.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.mdopenspec/specs/sessions/spec.mdopenspec/specs/slash-command-registry/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). (1)
- GitHub Check: Cloudflare Pages
🧰 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:
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/state.yamlopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/exploration.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/tasks.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.mdclients/agent-runtime/src/main.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdclients/agent-runtime/src/gateway/webhook_dispatch.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdclients/agent-runtime/crates/corvus-traits/src/memory.rsopenspec/specs/slash-command-registry/spec.mdclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/slash-command-registry/spec.mdopenspec/specs/sessions/spec.mdclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.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-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/exploration.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/tasks.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdopenspec/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/slash-command-registry/spec.mdopenspec/specs/sessions/spec.md
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/main.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.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/main.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/crates/corvus-traits/src/memory.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.rs
clients/agent-runtime/src/main.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
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
Keep startup path lean and avoid heavy initialization in command parsing flow
Files:
clients/agent-runtime/src/main.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/agent-runtime/src/main.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/crates/corvus-traits/src/memory.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.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/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.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/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/mod.rs
🧠 Learnings (10)
📓 Common learnings
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
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
📚 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:
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/tasks.mdclients/agent-runtime/src/main.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.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:
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/gateway/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/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-15-slash-command-context-permissions-result-contract/tasks.mdclients/agent-runtime/src/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/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/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/session_commands/service.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/main.rsclients/agent-runtime/src/pre_execution/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/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/src/main.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/pre_execution/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/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/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: 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/agent-runtime/src/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/gateway/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/main.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.md
[grammar] ~7-~7: Ensure spelling is correct
Context: ...vice shape, but replaces the narrow and stringly contracts with typed runtime models: 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/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] 49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 53-53: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (17)
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.md (1)
11-12: Strong auth-scope boundary definition here.The distinction between authenticated gateway identity and derived channel/CLI identity is clear and actionable, and it aligns well with caller-scoped
/resumeenforcement.Also applies to: 34-40
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.md (1)
1-129: Design document is well-structured and aligns with implementation.Architecture decisions, data flow diagrams, and interface contracts are clear. The testing strategy covers the key layers. Line 7's "stringly" is valid jargon (not a typo).
clients/agent-runtime/src/session_commands/registry.rs (3)
131-150: LGTM: Dispatch correctly propagates typed context and outcomes.The
dispatchmethod cleanly forwardsCommandContextthrough to handlers and preservesSessionCommandOutcomewithout flattening. Thesession_idclone at line 47 (in pre_execution) is necessary sincecontextownership transfers to the handler.
175-243: LGTM: Built-in registrations use typed requirement metadata.Clean separation of capability/permission/backend constants. The
/resumecommand correctly declaresRequiresCallerScopeandRequiresResumableSessionVisibilitypermissions alongside theSessionLifecyclecapability.
320-362: LGTM: Invocation validation returns typed failures.
validate_invocationcorrectly returnsSessionCommandFailurewithInvalidArgumentskind instead of a separate error type, keeping the outcome model consistent.openspec/specs/sessions/spec.md (1)
276-318: LGTM:/resumespec correctly requires typed context authorization.The updated requirements clearly specify:
- Caller-scoped visibility for both listing and targeted resume
- Distinct handling of authenticated vs derived vs unavailable caller identity
- Explicit denial when caller scope cannot be established
This aligns with the
service.rsimplementation usingcontext.caller.scope_key()andget_resumable_session_for_scope.openspec/specs/slash-command-registry/spec.md (2)
42-89: LGTM: Typed execution context contract is well-specified.The spec correctly requires preservation of:
- Session identity
- Typed caller identity (authenticated/derived/unavailable distinction per
CommandCallerenum)- Originating ingress kind
- Execution mode
- Evaluated requirement facts
Scenarios cover gateway authenticated vs CLI/channel derived identity semantics.
91-111: LGTM: Non-lossy outcome contract prevents error flattening.The spec explicitly requires machine-readable
SessionCommandFailureKindvariants (authorization-denied, unsupported-backend, etc.) to remain distinguishable, matching the implementation intypes.rs:374-385.clients/agent-runtime/src/pre_execution/mod.rs (2)
41-57: LGTM: Ingress evaluation correctly propagates typed context and outcomes.The
session_idclone on line 47 is necessary sincecontextownership transfers todispatch. The typedSessionCommandOutcomeis preserved without flattening into success/failure booleans.
311-363: LGTM: Context builder tests verify transport-specific identity semantics.Good coverage ensuring:
- CLI uses
DerivedCliScopecaller variant- Gateway HTTP uses
VerifiedTokenHashfor authenticated callers- Channel uses
DerivedChannelScopewith channel name preservedopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/slash-command-registry/spec.md (1)
1-126: LGTM: Delta spec cleanly documents contract additions and modifications.The change archive document clearly separates ADDED requirements (typed context, typed metadata, non-lossy outcomes) from MODIFIED requirements (descriptor contract, transport parity), with accurate parenthetical notes explaining previous behavior.
clients/agent-runtime/src/session_commands/service.rs (2)
192-338: LGTM:/resumeenforces caller-scoped authorization correctly.The implementation:
- Denies with
MissingCallerScopewhencontext.caller.scope_key()isNone(line 201-208)- Uses
get_resumable_session_for_scopeto validate target visibility against caller scope (line 211-225)- Returns
PermissionDeniedwhen the scoped lookup fails (line 221-224)- Lists only caller-visible sessions via
list_resumable_sessions(Some(caller_scope_key), ...)(line 307-313)This correctly implements the spec requirement that
/resumevisibility follows caller-scoped boundaries.
468-494: LGTM: Storage errors are sanitized before user-facing exposure.
map_storage_errorcorrectly:
- Detects unsupported backend errors and maps to
UnsupportedBackend- Sanitizes other errors via
sanitize_storage_error(which redacts paths/credentials)- Logs full error detail internally before returning sanitized message
As per coding guidelines: sensitive information is not logged in user-facing messages.
clients/agent-runtime/src/session_commands/types.rs (4)
9-46: LGTM: Storage error sanitization is thorough.The
sanitize_storage_errorfunction:
- Redacts file paths and connection strings via regex
- Maps common error types to fixed user-friendly messages
- Falls back to generic "storage unavailable" rather than leaking sanitized text
- Logs full detail internally at debug level
As per coding guidelines: secrets and sensitive payloads are never exposed in user-facing output.
102-235: LGTM: CommandContext builders correctly preserve transport identity semantics.Each builder (
for_cli,for_gateway_http,for_gateway_stream,for_webhook,for_channel) maps to the appropriateCommandCallervariant:
- Gateway paths use
VerifiedTokenHashfor authenticated callers- CLI uses
DerivedCliScope- Channel uses
DerivedChannelScopewith channel nameThe
CommandContextFactsare derived once at construction time, avoiding repeated interpretation.
271-288: LGTM:CommandCaller::scope_key()correctly distinguishes caller identity.The method returns
Some(&str)for all identity variants exceptUnavailable, enabling authorization checks to uniformly query scope availability without pattern matching at every call site.
332-385: LGTM: Typed outcome model preserves machine-readable failure kinds.
SessionCommandOutcomewithSuccess/Failurevariants andSessionCommandFailureKindenum keeps authorization denials (PermissionDenied,MissingCallerScope) distinct from other failures, satisfying the non-lossy internal contract requirement.
| // `/tldr` without persisted transcript returns a deterministic SSE success payload | ||
| assert_eq!(resp.status(), StatusCode::OK); | ||
| let collected = resp.into_body().collect().await.unwrap(); | ||
| let body_str = std::str::from_utf8(&collected.to_bytes()) | ||
| .unwrap() | ||
| .to_owned(); | ||
|
|
||
| // Debug: print body if test fails | ||
| if !body_str.contains("event: chunk") { | ||
| eprintln!("body_str: {:?}", body_str); | ||
| } | ||
|
|
||
| // Streaming stays on the shared ingress seam and returns the slash-session response without | ||
| // invoking the provider path. | ||
| assert!(body_str.contains("event: chunk")); | ||
| assert!(body_str.contains("No persisted session transcript is available yet.")); | ||
| assert!(body_str.contains("event: done")); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add the missing failure-path stream regression test.
This only locks in the success envelope. The new stream contract also sets HTTP 422 and emits event: error for SessionCommandOutcome::Failure; that behavior should have a direct test as well.
As per coding guidelines, "**/*: Security first, performance second. Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 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 6168 - 6177, Add a
regression test for the failure-path of the new stream contract: when the
session command outcome is SessionCommandOutcome::Failure, assert that the
response status is StatusCode::UNPROCESSABLE_ENTITY (422) and that the collected
response body (the body_str built from resp.into_body().collect()) contains the
SSE error envelope ("event: error") and the expected failure message instead of
the success chunks/done lines; update or add a test near the existing
success-case assertions that sets up a Failure outcome and verifies
resp.status() == StatusCode::UNPROCESSABLE_ENTITY and body_str.contains("event:
error") plus the failure text.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/agent-runtime/src/gateway/webhook_dispatch.rs (1)
459-462: 🧹 Nitpick | 🔵 TrivialRedundant recomputation of clamped execution mode.
resolve_webhook_execution_modeis called again at line 461-462 even thoughclamped_modewas already computed at lines 397-398. This is wasteful and could diverge if logic changes in one place but not the other.Proposed fix
let mut effective_config = config.clone(); - // Clamp request execution mode against server-configured mode (fail-closed to more restrictive) - effective_config.agent.execution_mode = - resolve_webhook_execution_mode(config.agent.execution_mode, Some(request.execution_mode)); + // Use the already-clamped mode computed for ingress evaluation + effective_config.agent.execution_mode = clamped_mode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/gateway/webhook_dispatch.rs` around lines 459 - 462, The code recomputes the clamped execution mode by calling resolve_webhook_execution_mode again when building effective_config; instead reuse the previously computed clamped_mode (the variable calculated earlier) instead of calling resolve_webhook_execution_mode a second time. Update the assignment to effective_config.agent.execution_mode to use clamped_mode (or its equivalent) so the value is not recomputed and the two places remain consistent (refer to resolve_webhook_execution_mode, effective_config, config.clone(), and clamped_mode).
♻️ Duplicate comments (1)
clients/agent-runtime/src/gateway/mod.rs (1)
6200-6209:⚠️ Potential issue | 🟡 MinorAdd the failure-path stream regression too.
This only locks in the success envelope. The new
SessionCommandOutcome::Failurebranch now returns HTTP 422 and emitsevent: error; without a companion test, that contract can still regress silently. Add a case that leaves the slash command unresolved and assertresp.status() == StatusCode::UNPROCESSABLE_ENTITY,body_str.contains("event: error"), and the failure message.As per coding guidelines, "
**/*: Security first, performance second. Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules."🤖 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 6200 - 6209, Add a regression test exercising the failure-path for SessionCommandOutcome::Failure: create a case similar to the existing success-path but leave the slash command unresolved so the gateway returns HTTP 422 and an SSE error envelope; then assert resp.status() == StatusCode::UNPROCESSABLE_ENTITY, collect the body into body_str (like the success test) and assert body_str.contains("event: error") and that it contains the expected failure message (e.g., "No persisted session transcript is available yet." or the specific failure text your code emits). Place this alongside the existing assertions that check the success SSE (reusing resp, collected, body_str variables) so the contract for failure emits is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/gateway/mod.rs`:
- Around line 1764-1775: The ingress contexts are currently built using the
server default execution mode, which causes requests that carried an explicit
execution_mode (e.g., "plan") to be downgraded before evaluate_ingress() and can
make permission/requirement checks incorrect; to fix, change
canonical_outcome_early_response(...) to accept the already-resolved effective
execution mode as an argument and pass that mode into
CommandContext::for_gateway_http (and similarly thread the mode into
for_gateway_stream(...)), then ensure the same effective mode is used later when
constructing the dispatcher/execution context so evaluate_ingress() and
subsequent checks run against the correct mode rather than the server default.
In `@clients/agent-runtime/src/session_commands/service.rs`:
- Around line 576-585: The truncation branch for context currently discards the
truncated string when no "\n- " boundary is found; update the logic around
context, excess, and the find("\n- ") check so you first compute the truncated
candidate (e.g., let truncated =
context.chars().skip(excess).collect::<String>()), then attempt to locate "\n- "
inside that truncated candidate and only replace context with the adjusted slice
if the boundary is found and starts with "- "; otherwise retain the truncated
candidate as the fallback (ensuring it is non-empty and still within
MAX_CONTEXT_LENGTH). Apply this change in the block handling MAX_CONTEXT_LENGTH
(the variables/functions: context, excess, MAX_CONTEXT_LENGTH, adjustment,
find("\n- ")) so you never silently lose content when no list boundary exists.
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.md`:
- Line 1: The first line uses a level-2 heading ("## Verification Report") and
triggers MD041; change it to a top-level heading by replacing "## Verification
Report" with "# Verification Report" so the document starts with a single
top-level heading and satisfies the linter rule.
- Around line 49-53: Add a blank line after the "#### Quality gates" and "####
Targeted behavioral tests" heading lines so each heading is followed by an empty
line (fixing MD022); update the verify-report.md content around the "Quality
gates" and "Targeted behavioral tests" headings to include a single blank line
immediately after each heading.
---
Outside diff comments:
In `@clients/agent-runtime/src/gateway/webhook_dispatch.rs`:
- Around line 459-462: The code recomputes the clamped execution mode by calling
resolve_webhook_execution_mode again when building effective_config; instead
reuse the previously computed clamped_mode (the variable calculated earlier)
instead of calling resolve_webhook_execution_mode a second time. Update the
assignment to effective_config.agent.execution_mode to use clamped_mode (or its
equivalent) so the value is not recomputed and the two places remain consistent
(refer to resolve_webhook_execution_mode, effective_config, config.clone(), and
clamped_mode).
---
Duplicate comments:
In `@clients/agent-runtime/src/gateway/mod.rs`:
- Around line 6200-6209: Add a regression test exercising the failure-path for
SessionCommandOutcome::Failure: create a case similar to the existing
success-path but leave the slash command unresolved so the gateway returns HTTP
422 and an SSE error envelope; then assert resp.status() ==
StatusCode::UNPROCESSABLE_ENTITY, collect the body into body_str (like the
success test) and assert body_str.contains("event: error") and that it contains
the expected failure message (e.g., "No persisted session transcript is
available yet." or the specific failure text your code emits). Place this
alongside the existing assertions that check the success SSE (reusing resp,
collected, body_str variables) so the contract for failure emits is covered.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c10ea50-a464-41a9-ba7e-8194f43ac308
📒 Files selected for processing (9)
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/service.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.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). (1)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.md
**/*
⚙️ 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:
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdclients/agent-runtime/src/channels/mod.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/service.rs
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/mod.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/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/service.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/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/service.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/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/service.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/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.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/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/main.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
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
Keep startup path lean and avoid heavy initialization in command parsing flow
Files:
clients/agent-runtime/src/main.rs
🧠 Learnings (10)
📓 Common learnings
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
📚 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:
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdclients/agent-runtime/src/channels/mod.rsopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.mdclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/session_commands/service.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:
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.mdclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/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: 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:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/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: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/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: 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/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsclients/agent-runtime/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: 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/gateway/mod.rsclients/agent-runtime/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: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/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: 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/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: 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/main.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.md
[grammar] ~7-~7: Ensure spelling is correct
Context: ...vice shape, but replaces the narrow and stringly contracts with typed runtime models: 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/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] 49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 53-53: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (17)
clients/agent-runtime/src/session_commands/service.rs (4)
192-338: LGTM on/resumecaller-scope enforcement.The authorization flow correctly:
- Requires a caller scope key (lines 201-208)
- Uses scoped lookup via
get_resumable_session_for_scope(lines 211-225)- Returns
PermissionDeniedwhen the target session isn't in the caller's scope- Validates target state (suspended, not ended) before allowing resume
This tightens the memory boundary as intended by the PR objectives.
23-79: LGTM on/tldrtyped outcome conversion.The handler correctly:
- Returns
SessionCommandOutcomeviaoutcome_from_result- Uses structured failure kinds (
UnsupportedBackend,InvalidState, etc.)- Sanitizes storage errors through
map_storage_error
468-493: Good: Storage errors are sanitized before being exposed in failure messages.The
map_storage_errorhelper correctly usessanitize_storage_errorto prevent leaking sensitive path or credential information in user-facing messages while logging the full detail internally at error level.
1135-1149: Good test coverage for storage error sanitization.Test
resume_list_storage_failures_are_sanitizedproperly validates that:
- Sensitive paths like
/tmp/secret.dbare not exposed in the failure message- The sanitized message contains
"storage access denied"insteadclients/agent-runtime/src/gateway/webhook_dispatch.rs (3)
396-412: Good: Execution mode is now clamped before ingress evaluation.This correctly addresses the security concern where
evaluate_ingresscould observe a less restrictive mode than what would actually be enforced. The clamped mode is computed upfront and passed toCommandContext::for_webhook.
417-436: LGTM on SessionCommand outcome mapping.The new typed contract is correctly handled:
SessionCommandOutcome::Success→WebhookTerminalOutcome::CompletedSessionCommandOutcome::Failure→WebhookTerminalOutcome::Failed- Response text is sourced from the appropriate message field
1068-1101: Good regression test for slash command interception.Test
execute_intercepts_slash_session_commands_before_provider_executionvalidates that:
- Provider is not called when a slash command is detected
- Outcome is
Failedfor unsupported backend (TestMemory returns "test", not "sqlite")- Response text contains the expected error message
clients/agent-runtime/src/channels/mod.rs (2)
704-712: Execution mode is now preserved through ingress evaluation.
process_channel_messagenow threadsctx.config.agent.execution_modeintoCommandContext::for_channel, so channel slash commands see the same plan/standard mode as the rest of the turn. That closes the earlier authz drift from the hardcoded standard mode.Also applies to: 1854-1888
3240-3248: Plan-mode regression coverage looks solid.The added
ExecutionMode::Plantest, alongside the explicit standard-mode call sites, gives this channel path good coverage against slipping back to standard-only behavior. Based on learnings: ImplementChanneltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with testsAlso applies to: 3275-3283, 3290-3313
clients/agent-runtime/src/main.rs (4)
1401-1424: Previous issue addressed: flag normalization now occurs before slash-command fast path.The refactor correctly moves
execution_modenormalization andperipheralvalidation ahead of themaybe_handle_cli_session_commandcall. This ensures theCommandContextis built with the correct plan-mode state and unsupported overrides are rejected early.
1530-1565: Typed context construction and outcome handling looks correct.The refactor properly:
- Derives
CommandSessionSource::ExplicitvsGeneratedfromCORVUS_SESSION_IDpresence- Passes the normalized
config.agent.execution_modeintoCommandContext::for_cli- Maps
SessionCommandOutcome::Success→Ok(Some(...))andFailure→Err(...)preserving explicit error semantics
1579-1585: LGTM!Consistent with the agent command refactor.
apply_code_session_confignormalizesexecution_modebefore the slash command check.
1466-1485: LGTM!Clean separation: execution-mode normalization happens early for the slash-command path, while provider/model/temperature overrides apply only when continuing to full agent execution.
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/proposal.md (1)
1-65: Proposal document aligns with implementation.The spec accurately describes the typed
CommandContext,SessionCommandOutcome, and/resumecaller-scope enforcement that the code implements. The path reference on line 40 now correctly points to the archived location.openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.md (1)
17-17: Tasks path reference is now correct.Line 17 properly references the archived tasks.md path. The previously flagged issue has been resolved.
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/specs/sessions/spec.md (1)
42-49: The previously requested scenario for unverifiable caller scope denial has been added.This scenario now tests the authorization rule from line 13, covering absent/tampered/unverifiable caller scope cases and asserting explicit denial without broadening visibility.
openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/design.md (1)
166-171: The CommandCaller enum now includes the DerivedCliScope variant.The design documentation has been updated to match the implementation, showing all four variants as defined in the codebase.
| async fn canonical_outcome_early_response( | ||
| state: &AppState, | ||
| session_id: &str, | ||
| session_source: crate::session_commands::CommandSessionSource, | ||
| scrubbed_message: &str, | ||
| caller_token_hash: Option<&str>, | ||
| ) -> Option<(WebhookResponse, bool)> { | ||
| match crate::pre_execution::evaluate_ingress( | ||
| state.mem.as_ref(), | ||
| let context = crate::session_commands::CommandContext::for_gateway_http( | ||
| session_id, | ||
| scrubbed_message, | ||
| caller_token_hash, | ||
| ) | ||
| .await | ||
| session_source, | ||
| state.config.lock().agent.execution_mode, | ||
| caller_token_hash.map(str::to_string), |
There was a problem hiding this comment.
Pass the effective execution mode into the typed command context.
Both ingress contexts are built from the server default mode, so a request with {"execution_mode":"plan"} on a Standard server is downgraded to Standard before evaluate_ingress(). That drops the plan-mode bit from the new typed contract and can make requirement/permission checks run against the wrong mode. Thread the already-resolved mode through canonical_outcome_early_response() and for_gateway_stream(...), then reuse that same value for dispatcher execution.
♻️ Proposed fix
async fn canonical_outcome_early_response(
state: &AppState,
session_id: &str,
session_source: crate::session_commands::CommandSessionSource,
+ execution_mode: ExecutionMode,
scrubbed_message: &str,
caller_token_hash: Option<&str>,
) -> Option<(WebhookResponse, bool)> {
let context = crate::session_commands::CommandContext::for_gateway_http(
session_id,
session_source,
- state.config.lock().agent.execution_mode,
+ execution_mode,
caller_token_hash.map(str::to_string),
); let config = state.config.lock().clone();
let server_execution_mode = config.agent.execution_mode;
+ let resolved_execution_mode =
+ resolve_webhook_execution_mode(server_execution_mode, webhook_body.execution_mode);
let dispatcher_enabled = webhook_dispatcher_enabled(&config); if let Some((response, persist_idempotency)) = canonical_outcome_early_response(
&state,
&session_id,
http_source,
+ resolved_execution_mode,
&scrubbed_message,
token_hash.as_deref(),
) let ingress_context = crate::session_commands::CommandContext::for_gateway_stream(
&session_id,
match session_source {
webhook_dispatch::WebhookSessionSource::Explicit => {
crate::session_commands::CommandSessionSource::Explicit
}
webhook_dispatch::WebhookSessionSource::Generated => {
crate::session_commands::CommandSessionSource::Generated
}
},
- server_execution_mode,
+ resolve_webhook_execution_mode(server_execution_mode, webhook_body.execution_mode),
token_hash.clone(),
);Also applies to: 2217-2229
🤖 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 1764 - 1775, The
ingress contexts are currently built using the server default execution mode,
which causes requests that carried an explicit execution_mode (e.g., "plan") to
be downgraded before evaluate_ingress() and can make permission/requirement
checks incorrect; to fix, change canonical_outcome_early_response(...) to accept
the already-resolved effective execution mode as an argument and pass that mode
into CommandContext::for_gateway_http (and similarly thread the mode into
for_gateway_stream(...)), then ensure the same effective mode is used later when
constructing the dispatcher/execution context so evaluate_ingress() and
subsequent checks run against the correct mode rather than the server default.
| if context.chars().count() > MAX_CONTEXT_LENGTH { | ||
| let prefix: String = context.chars().take(MAX_CONTEXT_LENGTH).collect(); | ||
| // Find last occurrence of "\n- " to keep only complete entries | ||
| if let Some(last_entry_pos) = prefix.rfind("\n- ") { | ||
| context = prefix[..last_entry_pos].to_string(); | ||
| } else { | ||
| context = prefix; | ||
| let excess = context.chars().count() - MAX_CONTEXT_LENGTH; | ||
| context = context.chars().skip(excess).collect::<String>(); | ||
| // Ensure we don't start mid-entry: skip past the newline to land on the "- " of the list entry | ||
| if let Some(pos) = context.find("\n- ") { | ||
| let adjustment: String = context.chars().skip(pos + 1).collect(); | ||
| if adjustment.starts_with("- ") { | ||
| context = adjustment; | ||
| } | ||
| } |
There was a problem hiding this comment.
Truncation logic may drop all content if no "\n- " is found.
When enforcing MAX_CONTEXT_LENGTH, the code skips excess characters from the start, then looks for "\n- " to land on a list boundary. If no such pattern exists in the truncated string (e.g., context is mostly summary with no list entries), the original truncated content is silently discarded and context remains unchanged—potentially still exceeding the limit or becoming empty.
Consider adding a fallback that keeps the truncated string when no list boundary is found:
Proposed fix
if context.chars().count() > MAX_CONTEXT_LENGTH {
let excess = context.chars().count() - MAX_CONTEXT_LENGTH;
- context = context.chars().skip(excess).collect::<String>();
- // Ensure we don't start mid-entry: skip past the newline to land on the "- " of the list entry
+ let truncated: String = context.chars().skip(excess).collect();
+ // Ensure we don't start mid-entry: skip past the newline to land on the "- " of the list entry
- if let Some(pos) = context.find("\n- ") {
- let adjustment: String = context.chars().skip(pos + 1).collect();
+ context = if let Some(pos) = truncated.find("\n- ") {
+ let adjustment: String = truncated.chars().skip(pos + 1).collect();
if adjustment.starts_with("- ") {
- context = adjustment;
+ adjustment
+ } else {
+ truncated
}
- }
+ } else {
+ truncated
+ };
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/session_commands/service.rs` around lines 576 -
585, The truncation branch for context currently discards the truncated string
when no "\n- " boundary is found; update the logic around context, excess, and
the find("\n- ") check so you first compute the truncated candidate (e.g., let
truncated = context.chars().skip(excess).collect::<String>()), then attempt to
locate "\n- " inside that truncated candidate and only replace context with the
adjusted slice if the boundary is found and starts with "- "; otherwise retain
the truncated candidate as the fallback (ensuring it is non-empty and still
within MAX_CONTEXT_LENGTH). Apply this change in the block handling
MAX_CONTEXT_LENGTH (the variables/functions: context, excess,
MAX_CONTEXT_LENGTH, adjustment, find("\n- ")) so you never silently lose content
when no list boundary exists.
| @@ -0,0 +1,91 @@ | |||
| ## Verification Report | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: First line should be a top-level heading.
Static analysis flagged MD041. Consider changing line 1 from ## Verification Report to # Verification Report for proper document structure.
Proposed fix
-## Verification Report
+# Verification Report📝 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.
| ## Verification Report | |
| # Verification Report |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.md`
at line 1, The first line uses a level-2 heading ("## Verification Report") and
triggers MD041; change it to a top-level heading by replacing "## Verification
Report" with "# Verification Report" so the document starts with a single
top-level heading and satisfies the linter rule.
| #### Quality gates | ||
| - `cargo fmt --all -- --check` ✅ passed | ||
| - `cargo clippy --manifest-path Cargo.toml --all-targets -- -D warnings` ✅ passed | ||
|
|
||
| #### Targeted behavioral tests |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Add blank lines after headings for markdown compliance.
MD022 flagged missing blank lines below headings at lines 49 and 53.
Proposed fix
#### Quality gates
+
- `cargo fmt --all -- --check` ✅ passed #### Targeted behavioral tests
+
- `cargo test --manifest-path Cargo.toml legacy_webhook_preview_intercepts_slash_session_commands` ✅📝 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.
| #### Quality gates | |
| - `cargo fmt --all -- --check` ✅ passed | |
| - `cargo clippy --manifest-path Cargo.toml --all-targets -- -D warnings` ✅ passed | |
| #### Targeted behavioral tests | |
| #### Quality gates | |
| - `cargo fmt --all -- --check` ✅ passed | |
| - `cargo clippy --manifest-path Cargo.toml --all-targets -- -D warnings` ✅ passed | |
| #### Targeted behavioral tests | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 53-53: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/verify-report.md`
around lines 49 - 53, Add a blank line after the "#### Quality gates" and "####
Targeted behavioral tests" heading lines so each heading is followed by an empty
line (fixing MD022); update the verify-report.md content around the "Quality
gates" and "Targeted behavioral tests" headings to include a single blank line
immediately after each heading.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
1 participant
Related Issues
Closes #540
Refs #527
Refs #541
Summary
This PR adds the typed slash command execution contract for
clients/agent-runtime.It introduces a richer
CommandContext, typed slash command requirement metadata, and a non-lossy internal slash command outcome model while preserving the existing external transport envelopes.It also tightens
/resumecaller-scope enforcement through the memory boundary, updates the ingress adapters to carry the typed contract end to end, and syncs the completed OpenSpec artifacts for the change.Tested Information
Validated with the Rust checks relevant to this change:
cargo fmt --all -- --checkcargo clippy --manifest-path clients/agent-runtime/Cargo.toml --all-targets -- -D warningsReviewers may want to focus on:
session_commands/types.rssession_commands/service.rspre_execution/mod.rsDocumentation Impact
openspec/specs/slash-command-registry/spec.mdopenspec/specs/sessions/spec.mdopenspec/changes/archive/2026-04-15-slash-command-context-permissions-result-contract/Breaking Changes
None.
Checklist