Skip to content

refactor(agent-runtime): centralize canonical pre-execution blocking logic#168

Merged
yacosta738 merged 4 commits into
mainfrom
feature/dallay-139-centralize-pre-execution-flow
Mar 8, 2026
Merged

refactor(agent-runtime): centralize canonical pre-execution blocking logic#168
yacosta738 merged 4 commits into
mainfrom
feature/dallay-139-centralize-pre-execution-flow

Conversation

@yacosta738
Copy link
Copy Markdown
Contributor

This pull request refactors the handling of pre-execution checks (such as approval requirements, timeouts, and fallback responses) for agent commands by introducing a new pre_execution module. The logic for evaluating and classifying blocking outcomes is now centralized, leading to cleaner and more maintainable code across the codebase. Environment variable handling and test configuration are also unified in the new module.

Key changes include:

Introduction of the pre_execution module:

  • Added a new pre_execution module (clients/agent-runtime/src/pre_execution/mod.rs) that centralizes logic for evaluating canonical outcomes, checking environment variables, and classifying blocking outcomes. This includes the new BlockingOutcome enum and related helper functions. [1] [2] [3]

Refactoring of pre-execution logic usage:

  • Updated handle_canonical_blocking_outcome, canonical_outcome_early_response, collect_unified_loop_result, and handle_agent_command to use the new pre_execution::evaluate and pre_execution::classify_blocking functions, replacing duplicated logic and direct environment variable checks. [1] [2] [3] [4] [5] [6]

Improved blocking outcome classification:

  • All blocking outcomes (approval required, timeout, fallback) are now handled through a single classification function, ensuring consistent behavior and simplifying the code paths for early exits and error handling. [1] [2] [3] [4]

Testing and configuration enhancements:

  • Added unit tests for the new blocking outcome classification logic, and unified test trigger configuration in the new module.

These changes make the codebase more modular and maintainable by isolating pre-execution logic and reducing duplication.

Closes: #159

…logic

Extract shared approval/timeout/fallback evaluation into a pre_execution module and reuse it across CLI, channels, and gateway to keep fail-closed behavior and blocking semantics consistent.
@linear
Copy link
Copy Markdown

linear Bot commented Mar 8, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 8, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Centralizes approval, timeout, and fallback checks in a new public pre_execution module and updates channels, gateway, and CLI entrypoints to call pre_execution::evaluate() and pre_execution::classify_blocking() to short‑circuit or continue execution based on a unified BlockingOutcome.

Changes

