Skip to content

Commit 01d2ca3

Browse files
fix: leave workspace trust to provider commands
agent-session-id: 591448c4-5967-4b12-ae43-1c6320dc1d25 agent-tool: Codex CLI agent-tool-version: 0.145.0 agent-model: unknown agent-runtime-profile: /nix/store/v2g542rika3abxn98x5yimz431j5br13-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/3aq75z8arl44fsvn49i2k4yqlzb1kcwb-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
1 parent 14fe7dc commit 01d2ca3

5 files changed

Lines changed: 115 additions & 137 deletions

File tree

docs/vrs/spec.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ validate ──► materialize ──► host-local st2 scheduler/reconciler
4444
never wake reconciliation; only create, modify, rename, or remove events may
4545
wake it before the bounded timer.
4646
- **R06:** st2 passes the complete effective task definition to the underlying
47-
launcher so manual and supervised restarts are equivalent.
47+
launcher so manual and supervised restarts are equivalent. Harness readiness
48+
that depends on a dynamically selected account belongs to that declared
49+
command. In particular, reconciliation never mutates an ambient Codex config
50+
before launch: the command may select an account-specific `CODEX_HOME` only
51+
after st2 starts it. `st2 pretrust` remains an explicit operator utility for
52+
commands that intentionally use the ambient Claude and Codex configs.
4853
- **R07:** Hook bundles are explicit, content-addressed, installed separately,
4954
and verified before materialization references them. Their receipts use the
5055
same resolved build identity as the binary's version surfaces for both

