Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion batten.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8029,9 +8029,22 @@ target = ".claude/rules/commits.md"
id = "hook wire duplicate"
gloss = "a registration on a merged surface outside this repository does not reach the mediator"
class = """
`PreToolUse` is ONE entry — the engine — and CLOUD-312 measured the alternative: six task-runner launches per Bash call, 1.247s serial to do milliseconds of policy, ~93% of it startup. A second decider registered beside the mediator re-introduces that cost and, worse, splits the verdict across two authorities that can disagree about one call. The wiring this judges is the launcher's own merged settings, which live outside this repository — CLOUD-1167's fact is what makes the question expressible at all, and the row that declares the file is the whole bound on what the module may read.
`PreToolUse` is ONE entry — the engine — and CLOUD-312 measured the alternative: six task-runner launches per Bash call, 1.247s serial to do milliseconds of policy, ~93% of it startup. A second decider registered beside the mediator re-introduces that cost and, worse, splits the verdict across two authorities that can disagree about one call. The wiring this judges is the launcher's own merged settings, which live outside this repository — and `batten wiring reclaim` removes the registration from them, so this IS repairable from here. It is a REPAIR AND NOT A FIX: a launcher that writes those registrations at session start writes them again next session, so the same refusal recurs and the same command answers it each time. CLOUD-1167's fact is what makes the question expressible at all, and the row that declares the file is the whole bound on what the module may read.
"""

# THE COMMAND FIRST, because a reader who follows only the first route must reach
# the thing that clears the refusal (CLOUD-1339). Both routes were `document`
# before and neither document names the verb — measured 2026-09-02, that cost one
# session three commits with `hooks-wiring-check` switched off, a false claim
# about this repository's capabilities written into two commit messages on
# main-bound history, and a three-option menu put to a human that one command
# already answered. The predicate was right every time; only the remedy was
# missing, which is the shape CLOUD-122's contract exists to refuse.
[[verdict.route]]
id = "verb run first"
kind = "command"
target = "batten wiring reclaim"

[[verdict.route]]
id = "prose read first"
kind = "document"
Expand Down
85 changes: 81 additions & 4 deletions crates/batten/src/doctor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,9 +663,21 @@ const SIBLING_REGISTERED: &str = "hook-wiring-sibling-registered";
/// The same, on a surface the host MERGES rather than the committed one.
///
/// Its own id because the remedy differs and the committed one's does not reach
/// it: a merged surface is under `$HOME`, so editing the repository cannot
/// it: a merged surface is under `$HOME`, so editing the REPOSITORY cannot
/// remove the registration — the same reason [`MERGED_REGISTRATION`] is separate
/// from [`EVENT_REGISTERED_N_TIMES`].
///
/// **That is a statement about editing tracked files, not about this
/// repository's reach, and it was read as the latter** (CLOUD-1339).
/// `batten wiring reclaim` removes a merged registration, and
/// `crates/batten/tests/it/wiring_reclaim.rs` drives it against a real `$HOME` —
/// so the remedy exists and ships here. Measured 2026-09-02, a session read this
/// comment beside the refusal, concluded the condition was unfixable from the
/// repository, switched the gate off for three commits and wrote that conclusion
/// into two commit messages. The verb is a REPAIR rather than a fix — a launcher
/// that registers at session start registers again next session — but a repair
/// is not nothing, and it is what the `hook wire duplicate` verdict now routes
/// to first.
const MERGED_SIBLING: &str = "hook-wiring-merged-sibling";
/// The wiring file is there and is not readable as the JSON object it must be.
///
Expand Down Expand Up @@ -911,11 +923,27 @@ fn reaches_engine(entry: &str, harness: hook::Harness) -> bool {
}
// (a) The derived argv appears as a contiguous run immediately after a token
// whose file stem is the binary's name.
let derived = ["hook", "--harness", harness.as_str()];
//
// DERIVED FROM THE `SURFACE` ROW, NOT SPELLED HERE (CLOUD-1191). This read
// `["hook", "--harness", harness.as_str()]` — a literal independent of the
// declaration — so it answered "does the wiring name THIS STRING" where the
// question is "does the wiring name the declared mediation path". Those
// differ exactly when it matters: against a settings file naming a command
// the surface no longer declares, the literal version returns `true` and
// reports stale wiring as healthy. The one diagnostic built for that failure
// was blind to it.
//
// No mediation row means no declared path for a registration to reach, so
// nothing reaches the engine. That is `false` — loud — rather than a literal
// fallback, which would be the spelling this change removes.
let Some(mut derived) = crate::surface::mediation_argv() else {
return false;
};
derived.push(harness.as_str().to_owned());
tokens.iter().enumerate().any(|(at, token)| {
Path::new(token)
.file_stem()
.is_some_and(|stem| stem == "batten")
.is_some_and(|stem| stem == crate::surface::BINARY)
&& tokens.len() >= at + 1 + derived.len()
&& tokens[at + 1..=at + derived.len()] == derived
})
Expand Down Expand Up @@ -1150,6 +1178,42 @@ mod tests {
dir
}

