lhumina_code/hero_shrimp

Fork 0

Wire engine-side plan-approval gate (honour approval.auto_approve) #100

New issue

Open

opened 2026-06-09 13:18:00 +00:00 by rawan · 3 comments

rawan commented

2026-06-09 13:18:00 +00:00

Member

Problem

The approval.auto_approve setting (Settings -> Auto-approve plans) now toggles and persists correctly, but turning it off has no effect: a do --plan job never pauses to ask for approval.

The plan-approval workflow is only half-wired. The plan_approval:<job_id> row is written by plan.approve and read back by plan.approval_status (UI display only). No code in the engine/agent loop consumes that row to gate execution. Jobs also start in mode: yolo, so nothing blocks.

This is documented as an unfinished follow-up in the code:

crates/hero_shrimp/src/commands/do_cmd.rs (~L90-103): "the engine-side resume hook isn't wired up yet, so jobs run with --plan may stall in planning indefinitely."
crates/hero_shrimp_server/src/rpc/methods/plan.rs (~L9-17): "an approval is captured but doesn't yet auto-resume execution."

Proposed feature

Implement the engine-side approval gate so the toggle is enforced for web/server-run plans:

When a plan is published and approval.auto_approve is false (and no per-agent override approves it), the agent loop pauses the job in an awaiting_approval state instead of executing.
Poll/consume the plan_approval:<job_id> state row: on approve (or edit) resume execution; on decline abort cleanly.
When approval.auto_approve is true, resume without waiting (current pre-recorded-decision behaviour).
Surface the awaiting/approved/declined state to the UI so the JobDrawer chip reflects the real gate, and clear the CLI stall note once the hook lands.

Acceptance criteria

auto-approve OFF: a do --plan job pauses and waits for an approval decision before executing.
auto-approve ON: the job resumes automatically with no prompt.
Approve resumes, decline aborts, edit resumes with the revised plan text.
Per-agent approval_mode: required still forces a human gate even when auto-approve is on.
Tests cover the gate (pause, resume-on-approve, abort-on-decline).

Notes

