From 930c81fd2139b46fcb6a00d5c29f0844c8bad565 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Sat, 24 Oct 2026 23:19:25 +0900 Subject: [PATCH 1/3] uphold hook claude-code holds the end of a turn to the [stop] gate, blocking only on red the session caused `[stop] run` in policy/principles.toml names the gate as argv arrays, run from the repository root with no shell. The same `uphold hook claude-code` entry, configured for SessionStart and Stop, dispatches on hook_event_name; every other event is judged as a pending tool call, as before. SessionStart (async) runs the gate once and records which commands failed under the worktree's git dir, keyed by a session_id held to a closed character set. Stop runs it again and blocks with {"decision":"block"} only on a command that was green at session start, carrying the last 40 lines of its output. Red the session found is named in systemMessage. With no baseline every red command blocks and says why; a baseline still being written is waited for, up to 300 seconds. A stop that follows a block (stop_hook_active) is not blocked again: a still-red gate goes to the person in systemMessage. In a real Claude Code session the first version, which left looping to the harness's cap, spent nine blocks on an agent that had already said it could not fix the gate; this one ends that turn after one. [stop] in an inherited file, and an entry naming no program, are refused at load. The JSON Schema is regenerated. Part of #421. Claude-Session: https://claude.ai/code/session_017EkhA3KMqw95DzAEYPkDn7 --- README.md | 7 +- docs/REFERENCE.md | 56 ++++++ docs/schema/principles.schema.json | 22 +++ src/config.rs | 54 +++++ src/hook.rs | 32 +++ src/hook/stop.rs | 305 +++++++++++++++++++++++++++++ src/main.rs | 2 +- tests/hook_cli.rs | 235 ++++++++++++++++++++++ 8 files changed, 711 insertions(+), 2 deletions(-) create mode 100644 src/hook/stop.rs diff --git a/README.md b/README.md index b7dcb9a..dd33a36 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ uphold guard --stage pre-push # the guards for that Git hook uphold shim gh pr create ... # stand in front of a command, then exec uphold shim --install # link this binary under each command's name uphold shim --status # what is linked, and whether PATH reaches it -uphold hook claude-code # judge a pending agent tool call, read on stdin +uphold hook claude-code # judge a pending tool call or a turn's end, read on stdin uphold hook codex # the same checks through Codex's PreToolUse hook uphold audit --for-publication # before flipping private -> public uphold supply-chain # scanners and the git-pin check over the pushed range; a missing one is exit 2 @@ -348,6 +348,11 @@ publish your text; the example covers the GitHub MCP server. See [Codex hooks](https://learn.chatgpt.com/docs/hooks) and the [configuration reference](docs/REFERENCE.md#configuring-it) for inline TOML. +The same command, configured for `SessionStart` and `Stop`, keeps an agent from +ending its turn with the gate red: `[stop] run` names the commands, and a turn +is blocked only on one that was green when the session started. See +[the end of a turn](docs/REFERENCE.md#the-end-of-a-turn-stop). + **`uphold hooks --identity DIR...`** and **`uphold probe`** answer two questions a single repository cannot answer about itself. A forked hook declaration is valid in every tree that holds it, so only a comparison across repositories diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 440a574..82ff2e1 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -3247,6 +3247,62 @@ report goes, and which exit code that harness reads as blocked. A name the table does not carry is refused and the known ones are listed, because none of those five values is derivable from a name. +### The end of a turn: `[stop]` + +An agent can finish, say it is done, and leave a tree the repository's own gate +refuses; the refusal then arrives at the next commit, often in another session. +The same `uphold hook claude-code` entry, configured for `SessionStart` and +`Stop`, holds the end of a turn to the commands a repository names: + +```toml +# policy/principles.toml +[stop] +run = [["uphold", "check"], ["cargo", "test", "--locked", "--quiet"]] +``` + +Each entry is one command as argv, run from the repository root with no shell. +A command that needs a pipeline names a script holding it. An entry with no +program is refused at load, and so is `[stop]` in a file read through +`[inherit]`: what a tree has to pass is that repository's own declaration. + +```jsonc +// .claude/settings.json +{"hooks": { + "SessionStart": [{"hooks": [{"type": "command", "command": "uphold hook claude-code", "async": true}]}], + "Stop": [{"hooks": [{"type": "command", "command": "uphold hook claude-code", "timeout": 900}]}] +}} +``` + +`hook_event_name` says which arrived. Every other event, and an event naming +none, is judged as a pending tool call, as above. + +- **`SessionStart`** runs every command once and records which failed, under + the worktree's git dir (`.git/uphold/stop/.json`). It prints + nothing on stdout, because Claude Code adds that to the conversation, and + `async` keeps the session from waiting for it. A `session_id` outside + `A-Z a-z 0-9 - _`, or longer than 128, is refused rather than used as a + file name. +- **`Stop`** runs the commands again. One that fails now and passed at session + start blocks the turn with `{"decision": "block", "reason": ...}`, where the + reason names the command and the last 40 lines of its output. One that was + already failing does not block, and is named in `systemMessage` for the + person. With no baseline, because `SessionStart` is not configured or the + id was unusable, every failing command blocks and the reason says so. A + baseline still being written is waited for, up to 300 seconds, so the + `timeout` above covers that wait plus one run of the gate. + +A turn's end is blocked once. When `stop_hook_active` says this stop follows +a block, a gate still red is named in `systemMessage` and the turn ends: the +reason told the agent to fix it or say it cannot, and blocking an agent that +said so would spend Claude Code's own cap (eight consecutive continuations, +reset by a tool call) on a turn that is over. A command that cannot be started +is exit `2`; at `Stop` +that sends stderr to the agent as the reason, so a broken gate fails closed. +With no policy, or no `[stop]`, nothing runs, and stderr says no gate was +checked rather than that one passed. `--junit` is not written for these two +events. The gate judges the session's working directory, so work done in a +separate `git worktree` is checked only if the session runs there. + ## `uphold audit --for-publication` Every guard across every seam conditions on *is the target public **now***. So diff --git a/docs/schema/principles.schema.json b/docs/schema/principles.schema.json index 5ae47a1..725fbf7 100644 --- a/docs/schema/principles.schema.json +++ b/docs/schema/principles.schema.json @@ -480,6 +480,24 @@ ], "type": "object" }, + "Stop": { + "additionalProperties": false, + "description": "`[stop]` in a repository's own policy: the gate `uphold hook` runs when an\nagent ends its turn.\n\nargv arrays and not a shell string, so what runs is what a reader sees:\nno quoting, no expansion, no second language inside the policy file\n(`least-power`). A command that needs a pipeline names a script that holds it.", + "properties": { + "run": { + "default": [], + "description": "Each entry is one command as argv, run in order from the repository\nroot, with no shell. Every one runs, red or green, so the report names\nall that failed rather than the first.", + "items": { + "items": { + "type": "string" + }, + "type": "array" + }, + "type": "array" + } + }, + "type": "object" + }, "SupplyChain": { "additionalProperties": false, "description": "`[supply_chain]` in a repository's own policy.", @@ -893,6 +911,10 @@ }, "type": "array" }, + "stop": { + "$ref": "#/$defs/Stop", + "description": "`[stop]`: the gate an agent's turn may not end red. See [`Stop`]." + }, "supply_chain": { "$ref": "#/$defs/SupplyChain", "description": "`[supply_chain]`: what this repository has said about its own\ndependency scan. See [`SupplyChain`]." diff --git a/src/config.rs b/src/config.rs index a3c1c39..63e6892 100644 --- a/src/config.rs +++ b/src/config.rs @@ -680,6 +680,39 @@ pub(crate) struct SupplyChain { pub forge_host: Option, } +/// `[stop]` in a repository's own policy: the gate `uphold hook` runs when an +/// agent ends its turn. +/// +/// argv arrays and not a shell string, so what runs is what a reader sees: +/// no quoting, no expansion, no second language inside the policy file +/// (`least-power`). A command that needs a pipeline names a script that holds it. +#[derive(Debug, Clone, Default, Deserialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub(crate) struct Stop { + /// Each entry is one command as argv, run in order from the repository + /// root, with no shell. Every one runs, red or green, so the report names + /// all that failed rather than the first. + #[serde(default)] + pub run: Vec>, +} + +impl Stop { + /// An empty argv, or one whose program is empty, cannot be spawned, and a + /// gate holding one would refuse every turn's end for a reason that is the + /// policy file's rather than the tree's. Refused at load instead. + fn validate(&self) -> std::result::Result<(), String> { + for argv in &self.run { + if argv.first().is_none_or(|program| program.trim().is_empty()) { + return Err(format!( + "`[stop] run` holds {argv:?}, which names no program. Each entry is one \ + command as argv, its program first" + )); + } + } + Ok(()) + } +} + /// The scanner a retired waiver may still name. uphold no longer runs it, so /// the row is read and ignored; naming any other scanner is an error, as it /// always was, because every scanner that is run carries its own suppression in @@ -949,6 +982,9 @@ pub(crate) struct PolicyFile { /// dependency scan. See [`SupplyChain`]. #[serde(default)] pub supply_chain: Option, + /// `[stop]`: the gate an agent's turn may not end red. See [`Stop`]. + #[serde(default)] + pub stop: Option, /// Whether every path-baseline entry must say who excused it and why. /// /// Off by default, and the default is not neutrality -- it is what every @@ -1266,6 +1302,8 @@ pub(crate) struct Policy { pub inherited_sets: Vec, /// The repository's own `[supply_chain]`, or the empty one. pub supply_chain: SupplyChain, + /// The repository's own `[stop]`, or the empty one. + pub stop: Stop, } impl Policy { @@ -1544,6 +1582,17 @@ fn refuse_inherited_override(path: &Path, file: &PolicyFile, kind: &str) -> Resu ), )); } + // The gate is the inheriting repository's own: what its tree has to pass + // is not a fact a bundled set can know. + if file.stop.is_some() { + return Err(Fatal::at( + path, + format!( + "`[stop]` names the commands this repository's tree has to pass, and {kind} \ + has no tree. Move it to the policy of the repository it gates" + ), + )); + } if let Some(id) = file.overrides.keys().next() { return Err(Fatal::at( path, @@ -2076,6 +2125,10 @@ pub(crate) fn load_text(root: &Path, policy_path: &Path, text: &str) -> Result