/// `reaches_engine` answers about the DECLARED mediation path, not a
/// literal — so wiring naming a command the surface no longer declares is
/// drift rather than health (CLOUD-1191).
///
/// **The second assertion is the whole row.** Before the derivation, the
/// check matched the literal `"hook"`, so a settings file naming a renamed
/// or removed verb still returned `true`: the one diagnostic built for this
/// failure reported it healthy, while the invocation itself became an
/// unknown subcommand — clap error, exit `1`, which every host reads as
/// allow. Fail-open, reported green.
///
/// Fails by: reverting `reaches_engine` to a literal argv.
#[test]
fn wiring_naming_an_undeclared_path_does_not_reach_the_engine() {
let harness = hook::Harness::ClaudeCode;
let live = hook::wiring_command(harness);
assert!(
reaches_engine(&live, harness),
"the command this build emits must reach the engine: {live}"
);

// The same shape with a verb the surface does not declare. This is what a
// half-done rename leaves in a committed wiring file. `adjudicate` is the
// path CLOUD-1192 proposes, so this is the exact string that row's
// rename would strand if it landed without this derivation.
let stale = format!(
"{} adjudicate --harness {}",
crate::surface::BINARY,
harness.as_str()
);
assert!(
!reaches_engine(&stale, harness),
"wiring naming an undeclared verb must read as drift, not health: {stale}"
);
}

#[test]
fn a_missing_config_is_named_rather_than_lumped_in() {
let dir = scratch("missing-config");
Expand Down Expand Up @@ -1793,8 +1857,21 @@ mod tests {
"a legitimate spelling must stay green — {spelling}"
);
}
// DERIVED, because a literal here is the fourth spelling CLOUD-1191
// removed — this assertion hardcoded `hook` and went red on the rename,
// which is the derivation catching its own test rather than the test
// catching the derivation.
let argv = crate::surface::mediation_argv().expect("declared");
assert!(
reaches_engine("/opt/bin/batten.exe hook --harness claude-code", harness),
reaches_engine(
&format!(
"/opt/bin/{}.exe {} {}",
crate::surface::BINARY,
argv.join(" "),
harness.as_str()
),
harness
),
"the stem match is what lets a Windows image pass"
);
// And the drift case the existing suite pins stays drift.
Expand Down
19 changes: 18 additions & 1 deletion crates/batten/src/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1164,8 +1164,25 @@ const CLAUDE_SPELLINGS: &[(Event, &str)] = &[
/// the one module allowed to. The search was an install concern. What the
/// paragraph above still says is unchanged, and it is what keeps the emitter able
/// to serve a consumer that does need one.
///
/// **The argv is DERIVED from the `SURFACE` row, never spelled here**
/// (CLOUD-1191). This was `format!("batten hook --harness {}", …)`, one of three
/// independent spellings; renaming the row left this one behind, and the
/// resulting unknown subcommand is a clap error — exit `1` — which every host
/// reads as allow. The disagreement would not have broken loudly, it would have
/// turned enforcement off everywhere while `doctor` reported green.
///
/// A surface declaring no mediation row yields the binary and the harness with
/// no verb between them — a command that matches nothing, so every registration
/// reports as drift. That is the LOUD direction, and it is why there is no
/// literal fallback here: emitting `"hook"` when the declaration is gone would
/// be the fourth spelling this removes, and it would report healthy.
pub(crate) fn wiring_command(harness: Harness) -> String {
format!("batten hook --harness {}", harness.as_str())
let argv = crate::surface::mediation_argv().unwrap_or_default();
let mut parts = vec![crate::surface::BINARY.to_owned()];
parts.extend(argv);
parts.push(harness.as_str().to_owned());
parts.join(" ")
}

/// Render one harness's registrations as the JSON its host reads.
Expand Down
14 changes: 11 additions & 3 deletions crates/batten/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,13 +560,21 @@ mod tests {
// verb ("structurally incapable, not merely well-behaved") cannot be
// made about a verb whose whole job is adjudicating someone else's
// write. Pinned here so the correction cannot be undone by a row edit.
// DERIVED FROM THE ROW (CLOUD-1191), so the pin survives the rename it
// is pinning against. Spelled `hook` until CLOUD-1192 moved the path to
// `adjudicate`; a literal here would have gone quietly green on that
// rename by asking about a path the surface no longer declares —
// asserting nothing, in the unsafe direction this case exists to stop.
let path = crate::surface::mediation()
.expect("the surface declares a mediation row")
.path;
let allowlist = read_only_allowlist(&spec());
assert!(
!allowlist.iter().any(|entry| entry.path == "hook"),
!allowlist.iter().any(|entry| entry.path == path),
"the mediation entrypoint leaked into the read-only allowlist: {allowlist:?}"
);
assert_eq!(effect_for("hook"), Effect::Unclassified);
assert!(!effect_for("hook").is_read_only());
assert_eq!(effect_for(path), Effect::Unclassified);
assert!(!effect_for(path).is_read_only());
}

#[test]
Expand Down
108 changes: 108 additions & 0 deletions crates/batten/src/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,6 +1872,70 @@ const ROOT_FLAGS: &[FlagDecl] = &[
},
];

