Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): close two measured blind spots in check:optional-error-sink - #11549
Merged
Conversation
…sink Both were narrowings the gate neither argued nor counted, so its own census line overstated what it had read. 1. A sink spelled with bare `Function` set `fn = false`, which hid its `error` from the population lookup AND made the shape impure, so it landed in no bucket at all — not the population, not `impure`, not `noErrorMember`. `isFunctionTyped` now reads `Function` as a channel: population 38 -> 41, red 2 -> 5. All three newly visible sinks are pre-existing reds in plugin-sharing whose options types are publicly exported, so they are recorded in the shrink-only ledger rather than flipped; tightening `warn` there is #10556's contract call. 2. The file prefilter matched on `error` alone — sound for the enforced population, unsound for the `noErrorMember` census line, which counts sinks declaring NO `error`. It is now derived from the whole channel vocabulary, so the tally is a count rather than a lower bound of unknown slack: 57 -> 95 on an unchanged tree. The reject side of (1) gets its own counted bucket, `unreadable`: a shape whose members are all channel names but whose member type is a named type this syntactic matcher cannot resolve. Measured cost today is zero missed reds. Gate exit code on a clean tree is unchanged (0).
Found by ablation: replacing the call site with a literal `/\berror.../` -- the exact regression this PR repairs -- restored the undercount (tally 96 -> 58) while every prefilter pin stayed green, because they all test the constant and none of them tests the scan. Part of #11069
The pin as first written tested `run.toString()` directly and stayed green under the very ablation it exists for: `toString()` returns comments, and the comment above the call site names CHANNEL_PREFILTER in prose. A pin satisfied by a sentence describing the code is satisfied by deleting the code and keeping the sentence. Part of #11069
…ks on every PR Not wired by the PR that introduced it, and wired since -- but the paragraph saying wiring 'is a lane-PM decision, recorded on #9754 rather than taken here' outlived the decision being taken. It runs from lint.yml on every pull request with no paths filter, so a red here blocks and the clean-tree exit code is load-bearing rather than informational. Same species as the blind spots this branch repairs: a claim nothing re-measures. Part of #11069
This was referenced Aug 24, 2026
os-steve
marked this pull request as ready for review
August 24, 2026 03:30
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#11069
Two narrowings in
scripts/check-optional-error-sink-contract.mjsthat the gate neither argued nor counted, so its own census line overstated what it had read. Both are repaired; the gate's exit code on a clean tree is unchanged (0), and nothing about what it enforces changes.Route taken for blind spot (1): triage's — recognise the catch-all callable into the population and baseline the newly visible reds. The discriminator the claim thread asked me to check: this gate does have a ledger (
scripts/optional-error-sink-contract.baseline.json) and it is shrink-only — a baselined entry that is no longer red fails the gate ("delete the entry"), and there is deliberately no--fix/--updateflag. So a baseline entry is the durable record, and the third-bucket fallback was not needed.What moved
Measured on
cd932772(the branch base), same tree throughout:noErrorMembertallyunreadableFunctionlimb onlyThe two fixes are independent and additive: 57 + 38 + 1 = 96.
1. Bare-
Functionsinks escaped the population entirelyerror?: Functionsetfn = false, which both hid theerrormember from the population lookup and made the whole shape impure — so such a sink landed in no bucket: not the population, notimpure, notnoErrorMember.isFunctionTypednow reads bareFunctionas a channel.Functionis the only catch-all spelling live in the tree (14 members, 4 files, allplugin-sharing) —(...args: any[]) => anyis already aFunctionTypeNodeand needed no case.Three red sinks were hidden this way, not two. The card named two from a hand search; widening the matcher found a third:
index.ts?sharing-service.ts:280logger@SharingServiceOptionsshare-link-service.ts:345logger@ShareLinkServiceOptionssharing-rule-service.ts:76logger@SharingRuleServiceOptionsAll three are baselined, not flipped. All three options types are publicly exported, so making
warnrequired breaks any host passing{ info, error }— that is #10556's contract call, out of scope here. Each ledger row says so, names the widening, and stays shrink-only. The third one is itself the argument for the structural fix: a hand-listed set of shapes is exactly what a structural population exists to replace.⛔ No file under
packages/plugins/plugin-sharingis touched by this PR.2. The
noErrorMembertally was undercounting by 40%The prefilter matched
/\berror\s*\??\s*[:(]/— sound for the enforced population (a sink declaringerrormust spell the token), unsound for the census line that counts sinks declaring noerror, which is exactly the set of files with no reason to contain it.It is now derived from
LOG_CHANNELSrather than written out, which makes it sound for every bucket: every shape this gate records in any bucket has at least one identifier-named member from the vocabulary, spelledname:,name?:,name(orname?(— all four covered. So the tally is a count again, not a lower bound of unknown slack. Deriving it also means adding a channel widens it automatically; a hand-written second copy of the vocabulary is how the previous narrowing outlived its reason.Verified by measurement, not only by that argument: removing the prefilter entirely and parsing all 1914 files produces a byte-identical 146-line
--listcensus. The prefilter still skips 1337/1914 files (69.9%), 2.8s vs 3.7s unfiltered.The specimen the card identified is the control:
packages/plugins/plugin-sharing/src/logger-shapes.tsdeclares one pure{ info?, warn? }sink and contains noerrortoken anywhere (verified: the old prefilter returnsfalseon it). It was skipped before the parser saw it; it now appears in the tally.--listalso lists the tally's members now — it was the one census line whose shapes were unnameable, and an unauditable count is what let a 40% undercount sit in this output unnoticed.3. The reject side of (1), counted rather than argued
isFunctionTypedreads syntax and this gate has no type checker (deliberately — a detector with no program to build cannot fail to build one in CI). Sowarn?: Logger['warn']is still opaque to it. Rather than let those vanish — the exact defect this card is about — a shape whose members are all channel names but which carries such a type is counted asunreadableand listed.Measured cost today: zero missed reds. Two of the three are
import-coerce.ts's{ error: FieldCoerceError }result envelopes witherrorREQUIRED; one issuspended-run-store.ts'sMinimalLogger={ warn?: Logger['warn']; debug?: Logger['debug'] }, which declares noerror. Keyword types (string,unknown,boolean) are deliberately not counted — those are result fields, unambiguously not callable.Non-vacuity — ablations, mutation proven on disk both ways
Each leg: anchor counted before (must be 1) and after (must be 0), injected marker counted (must be 1),
git hash-objectbefore/after,trap restore EXIT INT TERM, byte-identical restore verified.Functionlimberror-only regexunreadablebucketTwo process notes, both worth more than the numbers:
CHANNEL_PREFILTER.Function.prototype.toString()returns comments, and the comment above the call site namesCHANNEL_PREFILTERin prose. A pin satisfied by a sentence describing the code is satisfied by deleting the code and keeping the sentence. Comments are now stripped first. Only after that did leg B go red (third commit).//marker commented out the closing};and the leg measured a syntax error. Re-run with a block comment; the table above is the valid run.Verification
Gate union derived from the real change set via
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(2 paths, merge-basecd932772), all run at final HEADed579483:pnpm check:optional-error-sink✓ … (5 baselined, shrink-only)+ self-test✓ 19 case(s)pnpm check:cross-package-test-inputspnpm check:entry-guard✓ check:entry-guard: 139 scripts/ file(s) …pnpm check:parse-guard✓ check:parse-guard: 138 scripts/ file(s) …pnpm check:pnpm-filter-targets✓ check:pnpm-filter-targets: 120/148 …node scripts/check-ci-filter-parity.mjsnode scripts/check-cross-package-test-inputs.mjspnpm check:nul-bytes✓ … no raw ASCII control bytesAlso ran the two artifact-roster families the derivation flagged as sitting in a directory one of my paths is in (
check:pm-label-desc-cap,check-aggregator-roster.mjs) — both exit 0. Exit codes captured before any pipe.Lint — declared narrowing, with the measurement. Not the repo-wide run; the shared verify lock was held with a queue. Three evidences:
ESLint#isPathIgnoredovergit ls-files): 4958 of 6465 tracked files.scripts/check-optional-error-sink-contract.mjsis in it;scripts/optional-error-sink-contract.baseline.jsonis ignored by eslint, so excluding it excludes nothing.--format json: 1 file linted, 0 errors, 0 warnings (eslint --no-inline-config).eslint.config.mjswhich never enables type-aware linting for any file (noparserOptions.project, no typed@typescript-eslintrules) — stated and measured with a positive control in that config's own header. So this diff cannot move the verdict on any file it does not contain.Also in this branch: the header's
Wiringsection said UNWIREDFound while checking whether the exit-code requirement was load-bearing. The script's
## Wiringsection said the gate was "NOT wired into.github/workflows/lint.yml" and that wiring it "is a lane-PM decision, recorded on #9754 rather than taken here". It has since been wired:lint.ymlrunspnpm check:optional-error-sinkon every pull request, with nopaths:filter. So a red here blocks, and "exit code on a clean tree stays 0" is a load-bearing requirement rather than a courtesy.Corrected in place — same file, same defect class as the two blind spots (a claim nothing re-measures), no new verification surface.
One further drift found and deliberately not fixed here: the
lint.ymlstep comment for this gate states13 cases, 36 sink types, 15 baselined— all three now wrong, and15 baselinedhas been wrong since #10556's paydown. Editinglint.ymlwould add a hot conflict-magnet file and new gate families to ascripts/**-only PR, so it is filed as #11551 (unassigned,finding) with three options for what to do about numbers living in a workflow comment at all.Scope
scripts/**only and publishes nothing →skip-changeset.MinimalLoggertypes, now divergent after #10556 #10692 (pm:blocked, services) is aboutplugin-sharing's logger types, Pay down the optional-error sink ledger — 13 paid, 2 remain and both are DESIGN CALLS (was: "15 sink types") #10556 is the ledger paydown; neither covers the gate's blind spots. Control: the same queries returned known-present cards (check:optional-error-sink has two measured blind spots: bare-Functionsinks escape the population, and the no-errorcensus tally undercounts #11069 itself, Pay down the optional-error sink ledger — 13 paid, 2 remain and both are DESIGN CALLS (was: "15 sink types") #10556, plugin-sharing declares six separateMinimalLoggertypes, now divergent after #10556 #10692), so the search was live.37 → 38, tally54 → 57. The table above re-measures oncd932772.Generated by Claude Code
Generated by Claude Code