Follow-up to the checkbox enable/disable fix (#95). That fix made the flag toggle and persist; this issue covers the missing enforcement.

## Problem The `approval.auto_approve` setting (Settings -> Auto-approve plans) now toggles and persists correctly, but turning it **off** has no effect: a `do --plan` job never pauses to ask for approval. The plan-approval workflow is only half-wired. The `plan_approval:<job_id>` row is written by `plan.approve` and read back by `plan.approval_status` (UI display only). **No code in the engine/agent loop consumes that row to gate execution.** Jobs also start in `mode: yolo`, so nothing blocks. This is documented as an unfinished follow-up in the code: - `crates/hero_shrimp/src/commands/do_cmd.rs` (~L90-103): "the engine-side resume hook isn't wired up yet, so jobs run with --plan may stall in planning indefinitely." - `crates/hero_shrimp_server/src/rpc/methods/plan.rs` (~L9-17): "an approval is captured but doesn't yet auto-resume execution." ## Proposed feature Implement the engine-side approval gate so the toggle is enforced for web/server-run plans: - When a plan is published and `approval.auto_approve` is **false** (and no per-agent override approves it), the agent loop pauses the job in an `awaiting_approval` state instead of executing. - Poll/consume the `plan_approval:<job_id>` state row: on `approve` (or `edit`) resume execution; on `decline` abort cleanly. - When `approval.auto_approve` is **true**, resume without waiting (current pre-recorded-decision behaviour). - Surface the awaiting/approved/declined state to the UI so the JobDrawer chip reflects the real gate, and clear the CLI stall note once the hook lands. ## Acceptance criteria - [ ] auto-approve OFF: a `do --plan` job pauses and waits for an approval decision before executing. - [ ] auto-approve ON: the job resumes automatically with no prompt. - [ ] Approve resumes, decline aborts, edit resumes with the revised plan text. - [ ] Per-agent `approval_mode: required` still forces a human gate even when auto-approve is on. - [ ] Tests cover the gate (pause, resume-on-approve, abort-on-decline). ## Notes Follow-up to the checkbox enable/disable fix (#95). That fix made the flag toggle and persist; this issue covers the missing enforcement.

rawan commented

2026-06-10 08:27:19 +00:00

Author

Member

Implementation Spec for Issue #100

Objective

Wire the engine-side plan-approval gate so that approval.auto_approve = false actually pauses a plan-review job in an awaiting_approval state until an operator records a decision via plan.approve, then resumes (approve/edit) or aborts (decline). When auto_approve = true (and no per-agent override forces a gate), the job resumes automatically. The gate must honor per-agent approval_mode and the config auto_approve_agents / require_human_for_agents lists.

Requirements

A plan-review job (mode: plan_review / JobStartMode::PlanReview -> AutonomyMode::PlanOnly) that publishes a plan must NOT silently flip to completed when approval is required. Instead it must enter awaiting_approval and wait for plan_approval:<job_id>.
The decision row at plan_approval:<job_id> (written by plan.approve in crates/hero_shrimp_server/src/rpc/methods/plan.rs) must be consumed by server-side code, not just displayed.
approve / edit -> resume execution (spawn an Execute follow-up on the same workspace; for edit, carry the revised plan text). decline -> abort cleanly (declined terminal status).
Decision logic precedence: per-agent forced gate (config require_human_for_agents contains the agent, or profile approval_mode = Plan) -> always gate; else config auto_approve_agents contains the agent OR approval.auto_approve == true -> auto-resume; else gate.
Surface awaiting_approval / approved / declined to the UI (details.gate_state + job status) so PlanApproval.tsx / JobDrawer.tsx reflect the real gate.
Remove the CLI stall note in do_cmd.rs and the "follow-up not wired" note in plan.rs once landed.
Tests cover pause, resume-on-approve, abort-on-decline, and the per-agent-required override.

Files to Modify/Create

crates/hero_shrimp_server/src/rpc/methods/job/proof_run.rs - insert the approval gate where a plan-review run finishes (the approval_required == true completion block ~L904-922); add the poll-and-resolve helper and the auto-resume spawn.
crates/hero_shrimp_server/src/rpc/methods/job/start.rs - stamp the resolved gate decision (auto_approve vs gate) and the agent name into details at job start so proof_run can decide without re-deriving config.
crates/hero_shrimp_runtime/src/status.rs - recognize awaiting_approval and declined as canonical status words (active / failed respectively).
crates/hero_shrimp_server/src/rpc/methods/plan.rs - clarify/extend the doc + return note now that the engine consumes the row; no behavior change to method_plan_approve (it already persists the row).
crates/hero_shrimp/src/commands/do_cmd.rs - delete/replace the stale stall-warning note (~L90-103).
crates/hero_shrimp_web/ui/src/components/PlanApproval.tsx and crates/hero_shrimp_web/ui/src/components/JobDrawer.tsx - read the new awaiting_approval/approved/declined gate state for the chip.

Implementation Plan

Step 1: Add canonical gate statuses

Files: crates/hero_shrimp_runtime/src/status.rs

In phase(), classify awaiting_approval (and pending_approval) as Phase::Active explicitly (comment + unit test) so it stays non-terminal. Classify declined as Phase::Failed (alongside cancelled).
Add both to the existing canonical_mapping test arrays.
Why: awaiting_approval must stay non-terminal so the resumer/UI keep the job waiting; declined must be terminal-failed so waiters stop polling.
Dependencies: none.

Step 2: Compute and persist the gate decision at job start

Files: crates/hero_shrimp_server/src/rpc/methods/job/start.rs

In method_job_start, after pinned_agent is resolved and start_mode is known, add fn resolve_plan_gate(runtime, mode, agent_name) -> &'static str returning "none" (Yolo/Execute), "auto_approve", or "gate".
- Logic (only meaningful for JobStartMode::PlanReview): profile approval_mode == Some(ApprovalMode::Plan) -> gate; config require_human_for_agents contains agent -> gate; else auto_approve_agents contains agent -> auto_approve; else approval.auto_approve == true -> auto_approve; else gate. Forced gate wins over auto_approve == true.
Persist details["plan_gate"] and always-set details["agent_profile_name"] when an agent is pinned.
Why: centralizes the config/profile read at the synchronous RPC boundary.
Dependencies: none (parallel with Step 1).

Step 3: Insert the approval gate in the plan-review completion path

Files: crates/hero_shrimp_server/src/rpc/methods/job/proof_run.rs

Core change. In the spawned run task, at the block (~L904-922) that runs when row.status == "completed" && final_details["approval_required"] == Some(true) and a plan is published (plan_state_has_current), branch on final_details["plan_gate"]:
- "auto_approve"/"none": keep completed, set gate_state = "approved", optionally record an audit plan_approval:<job_id> row (approve, "auto-approved").
- "gate": set row.status = "awaiting_approval", gate_state = "awaiting_approval", persist BEFORE polling, then call await_plan_approval(...).
Add async fn await_plan_approval(...) that polls database.get_state_value("plan_approval:<artifact_job_id>") on a ~2s tick (timeout via HERO_SHRIMP_PLAN_APPROVAL_TIMEOUT_SECS, default ~3600), parses decision/edits, returns a typed decision.
On approve/edit: spawn an Execute follow-up (pattern from method_job_follow_up in eval.rs): StartProofRunRequest { mode: Yolo, proof: true, workspace_dir = plan job's workspace, session_id = same, parent_job_id = this job, prompt = "Execute the approved plan." (+ edits for edit) }, call start_proof_run(runtime_for_repair, req). Then set gating job status/gate_state = "approved", finished_at, persist.
On decline: set status/gate_state = "declined", last_error, finished_at, persist, release workspace leases.
Use the artifact/slug job id for the key, NOT numeric db id. Guard the resume so it fires once.
Dependencies: Steps 1 and 2.

Step 4: Clear the CLI stall note

Files: crates/hero_shrimp/src/commands/do_cmd.rs

Remove the stale stall-warning note block (~L90-103). Keep CLI do on mode: yolo (out of scope to change) and keep the existing --auto-approve pre-record path; that path now genuinely unblocks the gate.
Dependencies: Step 3.

Step 5: Update plan.rs documentation and return note

Files: crates/hero_shrimp_server/src/rpc/methods/plan.rs

Update the module doc (~L9-17) and the note_about_engine_handoff return field to reflect that the engine now consumes the row and auto-resumes/aborts. No persistence logic change.
Dependencies: Step 3.

Step 6: Crash/restart recovery for awaiting_approval jobs (resumer)

Files: crates/hero_shrimp_server/src/rpc/methods/job/reconcile.rs (and the boot-sweep/resumer)

For jobs parked in awaiting_approval, on restart check plan_approval:<job_id> and apply the same resolve logic as Step 3 (resume on approve/edit, decline -> declined). Extract the resolve+resume logic into a shared function used by both the live poll and the resumer.
Why: the in-flight poll is lost on daemon restart; the resumer revisits the parked job.
Dependencies: Step 3.

Step 7: Surface gate state in the UI chip

Files: crates/hero_shrimp_web/ui/src/components/PlanApproval.tsx, crates/hero_shrimp_web/ui/src/components/JobDrawer.tsx

PlanApproval.tsx: in fetchJobAndApproval, read details.gate_state and drive the chip/buttons from it (Approve/Decline only while awaiting_approval; terminal labels otherwise).
JobDrawer.tsx: map awaiting_approval -> "Awaiting approval", declined -> "Declined". Keep store.ts jobPhase() in lockstep with status.rs.
Dependencies: Steps 1, 3.

Step 8: Tests

Files: proof_run.rs / server test chunks, status.rs, start.rs

status.rs: awaiting_approval active/non-terminal; declined terminal-failed.
resolve_plan_gate table tests: Yolo -> none; PlanReview + auto_approve=false, no agent -> gate; PlanReview + auto_approve=true -> auto_approve; PlanReview + agent in require_human_for_agents (or profile Plan) + auto_approve=true -> gate; PlanReview + agent in auto_approve_agents + auto_approve=false -> auto_approve.
Gate behavior: (a) pause -> awaiting_approval; (b) resume-on-approve -> approved + follow-up spawned; (c) abort-on-decline -> declined + leases released; (d) edit -> resumes with revised plan text.
Factor the resolve/resume core to be unit-testable.
Dependencies: Steps 1-3, 6.

Acceptance Criteria

auto-approve OFF: a plan-review job pauses in awaiting_approval and waits for a decision before executing.
auto-approve ON: the job resumes automatically (status approved, Execute follow-up spawned) with no prompt.
Approve resumes; decline aborts (declined, leases released); edit resumes with the revised plan text.
Per-agent forced gate (require_human_for_agents / profile approval_mode = Plan) still gates even when auto_approve is true.
Tests cover pause, resume-on-approve, abort-on-decline, edit, and resolve_plan_gate precedence.
Stale notes removed from do_cmd.rs and plan.rs; UI chip reflects awaiting_approval/approved/declined.

Notes

Scope: CLI do always sends mode: yolo, so do --plan does NOT hit the new gate; it relies on the existing --auto-approve pre-record. The gate governs mode: plan_review jobs from web/server, matching the issue's framing. Do not change the CLI's mode.
Enum mapping: there is no ApprovalMode::Required variant (Plan | Default | AutoEdit | Yolo | SmartReview). Map per-agent "force gate" primarily to config require_human_for_agents, with profile approval_mode == Some(Plan) as a secondary signal.
Key format: plan_approval:<artifact_job_id> uses the slug job id (matching what plan.approve writes), NOT the numeric db id.
Row consumption: plan.approve uses INSERT OR REPLACE; there is no delete API and the UI reads the same row. Do NOT delete the row after consuming - leave it as audit; guard resume to fire once.
Leases: the declined branch must release workspace leases; the approve/edit follow-up acquires its own via start_proof_run - ensure the parent's leases are released/transferred to avoid a self-conflict (may need force: true on the resume request).

## Implementation Spec for Issue #100 ### Objective Wire the engine-side plan-approval gate so that `approval.auto_approve = false` actually pauses a plan-review job in an `awaiting_approval` state until an operator records a decision via `plan.approve`, then resumes (approve/edit) or aborts (decline). When `auto_approve = true` (and no per-agent override forces a gate), the job resumes automatically. The gate must honor per-agent `approval_mode` and the config `auto_approve_agents` / `require_human_for_agents` lists. ### Requirements - A plan-review job (`mode: plan_review` / `JobStartMode::PlanReview` -> `AutonomyMode::PlanOnly`) that publishes a plan must NOT silently flip to `completed` when approval is required. Instead it must enter `awaiting_approval` and wait for `plan_approval:<job_id>`. - The decision row at `plan_approval:<job_id>` (written by `plan.approve` in `crates/hero_shrimp_server/src/rpc/methods/plan.rs`) must be consumed by server-side code, not just displayed. - `approve` / `edit` -> resume execution (spawn an Execute follow-up on the same workspace; for `edit`, carry the revised plan text). `decline` -> abort cleanly (`declined` terminal status). - Decision logic precedence: per-agent forced gate (config `require_human_for_agents` contains the agent, or profile `approval_mode = Plan`) -> always gate; else config `auto_approve_agents` contains the agent OR `approval.auto_approve == true` -> auto-resume; else gate. - Surface `awaiting_approval` / `approved` / `declined` to the UI (`details.gate_state` + job status) so `PlanApproval.tsx` / `JobDrawer.tsx` reflect the real gate. - Remove the CLI stall note in `do_cmd.rs` and the "follow-up not wired" note in `plan.rs` once landed. - Tests cover pause, resume-on-approve, abort-on-decline, and the per-agent-required override. ### Files to Modify/Create - `crates/hero_shrimp_server/src/rpc/methods/job/proof_run.rs` - insert the approval gate where a plan-review run finishes (the `approval_required == true` completion block ~L904-922); add the poll-and-resolve helper and the auto-resume spawn. - `crates/hero_shrimp_server/src/rpc/methods/job/start.rs` - stamp the resolved gate decision (`auto_approve` vs `gate`) and the agent name into `details` at job start so `proof_run` can decide without re-deriving config. - `crates/hero_shrimp_runtime/src/status.rs` - recognize `awaiting_approval` and `declined` as canonical status words (active / failed respectively). - `crates/hero_shrimp_server/src/rpc/methods/plan.rs` - clarify/extend the doc + return note now that the engine consumes the row; no behavior change to `method_plan_approve` (it already persists the row). - `crates/hero_shrimp/src/commands/do_cmd.rs` - delete/replace the stale stall-warning note (~L90-103). - `crates/hero_shrimp_web/ui/src/components/PlanApproval.tsx` and `crates/hero_shrimp_web/ui/src/components/JobDrawer.tsx` - read the new `awaiting_approval`/`approved`/`declined` gate state for the chip. ### Implementation Plan #### Step 1: Add canonical gate statuses Files: `crates/hero_shrimp_runtime/src/status.rs` - In `phase()`, classify `awaiting_approval` (and `pending_approval`) as `Phase::Active` explicitly (comment + unit test) so it stays non-terminal. Classify `declined` as `Phase::Failed` (alongside `cancelled`). - Add both to the existing `canonical_mapping` test arrays. - Why: `awaiting_approval` must stay non-terminal so the resumer/UI keep the job waiting; `declined` must be terminal-failed so waiters stop polling. Dependencies: none. #### Step 2: Compute and persist the gate decision at job start Files: `crates/hero_shrimp_server/src/rpc/methods/job/start.rs` - In `method_job_start`, after `pinned_agent` is resolved and `start_mode` is known, add `fn resolve_plan_gate(runtime, mode, agent_name) -> &'static str` returning `"none"` (Yolo/Execute), `"auto_approve"`, or `"gate"`. - Logic (only meaningful for `JobStartMode::PlanReview`): profile `approval_mode == Some(ApprovalMode::Plan)` -> gate; config `require_human_for_agents` contains agent -> gate; else `auto_approve_agents` contains agent -> auto_approve; else `approval.auto_approve == true` -> auto_approve; else gate. Forced gate wins over `auto_approve == true`. - Persist `details["plan_gate"]` and always-set `details["agent_profile_name"]` when an agent is pinned. - Why: centralizes the config/profile read at the synchronous RPC boundary. Dependencies: none (parallel with Step 1). #### Step 3: Insert the approval gate in the plan-review completion path Files: `crates/hero_shrimp_server/src/rpc/methods/job/proof_run.rs` - Core change. In the spawned run task, at the block (~L904-922) that runs when `row.status == "completed" && final_details["approval_required"] == Some(true)` and a plan is published (`plan_state_has_current`), branch on `final_details["plan_gate"]`: - `"auto_approve"`/`"none"`: keep `completed`, set `gate_state = "approved"`, optionally record an audit `plan_approval:<job_id>` row (`approve`, "auto-approved"). - `"gate"`: set `row.status = "awaiting_approval"`, `gate_state = "awaiting_approval"`, persist BEFORE polling, then call `await_plan_approval(...)`. - Add `async fn await_plan_approval(...)` that polls `database.get_state_value("plan_approval:<artifact_job_id>")` on a ~2s tick (timeout via `HERO_SHRIMP_PLAN_APPROVAL_TIMEOUT_SECS`, default ~3600), parses `decision`/`edits`, returns a typed decision. - On `approve`/`edit`: spawn an Execute follow-up (pattern from `method_job_follow_up` in `eval.rs`): `StartProofRunRequest { mode: Yolo, proof: true, workspace_dir = plan job's workspace, session_id = same, parent_job_id = this job, prompt = "Execute the approved plan." (+ edits for edit) }`, call `start_proof_run(runtime_for_repair, req)`. Then set gating job `status/gate_state = "approved"`, `finished_at`, persist. - On `decline`: set `status/gate_state = "declined"`, `last_error`, `finished_at`, persist, release workspace leases. - Use the artifact/slug job id for the key, NOT numeric db id. Guard the resume so it fires once. Dependencies: Steps 1 and 2. #### Step 4: Clear the CLI stall note Files: `crates/hero_shrimp/src/commands/do_cmd.rs` - Remove the stale stall-warning note block (~L90-103). Keep CLI `do` on `mode: yolo` (out of scope to change) and keep the existing `--auto-approve` pre-record path; that path now genuinely unblocks the gate. Dependencies: Step 3. #### Step 5: Update plan.rs documentation and return note Files: `crates/hero_shrimp_server/src/rpc/methods/plan.rs` - Update the module doc (~L9-17) and the `note_about_engine_handoff` return field to reflect that the engine now consumes the row and auto-resumes/aborts. No persistence logic change. Dependencies: Step 3. #### Step 6: Crash/restart recovery for awaiting_approval jobs (resumer) Files: `crates/hero_shrimp_server/src/rpc/methods/job/reconcile.rs` (and the boot-sweep/resumer) - For jobs parked in `awaiting_approval`, on restart check `plan_approval:<job_id>` and apply the same resolve logic as Step 3 (resume on approve/edit, decline -> declined). Extract the resolve+resume logic into a shared function used by both the live poll and the resumer. - Why: the in-flight poll is lost on daemon restart; the resumer revisits the parked job. Dependencies: Step 3. #### Step 7: Surface gate state in the UI chip Files: `crates/hero_shrimp_web/ui/src/components/PlanApproval.tsx`, `crates/hero_shrimp_web/ui/src/components/JobDrawer.tsx` - `PlanApproval.tsx`: in `fetchJobAndApproval`, read `details.gate_state` and drive the chip/buttons from it (Approve/Decline only while `awaiting_approval`; terminal labels otherwise). - `JobDrawer.tsx`: map `awaiting_approval` -> "Awaiting approval", `declined` -> "Declined". Keep `store.ts` `jobPhase()` in lockstep with `status.rs`. Dependencies: Steps 1, 3. #### Step 8: Tests Files: `proof_run.rs` / server test chunks, `status.rs`, `start.rs` - `status.rs`: `awaiting_approval` active/non-terminal; `declined` terminal-failed. - `resolve_plan_gate` table tests: Yolo -> none; PlanReview + auto_approve=false, no agent -> gate; PlanReview + auto_approve=true -> auto_approve; PlanReview + agent in require_human_for_agents (or profile Plan) + auto_approve=true -> gate; PlanReview + agent in auto_approve_agents + auto_approve=false -> auto_approve. - Gate behavior: (a) pause -> awaiting_approval; (b) resume-on-approve -> approved + follow-up spawned; (c) abort-on-decline -> declined + leases released; (d) edit -> resumes with revised plan text. - Factor the resolve/resume core to be unit-testable. Dependencies: Steps 1-3, 6. ### Acceptance Criteria - [ ] auto-approve OFF: a plan-review job pauses in `awaiting_approval` and waits for a decision before executing. - [ ] auto-approve ON: the job resumes automatically (status `approved`, Execute follow-up spawned) with no prompt. - [ ] Approve resumes; decline aborts (`declined`, leases released); edit resumes with the revised plan text. - [ ] Per-agent forced gate (`require_human_for_agents` / profile `approval_mode = Plan`) still gates even when `auto_approve` is true. - [ ] Tests cover pause, resume-on-approve, abort-on-decline, edit, and `resolve_plan_gate` precedence. - [ ] Stale notes removed from `do_cmd.rs` and `plan.rs`; UI chip reflects `awaiting_approval`/`approved`/`declined`. ### Notes - Scope: CLI `do` always sends `mode: yolo`, so `do --plan` does NOT hit the new gate; it relies on the existing `--auto-approve` pre-record. The gate governs `mode: plan_review` jobs from web/server, matching the issue's framing. Do not change the CLI's `mode`. - Enum mapping: there is no `ApprovalMode::Required` variant (`Plan | Default | AutoEdit | Yolo | SmartReview`). Map per-agent "force gate" primarily to config `require_human_for_agents`, with profile `approval_mode == Some(Plan)` as a secondary signal. - Key format: `plan_approval:<artifact_job_id>` uses the slug job id (matching what `plan.approve` writes), NOT the numeric db id. - Row consumption: `plan.approve` uses INSERT OR REPLACE; there is no delete API and the UI reads the same row. Do NOT delete the row after consuming - leave it as audit; guard resume to fire once. - Leases: the `declined` branch must release workspace leases; the approve/edit follow-up acquires its own via `start_proof_run` - ensure the parent's leases are released/transferred to avoid a self-conflict (may need `force: true` on the resume request).

rawan commented

2026-06-10 09:03:39 +00:00

Author

Member

Test Results

Build: cargo build --workspace — pass

Crate	Passed	Ignored
hero_shrimp_runtime	575	1
hero_shrimp_server	314	3
hero_shrimp	27	0

New tests for this issue:

status.rs canonical_mapping: awaiting_approval/pending_approval -> Active, declined -> Failed
start.rs resolve_plan_gate_precedence_table: gate-decision precedence (6 cases)
approval_gate.rs parse_plan_decision (6 cases) + poll_plan_decision happy/edit/timeout (3 cases)

Coverage note: Unit tests cover status classification, gate-decision precedence, and decision parsing/polling. The end-to-end gate flow (pause -> awaiting_approval, resume-on-approve spawns Execute follow-up, abort-on-decline releases leases, edit carries revised plan) is composed from these unit-tested building blocks but is not yet covered by an automated integration harness; it remains integration/manual verification.

## Test Results **Build:** `cargo build --workspace` — pass | Crate | Passed | Failed | Ignored | |---|---|---|---| | hero_shrimp_runtime | 575 | 0 | 1 | | hero_shrimp_server | 314 | 0 | 3 | | hero_shrimp | 27 | 0 | 0 | **New tests for this issue:** - status.rs `canonical_mapping`: awaiting_approval/pending_approval -> Active, declined -> Failed - start.rs `resolve_plan_gate_precedence_table`: gate-decision precedence (6 cases) - approval_gate.rs `parse_plan_decision` (6 cases) + `poll_plan_decision` happy/edit/timeout (3 cases) **Coverage note:** Unit tests cover status classification, gate-decision precedence, and decision parsing/polling. The end-to-end gate flow (pause -> awaiting_approval, resume-on-approve spawns Execute follow-up, abort-on-decline releases leases, edit carries revised plan) is composed from these unit-tested building blocks but is not yet covered by an automated integration harness; it remains integration/manual verification.

rawan commented

2026-06-10 09:05:22 +00:00

Author

Member

Implementation Summary

The engine-side plan-approval gate is now wired so approval.auto_approve is enforced for server/web plan_review jobs.

Behavior

A plan_review job that publishes a plan now consults a plan_gate decision computed at job start. When a human gate is required it parks in awaiting_approval instead of completing.
While parked, the run polls plan_approval:<job_id> (the row written by plan.approve). On approve/edit it resumes by spawning an Execute follow-up on the same workspace/session (edit carries the revised plan text); on decline it aborts with status declined and releases workspace leases.
When auto-approve is on (or the agent is in auto_approve_agents), the job resumes automatically and is stamped approved.
Per-agent forced gate (agent in require_human_for_agents, or profile approval_mode = Plan) overrides auto-approve and always gates.
If the daemon restarts while a job is awaiting approval, the reconciler picks up the parked job, reads any recorded decision, and resumes or aborts accordingly.

Files changed

crates/hero_shrimp_runtime/src/status.rs - awaiting_approval/pending_approval classified as active (non-terminal); declined as failed (terminal).
crates/hero_shrimp_server/src/rpc/methods/job/start.rs - resolve_plan_gate(...) computes none/auto_approve/gate and stamps details.plan_gate + details.agent_profile_name.
crates/hero_shrimp_server/src/rpc/methods/job/approval_gate.rs (new) - shared PlanDecision, parse_plan_decision, poll_plan_decision, resume_after_approval, reused by the live run and the reconciler.
crates/hero_shrimp_server/src/rpc/methods/job/proof_run.rs - the gate branch: pause in awaiting_approval, poll, resume on approve/edit, abort on decline; auto-approve path stamps gate_state and records an audit row.
crates/hero_shrimp_server/src/rpc/methods/job/reconcile.rs - restart recovery for awaiting_approval jobs.
crates/hero_shrimp_server/src/rpc/methods/plan.rs - docs/return note updated (engine now consumes the decision); persistence unchanged.
crates/hero_shrimp/src/commands/do_cmd.rs - removed the stale "resume hook not wired" stall note.
crates/hero_shrimp_web/ui/src/components/PlanApproval.tsx, crates/hero_shrimp_web/ui/src/store.ts - UI reads details.gate_state and surfaces awaiting_approval/approved/declined (JobDrawer chip routes through the central jobStatusView mapping in store.ts).

Tests

cargo build --workspace passes. Test totals: hero_shrimp_runtime 575 passed, hero_shrimp_server 314 passed, hero_shrimp 27 passed (0 failures).
New coverage: status classification for the gate states; resolve_plan_gate precedence table (6 cases); parse_plan_decision (6 cases) and poll_plan_decision happy/edit/timeout (3 cases).

Caveat

The end-to-end gate flow (pause -> resume-on-approve spawns the Execute follow-up -> abort-on-decline releases leases -> edit carries revised plan) is composed from the unit-tested building blocks above but is not yet covered by an automated integration harness; that path still warrants manual/integration verification. CLI do --plan remains on mode: yolo and relies on the existing --auto-approve pre-record, which now genuinely unblocks the gate (per the issue's web/server scope).

## Implementation Summary The engine-side plan-approval gate is now wired so `approval.auto_approve` is enforced for server/web `plan_review` jobs. ### Behavior - A `plan_review` job that publishes a plan now consults a `plan_gate` decision computed at job start. When a human gate is required it parks in `awaiting_approval` instead of completing. - While parked, the run polls `plan_approval:<job_id>` (the row written by `plan.approve`). On approve/edit it resumes by spawning an Execute follow-up on the same workspace/session (edit carries the revised plan text); on decline it aborts with status `declined` and releases workspace leases. - When auto-approve is on (or the agent is in `auto_approve_agents`), the job resumes automatically and is stamped `approved`. - Per-agent forced gate (agent in `require_human_for_agents`, or profile `approval_mode = Plan`) overrides auto-approve and always gates. - If the daemon restarts while a job is awaiting approval, the reconciler picks up the parked job, reads any recorded decision, and resumes or aborts accordingly. ### Files changed - `crates/hero_shrimp_runtime/src/status.rs` - `awaiting_approval`/`pending_approval` classified as active (non-terminal); `declined` as failed (terminal). - `crates/hero_shrimp_server/src/rpc/methods/job/start.rs` - `resolve_plan_gate(...)` computes `none`/`auto_approve`/`gate` and stamps `details.plan_gate` + `details.agent_profile_name`. - `crates/hero_shrimp_server/src/rpc/methods/job/approval_gate.rs` (new) - shared `PlanDecision`, `parse_plan_decision`, `poll_plan_decision`, `resume_after_approval`, reused by the live run and the reconciler. - `crates/hero_shrimp_server/src/rpc/methods/job/proof_run.rs` - the gate branch: pause in `awaiting_approval`, poll, resume on approve/edit, abort on decline; auto-approve path stamps `gate_state` and records an audit row. - `crates/hero_shrimp_server/src/rpc/methods/job/reconcile.rs` - restart recovery for `awaiting_approval` jobs. - `crates/hero_shrimp_server/src/rpc/methods/plan.rs` - docs/return note updated (engine now consumes the decision); persistence unchanged. - `crates/hero_shrimp/src/commands/do_cmd.rs` - removed the stale "resume hook not wired" stall note. - `crates/hero_shrimp_web/ui/src/components/PlanApproval.tsx`, `crates/hero_shrimp_web/ui/src/store.ts` - UI reads `details.gate_state` and surfaces `awaiting_approval`/`approved`/`declined` (JobDrawer chip routes through the central `jobStatusView` mapping in store.ts). ### Tests `cargo build --workspace` passes. Test totals: hero_shrimp_runtime 575 passed, hero_shrimp_server 314 passed, hero_shrimp 27 passed (0 failures). New coverage: status classification for the gate states; `resolve_plan_gate` precedence table (6 cases); `parse_plan_decision` (6 cases) and `poll_plan_decision` happy/edit/timeout (3 cases). ### Caveat The end-to-end gate flow (pause -> resume-on-approve spawns the Execute follow-up -> abort-on-decline releases leases -> edit carries revised plan) is composed from the unit-tested building blocks above but is not yet covered by an automated integration harness; that path still warrants manual/integration verification. CLI `do --plan` remains on `mode: yolo` and relies on the existing `--auto-approve` pre-record, which now genuinely unblocks the gate (per the issue's web/server scope).