Cohort / File(s) Summary
Pre-execution abstraction module
clients/agent-runtime/src/pre_execution/mod.rs
New public module exposing BlockingOutcome enum, evaluate(session_id, prompt) async wrapper around unified_entrypoint::run_canonical_outcome, classify_blocking(&CanonicalOutcome), env/test helpers, and unit tests.
Gateway HTTP response handling
clients/agent-runtime/src/gateway/mod.rs
canonical_outcome_early_response now returns Option<(WebhookResponse, bool)>. Replaced inline canonical checks with pre_execution::evaluate() + classify_blocking(). Early-returns structured responses for ApprovalRequired (403), TimeoutAborted (408, idempotency preserved), or Fallback (200). Call sites updated to respect returned idempotency flag.
Channel execution flow
clients/agent-runtime/src/channels/mod.rs
Replaces previous blocking handling with pre_execution::evaluate() + classify_blocking(). Builds and sends user-facing messages for ApprovalRequired, TimeoutAborted, and Fallback and short‑circuits processing by returning Some(()) on blocking outcomes.
CLI command handling / main loop
clients/agent-runtime/src/main.rs
collect_unified_loop_result() and handle_agent_command() now call pre_execution::evaluate() and use classify_blocking() to map blocking variants to errors/messages instead of prior per-field canonical checks; added mod pre_execution import.
Public API surface
clients/agent-runtime/src/lib.rs
Exports the new pre_execution module (pub mod pre_execution).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant PreExec as pre_execution
    participant Unified as unified_entrypoint
    participant Handler

    Client->>PreExec: evaluate(session_id, prompt)
    PreExec->>Unified: run_canonical_outcome(..., approval_flag, cfg)
    Unified-->>PreExec: CanonicalOutcome
    PreExec->>PreExec: classify_blocking(outcome)
    alt ApprovalRequired
        PreExec-->>Handler: ApprovalRequired { tool }
        Handler-->>Client: 403 Denial JSON
    else TimeoutAborted
        PreExec-->>Handler: TimeoutAborted
        Handler-->>Client: 408 Request Timeout (idempotency preserved)
    else Fallback
        PreExec-->>Handler: Fallback { response }
        Handler-->>Client: 200 Sanitized fallback
    else None
        PreExec-->>Handler: No blocking -> continue
        Handler-->>Client: proceed with normal execution
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title follows Conventional Commit style with 'refactor' prefix but exceeds the 72-character limit at 74 characters. Shorten the title to 72 characters or less; consider: 'refactor(agent-runtime): centralize pre-execution blocking logic' (66 chars).
Docstring Coverage ⚠️ Warning Docstring coverage is 52.94% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description comprehensively covers the changes, includes Related Issues section, Summary, Testing Information, and Checklist; it fully aligns with the template.
Linked Issues check ✅ Passed The PR implements all acceptance criteria from #159: a shared pre_execution module, consistent approval/timeout/fallback handling across channels/gateway/CLI, and regression tests for blocking outcomes.
Out of Scope Changes check ✅ Passed All changes are tightly scoped to the pre_execution refactor: module introduction, call-site updates, and blocking outcome classification. No unrelated functionality modifications detected.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/dallay-139-centralize-pre-execution-flow

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.

❤️ Share

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 8, 2026

✅ Contributor Report

User: @yacosta738
Status: Passed (12/13 metrics passed)

Metric Description Value Threshold Status
PR Merge Rate PRs merged vs closed 88% >= 30%
Repo Quality Repos with ≥100 stars 0 >= 0
Positive Reactions Positive reactions received 9 >= 1
Negative Reactions Negative reactions received 0 <= 5
Account Age GitHub account age 3054 days >= 30 days
Activity Consistency Regular activity over time 108% >= 0%
Issue Engagement Issues with community engagement 0 >= 0
Code Reviews Code reviews given to others 395 >= 0
Merger Diversity Unique maintainers who merged PRs 2 >= 0
Repo History Merge Rate Merge rate in this repo 90% >= 0%
Repo History Min PRs Previous PRs in this repo 124 >= 0
Profile Completeness Profile richness (bio, followers) 90 >= 0
Suspicious Patterns Spam-like activity detection 1 N/A

Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-08 to 2026-03-08

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 1563-1570: The Fallback branch returns the raw `response` (in
`BlockingOutcome::Fallback { response }`) without any redaction; update this
branch to pass `response` through the same sanitization/scrubbing utility used
for normal webhook/SSE responses before building the JSON body so
sensitive/echoed prompt content is redacted—i.e., call the existing sanitizer
(the function used for SSE/webhook payloads) on `response`, then include the
sanitized text in the `body` instead of the raw `response`.

In `@clients/agent-runtime/src/pre_execution/mod.rs`:
- Around line 47-102: Add two edge-case tests for classify_blocking: one where a
CanonicalOutcome has approval_required=None, timeout_aborted=false, and
fallback_response=None and assert classify_blocking returns None; and another
where timeout_aborted=true and fallback_response=Some(...) (with
approval_required=None) and assert classify_blocking returns
Some(BlockingOutcome::TimeoutAborted) to confirm timeout precedence over
fallback. Place these new tests alongside the existing test functions in the
tests module referencing CanonicalOutcome, classify_blocking, and
BlockingOutcome.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fb443749-aa69-4c73-b9e8-6a9d943bec0d