src/main.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,9 @@ enum Command {
169169
#[arg(conflicts_with = "catalog_path")]
170170
root: Option<PathBuf>,
171171
},
172-
/// Pre-trust agent workspaces in the claude config (`$CLAUDE_CONFIG_DIR/.claude.json` else
173-
/// `~/.claude.json`) BEFORE they boot, so a kick-driven `claude` never hangs on the "Is this a
174-
/// project you trust?" dialog. One atomic batch write for all dirs closes the multi-spawn trust
175-
/// race; merges into existing entries (never clobbers). Run it before `st2 up`.
172+
/// Explicitly pre-trust workspaces in the ambient Claude and Codex configs. This is an operator
173+
/// utility for harnesses that use those ambient configs; `st2 up` never calls it automatically.
174+
/// Account-selecting commands should instead declare trust in the selected harness invocation.
176175
Pretrust {
177176
/// Workspace directories to mark trusted.
178177
#[arg(required = true)]
@@ -934,7 +933,7 @@ fn env_cmd(root: &Path) -> Result<()> {
934933
fn pretrust_cmd(dirs: &[PathBuf]) -> Result<()> {
935934
let n = st2::pretrust::pretrust(dirs)?;
936935
println!(
937-
"pre-trusted {n} workspace{} in the claude config",
936+
"pre-trusted {n} workspace{} in the ambient Claude and Codex configs",
938937
if n == 1 { "" } else { "s" }
939938
);
940939
Ok(())

src/pretrust.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ pub fn pretrust(dirs: &[PathBuf]) -> Result<usize> {
4141
Ok(n)
4242
}
4343

44-
/// Pre-trust workspaces for Codex only. Reconciliation uses this immediately before launching a
45-
/// missing Codex agent task, so a synced declaration cannot park on the interactive workspace-trust
46-
/// prompt. Keeping this Codex-specific avoids touching Claude configuration during a Codex rollout.
44+
/// Pre-trust workspaces for Codex only in the caller's ambient config. This remains available to
45+
/// explicit tooling, but reconciliation does not call it: a provider command may select an
46+
/// account-specific `CODEX_HOME` only after st2 launches it.
4747
pub fn pretrust_codex(dirs: &[PathBuf]) -> Result<usize> {
4848
pretrust_codex_at(&codex_config_path()?, dirs)
4949
}

src/run.rs

Lines changed: 31 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -804,52 +804,38 @@ fn reconcile_pass(
804804
debounce.observe(&sessions, now);
805805
let mut plan = crate::reconcile(&eligible_specs, &sessions, this_host);
806806
report.deferred = debounce.defer_flickers(&mut plan, now);
807-
gate_codex_launches(
808-
root,
809-
&mut plan,
810-
&mut report,
811-
|| match &hook_error {
812-
Some(error) => anyhow::bail!("{error}"),
813-
None => Ok(()),
814-
},
815-
crate::pretrust::pretrust_codex,
816-
);
807+
gate_codex_launches_on_hooks(&mut plan, &mut report, || match &hook_error {
808+
Some(error) => anyhow::bail!("{error}"),
809+
None => Ok(()),
810+
});
817811
execute(&plan, runner, cap, &mut report);
818812
report
819813
}
820814

821-
/// A missing Codex agent must be trusted before its pty exists. Codex's bypass flags do not bypass
822-
/// the workspace-trust prompt; without this gate a remotely synced declaration appears launched but
823-
/// is really parked waiting for a human keystroke. Batch every workspace into one atomic config
824-
/// update, and fail closed: if trust cannot be established, suppress the affected agent launches
825-
/// (including their sidecars) and surface the error. Already-live/adopted agents never enter this
826-
/// path.
827-
fn gate_codex_launches<'a, V, F>(
828-
catalog_root: &Path,
815+
/// A missing Codex agent must not launch against stale lifecycle hooks. Suppress the affected agent
816+
/// launches (including their sidecars) and surface the error when hook verification fails.
817+
///
818+
/// Workspace trust belongs to the declared provider command and its selected account-specific
819+
/// runtime. Reconciliation deliberately does not mutate an ambient Codex config: an account selector
820+
/// may choose `CODEX_HOME` only after this process launches the command, so such a write would target
821+
/// the wrong state and could not satisfy the launched seat's trust gate.
822+
fn gate_codex_launches_on_hooks<'a, V>(
829823
plan: &mut ReconcilePlan<'a>,
830824
report: &mut UpReport,
831825
verify_hooks: V,
832-
pretrust: F,
833826
) where
834827
V: FnOnce() -> anyhow::Result<()>,
835-
F: FnOnce(&[PathBuf]) -> anyhow::Result<usize>,
836828
{
837-
let mut workspaces = Vec::new();
838829
let mut gated_agents = Vec::new();
839830
for launch in &plan.launch {
840-
let Some(agent) = launch.tasks.iter().find(|target| {
831+
let Some(_) = launch.tasks.iter().find(|target| {
841832
target.name == "agent" && crate::hooks::command_invokes_codex(&target.command)
842833
}) else {
843834
continue;
844835
};
845-
let spec_dir = launch.spec.path.parent().unwrap_or_else(|| Path::new("."));
846-
let workspace = resolve_task_cwd(agent, spec_dir, catalog_root);
847-
if !workspaces.contains(&workspace) {
848-
workspaces.push(workspace);
849-
}
850836
gated_agents.push(launch.spec.identity.clone());
851837
}
852-
if workspaces.is_empty() {
838+
if gated_agents.is_empty() {
853839
return;
854840
}
855841

@@ -860,16 +846,6 @@ fn gate_codex_launches<'a, V, F>(
860846
"verify lifecycle hooks for new Codex agent(s) {}: {error}; launch suppressed",
861847
gated_agents.join(", ")
862848
));
863-
return;
864-
}
865-
866-
if let Err(error) = pretrust(&workspaces) {
867-
plan.launch
868-
.retain(|launch| !gated_agents.contains(&launch.spec.identity));
869-
report.errors.push(format!(
870-
"pretrust Codex workspace(s) for {}: {error}; launch suppressed",
871-
gated_agents.join(", ")
872-
));
873849
}
874850
}
875851

@@ -1421,7 +1397,7 @@ mod tests {
14211397
}
14221398

14231399
#[test]
1424-
fn codex_pretrust_batches_every_new_workspace_once() {
1400+
fn codex_hook_gate_accepts_new_agents_without_mutating_the_launch_plan() {
14251401
let mut left = spec_fixture();
14261402
left.identity = "left".into();
14271403
left.path = PathBuf::from("/catalog/node/left/agent.kdl");
@@ -1441,96 +1417,29 @@ mod tests {
14411417
spec: &right,
14421418
tasks: vec![right_agent],
14431419
});
1444-
let captured = RefCell::new(Vec::new());
1445-
1446-
gate_codex_launches(
1447-
Path::new("/catalog"),
1448-
&mut plan,
1449-
&mut UpReport::default(),
1450-
|| Ok(()),
1451-
|workspaces| {
1452-
captured.borrow_mut().extend_from_slice(workspaces);
1453-
Ok(workspaces.len())
1454-
},
1455-
);
1456-
1457-
assert_eq!(
1458-
captured.into_inner(),
1459-
[PathBuf::from("/workspaces/shared")],
1460-
"all affected workspaces are passed in one deduplicated batch"
1461-
);
1462-
assert_eq!(plan.launch.len(), 2);
1463-
}
1464-
1465-
#[test]
1466-
fn codex_pretrust_failure_suppresses_every_affected_agent_and_sidecar() {
1467-
let mut left = spec_fixture();
1468-
left.identity = "left".into();
1469-
left.path = PathBuf::from("/catalog/node/left/agent.kdl");
1470-
let mut right = spec_fixture();
1471-
right.identity = "right".into();
1472-
right.path = PathBuf::from("/catalog/node/right/agent.kdl");
1473-
let mut other = spec_fixture();
1474-
other.identity = "other".into();
1475-
other.path = PathBuf::from("/catalog/node/other/agent.kdl");
1476-
1477-
let mut left_agent = target("node.left.agent", "exec codex");
1478-
left_agent.workspace = Some("/workspaces/left".into());
1479-
let mut left_ding = target("node.left.ding", "st2 ding");
1480-
left_ding.name = "ding".into();
1481-
let mut right_agent = target("node.right.agent", "/opt/bin/codex --model gpt-5");
1482-
right_agent.workspace = Some("/workspaces/right".into());
1483-
let mut right_ding = target("node.right.ding", "st2 ding");
1484-
right_ding.name = "ding".into();
1485-
let non_codex = target("node.other.agent", "exec claude");
1486-
let mut plan = ReconcilePlan::default();
1487-
plan.launch.push(Launch {
1488-
spec: &left,
1489-
tasks: vec![left_agent, left_ding],
1490-
});
1491-
plan.launch.push(Launch {
1492-
spec: &right,
1493-
tasks: vec![right_agent, right_ding],
1494-
});
1495-
plan.launch.push(Launch {
1496-
spec: &other,
1497-
tasks: vec![non_codex],
1498-
});
1420+
let expected = plan
1421+
.launch
1422+
.iter()
1423+
.map(|launch| launch.spec.identity.clone())
1424+
.collect::<Vec<_>>();
14991425
let mut report = UpReport::default();
15001426

1501-
gate_codex_launches(
1502-
Path::new("/catalog"),
1503-
&mut plan,
1504-
&mut report,
1505-
|| Ok(()),
1506-
|workspaces| {
1507-
assert_eq!(
1508-
workspaces,
1509-
[
1510-
PathBuf::from("/workspaces/left"),
1511-
PathBuf::from("/workspaces/right")
1512-
]
1513-
);
1514-
anyhow::bail!("read-only Codex config")
1515-
},
1516-
);
1427+
gate_codex_launches_on_hooks(&mut plan, &mut report, || Ok(()));
15171428

15181429
assert_eq!(
15191430
plan.launch
15201431
.iter()
1521-
.map(|launch| launch.spec.identity.as_str())
1432+
.map(|launch| launch.spec.identity.clone())
15221433
.collect::<Vec<_>>(),
1523-
["other"],
1524-
"both Codex agents and all their sidecars fail closed"
1434+
expected,
1435+
"successful hook verification must leave the launch plan unchanged"
15251436
);
1526-
assert_eq!(plan.launch[0].tasks[0].pty_id, "node.other.agent");
1527-
assert_eq!(report.errors.len(), 1);
1528-
assert!(report.errors[0].contains("left, right"));
1529-
assert!(report.errors[0].contains("launch suppressed"));
1437+
assert_eq!(plan.launch.len(), 2);
1438+
assert!(report.errors.is_empty());
15301439
}
15311440

15321441
#[test]
1533-
fn codex_pretrust_does_not_touch_adopted_agents_or_sidecar_only_repairs() {
1442+
fn codex_hook_gate_does_not_touch_adopted_agents_or_sidecar_only_repairs() {
15341443
let mut spec = spec_fixture();
15351444
spec.identity = "root".into();
15361445
let mut ding = target("node.root.ding", "st2 ding");
@@ -1543,12 +1452,10 @@ mod tests {
15431452
});
15441453
let mut report = UpReport::default();
15451454

1546-
gate_codex_launches(
1547-
Path::new("/catalog"),
1455+
gate_codex_launches_on_hooks(
15481456
&mut plan,
15491457
&mut report,
15501458
|| panic!("an already-live Codex agent must not enter the hook gate"),
1551-
|_| panic!("an already-live Codex agent must not enter the pretrust gate"),
15521459
);
15531460

15541461
assert_eq!(plan.adopt, [&spec]);
@@ -1579,13 +1486,9 @@ mod tests {
15791486
});
15801487
let mut report = UpReport::default();
15811488

1582-
gate_codex_launches(
1583-
Path::new("/catalog"),
1584-
&mut plan,
1585-
&mut report,
1586-
|| anyhow::bail!("stale receipt"),
1587-
|_| panic!("pretrust must not run after hook verification fails"),
1588-
);
1489+
gate_codex_launches_on_hooks(&mut plan, &mut report, || {
1490+
anyhow::bail!("stale receipt")
1491+
});
15891492

15901493
assert_eq!(
15911494
plan.launch

tests/hooks.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,77 @@ fn up_once_suppresses_a_new_codex_agent_without_mutating_hooks() {
417417
);
418418
}
419419

420+
#[test]
421+
fn up_once_never_mutates_the_ambient_codex_config_before_account_selection() {
422+
let tmp = tempfile::tempdir().unwrap();
423+
let catalog = tmp.path().join("catalog");
424+
let workspace = tmp.path().join("workspace");
425+
let hooks_root = tmp.path().join("hooks");
426+
let ambient_codex_home = tmp.path().join("ambient-codex-home");
427+
let selected_codex_home = tmp.path().join("selected-codex-home");
428+
let bin = tmp.path().join("bin");
429+
let pty_log = tmp.path().join("pty.log");
430+
let declaration = catalog.join("agents/h/worker/agent.kdl");
431+
fs::create_dir_all(declaration.parent().unwrap()).unwrap();
432+
fs::create_dir_all(&workspace).unwrap();
433+
fs::create_dir_all(&bin).unwrap();
434+
fs::write(
435+
&declaration,
436+
format!(
437+
"agent \"worker\" {{\n host \"h\"\n workspace \"{}\"\n \
438+
env {{ ST_AGENT \"h.worker\" CODEX_HOME \"{}\" }}\n \
439+
command \"exec codex -c \
440+
'projects={{\\\"{}\\\"={{trust_level=\\\"trusted\\\"}}}}'\"\n}}\n",
441+
workspace.display(),
442+
selected_codex_home.display(),
443+
workspace.display()
444+
),
445+
)
446+
.unwrap();
447+
write_executable(
448+
&bin.join("pty"),
449+
&format!(
450+
"#!/bin/sh\nprintf '%s\\n' \"$*\" >> '{}'\nif [ \"$1\" = list ]; then printf '[]\\n'; fi\n",
451+
pty_log.display()
452+
),
453+
);
454+
assert!(
455+
command(&hooks_root)
456+
.args(["hooks", "install"])
457+
.status()
458+
.unwrap()
459+
.success()
460+
);
461+
462+
let output = command(&hooks_root)
463+
.arg("up")
464+
.arg(&catalog)
465+
.args(["--host", "h", "--once"])
466+
.env("PATH", &bin)
467+
.env("CODEX_HOME", &ambient_codex_home)
468+
.output()
469+
.unwrap();
470+
let report = format!(
471+
"{}{}",
472+
String::from_utf8_lossy(&output.stdout),
473+
String::from_utf8_lossy(&output.stderr)
474+
);
475+
476+
assert!(output.status.success(), "{report}");
477+
assert!(report.contains("launched (1): h.worker"), "{report}");
478+
assert!(
479+
fs::read_to_string(&pty_log)
480+
.unwrap_or_default()
481+
.lines()
482+
.any(|line| line.starts_with("run ")),
483+
"the declared account-selecting Codex command must launch"
484+
);
485+
assert!(
486+
!ambient_codex_home.exists(),
487+
"st2 up must not create or mutate an ambient Codex config before the command selects its account"
488+
);
489+
}
490+
420491
#[test]
421492
fn missing_hooks_do_not_rewrite_or_stop_an_already_live_codex_agent() {
422493
let tmp = tempfile::tempdir().unwrap();

0 commit comments

Comments
 (0)