docs(issues): record the five open follow-ups, and stop clean-worktree claiming "0 commits ahead" when it isn't - #2117
Merged
Merged
Conversation
…config teeth `.claude/hooks/session-start.sh` was committed as mode 100644 while both its siblings were 100755, and it is the only hook registered by bare path rather than through `bash`. Its whole body is gated on CLAUDE_CODE_REMOTE=true, so the one environment it does any work in is a Linux web container — exactly where a non-executable checkout cannot run. It is also the script that provisions the Node 24 the engine floor requires, after npm ci EBADENGINE blocked PRs #1611, #1697, #1705 and #1740. The defect was invisible locally: the primary workstation is a Windows ReFS Dev Drive with core.fileMode=false, so git ignores filesystem permission bits and a local `chmod +x` is a silent no-op. Only `git update-index --chmod=+x` can fix it. Fixed three ways so it cannot recur: the index mode, a `bash "..."` registration that stops the mode being load-bearing, and a contract test. Also in this change: - .claude/settings.json gains a permissions block. AGENTS.md's provider confirmation boundary was prose-only; this encodes it as deny/ask rules, including deny on reading .env* (a staging key leaked on 2026-08-18) and on the Supabase MCP write tools. The repo already learned that prose does not hold here — see the comment in pr-handoff-stop.sh. - check-base-freshness.mjs now emits its stale-base warning on stdout as hook JSON. Every human-readable branch used console.error, and Claude Code injects only stdout into context, so the tripwire never reached the agent. The origin/main fetch also gains a 10s timeout so a hung remote cannot burn the whole SessionStart budget. - clean-worktree.mjs gains list-only `--merged` and `--squashed`. Nothing reclaimed merged worktrees, so 49 accumulated, ~19 GB of duplicated node_modules on a 50 GB Dev Drive. Ancestor detection alone finds 2 of 49 because this repo squash-merges; the patch-id test finds 9. A `confidence:` line distinguishes proven from inferred, because the two are not the same claim and one candidate had 2 of 21 files still differing. - Explicit hook timeouts, a PreCompact hook that asks for /issues capture while the context still exists, and a push format guard that only fires where the .githooks pre-push guard is not wired. Removal stays a separate opt-in throughout; `runWorktreeCleanup()` is byte identical, so verify:preflight is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… run `tests/session-start-hook.test.ts` asserted that the written env file contains `join(home, ".node24", …)`. On Windows `home` comes from mkdtempSync(tmpdir()) as `C:\Users\…\AppData\Local\Temp\session-start-home-XXXX`, while the hook runs under Git Bash and writes the POSIX view of the same directory, `/tmp/ session-start-home-XXXX`. The assertion therefore failed on every Windows run regardless of the diff under test. That is worse than a red test: it is a red test everyone learns to ignore. It fails inside `npm run test`, which is the last step of `verify:pr-local`, so the whole gate goes red locally for every change and the only way to use it is to decide which failures do not count. Compare the path tail instead. The unique mkdtemp basename still pins the assertion to this test's own HOME, so it loses no strength, and it now holds on both platforms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…k answerable Closes the four items left open when PR #2113 was opened, each with the smallest fix that actually resolves it rather than restating it. 1. The claim that no `allow` rule can reach a provider-backed script was asserted in review and never measured. It is now a test: 36 provider-backed scripts, derived from script names rather than hand-listed, each asserted unreachable through `allow` and covered by an explicit `ask`. Mutation-checked — injecting a broad `Bash(npm run check:*)` allow rule turns 4 of them red. 2. Hook registrations are pinned to invoke through an interpreter rather than a bare path, and to carry an explicit timeout. Bare-path registration is what made the `session-start.sh` mode bug reachable; the mutation check confirms both guards bite. 3. The PreCompact hook's known limit could not be closed by reading code — the installed CLI ships a compiled binary with no inspectable bundle, so whether the platform injects its stdout into model context is not determinable here. Instead of leaving that permanently unverified, the hook now appends one line per firing to a log under the git dir. After the next compaction the log distinguishes "hook never ran" from "hook ran but its output went nowhere", and both answers are actionable. The log lives outside the worktree so it can never be staged. 4. The `newtask` skill said "~40 worktrees". It was 48 on 2026-08-18 and reached 50 during one session, so the number now carries the reason it drifts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…BigSimmo/Database into claude/code-setup-review-a95519
Each of these is blocked on something outside the repo, so capturing them is the smallest fix that resolves them — the alternative is that they die with the session context. - P1 rec: PR churn has exhausted both review bots' budgets, so PR #2113 landed with zero automated review and subsequent PRs will too. AGENTS.md already measured the CI half of this cost; this is the second bill and the worse one. Needs a decision on whether the bundling rule gets a gate. - P2 task: nine landed worktrees, ~4.5 GB on a 51%-full Dev Drive, deliberately not removed — re-verification immediately before deletion showed two held unmerged commits despite the scan reporting none minutes earlier, and a third had been switched branches mid-scan by a live session. - P3 task: confirm D:\.npm-cache is a registered Dev Drive trusted cache; fsutil needs elevation and the non-elevated registry fallback reads empty. - P3 task: read the PreCompact hook's own log after the next compaction to settle whether its output reaches model context. The log lives under the worktree's git dir, so check it before cleaning that worktree up. - P3 task: confirm on a real web session that session-start.sh now runs. The mode bug was proven; the failure it would cause on Linux was not, because no container was available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…head" when it isn't Every `--squashed` candidate printed "0 commits ahead" while being genuinely ahead of origin/main — by 11, 2, 1 commits on the real fleet. A squash-merged branch keeps its original commits forever, so it stays ahead permanently; what is zero is the count of UNLANDED commits, which is a different claim. The line was therefore stating something a reader could disprove with one `git rev-list`, on a tool whose entire job is to be trusted before a deletion. It now reports both numbers: "11 ahead of origin/main, 0 unlanded commits". Also corrects the comment above the ahead check, which described it as belt-and-braces against the merge test. That is true in ancestor mode, where it is the real gate. In squash mode it is not a second opinion at all — gitAheadUnlandedCount returns 0 for any branch the squash test just accepted, so the check is satisfied by construction and can only fire on a candidate that was already skipped. Keeping it is correct; describing it as independent evidence was not. The raw count is reporting only and never gates, so no candidate set changes. Verified against the live fleet: same 9 candidates before and after, worktree count unchanged, --remove not run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 25 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 101 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo
enabled auto-merge (squash)
August 18, 2026 12:41
BigSimmo
disabled auto-merge
August 18, 2026 12:47
BigSimmo
enabled auto-merge (squash)
August 18, 2026 13:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-on to #2113, which merged while these were being written. Two independently revertible items.
Records the five follow-ups that PR fix(claude): make the session-start hook runnable, and give the agent config teeth #2113 could not close. Each is blocked on something outside the repository — an idle worktree fleet, an elevated prompt, a future compaction, a first web session, and a decision from the owner — so capturing them durably is the smallest fix that actually resolves them rather than restating them. They land as immutable request files under
docs/outstanding-issues-inbox/;npm run issues:reconcileapplies them to the canonical ledger from a serialized branch after this lands. The P1 among them is that PR churn has exhausted both review bots' budgets: CodeRabbit reported 101 included reviews in the past 7 days and an organization spending cap reached, the Codex connector reported its own usage limit, and the net effect is that fix(claude): make the session-start hook runnable, and give the agent config teeth #2113 received no automated review at all and subsequent PRs will not either. AGENTS.md already measured the CI half of that cost on 2026-07-30; this is the second and more consequential bill, and it needs a decision on whether the bundling rule gets a gate rather than prose.Fixes a false reassurance in
clean-worktree.mjs, shipped in fix(claude): make the session-start hook runnable, and give the agent config teeth #2113. Every--squashedcandidate printed0 commits aheadwhile being genuinely ahead oforigin/main— by 11, 2 and 1 commits on the live fleet. A squash-merged branch keeps its original commits forever, so it stays ahead permanently; what is zero is the count of unlanded commits, which is a different claim. The line was stating something a reader could disprove with onegit rev-list, on a tool whose entire purpose is to be trusted immediately before a deletion. It now reports both numbers, for example11 ahead of origin/main, 0 unlanded commits. The comment above the ahead check is corrected in the same change: it described the check as belt-and-braces against the merge test, which is true in ancestor mode where it is the real gate, but false in squash mode —gitAheadUnlandedCountreturns 0 for any branch the squash test has just accepted, so the check is satisfied by construction and can only fire on a candidate that was already skipped. Keeping it is right; describing it as independent evidence was not.The raw ahead count is reporting only and never gates, so no candidate set changes.
Verification
node scripts/clean-worktree.mjs --self-test— passed, covering merged-and-clean as a candidate and dirty, ahead, detached, locked, unmerged, current and main worktrees all skipped.node scripts/clean-worktree.mjs --merged --squashed— run against the live 48-worktree fleet before and after the change. Same 9 candidates, worktree count unchanged,--removenot run. The corrected line readsbranch merged into origin/main; clean tree; 11 ahead of origin/main, 0 unlanded commits (tip 15fd06314).npm run check:outstanding-issues—Outstanding-issues guard passed: 361 rows (105 open, 256 archived), unique display and durable ids, collision-free allocation enabled, no merge driver, no ids deleted from base 5ae2bb6ec703.npm run check:ledger-write-discipline—Ledger write discipline passed for 5ae2bb6ec703..HEAD.npx prettier --check scripts/clean-worktree.mjsandnpm run format:check— all matched files use Prettier code style.npx eslint scripts/clean-worktree.mjsclean.verify:ui(no UI, routing or styling surface), and every provider-backed gate includingverify:releaseandcheck:production-readiness(nothing here reaches OpenAI, Supabase, ingestion or clinical output).Risk and rollout
Low risk and reversible;
pr-policy's classifier returnsclinicalRisk: false,operationalRisk: false,ragRanking: false,ui: false.The inbox requests are additive files that change no behaviour and do not touch the canonical ledger —
docs/outstanding-issues.mdis edited only by a later serializedissues:reconcile, never by this PR. Theclean-worktree.mjschange is confined to the human-readable reason string and two comments; the gate that decides which worktrees are nominated is untouched, and the live before-and-after run above confirms an identical candidate set.runWorktreeCleanup(), whichverify:preflightcalls, remains unmodified.Rollback is
git reverton either commit independently.One operational note for whoever reads the ledger next: two landed worktrees were removed manually while preparing this change (
clinical-medication-graph-dedupandreview-merge-open-prs-0cd8ea), taking the fleet from 50 to 48 and D: from 51% to 48% full. Seven candidates were deliberately left: one was in active use with its git index touched seconds earlier, two were flagged as not fully corroborated by the patch-id test, and four live on the C: drive and belong to other agents' sessions.🤖 Generated with Claude Code