📥 Commits

Reviewing files that changed from the base of the PR and between 8ab302d and ea4fce7.

📒 Files selected for processing (5)
  • clients/agent-runtime/src/channels/mod.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/lib.rs
  • clients/agent-runtime/src/main.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: pr-checks
  • GitHub Check: sonar
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
clients/agent-runtime/src/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency

Files:

  • clients/agent-runtime/src/lib.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/agent-runtime/src/main.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/channels/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, and cargo test for code validation, or document which checks were skipped and why

Files:

  • clients/agent-runtime/src/lib.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/agent-runtime/src/main.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/channels/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/lib.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/agent-runtime/src/main.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/channels/mod.rs
**/*

⚙️ CodeRabbit configuration file

**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.

Files:

  • clients/agent-runtime/src/lib.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/agent-runtime/src/main.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/channels/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
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/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/mod.rs
clients/agent-runtime/src/channels/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Implement Channel trait in src/channels/ with consistent send, listen, and health_check semantics and cover auth/allowlist/health behavior with tests

Files:

  • clients/agent-runtime/src/channels/mod.rs
🧠 Learnings (9)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths

Applied to files:

  • clients/agent-runtime/src/lib.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/agent-runtime/src/main.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/channels/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/lib.rs
  • 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/lib.rs
  • 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/**/*.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.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
  • clients/agent-runtime/src/channels/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,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.rs
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/channels/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/main.rs
  • clients/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:

  • clients/agent-runtime/src/channels/mod.rs
🔇 Additional comments (9)
clients/agent-runtime/src/lib.rs (1)

63-63: LGTM!

Module declaration follows the established alphabetical ordering and cleanly exposes the new pre_execution API.

clients/agent-runtime/src/pre_execution/mod.rs (3)

1-16: LGTM!

The BlockingOutcome enum cleanly models the three blocking states. Environment helpers default to deny (approval not granted) which maintains secure-by-default posture.


18-28: LGTM!

Clean delegation to run_canonical_outcome with proper parameter ordering. The async function keeps the request path non-blocking.


30-45: LGTM!

Priority ordering (approval → timeout → fallback) ensures fail-closed behavior. The clones are necessary since the function borrows CanonicalOutcome but must produce owned BlockingOutcome values.

clients/agent-runtime/src/channels/mod.rs (1)

616-645: LGTM!

Clean integration with the centralized pre_execution module. The session-prefixed messages provide traceability, and the early-return pattern maintains fail-closed security posture.

clients/agent-runtime/src/main.rs (3)

64-64: LGTM!

Module declaration follows established ordering.


539-539: LGTM!

Delegates to centralized pre_execution::evaluate, consistent with the refactor goals.


888-909: LGTM!

Blocking outcomes result in explicit errors with session-prefixed messages. This preserves the CLI contract (explicit errors over silent fallback) and maintains fail-closed security.

clients/agent-runtime/src/gateway/mod.rs (1)

1532-1562: Centralized blocking classification looks good.

Routing approval and timeout exits through pre_execution::classify_blocking keeps the gateway fail-closed and aligned with the shared pre-execution flow. Based on learnings: Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable.

Comment thread clients/agent-runtime/src/gateway/mod.rs Outdated
Comment thread clients/agent-runtime/src/pre_execution/mod.rs
Scrub fallback responses in gateway canonical early-exit payloads and add edge-case coverage for classify_blocking none/timeout precedence behavior.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Mar 8, 2026

Deploying corvus with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4247887
Status: ✅  Deploy successful!
Preview URL: https://0ccaf545.corvus-42x.pages.dev
Branch Preview URL: https://feature-dallay-139-centraliz.corvus-42x.pages.dev

View logs

@sentry
Copy link
Copy Markdown

