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
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ knob with a sane default — not an operator toggle. Prefer a good default over
## Workflow

- Branch + PR; never commit directly to `main`. Merge on green CI.
- Review checklist: a PR that removes a consumer of a shared layer states the
layer's remaining live-consumer count in one line; a drop-to-one is a refactor
smell to flag, not merge silently ([ADR-0042](docs/adr/0042-flag-remaining-consumer-count-on-consumer-removal.md)).
- **Releases follow strict semver**, computed from the conventional-commit subjects since
the last tag: `feat` → **minor**; `fix`/`perf`/`refactor`/`chore`/`docs`/`ci`/`build` →
**patch**; `!` or `BREAKING CHANGE` → **major** (pre-1.0: breaking → minor). Do not
Expand Down
105 changes: 105 additions & 0 deletions docs/adr/0042-flag-remaining-consumer-count-on-consumer-removal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# 0042. Flag a shared layer's remaining live-consumer count when a PR removes a consumer

- Status: Accepted
- Date: 2026-08-08
- Relates to: [0024](0024-no-redundant-by-construction-predicates.md)

## Context

PR #189 (`perf(model): cache all ollama completions in one bounded client middleware`)
added a bounded LRU cache in front of `chat()`, deliberately keyed at the model-client
boundary "so EVERY model consumer is covered by one mechanism" — its two consumers at
the time were the adjudicator (ADR-0013) and the model-backed hypothesis stage
(`ModelHypothesizer`). Reviewed alone, it is a clean shared-layer change: one cache,
two call sites, no gap.

