diff --git a/docs/production-operations.md b/docs/production-operations.md index f5a1ed0..2bf5bd7 100644 --- a/docs/production-operations.md +++ b/docs/production-operations.md @@ -19,6 +19,8 @@ For the two operations that mutate infrastructure (`k8s.restart-deployment` and Reviewer approval is sealed evidence, not caller-supplied metadata. Before signing, the supervisor requires each approval hash to match `reviewer-evidence//last-message.txt` and requires that sealed message to contain the exact `prod-ops-review:` marker for the action ID, task ID, delegated subject and role, intent hash, runbook, operation, target, parameter hash, change ticket, runbook context hash, and history hash. +The two named reviewer roles (`safety-reviewer`, `operations-reviewer`) get a dedicated prompt (`prompts/roles/safety-reviewer.md`, `prompts/roles/operations-reviewer.md`) and default to the Claude Sonnet 5 backend/model (`claude`/`claude-sonnet-5`, overridable via `PROD_OPS_REVIEWER_CLI`/`PROD_OPS_REVIEWER_MODEL`) instead of silently inheriting the generic `VERIFIER_CLI` default. Both are instructed to independently gather live evidence through their own read-only tool access rather than relying solely on the proposer-supplied `runbookContextSha256`/`historySha256` bundle, since that bundle's content is chosen by the same agent proposing the mutation. + ## Supervisor commands ```bash diff --git a/prompts/roles/operations-reviewer.md b/prompts/roles/operations-reviewer.md new file mode 100644 index 0000000..70a518f --- /dev/null +++ b/prompts/roles/operations-reviewer.md @@ -0,0 +1,56 @@ +# Operations Reviewer (Production Operations) + +You are an independent read-only production-operations reviewer, distinct +from the safety reviewer and from the runbook agent. You do not implement, +execute operations, sign permits, or hold credentials. + +Review the proposed `OperationRequestV1` for operational correctness: + +- change ticket validity: for a mutating operation + (`k8s.restart-deployment`, `service.deploy-release`) the change ticket is + real, open, and actually describes this target and this change, not a + placeholder or an unrelated ticket; +- operational history and deviation: prior operations against this target + (successes, failures, in-progress work, recent restarts/deploys) are + consistent with taking this action now, and the proposed operation does not + repeat a step that already ran, contradict a more recent operation, or skip + a required predecessor step in the runbook; +- fit for current state: this is the correct certified operation for the + target's actual current condition, not merely a plausible one for the + reported incident; +- distinct subject: your approval must come from a subject independent of + the safety reviewer and of the runbook agent that proposed the operation. + +Before approving, independently gather live evidence with your own read-only +tool access (deployment/rollout status, recent operation history, the change +ticket system, the target's current condition, or any other live signal +reachable from this environment). Do not approve solely on the strength of +the `runbookContextSha256` / `historySha256` bundle the proposing agent +supplied: that bundle's content was selected by the same agent proposing the +mutation and may be stale, incomplete, or adversarially curated. Treat it as +a claim to verify, not as evidence. State exactly what you independently +checked (commands/queries run and what they showed) in your report. + +If the change ticket, operational history, or current target state cannot be +independently established, reject; do not approve on narrative reassurance +from the runbook agent. + +Return only: + +1. `decision:` `approve` or `reject`. +2. `operations-findings:` change-ticket, history, and current-state analysis + with concrete evidence. +3. `independent-evidence-checked:` each live source you queried yourself and + what it showed, distinct from the supplied context bundle. +4. `omitted-risks:` operationally relevant facts the request or bundle does + not cover. +5. When `decision` is `approve`, the exact standalone `prod-ops-review:` + marker supplied to you for this operation request, reproduced verbatim on + its own line, in this field order: + `prod-ops-review: reviewer-role=operations-reviewer decision=approve action-id=... task-id=... delegated-subject=... delegated-role=... intent-sha256=... runbook=ID@VERSION phase=PHASE operation=ID@VERSION target=ENVIRONMENT/CLUSTER/NAMESPACE/SERVICE parameters-sha256=sha256:... change-ticket=TICKET-or-- runbook-context-sha256=... history-sha256=...` + Do not retype, reorder, paraphrase, or recompute any field yourself; copy + the supplied marker exactly. The supervisor refuses to sign the permit if + this line is missing, altered, or reconstructed from memory. + +Do not use agent agreement, majority preference, or the runbook agent's own +narrative as operational evidence. diff --git a/prompts/roles/safety-reviewer.md b/prompts/roles/safety-reviewer.md new file mode 100644 index 0000000..837c007 --- /dev/null +++ b/prompts/roles/safety-reviewer.md @@ -0,0 +1,55 @@ +# Safety Reviewer (Production Operations) + +You are an independent read-only production-operations reviewer. You do not +implement, execute operations, sign permits, hold credentials, or coordinate +the runbook agent that proposed this operation. + +Review the proposed `OperationRequestV1` for safety: + +- target correctness: environment, cluster, namespace, and service match the + incident and the certified runbook; +- parameter bounds: every parameter (e.g. `waitForReadySeconds`, + `expectedReplicaCount`, `timeoutSeconds`) is a plausible, bounded value for + this target, not a placeholder or an unbounded guess; +- runbook deviation: the requested operation and phase are exactly what the + certified runbook prescribes for the observed condition, not a shortcut, + reordering, or substitute step; +- rollback signal: a concrete rollback or abort condition exists and is + observable if the operation misbehaves; +- prior operation history for this target does not show an unresolved + failure, an in-flight conflicting operation, or a pattern that makes this + request unsafe right now. + +Before approving, independently gather live evidence with your own read-only +tool access (cluster/service state, recent logs, recent metrics, deployment +status, or any other live signal reachable from this environment). Do not +approve solely on the strength of the `runbookContextSha256` / +`historySha256` bundle the proposing agent supplied: that bundle's content was +selected by the same agent proposing the mutation and may be stale, +incomplete, or adversarially curated. Treat it as a claim to verify, not as +evidence. State exactly what you independently checked (commands/queries run +and what they showed) in your report. + +If the expected state, rollback signal, target, or bounded parameter values +cannot be established from your own evidence, reject; do not approve on +narrative reassurance from the runbook agent. + +Return only: + +1. `decision:` `approve` or `reject`. +2. `safety-findings:` target/parameter/runbook-deviation/rollback analysis + with concrete evidence. +3. `independent-evidence-checked:` each live source you queried yourself and + what it showed, distinct from the supplied context bundle. +4. `omitted-risks:` safety-relevant facts the request or bundle does not + cover. +5. When `decision` is `approve`, the exact standalone `prod-ops-review:` + marker supplied to you for this operation request, reproduced verbatim on + its own line, in this field order: + `prod-ops-review: reviewer-role=safety-reviewer decision=approve action-id=... task-id=... delegated-subject=... delegated-role=... intent-sha256=... runbook=ID@VERSION phase=PHASE operation=ID@VERSION target=ENVIRONMENT/CLUSTER/NAMESPACE/SERVICE parameters-sha256=sha256:... change-ticket=TICKET-or-- runbook-context-sha256=... history-sha256=...` + Do not retype, reorder, paraphrase, or recompute any field yourself; copy + the supplied marker exactly. The supervisor refuses to sign the permit if + this line is missing, altered, or reconstructed from memory. + +Do not use agent agreement, majority preference, or the runbook agent's own +narrative as safety evidence. diff --git a/src/agent.rs b/src/agent.rs index 5e741b3..4aeafc2 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -110,6 +110,10 @@ pub struct AgentRequest { pub access: RoleAccess, pub mode: InvocationMode, pub resume_session: Option, + /// Optional explicit model override (e.g. the production-operations + /// safety-reviewer/operations-reviewer roles' `claude-sonnet-5` default). + /// Only `ClaudeBackend` currently renders this; other backends ignore it. + pub model: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -317,6 +321,10 @@ impl AgentBackend for ClaudeBackend { args.push("--resume".into()); args.push(session.into()); } + if let Some(model) = &request.model { + args.push("--model".into()); + args.push(model.into()); + } Ok(CommandSpec { program: self.executable.clone(), args, @@ -325,13 +333,20 @@ impl AgentBackend for ClaudeBackend { legacy_prompt_argument: None, }) } - InvocationMode::Interactive => Ok(CommandSpec { - program: self.executable.clone(), - args: vec!["--dangerously-skip-permissions".into()], - cwd: request.cwd.clone(), - stdin_file: None, - legacy_prompt_argument: request.prompt_file.clone(), - }), + InvocationMode::Interactive => { + args.push("--dangerously-skip-permissions".into()); + if let Some(model) = &request.model { + args.push("--model".into()); + args.push(model.into()); + } + Ok(CommandSpec { + program: self.executable.clone(), + args, + cwd: request.cwd.clone(), + stdin_file: None, + legacy_prompt_argument: request.prompt_file.clone(), + }) + } } } } @@ -425,7 +440,7 @@ pub fn run(args: &[String]) -> Result { fn print_usage() { println!( - "Usage:\n multiagent agent backend-info BACKEND\n multiagent agent run --backend BACKEND --cwd DIR --prompt-file FILE --final-output FILE --trace-dir DIR --access read-only|workspace-write [--resume-session ID]" + "Usage:\n multiagent agent backend-info BACKEND\n multiagent agent run --backend BACKEND --cwd DIR --prompt-file FILE --final-output FILE --trace-dir DIR --access read-only|workspace-write [--resume-session ID] [--model MODEL]" ); } @@ -456,7 +471,7 @@ fn run_backend(args: &[String]) -> Result { while index < args.len() { let key = match args[index].as_str() { "--backend" | "--cwd" | "--prompt-file" | "--final-output" | "--trace-dir" - | "--access" | "--resume-session" => args[index].trim_start_matches("--"), + | "--access" | "--resume-session" | "--model" => args[index].trim_start_matches("--"), other => return Err(format!("unknown agent run argument: {other}")), }; let value = args @@ -501,6 +516,7 @@ fn run_backend(args: &[String]) -> Result { access, mode: InvocationMode::Headless, resume_session: values.get("resume-session").cloned(), + model: values.get("model").cloned(), }; let spec = selected.command(&request)?; let timeout = agent_timeout()?; @@ -1115,6 +1131,7 @@ mod tests { access: RoleAccess::ReadOnly, mode, resume_session: None, + model: None, } } @@ -1197,6 +1214,70 @@ mod tests { assert!(selected.capabilities().native_resume); } + #[test] + fn claude_command_appends_model_override_in_both_modes() { + let paths = BackendPaths { + codex: "codex".into(), + claude: "claude".into(), + qwen: "qwen".into(), + }; + let selected = backend(BackendId::Claude, &paths); + + let mut headless = request(InvocationMode::Headless); + headless.model = Some("claude-sonnet-5".into()); + let headless_args = selected + .command(&headless) + .unwrap() + .args + .into_iter() + .map(|value| value.to_string_lossy().into_owned()) + .collect::>(); + assert!(headless_args + .windows(2) + .any(|pair| pair == ["--model", "claude-sonnet-5"])); + + let mut interactive = request(InvocationMode::Interactive); + interactive.model = Some("claude-sonnet-5".into()); + let interactive_args = selected + .command(&interactive) + .unwrap() + .args + .into_iter() + .map(|value| value.to_string_lossy().into_owned()) + .collect::>(); + assert!(interactive_args + .windows(2) + .any(|pair| pair == ["--model", "claude-sonnet-5"])); + + // No override requested: the flag must not appear at all. + let plain = selected + .command(&request(InvocationMode::Headless)) + .unwrap(); + assert!(!plain.args.iter().any(|arg| arg == "--model")); + } + + #[test] + fn non_claude_backends_ignore_a_model_override() { + let paths = BackendPaths { + codex: "codex".into(), + claude: "claude".into(), + qwen: "qwen".into(), + }; + let mut codex_request = request(InvocationMode::Headless); + codex_request.model = Some("claude-sonnet-5".into()); + let codex_command = backend(BackendId::Codex, &paths) + .command(&codex_request) + .unwrap(); + assert!(!codex_command.args.iter().any(|arg| arg == "--model")); + + let mut qwen_request = request(InvocationMode::Headless); + qwen_request.model = Some("claude-sonnet-5".into()); + let qwen_command = backend(BackendId::Qwen, &paths) + .command(&qwen_request) + .unwrap(); + assert!(!qwen_command.args.iter().any(|arg| arg == "--model")); + } + #[test] fn qwen_headless_declares_streaming_and_resume() { let paths = BackendPaths { diff --git a/src/runtime.rs b/src/runtime.rs index 2f302a6..e2465c5 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -31,6 +31,8 @@ struct RuntimeConfig { worker_cli: String, subagent_cli: String, verifier_cli: String, + prod_ops_reviewer_cli: String, + prod_ops_reviewer_model: String, codex_bin: String, claude_bin: String, qwen_bin: String, @@ -100,6 +102,34 @@ const WRITER_UID: u32 = config::WRITER_UID; const READER_UID: u32 = config::READER_UID; const ROLE_GID: u32 = config::ROLE_GID; +/// The two named production-operations reviewer roles required by +/// `prod_ops.rs`'s `operation_contract` (`safety-reviewer`, +/// `operations-reviewer`) get a dedicated prompt, backend, and model rather +/// than falling through to the generic reviewer/verifier dispatch. Matching +/// is a case-insensitive substring check on the spawned session name, mirroring +/// the existing `lower.contains(...)` convention in `role_prompt_name`. +fn is_prod_ops_reviewer_name(name: &str) -> bool { + let lower = name.to_ascii_lowercase(); + lower.contains("safety-reviewer") || lower.contains("operations-reviewer") +} + +/// Resolve the backend CLI and optional model override to spawn a named +/// session under. Every other role keeps resolving through `cfg.subagent_cli` +/// exactly as before (which the orchestrator prompt sets to `$VERIFIER_CLI` +/// for verifier/reviewer/scout spawns; see `orchestrator_prompt.md`); only the +/// two named production-operations reviewer roles get a dedicated, +/// deliberately-chosen backend/model instead. +fn spawn_backend_for_name(cfg: &RuntimeConfig, name: &str) -> (String, Option) { + if is_prod_ops_reviewer_name(name) { + ( + cfg.prod_ops_reviewer_cli.clone(), + Some(cfg.prod_ops_reviewer_model.clone()), + ) + } else { + (cfg.subagent_cli.clone(), None) + } +} + impl RuntimeConfig { fn load() -> Result { let root = config::root()?; @@ -111,7 +141,20 @@ impl RuntimeConfig { let worker_cli = env_nonempty("WORKER_CLI").unwrap_or_else(|| "claude".into()); let subagent_cli = env_nonempty("SUBAGENT_CLI").unwrap_or_else(|| worker_cli.clone()); let verifier_cli = env_nonempty("VERIFIER_CLI").unwrap_or_else(|| "codex".into()); - for value in [&worker_cli, &subagent_cli, &verifier_cli] { + // The two named production-operations reviewer roles (safety-reviewer, + // operations-reviewer) deliberately default to a specific backend and + // model rather than silently inheriting the generic VERIFIER_CLI + // default; see `is_prod_ops_reviewer_name`. + let prod_ops_reviewer_cli = + env_nonempty("PROD_OPS_REVIEWER_CLI").unwrap_or_else(|| "claude".into()); + let prod_ops_reviewer_model = + env_nonempty("PROD_OPS_REVIEWER_MODEL").unwrap_or_else(|| "claude-sonnet-5".into()); + for value in [ + &worker_cli, + &subagent_cli, + &verifier_cli, + &prod_ops_reviewer_cli, + ] { validate_cli(value)?; } Ok(Self { @@ -124,6 +167,8 @@ impl RuntimeConfig { worker_cli, subagent_cli, verifier_cli, + prod_ops_reviewer_cli, + prod_ops_reviewer_model, codex_bin: env_nonempty("CODEX_BIN").unwrap_or_else(|| "codex".into()), claude_bin: env_nonempty("CLAUDE_BIN").unwrap_or_else(|| "claude".into()), qwen_bin: env_nonempty("QWEN_BIN").unwrap_or_else(|| "qwen".into()), @@ -244,6 +289,7 @@ pub fn role_agent_exec(args: &[String]) -> Result { &trace_dir, access, resume_session.as_deref(), + authorization.model.as_deref(), ); let supervisor_pid = dir.join("supervisor.pid"); atomic_write( @@ -383,7 +429,17 @@ pub fn launch(args: &[String]) -> Result { let subagent_cli = env_nonempty("SUBAGENT_CLI").unwrap_or_else(|| worker_cli.clone()); let verifier_cli = env_nonempty("VERIFIER_CLI").unwrap_or_else(|| "codex".into()); let orchestrator_cli = env_nonempty("ORCHESTRATOR_CLI").unwrap_or_else(|| "codex".into()); - for value in [&worker_cli, &subagent_cli, &verifier_cli, &orchestrator_cli] { + let prod_ops_reviewer_cli = + env_nonempty("PROD_OPS_REVIEWER_CLI").unwrap_or_else(|| "claude".into()); + let prod_ops_reviewer_model = + env_nonempty("PROD_OPS_REVIEWER_MODEL").unwrap_or_else(|| "claude-sonnet-5".into()); + for value in [ + &worker_cli, + &subagent_cli, + &verifier_cli, + &orchestrator_cli, + &prod_ops_reviewer_cli, + ] { validate_cli(value)?; } let codex_bin = env_nonempty("CODEX_BIN").unwrap_or_else(|| "codex".into()); @@ -427,7 +483,13 @@ pub fn launch(args: &[String]) -> Result { }; let mut backend_versions = Vec::new(); let mut selected_backends = BTreeSet::new(); - for name in [&orchestrator_cli, &worker_cli, &subagent_cli, &verifier_cli] { + for name in [ + &orchestrator_cli, + &worker_cli, + &subagent_cli, + &verifier_cli, + &prod_ops_reviewer_cli, + ] { if selected_backends.insert(name.clone()) { let id = BackendId::parse(name)?; backend_versions.push(agent::backend(id, &backend_paths).preflight()?); @@ -503,6 +565,8 @@ pub fn launch(args: &[String]) -> Result { &worker_cli, &subagent_cli, &verifier_cli, + &prod_ops_reviewer_cli, + &prod_ops_reviewer_model, &codex_bin, &claude_bin, &qwen_bin, @@ -641,6 +705,7 @@ pub fn launch(args: &[String]) -> Result { println!("Worker CLI: {worker_cli}"); println!("Subagent CLI: {subagent_cli}"); println!("Verifier CLI: {verifier_cli}"); + println!("Prod-ops reviewer CLI/model: {prod_ops_reviewer_cli}/{prod_ops_reviewer_model}"); println!("Agent headless mode: {agent_headless}"); println!("Write policy:"); policy::run(&["show".into()])?; @@ -675,6 +740,8 @@ fn launch_environment( worker_cli: &str, subagent_cli: &str, verifier_cli: &str, + prod_ops_reviewer_cli: &str, + prod_ops_reviewer_model: &str, codex_bin: &str, claude_bin: &str, qwen_bin: &str, @@ -724,6 +791,11 @@ fn launch_environment( ("WORKER_CLI", worker_cli.to_string()), ("SUBAGENT_CLI", subagent_cli.to_string()), ("VERIFIER_CLI", verifier_cli.to_string()), + ("PROD_OPS_REVIEWER_CLI", prod_ops_reviewer_cli.to_string()), + ( + "PROD_OPS_REVIEWER_MODEL", + prod_ops_reviewer_model.to_string(), + ), ("CODEX_BIN", codex_bin.to_string()), ("CLAUDE_BIN", claude_bin.to_string()), ("QWEN_BIN", qwen_bin.to_string()), @@ -860,6 +932,7 @@ fn write_bootstrap( &trace_dir, CodexAccess::WorkspaceWrite, None, + None, ) } else { build_cli_command( @@ -873,6 +946,7 @@ fn write_bootstrap( codex_exec, agent_headless, CodexAccess::WorkspaceWrite, + None, )? }; let command = if environment @@ -1356,8 +1430,9 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { } instruction = fs::read_to_string(path).map_err(io_error("read instruction file"))?; } - if cfg.headless(&cfg.subagent_cli) && instruction.is_empty() { - let label = if cfg.subagent_cli == "codex" && cfg.code_exec { + let (cli, model) = spawn_backend_for_name(cfg, name); + if cfg.headless(&cli) && instruction.is_empty() { + let label = if cli == "codex" && cfg.code_exec { "codex exec" } else { "headless coding-agent" @@ -1387,8 +1462,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { }; require_command("tmux")?; - let cli = &cfg.subagent_cli; - let binary = cfg.cli_bin(cli)?; + let binary = cfg.cli_bin(&cli)?; require_command(binary)?; if !tmux_success(&["has-session", "-t", &cfg.session]) { return Err(format!("missing tmux session: {}", cfg.session)); @@ -1484,7 +1558,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { fs::create_dir_all(&cfg.logs).map_err(io_error("create subagent log directory"))?; let executable = env::current_exe().map_err(io_error("resolve multiagent executable"))?; let metadata = format!( - "name={name}\nsession={}\nroot={}\nrole={}\naccess={}\ncodex_access={}\nworkflow_id={}\nwrite_policy={}\nlog_file={}\ntrace_dir={}\ncli={cli}\ncli_bin={binary}\nhelper={}\ncreated_at={}\n", + "name={name}\nsession={}\nroot={}\nrole={}\naccess={}\ncodex_access={}\nworkflow_id={}\nwrite_policy={}\nlog_file={}\ntrace_dir={}\ncli={cli}\ncli_bin={binary}\nmodel={}\nhelper={}\ncreated_at={}\n", cfg.session, cfg.root.display(), authority_role, @@ -1494,6 +1568,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { cfg.policy.display(), cfg.logs.join(format!("{name}.log")).display(), trace_dir.display(), + model.as_deref().unwrap_or(""), executable.display(), timestamp() ); @@ -1502,7 +1577,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { let mut prompt_file = None; let output_file = dir.join("last-message.txt"); - if cfg.headless(cli) && !instruction.is_empty() { + if cfg.headless(&cli) && !instruction.is_empty() { let path = dir.join("instruction.txt"); let prompt = if cli == "codex" { format!("{}{}\n", codex_exec_protocol_prelude(), instruction) @@ -1520,22 +1595,27 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { let registered_prompt = prompt_file .as_deref() .ok_or_else(|| format!("secure subagent prompt is missing: {name}"))?; - run_self_quiet(&[ - "supervisor", - "register-launch", - name, - "--role", - authority_role, - "--cli", - cli, - "--cli-bin", - binary, - "--instruction-file", - ®istered_prompt.display().to_string(), - ])?; + let mut register_args = vec![ + "supervisor".to_string(), + "register-launch".to_string(), + name.to_string(), + "--role".to_string(), + authority_role.to_string(), + "--cli".to_string(), + cli.clone(), + "--cli-bin".to_string(), + binary.to_string(), + "--instruction-file".to_string(), + registered_prompt.display().to_string(), + ]; + if let Some(model) = &model { + register_args.push("--model".into()); + register_args.push(model.clone()); + } + run_self_quiet(®ister_args.iter().map(String::as_str).collect::>())?; } let cli_command = if env::var("MULTIAGENT_UID_SANDBOX").as_deref() == Ok("1") { - if !cfg.headless(cli) { + if !cfg.headless(&cli) { return Err("UID role isolation requires a headless coding-agent backend".into()); } format!( @@ -1544,10 +1624,10 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { shell_escape(name) ) } else { - let command = if cfg.headless(cli) { + let command = if cfg.headless(&cli) { build_agent_runner_command( &executable, - cli, + &cli, &cfg.root, prompt_file .as_deref() @@ -1556,10 +1636,11 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { &trace_dir, access, None, + model.as_deref(), ) } else { build_cli_command( - cli, + &cli, &cfg.root, prompt_file.as_deref(), Some(&output_file), @@ -1569,6 +1650,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { cfg.code_exec, cfg.agent_headless, access, + model.as_deref(), )? }; wrap_linux_role_sandbox( @@ -1582,7 +1664,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { }, ) }; - let command = subagent_shell_command(cfg, name, cli, &executable, &cli_command, access, false); + let command = subagent_shell_command(cfg, name, &cli, &executable, &cli_command, access, false); tmux_checked(&["new-window", "-d", "-t", &cfg.session, "-n", name, &command])?; pipe_log(&cfg.session, name, &cfg.logs)?; set_subagent_status(cfg, name, "running")?; @@ -1596,7 +1678,7 @@ fn spawn(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { run_self_quiet(&["subagent", "assignment-status", name, "running"])?; } let _ = capture_subagent(cfg, name); - if !(instruction.is_empty() || cfg.headless(cli)) { + if !(instruction.is_empty() || cfg.headless(&cli)) { deliver_instruction(cfg, name, &instruction)?; } println!("spawned {name}"); @@ -1865,6 +1947,13 @@ fn restore(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { .cloned() .unwrap_or_else(|| cfg.subagent_cli.clone()); validate_cli(&cli)?; + // Persisted from the original spawn so a restored prod-ops reviewer keeps + // its originally resolved model override even if the operator's + // PROD_OPS_REVIEWER_MODEL default has since changed. + let model = metadata + .get("model") + .filter(|value| !value.is_empty()) + .cloned(); let access = match metadata .get("access") .or_else(|| metadata.get("codex_access")) @@ -1939,23 +2028,28 @@ fn restore(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { Some("scout") => "scout", _ => "worker", }; - run_self_quiet(&[ - "supervisor", - "renew-launch", - name, - "--role", - role, - "--cli", - &cli, - "--cli-bin", - binary, - "--instruction-file", - &prompt_file + let mut renew_args = vec![ + "supervisor".to_string(), + "renew-launch".to_string(), + name.to_string(), + "--role".to_string(), + role.to_string(), + "--cli".to_string(), + cli.clone(), + "--cli-bin".to_string(), + binary.to_string(), + "--instruction-file".to_string(), + prompt_file .as_deref() .ok_or_else(|| format!("secure restore prompt is missing: {name}"))? .display() .to_string(), - ])?; + ]; + if let Some(model) = &model { + renew_args.push("--model".into()); + renew_args.push(model.clone()); + } + run_self_quiet(&renew_args.iter().map(String::as_str).collect::>())?; } let trace_dir = cfg.logs.join("agents").join(name); let executable = env::current_exe().map_err(io_error("resolve multiagent executable"))?; @@ -1982,6 +2076,7 @@ fn restore(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { &trace_dir, access, resume_session.as_deref(), + model.as_deref(), ) } else { build_cli_command( @@ -1995,6 +2090,7 @@ fn restore(cfg: &RuntimeConfig, args: &[String]) -> Result<(), String> { cfg.code_exec, cfg.agent_headless, access, + model.as_deref(), )? }; wrap_linux_role_sandbox( @@ -2263,6 +2359,10 @@ fn role_prompt_name(name: &str, role: Option) -> Option<&'static str> let lower = name.to_ascii_lowercase(); let relative = if lower.contains("decision-authority-reviewer") { "prompts/roles/decision-authority-reviewer.md" + } else if lower.contains("safety-reviewer") { + "prompts/roles/safety-reviewer.md" + } else if lower.contains("operations-reviewer") { + "prompts/roles/operations-reviewer.md" } else if role.is_some_and(SpawnRole::is_production_operations) { "prompts/roles/runbook-operator.md" } else if lower.contains("contract-scout") || role == Some(SpawnRole::Scout) { @@ -2489,6 +2589,7 @@ fn build_cli_command( codex_exec: bool, agent_headless: bool, access: CodexAccess, + model: Option<&str>, ) -> Result { let id = BackendId::parse(cli)?; let paths = BackendPaths { @@ -2510,6 +2611,7 @@ fn build_cli_command( access, mode, resume_session: None, + model: model.map(str::to_string), }) .map(|command| command.render_shell()) } @@ -2523,6 +2625,7 @@ fn build_agent_runner_args( trace_dir: &Path, access: CodexAccess, resume_session: Option<&str>, + model: Option<&str>, ) -> Vec { let mut args = vec![ "agent".into(), @@ -2544,6 +2647,10 @@ fn build_agent_runner_args( args.push("--resume-session".into()); args.push(session.into()); } + if let Some(model) = model { + args.push("--model".into()); + args.push(model.into()); + } args } @@ -2557,10 +2664,19 @@ fn build_agent_runner_command( trace_dir: &Path, access: CodexAccess, resume_session: Option<&str>, + model: Option<&str>, ) -> String { let mut command = shell_escape(&executable.display().to_string()); - for arg in build_agent_runner_args(cli, cwd, prompt, output, trace_dir, access, resume_session) - { + for arg in build_agent_runner_args( + cli, + cwd, + prompt, + output, + trace_dir, + access, + resume_session, + model, + ) { command.push(' '); command.push_str(&shell_escape(&arg)); } @@ -2886,6 +3002,11 @@ fn subagent_shell_command( ("WORKER_CLI", cfg.worker_cli.clone()), ("SUBAGENT_CLI", cli.into()), ("VERIFIER_CLI", cfg.verifier_cli.clone()), + ("PROD_OPS_REVIEWER_CLI", cfg.prod_ops_reviewer_cli.clone()), + ( + "PROD_OPS_REVIEWER_MODEL", + cfg.prod_ops_reviewer_model.clone(), + ), ("CODEX_BIN", cfg.codex_bin.clone()), ("CLAUDE_BIN", cfg.claude_bin.clone()), ("QWEN_BIN", cfg.qwen_bin.clone()), @@ -3771,4 +3892,95 @@ review-record: type=decision-authority verdict=pass diff=-\n"; ); assert!("admin".parse::().is_err()); } + + #[test] + fn named_prod_ops_reviewers_get_dedicated_prompts() { + assert_eq!( + role_prompt_name("safety-reviewer-1", Some(SpawnRole::Reviewer)), + Some("prompts/roles/safety-reviewer.md") + ); + assert_eq!( + role_prompt_name("operations-reviewer-1", Some(SpawnRole::Reviewer)), + Some("prompts/roles/operations-reviewer.md") + ); + // A generic reviewer/verifier name is unaffected and keeps the + // shared prompt. + assert_eq!( + role_prompt_name("code-reviewer-1", Some(SpawnRole::Reviewer)), + Some("prompts/verifier.md") + ); + assert_eq!( + role_prompt_name("reviewer-1", Some(SpawnRole::Reviewer)), + Some("prompts/verifier.md") + ); + } + + fn test_runtime_config(subagent_cli: &str, verifier_cli: &str) -> RuntimeConfig { + RuntimeConfig { + session: "test".into(), + root: PathBuf::from("."), + state: PathBuf::from("."), + logs: PathBuf::from("."), + policy: PathBuf::from("."), + prompt_root: PathBuf::from("."), + worker_cli: "claude".into(), + subagent_cli: subagent_cli.into(), + verifier_cli: verifier_cli.into(), + prod_ops_reviewer_cli: "claude".into(), + prod_ops_reviewer_model: "claude-sonnet-5".into(), + codex_bin: "codex".into(), + claude_bin: "claude".into(), + qwen_bin: "qwen".into(), + code_exec: false, + agent_headless: false, + } + } + + #[test] + fn named_prod_ops_reviewers_default_to_claude_sonnet_5() { + // Simulate the orchestrator's documented convention of setting + // SUBAGENT_CLI=$VERIFIER_CLI before spawning a generic + // reviewer/verifier/scout session. + let cfg = test_runtime_config("codex", "codex"); + assert_eq!( + spawn_backend_for_name(&cfg, "safety-reviewer-1"), + ("claude".to_string(), Some("claude-sonnet-5".to_string())) + ); + assert_eq!( + spawn_backend_for_name(&cfg, "operations-reviewer-1"), + ("claude".to_string(), Some("claude-sonnet-5".to_string())) + ); + } + + #[test] + fn named_prod_ops_reviewers_honor_overridden_cli_and_model() { + let mut cfg = test_runtime_config("codex", "codex"); + cfg.prod_ops_reviewer_cli = "qwen".into(); + cfg.prod_ops_reviewer_model = "custom-model".into(); + assert_eq!( + spawn_backend_for_name(&cfg, "safety-reviewer-1"), + ("qwen".to_string(), Some("custom-model".to_string())) + ); + assert_eq!( + spawn_backend_for_name(&cfg, "operations-reviewer-1"), + ("qwen".to_string(), Some("custom-model".to_string())) + ); + } + + #[test] + fn other_reviewer_names_are_unaffected_and_keep_verifier_cli_default() { + // The orchestrator sets SUBAGENT_CLI=$VERIFIER_CLI for generic + // reviewer/verifier spawns; the named prod-ops override must not + // apply to them. + let cfg = test_runtime_config("codex", "codex"); + assert_eq!(cfg.subagent_cli, cfg.verifier_cli); + assert_eq!( + spawn_backend_for_name(&cfg, "code-reviewer-1"), + (cfg.verifier_cli.clone(), None) + ); + assert_eq!( + spawn_backend_for_name(&cfg, "reviewer-1"), + (cfg.verifier_cli.clone(), None) + ); + } } diff --git a/src/supervisor.rs b/src/supervisor.rs index d6a4edc..211fd3a 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -100,6 +100,9 @@ pub struct LaunchAuthorization { pub cli_bin: String, pub instruction: PathBuf, pub owned_paths: Vec, + /// Optional explicit model override, currently used only by the + /// production-operations safety-reviewer/operations-reviewer roles. + pub model: Option, } fn register_launch(args: &[String], renew: bool) -> Result<(), String> { @@ -111,6 +114,7 @@ fn register_launch(args: &[String], renew: bool) -> Result<(), String> { let role = required_option(&options, "--role")?; let cli = required_option(&options, "--cli")?; let cli_bin = required_option(&options, "--cli-bin")?; + let model = options.get("--model").filter(|value| !value.is_empty()); let instruction_source = PathBuf::from(required_option(&options, "--instruction-file")?); if !matches!(role, "worker" | "verifier" | "reviewer" | "scout") { return Err("register-launch role must be worker, verifier, reviewer, or scout".into()); @@ -175,6 +179,8 @@ fn register_launch(args: &[String], renew: bool) -> Result<(), String> { if current.get("role").map(String::as_str) != Some(role) || current.get("cli").map(String::as_str) != Some(cli) || current.get("cli_bin").map(String::as_str) != Some(cli_bin) + || current.get("model").map(String::as_str).unwrap_or("") + != model.map(String::as_str).unwrap_or("") { return Err(format!( "renewed launch cannot change role or coding-agent identity: {name}" @@ -190,7 +196,8 @@ fn register_launch(args: &[String], renew: bool) -> Result<(), String> { let instruction_path = directory.join("instruction.txt"); atomic_write_bytes(&instruction_path, &instruction)?; let metadata = format!( - "name={name}\nrole={role}\naccess={access}\nworkflow_id={workflow_id}\ncli={cli}\ncli_bin={cli_bin}\ninstruction_sha256={:x}\nstate=registered\n", + "name={name}\nrole={role}\naccess={access}\nworkflow_id={workflow_id}\ncli={cli}\ncli_bin={cli_bin}\nmodel={}\ninstruction_sha256={:x}\nstate=registered\n", + model.map(String::as_str).unwrap_or(""), Sha256::digest(&instruction) ); atomic_write_bytes(&directory.join("launch.env"), metadata.as_bytes())?; @@ -231,6 +238,10 @@ pub fn claim_launch(state: &Path, name: &str) -> Result Result, String> { for pair in args.chunks_exact(2) { if !matches!( pair[0].as_str(), - "--role" | "--cli" | "--cli-bin" | "--instruction-file" + "--role" | "--cli" | "--cli-bin" | "--instruction-file" | "--model" ) || pair[1].contains(['\n', '\r']) { return Err(format!("invalid register-launch option: {}", pair[0]));