feat: introduce mission layer for autonomous objective orchestration#132
Conversation
|
Warning Rate limit exceeded
⌛ 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. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (28)
📝 WalkthroughWalkthroughIntroduces a mission orchestration layer for agent-runtime with state machine, governance boundaries (budget/SLA/timeout), and delegation flow. Reuses existing dispatcher, policy, and approval paths with new public mission data structures and configuration. Adds integration tests for lifecycle, governance, and security parity, extending legacy loop tests to verify backward compatibility when mission mode is disabled. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Agent as Agent Runtime
participant Planner as Mission Planner
participant Dispatcher as Dispatcher
participant Security as Policy/Approval
participant Observer as Observability
Client->>Agent: Submit mission objective
Agent->>Observer: Emit MissionState::ObjectiveAccepted
Agent->>Planner: Generate execution plan
Planner-->>Agent: Checkpoints & steps
Agent->>Observer: Emit MissionState::Planned
Agent->>Agent: Check governance (budget/SLA/steps)
Agent->>Observer: Emit MissionState::Active
loop For each checkpoint
Agent->>Dispatcher: Dispatch tool call (mission-originated)
Dispatcher->>Security: Classify risk & enforce policy
alt Policy allows
Security->>Security: Check approval requirement
alt Approval granted
Dispatcher-->>Agent: Execute & return result
Agent->>Observer: Emit checkpoint progress
else Approval denied
Security-->>Agent: Return denial
Agent->>Observer: Emit MissionState::Terminated(ApprovalDenied)
end
else Policy denies
Security-->>Agent: Return denial
Agent->>Observer: Emit MissionState::Terminated(PolicyDenied)
end
end
Agent->>Agent: Evaluate mission completion or replan
alt Completed successfully
Agent->>Observer: Emit MissionState::Completed with MissionOutcome
else Governance exceeded
Agent->>Observer: Emit MissionState::Terminated(BudgetExhausted/SlaExceeded)
else Needs replan
Agent->>Agent: Transition to Replanning
Agent->>Planner: Revise plan preserving prior checkpoints
Planner-->>Agent: Updated checkpoints & steps
Agent->>Agent: Resume execution from current checkpoint
end
Agent-->>Client: Return final MissionOutcome
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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-03-04 to 2026-03-04 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/agent-runtime-mission-layer/design.md`:
- Around line 206-212: Resolve the two open design questions by updating the
document to state explicit decisions: set the mission KPI overhead limits to a
p95 latency increase ≤10ms and memory ≤5MB per mission (measured in CI
performance tests, tracked observationally in initial rollout and enforced as a
hard limit in phase 3) and specify that mission checkpoint supervision will run
as a lightweight, independent component (start on daemon init when
mission.enabled=true, stop on daemon shutdown, not tied to cron/scheduler);
update the "Open Questions" section to "Resolved Design Decisions" and include
these exact constraints and the mission supervisor lifecycle so implementers
reuse the same parameters.
- Line 118: The Observer trait is being extended which will break existing
implementations (NoopObserver, VerboseObserver, LogObserver, PrometheusObserver,
OtelObserver, MultiObserver); instead add new mission/governance telemetry as
new variants on the existing ObserverEvent enum (e.g., MissionStarted,
MissionCheckpointCompleted, MissionGovernanceViolation, MissionCompleted,
MissionTerminated) and emit those via the existing Observer::record_event(&self,
event: ObserverEvent) so no trait signature changes are required; alternatively,
if new methods are unavoidable, make them optional by providing default
implementations on the Observer trait to preserve backward compatibility.
- Around line 170-183: Add explicit fail-closed default functions and a Default
impl for MissionConfig: implement default_mission_max_runtime_ms(),
default_mission_max_steps(), and default_mission_max_estimated_cost_cents()
returning conservative values (e.g., 300_000 ms, 10 steps, 100 cents) and
implement Default for MissionConfig to set enabled: false and populate
max_runtime_ms, max_steps, and max_estimated_cost_cents from those functions so
serde defaults are well-defined and enforce conservative limits.
- Around line 170-183: Add an explicit conversion impl From<MissionConfig> for
MissionGovernance to ensure fields stay in sync: implement fn from(config:
MissionConfig) -> Self that copies max_runtime_ms, max_steps, and
max_estimated_cost_cents from MissionConfig into MissionGovernance and
initializes runtime-only fields elapsed_ms, completed_steps, and
accumulated_cost_cents to zero; place this impl alongside the
MissionConfig/MissionGovernance definitions so future additions are obvious and
compiler-driven.
- Around line 126-168: Add a MissionCoordinator state-machine interface to the
mission layer to standardize transitions: declare a pub struct
MissionCoordinator with fields state: Arc<Mutex<MissionState>>, governance:
MissionGovernance, accumulated_cost_cents: Arc<Mutex<u32>>, accumulated_steps:
Arc<Mutex<u32>>, and elapsed_ms: Arc<Mutex<u64>>, and add the impl signatures
for pub fn transition(&self, target: MissionState) -> Result<MissionState,
MissionTerminationReason> and pub fn validate_governance(&self) -> Result<(),
MissionTerminationReason>; this provides a single contract (MissionCoordinator,
transition, validate_governance) for transition validation, governance checks,
and fail-closed accounting without changing CLI behavior.
- Around line 149-153: MissionGovernance currently only has ceiling limits
(max_runtime_ms, max_steps, max_estimated_cost_cents); add accumulated/current
fields (e.g., elapsed_ms: u64, completed_steps: u32, accumulated_cost_cents:
u32) to the struct so enforcement can compare current values to the maxima, and
update any methods that construct or update MissionGovernance (e.g., wherever
MissionGovernance is instantiated or mutated) to increment these fields
(elapsed_ms, completed_steps, accumulated_cost_cents) during runtime checks and
cost calculations so the governance checks can use these accumulated values when
deciding to halt a mission.
In `@openspec/changes/agent-runtime-mission-layer/proposal.md`:
- Around line 83-95: Update the "Rollback Plan" section to add concrete
observability and test gates: specify CI gate requirement that the
`legacy_loop_guard.rs` suite must pass 100% before deploy, add a runtime metric
name `mission.governance.bypass_attempt` (emit and alert if >0) and a canary
policy (1% traffic with automated rollback if error rate >2x baseline), and
require audit logs for each mission action including fields mission_id,
action_type, dispatcher_result, policy_decision, approval_required; also add
short threat/risk notes and per-area rollback steps for security, runtime
governance, and gateway compatibility so operators know detection, alerting, and
rollback actions.
- Around line 102-110: Update the "Success Criteria" section to include
explicit, quantitative performance limits: add two bullets after the existing
list (under the "Success Criteria" header / near the "mission mode" wording)
stating "Performance envelope tests confirm mission mode overhead ≤10ms p95
latency and ≤5MB memory per mission." and "Benchmark suite shows non-mission
loop throughput unchanged (within 2% variance) when mission.enabled=false."
Ensure the wording matches the proposed addition exactly so the document now
specifies p95 latency and per-mission memory caps and a throughput variance
target.
- Around line 76-81: Update the mitigations to be concrete: gate the mission
layer behind the `mission.enabled` config flag (default false) and preserve the
legacy loop path; require the `legacy_loop_guard.rs` integration tests to assert
parity for approval, timeout, and tool dispatch semantics; enforce that all
mission actions route through the central dispatcher→policy→approval path and
make `MissionState` handled via exhaustive enum matching so any unknown state
triggers an explicit fail-closed response (return `PolicyDenied` on
classification/matching error); and specify performance limits measured by CI
performance tests that run `mission_telemetry_overhead_test.rs`, asserting p95
latency increase <10ms and memory increase <5MB per mission using the existing
telemetry harness (measure p95 over N runs with warmup and compare baseline vs
mission-enabled).
In `@openspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.md`:
- Around line 59-65: The spec's "structured denial result into mission state" is
underspecified—update the scenario so the runtime MUST emit the ApprovalDenial
structure (the ApprovalDenial struct with fields code, tool, reason from
approval/mod.rs:32-37) or a mission-specific type that preserves those same
semantic fields; ensure mission orchestration and any mission-state mutation
paths serialize/deserialize to that schema and validate conformance to prevent
divergence between approval and mission modules.
- Around line 5-10: Add an explicit requirement that state transitions are
atomic and concurrency-safe by inserting a new scenario titled "Concurrent state
transition attempts" after the "Mission Lifecycle Contract" section: describe
that for a mission in active state when concurrent governance and checkpoint
events try to change state, the runtime MUST serialize transitions using atomic
operations or locking (or equivalent concurrency primitives), ensure only one
transition succeeds while the other is deterministically queued or rejected, and
guarantee the resulting state is a valid progression of the mission state
machine; reference the mission lifecycle/state transition behavior and the new
"Scenario: Concurrent state transition attempts" heading so implementers know
where to enforce atomicity.
- Around line 82-85: The ObserverEvent::Error emission in observability/log.rs
is currently emitting raw error details—update the emission path so all
telemetry payloads are redacted before calling record_event(): run error
message/arguments through redact_runtime_error(), apply
redact_headers_for_display() to any header maps, and pass diagnostic blobs
through redact_diagnostic() (from tools/mod.rs, tools/http_request.rs,
mcp/client.rs respectively) so secrets (password, token, secret, api_key, auth
patterns) are replaced with [REDACTED]/***REDACTED***; ensure every codepath
that constructs an ObserverEvent::Error calls these redactors first and only
then forwards the sanitized payload to record_event().
- Around line 28-31: Update the spec to concretely define "unknown or invalid
governance state" for the MissionGovernance struct: enumerate cases such as
missing required fields in MissionGovernance, non-positive values for numeric
fields (max_runtime_ms <= 0, max_steps <= 0, max_estimated_cost_cents <= 0), NaN
or Infinity in any numeric field, integer overflow/underflow in accounting
updates, and any corrupted or unparsable governance payload; state that if any
of these occur at load-time or mid-execution the runtime MUST "fail closed" by
immediately terminating the mission with a deterministic termination reason like
governance_constraint_violated and preventing further checkpoints or delegated
execution. Ensure the spec references the MissionGovernance field names
(max_runtime_ms, max_steps, max_estimated_cost_cents) and the required
termination behavior (terminate + block further execution/checkpointing) so
tests can assert these concrete cases.
In `@openspec/changes/agent-runtime-mission-layer/tasks.md`:
- Around line 49-64: Add a new rollback scenario test file
clients/agent-runtime/tests/mission_config_toggle.rs that programmatically flips
the mission.enabled flag from true→false mid-deployment and asserts (1) new
incoming requests are routed to the legacy loop path (reuse assertions from
legacy_loop_guard.rs), (2) any in-flight mission work is safely aborted/cleaned
up (no zombie processes, no orphaned resource handles), and (3) telemetry/logs
record termination/rollback reasons and no leaked checkpoints; implement the
test using the same harness and mocks used by mission_lifecycle_integration.rs
and mission_governance_integration.rs so it can be run in the focused
verification command and added as task 4.5 in Phase 4.
- Around line 36-47: Add a new task entry (3.2.1) after 3.2 in the Phase 3
checklist to require tests that assert invalid/malformed MissionConfig values
(e.g., max_runtime_ms: 0, max_steps: 0, negative ceilings, missing fields, or
malformed JSON) cause deterministic "fail-closed" termination before execution;
reference the test file
clients/agent-runtime/tests/mission_governance_integration.rs for the tests and
the enforcement point in clients/agent-runtime/src/agent/mission.rs
(MissionConfig validation and pre-checkpoint enforcement) so implementers know
where to add validation and where tests should verify behavior.
- Around line 61-64: Update the verification step to use the correct package
name and include the missing validation steps: change `-p agent-runtime` to `-p
corvus`, remove or mark as TODO the non-existent test modules
(`mission_lifecycle_integration`, `mission_governance_integration`,
`mission_security_parity`) leaving only `legacy_loop_guard` (or add those
modules if they are implemented), and prepend the checklist with `cargo fmt
--all -- --check` and `cargo clippy --all-targets -- -D warnings` before running
`cargo test` (keep references to `cargo test` and `legacy_loop_guard` so
reviewers can locate the command to edit).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fd9f9e92-1e00-486b-9a7a-4d8ac77a1ea1
📒 Files selected for processing (4)
openspec/changes/agent-runtime-mission-layer/design.mdopenspec/changes/agent-runtime-mission-layer/proposal.mdopenspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.mdopenspec/changes/agent-runtime-mission-layer/tasks.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/agent-runtime-mission-layer/tasks.mdopenspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.mdopenspec/changes/agent-runtime-mission-layer/proposal.mdopenspec/changes/agent-runtime-mission-layer/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/agent-runtime-mission-layer/tasks.mdopenspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.mdopenspec/changes/agent-runtime-mission-layer/proposal.mdopenspec/changes/agent-runtime-mission-layer/design.md
🧠 Learnings (5)
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Document agent configurations and capabilities in AGENTS.md
Applied to files:
openspec/changes/agent-runtime-mission-layer/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/**/*.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/agent-runtime-mission-layer/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: 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/agent-runtime-mission-layer/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 : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
openspec/changes/agent-runtime-mission-layer/design.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/{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/agent-runtime-mission-layer/design.md
🔇 Additional comments (1)
openspec/changes/agent-runtime-mission-layer/design.md (1)
107-123: Remove API breaking change concern; design adds ObserverEvent variants, not required trait methods.The design specifies adding mission-specific telemetry events (
mission_started,mission_checkpoint_progress,mission_replanned,mission_completed,mission_terminated) as newObserverEventenum variants, not as new required methods on the Observer trait. Since existing implementors (VerboseObserver, LogObserver, PrometheusObserver, OtelObserver, MultiObserver, NoopObserver) all use non-exhaustive pattern matching with_ => {}, adding new enum variants is backward compatible and requires no changes to existing implementations.For dispatcher.rs, security/policy.rs, and approval/mod.rs, the design explicitly states "preserve," "reuse/extend," and "ensure parity"—not breaking changes. No evidence of new public types being exported from these modules; reuse of existing
ApprovalResponse,ToolPolicyDecision, andDispatchActionis sufficient.> Likely an incorrect or invalid review comment.
| | `clients/agent-runtime/src/daemon/mod.rs` | Modify | Wire optional mission checkpoint supervision into existing daemon lifecycle. | | ||
| | `clients/agent-runtime/src/security/policy.rs` | Modify | Reuse/extend policy decision helpers for mission-originated delegated actions. | | ||
| | `clients/agent-runtime/src/approval/mod.rs` | Modify | Ensure mission delegated actions produce structured denial parity. | | ||
| | `clients/agent-runtime/src/observability/traits.rs` | Modify | Add mission lifecycle and governance telemetry events/metrics. | |
There was a problem hiding this comment.
Risk: Observer trait extension will break all existing implementations.
Line 118 states "Add mission lifecycle and governance telemetry events/metrics" to observability/traits.rs. The relevant code snippet (observability/traits.rs:65-81) shows the Observer trait has required methods (record_event, record_metric, name, as_any). Adding new required methods will break at least 6 implementations: NoopObserver, VerboseObserver, LogObserver, PrometheusObserver, OtelObserver, MultiObserver.
Mitigation options:
- Preferred: Add new telemetry via existing
record_event()method with new event types (e.g.,ObserverEvent::MissionStarted,ObserverEvent::GovernanceViolation) - backward compatible. - Add optional trait methods with default implementations.
- Create a separate
MissionObservertrait that extendsObserver.
✅ Proposed backward-compatible approach
// Add to existing ObserverEvent enum (non-breaking)
#[derive(Debug, Clone)]
pub enum ObserverEvent {
// ... existing variants
MissionStarted { mission_id: String, timestamp: String },
MissionCheckpointCompleted { mission_id: String, checkpoint_index: u32, duration_ms: u64 },
MissionGovernanceViolation { mission_id: String, reason: String },
MissionCompleted { mission_id: String, checkpoints_completed: u32 },
MissionTerminated { mission_id: String, reason: String },
}
// No new required methods needed - reuse existing record_event()As per coding guidelines, look for contract breaks across modules - breaking Observer implementations is a major compatibility issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/design.md` at line 118, The
Observer trait is being extended which will break existing implementations
(NoopObserver, VerboseObserver, LogObserver, PrometheusObserver, OtelObserver,
MultiObserver); instead add new mission/governance telemetry as new variants on
the existing ObserverEvent enum (e.g., MissionStarted,
MissionCheckpointCompleted, MissionGovernanceViolation, MissionCompleted,
MissionTerminated) and emit those via the existing Observer::record_event(&self,
event: ObserverEvent) so no trait signature changes are required; alternatively,
if new methods are unavoidable, make them optional by providing default
implementations on the Observer trait to preserve backward compatibility.
| ```rust | ||
| // clients/agent-runtime/src/agent/mission.rs | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub enum MissionState { | ||
| ObjectiveAccepted, | ||
| Planned, | ||
| Active, | ||
| Replanning, | ||
| Completed, | ||
| Terminated, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub enum MissionTerminationReason { | ||
| BudgetExhausted, | ||
| SlaExceeded, | ||
| PolicyDenied, | ||
| ApprovalDenied, | ||
| GuardrailViolation, | ||
| Unrecoverable, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct MissionGovernance { | ||
| pub max_runtime_ms: u64, | ||
| pub max_steps: u32, | ||
| pub max_estimated_cost_cents: u32, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct MissionCheckpoint { | ||
| pub index: u32, | ||
| pub objective_fragment: String, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct MissionOutcome { | ||
| pub mission_id: String, | ||
| pub state: MissionState, | ||
| pub termination: Option<MissionTerminationReason>, | ||
| pub checkpoints_completed: u32, | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Add mission state transition method interface to prevent divergent implementations.
The interface section defines data structures (MissionState, MissionTerminationReason, etc.) but omits the critical state transition logic. Without a defined transition() method signature, implementations may diverge on validation, atomicity, and governance enforcement. The relevant code snippet from approval/mod.rs (lines 75-145) shows explicit validation guards in needs_approval() - mission state transitions need equivalent guard patterns.
🔧 Proposed addition to interfaces
Add after line 168:
// Mission state machine coordinator
pub struct MissionCoordinator {
state: Arc<Mutex<MissionState>>,
governance: MissionGovernance,
accumulated_cost_cents: Arc<Mutex<u32>>,
accumulated_steps: Arc<Mutex<u32>>,
elapsed_ms: Arc<Mutex<u64>>,
}
impl MissionCoordinator {
/// Attempt a state transition with validation and governance checks.
/// Returns Ok(new_state) if transition is valid, Err(reason) if denied.
pub fn transition(
&self,
target: MissionState,
) -> Result<MissionState, MissionTerminationReason>;
/// Check governance limits before allowing next checkpoint.
/// Fail-closed: returns Err on unknown/invalid accounting state.
pub fn validate_governance(&self) -> Result<(), MissionTerminationReason>;
}Based on learnings, preserve CLI contract unless change is intentional and documented - the transition interface is a critical contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/design.md` around lines 126 -
168, Add a MissionCoordinator state-machine interface to the mission layer to
standardize transitions: declare a pub struct MissionCoordinator with fields
state: Arc<Mutex<MissionState>>, governance: MissionGovernance,
accumulated_cost_cents: Arc<Mutex<u32>>, accumulated_steps: Arc<Mutex<u32>>, and
elapsed_ms: Arc<Mutex<u64>>, and add the impl signatures for pub fn
transition(&self, target: MissionState) -> Result<MissionState,
MissionTerminationReason> and pub fn validate_governance(&self) -> Result<(),
MissionTerminationReason>; this provides a single contract (MissionCoordinator,
transition, validate_governance) for transition validation, governance checks,
and fail-closed accounting without changing CLI behavior.
| pub struct MissionGovernance { | ||
| pub max_runtime_ms: u64, | ||
| pub max_steps: u32, | ||
| pub max_estimated_cost_cents: u32, | ||
| } |
There was a problem hiding this comment.
Add accumulated tracking fields to MissionGovernance for enforcement.
The MissionGovernance struct defines ceiling limits (max_runtime_ms, max_steps, max_estimated_cost_cents) but lacks the corresponding accumulated/current values needed to actually enforce those limits. Without fields like elapsed_ms, completed_steps, and accumulated_cost_cents, the governance checks cannot be implemented. This creates a gap between the defined interface and the enforcement requirement from spec.md lines 33-38.
💰 Proposed fix
#[derive(Debug, Clone)]
pub struct MissionGovernance {
pub max_runtime_ms: u64,
pub max_steps: u32,
pub max_estimated_cost_cents: u32,
+ // Current accumulated values for enforcement
+ pub elapsed_ms: u64,
+ pub completed_steps: u32,
+ pub accumulated_cost_cents: u32,
}
+
+impl MissionGovernance {
+ /// Check if any governance limit is exceeded.
+ /// Fail-closed: treats overflow or invalid arithmetic as exceeded.
+ pub fn is_exceeded(&self) -> Option<MissionTerminationReason> {
+ if self.elapsed_ms >= self.max_runtime_ms {
+ return Some(MissionTerminationReason::SlaExceeded);
+ }
+ if self.completed_steps >= self.max_steps {
+ return Some(MissionTerminationReason::BudgetExhausted);
+ }
+ if self.accumulated_cost_cents >= self.max_estimated_cost_cents {
+ return Some(MissionTerminationReason::BudgetExhausted);
+ }
+ None
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/design.md` around lines 149 -
153, MissionGovernance currently only has ceiling limits (max_runtime_ms,
max_steps, max_estimated_cost_cents); add accumulated/current fields (e.g.,
elapsed_ms: u64, completed_steps: u32, accumulated_cost_cents: u32) to the
struct so enforcement can compare current values to the maxima, and update any
methods that construct or update MissionGovernance (e.g., wherever
MissionGovernance is instantiated or mutated) to increment these fields
(elapsed_ms, completed_steps, accumulated_cost_cents) during runtime checks and
cost calculations so the governance checks can use these accumulated values when
deciding to halt a mission.
| ```rust | ||
| // clients/agent-runtime/src/config/schema.rs (additive) | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct MissionConfig { | ||
| #[serde(default)] | ||
| pub enabled: bool, | ||
| #[serde(default = "default_mission_max_runtime_ms")] | ||
| pub max_runtime_ms: u64, | ||
| #[serde(default = "default_mission_max_steps")] | ||
| pub max_steps: u32, | ||
| #[serde(default = "default_mission_max_estimated_cost_cents")] | ||
| pub max_estimated_cost_cents: u32, | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Define MissionConfig default functions to prevent silent misconfiguration.
The MissionConfig struct uses serde defaults (lines 174-181) but doesn't show the implementation of default_mission_max_runtime_ms(), default_mission_max_steps(), and default_mission_max_estimated_cost_cents(). Without seeing these functions, reviewers cannot verify fail-closed defaults. The proposal states "enabled = false fail-closed defaults" (tasks.md line 7), so these functions must return conservative/restrictive values.
🔐 Proposed addition
Add after line 183:
// Fail-closed defaults: conservative limits to prevent runaway execution
fn default_mission_max_runtime_ms() -> u64 {
300_000 // 5 minutes
}
fn default_mission_max_steps() -> u32 {
10 // Conservative step limit
}
fn default_mission_max_estimated_cost_cents() -> u32 {
100 // $1.00 default budget
}
impl Default for MissionConfig {
fn default() -> Self {
Self {
enabled: false, // Fail-closed: mission mode off by default
max_runtime_ms: default_mission_max_runtime_ms(),
max_steps: default_mission_max_steps(),
max_estimated_cost_cents: default_mission_max_estimated_cost_cents(),
}
}
}As per coding guidelines, do not silently weaken security policy - default values must be documented and conservative.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/design.md` around lines 170 -
183, Add explicit fail-closed default functions and a Default impl for
MissionConfig: implement default_mission_max_runtime_ms(),
default_mission_max_steps(), and default_mission_max_estimated_cost_cents()
returning conservative values (e.g., 300_000 ms, 10 steps, 100 cents) and
implement Default for MissionConfig to set enabled: false and populate
max_runtime_ms, max_steps, and max_estimated_cost_cents from those functions so
serde defaults are well-defined and enforce conservative limits.
🛠️ Refactor suggestion | 🟠 Major
Add explicit From → MissionGovernance conversion to prevent field divergence.
The MissionConfig struct (lines 170-183) and MissionGovernance struct (lines 149-153) have parallel field structures but no explicit conversion defined. The relevant code snippet from config/schema.rs (lines 1-80) shows the Config struct uses composition with sub-configs. Without an explicit conversion, implementations may copy fields incorrectly or fail to update both structs when fields are added.
🔄 Proposed addition
Add after line 183:
impl From<MissionConfig> for MissionGovernance {
fn from(config: MissionConfig) -> Self {
Self {
max_runtime_ms: config.max_runtime_ms,
max_steps: config.max_steps,
max_estimated_cost_cents: config.max_estimated_cost_cents,
elapsed_ms: 0,
completed_steps: 0,
accumulated_cost_cents: 0,
}
}
}Based on learnings, preserve CLI contract and prevent silent divergence between config and runtime structures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/design.md` around lines 170 -
183, Add an explicit conversion impl From<MissionConfig> for MissionGovernance
to ensure fields stay in sync: implement fn from(config: MissionConfig) -> Self
that copies max_runtime_ms, max_steps, and max_estimated_cost_cents from
MissionConfig into MissionGovernance and initializes runtime-only fields
elapsed_ms, completed_steps, and accumulated_cost_cents to zero; place this impl
alongside the MissionConfig/MissionGovernance definitions so future additions
are obvious and compiler-driven.
| ## Open Questions | ||
|
|
||
| - [ ] Should mission KPI overhead limits be enforced with an explicit p95 latency threshold in CI or | ||
| tracked first as an observational metric? | ||
| - [ ] Should daemon-driven mission checkpoint supervision run only when cron/scheduler is enabled, | ||
| or | ||
| independently as its own lightweight supervised component? |
There was a problem hiding this comment.
Resolve open questions before implementation begins.
The open questions about p95 latency thresholds (line 208) and daemon supervision independence (line 210) are architectural decisions that should be resolved during design phase, not left open for implementation. Unresolved questions risk inconsistent implementations across the phased rollout. The proposal.md mentions "agreed runtime overhead envelope" (line 80) but doesn't specify the agreement.
Recommendations:
- p95 latency threshold: Based on proposal.md context, suggest explicit limit of <10ms p95 increase with mission mode enabled, measured in CI performance tests.
- Daemon supervision coupling: Recommend mission checkpoint supervision run independently of cron/scheduler to avoid coupling mission lifecycle to unrelated scheduling features.
📋 Proposed resolution
Replace open questions section with:
## Resolved Design Decisions
- **Mission KPI overhead limit:** Enforce p95 latency increase ≤10ms and memory ≤5MB per mission via CI performance regression tests. Track as observational metric in first rollout phase, enforce as hard limit in phase 3.
- **Daemon mission supervision:** Run mission checkpoint supervision as lightweight independent component, not coupled to cron/scheduler. Mission supervisor lifecycle: start on daemon init if `mission.enabled=true`, stop on daemon shutdown, independent of scheduler config.Based on learnings, prefer explicit errors over silent fallback for unsupported critical paths - unresolved design questions create ambiguity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/design.md` around lines 206 -
212, Resolve the two open design questions by updating the document to state
explicit decisions: set the mission KPI overhead limits to a p95 latency
increase ≤10ms and memory ≤5MB per mission (measured in CI performance tests,
tracked observationally in initial rollout and enforced as a hard limit in phase
3) and specify that mission checkpoint supervision will run as a lightweight,
independent component (start on daemon init when mission.enabled=true, stop on
daemon shutdown, not tied to cron/scheduler); update the "Open Questions"
section to "Resolved Design Decisions" and include these exact constraints and
the mission supervisor lifecycle so implementers reuse the same parameters.
| #### Scenario: Delegated mission step denied by policy | ||
|
|
||
| - GIVEN a mission checkpoint that resolves to a denied action by current policy | ||
| - WHEN mission orchestration requests execution | ||
| - THEN the runtime MUST return a structured denial result into mission state | ||
| - AND the mission MUST transition using fail-closed governance semantics without unauthorized side | ||
| effects. |
There was a problem hiding this comment.
Specify the structured denial result format explicitly.
Line 63 requires "structured denial result into mission state" but doesn't reference a schema or format. The relevant code snippet shows ApprovalDenial struct exists (approval/mod.rs:32-37) with fields code, tool, reason. The spec should explicitly require mission denial results to use this same structure or define the mission-specific variant.
📝 Proposed clarification
Add after line 63:
- AND the denial result MUST use the structured ApprovalDenial format (code, tool, reason)
defined in the approval module, or an equivalent mission-specific structure with the same
semantic fieldsAs per coding guidelines, validate contract breaks across modules - ensuring mission code uses the existing ApprovalDenial type prevents silent divergence between delegation paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.md` around
lines 59 - 65, The spec's "structured denial result into mission state" is
underspecified—update the scenario so the runtime MUST emit the ApprovalDenial
structure (the ApprovalDenial struct with fields code, tool, reason from
approval/mod.rs:32-37) or a mission-specific type that preserves those same
semantic fields; ensure mission orchestration and any mission-state mutation
paths serialize/deserialize to that schema and validate conformance to prevent
divergence between approval and mission modules.
| - GIVEN a mission terminated due to governance or policy constraints | ||
| - WHEN the mission enters a terminal failure state | ||
| - THEN the runtime MUST emit a telemetry event describing the guardrail class and termination reason | ||
| - AND telemetry payloads MUST avoid exposing sensitive arguments or secret values. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for redaction/sanitization utilities
echo "=== Searching for redaction/sanitization utilities ==="
rg -n "redact|sanitize|mask|secret.*filter" clients/agent-runtime/src --type rust -C2 | head -100
echo ""
echo "=== Searching for observability/telemetry traits and structures ==="
rg -n "Observ|Telemetry|Event.*emit" clients/agent-runtime/src --type rust -C3 | head -150
echo ""
echo "=== Searching for sensitive data handling patterns ==="
rg -n "password|token|secret|api_key|credential" clients/agent-runtime/src --type rust -C2 | head -100Repository: dallay/corvus
Length of output: 27693
🏁 Script executed:
#!/bin/bash
# Check the spec file itself for more context
echo "=== Reading the full spec section around lines 82-85 ==="
if [ -f "openspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.md" ]; then
cat -n "openspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.md" | sed -n '70,100p'
else
echo "File not found, searching for spec.md files:"
find . -name "spec.md" -path "*/agent-loop/*" 2>/dev/null
fiRepository: dallay/corvus
Length of output: 1787
🏁 Script executed:
#!/bin/bash
# Check if there are existing utility functions for filtering sensitive data
echo "=== Searching for utility functions that filter arguments or payloads ==="
rg -n "filter.*arg|filter.*payload|redact.*field|sanitize.*field" clients/agent-runtime/src --type rust -C3
echo ""
echo "=== Checking for observability configuration or policy ==="
fd -e rs -path "*/observability/*" clients/agent-runtime/src | xargs -I {} sh -c 'echo "=== {} ===" && head -50 "{}"'Repository: dallay/corvus
Length of output: 3225
Integrate redaction utilities into telemetry emission to prevent secrets in error events.
The spec correctly requires avoiding secrets in telemetry payloads, but the observability layer must apply redaction before events are logged. The codebase already has working redaction patterns—redact_runtime_error() (tools/mod.rs), redact_headers_for_display() (tools/http_request.rs), and redact_diagnostic() (mcp/client.rs)—that detect and replace sensitive keys (password, token, secret, api_key, auth patterns) with [REDACTED] or ***REDACTED***.
However, ObserverEvent::Error in observability/log.rs currently logs raw error messages without redaction. Apply the existing redaction pattern to all telemetry payloads that carry user-supplied arguments or error details before record_event() emits them. Align with learnings: never log secrets/tokens/raw credentials, keep security-by-default with deny-by-default policy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/specs/agent-loop/spec.md` around
lines 82 - 85, The ObserverEvent::Error emission in observability/log.rs is
currently emitting raw error details—update the emission path so all telemetry
payloads are redacted before calling record_event(): run error message/arguments
through redact_runtime_error(), apply redact_headers_for_display() to any header
maps, and pass diagnostic blobs through redact_diagnostic() (from tools/mod.rs,
tools/http_request.rs, mcp/client.rs respectively) so secrets (password, token,
secret, api_key, auth patterns) are replaced with [REDACTED]/***REDACTED***;
ensure every codepath that constructs an ObserverEvent::Error calls these
redactors first and only then forwards the sanitized payload to record_event().
| - [ ] 4.4 Run focused verification ( | ||
| `cargo test -p agent-runtime mission_lifecycle_integration mission_governance_integration mission_security_parity legacy_loop_guard`) | ||
| and full regression (`make test`), then capture any follow-up fixes in the same files before | ||
| handoff. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the actual package name in Cargo.toml
find clients -name "Cargo.toml" -type f | head -20Repository: dallay/corvus
Length of output: 308
🏁 Script executed:
# Look for the agent-runtime Cargo.toml specifically
cat clients/agent-runtime/Cargo.toml | head -30Repository: dallay/corvus
Length of output: 1081
🏁 Script executed:
# Search for the test files mentioned in the cargo test command
fd -t f "(mission_lifecycle_integration|mission_governance_integration|mission_security_parity|legacy_loop_guard)" clients/agent-runtimeRepository: dallay/corvus
Length of output: 105
🏁 Script executed:
# Check if there are other verification/test commands documented in the agent-runtime area
rg -n "cargo (test|fmt|clippy)" clients/agent-runtime --type md -A2 -B2Repository: dallay/corvus
Length of output: 3057
Fix package name and include missing test modules and validation steps.
The -p agent-runtime should be -p corvus (the actual package name). Additionally, the cargo test command references mission_lifecycle_integration, mission_governance_integration, and mission_security_parity test modules that don't exist yet—only legacy_loop_guard is present. Per CONTRIBUTING.md, the verification checklist must include cargo fmt --all -- --check and cargo clippy --all-targets -- -D warnings before running tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/agent-runtime-mission-layer/tasks.md` around lines 61 - 64,
Update the verification step to use the correct package name and include the
missing validation steps: change `-p agent-runtime` to `-p corvus`, remove or
mark as TODO the non-existent test modules (`mission_lifecycle_integration`,
`mission_governance_integration`, `mission_security_parity`) leaving only
`legacy_loop_guard` (or add those modules if they are implemented), and prepend
the checklist with `cargo fmt --all -- --check` and `cargo clippy --all-targets
-- -D warnings` before running `cargo test` (keep references to `cargo test` and
`legacy_loop_guard` so reviewers can locate the command to edit).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
1 participant
This pull request introduces a new "mission" orchestration layer to the agent runtime, enabling autonomous objectives to be managed with explicit planning, checkpointing, replanning, and mission-level governance, while preserving existing security, approval, and compatibility guarantees. The changes are additive and carefully scoped to avoid regressions, with thorough integration and regression testing to ensure backward compatibility and security parity.
Key changes include:
Mission Lifecycle and Governance:
Architecture and Integration:
Observability and Telemetry:
Testing and Compatibility:
Migration and Rollback:
These changes lay the foundation for safe, autonomous mission execution in the agent runtime, while rigorously preserving existing security and compatibility guarantees.