Eight minutes later, PR #190 (`refactor(engine): remove the model-backed hypothesis
stage — deterministic proof only`) deleted `ModelHypothesizer` entirely: `proof::prove`
already enumerated every structurally-proven chain by exhaustive walk, so the
hypothesis stage discovered nothing and only burned a completion per pass. Reviewed
alone, it is also a clean deletion — the commit message even names the consequence
correctly ("The ONLY remaining Ollama consumer is the adjudicator").

Neither PR was wrong on its own, and neither review caught what their conjunction
did: PR #190 deleted one of the cache's two consumers, dropping PR #189's
just-added shared layer to a single caller. With one consumer, the LRU stopped
being a *shared* cache and became a redundant wrapper around a cache that already
existed one layer up — the deterministic verdict store (ADR keyed on the exact
prompt hash) already short-circuited `chat()` on a hit; the LRU added nothing on a
miss but a *correctness hazard*, because it cached any HTTP 200 including replies
that parse to `Uncertain`, while the verdict store deliberately never caches
`Uncertain` (it must retry). The bug shipped, then had to be found and reverted in
PR #191 (`revert(model): drop the ollama completion cache — redundant +
Uncertain-pinning hazard`), less than an hour after PR #190 merged.

The root cause was not a defect in either diff. It was that **per-PR review has no
view of a neighboring PR's effect on the same abstraction.** PR #189's reviewer
had no reason to open `hypothesis.rs`; PR #190's reviewer had no reason to open
`model/cache.rs`. The redundancy existed only in the two PRs' conjunction, and nothing
in either diff, or in CI, surfaced it. This is the same failure family
[ADR-0024](0024-no-redundant-by-construction-predicates.md) names for a single PR
(a predicate whose result is already fixed by an existing arm is dead code, however
tidy and tested) — except here the fixing arm and the redundant layer landed as *two
separate, individually-reasonable* PRs, which is exactly the case ADR-0024's per-PR
framing cannot catch.

## Decision

**When a PR removes a consumer of a shared layer or abstraction (a cache, a port, a
shared middleware, a common helper used by more than one call site), the PR
description states the layer's remaining live-consumer count in one line** — e.g.
"the model-client cache now has 1 remaining consumer (the adjudicator)." This is a
statement of fact the author already has (they just read the call sites to make the
deletion), not new analysis.

A remaining count of **one** is flagged in review as a refactor smell, not merged
silently: a layer built to serve multiple consumers that now serves one is either (a)
no longer earning its abstraction — inline it or fold it into its sole caller, or (b)
still justified (a real seam the count will grow back into) — in which case the PR
description says so in the same line. Either way the reviewer sees the number and
makes the call explicitly, instead of the drop-to-one passing unnoticed because the
diff itself looks like a clean, unrelated deletion.

This is a review-time discipline, not a new automated check: no tool here reliably
knows what counts as "a shared layer" or enumerates its call sites across the whole
tree on every consumer-deletion diff. It is enforced the way ADR-0024's rule is
enforced — a reviewer checklist item and a citable ADR to point at.

## Consequences

Easier:
- The exact failure PR #189/#190/#191 hit is caught at review time on the
consumer-removal PR, before merge, instead of being found by a later revert.
- Reviewers get a one-line, low-cost signal (a count, not a design review) that
surfaces the cross-PR case ADR-0024 already covers for the single-PR case.
- A drop-to-one that *is* still justified gets its justification recorded in the PR
description at the moment it happens, rather than reconstructed later from git
archaeology.

Harder / accepted:
- This depends on the PR author actually running the count and writing the line;
nothing in CI enforces it (see Scope, below). Accepted: this repo's review
discipline is documented as reviewer checklist items elsewhere (ADR-0024, this
ADR) rather than as tooling, and a one-line prompt is cheap enough to expect a
reviewer to ask for it when it's missing.
- Does not by itself detect the case in reverse — a PR that *adds* a second consumer
to a layer built for one is not this ADR's concern; that is ordinary code reuse and
is not the redundancy failure mode observed here.
- Scope: catching redundancy that spans a whole review *batch* (not just a single
consumer-removal PR against the tree at merge time) is a workflow-tooling concern,
not a repo convention — out of scope for this ADR.

## References

- [0024](0024-no-redundant-by-construction-predicates.md) — the sibling rule for the
single-PR case (a predicate whose result is already fixed by an existing arm is not
defense in depth, it is dead code); this ADR extends the same discipline across a
two-PR conjunction that no single diff shows.
- PR #189 (`perf(model): cache all ollama completions in one bounded client
middleware`) — added the shared cache "so EVERY model consumer is covered."
- PR #190 (`refactor(engine): remove the model-backed hypothesis stage —
deterministic proof only`) — deleted one of the cache's two consumers eight
minutes later, dropping it to one.
- PR #191 (`revert(model): drop the ollama completion cache — redundant +
Uncertain-pinning hazard`) — the revert that found the conjunction, less than an
hour after PR #190 merged.
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ Copy [`0000-template.md`](0000-template.md) to start one.

| [0040](0040-node-scoped-containment-mechanism-escalation.md) | Node-scoped containment is a **deterministic mechanism escalation** of a model-decided target: when typed evidence proves the adversary broke the *pod* boundary (host-cred read; root + escape edge; ptrace/module-load; or ≥2 model-confirmed actively-exploited pods co-resident), the resolver escalates the model-named workload's cut from a pod NetworkPolicy to `ContainNode` (cordon + co-resident default-deny) — the model keeps *what*, determinism owns *how* (rejects a model-selectable node menu line: highest blast on the weakest measured axis). Own `node` action class at ladder rung 3 (above quarantine), propose-first by construction (a node always has alive collateral), reversible + ownership-marked + one-node-cap + control-plane-excluded + worker-floor; posture-derived `nodes` RBAC; ships shadow-first (trigger/proposal, then actuator). Also records: adversary-reach is a **presentation annotation, never judge context** (context-not-evidence enforced by absence from the prompt), and off-path auto-containment is a **won't-build** (reaffirms 0032 §6, strengthened by 0038). Refines 0032/0034; reuses 0010/0017/0021/0035/0036 rails | Proposed |
| [0041](0041-narrow-blanket-notable-exec-corroboration.md) | Narrow the blanket notable-exec corroboration arm to shapes: a bare interactive-shell/pkg-mgr `ProcessExec` stops blanket-corroborating every objective (`Behavior::ProcessExec → false` in the flat arm — symmetric with PrivilegeChange/PtraceAttach/ModuleLoad, restoring the 0011 on-call-engineer FP guard), and exec-based corroboration re-homes in an entry-scoped `reverse_shell_on_foothold` shape (interactive-shell exec + internet NetworkConnection within a symmetric 60s window, foothold-gated) — unmasking the deferred reverse-shell shape per 0024. Corrects the framing: the model never sees `corroborated` (removed from the prompt by 0034); narrowing buys deterministic auto-apply-gate precision + honest latent/live presentation + 0024 unmasking, NOT model-input precision. Safe **now** because Falco's `Alert` arm still backstops (`falco.enabled: true`), pre-arming in `audit`. One real behavior change (inert under audit): a shell-only + decisive-attack incident becomes a proposal not an auto-cut in enforce (0011-correct). Package managers excluded (always egress); `is_alarming_now`/menu seeding untouched; a narrowing-delta counter feeds the retire-Falco parity bake (0037). Amends 0009's action-bar input | Proposed |
| [0042](0042-flag-remaining-consumer-count-on-consumer-removal.md) | Flag a shared layer's remaining live-consumer count when a PR removes a consumer: a consumer-removal PR states the layer's remaining live-consumer count in one line, and a drop-to-one is flagged in review as a refactor smell rather than merged silently — the discipline PR #189's shared model-completion cache and PR #190's hypothesis-stage deletion needed (their conjunction, invisible to either single-PR review, left a redundant, `Uncertain`-pinning cache that PR #191 had to revert). Extends 0024's single-PR redundant-by-construction rule across a two-PR conjunction | Accepted |

See also [`../VISION.md`](../VISION.md) for the longer-form narrative this ADR realizes.