sentry Bot commented Mar 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 1534-1561: The pre-execution TimeoutAborted branch
(crate::pre_execution::BlockingOutcome::TimeoutAborted) currently returns a 408
while handle_webhook already records X-Idempotency-Key earlier, causing the key
to be consumed even though no provider call ran; fix by ensuring idempotency
keys are only persisted after pre-execution checks succeed (i.e., move the
X-Idempotency-Key persistence out of the early portion of handle_webhook and
perform it after classify_blocking returns a non-TimeoutAborted outcome), or
alternatively change the TimeoutAborted return path to a special non-persisting
response variant that handle_webhook recognizes and therefore does not store the
idempotency key; update handle_webhook and the BlockingOutcome handling
accordingly so TimeoutAborted responses remain retryable and do not mark the key
as "duplicate."

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ee08a57-addd-4377-a6f9-101b107786a7

📥 Commits

Reviewing files that changed from the base of the PR and between ea4fce7 and 57dfdc8.

📒 Files selected for processing (2)
  • clients/agent-runtime/src/gateway/mod.rs
  • clients/agent-runtime/src/pre_execution/mod.rs
📜 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). (4)
  • GitHub Check: pr-checks
  • GitHub Check: sonar
  • GitHub Check: pr-checks
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (6)
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/pre_execution/mod.rs
  • clients/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, and cargo test for code validation, or document which checks were skipped and why

Files:

  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/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/pre_execution/mod.rs
  • clients/agent-runtime/src/gateway/mod.rs
**/*

⚙️ CodeRabbit configuration file

**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.

Files:

  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/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/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/mod.rs
🧠 Learnings (4)
📓 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/{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
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
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths

Applied to files:

  • clients/agent-runtime/src/pre_execution/mod.rs
  • clients/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/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/gateway/mod.rs
🔇 Additional comments (1)
clients/agent-runtime/src/pre_execution/mod.rs (1)

18-45: Centralized blocking precedence looks solid.

The shared evaluate/classify_blocking split makes the approval → timeout → fallback ordering explicit, and the added tests lock that contract down cleanly.

Also applies to: 47-131

Comment thread clients/agent-runtime/src/gateway/mod.rs Outdated
Defer idempotency key persistence until after canonical pre-execution, and skip persistence for timeout-aborted outcomes so retries are not marked as duplicates. Add gateway regression coverage for timeout idempotency behavior.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Mar 8, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3bb08b6d-6508-4408-944a-55baf67af255

📥 Commits

Reviewing files that changed from the base of the PR and between 57dfdc8 and 86b6c55.

📒 Files selected for processing (1)
  • clients/agent-runtime/src/gateway/mod.rs
📜 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). (4)
  • GitHub Check: pr-checks
  • GitHub Check: sonar
  • GitHub Check: pr-checks
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (6)
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/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/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, and cargo test for code validation, or document which checks were skipped and why

Files:

  • clients/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/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/gateway/mod.rs
**/*

⚙️ CodeRabbit configuration file

**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.

Files:

  • clients/agent-runtime/src/gateway/mod.rs
🧠 Learnings (4)
📓 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/{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
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths

Applied to files:

  • clients/agent-runtime/src/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/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/gateway/mod.rs
🔇 Additional comments (2)
clients/agent-runtime/src/gateway/mod.rs (2)

1563-1571: Good: fallback responses are scrubbed before returning.

This keeps the pre-execution fallback path aligned with the normal webhook/SSE redaction boundary.


2784-2837: Good regression coverage for retryable timeout semantics.

The new test exercises the shared X-Idempotency-Key timeout path and verifies no provider call slips through.

Comment thread clients/agent-runtime/src/gateway/mod.rs Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Yuniel Acosta Pérez <33158051+yacosta738@users.noreply.github.com>
@yacosta738 yacosta738 merged commit 09cad72 into main Mar 8, 2026
16 of 17 checks passed
@yacosta738 yacosta738 deleted the feature/dallay-139-centralize-pre-execution-flow branch March 8, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Centralize pre-execution flow (approval/timeout/fallback)

1 participant