Repository navigation
feat(hook): adjudicate mediated calls from a declarative rule table - #203
Conversation
`hook`'s policy was a hardcoded `match (pair[0], pair[1])` in `hook.rs` carrying four `gh` shapes and their reason strings — the ad-hoc `if` chain of CLOUD-48's title. It now dispatches on `[[rule]]` rows, and this repo's four rows live in its own `batten.toml`. The engine stops carrying one consumer's vocabulary (non-negotiable rule 1): `mise run land`, `mise run ci-wait` and the `gh` lifecycle leave `crates/batten` entirely, and a reviewer can read what will be refused without reading Rust (§9). ## The schema delta `pattern` is reused rather than adding a shape column: for `forbid` it is a substring banned from files, for `shape` a banned command line whose first word is the effective program and the rest the adjacent non-flag words. One key, one meaning, two domains. New: `reason` (required by `shape` — a mediated deny is the whole explanation a caller gets, so an id alone is un-actionable, CLOUD-122), optional `policy_url`, and `contains`. `contains` earns its place on one real case: the landing directive lives inside a quoted `--body`, so it is not one of the words `pattern` matches, and without it every ordinary `gh pr comment` would be refused alongside the one that carries it. `glob` becomes optional, since a shape rule inspects no files, and `RuleScope::MediatedCall` is what routes a rule to the surface that evaluates it. Spelled `mediated_call`, not `command`, because `command` is already a `RuleKind` token and one word meaning two unrelated things across two keys in one table is the confusion the scope/severity separation exists to prevent. ## Validation by census, not a per-kind match `Rule::validate` was two match arms naming one required and one forbidden field each. That was correct for two kinds and three columns and fails silently the moment either grows — a new column appears in no arm, so every kind accepts it. Columns are now listed once (`Rule::columns`) and every kind is asked about all of them via `requires`/`permits`/`scopes`, with `every_optional_rule_field_is_classified_by_every_kind` failing if a column is added and left unplaced. Validation also moves to load time. It was defensible while the tree engine was the only runner; `hook` is a second one, and a malformed `mediated_call` row validated only by `check` is a row that loads, matches nothing at the mediation channel, and reads as coverage. ## What keeps `hook` unable to run configured code `Policy::from_resolved` filters on scope alone. That is safe because `RuleKind::scopes` pairs every spawning kind with `Tree` only, which `no_mediated_call_kind_spawns_a_process` pins over the whole cross product rather than a named pair. Resolving through `resolve` rather than `config::load` is deliberate: a `batten.local.toml` may only tighten, adding a shape row is a tightening, and a raise-only override model is worth nothing at a surface that ignores it. `--config-from` comes along for free. Absent authority is the empty policy — allow, silently, because `hook` mediates every call in whatever directory the agent is in. An authority that exists and cannot be read is exit 1, loud, never a deny: §7 spends 2 on the verdict alone. ## Corrections to the ticket, and one recorded negative The Ready block's acceptance (e) said an invalid `severity` exits 2. It cannot: 2 is the deny code, so a config typo would refuse every mediated call, which non-negotiable rule 5 and `no_failure_code_can_deny_a_mediated_call` forbid. Implemented as exit 1, matching every sibling config error. `the_schema_cannot_express_per_kind_requirements` records an honest gap rather than hiding it. `Rule` is a flat `deny_unknown_fields` struct, so schemars emits one `required` list and "required iff kind == shape" has nowhere to live; `glob` and `reason` are looser in the published schema than in the binary. The binary is the authority and still refuses, which is the safe direction, but it is asserted rather than left to be discovered. `hook` stays `Effect::Unclassified`. After this change `read` would be defensible — it reaches no user-supplied code — but the classification is load-bearing in exactly one place, the derived agent allowlist, and putting a deny-issuing mediator on it buys nothing an agent needs. The reason is now stated in `surface.rs` beside the row. Refs: CLOUD-48
CLOUD-48 Replace the ad-hoc command `if` chain with a declarative rule table
Why Definition of done
Acceptance
Refinement — reconciled seam (severity ownership → CLOUD-61) The severity model (enum, scope≠severity, explicit per-field defaults, conformance test) is owned solely by CLOUD-61 to avoid two definitions drifting. This issue is now scoped to the rule-table structure — stable Re-scoped against the landed engine (CLOUD-12). The Refinement — Ready (the hook's hardcoded deny table becomes governed Refinement gate: Definition of Ready & Done. This body carries only specializations.
|
|
/fast-forward |
Closes CLOUD-48. Depends on CLOUD-269 (landed).
hook's policy was a hardcodedmatch (pair[0], pair[1])inhook.rscarrying fourghshapes and their reason strings — the ad-hocifchain of the title. It now dispatches on[[rule]]rows, and this repo's four rows live in its ownbatten.toml.The payoff is non-negotiable rule 1, which previously did not hold:
mise run land,mise run ci-waitand the wholeghlifecycle have leftcrates/batten.git grep -Fn "mise run land" crates/now finds only a test fixture. A reviewer can read what will be refused without reading Rust (§9).All 18 pre-existing hook tests pass unchanged against a fixture
Policy, including every CLOUD-181 wrapper-lookthrough case — the policy moved from code to config without a single verdict changing.Schema
patternis reused rather than adding a shape column: forforbida substring banned from files, forshapea banned command line whose first word is the effective program and the rest the adjacent non-flag words. One key, one meaning, two domains.New columns:
reason(required byshape— a mediated deny is the entire explanation a caller gets, so an id alone is un-actionable per CLOUD-122), optionalpolicy_url, andcontains.containsearns its place on one real case: the landing directive lives inside a quoted--body, so it is not one of the wordspatternmatches, and without it every ordinarygh pr commentwould be refused alongside the one carrying it.globbecomes optional (a shape rule inspects no files) andRuleScope::MediatedCallroutes a rule to its surface. Spelledmediated_call, notcommand, becausecommandis already aRuleKindtoken — one word meaning two unrelated things across two keys in one table is the confusion the scope/severity separation exists to prevent.Validation by census
Rule::validatewas two match arms naming one required and one forbidden field each. Correct for two kinds and three columns; silently wrong the moment either grows, because a new column appears in no arm and every kind then accepts it. Columns are now listed once (Rule::columns) and each kind is asked about all of them viarequires/permits/scopes, withevery_optional_rule_field_is_classified_by_every_kindfailing if one is added and left unplaced.Validation also moves to load time. Defensible while the tree engine was the only runner;
hookis a second one, and a malformedmediated_callrow validated only bycheckis a row that loads, matches nothing at the mediation channel, and reads as coverage.What keeps
hookunable to run configured codePolicy::from_resolvedfilters on scope alone. That is safe only becauseRuleKind::scopespairs every spawning kind withTree— asserted byno_mediated_call_kind_spawns_a_processover the whole cross product, not a named pair.Resolving through
resolverather thanconfig::loadis deliberate: abatten.local.tomlmay only tighten, adding a shape row is a tightening, and a raise-only override model is worth nothing at a surface that ignores it.--config-fromcomes along for free.Absent authority is the empty policy (allow, silently) because
hookmediates every call in whatever directory the agent is in. An authority that exists and cannot be read is exit 1, loud, never a deny — §7 spends 2 on the verdict alone. That discharges CLOUD-40's "fails open loudly" clause, which was unreachable whilehookread no config.Two corrections and one recorded negative
severity; 2 is the deny code, so a config typo would refuse every mediated call, which non-negotiable rule 5 andno_failure_code_can_deny_a_mediated_callforbid. Implemented as exit 1. Ready block amended.committed_rules_pin_severity_and_scope_explicitlyassertedscope == "tree"for every committed rule — a literal correct with one scope, now a vocabulary check.the_schema_cannot_express_per_kind_requirementsrecords an honest gap.Ruleis a flatdeny_unknown_fieldsstruct (aflattenenum would defeat that guarantee), so schemars emits onerequiredlist and "required iff kind == shape" has nowhere to live:globandreasonare looser in the published schema than in the binary. The binary is the authority and still refuses — the safe direction — but it is asserted rather than left to be discovered.Notes for review
hook_reads_no_config_so_a_broken_one_cannot_blockis deleted here by design. CLOUD-40 wrote it to pin "config is not load-bearing"; its deletion is the seam where that stopped being true.the_committed_shape_rules_fire_on_every_banned_shapeis the obligation the fixture tests do not discharge: every other hook test supplies its own policy, so deleting a row from the realbatten.tomlwould otherwise break nothing while silently disarming the guard chain.shape-mediated-callsatisfiesthe_corpus_covers_every_rule_kind(landed in feat(gate): discover a fixture-repo corpus, and collapse nine copies of the materializer #191 mid-flight) and pins the routing property:checkwalks past amediated_callrow without erroring and does not mistake it for a file rule.hookstaysEffect::Unclassified. After this changereadwould be defensible — it reaches no user-supplied code — but the classification is load-bearing in exactly one place, the derived agent allowlist, and putting a deny-issuing mediator on it buys nothing an agent needs. The reason is now stated insurface.rsbeside the row, replacing a bare CLOUD-202 citation.min_batten_version→0.0.42, the current workspace version. Not the next patch:check_min_versioncompares against the running build, so a higher floor makes the tree refuse its own binary. Precedent:20f138eset0.0.30whileCargo.tomlwas0.0.32.mise run verifygreen on this SHA, rebased onorigin/mainea48b71.