Skip to content

fix(usage-gate): fail closed when usage cannot be read - #274

Merged
thedavidmeister merged 2 commits into
mainfrom
2026-08-11-issue-273-usage-gate-fail-closed
Aug 12, 2026
Merged

fix(usage-gate): fail closed when usage cannot be read#274
thedavidmeister merged 2 commits into
mainfrom
2026-08-11-issue-273-usage-gate-fail-closed

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Closes #273

The usage-gate's None-reading arm now returns UsageVerdict::Pause (exit 10 → one skip row, tick skipped) instead of Run. The pause line — PAUSE: no usage reading — endpoint unreachable and no fallback reading set — cannot pace blind, failing closed — shares no wording with the pace or ceiling pauses, so a blind pause is diagnosable as an endpoint problem from a single campaign.log / review.log / skip-row line. --force remains the operator's one-tick way past it, like any other POLICY pause; the config REFUSAL path (exit 2) is untouched.

No shell logic changes: the runners already branch on exit 10 and record the gate's own line verbatim on the skip row. Prose describing the retired inert behavior is rewritten to the fail-closed behavior in campaign-run.sh, review-run.sh, cron.env.example, the gate's doc comments (usage_gate_decide, oauth_token, fallback_reading, fetch_usage, the #97 five-hour rationale) and the usage_gate_skip.rs preamble.

Motivating incident: 2026-08-10T23:00Z, the only scheduled tick in 24h that executed did so at 39% used against a ~21% linear pace — every measurable tick that night paused, and the one tick the gate could not measure ran.

QA

  • Discriminating tests: no_reading_fails_closed_and_pauses (replaces no_reading_is_inert_and_runs) — fails on base, verified by restoring the base's exact None arm as mutant 1 below: the mutant IS the pre-change code, and the test fails against it exactly as it would against a base checkout.
  • Mutations applied: (1) usage_gate_decide None arm → UsageVerdict::Pause(...) reverted to the base's UsageVerdict::Run("OK: … gate inert") → killed: no_reading_fails_closed_and_pauses FAILED, exit 101, panic at the assert_eq!(v.code(), 10) assertion, 1 test run / 1277 filtered. (2) Reason garbled to UsageVerdict::Pause("PAUSE: x") (right verdict, wording gone) → killed: same test FAILED, exit 101, the PAUSE:-prefix assert passes and the contains("no usage reading") reason assert fires — the test pins the diagnosable wording, not just the exit code. Both mutants reverted; full suite green on the unmutated head: cargo test exit 0, incl. all 24 usage_gate_skip integration tests and 17 usage_gate_tests unit tests.
  • Oracle: expected verdict comes from the usage-gate fails open when it cannot read usage — must fail closed #273 ruling (unreadable usage ⇒ pause), the exit-10 contract from UsageVerdict::code()'s documented runner interface, and the reason-shape requirements from the issue's observability acceptance — none recomputed from the implementation.
  • Category check: issue names the inert arm, the enshrined unit test, and every comment describing inert behavior (main.rs, campaign-run.sh, review-run.sh); covered all three plus the two prose sites the issue's category implies but does not name (cron.env.example fallback block, usage_gate_skip.rs preamble — found by a repo-wide sweep for the inert wording). unreadable_usage_shapes_never_produce_a_reading already pins that malformed/expired/unparseable shapes collapse to None, so the single flipped arm covers the whole failure category.

Summary by CodeRabbit

  • Bug Fixes

    • Usage checks now pause campaign and review runs when usage data cannot be read and no fallback is configured.
    • Runs no longer proceed while usage status is unknown.
    • A single --force invocation can bypass an exit-10 pause.
    • Other non-zero usage-gate results continue to be treated as configuration refusals.
  • Documentation

    • Updated guidance to clarify unreadable usage endpoints, fallback behavior, pause conditions, and forced runs.

An unreadable endpoint with no fallback reading now pauses the tick (exit
10, one skip row) instead of running it. The pause reason — 'PAUSE: no
usage reading — endpoint unreachable and no fallback reading set — cannot
pace blind, failing closed' — shares no wording with the pace or ceiling
pauses, so a blind pause is diagnosable from a single log or skip-row
line. --force remains the operator's one-tick way past it, like any other
POLICY pause.

Prose describing the retired inert behavior is updated in the runners,
cron.env.example, the gate's doc comments and the integration-test
preamble; the unit test flips to pin the Pause verdict and its reason
shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Aug 11, 2026
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ceb1e102-1535-40e4-9b97-0f61e89e33d6

📥 Commits

Reviewing files that changed from the base of the PR and between c87278d and a8d906e.

📒 Files selected for processing (1)
  • pr-review-report-rs/src/main.rs

Walkthrough

The usage gate now fails closed when usage cannot be read and no fallback exists. Rust logic returns a diagnostic pause, runner scripts preserve the pause, and documentation and tests describe the behavior.

Changes

Usage gate fail-closed behavior

Layer / File(s) Summary
Usage gate decision and validation
pr-review-report-rs/src/main.rs
Missing usage readings now return a pause verdict with a distinct endpoint/read-failure reason. Related fallback, fetch, credential, and test documentation reflects the fail-closed path.
Runner pause handling and operational coverage
campaign-run.sh, review-run.sh, cron.env.example, pr-review-report-rs/tests/usage_gate_skip.rs
Runner scripts pause when usage is unreadable without a fallback. Cron and forced-pause test documentation describes the updated behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

Suggested labels: ai:ready

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making the usage gate fail closed when usage cannot be read.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-08-11-issue-273-usage-gate-fail-closed

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.45.1)
pr-review-report-rs/src/main.rs

