Retire the fake l0/l1 sensor + auth pre-flight (close loop loose ends)#89
Conversation
Follow-up to #88 (loop closure). Closes the mechanical loose ends. - Retire the fake l0/l1 audit hooks to no-op deprecation stubs: l0 logged latency_ms (100% null), l1 read tool_call_count (100% zero) then grepped the agent's own prose (h correlated with U). Stubs drain stdin + exit 0 so any pre-v0.30.0 wiring gets a clean exit, not a missing-file error, and write no more fake rows. A future release deletes them. - Re-source doctor §16 (L0) + §17 (L1) from the leverage-sensor's .control/leverage-state.json (m2/m3/m4, m1) instead of the retired logs — real transcript-derived numbers, no misleading "wire the hook" nudge. §18 (L2 promotions) unchanged; compute-lambda reads static parameters.toml, so it is unaffected. - Add auth-preflight-hook.sh (SessionStart): warn (never block) if gh is unauthenticated, so an autonomous arc learns it up front instead of dying on the push/PR step after the work is done. Minimal (gh only) per rule-of-three. - tests/loop-retirement.test.sh: stubs are silent no-ops writing no audit rows; doctor dropped the L0_LOG/L1_LOG reads; auth pre-flight warns-without-blocking. Deferred (tracked): m4 allowlist (needs the settings merger extended beyond .hooks + a posture decision; real m4 is 0.02/session, not an active problem), Read-before-Edit guard (redundant with CC's native guard), dither (BRO-1698). Empirical (P11): full tests/*.test.sh + canary green; doctor self-check 99/99; dogfooded vs real ~/broomva (§16/§17 render live metrics). VERSION 0.30.0→0.31.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRetires the L0/L1 fake audit hook scripts into no-op deprecation stubs, updates doctor.sh to re-source L0/L1 reporting from leverage-state.json, adds a non-blocking gh auth pre-flight hook wired into SessionStart, and adds regression tests plus changelog/docs/VERSION updates. ChangesLoop Retirement and Auth Pre-flight
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant doctor_sh as doctor.sh
participant LeverageState as leverage-state.json
User->>doctor_sh: run bstack doctor
doctor_sh->>LeverageState: read metrics for L0/L1
LeverageState-->>doctor_sh: m1/m2/m3/m4 values
doctor_sh-->>User: print report or informational message
sequenceDiagram
participant SessionStart
participant AuthPreflightHook as auth-preflight-hook.sh
participant GH as gh CLI
SessionStart->>AuthPreflightHook: invoke on session start
AuthPreflightHook->>GH: check auth status
GH-->>AuthPreflightHook: authenticated or not
AuthPreflightHook-->>SessionStart: warn if unauthenticated, always exit 0
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/doctor.sh (1)
806-816: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated python-heredoc pattern across §16/§17.
Both sections use nearly identical
python3 - "$LEV_STATE" <<'PYEOF' ... except Exception: print("unreadable")boilerplate to load and parse the same JSON file. Could be factored into a single helper that loads once and prints per-section fields, but given this is informational doctor output, the duplication is low risk.Also applies to: 832-843
🤖 Prompt for 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. In `@scripts/doctor.sh` around lines 806 - 816, The Python heredoc used to read and print metrics in the doctor output is duplicated across the L0 summary blocks, so consolidate the repeated JSON-loading logic into a single reusable helper and call it from the existing summary sections. Update the shell script around the L0_SUMMARY/L1 summary generation to use that shared helper while keeping the same output format and the existing unreadable fallback behavior.tests/loop-retirement.test.sh (1)
42-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAuth pre-flight test doesn't verify the warning message content, only the wiring/exit code.
Line 42 only checks that the hook exits 0 regardless of
ghstate; it never asserts the actual warning text appears whenghis unauthenticated (e.g. by mockingghinPATHto failgh auth status). This leaves the warning-message behavior itself untested, only the non-blocking exit contract.🤖 Prompt for 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. In `@tests/loop-retirement.test.sh` around lines 42 - 44, The auth pre-flight test only checks the exit code and hook wiring, but not the warning message content emitted by the auth-preflight path. Update the test in tests/loop-retirement.test.sh to simulate an unauthenticated gh by shadowing gh in PATH so gh auth status fails, then assert the expected warning text is printed while still exiting 0. Use the existing auth-preflight and SessionStart wiring checks as anchors, but make the test validate both the non-blocking behavior and the logged warning content.
🤖 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.
Nitpick comments:
In `@scripts/doctor.sh`:
- Around line 806-816: The Python heredoc used to read and print metrics in the
doctor output is duplicated across the L0 summary blocks, so consolidate the
repeated JSON-loading logic into a single reusable helper and call it from the
existing summary sections. Update the shell script around the L0_SUMMARY/L1
summary generation to use that shared helper while keeping the same output
format and the existing unreadable fallback behavior.
In `@tests/loop-retirement.test.sh`:
- Around line 42-44: The auth pre-flight test only checks the exit code and hook
wiring, but not the warning message content emitted by the auth-preflight path.
Update the test in tests/loop-retirement.test.sh to simulate an unauthenticated
gh by shadowing gh in PATH so gh auth status fails, then assert the expected
warning text is printed while still exiting 0. Use the existing auth-preflight
and SessionStart wiring checks as anchors, but make the test validate both the
non-blocking behavior and the logged warning content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee285fa9-5014-4caa-bad1-2ea2a4fbe820
📒 Files selected for processing (9)
CHANGELOG.mdVERSIONassets/templates/settings.json.snippetreferences/new-workspace-flow.mdscripts/auth-preflight-hook.shscripts/doctor.shscripts/l0-tool-audit-hook.shscripts/l1-reflex-audit-hook.shtests/loop-retirement.test.sh
… BRO-1699) P20 review of this PR flagged that scripts/compute-budget-status.sh still reads the now-retired l0-tools/l1-reflexes logs for doctor §19's L0/L1 observed overlay — which read empty after the stubs, so §19's L0/L1 default to `stable` forever. Non-blocking (graceful, exit 0, nothing in CI invokes it, composite ω from static parameters.toml is unaffected, real L0/L1 signal is in §16/§17/§23) — but leaving it silent in a loop-closure PR is the blind-checker smell this work exists to kill. - compute-budget-status.sh: comment the retired-log reads + point at BRO-1699. - doctor.sh §16/§17: fix version attribution (re-source landed v0.31.0; sensor v0.30.0). - new-workspace-flow.md + CHANGELOG: disclose the §19 adjacency + BRO-1699 follow-up. No logic change (comments + docs). Repoint tracked as BRO-1699. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Follow-up to #88 (self-improvement loop closure, v0.30.0). Closes the mechanical loose ends noted at that merge. BRO-1697.
Retire the fake l0/l1 sensor
The
l0-tool-audit/l1-reflex-audithooks were left on disk (unwired) by #88. They logged fields Claude Code never emits (latency_ms→ 100% null;tool_call_count→ 100% zero, then grepped the agent's own prose =hcorrelated withU). Now they are no-op deprecation stubs: drain stdin, exit 0, write nothing. Any workspace still wiring them from a pre-v0.30.0 install gets a clean exit instead of a missing-file error; a future release deletes them.Re-source doctor §16 / §17
Instead of reading the retired logs (which would now show a misleading "wire the hook" nudge forever), §16 (L0 plant) and §17 (L1 reflex) now read straight from the leverage-sensor's
.control/leverage-state.json:§18 (L2 promotions, reads
l2-promotions.jsonl) is unchanged.compute-lambdaderives λ from staticrcs-parameters.toml, never the audit logs — so it was never affected (my #88 note overstated this; corrected here).Auth pre-flight
auth-preflight-hook.sh(SessionStart) warns — never blocks — ifghis unauthenticated, so an autonomous arc learns it up front instead of dying on the push/PR step after the work is done. Minimal (gh only) per rule-of-three; silent when authed.Deferred (tracked, with reasons — not dropped)
permissions.allowdefault needs the bootstrap merger extended (it merges only.hookstoday) and changes the user's permission posture (a call best made with the user present). Real-workspace m4 is 0.02/session — far below the 0.50 target, so not an active problem. (BRO-1697)Validation (P11)
Full
tests/*.test.sh+ canary green (incl. newtests/loop-retirement.test.sh); doctor self-check 99/99 exit 0; dogfooded against real~/broomva(§16/§17 render live metrics; auth hook silent when authed). Primitive count unchanged (20).VERSION0.30.0 → 0.31.0.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores