diff --git a/orchestrator_prompt.md b/orchestrator_prompt.md index 6931e68..190d107 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. @@ -54,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 @@ -64,14 +73,28 @@ 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 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. 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 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 76ae64d..1c851b4 100644 --- a/prompts/playbooks/agent-spawning.md +++ b/prompts/playbooks/agent-spawning.md @@ -185,6 +185,13 @@ 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 + +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 `multiagent subagent assignment-status NAME failed` for killed workers or 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 4803f36..5719432 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. @@ -43,16 +51,30 @@ 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. + +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 -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 -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 new file mode 100644 index 0000000..bcdbacb --- /dev/null +++ b/prompts/playbooks/reviewed-ops-cycle.md @@ -0,0 +1,59 @@ +# 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, 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 + +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/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..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` 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. -- If and only if the request matches the goal and runbook, make the first non-empty line exactly `Verdict: ACCEPTED`. +- 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`, 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/runbooks/grafana-log-read.md b/runbooks/grafana-log-read.md index 5c79990..2b0ac23 100644 --- a/runbooks/grafana-log-read.md +++ b/runbooks/grafana-log-read.md @@ -41,12 +41,9 @@ 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: - -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`. - -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/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/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 9ebe7ef..988f468 100644 --- a/src/prod_ops.rs +++ b/src/prod_ops.rs @@ -43,11 +43,16 @@ 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)?; let encoded = serde_json::to_vec_pretty(&template) .map_err(|error| format!("encode bound ops request: {error}"))?; fs::write(&request_file, encoded) @@ -63,10 +68,18 @@ 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")?; 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={}", @@ -85,17 +98,27 @@ fn review_bind(args: &[String]) -> Result { )? ); println!("runbook-content-sha256={runbook_content_sha256}"); + println!("review-binding-artifact={}", binding_path.display()); 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()); @@ -117,6 +140,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() @@ -175,6 +213,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")?; @@ -303,10 +342,157 @@ fn verify_reviewer( if !accepted { return Err("ops reviewer did not accept the operation".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() + .ok_or("ops reviewer evidence has no completion timestamp")?; + Ok(TrustedApproval { + subject: reviewer.into(), + role: "operations-reviewer", + evidence_sha256, + approved_at, + }) +} + +fn reviewer_accepted(evidence: &str) -> bool { + 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()]; + } + } + 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 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 markers = [ + Ok(json!({ + "apiVersion": "multiagent.moveindustries.io/v1", + "kind": "OpsReviewBinding", + "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, + })) +} + +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, + 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={}", @@ -326,38 +512,9 @@ fn verify_reviewer( ), format!("runbook-content-sha256={runbook_content_sha256}"), ]; - if markers.iter().any(|marker| !evidence.contains(marker)) { - return Err("ops reviewer evidence is not bound to the request, goal, and runbook".into()); - } - let approved_at = metadata - .get("completed_at") - .cloned() - .ok_or("ops reviewer evidence has no completion timestamp")?; - Ok(TrustedApproval { - subject: reviewer.into(), - role: "operations-reviewer", - evidence_sha256: format!("sha256:{actual_output}"), - approved_at, - }) -} - -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()]; - } - } - value.eq_ignore_ascii_case("verdict: accepted") - }) + Ok(legacy_markers + .iter() + .all(|marker| evidence.contains(marker))) } fn required_object<'a>( @@ -370,6 +527,95 @@ 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() @@ -390,6 +636,43 @@ 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 @@ -413,8 +696,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 { @@ -824,8 +1106,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, - 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; @@ -869,9 +1152,52 @@ 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 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 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() { @@ -907,6 +1233,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 = diff --git a/src/runtime.rs b/src/runtime.rs index 7141356..b5c114b 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); @@ -1374,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}")), @@ -1383,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." ); } @@ -1503,6 +1505,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 +1712,158 @@ 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 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()); @@ -2027,12 +2182,15 @@ 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, @@ -3943,6 +4101,47 @@ 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(); + } + + #[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() { diff --git a/src/supervisor.rs b/src/supervisor.rs index e8ab67b..fecb73d 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,29 @@ 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 +394,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()) } 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 diff --git a/tests/run.sh b/tests/run.sh index b2a1821..491a841 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -995,6 +995,12 @@ 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" "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" assert_file_contains "$ROOT/prompts/playbooks/agent-spawning.md" 'SUBAGENT_CLI="$WORKER_CLI" multiagent subagent spawn' @@ -1941,6 +1947,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 +1986,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"