/// The binary's own name, as a wiring entry spells it.
///
/// Here rather than beside each consumer for [`mediation`]'s reason: the
/// generator emits it and the diagnostic matches a token's file stem against it,
/// and those two agreeing is what makes a registration reach the engine.
pub const BINARY: &str = "batten";

/// The stable id of the row that adjudicates a mediated tool call.
///
/// **The anchor is the id and never the path**, which is [`CommandDecl::id`]'s
/// whole contract: the path is "the one thing about a row that is expected to
/// change", so a derivation keyed on it re-breaks on exactly the rename it
/// exists to survive.
pub const MEDIATION_ID: &str = "hook";

/// The mediation row, resolved from [`SURFACE`] by [`MEDIATION_ID`].
///
/// **One authority for how the mediator is invoked** (CLOUD-1191). Before this,
/// the argv was spelled independently in three places with nothing linking them
/// — the row here, the generator in [`crate::hook::wiring_command`], and the
/// diagnostic in `doctor`'s `reaches_engine` — plus five committed wiring files
/// carrying it as data.
///
/// # Why a disagreement between them is worse than ordinary drift
///
/// It is a **silent fail-open**. An unknown subcommand is a clap error, which is
/// [`crate::exit::ExitCode::Usage`] (`1`), and `exit.rs` states the consequence
/// as a design property: every host reads anything but `0`/`2` as "the hook
/// itself failed, let the call through". So three literals that disagree do not
/// break loudly — they turn enforcement off across every harness while `doctor`
/// reports green. Renaming the row was measured safe on paper and would have
/// done exactly that.
///
/// Returns `None` only if the row is absent, which
/// [`tests::the_mediation_row_resolves`] refuses. Callers treat `None` as "no
/// declared mediation path" and fail loud rather than falling back to a literal
/// — a fallback would reintroduce the fourth spelling this exists to remove.
#[must_use]
pub fn mediation() -> Option<&'static CommandDecl> {
SURFACE.iter().find(|row| row.id == MEDIATION_ID)
}

/// The mediation row's argv as the wiring spells it: the path, then each
/// required flag as `--long`.
///
/// Derived rather than formatted, so a change to the row's `path` or to its
/// required flags moves the emitted wiring and the diagnostic's expectation in
/// the same build.
/// The value each required flag takes is the CALLER's — the harness — so this
/// stops at the flag and the caller appends it. That keeps the one thing that
/// varies per registration out of a function whose whole job is the part that
/// does not.
#[must_use]
pub fn mediation_argv() -> Option<Vec<String>> {
let row = mediation()?;
let mut argv = vec![row.path.to_owned()];
for flag in row.flags.iter().filter(|flag| flag.required) {
if let Some(long) = flag.long {
argv.push(format!("--{long}"));
}
}
Some(argv)
}

/// The command tree: every subcommand, with its summary, effect, and flags.
///
/// Order is declaration order and does not matter — [`command`] groups rows by
Expand Down Expand Up @@ -4033,6 +4097,50 @@ mod tests {
assert_eq!(seen.len(), total, "a command path is declared twice");
}

/// The mediation row resolves, which is what lets every consumer treat a
/// `None` from [`mediation`] as "the surface declares none" rather than as a
/// bug it has to guard.
///
/// Fails by: renaming or deleting the row's `id`. That is the one edit
/// [`MEDIATION_ID`] does not survive, and it must be loud — the id is the
/// stable anchor precisely so the `path` can move without touching it.
#[test]
fn the_mediation_row_resolves() {
let row = mediation().expect("the surface declares a mediation row");
assert_eq!(row.id, MEDIATION_ID);
assert!(
row.flags.iter().any(|flag| flag.required),
"the mediation row must carry a required flag, or `mediation_argv` \
emits a bare path and every registration reads as drift"
);
}

/// The emitted argv follows the row's `path`, not a literal.
///
/// **This is the case that would have caught the defect.** With three
/// independent spellings, renaming the row left the generator and the
/// diagnostic behind, and the resulting unknown subcommand exits `1` — which
/// every host reads as allow. Fails by: reverting either consumer to a
/// `"hook"` literal.
#[test]
fn the_emitted_argv_is_the_rows_path_and_its_required_flags() {
let row = mediation().expect("declared");
let argv = mediation_argv().expect("declared");
assert_eq!(
argv.first().map(String::as_str),
Some(row.path),
"the argv must open with the row's path"
);
for flag in row.flags.iter().filter(|flag| flag.required) {
if let Some(long) = flag.long {
assert!(
argv.iter().any(|word| word == &format!("--{long}")),
"a required flag the row declares is missing from the argv: {long}"
);
}
}
}

#[test]
fn every_declared_parent_is_itself_declared() {
// `config show` without a `config` row would build a subcommand tree
Expand Down
Loading
Loading