Result

>, + /// The ones that failed. + red: Vec>, +} + +struct Outcome { + argv: Vec, + code: Option, + output: String, +} + +impl Outcome { + fn passed(&self) -> bool { + self.code == Some(0) + } + + fn shown(&self) -> String { + self.argv.join(" ") + } +} + +/// The harness's session id, if it is one this hook may turn into a path. +fn session_id(event: &Value) -> std::result::Result<&str, String> { + let id = event + .pointer("/session_id") + .and_then(Value::as_str) + .ok_or_else(|| "the event carries no session_id".to_owned())?; + let safe = !id.is_empty() + && id.len() <= 128 + && id + .chars() + .all(|character| character.is_ascii_alphanumeric() || matches!(character, '-' | '_')); + if safe { + Ok(id) + } else { + Err(format!( + "the event's session_id {id:?} is not 1 to 128 of A-Z, a-z, 0-9, '-' and '_', so it \ + is not used as a file name" + )) + } +} + +fn baseline_dir(root: &Path) -> Result { + Ok(git::dir(root)?.join("uphold").join("stop")) +} + +fn run_gate(root: &Path, commands: &[Vec]) -> Result> { + commands + .iter() + .map(|argv| { + let (program, arguments) = argv + .split_first() + .ok_or_else(|| Fatal::new("`[stop] run` holds an empty command"))?; + let output = Command::new(program) + .args(arguments) + .current_dir(root) + .stdin(Stdio::null()) + .output() + .map_err(|error| { + Fatal::new(format!( + "`[stop] run` names `{}`, which could not be started: {error}", + argv.join(" ") + )) + })?; + let mut text = String::from_utf8_lossy(&output.stdout).into_owned(); + text.push_str(&String::from_utf8_lossy(&output.stderr)); + Ok(Outcome { + argv: argv.clone(), + code: output.status.code(), + output: text, + }) + }) + .collect() +} + +/// Record which `[stop]` commands fail as the session starts. +/// +/// Prints nothing on stdout: Claude Code adds a `SessionStart` hook's stdout to +/// the conversation, and a baseline is not something the agent should read. +pub(super) fn session_start(event: &Value, root: &Path, policy: &Policy) -> Result { + let id = session_id(event).map_err(Fatal::new)?; + if policy.stop.run.is_empty() { + return Ok(Exit::Clean); + } + let directory = baseline_dir(root)?; + std::fs::create_dir_all(&directory)?; + let pending = directory.join(format!("{id}.pending")); + std::fs::write(&pending, b"")?; + let outcomes = run_gate(root, &policy.stop.run); + let written = outcomes.and_then(|outcomes| { + let baseline = Baseline { + ran: outcomes + .iter() + .map(|outcome| outcome.argv.clone()) + .collect(), + red: outcomes + .iter() + .filter(|outcome| !outcome.passed()) + .map(|outcome| outcome.argv.clone()) + .collect(), + }; + let document = serde_json::to_vec(&baseline) + .map_err(|error| Fatal::new(format!("the baseline is not writable: {error}")))?; + // Renamed into place, so `Stop` never reads half a file. + let partial = directory.join(format!("{id}.json.partial")); + std::fs::write(&partial, document)?; + std::fs::rename(&partial, directory.join(format!("{id}.json")))?; + Ok(()) + }); + std::fs::remove_file(&pending).ok(); + written.map(|()| Exit::Clean) +} + +/// The baseline for this session, or why there is none. +fn baseline(event: &Value, root: &Path) -> Result> { + let id = match session_id(event) { + Ok(id) => id, + Err(why) => return Ok(Err(why)), + }; + let directory = baseline_dir(root)?; + let recorded = directory.join(format!("{id}.json")); + let pending = directory.join(format!("{id}.pending")); + let started = Instant::now(); + while !recorded.is_file() && pending.is_file() && started.elapsed() < WAIT_FOR_BASELINE { + std::thread::sleep(POLL); + } + if !recorded.is_file() { + return Ok(Err(if pending.is_file() { + format!( + "the SessionStart baseline was still running after {}s", + WAIT_FOR_BASELINE.as_secs() + ) + } else { + "no SessionStart baseline was recorded for this session; configure the \ + SessionStart hook beside the Stop hook" + .to_owned() + })); + } + let text = std::fs::read(&recorded)?; + Ok(serde_json::from_slice(&text).map_err(|error| { + format!( + "the baseline at {} is unreadable ({error})", + recorded.display() + ) + })) +} + +fn tail(output: &str) -> String { + let lines: Vec<&str> = output.trim_end().lines().collect(); + let skip = lines.len().saturating_sub(TAIL_LINES); + lines.into_iter().skip(skip).collect::>().join("\n") +} + +/// Refuse the end of a turn that leaves a command red that was green when the +/// session started. +pub(super) fn stop(event: &Value, root: &Path, policy: &Policy) -> Result { + if policy.stop.run.is_empty() { + eprintln!( + "uphold hook: this policy declares no `[stop] run`, so the end of this turn was not \ + checked. That is no gate, not a passing one." + ); + return Ok(Exit::Clean); + } + let known = baseline(event, root)?; + let outcomes = run_gate(root, &policy.stop.run)?; + let (preexisting, blocking): (Vec<&Outcome>, Vec<&Outcome>) = outcomes + .iter() + .filter(|outcome| !outcome.passed()) + .partition(|outcome| { + known.as_ref().is_ok_and(|baseline| { + baseline.ran.contains(&outcome.argv) && baseline.red.contains(&outcome.argv) + }) + }); + + let mut document = Map::new(); + let mut told: Vec = Vec::new(); + let sent_back = event + .pointer("/stop_hook_active") + .and_then(Value::as_bool) + .unwrap_or(false); + if !blocking.is_empty() && sent_back { + let names: Vec = blocking + .iter() + .map(|outcome| format!("`{}`", outcome.shown())) + .collect(); + told.push(format!( + "uphold: the [stop] gate is still red after the agent was sent back once, so the turn \ + ends with it red: {}", + names.join(", ") + )); + } else if !blocking.is_empty() { + let mut reason = String::from( + "uphold: the repository's [stop] gate is red, and these commands were not failing \ + when this session started:\n\n", + ); + if let Err(why) = &known { + writeln!(reason, "(No baseline to compare against: {why}.)\n").ok(); + } + for outcome in &blocking { + let code = outcome.code.map_or_else( + || "was killed by a signal".to_owned(), + |code| format!("exited {code}"), + ); + writeln!( + reason, + "`{}` {code}:\n{}\n", + outcome.shown(), + tail(&outcome.output) + ) + .ok(); + } + reason.push_str( + "Fix what this session broke before ending the turn. If it cannot be fixed here, say \ + so plainly in the reply. This is the only block: the next stop ends the turn, and a \ + gate still red is reported to the person.", + ); + eprintln!("{reason}"); + document.insert("decision".to_owned(), Value::from("block")); + document.insert("reason".to_owned(), Value::from(reason)); + } + if !preexisting.is_empty() { + let names: Vec = preexisting + .iter() + .map(|outcome| format!("`{}`", outcome.shown())) + .collect(); + told.push(format!( + "uphold: these [stop] commands were already failing when this session started, so \ + they did not block: {}", + names.join(", ") + )); + } + if !told.is_empty() { + let message = told.join("\n"); + eprintln!("{message}"); + document.insert("systemMessage".to_owned(), Value::from(message)); + } + if !document.is_empty() { + println!("{}", Value::Object(document)); + } + Ok(Exit::Clean) +} diff --git a/src/main.rs b/src/main.rs index bde5cd2..b633c2e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -143,7 +143,7 @@ usage: versions of this binary can be diffed uphold rules --effective [--json] every rule this repository resolves to, and the git hooks each one fires at - uphold hook claude-code|codex judge a pending agent tool call, read on stdin + uphold hook claude-code|codex judge a pending tool call or a turn's end, read on stdin [--junit DIR] uphold shim [args...] check what a command would publish, then run it uphold shim --install [COMMAND...] link this binary under each command's name diff --git a/tests/hook_cli.rs b/tests/hook_cli.rs index be6cba2..327f9c6 100644 --- a/tests/hook_cli.rs +++ b/tests/hook_cli.rs @@ -1079,3 +1079,238 @@ fn the_flag_names_the_directory_on_either_side_of_the_harness() { let (suite, _) = support::junit::suite(files.first().unwrap()); assert_eq!(suite.name.as_str(), "uphold hook codex"); } + +// -- the end of a turn --------------------------------------------------------- + +/// A gate that is green while the file `red` is absent: argv, no shell, and a +/// program every test machine has. +const STOP_POLICY: &str = "[stop]\nrun = [[\"test\", \"!\", \"-e\", \"red\"]]\n"; + +/// A repository with a `[stop]` gate. A git repository, because the baseline +/// lives under the worktree's git dir. +fn gated(name: &str, policy: &str) -> PathBuf { + let root = workspace(name, Some(policy)); + support::git(&root, &["init", "-q"]); + root +} + +/// Run the hook with the fixture's own git, never the one a commit hook running +/// this suite exported. +fn turn(root: &Path, stdin: &str) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command + .args(["hook", "claude-code"]) + .current_dir(root) + .env_remove("UPHOLD_ALLOW") + .env("HOME", "/home/example-user") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + support::without_git_environment(&mut command); + let mut child = command.spawn().unwrap(); + child + .stdin + .as_mut() + .unwrap() + .write_all(stdin.as_bytes()) + .ok(); + child.wait_with_output().unwrap() +} + +fn moment(name: &str, session: &str) -> String { + serde_json::json!({ + "hook_event_name": name, + "session_id": session, + "cwd": "/unused", + "stop_hook_active": false, + }) + .to_string() +} + +fn decision(output: &Output) -> Value { + serde_json::from_str(stdout(output).trim()).unwrap() +} + +/// The point of the seam: green when the session started, red when it ends. +#[test] +fn a_command_this_session_turned_red_blocks_the_end_of_the_turn() { + let root = gated("stop-new-red", STOP_POLICY); + let started = turn(&root, &moment("SessionStart", "s-new")); + assert_eq!(code(&started), 0, "{}", stderr(&started)); + // Claude Code adds SessionStart stdout to the conversation. + assert_eq!(stdout(&started), ""); + std::fs::write(root.join("red"), "").unwrap(); + let stopped = turn(&root, &moment("Stop", "s-new")); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + let document = decision(&stopped); + assert_eq!(document.get("decision"), Some(&Value::from("block"))); + let reason = document.get("reason").and_then(Value::as_str).unwrap(); + assert!(reason.contains("`test ! -e red` exited 1"), "{reason}"); + assert!(document.get("systemMessage").is_none(), "{document}"); +} + +/// An agent sent back once that says it cannot fix the gate has followed the +/// reason. Blocking it again spends the harness's cap on a turn that is over. +#[test] +fn a_stop_that_follows_a_block_ends_the_turn_and_tells_the_person() { + let root = gated("stop-sent-back", STOP_POLICY); + turn(&root, &moment("SessionStart", "s-back")); + std::fs::write(root.join("red"), "").unwrap(); + let again = serde_json::json!({ + "hook_event_name": "Stop", + "session_id": "s-back", + "stop_hook_active": true, + }); + let stopped = turn(&root, &again.to_string()); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + let document = decision(&stopped); + assert!(document.get("decision").is_none(), "{document}"); + let message = document + .get("systemMessage") + .and_then(Value::as_str) + .unwrap(); + assert!( + message.contains("still red after the agent was sent back once"), + "{message}" + ); + assert!(message.contains("`test ! -e red`"), "{message}"); +} + +#[test] +fn a_green_gate_lets_the_turn_end_silently() { + let root = gated("stop-green", STOP_POLICY); + turn(&root, &moment("SessionStart", "s-green")); + let stopped = turn(&root, &moment("Stop", "s-green")); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + assert_eq!(stdout(&stopped), ""); +} + +/// Red the session found is named to the person, and does not hold the agent. +#[test] +fn a_command_already_red_at_session_start_is_reported_and_does_not_block() { + let root = gated("stop-old-red", STOP_POLICY); + std::fs::write(root.join("red"), "").unwrap(); + turn(&root, &moment("SessionStart", "s-old")); + let stopped = turn(&root, &moment("Stop", "s-old")); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + let document = decision(&stopped); + assert!(document.get("decision").is_none(), "{document}"); + let message = document + .get("systemMessage") + .and_then(Value::as_str) + .unwrap(); + assert!(message.contains("`test ! -e red`"), "{message}"); +} + +/// Without a baseline nothing separates found red from caused red, so it +/// blocks, and says the comparison was missing. +#[test] +fn with_no_baseline_every_red_command_blocks_and_says_why() { + let root = gated("stop-no-baseline", STOP_POLICY); + std::fs::write(root.join("red"), "").unwrap(); + let stopped = turn(&root, &moment("Stop", "s-none")); + let document = decision(&stopped); + assert_eq!(document.get("decision"), Some(&Value::from("block"))); + let reason = document.get("reason").and_then(Value::as_str).unwrap(); + assert!( + reason.contains("No baseline to compare against"), + "{reason}" + ); + assert!( + reason.contains("configure the SessionStart hook"), + "{reason}" + ); +} + +/// A first turn can end before a slow gate's first run has. Stop waits for +/// the baseline rather than racing it. +#[test] +fn stop_waits_for_a_baseline_that_is_still_being_written() { + let root = gated("stop-pending", STOP_POLICY); + std::fs::write(root.join("red"), "").unwrap(); + let directory = root.join(".git/uphold/stop"); + std::fs::create_dir_all(&directory).unwrap(); + std::fs::write(directory.join("s-wait.pending"), "").unwrap(); + let writer = { + std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_secs(1)); + let baseline = r#"{"ran":[["test","!","-e","red"]],"red":[["test","!","-e","red"]]}"#; + std::fs::write(directory.join("s-wait.json"), baseline).unwrap(); + std::fs::remove_file(directory.join("s-wait.pending")).unwrap(); + }) + }; + let stopped = turn(&root, &moment("Stop", "s-wait")); + writer.join().unwrap(); + let document = decision(&stopped); + assert!(document.get("decision").is_none(), "{document}"); + assert!( + document.get("systemMessage").is_some_and(Value::is_string), + "{document}" + ); +} + +/// The harness supplies the id that becomes a file name. +#[test] +fn a_session_id_that_could_name_another_path_is_not_used_as_one() { + let root = gated("stop-unsafe-id", STOP_POLICY); + let started = turn(&root, &moment("SessionStart", "../escaped")); + assert_eq!(code(&started), 2, "{}", stderr(&started)); + assert!(stderr(&started).contains("is not used as a file name")); + assert!(!root.join(".git/uphold/escaped.json").exists()); + assert!(!root.join(".git/uphold/stop").exists()); + + std::fs::write(root.join("red"), "").unwrap(); + let stopped = turn(&root, &moment("Stop", "../escaped")); + let document = decision(&stopped); + assert_eq!(document.get("decision"), Some(&Value::from("block"))); + assert!( + document + .get("reason") + .and_then(Value::as_str) + .unwrap() + .contains("is not used as a file name") + ); +} + +#[test] +fn a_policy_without_a_stop_gate_says_nothing_was_checked() { + let root = gated("stop-undeclared", POLICY); + let stopped = turn(&root, &moment("Stop", "s-undeclared")); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + assert_eq!(stdout(&stopped), ""); + assert!(stderr(&stopped).contains("declares no `[stop] run`")); +} + +/// A gate that cannot run is not a gate that passed. Exit 2 at Stop sends +/// stderr to the agent, so this fails closed. +#[test] +fn a_command_that_cannot_start_is_could_not_look() { + let root = gated( + "stop-unstartable", + "[stop]\nrun = [[\"uphold-no-such-program\"]]\n", + ); + let stopped = turn(&root, &moment("Stop", "s-unstartable")); + assert_eq!(code(&stopped), 2, "{}", stderr(&stopped)); + assert!(stderr(&stopped).contains("could not be started")); +} + +#[test] +fn an_entry_naming_no_program_is_refused_at_load() { + let root = gated("stop-empty-argv", "[stop]\nrun = [[]]\n"); + let stopped = turn(&root, &moment("Stop", "s-empty")); + assert_eq!(code(&stopped), 2, "{}", stderr(&stopped)); + assert!(stderr(&stopped).contains("names no program")); +} + +/// What a repository's tree has to pass is not a fact another file can know. +#[test] +fn a_stop_gate_in_an_inherited_file_is_refused() { + let root = gated( + "stop-inherited", + "[inherit]\npaths = [\"policy/shared.toml\"]\n", + ); + std::fs::write(root.join("policy/shared.toml"), STOP_POLICY).unwrap(); + let stopped = turn(&root, &moment("Stop", "s-inherited")); + assert_eq!(code(&stopped), 2, "{}", stderr(&stopped)); + assert!(stderr(&stopped).contains("`[stop]` names the commands")); +} From 0d18e6a126c547b4f953e6d94761719e3b8c5219 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Sat, 24 Oct 2026 23:40:13 +0900 Subject: [PATCH 2/3] A baseline whose tree moved while it ran is discarded, and a gate that cannot start blocks only once SessionStart runs async, so the agent can edit files while a slow gate is still reading them, and the session's own red was recorded as red it found. The tree is now fingerprinted before and after the run (HEAD, and the size and mtime of every changed or untracked path); a baseline whose tree moved is written as unusable, so every red command blocks. A command that could not start returned before stop_hook_active was read, so every follow-up stop exited 2 and blocked again. On a stop that follows a block it is now named in systemMessage and the turn ends. Claude-Session: https://claude.ai/code/session_017EkhA3KMqw95DzAEYPkDn7 --- docs/REFERENCE.md | 10 ++++- src/hook/stop.rs | 101 ++++++++++++++++++++++++++++++++++++++++------ tests/hook_cli.rs | 43 ++++++++++++++++++++ 3 files changed, 139 insertions(+), 15 deletions(-) diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 82ff2e1..958247b 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -3279,7 +3279,12 @@ none, is judged as a pending tool call, as above. - **`SessionStart`** runs every command once and records which failed, under the worktree's git dir (`.git/uphold/stop/.json`). It prints nothing on stdout, because Claude Code adds that to the conversation, and - `async` keeps the session from waiting for it. A `session_id` outside + `async` keeps the session from waiting for it. Because the agent is already + working while it runs, the tree is fingerprinted before and after (HEAD, and + the size and modification time of every changed or untracked path); a + baseline whose tree moved is discarded, so every red command blocks. A gate + that writes outside ignored paths therefore never leaves a usable baseline. + A `session_id` outside `A-Z a-z 0-9 - _`, or longer than 128, is refused rather than used as a file name. - **`Stop`** runs the commands again. One that fails now and passed at session @@ -3297,7 +3302,8 @@ reason told the agent to fix it or say it cannot, and blocking an agent that said so would spend Claude Code's own cap (eight consecutive continuations, reset by a tool call) on a turn that is over. A command that cannot be started is exit `2`; at `Stop` -that sends stderr to the agent as the reason, so a broken gate fails closed. +that sends stderr to the agent as the reason, so a broken gate fails closed, +once: on a stop that follows a block it is named in `systemMessage` instead. With no policy, or no `[stop]`, nothing runs, and stderr says no gate was checked rather than that one passed. `--junit` is not written for these two events. The gate judges the session's working directory, so work done in a diff --git a/src/hook/stop.rs b/src/hook/stop.rs index 4f9188a..0e30313 100644 --- a/src/hook/stop.rs +++ b/src/hook/stop.rs @@ -18,6 +18,13 @@ //! that was already red is named to the person in `systemMessage`, and the turn //! ends. //! +//! `SessionStart` is configured `async`, so the agent is already working while +//! the baseline runs, and a slow gate can read a file after the agent changed +//! it -- recording the session's own red as red it found. So the tree is +//! fingerprinted before and after the run, and a baseline whose tree moved +//! underneath it is discarded: with no baseline every red command blocks, +//! which fails toward the gate rather than away from it. +//! //! The baseline lives under the worktree's git dir, keyed by `session_id`. The //! harness supplies that id, so it is held to a closed character set before it //! becomes a path: an id that could name `../` is an id that could make this @@ -65,6 +72,10 @@ struct Baseline { ran: Vec>, /// The ones that failed. red: Vec>, + /// Why this run cannot be compared against, where it cannot: the tree + /// changed while it ran. + #[serde(default, skip_serializing_if = "Option::is_none")] + unusable: Option, } struct Outcome { @@ -108,6 +119,42 @@ fn baseline_dir(root: &Path) -> Result { Ok(git::dir(root)?.join("uphold").join("stop")) } +/// What the tree looks like to a gate: HEAD, and the size and modification +/// time of every path git reports as changed or untracked. Any edit to a file +/// moves its time, and a commit moves HEAD or the status list, so two equal +/// fingerprints mean nothing the gate reads moved in between -- ignored paths +/// aside, which is where builds put what they write. +fn fingerprint(root: &Path) -> Result { + let mut seen = + git::try_run(root, &["rev-parse", "-q", "--verify", "HEAD"])?.unwrap_or_default(); + let entries = git::run_z( + root, + &[ + "--no-optional-locks", + "status", + "--porcelain=v1", + "-z", + "--untracked-files=all", + ], + )?; + for entry in entries { + let path = entry.get(3..).unwrap_or(&entry); + let stamp = std::fs::symlink_metadata(root.join(path)).map_or_else( + |_| "missing".to_owned(), + |metadata| { + let modified = metadata + .modified() + .ok() + .and_then(|time| time.duration_since(std::time::UNIX_EPOCH).ok()) + .map_or(0, |since| since.as_nanos()); + format!("{} {modified}", metadata.len()) + }, + ); + write!(seen, "\n{entry}\0{stamp}").ok(); + } + Ok(seen) +} + fn run_gate(root: &Path, commands: &[Vec]) -> Result> { commands .iter() @@ -150,8 +197,13 @@ pub(super) fn session_start(event: &Value, root: &Path, policy: &Policy) -> Resu std::fs::create_dir_all(&directory)?; let pending = directory.join(format!("{id}.pending")); std::fs::write(&pending, b"")?; - let outcomes = run_gate(root, &policy.stop.run); - let written = outcomes.and_then(|outcomes| { + let written = fingerprint(root).and_then(|before| { + let outcomes = run_gate(root, &policy.stop.run)?; + let unusable = (fingerprint(root)? != before).then(|| { + "the tree changed while the SessionStart baseline ran, so it cannot say what was \ + red before this session" + .to_owned() + }); let baseline = Baseline { ran: outcomes .iter() @@ -162,6 +214,7 @@ pub(super) fn session_start(event: &Value, root: &Path, policy: &Policy) -> Resu .filter(|outcome| !outcome.passed()) .map(|outcome| outcome.argv.clone()) .collect(), + unusable, }; let document = serde_json::to_vec(&baseline) .map_err(|error| Fatal::new(format!("the baseline is not writable: {error}")))?; @@ -201,12 +254,19 @@ fn baseline(event: &Value, root: &Path) -> Result read, + Err(error) => { + return Ok(Err(format!( + "the baseline at {} is unreadable ({error})", + recorded.display() + ))); + } + }; + Ok(match read.unusable { + Some(why) => Err(why), + None => Ok(read), + }) } fn tail(output: &str) -> String { @@ -225,8 +285,27 @@ pub(super) fn stop(event: &Value, root: &Path, policy: &Policy) -> Result ); return Ok(Exit::Clean); } + let sent_back = event + .pointer("/stop_hook_active") + .and_then(Value::as_bool) + .unwrap_or(false); let known = baseline(event, root)?; - let outcomes = run_gate(root, &policy.stop.run)?; + let outcomes = match run_gate(root, &policy.stop.run) { + Ok(outcomes) => outcomes, + // A gate that cannot run blocks the first stop, which fails closed. It + // blocks no second one: the agent was sent back once, and a missing + // program is not something every further turn can fix. + Err(error) if sent_back => { + let message = format!( + "uphold: the [stop] gate could not run after the agent was sent back once, so \ + the turn ends unchecked: {error}" + ); + eprintln!("{message}"); + println!("{}", serde_json::json!({ "systemMessage": message })); + return Ok(Exit::Clean); + } + Err(error) => return Err(error), + }; let (preexisting, blocking): (Vec<&Outcome>, Vec<&Outcome>) = outcomes .iter() .filter(|outcome| !outcome.passed()) @@ -238,10 +317,6 @@ pub(super) fn stop(event: &Value, root: &Path, policy: &Policy) -> Result let mut document = Map::new(); let mut told: Vec = Vec::new(); - let sent_back = event - .pointer("/stop_hook_active") - .and_then(Value::as_bool) - .unwrap_or(false); if !blocking.is_empty() && sent_back { let names: Vec = blocking .iter() diff --git a/tests/hook_cli.rs b/tests/hook_cli.rs index 327f9c6..301ec83 100644 --- a/tests/hook_cli.rs +++ b/tests/hook_cli.rs @@ -1176,6 +1176,49 @@ fn a_stop_that_follows_a_block_ends_the_turn_and_tells_the_person() { assert!(message.contains("`test ! -e red`"), "{message}"); } +/// A gate that cannot start blocks once. It does not hold every later turn. +#[test] +fn a_gate_that_cannot_start_does_not_block_a_stop_that_follows_a_block() { + let root = gated( + "stop-unstartable-again", + "[stop]\nrun = [[\"uphold-no-such-program\"]]\n", + ); + let again = serde_json::json!({ + "hook_event_name": "Stop", + "session_id": "s-unstartable-again", + "stop_hook_active": true, + }); + let stopped = turn(&root, &again.to_string()); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + let document = decision(&stopped); + let message = document + .get("systemMessage") + .and_then(Value::as_str) + .unwrap(); + assert!(message.contains("could not be started"), "{message}"); +} + +/// The baseline runs while the agent works. A tree that moved under it cannot +/// say what was red before the session, so it is not compared against. +#[test] +fn a_baseline_whose_tree_changed_while_it_ran_is_not_trusted() { + let root = gated( + "stop-moved-tree", + "[stop]\nrun = [[\"touch\", \"written-during-baseline\"], [\"test\", \"!\", \"-e\", \"red\"]]\n", + ); + std::fs::write(root.join("red"), "").unwrap(); + let started = turn(&root, &moment("SessionStart", "s-moved")); + assert_eq!(code(&started), 0, "{}", stderr(&started)); + let stopped = turn(&root, &moment("Stop", "s-moved")); + let document = decision(&stopped); + assert_eq!(document.get("decision"), Some(&Value::from("block"))); + let reason = document.get("reason").and_then(Value::as_str).unwrap(); + assert!( + reason.contains("the tree changed while the SessionStart baseline ran"), + "{reason}" + ); +} + #[test] fn a_green_gate_lets_the_turn_end_silently() { let root = gated("stop-green", STOP_POLICY); From c50a088c138bfc12f7a0e5b2695e9ff74b1c8fc9 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Sat, 24 Oct 2026 23:56:31 +0900 Subject: [PATCH 3/3] A resumed session keeps its first baseline, a killed one is not waited on again, and an unreadable one is missing rather than fatal Claude Code fires SessionStart again on resume and compaction under the same session_id, and the second run replaced the baseline, recording the session's own red as red it found. The first baseline now stands: the marker is created with create_new, and an existing baseline or marker skips the run. The wait for a running baseline is now measured from the marker's mtime, so a SessionStart that was killed does not cost every later stop the whole 300 seconds. baseline() no longer propagates I/O errors (no git dir, an unreadable file); they are a missing baseline, so a follow-up stop still honours the one-block rule instead of exiting 2. Claude-Session: https://claude.ai/code/session_017EkhA3KMqw95DzAEYPkDn7 --- docs/REFERENCE.md | 6 ++-- src/hook/stop.rs | 75 +++++++++++++++++++++++++++++++---------------- tests/hook_cli.rs | 61 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 28 deletions(-) diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 958247b..f65d026 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -3284,7 +3284,8 @@ none, is judged as a pending tool call, as above. the size and modification time of every changed or untracked path); a baseline whose tree moved is discarded, so every red command blocks. A gate that writes outside ignored paths therefore never leaves a usable baseline. - A `session_id` outside + Claude Code fires `SessionStart` again on resume and compaction under the + same id; the first baseline, finished or not, is kept. A `session_id` outside `A-Z a-z 0-9 - _`, or longer than 128, is refused rather than used as a file name. - **`Stop`** runs the commands again. One that fails now and passed at session @@ -3293,7 +3294,8 @@ none, is judged as a pending tool call, as above. already failing does not block, and is named in `systemMessage` for the person. With no baseline, because `SessionStart` is not configured or the id was unusable, every failing command blocks and the reason says so. A - baseline still being written is waited for, up to 300 seconds, so the + baseline still being written is waited for, up to 300 seconds from when it + started (a killed one is not waited on again), so the `timeout` above covers that wait plus one run of the gate. A turn's end is blocked once. When `stop_hook_active` says this stop follows diff --git a/src/hook/stop.rs b/src/hook/stop.rs index 0e30313..b9dfc5e 100644 --- a/src/hook/stop.rs +++ b/src/hook/stop.rs @@ -44,7 +44,7 @@ use std::fmt::Write as _; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::time::{Duration, Instant}; +use std::time::{Duration, SystemTime}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; @@ -195,8 +195,25 @@ pub(super) fn session_start(event: &Value, root: &Path, policy: &Policy) -> Resu } let directory = baseline_dir(root)?; std::fs::create_dir_all(&directory)?; + // Claude Code fires SessionStart again on resume and on compaction, under + // the same id. A second run would record the session's own red as red it + // found, so the first baseline -- finished, running, or killed -- stands. + let recorded = directory.join(format!("{id}.json")); let pending = directory.join(format!("{id}.pending")); - std::fs::write(&pending, b"")?; + if recorded.exists() { + return Ok(Exit::Clean); + } + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&pending) + { + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + return Ok(Exit::Clean); + } + Err(error) => return Err(error.into()), + } let written = fingerprint(root).and_then(|before| { let outcomes = run_gate(root, &policy.stop.run)?; let unusable = (fingerprint(root)? != before).then(|| { @@ -221,7 +238,7 @@ pub(super) fn session_start(event: &Value, root: &Path, policy: &Policy) -> Resu // Renamed into place, so `Stop` never reads half a file. let partial = directory.join(format!("{id}.json.partial")); std::fs::write(&partial, document)?; - std::fs::rename(&partial, directory.join(format!("{id}.json")))?; + std::fs::rename(&partial, &recorded)?; Ok(()) }); std::fs::remove_file(&pending).ok(); @@ -229,44 +246,50 @@ pub(super) fn session_start(event: &Value, root: &Path, policy: &Policy) -> Resu } /// The baseline for this session, or why there is none. -fn baseline(event: &Value, root: &Path) -> Result> { - let id = match session_id(event) { - Ok(id) => id, - Err(why) => return Ok(Err(why)), - }; - let directory = baseline_dir(root)?; +fn baseline(event: &Value, root: &Path) -> std::result::Result { + let id = session_id(event)?; + let directory = baseline_dir(root) + .map_err(|error| format!("there is no git dir to keep a baseline in ({error})"))?; let recorded = directory.join(format!("{id}.json")); let pending = directory.join(format!("{id}.pending")); - let started = Instant::now(); - while !recorded.is_file() && pending.is_file() && started.elapsed() < WAIT_FOR_BASELINE { + // Measured from when the marker was written, not from now: a SessionStart + // that was killed leaves its marker behind, and waiting afresh on every + // stop would charge each one the whole bound for a run that is not coming. + let deadline = std::fs::metadata(&pending) + .and_then(|metadata| metadata.modified()) + .map(|written| written + WAIT_FOR_BASELINE); + while !recorded.is_file() + && pending.is_file() + && deadline + .as_ref() + .is_ok_and(|deadline| SystemTime::now() < *deadline) + { std::thread::sleep(POLL); } if !recorded.is_file() { - return Ok(Err(if pending.is_file() { + return Err(if pending.is_file() { format!( - "the SessionStart baseline was still running after {}s", + "the SessionStart baseline did not finish within {}s of starting", WAIT_FOR_BASELINE.as_secs() ) } else { "no SessionStart baseline was recorded for this session; configure the \ SessionStart hook beside the Stop hook" .to_owned() - })); + }); } - let text = std::fs::read(&recorded)?; - let read: Baseline = match serde_json::from_slice(&text) { - Ok(read) => read, - Err(error) => { - return Ok(Err(format!( - "the baseline at {} is unreadable ({error})", - recorded.display() - ))); - } + let unreadable = |error: &dyn std::fmt::Display| { + format!( + "the baseline at {} is unreadable ({error})", + recorded.display() + ) }; - Ok(match read.unusable { + let text = std::fs::read(&recorded).map_err(|error| unreadable(&error))?; + let read: Baseline = serde_json::from_slice(&text).map_err(|error| unreadable(&error))?; + match read.unusable { Some(why) => Err(why), None => Ok(read), - }) + } } fn tail(output: &str) -> String { @@ -289,7 +312,7 @@ pub(super) fn stop(event: &Value, root: &Path, policy: &Policy) -> Result .pointer("/stop_hook_active") .and_then(Value::as_bool) .unwrap_or(false); - let known = baseline(event, root)?; + let known = baseline(event, root); let outcomes = match run_gate(root, &policy.stop.run) { Ok(outcomes) => outcomes, // A gate that cannot run blocks the first stop, which fails closed. It diff --git a/tests/hook_cli.rs b/tests/hook_cli.rs index 301ec83..ca83f09 100644 --- a/tests/hook_cli.rs +++ b/tests/hook_cli.rs @@ -1219,6 +1219,67 @@ fn a_baseline_whose_tree_changed_while_it_ran_is_not_trusted() { ); } +/// Claude Code fires `SessionStart` again on resume and compaction, under the +/// same id. The second one must not re-baseline the session's own red. +#[test] +fn a_repeated_session_start_keeps_the_first_baseline() { + let root = gated("stop-resumed", STOP_POLICY); + turn(&root, &moment("SessionStart", "s-resumed")); + std::fs::write(root.join("red"), "").unwrap(); + let resumed = turn(&root, &moment("SessionStart", "s-resumed")); + assert_eq!(code(&resumed), 0, "{}", stderr(&resumed)); + let stopped = turn(&root, &moment("Stop", "s-resumed")); + assert_eq!( + decision(&stopped).get("decision"), + Some(&Value::from("block")) + ); +} + +/// A `SessionStart` that was killed leaves its marker. Stop does not wait the +/// whole bound again for a run that is not coming. +#[test] +fn a_stale_pending_marker_is_not_waited_on() { + let root = gated("stop-stale-pending", STOP_POLICY); + std::fs::write(root.join("red"), "").unwrap(); + let directory = root.join(".git/uphold/stop"); + std::fs::create_dir_all(&directory).unwrap(); + let marker = directory.join("s-stale.pending"); + std::fs::write(&marker, "").unwrap(); + let aged = Command::new("touch") + .args(["-t", "202001010000"]) + .arg(&marker) + .status() + .unwrap(); + assert!(aged.success()); + let started = std::time::Instant::now(); + let stopped = turn(&root, &moment("Stop", "s-stale")); + assert!(started.elapsed() < std::time::Duration::from_secs(30)); + let reason = decision(&stopped) + .get("reason") + .and_then(Value::as_str) + .unwrap() + .to_owned(); + assert!(reason.contains("did not finish within"), "{reason}"); +} + +/// Outside a git repository there is nowhere to keep a baseline. That is a +/// missing baseline, which still honours the one-block rule. +#[test] +fn outside_a_git_repository_the_baseline_is_missing_not_fatal() { + let root = workspace("stop-no-git", Some(STOP_POLICY)); + std::fs::write(root.join("red"), "").unwrap(); + let again = serde_json::json!({ + "hook_event_name": "Stop", + "session_id": "s-no-git", + "stop_hook_active": true, + }); + let stopped = turn(&root, &again.to_string()); + assert_eq!(code(&stopped), 0, "{}", stderr(&stopped)); + let document = decision(&stopped); + assert!(document.get("decision").is_none(), "{document}"); + assert!(document.get("systemMessage").is_some_and(Value::is_string)); +} + #[test] fn a_green_gate_lets_the_turn_end_silently() { let root = gated("stop-green", STOP_POLICY);