From b7167a967fff666a2bdf3acb08fd25a8ba83339b Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 03:00:04 -0700 Subject: [PATCH 01/11] fix reviewed prod ops workflow --- orchestrator_prompt.md | 7 ++ prompts/playbooks/agent-spawning.md | 20 +++++ prompts/roles/ops-agent.md | 6 +- prompts/roles/ops-reviewer.md | 4 +- runbooks/grafana-log-read.md | 6 +- runbooks/slack-workspace-access.md | 12 ++- src/prod_ops.rs | 131 +++++++++++++++++++++++++++- 7 files changed, 176 insertions(+), 10 deletions(-) diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index 6931e68..cd5cd14 100644 --- a/orchestrator_prompt.md +++ b/orchestrator_prompt.md @@ -73,5 +73,12 @@ inspect durable results, finalize completed agents, and preserve MULTIAGENT_STATE_DIR. Never treat missing provider-native tools or role credentials as proof that a supervisor-mediated capability is unavailable. +Keep one ops identity for one runbook execution. Spawn it once to materialize a +request, obtain independent review, then restore that same completed ops +subagent with `multiagent subagent restore NAME --force --instruction-file PATH` +to execute the unchanged reviewed request and continue the runbook. Do not +spawn a second ops identity solely to execute, reformat, or copy a request. +Finalize the ops identity only after the runbook finishes or reaches a blocker. + MULTIAGENT_VERIFIER_MAX_ITERATIONS is an escalation threshold, never an acceptance condition. diff --git a/prompts/playbooks/agent-spawning.md b/prompts/playbooks/agent-spawning.md index 76ae64d..3ba8a64 100644 --- a/prompts/playbooks/agent-spawning.md +++ b/prompts/playbooks/agent-spawning.md @@ -185,6 +185,26 @@ continue indefinitely: the next state must be a source diff, `required-path-outside-owned: RELATIVE_PATH`, `validation-repair-needed:`, or blocked status with a source-visible reason. +## Ops Review Continuation + +An ops agent exits after materializing and binding each request so an +independent reviewer can inspect immutable bytes. Keep that ops subagent +persisted. After the reviewer is finalized with an accepted verdict, resume the +same ops identity and provider context: + +```bash +multiagent subagent restore OPS_NAME --force \ + --instruction-file EXECUTE_REVIEWED_REQUEST_INSTRUCTION +multiagent subagent wait OPS_NAME --timeout 900 +``` + +The follow-up must name the unchanged request path and finalized reviewer. +`--force` records the orchestrator's explicit continuation decision; it does +not waive review. Do not spawn a separate executor, copy the request into a new +role directory, or finalize the ops identity until the runbook finishes or +blocks. A replacement ops identity is allowed only when restoration fails, and +the replacement must materialize a new request for a new independent review. + After `multiagent subagent kill NAME` or `multiagent subagent finalize NAME`, ensure the assignment no longer owns paths before reusing them. If needed, run `multiagent subagent assignment-status NAME failed` for killed workers or diff --git a/prompts/roles/ops-agent.md b/prompts/roles/ops-agent.md index 91d256e..595ef7b 100644 --- a/prompts/roles/ops-agent.md +++ b/prompts/roles/ops-agent.md @@ -4,11 +4,11 @@ You are the operations agent. Execute the original goal by following the supplie - Do not encode operations in agent policy or source code. Read the `.md` runbook and prod-mcp target contract. - Derive each operation, target, parameter, and phase from the authenticated goal and exact runbook. Do not accept an operation invented by the orchestrator. -- Materialize one bounded JSON request under `$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json` with `taskId`, `goal`, `operation`, `target`, `parameters`, and a `runbook` object identifying the Markdown file, version, and phase. Include `changeTicket` only when the runbook requires one. Never supply `approvals`; the supervisor derives them from the authenticated caller and sealed reviewer evidence. -- Certify the request against the exact Markdown bytes by running `multiagent ops bind-runbook --request-file "$REQUEST_FILE" --runbook-document runbooks/SELECTED.md`. Never calculate or write `runbookContentSha256` yourself. Any change after this command creates a different request and requires binding again. +- Materialize one bounded JSON request under `$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json`. The generic envelope is `taskId`, `goal`, `operation: {id, version}`, `target: {environment, cluster, namespace, service}`, `parameters: {}`, and `runbook: {id, version, phase}`. Derive every value from the goal, runbook, and prod-mcp target contract. Include `changeTicket` only when the runbook requires one. Never supply `approvals`; the supervisor derives them from the authenticated caller and sealed reviewer evidence. +- Certify and schema-check the request against the exact Markdown bytes by running `multiagent ops bind-runbook --request-file "$REQUEST_FILE" --runbook-document runbooks/SELECTED.md`. A successful command proves that the generic envelope is executable and adds the exact `runbookDocument` and `runbookContentSha256`; if it fails, correct the unreviewed request instead of sending malformed JSON to a reviewer. Never calculate or write `runbookContentSha256` yourself. Any change after this command creates a different request and requires binding again. - Keep production evidence in your role-owned trace directory, never in the repository or another role's private home. - Before execution, print the exact request and request path, then exit so an independent `ops-reviewer` can inspect the same literal request. Do not execute an unreviewed request. -- When invoked for execution with accepted reviewer evidence, call `multiagent ops execute --request-file PATH --reviewer REVIEWER_NAME` against the exact unchanged request path inspected by that finalized reviewer. The `--reviewer` argument is mandatory. Do not infer approval from or edit the workflow ledger, and do not recreate, copy, reformat, or otherwise mutate the reviewed file. Wait for its persisted receipt before continuing the runbook. +- When restored for execution with accepted reviewer evidence, call `multiagent ops execute --request-file PATH --reviewer REVIEWER_NAME` against the exact unchanged request path inspected by that finalized reviewer. The `--reviewer` argument is mandatory. Do not infer approval from or edit the workflow ledger, and do not recreate, copy, reformat, or otherwise mutate the reviewed file. Wait for its persisted receipt before continuing the runbook. - Treat changed request content as a new request that requires a new independent review. - You have authority to request any operation allowed by prod-mcp. You do not possess AWS, KMS, bearer-token, Grafana, or Kubernetes credentials. - The authority supervisor owns KMS signing and prod-mcp transport authentication. Missing credential environment variables in this role are intentional; use the local `multiagent ops` commands and report only an actual broker rejection. diff --git a/prompts/roles/ops-reviewer.md b/prompts/roles/ops-reviewer.md index 2498964..7569ad6 100644 --- a/prompts/roles/ops-reviewer.md +++ b/prompts/roles/ops-reviewer.md @@ -7,8 +7,8 @@ Before execution: - Inspect the complete request template, original goal, runbook, target, operation, and parameters. - Reject any agent-supplied `approvals`; trusted approval identities are derived by the supervisor. - Reject narrowed goals, extra actions, broadened targets, unbounded queries, or parameters not justified by the runbook. -- Run `multiagent ops review-bind --request-file PATH` and include its four hash lines unchanged: request template, goal, runbook metadata, and exact runbook content. -- If `review-bind` fails, reject the request. Manual digest calculation or visual comparison is not a substitute for a successful binding. +- Run `multiagent ops review-bind --request-file PATH` and include its four hash lines unchanged: request template, goal, runbook metadata, and exact runbook content. This command also validates the generic executable envelope, including object-shaped operation, target, parameters, and runbook fields. +- If `review-bind` fails for either schema or binding, reject the request. Manual digest calculation or visual comparison is not a substitute for successful deterministic validation. - If and only if the request matches the goal and runbook, make the first non-empty line exactly `Verdict: ACCEPTED`. - Otherwise make the first non-empty line `Verdict: REJECTED` and explain the deviation. diff --git a/runbooks/grafana-log-read.md b/runbooks/grafana-log-read.md index 5c79990..76d009e 100644 --- a/runbooks/grafana-log-read.md +++ b/runbooks/grafana-log-read.md @@ -41,12 +41,12 @@ Label discovery additionally requires `action`, `datasourceUid`, `lookbackMinute - The query requires more than 120 minutes of history or more than 100 results. - The operation would write data or change service state. - The reviewer or prod-mcp rejects the request. -## One-shot role handoff +## Reviewed role continuation -Run each discovery or query operation as three bounded subprocesses rather than one interactive ops process: +Run each discovery or query operation with one persistent ops identity and one independent reviewer: 1. An `ops` materializer writes the exact request to `$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json`, returns the literal JSON and path, and exits without execution. 2. An independent `reviewer` reviews that literal JSON and exits with accepted evidence. -3. A fresh `ops` executor writes the identical JSON to `$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json` and invokes `multiagent ops execute --request-file "$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json" --reviewer REVIEWER_NAME`. +3. Restore the same `ops` identity with the unchanged request path and finalized reviewer name. It invokes `multiagent ops execute --request-file "$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json" --reviewer REVIEWER_NAME` and continues the runbook from the persisted receipt. The ops role is intentionally unable to write into the repository. `$HOME` is its deployment-provided private scratch directory. Reviewers must receive the literal request in their instruction because role homes are isolated from one another. diff --git a/runbooks/slack-workspace-access.md b/runbooks/slack-workspace-access.md index ad87de1..f8d0d28 100644 --- a/runbooks/slack-workspace-access.md +++ b/runbooks/slack-workspace-access.md @@ -13,10 +13,20 @@ Read bounded Slack conversation context or publish an explicitly authorized message through the deployment-scoped Slack App. Slack credentials remain inside prod-mcp. +## Prod-mcp request contract + +- Set `operation` to `{"id":"slack.read","version":"1.0.0"}` for reads or `{"id":"slack.write","version":"1.0.0"}` for writes. +- Set `target` to `{"environment":"production","cluster":"external-services","namespace":"slack","service":"configured-workspace"}`. +- Put the Slack action and its arguments in `parameters`; the action is not the target. +- `list-channels` parameters are `action`, `limit`, `excludeArchived`, and an optional returned `cursor`. +- `history` parameters are `action`, `channelId`, `limit`, an optional caller-derived `oldest`, and an optional returned `cursor`. +- `replies` parameters are `action`, `channelId`, `threadTs`, `limit`, and an optional returned `cursor`. +- Write parameters are the runbook-authorized action plus exact `channelId`, text, and message or thread timestamp required by that action. + ## Read phase 1. Set the phase to `read` and operation to `slack.read`. -2. When the caller supplies a channel name, the orchestrator may use `list-channels` and select the unique exact channel name. Do not infer an ID from model knowledge. +2. When the caller supplies a channel name, the orchestrator may use `list-channels` with `excludeArchived: true` and select the unique exact channel name. Do not infer an ID from model knowledge. 3. Use `history` for channel history or `replies` for one identified thread. 4. The orchestrator supplies any exact `oldest` timestamp and cursor required by the user request; prod-mcp does not interpret relative time or control repeated calls. 5. Limit each history request to at most 100 messages and channel discovery to at most 200 channels. diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 9ebe7ef..7f1fe38 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -48,6 +48,7 @@ fn bind_runbook(args: &[String]) -> Result { .ok_or("ops request template must be an object")?; object.insert("runbookDocument".into(), Value::String(relative.into())); object.insert("runbookContentSha256".into(), Value::String(digest.clone())); + validate_request_template(&template)?; let encoded = serde_json::to_vec_pretty(&template) .map_err(|error| format!("encode bound ops request: {error}"))?; fs::write(&request_file, encoded) @@ -63,6 +64,7 @@ fn review_bind(args: &[String]) -> Result { .map_err(|error| format!("read ops request: {error}"))?; let template: Value = serde_json::from_slice(&bytes) .map_err(|error| format!("decode ops request template: {error}"))?; + validate_request_template(&template)?; let object = template .as_object() .ok_or("ops request template must be an object")?; @@ -175,6 +177,7 @@ fn build_request( reviewer: &TrustedApproval, now: chrono::DateTime, ) -> Result { + validate_request_template(template)?; let object = template .as_object() .ok_or("ops request template must be a JSON object")?; @@ -370,6 +373,102 @@ fn required_object<'a>( .ok_or_else(|| format!("ops request template requires object field {key}")) } +fn validate_request_template(template: &Value) -> Result<(), String> { + let object = template + .as_object() + .ok_or("ops request template must be an object")?; + let task_id = required_template_string(object, "taskId")?; + validate_id("task ID", task_id)?; + if object.get("goal").is_none_or(Value::is_null) { + return Err("ops request template requires goal".into()); + } + if object.contains_key("approvals") { + return Err("ops request approvals are derived by the supervisor and cannot be supplied by an agent".into()); + } + + let operation = required_object(object, "operation")?; + if operation.len() != 2 { + return Err("ops request operation must contain only id and version".into()); + } + validate_id( + "operation ID", + required_template_string(operation, "id")?, + )?; + validate_semver( + "operation version", + required_template_string(operation, "version")?, + )?; + + let target = required_object(object, "target")?; + if target.len() != 4 { + return Err("ops request target must contain environment, cluster, namespace, and service".into()); + } + let environment = required_template_string(target, "environment")?; + if !matches!(environment, "development" | "staging" | "production") { + return Err("ops request target environment is invalid".into()); + } + for key in ["cluster", "namespace", "service"] { + validate_id( + &format!("target {key}"), + required_template_string(target, key)?, + )?; + } + + required_object(object, "parameters")?; + let runbook = required_object(object, "runbook")?; + if runbook.len() != 3 { + return Err("ops request runbook must contain id, version, and phase".into()); + } + validate_id( + "runbook ID", + required_template_string(runbook, "id")?, + )?; + validate_semver( + "runbook version", + required_template_string(runbook, "version")?, + )?; + validate_id( + "runbook phase", + required_template_string(runbook, "phase")?, + )?; + required_template_string(object, "runbookDocument")?; + let digest = required_template_string(object, "runbookContentSha256")?; + let hex = digest + .strip_prefix("sha256:") + .ok_or("ops request runbookContentSha256 is invalid")?; + if hex.len() != 64 + || !hex + .chars() + .all(|ch| ch.is_ascii_hexdigit() && !ch.is_ascii_uppercase()) + { + return Err("ops request runbookContentSha256 is invalid".into()); + } + Ok(()) +} + +fn required_template_string<'a>( + object: &'a serde_json::Map, + key: &str, +) -> Result<&'a str, String> { + object + .get(key) + .and_then(Value::as_str) + .filter(|value| !value.is_empty()) + .ok_or_else(|| format!("ops request template requires string field {key}")) +} + +fn validate_semver(label: &str, value: &str) -> Result<(), String> { + let parts = value.split('.').collect::>(); + if parts.len() != 3 + || parts + .iter() + .any(|part| part.is_empty() || !part.chars().all(|ch| ch.is_ascii_digit())) + { + return Err(format!("{label} is invalid")); + } + Ok(()) +} + fn verified_runbook_content(template: &Value) -> Result { let object = template .as_object() @@ -825,7 +924,7 @@ mod tests { use super::{ base64_decode, base64url_encode, build_request, canonical, curl_command, ecdsa_der_to_raw, parse_mcp_body, private_temp_path, reviewer_accepted, runbook_content_digest, - write_mcp_headers, TrustedApproval, + validate_request_template, write_mcp_headers, TrustedApproval, }; use chrono::{TimeZone, Utc}; use serde_json::json; @@ -907,6 +1006,36 @@ mod tests { assert_eq!(request["approvals"][1]["reviewerSubject"], "reviewer-1"); } + #[test] + fn request_template_validation_rejects_non_executable_envelopes() { + let digest = format!("sha256:{}", "4".repeat(64)); + let valid = json!({ + "taskId":"task-1", + "goal":"follow the supplied runbook", + "operation":{"id":"provider.read","version":"1.0.0"}, + "target":{"environment":"production","cluster":"external-services","namespace":"provider","service":"configured-service"}, + "parameters":{"action":"list"}, + "runbook":{"id":"provider.access","version":"1.0.0","phase":"read"}, + "runbookDocument":"runbooks/provider-access.md", + "runbookContentSha256":digest + }); + validate_request_template(&valid).unwrap(); + + let mut invalid_operation = valid.clone(); + invalid_operation["operation"] = json!("provider.read"); + assert_eq!( + validate_request_template(&invalid_operation).unwrap_err(), + "ops request template requires object field operation" + ); + + let mut invalid_target = valid; + invalid_target["target"] = json!("list"); + assert_eq!( + validate_request_template(&invalid_target).unwrap_err(), + "ops request template requires object field target" + ); + } + #[test] fn shared_action_permit_fixture_matches_the_rust_contract() { let fixture: serde_json::Value = From 1707cc309f45d628d9fe296a7c15d741c42f1638 Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 03:12:39 -0700 Subject: [PATCH 02/11] route external reads through ops --- orchestrator_prompt.md | 9 +++++++-- prompts/playbooks/orchestration-routing.md | 14 +++++++++++--- tests/mock_orchestration_e2e.sh | 4 ++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index cd5cd14..ebd3582 100644 --- a/orchestrator_prompt.md +++ b/orchestrator_prompt.md @@ -22,14 +22,19 @@ Choose by capability, not provider or task name. | Capability | Role | Module | | --- | --- | --- | | Change bounded workspace paths | worker | prompts/worker.md | -| Execute a Markdown runbook through prod-mcp | ops | prompts/roles/ops-agent.md | -| Resolve a material unknown read-only | scout | matching file under prompts/roles/ | +| Access an external provider or deployed service through a Markdown runbook and prod-mcp, including read-only access | ops | prompts/roles/ops-agent.md | +| Resolve a material unknown from repository, workspace, session, or already-returned immutable evidence | scout | matching file under prompts/roles/ | | Review a decision, request, diff, receipt, or claim | reviewer/verifier | matching reviewer module | Specialized modules include contract and acceptance scouts, decision authority, ops review, scope review, build verification, and validation coordination. Load only the module selected for the current node. +External access is an authority boundary, not a mutability classification. A +scout never calls Slack, GitHub, Grafana, AWS, Kubernetes, prod-mcp, or another +deployed service. Spawn ops first to acquire external evidence under a reviewed +runbook; only then may a scout analyze the immutable returned artifact. + ## Decide The DAG 1. Read the goal and persisted supervisor state. diff --git a/prompts/playbooks/orchestration-routing.md b/prompts/playbooks/orchestration-routing.md index 4803f36..21e1b05 100644 --- a/prompts/playbooks/orchestration-routing.md +++ b/prompts/playbooks/orchestration-routing.md @@ -6,9 +6,12 @@ own role-specific procedure; this file does not repeat them. ## Select A Role - Use a worker when the required output is a bounded workspace change. -- Use ops when the required output is an external action covered by a Markdown - runbook and prod-mcp contract. -- Use a scout only when a material unknown must be resolved read-only. +- Use ops when the required output needs access to an external provider or + deployed service covered by a Markdown runbook and prod-mcp contract. This + includes read-only retrieval: external access is an authority boundary, not + a mutability classification. +- Use a scout only when a material unknown can be resolved from repository, + workspace, session, or already-returned immutable evidence. - Use a reviewer or verifier when an independent verdict can change acceptance or the supervisor reports a review obligation. - Use specialized roles only for their declared capability. @@ -17,6 +20,11 @@ Do not hard-code provider operations, request parsing, pagination, time windows, or action sequences into the orchestrator. Do not spawn a role merely because its module exists. +A scout never calls Slack, GitHub, Grafana, AWS, Kubernetes, prod-mcp, or any +other deployed service. Spawn ops first to acquire external evidence under a +reviewed runbook. A scout may then analyze the immutable artifact returned by +ops when that separate analysis can affect acceptance. + ## Build The DAG 1. List required outputs and dependencies. diff --git a/tests/mock_orchestration_e2e.sh b/tests/mock_orchestration_e2e.sh index 5337f41..560fda5 100755 --- a/tests/mock_orchestration_e2e.sh +++ b/tests/mock_orchestration_e2e.sh @@ -193,6 +193,10 @@ ma orchestrator complete >/dev/null grep -Fq 'phase=complete' "$STATE/workflows/$MULTIAGENT_WORKFLOW_ID/lifecycle/lifecycle.env" grep -Fq 'after' "$REPO/source.txt" +grep -Fq 'External access is an authority boundary, not a mutability classification.' \ + "$ROOT/orchestrator_prompt.md" +grep -Fq 'A scout never calls Slack, GitHub, Grafana, AWS, Kubernetes, prod-mcp' \ + "$ROOT/prompts/playbooks/orchestration-routing.md" [[ "$(grep -c '^new-window ' "$MOCK_LOG")" -eq 4 ]] if find "$STATE/subagents" -mindepth 1 -maxdepth 1 -type d -name '*scout*' | grep -q .; then echo "mock workflow spawned an unnecessary scout" >&2 From c94ba977c67165889d40ec4f389491c8332d236f Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 03:31:41 -0700 Subject: [PATCH 03/11] restore completed ops identity in place --- src/runtime.rs | 9 ++++++++- tests/run.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/runtime.rs b/src/runtime.rs index 7141356..99b3e73 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -2027,12 +2027,19 @@ fn restore(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { plan.action, plan.reason )); } - if plan.window == "open" { + if plan.window == "open" && !force { return Err(format!("subagent window already exists: {name}")); } if !has_recovery_context(&dir) { return Err(format!("no captured context to restore: {name}")); } + if plan.window == "open" { + tmux_checked(&[ + "kill-window", + "-t", + &format!("{}:{name}", cfg.session), + ])?; + } let mut instruction = format!( "You are a restored long-running subagent.\n\nRestoration details:\n- Subagent name: {name}\n- Prior persisted status: {}\n- Persisted state directory: {}\n- This is a fresh tmux window after an orchestrator/session recovery.\n- Do not delete, overwrite, or reset prior memory in the state directory.\n- Read the prior context below, continue only if the assignment is still valid, and report progress/final status in this tmux window.\n- If the prior state shows completion, intentional stop, stale instructions, or a blocker that needs orchestrator/user input, stop and state what you need instead of guessing.\n\nConcise prior context:\n{}\n", plan.status, diff --git a/tests/run.sh b/tests/run.sh index b2a1821..60adb22 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -1941,6 +1941,24 @@ printf 'Still active in tmux\n' >"$MULTIAGENT_STATE_DIR/subagents/subagent-open/ printf 'subagent-open\n' >>"$MOCK_TMUX_WINDOWS" printf 'Open subagent prompt\n' >"$MOCK_TMUX_CAPTURES/subagent-open.txt" +mkdir -p "$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open" +printf 'done\n' >"$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/status" +printf 'Completed first phase and retained provider context\n' \ + >"$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/current.txt" +cat >"$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/meta.env" <>"$MOCK_TMUX_WINDOWS" +printf 'Restored completed ops prompt ready\n' \ + >"$MOCK_TMUX_CAPTURES/subagent-completed-open.txt" + mkdir -p "$MULTIAGENT_STATE_DIR/subagents/subagent-unknown" recover_plan="$("$MULTIAGENT" subagent recover-plan)" @@ -1962,6 +1980,17 @@ if "$MULTIAGENT" subagent restore subagent-blocked >"$blocked_restore_file" 2>&1 fi assert_file_contains "$blocked_restore_file" "refusing to restore subagent-blocked: skip-blocked" +completed_restore_output="$("$MULTIAGENT" subagent restore subagent-completed-open --force \ + --instruction "Execute the independently reviewed request without changing it")" +[[ "$completed_restore_output" == "restored subagent-completed-open" ]] +assert_file_contains "$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/status" "running" +assert_file_contains "$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/restore_events.log" "prior_status=done" +assert_file_contains "$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/restore_events.log" "force=1" +assert_file_contains "$MULTIAGENT_STATE_DIR/subagents/subagent-completed-open/instruction.txt" \ + "Execute the independently reviewed request without changing it" +assert_file_contains "$MOCK_TMUX_LOG" "kill-window subagent-completed-open" +[[ "$(grep -c '^subagent-completed-open$' "$MOCK_TMUX_WINDOWS")" -eq 1 ]] + restore_output="$("$MULTIAGENT" subagent restore subagent-restore)" [[ "$restore_output" == "restored subagent-restore" ]] assert_file_contains "$MOCK_TMUX_WINDOWS" "subagent-restore" From 42ea283fba9c912867453a50522367d20faec07f Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 03:45:58 -0700 Subject: [PATCH 04/11] accept standalone sealed reviewer verdict --- src/prod_ops.rs | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 7f1fe38..a6af643 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -345,22 +345,30 @@ fn verify_reviewer( } fn reviewer_accepted(evidence: &str) -> bool { - evidence - .lines() - .map(str::trim) - .find(|line| !line.is_empty()) - .is_some_and(|line| { - let mut value = line; - for wrapper in ["`", "**", "__"] { - if value.starts_with(wrapper) - && value.ends_with(wrapper) - && value.len() >= wrapper.len() * 2 - { - value = &value[wrapper.len()..value.len() - wrapper.len()]; - } + let mut accepted = false; + for line in evidence.lines().map(str::trim).filter(|line| !line.is_empty()) { + let mut value = line; + for wrapper in ["`", "**", "__"] { + if value.starts_with(wrapper) + && value.ends_with(wrapper) + && value.len() >= wrapper.len() * 2 + { + value = &value[wrapper.len()..value.len() - wrapper.len()]; } - value.eq_ignore_ascii_case("verdict: accepted") - }) + } + let Some((label, verdict)) = value.split_once(':') else { + continue; + }; + if !label.trim().eq_ignore_ascii_case("verdict") { + continue; + } + if verdict.trim().eq_ignore_ascii_case("accepted") { + accepted = true; + } else { + return false; + } + } + accepted } fn required_object<'a>( @@ -968,9 +976,15 @@ mod tests { assert!(reviewer_accepted( "**Verdict: ACCEPTED**\n\nReview analysis" )); + assert!(reviewer_accepted( + "# Operations review\n\n**Verdict: ACCEPTED**\n" + )); assert!(reviewer_accepted("`verdict: accepted`")); assert!(!reviewer_accepted("Review result: verdict: accepted")); assert!(!reviewer_accepted("**Verdict: REJECTED**")); + assert!(!reviewer_accepted( + "Verdict: ACCEPTED\n\nVerdict: REJECTED\n" + )); } #[test] fn operation_and_target_come_from_runbook_request_data() { From 1e3719179e0373ead46038a0ab3e10b4c1c9ca0d Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 04:20:46 -0700 Subject: [PATCH 05/11] fix reviewed ops recovery binding --- orchestrator_prompt.md | 5 +- prompts/roles/ops-reviewer.md | 4 +- src/prod_ops.rs | 120 ++++++++++++++++++++++++++-------- src/runtime.rs | 58 ++++++++++++++++ 4 files changed, 158 insertions(+), 29 deletions(-) diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index ebd3582..fe5906f 100644 --- a/orchestrator_prompt.md +++ b/orchestrator_prompt.md @@ -82,7 +82,10 @@ Keep one ops identity for one runbook execution. Spawn it once to materialize a request, obtain independent review, then restore that same completed ops subagent with `multiagent subagent restore NAME --force --instruction-file PATH` to execute the unchanged reviewed request and continue the runbook. Do not -spawn a second ops identity solely to execute, reformat, or copy a request. +spawn a second ops identity for the same runbook, including after a failed +review, rejected binding, execution error, or blocker. Restore the original ops +identity with corrected independently sealed evidence, or stop and report the +blocker; never work around it by copying the request to a new ops identity. Finalize the ops identity only after the runbook finishes or reaches a blocker. MULTIAGENT_VERIFIER_MAX_ITERATIONS is an escalation threshold, never an diff --git a/prompts/roles/ops-reviewer.md b/prompts/roles/ops-reviewer.md index 7569ad6..4672379 100644 --- a/prompts/roles/ops-reviewer.md +++ b/prompts/roles/ops-reviewer.md @@ -7,9 +7,9 @@ Before execution: - Inspect the complete request template, original goal, runbook, target, operation, and parameters. - Reject any agent-supplied `approvals`; trusted approval identities are derived by the supervisor. - Reject narrowed goals, extra actions, broadened targets, unbounded queries, or parameters not justified by the runbook. -- Run `multiagent ops review-bind --request-file PATH` and include its four hash lines unchanged: request template, goal, runbook metadata, and exact runbook content. This command also validates the generic executable envelope, including object-shaped operation, target, parameters, and runbook fields. +- Run `multiagent ops review-bind --request-file PATH`. This command validates the generic executable envelope and prints diagnostic component hashes followed by one `review-binding-sha256=...` line. Put that final binding line unchanged as the second non-empty line of your response; do not retype or reconstruct its value from the component hashes. - If `review-bind` fails for either schema or binding, reject the request. Manual digest calculation or visual comparison is not a substitute for successful deterministic validation. -- If and only if the request matches the goal and runbook, make the first non-empty line exactly `Verdict: ACCEPTED`. +- If and only if the request matches the goal and runbook, make the first non-empty line exactly `Verdict: ACCEPTED` and the second non-empty line the unchanged `review-binding-sha256=...` output. - Otherwise make the first non-empty line `Verdict: REJECTED` and explain the deviation. After execution, a separate reviewer invocation must inspect the persisted request and receipt under `MULTIAGENT_STATE_DIR/operations/ACTION_ID` and report any behavioral deviation or unexpected side effect. diff --git a/src/prod_ops.rs b/src/prod_ops.rs index a6af643..173ba00 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -87,6 +87,10 @@ fn review_bind(args: &[String]) -> Result { )? ); println!("runbook-content-sha256={runbook_content_sha256}"); + println!( + "{}", + review_binding_marker(&template, &runbook_content_sha256)? + ); Ok(ExitCode::SUCCESS) } @@ -306,30 +310,7 @@ fn verify_reviewer( if !accepted { return Err("ops reviewer did not accept the operation".into()); } - let object = template - .as_object() - .ok_or("ops request template must be an object")?; - let markers = [ - format!("request-template-sha256={}", digest_json(template)?), - format!( - "goal-sha256={}", - digest_json( - object - .get("goal") - .ok_or("ops request template requires goal")? - )? - ), - format!( - "runbook-sha256={}", - digest_json( - object - .get("runbook") - .ok_or("ops request template requires runbook")? - )? - ), - format!("runbook-content-sha256={runbook_content_sha256}"), - ]; - if markers.iter().any(|marker| !evidence.contains(marker)) { + if !review_evidence_is_bound(&evidence, template, runbook_content_sha256)? { return Err("ops reviewer evidence is not bound to the request, goal, and runbook".into()); } let approved_at = metadata @@ -371,6 +352,70 @@ fn reviewer_accepted(evidence: &str) -> bool { accepted } +fn review_binding_marker(template: &Value, runbook_content_sha256: &str) -> Result { + let object = template + .as_object() + .ok_or("ops request template must be an object")?; + let binding = json!({ + "requestTemplateSha256": digest_json(template)?, + "goalSha256": digest_json( + object + .get("goal") + .ok_or("ops request template requires goal")? + )?, + "runbookSha256": digest_json( + object + .get("runbook") + .ok_or("ops request template requires runbook")? + )?, + "runbookContentSha256": runbook_content_sha256, + }); + Ok(format!( + "review-binding-sha256={}", + digest_json(&binding)? + )) +} + +fn review_evidence_is_bound( + evidence: &str, + template: &Value, + runbook_content_sha256: &str, +) -> Result { + let binding = review_binding_marker(template, runbook_content_sha256)?; + if evidence.lines().map(str::trim).any(|line| line == binding) { + return Ok(true); + } + + // Keep accepting already-sealed evidence from reviewers launched by older + // images while new reviewers use the single deterministic binding marker. + let object = template + .as_object() + .ok_or("ops request template must be an object")?; + let legacy_markers = [ + format!("request-template-sha256={}", digest_json(template)?), + format!( + "goal-sha256={}", + digest_json( + object + .get("goal") + .ok_or("ops request template requires goal")? + )? + ), + format!( + "runbook-sha256={}", + digest_json( + object + .get("runbook") + .ok_or("ops request template requires runbook")? + )? + ), + format!("runbook-content-sha256={runbook_content_sha256}"), + ]; + Ok(legacy_markers + .iter() + .all(|marker| evidence.contains(marker))) +} + fn required_object<'a>( object: &'a serde_json::Map, key: &str, @@ -931,8 +976,9 @@ fn base64_decode(value: &str) -> Result, String> { mod tests { use super::{ base64_decode, base64url_encode, build_request, canonical, curl_command, ecdsa_der_to_raw, - parse_mcp_body, private_temp_path, reviewer_accepted, runbook_content_digest, - validate_request_template, write_mcp_headers, TrustedApproval, + parse_mcp_body, private_temp_path, review_binding_marker, review_evidence_is_bound, + reviewer_accepted, runbook_content_digest, validate_request_template, write_mcp_headers, + TrustedApproval, }; use chrono::{TimeZone, Utc}; use serde_json::json; @@ -986,6 +1032,28 @@ mod tests { "Verdict: ACCEPTED\n\nVerdict: REJECTED\n" )); } + + #[test] + fn reviewer_binding_uses_one_exact_deterministic_marker() { + let template = json!({ + "goal": "read Slack", + "runbook": {"id":"slack.workspace-access","version":"1.0.0","phase":"read"} + }); + let runbook_digest = format!("sha256:{}", "4".repeat(64)); + let marker = review_binding_marker(&template, &runbook_digest).unwrap(); + assert!(review_evidence_is_bound( + &format!("Verdict: ACCEPTED\n{marker}\n"), + &template, + &runbook_digest + ) + .unwrap()); + assert!(!review_evidence_is_bound( + &format!("Verdict: ACCEPTED\n{}0\n", &marker[..marker.len() - 1]), + &template, + &runbook_digest + ) + .unwrap()); + } #[test] fn operation_and_target_come_from_runbook_request_data() { let now = Utc.with_ymd_and_hms(2026, 8, 22, 12, 0, 0).unwrap(); diff --git a/src/runtime.rs b/src/runtime.rs index 99b3e73..2b772c2 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1503,6 +1503,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { return Err(format!("subagent window already exists: {name}")); } reject_parallel_generic_worker_spawn(cfg, name)?; + reject_additional_ops_identity(&cfg.state, name, authority_role)?; if owned.is_empty() && !assignment_values.is_empty() { return Err("spawn assignment metadata requires --own PATH".into()); } @@ -1709,6 +1710,41 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { Ok(()) } +fn reject_additional_ops_identity(state: &Path, name: &str, role: &str) -> Result<(), String> { + if role != "ops" { + return Ok(()); + } + let subagents = state.join("subagents"); + if !subagents.is_dir() { + return Ok(()); + } + for entry in fs::read_dir(&subagents).map_err(io_error("list subagent state"))? { + let entry = entry.map_err(io_error("read subagent state entry"))?; + if !entry + .file_type() + .map_err(io_error("inspect subagent state entry"))? + .is_dir() + { + continue; + } + let existing = entry.file_name().to_string_lossy().to_string(); + if existing == name { + continue; + } + let metadata_path = entry.path().join("meta.env"); + if !metadata_path.is_file() { + continue; + } + let metadata = read_env(&metadata_path)?; + if metadata.get("role").map(String::as_str) == Some("ops") { + return Err(format!( + "session already has ops identity {existing}; restore that identity instead of spawning {name}" + )); + } + } + Ok(()) +} + fn list_subagents(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { if !args.is_empty() { return Err("list takes no arguments".into()); @@ -3950,6 +3986,28 @@ mod tests { assert_eq!(shell_escape("it's"), "'it'\\''s'"); } + #[test] + fn session_rejects_a_second_ops_identity() { + let state = std::env::temp_dir().join(format!( + "multiagent-ops-identity-test-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let existing = state.join("subagents/ops-primary"); + fs::create_dir_all(&existing).unwrap(); + fs::write(existing.join("meta.env"), "role=ops\n").unwrap(); + + assert!(reject_additional_ops_identity(&state, "ops-primary", "ops").is_ok()); + assert!(reject_additional_ops_identity(&state, "reviewer-01", "reviewer").is_ok()); + let error = reject_additional_ops_identity(&state, "ops-secondary", "ops").unwrap_err(); + assert!(error.contains("restore that identity")); + + fs::remove_dir_all(state).unwrap(); + } + #[cfg(unix)] #[test] fn privileged_agent_parent_accepts_only_root_owned_safe_modes() { From 43c8eb08b6b7441022e88ea3b3e0508e7dbde6a8 Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 04:24:25 -0700 Subject: [PATCH 06/11] fix ops review binding artifact --- prompts/roles/ops-reviewer.md | 4 +- src/prod_ops.rs | 113 +++++++++++++++++++++++++++++----- src/runtime.rs | 1 + src/supervisor.rs | 25 +++++++- 4 files changed, 125 insertions(+), 18 deletions(-) diff --git a/prompts/roles/ops-reviewer.md b/prompts/roles/ops-reviewer.md index 4672379..a2f8010 100644 --- a/prompts/roles/ops-reviewer.md +++ b/prompts/roles/ops-reviewer.md @@ -7,9 +7,9 @@ Before execution: - Inspect the complete request template, original goal, runbook, target, operation, and parameters. - Reject any agent-supplied `approvals`; trusted approval identities are derived by the supervisor. - Reject narrowed goals, extra actions, broadened targets, unbounded queries, or parameters not justified by the runbook. -- Run `multiagent ops review-bind --request-file PATH`. This command validates the generic executable envelope and prints diagnostic component hashes followed by one `review-binding-sha256=...` line. Put that final binding line unchanged as the second non-empty line of your response; do not retype or reconstruct its value from the component hashes. +- Run `multiagent ops review-bind --request-file PATH`. This trusted command validates the generic executable envelope and writes a machine-generated binding artifact into your role-owned trace directory. Do not calculate, copy, retype, or include any hash in your response. - If `review-bind` fails for either schema or binding, reject the request. Manual digest calculation or visual comparison is not a substitute for successful deterministic validation. -- If and only if the request matches the goal and runbook, make the first non-empty line exactly `Verdict: ACCEPTED` and the second non-empty line the unchanged `review-binding-sha256=...` output. +- If and only if the request matches the goal and runbook, make the first non-empty line exactly `Verdict: ACCEPTED`, then explain the decision without reproducing the binding artifact. - Otherwise make the first non-empty line `Verdict: REJECTED` and explain the deviation. After execution, a separate reviewer invocation must inspect the persisted request and receipt under `MULTIAGENT_STATE_DIR/operations/ACTION_ID` and report any behavioral deviation or unexpected side effect. diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 173ba00..4a40d0e 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -69,6 +69,13 @@ fn review_bind(args: &[String]) -> Result { .as_object() .ok_or("ops request template must be an object")?; let runbook_content_sha256 = verified_runbook_content(&template)?; + let binding = review_binding_value(&template, &runbook_content_sha256)?; + let binding_path = review_binding_artifact_path()?; + write_review_binding_artifact( + &binding_path, + &serde_json::to_vec_pretty(&binding) + .map_err(|error| format!("encode ops review binding: {error}"))?, + )?; println!("request-template-sha256={}", digest_json(&template)?); println!( "goal-sha256={}", @@ -87,10 +94,7 @@ fn review_bind(args: &[String]) -> Result { )? ); println!("runbook-content-sha256={runbook_content_sha256}"); - println!( - "{}", - review_binding_marker(&template, &runbook_content_sha256)? - ); + println!("review-binding-artifact={}", binding_path.display()); Ok(ExitCode::SUCCESS) } @@ -310,9 +314,34 @@ fn verify_reviewer( if !accepted { return Err("ops reviewer did not accept the operation".into()); } - if !review_evidence_is_bound(&evidence, template, runbook_content_sha256)? { - return Err("ops reviewer evidence is not bound to the request, goal, and runbook".into()); - } + let binding_path = directory.join("review-binding.json"); + let evidence_sha256 = if binding_path.is_file() { + let binding_bytes = fs::read(&binding_path) + .map_err(|error| format!("read sealed ops review binding: {error}"))?; + let expected_binding = metadata + .get("binding_sha256") + .ok_or("ops review binding has no supervisor seal")?; + let actual_binding = format!("{:x}", Sha256::digest(&binding_bytes)); + if !actual_binding.eq_ignore_ascii_case(expected_binding) { + return Err("ops review binding failed its supervisor seal".into()); + } + let binding: Value = serde_json::from_slice(&binding_bytes) + .map_err(|error| format!("decode sealed ops review binding: {error}"))?; + if !review_binding_matches(&binding, template, runbook_content_sha256)? { + return Err("ops review binding does not match the request, goal, and runbook".into()); + } + digest_json(&json!({ + "reviewerOutputSha256": format!("sha256:{actual_output}"), + "reviewBindingSha256": format!("sha256:{actual_binding}"), + }))? + } else { + // Compatibility for evidence sealed by an already-running older + // session. New reviewers always use the supervisor-sealed artifact. + if !review_evidence_is_bound(&evidence, template, runbook_content_sha256)? { + return Err("ops reviewer evidence is not bound to the request, goal, and runbook".into()); + } + format!("sha256:{actual_output}") + }; let approved_at = metadata .get("completed_at") .cloned() @@ -320,7 +349,7 @@ fn verify_reviewer( Ok(TrustedApproval { subject: reviewer.into(), role: "operations-reviewer", - evidence_sha256: format!("sha256:{actual_output}"), + evidence_sha256, approved_at, }) } @@ -352,11 +381,13 @@ fn reviewer_accepted(evidence: &str) -> bool { accepted } -fn review_binding_marker(template: &Value, runbook_content_sha256: &str) -> Result { +fn review_binding_value(template: &Value, runbook_content_sha256: &str) -> Result { let object = template .as_object() .ok_or("ops request template must be an object")?; - let binding = json!({ + Ok(json!({ + "apiVersion": "multiagent.moveindustries.io/v1", + "kind": "OpsReviewBinding", "requestTemplateSha256": digest_json(template)?, "goalSha256": digest_json( object @@ -369,13 +400,52 @@ fn review_binding_marker(template: &Value, runbook_content_sha256: &str) -> Resu .ok_or("ops request template requires runbook")? )?, "runbookContentSha256": runbook_content_sha256, - }); + })) +} + +fn review_binding_marker(template: &Value, runbook_content_sha256: &str) -> Result { + let binding = review_binding_value(template, runbook_content_sha256)?; Ok(format!( "review-binding-sha256={}", digest_json(&binding)? )) } +fn review_binding_matches( + binding: &Value, + template: &Value, + runbook_content_sha256: &str, +) -> Result { + Ok(binding == &review_binding_value(template, runbook_content_sha256)?) +} + +fn review_binding_artifact_path() -> Result { + let reviewer = required_env("MULTIAGENT_SUBAGENT_NAME")?; + validate_id("reviewer name", &reviewer)?; + let logs = fs::canonicalize(required_env("MULTIAGENT_LOG_DIR")?) + .map_err(|error| format!("resolve multiagent log directory: {error}"))?; + let trace_dir = fs::canonicalize(logs.join("agents").join(&reviewer)) + .map_err(|error| format!("resolve reviewer trace directory: {error}"))?; + if !trace_dir.starts_with(&logs) { + return Err("reviewer trace directory escaped MULTIAGENT_LOG_DIR".into()); + } + Ok(trace_dir.join("review-binding.json")) +} + +fn write_review_binding_artifact(path: &Path, contents: &[u8]) -> Result<(), String> { + let mut options = OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(target_os = "linux")] + options.mode(0o600).custom_flags(libc::O_NOFOLLOW); + let mut file = options + .open(path) + .map_err(|error| format!("create ops review binding: {error}"))?; + file.write_all(contents) + .map_err(|error| format!("write ops review binding: {error}"))?; + file.sync_all() + .map_err(|error| format!("sync ops review binding: {error}")) +} + fn review_evidence_is_bound( evidence: &str, template: &Value, @@ -976,9 +1046,9 @@ fn base64_decode(value: &str) -> Result, String> { mod tests { use super::{ base64_decode, base64url_encode, build_request, canonical, curl_command, ecdsa_der_to_raw, - parse_mcp_body, private_temp_path, review_binding_marker, review_evidence_is_bound, - reviewer_accepted, runbook_content_digest, validate_request_template, write_mcp_headers, - TrustedApproval, + parse_mcp_body, private_temp_path, review_binding_marker, review_binding_matches, + review_binding_value, review_evidence_is_bound, reviewer_accepted, runbook_content_digest, + validate_request_template, write_mcp_headers, TrustedApproval, }; use chrono::{TimeZone, Utc}; use serde_json::json; @@ -1054,6 +1124,21 @@ mod tests { ) .unwrap()); } + + #[test] + fn reviewer_binding_artifact_is_machine_verified_without_model_hash_text() { + let template = json!({ + "goal": "read Slack", + "runbook": {"id":"slack.workspace-access","version":"1.0.0","phase":"read"} + }); + let runbook_digest = format!("sha256:{}", "4".repeat(64)); + let binding = review_binding_value(&template, &runbook_digest).unwrap(); + assert!(review_binding_matches(&binding, &template, &runbook_digest).unwrap()); + + let mut mistyped = binding; + mistyped["runbookContentSha256"] = format!("sha256:{}", "5".repeat(64)).into(); + assert!(!review_binding_matches(&mistyped, &template, &runbook_digest).unwrap()); + } #[test] fn operation_and_target_come_from_runbook_request_data() { let now = Utc.with_ymd_and_hms(2026, 8, 22, 12, 0, 0).unwrap(); diff --git a/src/runtime.rs b/src/runtime.rs index 2b772c2..e2c067a 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -318,6 +318,7 @@ pub fn role_agent_exec(args: &[String]) -> Result { &authorization.workflow_id, &output, &public_output, + &trace_dir, ); supervisor::finish_launch(&cfg.state, name)?; drop(writer_lock); diff --git a/src/supervisor.rs b/src/supervisor.rs index e8ab67b..5c42ed0 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -332,8 +332,9 @@ pub fn seal_role_output( workflow_id: &str, private_output: &Path, public_output: &Path, + trace_dir: &Path, ) -> Result<(), String> { - use std::os::unix::fs::PermissionsExt; + use std::os::unix::fs::{MetadataExt, PermissionsExt}; let bytes = fs::read(private_output).map_err(|error| format!("read private role output: {error}"))?; @@ -355,8 +356,27 @@ pub fn seal_role_output( .map_err(|error| format!("protect reviewer evidence directory: {error}"))?; atomic_write_bytes(&directory.join("last-message.txt"), &bytes)?; let completed_at = Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true); + let mut binding_metadata = String::new(); + if role == "reviewer" { + let binding_path = trace_dir.join("review-binding.json"); + if binding_path.exists() { + let binding_file_metadata = fs::symlink_metadata(&binding_path) + .map_err(|error| format!("inspect ops review binding: {error}"))?; + if !binding_file_metadata.is_file() + || binding_file_metadata.file_type().is_symlink() + || binding_file_metadata.uid() != config::READER_UID + || binding_file_metadata.permissions().mode() & 0o077 != 0 + { + return Err("ops review binding must be a private reviewer-owned regular file".into()); + } + let binding = fs::read(&binding_path) + .map_err(|error| format!("read ops review binding: {error}"))?; + atomic_write_bytes(&directory.join("review-binding.json"), &binding)?; + binding_metadata = format!("binding_sha256={:x}\n", Sha256::digest(&binding)); + } + } let metadata = format!( - "name={name}\nrole={role}\naccess=read-only\nworkflow_id={workflow_id}\nstate=completed\ncompleted_at={completed_at}\noutput_sha256={:x}\n", + "name={name}\nrole={role}\naccess=read-only\nworkflow_id={workflow_id}\nstate=completed\ncompleted_at={completed_at}\noutput_sha256={:x}\n{binding_metadata}", Sha256::digest(&bytes) ); atomic_write_bytes(&directory.join("evidence.env"), metadata.as_bytes())?; @@ -372,6 +392,7 @@ pub fn seal_role_output( _workflow_id: &str, _private_output: &Path, _public_output: &Path, + _trace_dir: &Path, ) -> Result<(), String> { Err("sealed role output requires Linux UID isolation".into()) } From 4372a845ca410bc2287aacad84e60f4a90d0a3b4 Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 07:06:01 -0700 Subject: [PATCH 07/11] Format reviewed ops control plane --- src/agent.rs | 7 ++++++- src/prod_ops.rs | 34 +++++++++++++++------------------- src/runtime.rs | 6 +----- src/supervisor.rs | 4 +++- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/agent.rs b/src/agent.rs index 2285a8e..b2f2925 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -1054,7 +1054,12 @@ fn token_counts(usage: &Value) -> TokenCounts { TokenCounts { input: token_value( usage, - &["input_tokens", "inputTokens", "prompt_tokens", "promptTokens"], + &[ + "input_tokens", + "inputTokens", + "prompt_tokens", + "promptTokens", + ], ), cached_input: token_value(usage, &["cached_input_tokens", "cachedInputTokens"]), output: token_value( diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 4a40d0e..80b3027 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -338,7 +338,9 @@ fn verify_reviewer( // Compatibility for evidence sealed by an already-running older // session. New reviewers always use the supervisor-sealed artifact. if !review_evidence_is_bound(&evidence, template, runbook_content_sha256)? { - return Err("ops reviewer evidence is not bound to the request, goal, and runbook".into()); + return Err( + "ops reviewer evidence is not bound to the request, goal, and runbook".into(), + ); } format!("sha256:{actual_output}") }; @@ -356,7 +358,11 @@ fn verify_reviewer( fn reviewer_accepted(evidence: &str) -> bool { let mut accepted = false; - for line in evidence.lines().map(str::trim).filter(|line| !line.is_empty()) { + for line in evidence + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + { let mut value = line; for wrapper in ["`", "**", "__"] { if value.starts_with(wrapper) @@ -405,10 +411,7 @@ fn review_binding_value(template: &Value, runbook_content_sha256: &str) -> Resul fn review_binding_marker(template: &Value, runbook_content_sha256: &str) -> Result { let binding = review_binding_value(template, runbook_content_sha256)?; - Ok(format!( - "review-binding-sha256={}", - digest_json(&binding)? - )) + Ok(format!("review-binding-sha256={}", digest_json(&binding)?)) } fn review_binding_matches( @@ -513,10 +516,7 @@ fn validate_request_template(template: &Value) -> Result<(), String> { if operation.len() != 2 { return Err("ops request operation must contain only id and version".into()); } - validate_id( - "operation ID", - required_template_string(operation, "id")?, - )?; + validate_id("operation ID", required_template_string(operation, "id")?)?; validate_semver( "operation version", required_template_string(operation, "version")?, @@ -524,7 +524,9 @@ fn validate_request_template(template: &Value) -> Result<(), String> { let target = required_object(object, "target")?; if target.len() != 4 { - return Err("ops request target must contain environment, cluster, namespace, and service".into()); + return Err( + "ops request target must contain environment, cluster, namespace, and service".into(), + ); } let environment = required_template_string(target, "environment")?; if !matches!(environment, "development" | "staging" | "production") { @@ -542,18 +544,12 @@ fn validate_request_template(template: &Value) -> Result<(), String> { if runbook.len() != 3 { return Err("ops request runbook must contain id, version, and phase".into()); } - validate_id( - "runbook ID", - required_template_string(runbook, "id")?, - )?; + validate_id("runbook ID", required_template_string(runbook, "id")?)?; validate_semver( "runbook version", required_template_string(runbook, "version")?, )?; - validate_id( - "runbook phase", - required_template_string(runbook, "phase")?, - )?; + validate_id("runbook phase", required_template_string(runbook, "phase")?)?; required_template_string(object, "runbookDocument")?; let digest = required_template_string(object, "runbookContentSha256")?; let hex = digest diff --git a/src/runtime.rs b/src/runtime.rs index e2c067a..193495e 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -2071,11 +2071,7 @@ fn restore(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { return Err(format!("no captured context to restore: {name}")); } if plan.window == "open" { - tmux_checked(&[ - "kill-window", - "-t", - &format!("{}:{name}", cfg.session), - ])?; + tmux_checked(&["kill-window", "-t", &format!("{}:{name}", cfg.session)])?; } let mut instruction = format!( "You are a restored long-running subagent.\n\nRestoration details:\n- Subagent name: {name}\n- Prior persisted status: {}\n- Persisted state directory: {}\n- This is a fresh tmux window after an orchestrator/session recovery.\n- Do not delete, overwrite, or reset prior memory in the state directory.\n- Read the prior context below, continue only if the assignment is still valid, and report progress/final status in this tmux window.\n- If the prior state shows completion, intentional stop, stale instructions, or a blocker that needs orchestrator/user input, stop and state what you need instead of guessing.\n\nConcise prior context:\n{}\n", diff --git a/src/supervisor.rs b/src/supervisor.rs index 5c42ed0..fecb73d 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -367,7 +367,9 @@ pub fn seal_role_output( || binding_file_metadata.uid() != config::READER_UID || binding_file_metadata.permissions().mode() & 0o077 != 0 { - return Err("ops review binding must be a private reviewer-owned regular file".into()); + return Err( + "ops review binding must be a private reviewer-owned regular file".into(), + ); } let binding = fs::read(&binding_path) .map_err(|error| format!("read ops review binding: {error}"))?; From 94d121daabee2dd9e975a5e3e7d6f2ed54d08787 Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 04:37:34 -0700 Subject: [PATCH 08/11] Extract deterministic reviewed ops lifecycle --- orchestrator_prompt.md | 17 ++- prompts/playbooks/agent-spawning.md | 21 +--- prompts/playbooks/reviewed-ops-cycle.md | 43 +++++++ runbooks/grafana-log-read.md | 11 +- src/prod_ops.rs | 34 +++++- src/runtime.rs | 144 +++++++++++++++++++++++- 6 files changed, 231 insertions(+), 39 deletions(-) create mode 100644 prompts/playbooks/reviewed-ops-cycle.md diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index fe5906f..8856095 100644 --- a/orchestrator_prompt.md +++ b/orchestrator_prompt.md @@ -72,21 +72,20 @@ Load prompts/playbooks/orchestration-routing.md to select a role and prompts/playbooks/agent-spawning.md to spawn or finalize it. Load prompts/playbooks/finding-todo-loop.md only for findings and repair, and prompts/playbooks/validation-scheduling.md only when validation could overlap. +When selecting ops, load prompts/playbooks/reviewed-ops-cycle.md and use its +runtime command instead of constructing review and continuation steps yourself. Keep at most one active agent for the same responsibility. Use bounded waits, inspect durable results, finalize completed agents, and preserve MULTIAGENT_STATE_DIR. Never treat missing provider-native tools or role credentials as proof that a supervisor-mediated capability is unavailable. -Keep one ops identity for one runbook execution. Spawn it once to materialize a -request, obtain independent review, then restore that same completed ops -subagent with `multiagent subagent restore NAME --force --instruction-file PATH` -to execute the unchanged reviewed request and continue the runbook. Do not -spawn a second ops identity for the same runbook, including after a failed -review, rejected binding, execution error, or blocker. Restore the original ops -identity with corrected independently sealed evidence, or stop and report the -blocker; never work around it by copying the request to a new ops identity. -Finalize the ops identity only after the runbook finishes or reaches a blocker. +Keep one ops identity for the entire session. It selects and follows runbooks, +materializes immutable requests, and continues after each reviewed operation. +For every request, invoke `multiagent subagent reviewed-ops-cycle`; do not +manually spawn its reviewer, construct binding evidence, restore the ops agent, +or create a replacement ops identity. Finalize the ops identity only after the +session's operational work finishes or reaches a blocker. MULTIAGENT_VERIFIER_MAX_ITERATIONS is an escalation threshold, never an acceptance condition. diff --git a/prompts/playbooks/agent-spawning.md b/prompts/playbooks/agent-spawning.md index 3ba8a64..1c851b4 100644 --- a/prompts/playbooks/agent-spawning.md +++ b/prompts/playbooks/agent-spawning.md @@ -187,23 +187,10 @@ blocked status with a source-visible reason. ## Ops Review Continuation -An ops agent exits after materializing and binding each request so an -independent reviewer can inspect immutable bytes. Keep that ops subagent -persisted. After the reviewer is finalized with an accepted verdict, resume the -same ops identity and provider context: - -```bash -multiagent subagent restore OPS_NAME --force \ - --instruction-file EXECUTE_REVIEWED_REQUEST_INSTRUCTION -multiagent subagent wait OPS_NAME --timeout 900 -``` - -The follow-up must name the unchanged request path and finalized reviewer. -`--force` records the orchestrator's explicit continuation decision; it does -not waive review. Do not spawn a separate executor, copy the request into a new -role directory, or finalize the ops identity until the runbook finishes or -blocks. A replacement ops identity is allowed only when restoration fails, and -the replacement must materialize a new request for a new independent review. +Use one ops identity per session. Load +`prompts/playbooks/reviewed-ops-cycle.md` and invoke its supervisor-owned command +for each immutable request. Do not manually reproduce the reviewer, binding, +restore, or wait sequence, and never create a replacement ops identity. After `multiagent subagent kill NAME` or `multiagent subagent finalize NAME`, ensure the assignment no longer owns paths before reusing them. If needed, run diff --git a/prompts/playbooks/reviewed-ops-cycle.md b/prompts/playbooks/reviewed-ops-cycle.md new file mode 100644 index 0000000..7f97611 --- /dev/null +++ b/prompts/playbooks/reviewed-ops-cycle.md @@ -0,0 +1,43 @@ +# Reviewed Ops Cycle + +Use this playbook whenever an ops agent has materialized an immutable prod-mcp +request. It centralizes the review and continuation mechanics; provider runbooks +define what operations mean, not how agents are spawned. + +## Session invariant + +Use one persistent `ops` identity for the entire session. That agent may follow +multiple runbooks and materialize multiple requests, but no second ops identity +may be created. Agent judgment selects the runbook, operation, target, and +parameters. This playbook only makes the authorization lifecycle deterministic. + +## Reviewed request + +After the ops agent writes and binds its request, run: + +```bash +multiagent subagent reviewed-ops-cycle OPS_NAME \ + --request-file "$MULTIAGENT_LOG_DIR/agents/OPS_NAME/request.json" \ + --reviewer ops-reviewer-NN \ + --timeout 900 +``` + +Use a fresh `ops-reviewer-NN` identity for every immutable request. The command: + +1. verifies that the request belongs to the session's ops identity; +2. computes the exact review binding; +3. spawns an independent ops reviewer with the literal request and binding; +4. waits for and finalizes the reviewer; +5. rejects missing, negative, or incorrectly bound evidence before execution; +6. restores the same ops identity with the exact execute command; and +7. waits for that ops identity to inspect the receipt and continue its runbook. + +Do not manually reconstruct these steps in prompts or shell commands. + +## Failure behavior + +If review is rejected, binding preflight fails, restoration fails, or prod-mcp +rejects execution, stop and report the exact blocker. If a corrected independent +review is appropriate, use a fresh reviewer on the same immutable request. If +the request bytes must change, the same ops identity materializes the new bytes +and starts a new reviewed cycle. Never spawn a replacement ops identity. diff --git a/runbooks/grafana-log-read.md b/runbooks/grafana-log-read.md index 76d009e..2b0ac23 100644 --- a/runbooks/grafana-log-read.md +++ b/runbooks/grafana-log-read.md @@ -43,10 +43,7 @@ Label discovery additionally requires `action`, `datasourceUid`, `lookbackMinute - The reviewer or prod-mcp rejects the request. ## Reviewed role continuation -Run each discovery or query operation with one persistent ops identity and one independent reviewer: - -1. An `ops` materializer writes the exact request to `$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json`, returns the literal JSON and path, and exits without execution. -2. An independent `reviewer` reviews that literal JSON and exits with accepted evidence. -3. Restore the same `ops` identity with the unchanged request path and finalized reviewer name. It invokes `multiagent ops execute --request-file "$MULTIAGENT_LOG_DIR/agents/$MULTIAGENT_SUBAGENT_NAME/request.json" --reviewer REVIEWER_NAME` and continues the runbook from the persisted receipt. - -The ops role is intentionally unable to write into the repository. `$HOME` is its deployment-provided private scratch directory. Reviewers must receive the literal request in their instruction because role homes are isolated from one another. +Use the provider-neutral lifecycle in +`prompts/playbooks/reviewed-ops-cycle.md` for every immutable request. This +runbook defines Grafana operations and limits; it does not redefine agent +spawning, independent review, binding, restoration, or receipt handling. diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 80b3027..4a17f4f 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -98,14 +98,23 @@ fn review_bind(args: &[String]) -> Result { Ok(ExitCode::SUCCESS) } -fn execute(args: &[String]) -> Result { - let options = options(args)?; - let reviewer = required(&options, "--reviewer")?; +pub(crate) fn review_binding_for_request(request_file: &Path) -> Result { + let bytes = fs::read(request_file).map_err(|error| format!("read ops request: {error}"))?; + let template: Value = serde_json::from_slice(&bytes) + .map_err(|error| format!("decode ops request template: {error}"))?; + validate_request_template(&template)?; + let runbook_content_sha256 = verified_runbook_content(&template)?; + review_binding_marker(&template, &runbook_content_sha256) +} + +fn load_reviewed_request( + request_file: &Path, + reviewer: &str, +) -> Result<(PathBuf, Value, TrustedApproval), String> { validate_id("reviewer name", reviewer)?; - let request_file = PathBuf::from(required(&options, "--request-file")?); let state = fs::canonicalize(required_env("MULTIAGENT_STATE_DIR")?) .map_err(|error| format!("resolve multiagent state: {error}"))?; - let request_file = fs::canonicalize(&request_file) + let request_file = fs::canonicalize(request_file) .map_err(|error| format!("resolve ops request file: {error}"))?; if !request_file.starts_with(&state) { return Err("ops request file must be inside MULTIAGENT_STATE_DIR".into()); @@ -127,6 +136,21 @@ fn execute(args: &[String]) -> Result { .map_err(|error| format!("decode ops request template: {error}"))?; let runbook_content_sha256 = verified_runbook_content(&template)?; let reviewer_approval = verify_reviewer(&state, reviewer, &template, &runbook_content_sha256)?; + Ok((state, template, reviewer_approval)) +} + +pub(crate) fn preflight_reviewed_request( + request_file: &Path, + reviewer: &str, +) -> Result<(), String> { + load_reviewed_request(request_file, reviewer).map(|_| ()) +} + +fn execute(args: &[String]) -> Result { + let options = options(args)?; + let reviewer = required(&options, "--reviewer")?; + let request_file = PathBuf::from(required(&options, "--request-file")?); + let (state, template, reviewer_approval) = load_reviewed_request(&request_file, reviewer)?; let now = Utc::now(); let caller_subject = env::var("MULTIAGENT_CALLER_SUBJECT") .ok() diff --git a/src/runtime.rs b/src/runtime.rs index 193495e..f17def5 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1375,6 +1375,7 @@ pub fn subagent(args: &[String]) -> Result { "recover-plan" => recover_plan(&cfg, &args[1..])?, "restore" => restore(&cfg, &args[1..])?, "restore-all" => restore_all(&cfg, &args[1..])?, + "reviewed-ops-cycle" => reviewed_ops_cycle(&cfg, &args[1..])?, "finalize" => finalize(&cfg, &args[1..])?, "kill" => kill(&cfg, &args[1..])?, command => return Err(format!("unknown command: {command}")), @@ -1384,7 +1385,7 @@ pub fn subagent(args: &[String]) -> Result { fn print_subagent_usage() { println!( - "Usage:\n multiagent subagent spawn NAME [--own PATH[,PATH...] ...] [--assignment-id ID] [--workflow-id ID --decision-id ID --plan-id ID] [--branch BRANCH] [--start-commit COMMIT] [--role ROLE] [--instruction TEXT | --instruction-file PATH | -- TEXT]\n multiagent subagent restore NAME [--force] [--instruction TEXT | --instruction-file PATH]\n multiagent subagent list|recover-plan|restore-all|gate-check\n multiagent subagent poll|inspect|finalize|kill NAME [OPTIONS]\n multiagent subagent wait NAME [--timeout SECONDS] [--poll-interval SECONDS]\n\nAll durable state and tmux subprocess orchestration are implemented by the Rust CLI." + "Usage:\n multiagent subagent spawn NAME [--own PATH[,PATH...] ...] [--assignment-id ID] [--workflow-id ID --decision-id ID --plan-id ID] [--branch BRANCH] [--start-commit COMMIT] [--role ROLE] [--instruction TEXT | --instruction-file PATH | -- TEXT]\n multiagent subagent restore NAME [--force] [--instruction TEXT | --instruction-file PATH]\n multiagent subagent reviewed-ops-cycle OPS_NAME --request-file PATH --reviewer NAME [--timeout SECONDS]\n multiagent subagent list|recover-plan|restore-all|gate-check\n multiagent subagent poll|inspect|finalize|kill NAME [OPTIONS]\n multiagent subagent wait NAME [--timeout SECONDS] [--poll-interval SECONDS]\n\nAll durable state and tmux subprocess orchestration are implemented by the Rust CLI." ); } @@ -1746,6 +1747,128 @@ fn reject_additional_ops_identity(state: &Path, name: &str, role: &str) -> Resul Ok(()) } +fn reviewed_ops_reviewer_instruction( + request_file: &Path, + request: &str, + binding: &str, +) -> String { + format!( + "Independently review the immutable ops request below against its stated goal, operation, target, parameters, and certified runbook. Do not modify or execute it. If and only if it is acceptable, end with an accepted verdict and reproduce the binding marker exactly. Otherwise reject it with concrete findings.\n\nrequest-path: {}\n{}\n\nimmutable-request:\n{}", + request_file.display(), + binding, + request + ) +} + +fn reviewed_ops_execute_instruction(request_file: &Path, reviewer: &str) -> String { + format!( + "Continue the same runbook with the independently reviewed immutable request. Execute exactly:\n\nmultiagent ops execute --request-file {} --reviewer {}\n\nInspect the persisted receipt, continue the runbook if more operations are needed, and report the result or exact blocker. Do not create a replacement ops identity.", + shell_escape(&request_file.display().to_string()), + shell_escape(reviewer) + ) +} + +fn reviewed_ops_cycle(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { + let ops_name = args + .first() + .filter(|value| !value.is_empty()) + .ok_or_else(|| "reviewed-ops-cycle requires OPS_NAME".to_string())?; + validate_name(ops_name)?; + let mut request_file = None::; + let mut reviewer = None::; + let mut timeout = "900".to_string(); + let mut index = 1; + while index < args.len() { + match args[index].as_str() { + "--request-file" => { + request_file = Some(PathBuf::from(required_value( + args, + index, + "reviewed-ops-cycle --request-file", + )?)); + index += 2; + } + "--reviewer" => { + reviewer = Some( + required_value(args, index, "reviewed-ops-cycle --reviewer")?.to_string(), + ); + index += 2; + } + "--timeout" => { + timeout = required_value(args, index, "reviewed-ops-cycle --timeout")?.to_string(); + timeout + .parse::() + .map_err(|_| "reviewed-ops-cycle --timeout must be a non-negative number".to_string())?; + index += 2; + } + other => return Err(format!("unknown reviewed-ops-cycle argument: {other}")), + } + } + let request_file = request_file.ok_or("reviewed-ops-cycle requires --request-file PATH")?; + let reviewer = reviewer.ok_or("reviewed-ops-cycle requires --reviewer NAME")?; + validate_name(&reviewer)?; + if !reviewer.starts_with("ops-reviewer-") { + return Err("reviewed-ops-cycle reviewer name must start with ops-reviewer-".into()); + } + + let ops_dir = cfg.state.join("subagents").join(ops_name); + let metadata = read_env(&ops_dir.join("meta.env"))?; + if metadata.get("role").map(String::as_str) != Some("ops") { + return Err(format!("reviewed-ops-cycle requires an existing ops identity: {ops_name}")); + } + let request_file = fs::canonicalize(&request_file) + .map_err(io_error("resolve reviewed ops request"))?; + let ops_logs = fs::canonicalize(cfg.logs.join("agents").join(ops_name)) + .map_err(io_error("resolve ops agent log directory"))?; + if !request_file.starts_with(&ops_logs) { + return Err(format!( + "reviewed ops request must belong to ops identity {ops_name}" + )); + } + let request = fs::read_to_string(&request_file).map_err(io_error("read reviewed ops request"))?; + let binding = crate::prod_ops::review_binding_for_request(&request_file)?; + let reviewer_instruction = + reviewed_ops_reviewer_instruction(&request_file, &request, &binding); + spawn( + cfg, + &[ + reviewer.clone(), + "--role".into(), + "ops-reviewer".into(), + "--instruction".into(), + reviewer_instruction, + ], + )?; + wait(cfg, &[reviewer.clone(), "--timeout".into(), timeout.clone()])?; + let reviewer_status = read_trimmed( + &cfg.state.join("subagents").join(&reviewer).join("status"), + ) + .unwrap_or_else(|| "unknown".into()); + if !matches!(reviewer_status.as_str(), "done" | "exited") { + return Err(format!( + "ops reviewer {reviewer} did not complete successfully: {reviewer_status}" + )); + } + finalize(cfg, std::slice::from_ref(&reviewer))?; + crate::prod_ops::preflight_reviewed_request(&request_file, &reviewer)?; + + let execute_instruction = reviewed_ops_execute_instruction(&request_file, &reviewer); + restore( + cfg, + &[ + ops_name.to_string(), + "--force".into(), + "--instruction".into(), + execute_instruction, + ], + )?; + wait( + cfg, + &[ops_name.to_string(), "--timeout".into(), timeout], + )?; + Ok(()) +} + fn list_subagents(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { if !args.is_empty() { return Err("list takes no arguments".into()); @@ -4005,6 +4128,25 @@ mod tests { fs::remove_dir_all(state).unwrap(); } + #[test] + fn reviewed_ops_cycle_instructions_are_provider_neutral_and_exact() { + let request = Path::new("/state/logs/agents/ops-primary/request.json"); + let review = reviewed_ops_reviewer_instruction( + request, + "{\"operation\":{\"id\":\"provider.read\"}}", + "review-binding-sha256=abc", + ); + assert!(review.contains("review-binding-sha256=abc")); + assert!(review.contains("provider.read")); + assert!(!review.contains("Slack")); + assert!(!review.contains("Grafana")); + + let execute = reviewed_ops_execute_instruction(request, "ops-reviewer-01"); + assert!(execute.contains( + "multiagent ops execute --request-file /state/logs/agents/ops-primary/request.json --reviewer ops-reviewer-01" + )); + } + #[cfg(unix)] #[test] fn privileged_agent_parent_accepts_only_root_owned_safe_modes() { From 94a704eaa5f6a48256dd5d4cd48c79b4ab4d98c0 Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 04:54:42 -0700 Subject: [PATCH 09/11] Route external-only tasks directly to ops --- orchestrator_prompt.md | 7 +++++++ prompts/playbooks/implementation-lifecycle.md | 6 ++++++ prompts/playbooks/orchestration-routing.md | 17 +++++++++++++---- tests/run.sh | 3 +++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index 8856095..dd12414 100644 --- a/orchestrator_prompt.md +++ b/orchestrator_prompt.md @@ -59,6 +59,10 @@ completion gates. prompts/playbooks/implementation-lifecycle.md gate. - An implementation without a contract scout still requires an independently reviewed, supervisor-approved implementation context before a worker starts. +- An ops-only task that does not change repository source does not enter the + source implementation lifecycle. Do not create an implementation context, + decision-authority reviewer, or implementation-phase transition for it; use + its Markdown runbook and independently reviewed ops requests directly. - multiagent ops execute requires the finalized independent reviewer bound to the exact request and runbook. - A completion request succeeds only after supervisor obligations and TODOs are @@ -74,6 +78,9 @@ prompts/playbooks/finding-todo-loop.md only for findings and repair, and prompts/playbooks/validation-scheduling.md only when validation could overlap. When selecting ops, load prompts/playbooks/reviewed-ops-cycle.md and use its runtime command instead of constructing review and continuation steps yourself. +`multiagent subagent spawn` composes the canonical role module automatically. +Do not search for, enumerate, or read role prompt files to discover how to +spawn a known role. Keep at most one active agent for the same responsibility. Use bounded waits, inspect durable results, finalize completed agents, and preserve diff --git a/prompts/playbooks/implementation-lifecycle.md b/prompts/playbooks/implementation-lifecycle.md index d474dd3..698ae72 100644 --- a/prompts/playbooks/implementation-lifecycle.md +++ b/prompts/playbooks/implementation-lifecycle.md @@ -3,6 +3,12 @@ This mandatory lifecycle applies to source implementation. Production actions use the selected Markdown runbook and ops review path instead. +Do not use this lifecycle for an external-only task that does not modify +repository source. In particular, do not create an implementation context, +spawn a decision-authority reviewer, or transition to `implementation` before +starting ops. Spawn the session's persistent ops identity directly and use +`prompts/playbooks/reviewed-ops-cycle.md` for its immutable requests. + ## State Machine Read persisted state with: diff --git a/prompts/playbooks/orchestration-routing.md b/prompts/playbooks/orchestration-routing.md index 21e1b05..d5b0f14 100644 --- a/prompts/playbooks/orchestration-routing.md +++ b/prompts/playbooks/orchestration-routing.md @@ -51,15 +51,24 @@ validation-scheduling.md and hold one validation lease per package. Give technic - Findings become TODOs and return through the lifecycle before repair. - Completion is a supervisor request, not an orchestrator assertion. +An external-only task with no repository mutation bypasses the source +implementation lifecycle. Its minimum DAG starts with the persistent ops +identity and uses the reviewed-ops cycle for each immutable request. Do not +manufacture a decision, approved implementation context, decision-authority +review, or source phase transition merely to authorize ops; the runbook, +request binding, independent ops reviewer, caller approval, and prod-mcp permit +are that path's authority chain. + If a gate rejects, use its concrete reason as the next dependency. Never create or edit supervisor-owned evidence. ## Agent Contract -Before spawning, load the selected role module and -prompts/playbooks/agent-spawning.md. Spawn only through multiagent subagent -spawn, wait for durable output, finalize completed read-only reviewers, and run -assignment checks for workers. The role module owns request shape, output +Load prompts/playbooks/agent-spawning.md and spawn only through `multiagent +subagent spawn`. The runtime selects and composes the canonical role module from +the role and identity name; do not find, list, or read role prompt files at +runtime. Wait for durable output, finalize completed read-only reviewers, and +run assignment checks for workers. The role module owns request shape, output markers, and provider procedure. ## Repair And Safety diff --git a/tests/run.sh b/tests/run.sh index 60adb22..57b14b9 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -995,6 +995,9 @@ fi assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "todo-create" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "todo-close" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "gate-check" +assert_file_contains "$ROOT/prompts/playbooks/orchestration-routing.md" "An external-only task with no repository mutation bypasses the source" +assert_file_contains "$ROOT/prompts/playbooks/orchestration-routing.md" "do not find, list, or read role prompt files" +assert_file_contains "$ROOT/prompts/playbooks/implementation-lifecycle.md" "Do not use this lifecycle for an external-only task" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "required-path-outside-owned:" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "ownership blocker" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" 'SUBAGENT_CLI="$WORKER_CLI" multiagent subagent spawn' From 00b4f96e4eab46861b3127af16ccee406cb40be9 Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 05:09:49 -0700 Subject: [PATCH 10/11] Extract canonical ops runbook mechanics --- orchestrator_prompt.md | 8 +++-- prompts/playbooks/orchestration-routing.md | 17 +++++---- prompts/playbooks/reviewed-ops-cycle.md | 20 +++++++++-- src/prod_ops.rs | 42 ++++++++++++++++++++-- tests/run.sh | 3 ++ 5 files changed, 77 insertions(+), 13 deletions(-) diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index dd12414..190d107 100644 --- a/orchestrator_prompt.md +++ b/orchestrator_prompt.md @@ -73,11 +73,13 @@ Prompt text cannot grant authority or waive a supervisor rejection. ## Coordination Load prompts/playbooks/orchestration-routing.md to select a role and -prompts/playbooks/agent-spawning.md to spawn or finalize it. Load +prompts/playbooks/agent-spawning.md only for worker, scout, verifier, or other +non-ops role lifecycles. Load prompts/playbooks/finding-todo-loop.md only for findings and repair, and prompts/playbooks/validation-scheduling.md only when validation could overlap. -When selecting ops, load prompts/playbooks/reviewed-ops-cycle.md and use its -runtime command instead of constructing review and continuation steps yourself. +When selecting ops, load only prompts/playbooks/reviewed-ops-cycle.md and use +its initial spawn and runtime command instead of loading agent-spawning.md or +constructing review and continuation steps yourself. `multiagent subagent spawn` composes the canonical role module automatically. Do not search for, enumerate, or read role prompt files to discover how to spawn a known role. diff --git a/prompts/playbooks/orchestration-routing.md b/prompts/playbooks/orchestration-routing.md index d5b0f14..5719432 100644 --- a/prompts/playbooks/orchestration-routing.md +++ b/prompts/playbooks/orchestration-routing.md @@ -59,17 +59,22 @@ review, or source phase transition merely to authorize ops; the runbook, request binding, independent ops reviewer, caller approval, and prod-mcp permit are that path's authority chain. +For this ops-only route, load reviewed-ops-cycle.md instead of +agent-spawning.md. The reviewed ops playbook owns the initial ops spawn and the +complete reviewed execution lifecycle. + If a gate rejects, use its concrete reason as the next dependency. Never create or edit supervisor-owned evidence. ## Agent Contract -Load prompts/playbooks/agent-spawning.md and spawn only through `multiagent -subagent spawn`. The runtime selects and composes the canonical role module from -the role and identity name; do not find, list, or read role prompt files at -runtime. Wait for durable output, finalize completed read-only reviewers, and -run assignment checks for workers. The role module owns request shape, output -markers, and provider procedure. +For non-ops roles, load prompts/playbooks/agent-spawning.md and spawn only +through `multiagent subagent spawn`. For ops, load reviewed-ops-cycle.md instead. +The runtime selects and composes the canonical role module from the role and +identity name; do not find, list, or read role prompt files at runtime. Wait for +durable output, finalize completed read-only reviewers, and run assignment +checks for workers. The role module owns request shape, output markers, and +provider procedure. ## Repair And Safety diff --git a/prompts/playbooks/reviewed-ops-cycle.md b/prompts/playbooks/reviewed-ops-cycle.md index 7f97611..bcdbacb 100644 --- a/prompts/playbooks/reviewed-ops-cycle.md +++ b/prompts/playbooks/reviewed-ops-cycle.md @@ -8,8 +8,24 @@ define what operations mean, not how agents are spawned. Use one persistent `ops` identity for the entire session. That agent may follow multiple runbooks and materialize multiple requests, but no second ops identity -may be created. Agent judgment selects the runbook, operation, target, and -parameters. This playbook only makes the authorization lifecycle deterministic. +may be created. Agent judgment selects the runbook, operation, and parameters. +`multiagent ops bind-runbook` copies canonical target metadata from the exact +Markdown runbook when it is declared there. This playbook only makes the +authorization lifecycle deterministic. + +## Start the ops identity + +Spawn exactly one ops identity for the session. The runtime composes the ops +role module; do not load the general agent-spawning playbook or role prompt +files to reconstruct it. + +```bash +multiagent subagent spawn OPS_NAME --role ops --instruction "Inspect the request, follow the applicable runbook, and prepare the reviewed operation." +multiagent subagent wait OPS_NAME --timeout 900 +``` + +Keep this identity for every reviewed operation in the session. Do not spawn a +replacement ops identity after review. ## Reviewed request diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 4a17f4f..983827c 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -43,9 +43,13 @@ fn bind_runbook(args: &[String]) -> Result { .map_err(|error| format!("decode ops request template: {error}"))?; let relative = required(&options, "--runbook-document")?; let digest = exact_runbook_content_sha256(relative)?; + let canonical_target = exact_runbook_target(relative)?; let object = template .as_object_mut() .ok_or("ops request template must be an object")?; + if let Some(target) = canonical_target { + object.insert("target".into(), target); + } object.insert("runbookDocument".into(), Value::String(relative.into())); object.insert("runbookContentSha256".into(), Value::String(digest.clone())); validate_request_template(&template)?; @@ -632,6 +636,41 @@ fn verified_runbook_content(template: &Value) -> Result { } fn exact_runbook_content_sha256(relative: &str) -> Result { + let bytes = exact_runbook_bytes(relative)?; + Ok(runbook_content_digest(&bytes)) +} + +fn exact_runbook_target(relative: &str) -> Result, String> { + canonical_runbook_target(&exact_runbook_bytes(relative)?) +} + +fn canonical_runbook_target(bytes: &[u8]) -> Result, String> { + const PREFIX: &str = "- Set `target` to `"; + const SUFFIX: &str = "`."; + let markdown = std::str::from_utf8(bytes) + .map_err(|error| format!("decode runbook document as UTF-8: {error}"))?; + let declarations = markdown + .lines() + .filter_map(|line| line.strip_prefix(PREFIX)) + .collect::>(); + if declarations.len() > 1 { + return Err("runbook document must contain at most one canonical target declaration".into()); + } + let Some(declaration) = declarations.first() else { + return Ok(None); + }; + let encoded = declaration + .strip_suffix(SUFFIX) + .ok_or("canonical runbook target declaration is malformed")?; + let target: Value = serde_json::from_str(encoded) + .map_err(|error| format!("decode canonical runbook target: {error}"))?; + if !target.is_object() { + return Err("canonical runbook target must be a JSON object".into()); + } + Ok(Some(target)) +} + +fn exact_runbook_bytes(relative: &str) -> Result, String> { let relative_path = Path::new(relative); if relative_path.is_absolute() || relative_path @@ -655,8 +694,7 @@ fn exact_runbook_content_sha256(relative: &str) -> Result { return Err("runbook document must be a regular file between 1 byte and 1 MiB".into()); } let bytes = fs::read(&document).map_err(|error| format!("read runbook document: {error}"))?; - let actual = runbook_content_digest(&bytes); - Ok(actual) + Ok(bytes) } fn runbook_content_digest(bytes: &[u8]) -> String { diff --git a/tests/run.sh b/tests/run.sh index 57b14b9..491a841 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -996,7 +996,10 @@ assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "todo-create" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "todo-close" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "gate-check" assert_file_contains "$ROOT/prompts/playbooks/orchestration-routing.md" "An external-only task with no repository mutation bypasses the source" +assert_file_contains "$ROOT/prompts/playbooks/orchestration-routing.md" "For this ops-only route, load reviewed-ops-cycle.md instead of" assert_file_contains "$ROOT/prompts/playbooks/orchestration-routing.md" "do not find, list, or read role prompt files" +assert_file_contains "$ROOT/orchestrator_prompt.md" "When selecting ops, load only prompts/playbooks/reviewed-ops-cycle.md" +assert_file_contains "$ROOT/prompts/playbooks/reviewed-ops-cycle.md" "multiagent subagent spawn OPS_NAME --role ops" assert_file_contains "$ROOT/prompts/playbooks/implementation-lifecycle.md" "Do not use this lifecycle for an external-only task" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "required-path-outside-owned:" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" "ownership blocker" From b523f7ec770d532e5dfb07e1d8a6e71b1d7b356e Mon Sep 17 00:00:00 2001 From: Bo Wu Date: Mon, 24 Aug 2026 07:08:27 -0700 Subject: [PATCH 11/11] Format integrated reviewed ops changes --- src/prod_ops.rs | 4 +++- src/runtime.rs | 47 +++++++++++++++++++++-------------------------- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/prod_ops.rs b/src/prod_ops.rs index 983827c..988f468 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -654,7 +654,9 @@ fn canonical_runbook_target(bytes: &[u8]) -> Result, String> { .filter_map(|line| line.strip_prefix(PREFIX)) .collect::>(); if declarations.len() > 1 { - return Err("runbook document must contain at most one canonical target declaration".into()); + return Err( + "runbook document must contain at most one canonical target declaration".into(), + ); } let Some(declaration) = declarations.first() else { return Ok(None); diff --git a/src/runtime.rs b/src/runtime.rs index f17def5..b5c114b 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -1747,11 +1747,7 @@ fn reject_additional_ops_identity(state: &Path, name: &str, role: &str) -> Resul Ok(()) } -fn reviewed_ops_reviewer_instruction( - request_file: &Path, - request: &str, - binding: &str, -) -> String { +fn reviewed_ops_reviewer_instruction(request_file: &Path, request: &str, binding: &str) -> String { format!( "Independently review the immutable ops request below against its stated goal, operation, target, parameters, and certified runbook. Do not modify or execute it. If and only if it is acceptable, end with an accepted verdict and reproduce the binding marker exactly. Otherwise reject it with concrete findings.\n\nrequest-path: {}\n{}\n\nimmutable-request:\n{}", request_file.display(), @@ -1789,16 +1785,15 @@ fn reviewed_ops_cycle(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String index += 2; } "--reviewer" => { - reviewer = Some( - required_value(args, index, "reviewed-ops-cycle --reviewer")?.to_string(), - ); + reviewer = + Some(required_value(args, index, "reviewed-ops-cycle --reviewer")?.to_string()); index += 2; } "--timeout" => { timeout = required_value(args, index, "reviewed-ops-cycle --timeout")?.to_string(); - timeout - .parse::() - .map_err(|_| "reviewed-ops-cycle --timeout must be a non-negative number".to_string())?; + timeout.parse::().map_err(|_| { + "reviewed-ops-cycle --timeout must be a non-negative number".to_string() + })?; index += 2; } other => return Err(format!("unknown reviewed-ops-cycle argument: {other}")), @@ -1814,10 +1809,12 @@ fn reviewed_ops_cycle(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String let ops_dir = cfg.state.join("subagents").join(ops_name); let metadata = read_env(&ops_dir.join("meta.env"))?; if metadata.get("role").map(String::as_str) != Some("ops") { - return Err(format!("reviewed-ops-cycle requires an existing ops identity: {ops_name}")); + return Err(format!( + "reviewed-ops-cycle requires an existing ops identity: {ops_name}" + )); } - let request_file = fs::canonicalize(&request_file) - .map_err(io_error("resolve reviewed ops request"))?; + let request_file = + fs::canonicalize(&request_file).map_err(io_error("resolve reviewed ops request"))?; let ops_logs = fs::canonicalize(cfg.logs.join("agents").join(ops_name)) .map_err(io_error("resolve ops agent log directory"))?; if !request_file.starts_with(&ops_logs) { @@ -1825,10 +1822,10 @@ fn reviewed_ops_cycle(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String "reviewed ops request must belong to ops identity {ops_name}" )); } - let request = fs::read_to_string(&request_file).map_err(io_error("read reviewed ops request"))?; + let request = + fs::read_to_string(&request_file).map_err(io_error("read reviewed ops request"))?; let binding = crate::prod_ops::review_binding_for_request(&request_file)?; - let reviewer_instruction = - reviewed_ops_reviewer_instruction(&request_file, &request, &binding); + let reviewer_instruction = reviewed_ops_reviewer_instruction(&request_file, &request, &binding); spawn( cfg, &[ @@ -1839,11 +1836,12 @@ fn reviewed_ops_cycle(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String reviewer_instruction, ], )?; - wait(cfg, &[reviewer.clone(), "--timeout".into(), timeout.clone()])?; - let reviewer_status = read_trimmed( - &cfg.state.join("subagents").join(&reviewer).join("status"), - ) - .unwrap_or_else(|| "unknown".into()); + wait( + cfg, + &[reviewer.clone(), "--timeout".into(), timeout.clone()], + )?; + let reviewer_status = read_trimmed(&cfg.state.join("subagents").join(&reviewer).join("status")) + .unwrap_or_else(|| "unknown".into()); if !matches!(reviewer_status.as_str(), "done" | "exited") { return Err(format!( "ops reviewer {reviewer} did not complete successfully: {reviewer_status}" @@ -1862,10 +1860,7 @@ fn reviewed_ops_cycle(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String execute_instruction, ], )?; - wait( - cfg, - &[ops_name.to_string(), "--timeout".into(), timeout], - )?; + wait(cfg, &[ops_name.to_string(), "--timeout".into(), timeout])?; Ok(()) }