ast-grep timed out on this file


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pr-review-report-rs/src/main.rs`:
- Around line 10322-10332: Update no_reading_fails_closed_and_pauses to assert
that usage_gate_decide returns the UsageVerdict::Pause variant and compare its
complete reason string against the specified exact pause reason, replacing the
current starts_with and contains assertions while preserving the existing code
check.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d122464a-dee7-4e9e-89d3-c84cb335204f

📥 Commits

Reviewing files that changed from the base of the PR and between 08a7fbc and c87278d.

📒 Files selected for processing (5)
  • campaign-run.sh
  • cron.env.example
  • pr-review-report-rs/src/main.rs
  • pr-review-report-rs/tests/usage_gate_skip.rs
  • review-run.sh

Comment thread pr-review-report-rs/src/main.rs
CodeRabbit on #274: fragment asserts let wording drift past the test and
never proved the variant. Assert UsageVerdict::Pause and the exact reason.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed a8d906e: ready — single behavioral change is the None-reading arm of usage_gate_decide flipping Run→Pause (exit 10) per #273; shell/config/doc changes are prose-only; all unreadable shapes collapse to the one pause; mutation evidence in QA kills both the base-arm revert and a garbled reason; suite green in-build at this head; CodeRabbit thread addressed and resolved; CI fully green.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed a8d906e: ready — single behavioral change is the None-reading arm of usage_gate_decide flipping Run→Pause (exit 10) per #273; shell/config/doc changes are prose-only; all unreadable shapes collapse to the one pause; mutation evidence in QA kills both the base-arm revert and a garbled reason; suite green in-build at this head; CodeRabbit thread addressed and resolved; CI fully green.
Rulings-conformance: checked against the CLAUDE.md invariants and the standing rulings for this work. (1) #273 ruling — unreadable usage MUST pause, never run: the None arm now returns UsageVerdict::Pause; the old fail-open precedent is rewritten out of every doc it lived in, not kept alongside. (2) Force overrides policy, never correctness — the blind pause is exit 10, the one exit --force overrides; the config-REFUSAL path (exit 2) and the flock stay unforceable, and usage_gate_skip.rs pins that pairing. (3) Comments describe current behavior only — all rewritten prose states the fail-closed behavior as-is; the 22-tick stall incident survives as rationale, not process narrative. (4) Adversarial-mutation on every code PR — QA block carries two killed mutants including the exact base-arm revert; the CodeRabbit thread is resolved with the test strengthened to pin the full reason string. (5) No FSM surface is touched — no label, transition, or human:* semantics change; the artifact is gate-internal plus prose.

@thedavidmeister
thedavidmeister merged commit a20a120 into main Aug 12, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

usage-gate fails open when it cannot read usage — must fail closed

1 participant