Skip to main content

mj_controller/controller/
reviewer.rs

1//! Staging the second-opinion reviewer's profile onto a session's target.
2//!
3//! The reviewer runs a different configured profile in the primary session's
4//! own target. Its harness home is a fresh copy of that profile, placed inside
5//! the primary worker root, so the reviewer never reads or writes the
6//! primary's home and nothing outside the worker root has to be provisioned.
7//!
8//! Nothing here creates a session record, a target, a checkout, or any target
9//! lifecycle operation. Staging a reviewer is a file copy into a directory the
10//! session's worker already owns.
11
12use std::path::Path;
13
14use anyhow::{Context, Result, bail, ensure};
15
16use super::worker_binary::{
17    apply_staged_execution_setting, bridge_launch, container_upload_ownership_args, stage_profile,
18};
19use super::{Controller, execute_checked};
20use crate::targets::{self, CommandExecutor, CommandSpec, ProcessExecutor};
21use mj_core::worker_launch::{
22    ReviewMcpDelivery, ReviewMcpServer, ReviewerLaunchConfig, reviewer_staging_profile_home,
23};
24
25/// Build the capability used by client-side chat views to stage reviewers.
26/// The blocking filesystem and target work remains implemented by the
27/// controller and is invoked from chat's supervised blocking task.
28pub fn reviewer_stager() -> mj_client::session::ReviewerStager {
29    mj_client::session::ReviewerStager::new(ControllerReviewerStager)
30}
31
32struct ControllerReviewerStager;
33
34impl mj_client::session::ReviewerStagerBackend for ControllerReviewerStager {
35    fn stage(
36        &self,
37        _config: mj_core::config::Config,
38        session: mj_core::state::SessionRecord,
39        profile_id: String,
40        generation: u64,
41        cancelled: std::sync::Arc<std::sync::atomic::AtomicBool>,
42    ) -> Result<ReviewerLaunchConfig> {
43        let session_id = session.id.clone();
44        let controller = Controller::load()?;
45        let executor = crate::targets::CancellableProcessExecutor::new(cancelled)
46            .with_deadline(std::time::Duration::from_secs(90));
47        controller.stage_reviewer_profile_controlled(
48            &session_id,
49            &profile_id,
50            generation,
51            &[],
52            &executor,
53        )
54    }
55}
56
57impl Controller {
58    /// Copy `profile_id`'s home into the session worker's reviewer directory
59    /// and describe how the worker should launch it.
60    ///
61    /// `generation` distinguishes reviewer lifetimes: bumping it tells the
62    /// worker to start a new conversation instead of reloading the last one.
63    pub fn stage_reviewer_profile(
64        &self,
65        session_id: &str,
66        profile_id: &str,
67        generation: u64,
68    ) -> Result<ReviewerLaunchConfig> {
69        self.stage_reviewer_profile_controlled(
70            session_id,
71            profile_id,
72            generation,
73            &[],
74            &ProcessExecutor,
75        )
76    }
77
78    pub fn stage_reviewer_profile_controlled(
79        &self,
80        session_id: &str,
81        profile_id: &str,
82        generation: u64,
83        mcp_servers: &[ReviewMcpServer],
84        executor: &impl CommandExecutor,
85    ) -> Result<ReviewerLaunchConfig> {
86        let profile = self
87            .config
88            .profiles
89            .get(profile_id)
90            .with_context(|| format!("unknown profile {profile_id:?}"))?;
91        ensure!(
92            profile.enabled,
93            "reviewer profile {profile_id:?} is disabled"
94        );
95        let session = self
96            .state
97            .sessions
98            .get(session_id)
99            .with_context(|| format!("unknown session {session_id}"))?;
100        let target = session.target_runtime_settings(&self.config)?;
101        let execution_policy = target.execution_policy;
102        let (backend, worker_root) = self.worker_placement(session_id)?;
103
104        let staging = tempfile::tempdir().context("create reviewer staging directory")?;
105        let local = staging.path().join("profile");
106        profile.ensure_ready(profile_id)?;
107        stage_profile(profile, &local).with_context(|| format!("stage profile {profile_id:?}"))?;
108        apply_staged_execution_setting(profile.kind, execution_policy, &local)
109            .with_context(|| format!("stage profile {profile_id:?}"))?;
110        // Harnesses that ignore MCP servers offered over ACP read their own
111        // configuration instead, so the servers are written into the copy
112        // being staged, before it is uploaded.
113        if !mcp_servers.is_empty()
114            && ReviewMcpDelivery::for_harness(profile.kind) == ReviewMcpDelivery::HarnessProfile
115        {
116            configure_staged_review_mcp(profile.kind, &local, mcp_servers)
117                .with_context(|| format!("configure reviewer MCP servers for {profile_id:?}"))?;
118        }
119        upload_reviewer_profile(executor, &backend, &worker_root, generation, &local)?;
120
121        let (bridge_command, bridge_args) = bridge_launch(profile.kind, execution_policy);
122        let mut environment = profile.environment.resolved().clone();
123        // The worker sets the harness home from the directory it staged, so
124        // sending one here could only point the reviewer somewhere it must not
125        // read.
126        environment.remove(profile.home_env());
127        // A reviewer on a ChatGPT Codex profile must not fall back to an API
128        // key any more than a session may.
129        let excluded_environment = profile.exclude_harness_environment(&mut environment);
130        // A Podman session runs as uid 0, where Claude Code rejects
131        // bypassPermissions unless told it is already sandboxed. The session's
132        // own launch sets this only when the session itself runs Claude.
133        if backend.container_engine() == Some("podman")
134            && profile.kind == mj_core::config::HarnessKind::Claude
135        {
136            environment.insert("IS_SANDBOX".into(), "1".into());
137        }
138        Ok(ReviewerLaunchConfig {
139            profile_id: profile_id.to_owned(),
140            harness: profile.kind,
141            bridge_command: bridge_command.into(),
142            bridge_args,
143            environment,
144            excluded_environment,
145            execution_policy,
146            model: None,
147            effort: None,
148            fast_mode: None,
149            generation,
150            // Keep ownership metadata for approval policy even when connection
151            // configuration lives in the staged profile. The worker owns ACP delivery.
152            mcp_servers: mcp_servers.to_vec(),
153        })
154    }
155}
156
157/// Writes `servers` into the staged profile of a harness that reads its MCP
158/// configuration from disk.
159///
160/// Claude Code reads `mcpServers` from `.claude.json` in its config directory;
161/// Kimi reads `mcpServers` from `mcp.json` in its home, and needs the runtime
162/// id its own schema carries. Both files are the reviewer's private copy, so
163/// nothing here can reach the user's own configuration.
164fn configure_staged_review_mcp(
165    harness: mj_core::config::HarnessKind,
166    profile_stage: &Path,
167    servers: &[ReviewMcpServer],
168) -> Result<()> {
169    let Some(file) = harness.mcp_config_file() else {
170        bail!("{harness:?} does not read MCP servers from its profile");
171    };
172    let kimi = harness == mj_core::config::HarnessKind::Kimi;
173    let path = profile_stage.join(file);
174    let mut document = match std::fs::read(&path) {
175        Ok(body) => serde_json::from_slice::<serde_json::Value>(&body)
176            .with_context(|| format!("parse staged reviewer configuration {}", path.display()))?,
177        Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
178            serde_json::Value::Object(serde_json::Map::new())
179        }
180        Err(error) => {
181            return Err(error)
182                .with_context(|| format!("read staged reviewer configuration {}", path.display()));
183        }
184    };
185    let root = document.as_object_mut().with_context(|| {
186        format!(
187            "staged reviewer configuration {} must contain a JSON object",
188            path.display()
189        )
190    })?;
191    let configured = root
192        .entry("mcpServers")
193        .or_insert_with(|| serde_json::Value::Object(serde_json::Map::new()))
194        .as_object_mut()
195        .with_context(|| {
196            format!(
197                "mcpServers in staged reviewer configuration {} must be a JSON object",
198                path.display()
199            )
200        })?;
201    for server in servers {
202        let mut entry = serde_json::json!({
203            "command": server.command,
204            "args": server.args,
205        });
206        if kimi {
207            let object = entry
208                .as_object_mut()
209                .expect("the server entry is a JSON object");
210            object.insert("transport".into(), "stdio".into());
211            object.insert("runtime_id".into(), "local".into());
212        } else {
213            let object = entry
214                .as_object_mut()
215                .expect("the server entry is a JSON object");
216            object.insert("type".into(), "stdio".into());
217        }
218        configured.insert(server.name.clone(), entry);
219    }
220    let mut body = serde_json::to_vec_pretty(&document)?;
221    body.push(b'\n');
222    mj_core::config::atomic_write(&path, &body)
223        .with_context(|| format!("write staged reviewer configuration {}", path.display()))
224}
225
226/// Where one immutable reviewer profile snapshot lives on the target.
227///
228/// The worker copies this source into each role's private harness home before
229/// launch. Keeping generations in separate directories means staging a lane
230/// cannot replace the profile a running role is using.
231fn reviewer_profile_home(worker_root: &str, generation: u64) -> String {
232    reviewer_staging_profile_home(Path::new(worker_root), generation)
233        .to_string_lossy()
234        .into_owned()
235}
236
237/// Replace this generation's staged profile with a fresh copy of `local`.
238///
239/// The previous snapshot for this generation is removed first: a reviewer
240/// profile is a snapshot of the user's configured home, and merging a new copy
241/// over an old one would leave credentials and skills the source no longer has.
242fn upload_reviewer_profile(
243    executor: &impl CommandExecutor,
244    locator: &targets::TargetLocator,
245    worker_root: &str,
246    generation: u64,
247    local: &Path,
248) -> Result<()> {
249    let home = reviewer_profile_home(worker_root, generation);
250    match locator {
251        targets::TargetLocator::LocalBare { .. } => {
252            for command in [
253                CommandSpec::new("rm", ["-rf", "--", &home])
254                    .purpose("clear the local reviewer profile"),
255                CommandSpec::new("mkdir", ["-p", &home])
256                    .purpose("create the local reviewer profile directory"),
257                CommandSpec::new(
258                    "cp",
259                    [
260                        "-R".to_owned(),
261                        format!("{}/.", local.display()),
262                        home.clone(),
263                    ],
264                )
265                .purpose("install the local reviewer profile"),
266                CommandSpec::new("chmod", ["-R", "go-rwx", &home])
267                    .purpose("restrict local reviewer profile permissions"),
268            ] {
269                execute_checked(executor, command)?;
270            }
271        }
272        targets::TargetLocator::LocalPodman { container_id, .. }
273        | targets::TargetLocator::LocalDocker { container_id, .. }
274        | targets::TargetLocator::AppleContainer { container_id, .. } => {
275            let engine = match locator {
276                targets::TargetLocator::LocalPodman { .. } => "podman",
277                targets::TargetLocator::LocalDocker { .. } => "docker",
278                targets::TargetLocator::AppleContainer { .. } => "container",
279                _ => unreachable!("matched local container target"),
280            };
281            for arguments in [
282                vec![
283                    "exec".to_owned(),
284                    container_id.clone(),
285                    "rm".to_owned(),
286                    "-rf".to_owned(),
287                    "--".to_owned(),
288                    home.clone(),
289                ],
290                vec![
291                    "exec".to_owned(),
292                    container_id.clone(),
293                    "mkdir".to_owned(),
294                    "-p".to_owned(),
295                    home.clone(),
296                ],
297                vec![
298                    "cp".to_owned(),
299                    format!("{}/.", local.display()),
300                    format!("{container_id}:{home}"),
301                ],
302                container_upload_ownership_args(engine, container_id, worker_root, &[&home]),
303                vec![
304                    "exec".to_owned(),
305                    container_id.clone(),
306                    "chmod".to_owned(),
307                    "-R".to_owned(),
308                    "go-rwx".to_owned(),
309                    home.clone(),
310                ],
311            ] {
312                execute_checked(
313                    executor,
314                    CommandSpec::new(engine, arguments).purpose("stage the reviewer profile"),
315                )?;
316            }
317        }
318        targets::TargetLocator::AwsEc2 { ssh, .. }
319        | targets::TargetLocator::SshBare { ssh, .. } => {
320            let incoming = format!("{home}.incoming");
321            execute_checked(
322                executor,
323                crate::targets::ssh_command(ssh, ["mkdir", "-p", worker_root])
324                    .purpose("create the reviewer directory"),
325            )?;
326            execute_checked(
327                executor,
328                crate::targets::ssh_command(ssh, ["rm", "-rf", "--", &incoming, &home])
329                    .purpose("clear the reviewer profile"),
330            )?;
331            execute_checked(
332                executor,
333                crate::targets::scp_upload(ssh, local, &incoming, true)
334                    .purpose("upload the reviewer profile"),
335            )?;
336            execute_checked(
337                executor,
338                crate::targets::ssh_command(ssh, ["mv", &incoming, &home])
339                    .purpose("install the reviewer profile"),
340            )?;
341            execute_checked(
342                executor,
343                crate::targets::ssh_command(ssh, ["chmod", "-R", "go-rwx", &home])
344                    .purpose("restrict reviewer profile permissions"),
345            )?;
346        }
347        targets::TargetLocator::SshPodman {
348            ssh, container_id, ..
349        }
350        | targets::TargetLocator::SshDocker {
351            ssh, container_id, ..
352        } => {
353            let engine = match locator {
354                targets::TargetLocator::SshPodman { .. } => "podman",
355                targets::TargetLocator::SshDocker { .. } => "docker",
356                _ => unreachable!("matched remote container target"),
357            };
358            // `worker_root` is a path inside the container, so it does not exist
359            // on the SSH host; stage on the host the way checkpoint uploads do.
360            let worker = Path::new(worker_root)
361                .file_name()
362                .map(|name| name.to_string_lossy().into_owned())
363                .filter(|name| !name.is_empty())
364                .ok_or_else(|| anyhow::anyhow!("worker root {worker_root:?} names no worker"))?;
365            let upload = format!(
366                "{}/{worker}-reviewer-{generation}",
367                targets::REMOTE_UPLOAD_STAGING
368            );
369            execute_checked(
370                executor,
371                crate::targets::ssh_command(ssh, ["mkdir", "-p", targets::REMOTE_UPLOAD_STAGING])
372                    .purpose("create remote reviewer staging"),
373            )?;
374            execute_checked(
375                executor,
376                crate::targets::ssh_command(ssh, ["rm", "-rf", "--", &upload])
377                    .purpose("clear remote reviewer staging"),
378            )?;
379            execute_checked(
380                executor,
381                crate::targets::scp_upload(ssh, local, &upload, true)
382                    .purpose("upload the remote reviewer profile"),
383            )?;
384            for arguments in [
385                vec![
386                    engine.to_owned(),
387                    "exec".to_owned(),
388                    container_id.clone(),
389                    "rm".to_owned(),
390                    "-rf".to_owned(),
391                    "--".to_owned(),
392                    home.clone(),
393                ],
394                vec![
395                    engine.to_owned(),
396                    "exec".to_owned(),
397                    container_id.clone(),
398                    "mkdir".to_owned(),
399                    "-p".to_owned(),
400                    home.clone(),
401                ],
402                vec![
403                    engine.to_owned(),
404                    "cp".to_owned(),
405                    format!("{upload}/."),
406                    format!("{container_id}:{home}"),
407                ],
408                std::iter::once(engine.to_owned())
409                    .chain(container_upload_ownership_args(
410                        engine,
411                        container_id,
412                        worker_root,
413                        &[&home],
414                    ))
415                    .collect(),
416                vec![
417                    engine.to_owned(),
418                    "exec".to_owned(),
419                    container_id.clone(),
420                    "chmod".to_owned(),
421                    "-R".to_owned(),
422                    "go-rwx".to_owned(),
423                    home.clone(),
424                ],
425            ] {
426                execute_checked(
427                    executor,
428                    crate::targets::ssh_command(ssh, arguments)
429                        .purpose("stage the remote reviewer profile"),
430                )?;
431            }
432            execute_checked(
433                executor,
434                crate::targets::ssh_command(ssh, ["rm", "-rf", "--", &upload])
435                    .purpose("remove remote reviewer staging"),
436            )?;
437        }
438    }
439    if home.trim().is_empty() {
440        bail!("the reviewer profile home resolved to an empty path");
441    }
442    Ok(())
443}
444
445#[cfg(test)]
446mod tests {
447    use std::cell::RefCell;
448    use std::collections::BTreeMap;
449
450    use super::*;
451    use crate::controller::test_support::checkpoint_test_session;
452    use mj_core::config::{Config, HarnessKind, HarnessProfile, TargetTemplate};
453    use mj_core::state::{SessionState, State};
454
455    use crate::targets::CommandOutput;
456
457    struct RecordingExecutor {
458        commands: RefCell<Vec<CommandSpec>>,
459    }
460
461    impl RecordingExecutor {
462        fn new() -> Self {
463            Self {
464                commands: RefCell::new(Vec::new()),
465            }
466        }
467
468        /// Every command as one line, for order-sensitive assertions.
469        fn script(&self) -> Vec<String> {
470            self.commands
471                .borrow()
472                .iter()
473                .map(|command| format!("{} {}", command.program, command.args.join(" ")))
474                .collect()
475        }
476    }
477
478    impl CommandExecutor for RecordingExecutor {
479        fn execute(&self, command: &CommandSpec) -> Result<CommandOutput> {
480            self.commands.borrow_mut().push(command.clone());
481            Ok(CommandOutput {
482                status: 0,
483                stdout: Vec::new(),
484                stderr: Vec::new(),
485            })
486        }
487    }
488
489    /// A controller with one running session and two configured profiles: the
490    /// session's own and a second one to review with.
491    const SESSION_ID: &str = "0123456789abcdef0123456789abcdef";
492
493    fn fixture(directory: &Path, locator: mj_core::state::TargetLocator) -> (Controller, String) {
494        let session_id = SESSION_ID;
495        let mut session = checkpoint_test_session(session_id);
496        session.target_template_id = "local".into();
497        session.state = SessionState::Running;
498        let template = match &locator {
499            mj_core::state::TargetLocator::LocalPodman { .. } => {
500                serde_json::from_str(r#"{"kind":"local-podman","image":"test"}"#).unwrap()
501            }
502            mj_core::state::TargetLocator::LocalDocker { .. } => {
503                serde_json::from_str(r#"{"kind":"local-docker","image":"test"}"#).unwrap()
504            }
505            mj_core::state::TargetLocator::SshPodman { .. } => serde_json::from_str(
506                r#"{"kind":"ssh-podman","host":"builder.test","image":"test"}"#,
507            )
508            .unwrap(),
509            _ => TargetTemplate::LocalBare,
510        };
511        session.target = Some(locator);
512        let mut config = Config::default();
513        config.targets.insert("local".into(), template);
514        for (id, kind) in [
515            ("codex", HarnessKind::Codex),
516            ("claude", HarnessKind::Claude),
517        ] {
518            let home = directory.join(id);
519            std::fs::create_dir_all(&home).unwrap();
520            config.profiles.insert(
521                id.to_owned(),
522                HarnessProfile {
523                    enabled: true,
524                    kind,
525                    home,
526                    environment: BTreeMap::from([("EXTRA".into(), "1".into())]).into(),
527                    context_window_bytes: None,
528                    subagents: Default::default(),
529                    guardian_review_model: None,
530                },
531            );
532        }
533        (
534            Controller {
535                config,
536                state: State {
537                    sessions: [(session_id.into(), session)].into_iter().collect(),
538                    ..State::default()
539                },
540            },
541            session_id.to_owned(),
542        )
543    }
544
545    #[test]
546    fn staging_copies_the_chosen_profile_into_the_worker_root() {
547        let directory = tempfile::tempdir().unwrap();
548        let worker_root = directory.path().join(SESSION_ID);
549        std::fs::create_dir_all(directory.path().join("claude")).unwrap();
550        // A file the allowlist copies, so the stage has something to move.
551        std::fs::write(directory.path().join("claude/CLAUDE.md"), b"reviewer").unwrap();
552        let (controller, session_id) = fixture(
553            directory.path(),
554            mj_core::state::TargetLocator::LocalBare {
555                worker_root: worker_root.clone(),
556            },
557        );
558        let executor = RecordingExecutor::new();
559
560        let config = controller
561            .stage_reviewer_profile_controlled(&session_id, "claude", 0, &[], &executor)
562            .unwrap();
563
564        assert_eq!(config.profile_id, "claude");
565        assert_eq!(config.harness, HarnessKind::Claude);
566        assert_eq!(config.generation, 0);
567        assert_eq!(config.model, None);
568        assert_eq!(config.effort, None);
569        // The worker owns the harness home, so the controller never sends one.
570        assert!(
571            !config
572                .environment
573                .contains_key(HarnessKind::Claude.home_env())
574        );
575        assert_eq!(
576            config.environment.get("EXTRA").map(String::as_str),
577            Some("1")
578        );
579
580        let home = format!("{}/reviewer/profile", worker_root.display());
581        let script = executor.script();
582        let cleared = script
583            .iter()
584            .position(|line| line.starts_with("rm ") && line.contains(&home))
585            .expect("the previous reviewer profile is cleared");
586        let copied = script
587            .iter()
588            .position(|line| line.starts_with("cp ") && line.ends_with(&home))
589            .expect("the staged profile is installed");
590        assert!(
591            cleared < copied,
592            "a stale profile must go before the new one lands: {script:?}"
593        );
594        assert!(
595            script
596                .iter()
597                .any(|line| line.contains("go-rwx") && line.contains(&home)),
598            "the reviewer profile must not be world readable: {script:?}"
599        );
600    }
601
602    // Hard-won: 251e812e42b5: SSH container uploads targeted a path that existed only inside the container.
603    #[test]
604    fn remote_container_targets_stage_the_reviewer_on_the_host_not_in_the_worker_root() {
605        // The worker root is a path inside the container. Uploading to it over
606        // scp failed with "No such file" on every remote podman session, so no
607        // turn review could start there (2026-10-02).
608        let directory = tempfile::tempdir().unwrap();
609        let container_id = crate::targets::resource_name(SESSION_ID).unwrap();
610        let (controller, session_id) = fixture(
611            directory.path(),
612            mj_core::state::TargetLocator::SshPodman {
613                host: "builder.test".into(),
614                container_id: container_id.clone(),
615                workspace_storage: Default::default(),
616                borrowed_from: None,
617            },
618        );
619        let executor = RecordingExecutor::new();
620
621        controller
622            .stage_reviewer_profile_controlled(&session_id, "codex", 3, &[], &executor)
623            .unwrap();
624
625        let script = executor.script();
626        let upload = script
627            .iter()
628            .find(|line| line.starts_with("scp "))
629            .expect("the profile is uploaded with scp")
630            .clone();
631        let staging = format!(
632            "{}/{session_id}-reviewer-3",
633            crate::targets::REMOTE_UPLOAD_STAGING
634        );
635        assert!(
636            upload.contains(&staging),
637            "the upload lands in host staging: {upload}"
638        );
639        assert!(
640            !upload.contains("/var/lib/hel/workers/"),
641            "the upload does not target the container's worker root: {upload}"
642        );
643        let copy = script
644            .iter()
645            .find(|line| line.contains("cp") && line.contains(&format!("{container_id}:")))
646            .unwrap_or_else(|| {
647                panic!("the engine copies the staged profile into the container: {script:?}")
648            });
649        assert!(
650            copy.contains(&format!("{staging}/.")) && copy.contains(&format!("{container_id}:")),
651            "the engine copies from host staging into the container: {copy}"
652        );
653        let last = script.last().unwrap();
654        assert!(
655            last.contains("'rm'")
656                && last.contains(&format!("'{staging}'"))
657                && !last.contains("podman"),
658            "host staging is removed afterwards: {script:?}"
659        );
660    }
661
662    #[test]
663
664    fn local_container_targets_stage_the_reviewer_through_their_engine() {
665        let directory = tempfile::tempdir().unwrap();
666        let container_id = crate::targets::resource_name(SESSION_ID).unwrap();
667        for (locator, engine) in [
668            (
669                mj_core::state::TargetLocator::LocalPodman {
670                    borrowed_from: None,
671                    container_id: container_id.clone(),
672                    workspace_storage: Default::default(),
673                },
674                "podman",
675            ),
676            (
677                mj_core::state::TargetLocator::LocalDocker {
678                    borrowed_from: None,
679                    container_id: container_id.clone(),
680                },
681                "docker",
682            ),
683        ] {
684            let (controller, session_id) = fixture(directory.path(), locator);
685            let executor = RecordingExecutor::new();
686
687            controller
688                .stage_reviewer_profile_controlled(&session_id, "codex", 3, &[], &executor)
689                .unwrap();
690
691            let script = executor.script();
692            assert!(
693                script
694                    .iter()
695                    .all(|line| line.starts_with(&format!("{engine} "))),
696                "a container target is reached only through its engine: {script:?}"
697            );
698            let home = script
699                .iter()
700                .find_map(|line| {
701                    line.split(' ')
702                        .find(|word| word.contains("/reviewer/profile"))
703                })
704                .expect("the reviewer profile is placed")
705                .to_owned();
706            assert!(
707                home.contains(&format!("/{session_id}")),
708                "the reviewer lives under this session's worker root: {home}"
709            );
710            // Nothing here provisions a target, a checkout, or another session.
711            assert!(
712                !script.iter().any(|line| {
713                    line.contains("run") || line.contains("git") || line.contains("create")
714                }),
715                "staging a reviewer provisions nothing: {script:?}"
716            );
717        }
718    }
719
720    #[test]
721    fn a_claude_reviewer_in_a_podman_session_is_told_it_is_sandboxed() {
722        let directory = tempfile::tempdir().unwrap();
723        let container_id = crate::targets::resource_name(SESSION_ID).unwrap();
724        let podman = mj_core::state::TargetLocator::LocalPodman {
725            borrowed_from: None,
726            container_id: container_id.clone(),
727            workspace_storage: Default::default(),
728        };
729        let docker = mj_core::state::TargetLocator::LocalDocker {
730            borrowed_from: None,
731            container_id,
732        };
733        for (locator, profile_id, sandboxed) in [
734            (podman.clone(), "claude", true),
735            (podman, "codex", false),
736            (docker, "claude", false),
737        ] {
738            let (controller, session_id) = fixture(directory.path(), locator);
739            let config = controller
740                .stage_reviewer_profile_controlled(
741                    &session_id,
742                    profile_id,
743                    0,
744                    &[],
745                    &RecordingExecutor::new(),
746                )
747                .unwrap();
748            assert_eq!(
749                config.environment.get("IS_SANDBOX").map(String::as_str),
750                sandboxed.then_some("1"),
751                "{profile_id}"
752            );
753        }
754    }
755
756    #[test]
757    fn a_muse_reviewer_is_staged_with_the_permission_profile_its_policy_enforces() {
758        let directory = tempfile::tempdir().unwrap();
759        let worker_root = directory.path().join(SESSION_ID);
760        let (mut controller, session_id) = fixture(
761            directory.path(),
762            mj_core::state::TargetLocator::LocalBare {
763                worker_root: worker_root.clone(),
764            },
765        );
766        // Muse reads its permission profile from this file, and nothing on the
767        // ACP wire overrides it.
768        let home = directory.path().join("muse");
769        std::fs::create_dir_all(&home).unwrap();
770        std::fs::write(
771            home.join("settings.json"),
772            br#"{"schema_version":1,"permissions":{"schema_version":1,"default_profile":":auto-review"}}"#,
773        )
774        .unwrap();
775        controller.config.profiles.insert(
776            "muse".into(),
777            HarnessProfile {
778                enabled: true,
779                kind: HarnessKind::Muse,
780                home,
781                environment: BTreeMap::new().into(),
782                context_window_bytes: None,
783                subagents: Default::default(),
784                guardian_review_model: None,
785            },
786        );
787        controller
788            .stage_reviewer_profile_controlled(&session_id, "muse", 0, &[], &ProcessExecutor)
789            .unwrap();
790
791        let staged: serde_json::Value = serde_json::from_slice(
792            &std::fs::read(worker_root.join("reviewer/profile/settings.json")).unwrap(),
793        )
794        .unwrap();
795        // The fixture's bare target keeps configured approvals.
796        assert_eq!(staged["permissions"]["default_profile"], ":ask-me");
797    }
798
799    #[test]
800    fn a_new_generation_travels_to_the_worker_so_it_starts_a_fresh_reviewer() {
801        let directory = tempfile::tempdir().unwrap();
802        let (controller, session_id) = fixture(
803            directory.path(),
804            mj_core::state::TargetLocator::LocalBare {
805                worker_root: directory.path().join(SESSION_ID),
806            },
807        );
808        let executor = RecordingExecutor::new();
809
810        let first = controller
811            .stage_reviewer_profile_controlled(&session_id, "codex", 0, &[], &executor)
812            .unwrap();
813        let second = controller
814            .stage_reviewer_profile_controlled(&session_id, "codex", 1, &[], &executor)
815            .unwrap();
816
817        assert!(first.reusable_for(&first));
818        assert!(
819            !first.reusable_for(&second),
820            "a new generation must not reload the old conversation"
821        );
822    }
823}