## macp-core re-export from macp-runtime's root
- **Plan:** `plans/rfc-macp-0013-commitment-hash-PROGRESS.md` (RFC-MACP-0013, PR 3 of 5)
- **Assumed:** CLAUDE.md states "the root crate re-exports the lower crates so the historical `macp_runtime::*` paths are preserved," and today `src/lib.rs` re-exports `macp_modes`, `macp_policy`, `macp_storage`, and `macp_auth` — but not `macp_core` itself (only thin `error`/`session` shims). An external consumer of the published `macp-runtime` crate therefore cannot reach `macp_core::commitment_hash::commitment_hash` through `macp_runtime::*` and must take a direct `macp-core` dependency.
- **Chose:** Left this alone in Phase 1 — `commitment_hash` is a new module-only export in `macp-core` (no flat function re-export at that crate's root either, since the crate's existing flat re-exports are all types, and a function re-export shadowing its own module name would read oddly at a call site). Whether `macp-runtime`'s root should also re-export `macp_core` wholesale is a pre-existing gap this phase surfaced, not one it introduced.
- **Alternatives:** Add `pub use macp_core;` (or a narrower `pub mod commitment_hash` shim) to `src/lib.rs` in this same PR.
- **Blast radius if wrong:** Low and cheap to reverse — adding a re-export later is backward-compatible (additive) and doesn't touch any existing call site.
- **Status:** CHANGED (2026-08-30) — re-export added; see `DECISIONS.md`. The entry under-scoped the
problem: `macp_core::mode::MessageContext` appears in the signature of `Mode::on_message_at`, a defaulted
method on the publicly re-exported `Mode` trait, so an external consumer could not override the one method
that offers a trustworthy clock. Resolved by `pub use macp_core;` plus adding `MessageContext` to the
existing `crates/macp-modes/src/mode/mod.rs` re-export.
## Startup config errors folded into the returned Err (stderr), not just tracing
- **Plan:** `plans/list-sessions-pagination.md` (Phase 2, config plumbing)
- **Assumed:** Phase 2's acceptance criterion says an invalid page-size env var must abort startup "with a message naming the variable." Verified against the binary that the per-variable detail from `validate_env_config` goes through `tracing::error!`, whose `tracing_subscriber::fmt()` default writer is **stdout**; only the generic summary (`startup aborted: N configuration error(s) detected`) reaches stderr. Under `RUST_LOG=off` the operator gets the summary and no indication of which variable is wrong.
- **Chose:** Folded the collected error details into the returned `Err` so they reach stderr regardless of logging configuration, keeping the existing `tracing::error!` calls for structured logs. Taken as a tier-2 call under `/drive` (needs judgment, not a one-way door): it is text-only, changes no abort condition, exit code, or validation order, and it is what makes the phase's own acceptance criterion actually true rather than true-only-with-logging-on. A startup abort is the one message that must be self-sufficient.
- **Alternatives:** Leave it and keep the tests asserting across stdout-or-stderr (the executor's original, correctly flagged rather than silently applied); or switch the subscriber's writer to stderr globally (larger blast radius — changes every log line's stream, not just the abort path).
- **Blast radius if wrong:** Low. Text of one error string; revert is a two-line change. Risk is cosmetic duplication (detail appears on both stdout via tracing and stderr via the Err) for operators who do run with logging on.
- **Status:** CONFIRMED (2026-08-30) — the stdout-default claim was independently verified against
`tracing-subscriber` and the running binary. The change removed an inconsistency rather than creating one:
this was the sole fatal startup path whose detail could be silenced by a log filter.
## Startup-gate tests must poll try_wait, never Command::output()
- **Plan:** `plans/list-sessions-pagination.md` (Phase 2, config plumbing)
- **Assumed:** A startup-abort test that uses `Command::output()` blocks until the child closes its pipes. When the validation under test is removed, the binary does **not** exit — it starts a server — so `output()` blocks forever. Observed directly: a mutation check hung and blew a 10-minute timeout before the tests were rewritten.
- **Chose:** `run_expecting_startup_abort` spawns with piped stdio and polls `try_wait` against a 15s deadline, killing and failing if the process is still alive. This converts a future regression into a **test failure in ~30s** instead of a hung CI job.
- **Alternatives:** `Command::output()` (the obvious form, and what the two pre-existing tests in this file still use — `startup_refuses_invalid_policies_dir` and `startup_refuses_without_auth_or_insecure_flag` carry the same latent hazard; left alone as out of scope for this plan).
- **Blast radius if wrong:** Low for this feature. But the two pre-existing tests remain a latent CI-hang risk if either of their validations is ever weakened — worth a separate follow-up, noted here so it is not lost.
- **Status:** CONFIRMED (2026-08-30), with a correction and the follow-up done. **The two pre-existing tests
did not carry the same hazard.** Only `startup_refuses_invalid_policies_dir` was genuinely hang-prone — it
sets `MACP_ALLOW_INSECURE=1`, so nothing but the policies-dir check stands between it and a running server;
it is now on the bounded helper. `startup_refuses_without_auth_or_insecure_flag` has a structural backstop
(the independent TLS gate in `src/main.rs`), so it keeps `output()` and instead gained `env_remove` for the
two TLS paths that would defeat that backstop. The helper drains its pipes only after exit — safe at a few
hundred bytes against a >=16KB buffer, now recorded in its doc comment.
## Config-consistency guard made symmetric (aborts instead of silently clamping)
- **Plan:** `plans/list-sessions-pagination.md` (Phase 2, config plumbing) — **revises the phase's own acceptance criterion 3**
- **Assumed:** As first built, an explicit `MACP_LIST_SESSIONS_DEFAULT_PAGE_SIZE > MACP_LIST_SESSIONS_MAX_PAGE_SIZE` aborted startup naming both values, but the identical operator error with `MAX` unset (e.g. `DEFAULT=5000`, effective max 1000) was silently clamped, announced only by a `tracing::warn!` that writes to stdout and disappears under `RUST_LOG=off`. Same mistake, two completely different outcomes, with the quieter one attached to the more likely case.
- **Chose:** Compare an explicitly-set `DEFAULT` against the **effective** max — explicit when set, otherwise the built-in 1000 — and abort in both cases, naming the effective max and whether it was explicit or the default. Kept `default.min(max)` in the resolver as defence-in-depth for library consumers of `SecurityLayer::from_env()`, who never reach `validate_env_config` (private to `src/main.rs`); it simply stops firing for the binary. Tier-2 call under `/drive`: reversible, config-only, no wire or persisted format.
- **Alternatives:** Leave the asymmetry and record it (what the verifier offered as the other option) — rejected because half a guard is worse than either whole option; or drop the abort entirely and always clamp — rejected because it discards stated operator intent silently, which is the failure mode the guard exists to prevent.
- **Blast radius if wrong:** Moderate-but-contained. A deployment that sets a large DEFAULT and relies on being clamped would now fail to start rather than running with a smaller page size. No such deployment can exist — both variables are new in this change and unreleased. Reversing is a small edit to one validation block.
- **Status:** CONFIRMED (2026-08-30), with two fixes. **Correction:** "it simply stops firing for the binary"
is wrong — the resolver clamp still fires for the binary when `MAX` is set and `DEFAULT` is not; only the
explicit-default branch stops firing. **Fixed:** `MACP_LIST_SESSIONS_MAX_PAGE_SIZE=0` was parsed in
`src/main.rs` without a `> 0` filter, so it produced a spurious second error naming an effective maximum of
`0` while `security.rs` treated the same value as the built-in 1000 — the two layers disagreed on what `0`
meant. The filter is now mirrored, and the abort message states the remedy.
## The env-var-to-field binding is unproven until an end-to-end test exists
- **Plan:** `plans/list-sessions-pagination.md` (Phase 2 → carried into Phase 3/4 as a hard requirement)
- **Assumed:** A verifier swapped the two env values feeding `resolve_list_sessions_page_sizes` in `from_env` and **the entire test suite passed** — 626 workspace tests, 92 Tier-1, 8 JWT, 5 Tier-2, zero failures. The swap would make `DEFAULT=2 MAX=900` resolve to a cap of 2 instead of 900, and `validate_env_config` accepts it. Nothing catches it because the unit tests drive the name-agnostic pure resolver and the single `from_env` assertion runs with both vars unset, where a swap is invisible.
- **Chose:** Two-part fix. Now: replace the positional `Option<String>` pair with a named struct so the call site must write the field name beside the env-var name. **Correction — this does NOT make the error unrepresentable, as first claimed.** The fixer demonstrated the residual: swapping the field initialisers is now a semantic no-op (field order carries no meaning), but transposing the env-var *name strings* while leaving the field names in place still compiles and still passes all 626 workspace and 105 integration tests. The struct converts an invisible positional hazard into a visible name/name mismatch in adjacent text; it does not eliminate the class. **The gap is closed only by the Phase 3 end-to-end test**, which is why a pointer comment sits at the call site. Later (recorded as a hard requirement in the Phase 3 section): Tier-1 coverage via `server_manager::start_with_env` asserting a distinctive non-default default page size and a **different** distinctive max, since identical values would not detect a swap.
- **Alternatives:** Rely on the named struct alone — rejected, and the demonstration above is why: it does not even prevent the mistake, only makes it more visible to a reader. Or add the Tier-1 test in Phase 2 — impossible, nothing reads the fields until the handler exists.
- **Blast radius if wrong:** High if it were to regress and go unnoticed — a silently wrong page cap is invisible to clients that ignore `next_page_token`, which is the exact failure mode this whole feature exists to fix. Low now, given the named struct plus the pending end-to-end test.
- **Status:** CONFIRMED (2026-08-30) — gap closed. Independently re-derived rather than taken from the phase
log: `page_size_above_max_is_clamped` sets D=2/M=3, so correct wiring resolves to `(2, 3)` and a transposition
to `(2, 2)`; the `page_size=1000` assertion then sees 2 where it expects 3 and fails. The 7/900 tests are
transposition-blind, as recorded. Residual: retuning that one test to reuse 7/900 would silently erase the
proof — the doc comment explaining the choice of 2 and 3 is the guard.
## CLAUDE.md is gitignored, so its env-table copy cannot be kept in sync by a PR
- **Plan:** `plans/list-sessions-pagination.md` (Phase 5, docs + env-table sync)
- **Assumed:** The plan treats `CLAUDE.md` as one of four mirrored env-var tables to be updated "in one commit so they cannot drift." It is **gitignored** (`.gitignore:20`) and `git log --all -- CLAUDE.md` is empty — it has never been tracked in this repository. Its edits are real on disk but will not appear in the PR diff and will not reach anyone who clones the repo.
- **Chose:** Updated it anyway (it is the file agents in this working copy actually read, so a stale copy here misleads every future session), and recorded the limitation rather than changing repo policy. **Did not un-ignore it** — adding a ~370-line instruction file to the tracked tree is a visible repo-policy decision outside this feature's scope, and it may be ignored deliberately.
- **Alternatives:** Remove `.gitignore:20` and commit `CLAUDE.md` so the fourth mirror actually ships (makes the anti-drift guarantee real, but is a repo-policy change the owner should make deliberately); or stop treating it as a mirror and delete the env table from it (loses the local reference).
- **Blast radius if wrong:** Low but persistent. Three tracked tables stay in sync; the fourth silently diverges for every clone. A future contributor reading a fresh checkout's `CLAUDE.md` — if they create one — would not see the two page-size variables at all.
- **Status:** CONFIRMED (2026-08-30) — repo owner's call: `CLAUDE.md` **stays gitignored**. Accepted
consequence: three tracked env-var tables stay in sync and the local copy silently diverges for anyone
cloning fresh. Not revisited unless the file is ever tracked.
## Quorum `threshold.value = 0` means two different things in the two layers
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 3, quorum threshold unification — **carved out of the phase's own differential-matrix criterion**)
- **Assumed:** Both layers gate the policy threshold on `value > 0.0` (`QuorumMode::effective_threshold`, `evaluate_quorum_commitment_outcome`) and then diverge *outside* that gate: the mode falls back to the ApprovalRequest's `required_approvals`, while the evaluator applies no approval bar at all. So a policy with `threshold.value: 0` bound to a session whose ApprovalRequest asked for 3 approvals gates `commitment_ready` at 3 but passes the evaluator at 0. The ceil-and-floor fix lives *inside* the gate and does not touch this.
- **Chose:** Left it, and carved `value = 0` out of the differential matrix explicitly (the test names the carve-out and says why, so nobody widens the grid and then "fixes" the failure by weakening the assertion). The two fallbacks are arguably both correct for their own layer — the mode has a request to fall back to and the evaluator does not, and an inert rule *should* mean "the mode's built-in rule stands". Unifying it means deciding what an absent policy bar means, which is a semantics question for RFC-MACP-0012, not a rounding bug.
- **Alternatives:** Pass `required_approvals` into the evaluator so both layers use the same fallback (changes a public signature and makes the evaluator depend on mode state); or treat `value: 0` as an explicit "no approvals required" bar in both layers (the degenerate case the floor-to-1 fix exists to make unreachable — strictly worse).
- **Blast radius if wrong:** Low and bounded in the safe direction. The mode is the stricter of the two: it will not call the evaluator until its own bar is met, so the evaluator's laxer reading can only fail to add a constraint, never remove one. It cannot produce a commitment the mode would have refused.
- **Status:** UNCONFIRMED (2026-09-10)
## The quorum mode silently tolerates a rules object the evaluator rejects
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 3 — named in the phase's edge cases as explicitly NOT in scope to unify)
- **Assumed:** `QuorumMode::effective_threshold` parses the bound policy's `rules` with `serde_json::from_value(...).unwrap_or_default()`, so a rules object that fails to deserialize (a type error — `"threshold": "majority"`) yields the schema defaults and an inert threshold, and the mode proceeds on the ApprovalRequest's `required_approvals`. The evaluator's `parse_rules` on the same object **denies the commitment**. A session can therefore be "ready to commit" by the mode's reckoning and then refused with `POLICY_DENIED` at the last step.
- **Chose:** Left both behaviours as they are. The refusal is fail-closed and nothing can be sealed on a policy neither layer understood, so the defect is a confusing error, not a governance hole — and after Phase 2 (registration-time shape validation) and Phase 3 (wildcard policies now validated against every mode's schema) the only way to reach it is a directly-constructed `PolicyDefinition` or a policy file edited under a running session.
- **Alternatives:** Make the mode propagate the parse error (`MacpError::InvalidModeState`-ish) so the failure surfaces at the first message rather than at commitment — better feedback, but it converts what is today a late refusal into an early one for every message in the session, which is a wider wire-visible change than the rounding fix warrants.
- **Blast radius if wrong:** Low. No commitment seals; the operator sees `POLICY_DENIED` where `INVALID_PAYLOAD` would have been clearer.
- **Status:** UNCONFIRMED (2026-09-10)
## A zero-ballot quorum decline is refused even though RFC-MACP-0011 §4a permits it
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 3, acceptance criterion 6 — "close it or move it to Blocked; do not leave it silent")
- **Assumed:** §4a makes a session eligible for a negative `Commitment` when `remaining_eligible + approvals < required_approvals`. Read literally, that is satisfied with **no ballot cast at all** whenever the bar exceeds the participant pool: `0 + N < required`. The runtime already refuses `required_approvals > participants` on the `ApprovalRequest` itself, so the only way to reach that state is a *policy* threshold, which §6 says replaces `required_approvals` but which nothing held to the same domain. The effect was that a coordinator could seal a binding `quorum.rejected` before anyone voted.
- **Chose:** Two guards, both narrower than they look. (1) The `ApprovalRequest` is refused when the effective threshold falls outside `1..=participants` — the domain the field it *replaces* already had to satisfy. (2) `commitment_ready`'s unreachable-threshold branch additionally requires at least one ballot, covering the replay path where an edited policy rebinds a larger threshold to a session whose request was already accepted. Guard (2) is a deliberate deviation from the literal §4a formula, and it is provably confined to the misconfigured case: with zero ballots the formula reduces to `participants < required`, which is unreachable once guard (1) holds. Every §4b scenario the RFC actually describes (all-abstain, or abstentions plus rejections) has at least one ballot behind it, and both are covered by tests.
- **Alternatives:** Clamp the effective threshold to `participants.len()` in both layers — rejected: it silently *loosens* a governance bar (an operator's "5 approvals" becomes 3), which is the wrong direction for a fail-closed runtime. Or refuse the policy at `SessionStart` instead of at `ApprovalRequest` — blames the right party but is a larger wire-visible change touching `runtime.rs`, and the participant count is already known at request time. Or leave it and record it — rejected because a binding negative commitment with no participation is exactly the "confident wrong answer" the issue reports.
- **Blast radius if wrong:** Moderate, in the fail-closed direction. A deployment whose quorum policy sets a threshold above a session's participant count now gets `INVALID_PAYLOAD` on the `ApprovalRequest` instead of a session that could only ever decline. No such policy can produce a positive outcome, so nothing that worked stops working — but the failure moved from "declines" to "refuses", which is visible. Reversal is a small edit to one match arm.
- **Status:** UNCONFIRMED (2026-09-10)
## Wildcard (`mode: "*"`) policies are now held to every mode's schema
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 3 — but **no acceptance criterion of Phase 3 covers this**. An earlier revision of this entry cited "Phase 3, item 4"; that is wrong, Phase 3's criterion 4 is the differential `{type} × {value} × {participant count}` threshold matrix. The wildcard fail-open reached Phase 3 only through the Phase 2 pinning test's own comment — `quorum_threshold_constraints_apply_to_wildcard_policies` in `crates/macp-policy/src/registry.rs`, renamed from `..._do_not_apply_to_...`, which pinned the fail-open and named Phase 3 as the place to close it. Phase 3's Files list records the resulting registry edit as a justified scope expansion, not as a planned criterion.)
- **Assumed:** `validate_rules_for_mode("*")` deserialized against `DecisionPolicyRules` only, calling it a "superset" it is not: it has no top-level `threshold`, and with no `deny_unknown_fields` a Quorum `threshold` inside a `"*"` policy was silently dropped. `validate_conditional_constraints` then gated the quorum checks on an exact mode match, and `Runtime::handle_session_start` bound the policy to a quorum session anyway — so `effective_threshold` read a value no layer had validated. This was plan option (a): validate against every mode's schema.
- **Chose:** Option (a). A `"*"` policy binds to every mode's sessions and every mode's evaluator re-parses the same rules through its own struct, so holding it to all five schemas and all their constraints is the only reading of `"*"` that is not a hole. In practice it is also the smallest change: `validate_conditional_constraints` has exactly three families (decision, quorum, all-mode commitment) and the decision one already ran for `"*"`, so option (b) — "validate the quorum block whenever `threshold` is present" — would have been the same behaviour with a narrower justification.
- **Alternatives:** Option (c), leave it open now that the floor-to-1 defuses the severe case, and document it. Rejected: the fail-open is a hole through *every* quorum constraint Phase 2 added, not just the rounding one, and "use `mode: "macp.mode.quorum.v1"`, not `"*"`" is documentation asking operators to avoid a trap rather than removing it.
- **Blast radius if wrong:** Low, and the plan's stated worry ("may refuse policies that register today") did not materialise in any test. Since no struct uses `deny_unknown_fields`, a field one mode's schema does not know is still ignored rather than refused — the built-in `policy.default`'s Decision-shaped rules register unchanged, and the only new refusals are values that are out-of-domain for the mode that owns them. What *would* break is a wildcard deliberately carrying a knowingly-invalid block for a mode it never expected to be used with.
- **Status:** UNCONFIRMED (2026-09-10)
## `EffectiveThreshold` is deliberately not `#[non_exhaustive]`
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 3, quorum threshold unification — the new `macp-core` type the shared resolver introduced)
- **Assumed:** Every other public enum and public-field struct `macp-core` exposes across a crate boundary carries `#[non_exhaustive]` *and* a rustdoc sentence giving its reason — six of them: `mode.rs:9` (`ModeResponse`), `mode.rs:32` (`MessageContext`), `session.rs:54` (`Session`), `policy/mod.rs:24` (`PolicyDecision`), `policy/mod.rs:118` (`CommitmentMode`), `error.rs:3` (`MacpError`). `EffectiveThreshold` departs from all six and, until now, said nothing about the departure — so a reader could only read the omission as an oversight.
- **Chose:** Keep it exhaustive, and document why in the enum's own rustdoc rather than only here. `#[non_exhaustive]` binds every crate *except* the defining one, and this enum has exactly two consumers, both outside `macp-core`: `QuorumMode::effective_threshold` (`crates/macp-modes/src/mode/quorum.rs:92-94`) and `evaluate_quorum_commitment_outcome` (`crates/macp-policy/src/evaluator.rs:712-725`). Their compile-time exhaustiveness **is** the guarantee the whole #145 fix buys — one rule, one interpretation, provable at build time. Adding the attribute would force a `_` arm at both, and a fail-closed `_` arm is strictly worse for a governance kernel than a build failure: a future variant would silently decline instead of failing to compile, which is the same class of silent mis-handling as #145 itself (a `_` arm reinterpreting `weighted` as a raw approval count is precisely what that fix deleted).
- **Alternatives:** Add `#[non_exhaustive]` for consistency with the other six and accept fail-closed `_` arms (rejected above); or add it and have both call sites `panic!`/`unreachable!` in the `_` arm to recover the loudness (trades a compile error for a runtime abort in a kernel that must not panic on policy data); or leave it exhaustive and undocumented (the status quo this entry exists to end — an unexplained departure from six documented precedents reads as an oversight and invites someone to "fix" it).
- **Blast radius if wrong:** Low and bounded to release mechanics. Adding a variant later is a breaking change for downstream matches, but it is **not** a silent one: `enum_variant_added` is a major `cargo-semver-checks` lint and `release-plz.toml:20` sets `semver_check = true`, so it blocks the release PR rather than shipping. The residual cost is release coordination (a deliberate minor bump on 0.x, with both in-tree call sites updated in the same commit), not an undetected break. If the enum ever grows a third consumer outside this workspace, revisit — the trade is sound while every consumer is in-tree.
- **Status:** UNCONFIRMED (2026-09-11)
## A negative weighted total fails the round, which moves one decline from DENY to ALLOW
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 4, the negative-weight evaluator hole — **the phase's stated premise was wrong against the code; see below**)
- **Assumed:** The plan describes the defect as "`evaluator.rs:388-390` returns `VotingResult::NoVotes` when `weighted_total == 0.0`; change it to `< 0.0`", and its criteria 1-3 use `weights: {"a": 1.0, "b": -1.0}`. Both are wrong arithmetically. That map sums to **exactly 0.0** with both agents voting, i.e. the schema-legal case the plan itself defers to spec issue #98 item 3. And a *negative* total never matched `== 0.0` at all — it fell straight through to `weighted_approve / weighted_total`, where a **negative denominator inverts** `ratio >= threshold`. Demonstrated: `{fraud: 1.0, growth: -2.0}` with fraud REJECTing and growth APPROVing gave `-2.0 / -1.0 = 2.0 >= 0.5` → `Passed` — a positive commitment **allowed** on a reject from the only voter whose weight is in-schema. The hole was an inverted comparison, not a "no votes" misreport.
- **Chose:** Ship the `< 0.0 => Failed` short-circuit as planned (the fix is right even though the diagnosis was not), and correct the account of it in the code, the `evaluate_decision_commitment_outcome` rustdoc table, and the tests. Kept the `== 0.0 => NoVotes` branch untouched, deferred to spec #98 item 3. Wrote the criteria-1-3 tests around a genuinely negative map (`{fraud: 1.0, growth: -2.0}`) and asserted the `VotingResult` variant directly via `check_voting_algorithm`, not only the `PolicyDecision` — necessary, because two of the three rounds were already denied before the change and a `PolicyDecision`-only assertion would have passed against the unfixed code. Every new test was mutation-checked: removing the branch reddens exactly the three negative tests, removing the `== 0.0` branch reddens only the zero test, removing the `:337` short-circuit reddens the guard test plus the two §4.1 tests it protects.
- **Alternatives:** Refuse the round with an error rather than `Failed` (out of scope — `check_voting_algorithm` has no error channel and every caller treats its result as a governance outcome); or clamp the total to zero and fall into `NoVotes` (keeps the decline direction unchanged but re-launders out-of-schema data as "nobody voted", the same silent reinterpretation the #145 fix removed).
- **Blast radius if wrong:** Bounded to out-of-schema policies — `voting.weights[*]` is `minimum: 0`, so registration already refuses these and only a directly-constructed `PolicyDefinition` reaches the arm. **Not purely a tightening.** In the approve direction it is one (`Passed` → `Failed`: a positive commitment that used to seal is now denied). In the decline direction it is a fail-open: on that same round a decline moves **DENY → ALLOW**, because a decline over `Passed` was refused while a decline over `Failed` is permitted once the universal reject-floor (`reject_count > 0`) is met. That is the intended outcome — the round is genuinely decided and an explicit reject backs the decline — but RFC-MACP-0012 §4.1's no-result branch is conditioned on no decisive vote having been *cast*, which is false here, so this **fills a gap §4.1 does not address** rather than moving toward conformance. It must be stated as a direction change in the release notes, not as a tightening.
- **Status:** UNCONFIRMED (2026-09-11)
## `supermajority` silently substitutes 2/3 for an out-of-domain threshold
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 4 edge cases — "Leave it; record it in `ASSUMPTIONS.md`")
- **Assumed:** `check_voting_algorithm`'s `supermajority` arm computes `if threshold > 0.5 { threshold } else { 2.0 / 3.0 }`. A policy asking for a 40% supermajority is therefore silently *raised* to 66.7% rather than refused, and the reported reason names the substituted value with no indication that it is not what the policy said. Unreachable through the registry: `registry.rs:445` refuses `supermajority` with `threshold <= 0.5` at registration, so only a directly-constructed `PolicyDefinition` reaches it.
- **Chose:** Left it. Not touched in Phase 4, which is confined to the weighted arm. The substitution fails **closed** (it can only raise the bar, never lower it), the reachable path is already refused at the door, and changing a silent substitution into a refusal inside the evaluator would move a policy-authoring error from commitment time to a place with no error channel — `check_voting_algorithm` returns a governance outcome, not a `Result`.
- **Alternatives:** Return `Failed` with an "out-of-domain threshold" reason (fails closed and is honest, but converts a conservative bar into a hard block for any consumer driving `macp-core` + `macp-modes` with its own evaluator and no registry); or delete the clamp and use `threshold` as given (fails **open** — a 40% "supermajority" — strictly worse).
- **Blast radius if wrong:** Very low. One unreachable branch whose only effect is a stricter bar than requested, and a reason string that under-explains itself.
- **Status:** UNCONFIRMED (2026-09-11)
## `unanimous` passes by vacuous truth on an empty participant list
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 4 edge cases — "record, do not fix here")
- **Assumed:** The `unanimous` arm's predicate is `participants.iter().all(|p| ...voted APPROVE)`, which is `true` for an empty slice. With at least one non-abstain ballot (needed to clear the `:337` short-circuit) and no REJECT among them, a zero-participant unanimous round returns `Passed`. Unreachable through the server: strict `SessionStart` requires a non-empty `participants` list for every standards-track mode, so an empty slice only arrives from a direct library call.
- **Chose:** Left it. Fixing it means deciding what `unanimous` means on an empty tally, which is spec issue #98 item 1 (RFC-MACP-0012 §4.1) and blocked — the same blocker that holds issue #147. Phase 4 deliberately did not touch the `unanimous` arm, and a local fix here would pre-empt the amendment and contradict the two §4.1 tests the phase is required to leave green (`all_abstain_returns_no_votes`, `no_decisive_votes_blocks_a_positive_commitment_only_under_require_vote_quorum`).
- **Alternatives:** Return `Failed` when `participants.is_empty()` (the honest reading, but it is exactly the §4.1 change #98 must ratify first); or assert non-empty participants at the function boundary (moves an unreachable case into a panic in a kernel that must not panic on policy data).
- **Blast radius if wrong:** Very low while unreachable from the wire. It becomes live for any consumer that drives the evaluator directly with no participants, and it will be revisited as part of the blocked #147 work rather than in isolation.
- **Status:** UNCONFIRMED (2026-09-11)
## The public effective-threshold accessor answers three questions in three layers, not one `Option`
- **Plan:** `plans/backlog-closeout-2026-09.md` (Phase 5, publish the corrected effective threshold — the plan's own signature suggestion had to be rejected; see below)
- **Assumed:** The plan specifies `effective_threshold_for_session(&Session) -> Option<u32>` with `None` meaning "no accepted `ApprovalRequest`". After Phase 3 the inner function already returned `Option<u32>` with `None` meaning **unsatisfiable threshold**, so that signature would have collapsed two unrelated answers — "this session has no request yet" and "this policy can never be satisfied" — into one value, which is the single thing a caller reading a governance bar cannot afford. Decoding `session.mode_state` adds a third outcome the plan does not mention at all: the state may not decode.
- **Chose:** `pub fn effective_threshold_for_session(&Session) -> Result<Option<ApprovalThreshold>, MacpError>` over a new two-variant `pub enum ApprovalThreshold { Approvals(u32), Unsatisfiable }`, with the request-level `pub fn effective_threshold(&Session, &ApprovalRequestRecord) -> ApprovalThreshold` sharing that enum. One layer per question, and no representable-but-impossible state: `Err` = undecodable state (which also catches a session of another mode, since `QuorumState`'s fields are not `#[serde(default)]`), `Ok(None)` = no request accepted, `Ok(Some(_))` = the resolved bar. The policy's `Inert` case is resolved *before* it reaches the caller, to `Approvals(request.required_approvals)`, because handing back `EffectiveThreshold::Inert` would hand back the fallback rule and re-create the 15-line mirror issue #146 exists to delete. `ApprovalThreshold` is deliberately exhaustive, for the reason already recorded for `EffectiveThreshold` above.
- **Alternatives:** `Option<EffectiveThreshold>` (the plan's second option) — rejected twice over: it widens `macp-core`'s `EffectiveThreshold` exposure into a second crate's public API, and its inner `Inert` variant would be unreachable through this path, i.e. an impossible state the caller must still match. A flat three-variant enum with a `NoRequest` member — rejected because the request-level form then carries a variant it can never return, and because `Option::map` is exactly the composition the two functions have. `Result<u32, Reason>` — rejected: it makes the ordinary "no request yet" case an error, and folds two non-error outcomes into an error channel. Keeping `effective_threshold` private and documenting `decode_mode_state` as the route — rejected: that is the reconstruct-the-internals path the issue asks to remove.
- **Blast radius if wrong:** Additive only — `cargo semver-checks check-release` on `macp-core`, `macp-modes` and `macp-runtime` is clean (exit 0, no major lints). The cost of being wrong is API churn: narrowing `Result<Option<_>, _>` later, or adding an `ApprovalThreshold` variant, is a breaking change, though `enum_variant_added` and signature lints are majors that `release-plz.toml`'s `semver_check = true` blocks on rather than shipping silently. If a third outcome for the session-level form ever appears, it belongs in a new `Ok` variant, not in a new error.
- **Status:** UNCONFIRMED (2026-09-11)