feat(agent-runtime): unify slash command transport handling (#541)#549
Conversation
Change pre-execution blocking responses to use HTTP 200 for SSE endpoints instead of 403/408 so the server-sent events contract is preserved. Add two tokio tests verifying the /web/chat/stream SSE output for approval_required and timeout blocking outcomes (checks event: error, expected error codes/messages, and that the provider is not executed). These tests enable webhook_dispatcher in the temp config to exercise the webhook dispatcher code path.
|
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 20 minutes and 46 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 selected for processing (10)
📝 WalkthroughWalkthroughThis PR implements "slash command transport parity" by introducing a shared, transport-neutral adapter layer that normalizes ingress outcomes across CLI, gateway HTTP/SSE, webhook, and channel transports. All five entry points now route recognized slash commands through a unified Changes
Sequence Diagram(s)sequenceDiagram
participant Transport as CLI/Gateway/Webhook/Channel
participant PreExec as pre_execution
participant Adapter as adapt_handled_ingress()
participant Outcome as HandledIngress
participant Dispatch as Provider/Response
Transport->>PreExec: evaluate_ingress(CommandContext)
PreExec-->>Adapter: IngressDecision
Adapter->>Adapter: classify_session_command_failure()
Adapter->>Outcome: HandledIngress::Handled(HandledIngressOutcome)
Outcome-->>Transport: SessionCommandSuccess/Failure/Blocking
alt Session Command Recognized
Transport->>Dispatch: transport-specific envelope wrapping
else Not Handled
Transport->>Dispatch: continue to provider execution
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-17 to 2026-04-17 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
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/main.rs (1)
3422-3444: 🧹 Nitpick | 🔵 TrivialTest gap: no regression coverage for the
SessionCommandSuccessarm of the newHandledIngressmatch.The existing tests cover the failure path (
/tldr→require sqlite) and theNotHandledfall-through (/resume-later), but no test exercisesHandledIngressOutcome::SessionCommandSuccess → Ok(Some(message))in the CLI. Given this PR's whole point is unifying handled-slash adaptation across transports, a minimal tokio test that submits a recognized command yielding a success outcome and asserts the returnedSome(message)would lock in the contract.As per coding guidelines: "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/main.rs` around lines 3422 - 3444, Add a tokio test that covers the SessionCommandSuccess arm of the HandledIngress match by calling maybe_handle_cli_session_command with a recognized slash command that should succeed and asserting the function returns Ok(Some(message)); locate and mirror existing tests (e.g., cli_session_commands_are_handled_before_agent_execution and cli_unknown_slash_like_input_falls_through) but use an input that triggers HandledIngressOutcome::SessionCommandSuccess and verify the returned Some string contains the expected success message.
🤖 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 1791-1796: The branch matching
crate::pre_execution::HandledIngress::Handled with
crate::pre_execution::HandledIngressOutcome::SessionCommandFailure currently
discards the failure.class and hard-codes "session_command_failed"; instead, map
the incoming failure.class to a stable external error code (e.g., via a helper
function map_session_command_failure_class or a match on the enum variants) and
use that mapped code in the response payload where "session_command_failed" is
now used; also update the SSE emission path that reports session command
failures to reuse the same mapping helper so permission-denied, missing-session,
and unsupported-backend remain distinct and transport-neutral.
- Around line 2279-2293: The SessionCommandFailure branch in
handle_chat_stream() currently returns StatusCode::UNPROCESSABLE_ENTITY which
breaks the SSE contract; change the StatusCode returned for
crate::pre_execution::HandledIngressOutcome::SessionCommandFailure to
StatusCode::OK so the handler keeps streaming an SSE "error" event instead of
sending a non-200 response, and update the test
web_chat_stream_returns_slash_session_error_sse_without_provider_execution to
expect an SSE error payload on HTTP 200 rather than a 422 status. Ensure you
only modify the StatusCode in that match arm and adjust the assertion in the
named test to check the SSE body/event contents while asserting status == 200.
In `@clients/agent-runtime/src/gateway/webhook_dispatch.rs`:
- Around line 980-1013: The test helper seed_resumable_session sets suspended_at
to the free-form string "now", which is not lexically sortable; update the
apply_session_state_patch call in seed_resumable_session so suspended_at uses an
RFC 3339 timestamp (either a fixed RFC3339 literal for a deterministic fixture
like "2026-04-17T00:00:00Z" or generate one at runtime via
chrono::Utc::now().to_rfc3339()) instead of "now" (look for the
SessionFieldPatch::Set("now".to_string()) assignment).
In `@clients/agent-runtime/src/main.rs`:
- Around line 1555-1557: The code currently discards the computed
SessionCommandFailureClass by using the wildcard in the match arm for
HandledIngress::Handled(HandledIngressOutcome::SessionCommandFailure { failure,
.. }) and always returns Err(anyhow!("{}", failure.message)), flattening
PermissionDenied into a generic failure; update that match arm to inspect
failure.class (the SessionCommandFailureClass enum) and branch on its variants
(e.g., SessionCommandFailureClass::PermissionDenied vs others) so the CLI
returns or logs a distinct outcome for authorization-denied failures (different
exit signaling or a message prefix) while still including failure.message for
diagnostics; modify the match in main.rs to pattern-match or if/else on
failure.class and map each class to the appropriate error wrapper or exit code
instead of the single anyhow!("...") response.
- Around line 1549-1562: The double-evaluation happens because
maybe_handle_cli_session_command calls crate::pre_execution::evaluate_ingress
which performs a fallback call to crate::pre_execution::evaluate(...) for
non-slash messages, and later handle_agent_command/collect_unified_loop_result
calls evaluate(...) again; fix by either short-circuiting before calling
evaluate_ingress from maybe_handle_cli_session_command (reintroduce a
lightweight recognizes()/registry.dispatch() pre-check there) or by changing
evaluate_ingress to accept a flag (e.g., skip_blocking_fallback) or return the
canonical IngressDecision+cached evaluate result so handle_agent_command can
reuse it; update maybe_handle_cli_session_command,
crate::pre_execution::evaluate_ingress, and the call sites in
handle_agent_command/collect_unified_loop_result to pass/use the flag or cached
outcome to ensure evaluate(...) is called at most once per CLI turn.
In `@clients/agent-runtime/src/pre_execution/mod.rs`:
- Around line 9-12: Remove the unnecessary #[allow(unused_imports)] attribute
applied to the pub re-export block so lints reflect actual usage: delete the
#[allow(unused_imports)] line above the pub use session_command_adapter::{
adapt_handled_ingress, HandledIngress, HandledIngressOutcome,
SessionCommandFailureClass } export and leave the pub use intact
(SessionCommandFailureClass can remain exported as part of the public API).
In `@clients/agent-runtime/src/pre_execution/session_command_adapter.rs`:
- Around line 48-63: Change the classifier to take the C-like enum by value
instead of by reference: update the function signature of
classify_session_command_failure to accept SessionCommandFailureKind (not
&SessionCommandFailureKind) and adjust all call sites that currently pass
&failure.kind to pass failure.kind directly (or remove the dereference) so
callers no longer need to borrow the enum; keep the match arms unchanged.
- Around line 34-44: The match uses the fully-qualified type
crate::session_commands::SessionCommandOutcome twice; update the top-level use
that currently imports SessionCommandFailure, SessionCommandFailureKind, and
SessionCommandSuccess to also import SessionCommandOutcome (e.g. add
SessionCommandOutcome to the same use list) and then replace the fully-qualified
occurrences in the match arms with the shorter SessionCommandOutcome identifier
to keep imports consistent and reduce noise.
In
`@openspec/changes/archive/2026-04-17-slash-command-transport-parity/exploration.md`:
- Line 3: Add blank lines before and after each Markdown heading to satisfy
MD022: insert an empty line above and below the headings "### Current State" and
the other headings located at the same positions (the headings at lines
referenced in the review), i.e., ensure there is one blank line immediately
before and one blank line immediately after each heading (apply the same change
for the headings at lines 18, 26, 37, 46, and 51).
- Line 1: The first heading in the document "Exploration: issue `#541` slash
command transport parity" is level 2 (starts with "##") but should be level 1
for markdownlint; edit the heading line to use a single "#" instead of "##" so
it becomes "# Exploration: issue `#541` slash command transport parity".
In
`@openspec/changes/archive/2026-04-17-slash-command-transport-parity/proposal.md`:
- Around line 10-18: The markdown headings "### In Scope" and "### Out of Scope"
lack a blank line before their following lists, triggering MD022; edit the
proposal.md content around the two headings and insert a single blank line after
each "### In Scope" and "### Out of Scope" heading so the bullet lists are
separated from the headings and satisfy markdownlint.
In
`@openspec/changes/archive/2026-04-17-slash-command-transport-parity/specs/slash-command-registry/spec.md`:
- Line 37: The RFC 2119 phrasing "no transport MUST require a separate
pre-dispatch recognition branch to determine that fallthrough" is ambiguous;
change the wording to an explicit prohibition such as "A transport MUST NOT
require a separate pre-dispatch recognition branch to determine fallthrough" (or
"Transports MUST NOT require...") wherever the original phrase appears (e.g.,
the phrase at the current occurrence and the similar occurrence around line 70)
so the requirement unambiguously forbids transport implementations from adding a
separate pre-dispatch recognition branch.
In
`@openspec/changes/archive/2026-04-17-slash-command-transport-parity/verify-report.md`:
- Line 93: The file ends with the "## Verdict" heading but is missing a trailing
newline which triggers markdownlint MD047; open verify-report.md and add a
single newline character at the end of the file (i.e., ensure the file ends with
an empty line after the "## Verdict" heading) so the markdown linter no longer
reports MD047.
In `@openspec/specs/slash-command-registry/spec.md`:
- Around line 210-211: The Scenario headings in the spec are split across two
lines causing a heading plus stray paragraph; fix by merging each split heading
into a single ATX line: change the two-line headings into single lines exactly
like "#### Scenario: Unknown slash-like input falls through consistently without
transport-local recognition branches" and "#### Scenario: …while outward
wrappers stay transport-specific" (remove the newline between the fragments) so
they render as one heading each and satisfy markdownlint MD022.
---
Outside diff comments:
In `@clients/agent-runtime/src/main.rs`:
- Around line 3422-3444: Add a tokio test that covers the SessionCommandSuccess
arm of the HandledIngress match by calling maybe_handle_cli_session_command with
a recognized slash command that should succeed and asserting the function
returns Ok(Some(message)); locate and mirror existing tests (e.g.,
cli_session_commands_are_handled_before_agent_execution and
cli_unknown_slash_like_input_falls_through) but use an input that triggers
HandledIngressOutcome::SessionCommandSuccess and verify the returned Some string
contains the expected success message.
🪄 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: 7492b6dc-8f8f-40c0-872c-b5a54100f1a2
📒 Files selected for processing (14)
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/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.rsopenspec/changes/archive/2026-04-17-slash-command-transport-parity/design.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/exploration.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/proposal.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/state.yamlopenspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/verify-report.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-17-slash-command-transport-parity/state.yamlopenspec/changes/archive/2026-04-17-slash-command-transport-parity/verify-report.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/exploration.mdclients/agent-runtime/src/channels/mod.rsopenspec/changes/archive/2026-04-17-slash-command-transport-parity/proposal.mdclients/agent-runtime/src/gateway/webhook_dispatch.rsopenspec/changes/archive/2026-04-17-slash-command-transport-parity/design.mdclients/agent-runtime/src/main.rsopenspec/specs/slash-command-registry/spec.mdclients/agent-runtime/src/pre_execution/mod.rsopenspec/changes/archive/2026-04-17-slash-command-transport-parity/specs/slash-command-registry/spec.mdclients/agent-runtime/src/pre_execution/session_command_adapter.rsclients/agent-runtime/src/gateway/mod.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-17-slash-command-transport-parity/verify-report.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/exploration.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/proposal.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/design.mdopenspec/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/specs/slash-command-registry/spec.md
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/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.rsclients/agent-runtime/src/gateway/mod.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/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.rsclients/agent-runtime/src/gateway/mod.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/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.rsclients/agent-runtime/src/gateway/mod.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 (9)
📓 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/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-17-slash-command-transport-parity/verify-report.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.mdclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsopenspec/changes/archive/2026-04-17-slash-command-transport-parity/design.mdclients/agent-runtime/src/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.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/**/*.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:
openspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.mdclients/agent-runtime/src/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.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-17-slash-command-transport-parity/tasks.mdclients/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: 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-17-slash-command-transport-parity/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/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
openspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.mdclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/webhook_dispatch.rsopenspec/changes/archive/2026-04-17-slash-command-transport-parity/design.mdclients/agent-runtime/src/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.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/{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/agent-runtime/src/main.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.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/{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/pre_execution/mod.rsclients/agent-runtime/src/pre_execution/session_command_adapter.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/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-17-slash-command-transport-parity/design.md
[style] ~130-~130: To form a complete sentence, be sure to include a subject.
Context: ...contract. - SessionCommandFailureKind should be reused as-is unless implementation u...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-17-slash-command-transport-parity/verify-report.md
[warning] 93-93: Files should end with a single newline character
(MD047, single-trailing-newline)
openspec/changes/archive/2026-04-17-slash-command-transport-parity/exploration.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 26-26: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 46-46: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 51-51: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-04-17-slash-command-transport-parity/proposal.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/specs/slash-command-registry/spec.md
[warning] 210-210: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 222-222: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (8)
openspec/changes/archive/2026-04-17-slash-command-transport-parity/exploration.md (1)
1-52: LGTM: Exploration document accurately describes the implemented solution.The document's technical content aligns precisely with the actual implementation shown in the context snippets:
- The shared
pre_execution::evaluate_ingress(...)seam and transport-neutraladapt_handled_ingress(...)adapter match context snippet 1.- The five transport paths (CLI, Gateway HTTP/SSE, webhook dispatcher, channel-backed) are accurately described and match the PR's unified approach.
- The recommended Approach 1 correctly predicted the
HandledIngressenum structure and permission-denied vs generic failure classification.- Affected areas, risks, and dependencies (
#539/#540) align with PR objectives.This exploration provides clear rationale for the transport parity changes and serves as solid design documentation.
clients/agent-runtime/src/channels/mod.rs (2)
1880-1927: Shared ingress adaptation preserves channel behavior.This keeps the transport-neutral decision path centralized while still preserving the channel-specific blocking text mapping locally. Nice containment of the parity change.
3251-3428: Good regression coverage for scoped/resume.The helper setup stays focused, and the two new tests pin both the authorized resume path and the permission-denied path that matter most for transport parity.
clients/agent-runtime/src/gateway/webhook_dispatch.rs (2)
414-461: LGTM — handled-ingress adapter flow is correctly wired.Control flow matches the shared contract: success →
Completedcarryingsuccess.message, failure →Failedcarryingfailure.message, blocking → existingmap_canonical_result,NotHandled→ fallthrough to provider execution. Theclass: _destructure in the failure arm is intentional (the webhook transport collapses permission/generic intoFailed, and the class is preserved upstream in the adapter for transports that need it).
1173-1249: Good regression coverage for authorized resume + permission-denied parity.Both tests assert
provider_impl.calls == 0(provider never invoked) and pin exactresponse_textincluding the[session:session-target]prefix, which locks in the webhook transport's short-circuit contract. The authorized/denied split is wired correctly: seed tokencaller-hashvsowner-hashwith the request carryingcaller-hashcleanly exercises both branches through the shared adapter.One nit: both tests reuse
session_id: "session-control"with targetsession-target— that's actually nice (verifies resume target differs from active session), but adding a brief comment would make that intent obvious to future readers.openspec/changes/archive/2026-04-17-slash-command-transport-parity/tasks.md (1)
1-27: LGTM — task checklist cleanly mirrors the phased adapter rollout.clients/agent-runtime/src/pre_execution/mod.rs (1)
319-400: Adapter unit tests LGTM — classification coverage is complete.Continue→NotHandled, Success preservation, permission-kind collapse (MissingCallerScope + PermissionDenied →
PermissionDenied), generic failure →Failed, and Blocking preservation are all covered. The iteration over both permission kinds inadapt_handled_ingress_classifies_permission_failuresis a nice touch.openspec/changes/archive/2026-04-17-slash-command-transport-parity/design.md (1)
1-168: LGTM — design doc accurately describes the implemented adapter seam.Failure-mapping table, interface sketch, and rollback plan all line up with the landed code in
session_command_adapter.rsand the four transport call sites.
|
|
||
| 1. Add one CLI success regression, one SSE permission-denied regression, and—if a slash command can legitimately yield blocking at runtime—one transport-level blocking regression to close the remaining parity-proof gaps. | ||
|
|
||
| ## Verdict |
There was a problem hiding this comment.
Add trailing newline (markdownlint MD047).
📝 Proposed fix
-PASS WITH WARNINGS
+PASS WITH WARNINGS
+🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 93-93: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-17-slash-command-transport-parity/verify-report.md`
at line 93, The file ends with the "## Verdict" heading but is missing a
trailing newline which triggers markdownlint MD047; open verify-report.md and
add a single newline character at the end of the file (i.e., ensure the file
ends with an empty line after the "## Verdict" heading) so the markdown linter
no longer reports MD047.
Deploying corvus with
|
| Latest commit: |
29bb6dd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f89c7c78.corvus-42x.pages.dev |
| Branch Preview URL: | https://feature-dallay-541-slash-tra.corvus-42x.pages.dev |
1 participant
Related Issues
Fixes #541
Summary
This PR unifies slash command transport handling through the shared pre-execution adapter and preserves transport-specific external envelopes.
It also fixes a stream contract regression introduced during the #541 work: dispatcher-backed
/web/chat/streamblocking outcomes now stay on HTTP 200 and emitevent: errorpayloads in the SSE body instead of switching the response status to 403/408.Tested Information
Validated with focused runtime checks and the repository pre-push hook:
cargo test --manifest-path "clients/agent-runtime/Cargo.toml" web_chat_stream_preserves_sse_contract_for_approval_required_blocking -- --nocapturecargo test --manifest-path "clients/agent-runtime/Cargo.toml" web_chat_stream_preserves_sse_contract_for_timeout_blocking -- --nocapturecargo fmt --manifest-path "clients/agent-runtime/Cargo.toml" --all -- --check3562tests passed)Reviewers should focus on SSE behavior for blocking outcomes in
clients/agent-runtime/src/gateway/mod.rs.Documentation Impact
openspec/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-17-slash-command-transport-parity/Breaking Changes
None.
Checklist