gate: the SQL column-literal ratchet never scanned scripts/, where the raw SQL actually is - #3000
Merged
Conversation
Collaborator
Author
|
Reproduced both directions independently. Injected a real function kbProbeQuery(tx) {
return tx.execute(sql`SELECT 1 FROM project."tasks" WHERE "column" = 'done'`);
}
Gate clean at 28 on the branch with no self-write. The finding method is the valuable part
That is the strongest version of this move available: not "does the gate catch a bad thing" but "does the gate notice a good thing." A ratchet that cannot see a removal is not measuring the population it claims to. Cheaper than a mutation, too, since you get it free every time you fix something in the scanned area — provided you look. Your "two changes, either alone sees nothing" note matches what #2978 hit exactly: root and extension had to move together, and adding one alone reports a reassuring zero. One probe note, on my own harnessMy first injection went in as a Worth noting the asymmetry that makes this class expensive: had I not re-checked, I would have filed a bug report against a correct gate — the same wasted-cycle cost as a false positive in the baseline, but aimed at a person instead of a file. |
…e raw SQL actually is `check-sql-column-literals` walked `packages/` only and took `.tsx?`, while every operator script is a repo-root `.mjs`. So the one place in the tree that writes raw SQL by hand was the one place this gate could not see. Found by removing a raw-SQL lane literal in #2999 and watching this gate report "22 known, none added" — unchanged and green. Its own header promises the opposite ("a LOWER count fails too so the baseline is ratcheted down"), so the silence was the tell. TWO changes, and either alone still sees nothing: the root and the extension. Adding one without the other scans nothing new and reports a reassuring zero — the same trap #2978 hit widening the lane-wiring census. Newly visible, audited rather than blind-baselined: audit-branch-cross-contamination.mjs:182 "column" IN ('triage','todo','in-progress','in-review') Real: the contamination audit scans only the legacy active lanes, so on a renamed board it scans nothing and reports no contamination. Read-only, and it does print its `scannedColumns`, which is the one thing keeping that from being silent. reconcile-leaked-soft-deletes.mjs:53,73 Both already fixed by #2999, which is the PR that exposed this gap. Proven able to fail, not just to count: a temporary `.mjs` holding one forbidden comparison was reported ("baseline allows 0") and the gate returned to green when it was removed. The ScriptKind move to JS for `.mjs` is DEFENSIVE and I could not demonstrate it was necessary — three JSX-ambiguous shapes all recovered under TSX with identical counts. Said plainly in the code, because the opposite claim would be easy to make and wrong. MERGE ORDER: #2999 removes both literals in reconcile-leaked-soft-deletes.mjs. Landing it after this one drops the count and this gate fails on DECREASE, needing a re-record. Merge #2999 first, or ping me and I will re-record here. Verified: gate green at 28 known / none added; its own suite 32 passed; eslint clean; census, lane-wiring and fnxc ratchets green.
#2999 removed both raw-SQL lane literals from reconcile-leaked-soft-deletes.mjs, so the population this branch recorded (28) is two higher than the tree. That reads as a DECREASE, which this gate fails on by design. Re-recorded to 26: audit-branch-cross-contamination.mjs keeps its 4, the leaked-soft- deletes entry is gone. This is the merge-order hand-off flagged on the PR, done rather than left for the operator.
gsxdsm
force-pushed
the
fix/sql-gate-scan-scripts
branch
from
July 31, 2026 07:02
d5046e1 to
a472120
Compare
Collaborator
Author
|
Rebased and re-recorded now that #2999 has landed — the merge-order hand-off flagged above is done, not left for you. #2999 removed both raw-SQL literals from Re-recorded to 26 — Worth noting the mechanism differs from its sibling: this gate auto-rewrites the baseline downward and tells you to commit it, where |
Contributor
|
Warning Review limit reached
Next review available in: 18 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 Plus Run ID: 📒 Files selected for processing (2)
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 |
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…lane logic lives (#3002) #3000 showed this gate never scanned `scripts/`. I went auditing my own instrument after that, and the roots have a **second** hole: the walk was rooted at `packages/` alone, so every lane parameter a plugin declares or calls sat outside the ratchet entirely. ## Measured with a control | probe | before | after | |---|---|---| | unwired seam under `packages/` | caught | caught | | **identical** seam under `plugins/` | **missed** | caught | | clean tree | exit 0 | exit 0 | ## The sibling gate already knew `check-lane-wiring` lists `plugins` in its roots, and its header records the incident that put it there: an unwired `completeColumnsByTaskId` sat on `main` unreported because the glasses plugin wasn't scanned. This gate re-opened the same hole rather than inheriting the lesson. Two scope holes in one instrument is the actual finding — **the roots deserve the same scrutiny as the matcher, and until now they had none.** Every blind spot found in this gate so far has been in the matcher; nobody, me included, thought to probe what it walks. ## Newly visible — audited, not blind-baselined `plugins/fusion-plugin-dependency-graph/src/GraphTaskNode.tsx` calls `isTaskStuck()` without the resolved flags, while **six of the seven** other call sites supply them. That's exactly the partial-supply shape this gate exists to catch, hidden purely by scope. It's exempted rather than wired, and the reason is the interesting part. The plugin has **no lane-trait source anywhere**: it's mounted as a dashboard view through `PluginDashboardViewContext`, and `DependencyGraph` receives `tasks: Task[]` and nothing else. Passing the argument here would pass `undefined` — an unsupplied optional parameter, which the learnings doc's first failure shape calls strictly worse than the literal it replaces, because it reads as converted and answers legacy forever. Correct supply needs the plugin **view context** to carry per-task flags: a published-API change. That's the same "needs a data change" category as the existing `TaskDetailModal` entry, not the "awkward means wire it" case the exemption rule refuses. I checked that distinction against my own rule before taking the exemption, because the rule exists to stop exactly this kind of convenient reading. Filed for the plugin-API owner rather than bodged here. ## Measured - seam population **22 → 23** with plugins in scope - gate's own suite: **18/18 green** - lint and the FNXC gate green Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…laimed it had
Two halves of the same wrong answer.
The query allowlisted `"column" IN ('triage','todo','in-progress','in-review')`. On a
board whose lanes are named anything else that matches NOTHING, so the audit scans zero
rows and reports zero contamination — a clean bill of health from a scan that never
happened. `triage` is in that list too, a lane U11 (#2515) deleted.
And `scannedColumns` was the same four ids as a literal array, printed regardless of
what the query returned. So the report ASSERTED coverage of four lanes it had not
looked at. That field is the only thing standing between this audit and a silent wrong
answer, which is exactly why it has to be an observation rather than a claim; it is now
derived from the rows that came back.
The query is inverted so the default is the safe one: an unrecognised lane is active
work by assumption and IS audited, while only lanes that genuinely mean finished drop
out. An allowlist fails closed (skip everything unknown), a denylist fails open (look at
it), and for an audit one extra finished branch is a far smaller error than auditing
nothing.
Filtered in JS rather than by building a dynamic SQL exclusion: it keeps ONE place
deciding what "finished" means and removes the last raw-SQL lane literal from this file.
Reverted, both new cases fail — the report claims the four legacy ids for a board that
has none of them.
MERGE ORDER: this removes the 4 literals #3000 baselines. Landing this after #3000
drops that count and its gate fails on DECREASE; that gate auto-rewrites the baseline
and asks for the commit. Either order works, one of them needs a re-record — ping me.
Verified: 3 passed; node --check and eslint clean; census, lane-wiring and fnxc ratchets
green. The SQL gate still reads 22 here because this branch predates #3000, which is the
blind spot #3000 exists to close. No changeset — root `scripts/` is repo tooling.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
#3000 widened check-sql-column-literals to scan scripts/, baselining this file's 4-literal IN clause. This branch removes that clause, so the recorded population sits 4 above the tree — a DECREASE, which the gate fails on by design and auto-rewrites. Re-recorded to 22. This is the merge-order hand-off flagged on the PR, done rather than left for the operator. Still blocked on #3008 for the Lint check: main is red on check-fnxc-future-dates because of four hour-26 stamps I wrote in #2994, and CI lints the branch MERGED with main. Nothing in this branch can fix that; #3008 is the fix.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…laimed it had
Two halves of the same wrong answer.
The query allowlisted `"column" IN ('triage','todo','in-progress','in-review')`. On a
board whose lanes are named anything else that matches NOTHING, so the audit scans zero
rows and reports zero contamination — a clean bill of health from a scan that never
happened. `triage` is in that list too, a lane U11 (#2515) deleted.
And `scannedColumns` was the same four ids as a literal array, printed regardless of
what the query returned. So the report ASSERTED coverage of four lanes it had not
looked at. That field is the only thing standing between this audit and a silent wrong
answer, which is exactly why it has to be an observation rather than a claim; it is now
derived from the rows that came back.
The query is inverted so the default is the safe one: an unrecognised lane is active
work by assumption and IS audited, while only lanes that genuinely mean finished drop
out. An allowlist fails closed (skip everything unknown), a denylist fails open (look at
it), and for an audit one extra finished branch is a far smaller error than auditing
nothing.
Filtered in JS rather than by building a dynamic SQL exclusion: it keeps ONE place
deciding what "finished" means and removes the last raw-SQL lane literal from this file.
Reverted, both new cases fail — the report claims the four legacy ids for a board that
has none of them.
MERGE ORDER: this removes the 4 literals #3000 baselines. Landing this after #3000
drops that count and its gate fails on DECREASE; that gate auto-rewrites the baseline
and asks for the commit. Either order works, one of them needs a re-record — ping me.
Verified: 3 passed; node --check and eslint clean; census, lane-wiring and fnxc ratchets
green. The SQL gate still reads 22 here because this branch predates #3000, which is the
blind spot #3000 exists to close. No changeset — root `scripts/` is repo tooling.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
#3000 widened check-sql-column-literals to scan scripts/, baselining this file's 4-literal IN clause. This branch removes that clause, so the recorded population sits 4 above the tree — a DECREASE, which the gate fails on by design and auto-rewrites. Re-recorded to 22. This is the merge-order hand-off flagged on the PR, done rather than left for the operator. Still blocked on #3008 for the Lint check: main is red on check-fnxc-future-dates because of four hour-26 stamps I wrote in #2994, and CI lints the branch MERGED with main. Nothing in this branch can fix that; #3008 is the fix.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…laimed it had (#3005) ## An audit that scanned four legacy lanes — and claimed it had Two halves of the same wrong answer. **The query allowlisted the lanes:** ```sql WHERE deleted_at IS NULL AND "column" IN ('triage','todo','in-progress','in-review') ``` On a board whose lanes are named anything else that matches **nothing**, so the audit scans zero rows and reports zero contamination — a clean bill of health from a scan that never happened. `triage` is in that list too, a lane U11 (#2515) deleted. **And the report asserted the coverage it did not have:** ```js scannedColumns: ["triage", "todo", "in-progress", "in-review"], ``` printed regardless of what the query returned. When I first surveyed this script I called that field "the one thing keeping it from being fully silent" — it turns out it was a **claim, not an observation**, so it was not keeping it honest at all. It is now derived from the rows that came back. ## Fix: exclude finished lanes instead of allowlisting active ones Inverted so the default is the safe one — an unrecognised lane is active work by assumption and **is** audited; only lanes that genuinely mean finished drop out. An allowlist fails **closed** (skip everything unknown), a denylist fails **open** (look at it), and for an audit one extra finished branch is a far smaller error than auditing nothing. Filtered in JS rather than by building a dynamic SQL exclusion: it keeps **one** place deciding what "finished" means, and removes the last raw-SQL lane literal from this file. ## Revert proof ``` ✖ scannedColumns reports the board's real lanes, not a fixed legacy claim ✖ reports each scanned lane once, and nothing at all for an empty board ℹ pass 1 ℹ fail 2 ``` ## A demonstration of #3000, for free This PR removes a 4-literal raw-SQL clause, and `check-sql-column-literals` here reports **22, unchanged and green** — because this branch predates #3000 and the gate still walks `packages/` only. That is precisely the blind spot #3000 closes, reproduced a second time. ## Merge order This removes the 4 literals #3000 baselines. Landing this **after** #3000 drops that count and its gate fails on DECREASE — that gate auto-rewrites the baseline and asks for the commit, unlike `check-lane-wiring` which needs an explicit `--update-baseline`. Either order works; one of them needs a re-record, and I am happy to push it. ## Verification (measured) - `node --test` — **3 passed / 0 failed** (1 pre-existing + 2 new) - `node --check`, `eslint` — clean - `lifecycle-column-census --strict`, `check-lane-wiring`, `check-fnxc-future-dates` — green No changeset: root `scripts/` is repo tooling, not part of the published package. ## Territory status This was the last item I know of in `scripts/`. The four operator scripts holding lane assumptions — `recover-stale-blocked-by` (#2992), `reconcile-task-state-consistency` (#2994), `reconcile-leaked-soft-deletes` (#2999) and this one — are now either resolved or, where a script genuinely cannot resolve lanes, made loud rather than silent.
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
…he board does not declare (#3046) ## Invisible to both censuses `archiveDefinedFeatureBootstrapDuplicate` writes `tasks.column` **directly** rather than through `moveTask`: ```ts .set({ column: "archived", updatedAt: … }) ``` - the **lifecycle census** reads comparisons — an assignment isn't one - the **move-target census** reads `moveTask` call arguments — this never calls it So on a board whose archive lane is renamed, the duplicate landed in a column that workflow doesn't declare: a card in a lane the board can't render, from a path that runs during ordinary feature bootstrap. ## Reuses the helper this class already has `archivedLanesFor(taskId)` was added for the guards further up the same file. It returns the legacy id when the task has no resolvable workflow, so an **unconverted board is byte-identical**. No new resolution machinery — the two `<> 'archived'` guards become `notInArray(column, [...lanes])` and the write targets the resolved lane. A board declaring several archive lanes is arbitrated by taking the first, the same choice `resolveLifecycleColumns` makes. Multiple archive lanes aren't a shape the builtin lineages produce. ## Measured | check | result | |---|---| | mission-store PG suite | **36 → 38**, all green | | new pair | differential — `filed` collides with no legacy id, and the default-lineage control still lands in `archived` | | mutation (hardcode the target back) | fails the renamed case | | SQL literal gate · `tsc` | green | ## How this was found Measuring the literal-column-**write** population for #2839: 51 raw sites, of which 20 are the four builtin workflow IRs declaring their own columns (correct by definition) and several more are archive-*entry record* fields rather than board columns. This is the one I verified is a real board write on a live path. Worth noting the measurement itself was wrong twice first — my glob was `packages/*/src/**/*.ts`, which requires a subdirectory and silently skipped every top-level file in `src/` (including this one), and my script printed only the first 14 findings so the grouping was over a truncated list. Same scope-blindness class as #3000 and #3002, this time in a throwaway scanner. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The gate could not see the one place raw SQL is actually written by hand
check-sql-column-literalswalkedpackages/only and took.tsx?. Every operator script is a repo-root.mjs.I found it by removing a raw-SQL lane literal in #2999 and watching this gate report:
Unchanged, and green. Its own header promises the opposite — "a LOWER count fails too so the baseline is ratcheted down" — so the silence was the tell.
Two changes, and either alone still sees nothing: the root and the extension. Adding one without the other scans nothing new and reports a reassuring zero — the same trap #2978 hit when widening the lane-wiring census.
Newly visible: 6 sites, audited not blind-baselined
audit-branch-cross-contamination.mjs:182—"column" IN ('triage','todo','in-progress','in-review')scannedColumns, which is the one thing keeping that from being fully silent.reconcile-leaked-soft-deletes.mjs:53, :73Proven able to fail, not just to count
A guard that has only ever printed a number is a number. A temporary
.mjsholding one forbidden comparison:and the gate returned to green once removed.
One claim I withdrew
I initially wrote that the
ScriptKindmove toJSfor.mjswas needed because "TSX treats<as JSX and would misparse an ordinary comparison". I could not demonstrate it. I tried three JSX-ambiguous shapes —x <div> y,f<b, c>(d), and a literal sandwiched between<and>comparisons — and TSX recovered from all three with counts identical to JS.So
JSis used because it is the correct kind for the file, not because a miss was observed, and the code now says exactly that. The opposite claim would have been easy to make and wrong, and this gate's whole value is that its statements about its own coverage are true.Merge order
#2999 removes both literals in
reconcile-leaked-soft-deletes.mjs. Landing it after this PR drops the count, and this gate fails on DECREASE (by design), needing a re-record. Merge #2999 first, or say the word and I will re-record here.Note the widening is self-protecting afterwards: if someone narrows the walk back to
packages/, the recordedscripts/entries vanish from the scan and the gate goes red on decrease.Verification (measured)
packages/only)eslint— cleanlifecycle-column-census --strict,check-lane-wiring,check-fnxc-future-dates— greenGate/tooling only; no product file touched.