You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Two packages/cli scans read serve.ts with no comment/code separator at all, so prose counts as code — the direction js-comment-mask's header calls the worse one #10514
Filed unassigned from the #10453 dev seat (session session_019bmVFqoQPq63zhKrxdYG1r, PR #10513). Found by re-deriving that card's file paths at head instead of trusting them.
How it surfaced
#10453 names two packages/cli contract tests as carrying #9367's naive stripComments. Re-derived at 5c3faa70d by searching for the strip pattern: only one does.serve-email-config-parity.contract.test.ts has no comment handling at all, and git log -S stripComments on that path returns nothing — it never had one. PR #10513 converts the one that really carries it and leaves this alone, because the fix is not the same fix.
But "has no strip" is not the same as "is fine". Two scans in that directory read serve.tsraw, comments included. A naive strip is wrong in the BLINDS direction (real code deleted). Reading raw is wrong in the FABRICATES direction — the one scripts/js-comment-mask.mjs's own header singles out:
the gate reads genuinely commented-out text as live code: it FABRICATES a hit rather than missing one.
Same classification #10427 applies to scanSource's desync. Neither scan is wrong today; both are one ordinary comment away from being wrong.
keysReadFromConfigEmail() matches /cfgEmail\.[A-Za-z_$][\w$]*/g over raw SERVE_SOURCE. Its docblock argues the scan is "an exact measure rather than an approximation" because every read of cfgEmail is a dot access — true, and beside the point: it never considers that a dot access can appear in prose.
Both of its assertions break, in opposite directions. Measured on today's serve.ts:
today: ["apiKey","appName","defaultFrom","defaultTemplateContext","options","persist","provider","queueDelivery","retries"]
+ one COMMENT naming cfgEmail.bogusKey -> undeclared = ["bogusKey"]
('declares every config.email key the resolver reads' asserts [] -> RED, falsely)
reader stops reading `persist`, but a COMMENT still names it
-> persist still counted as read? true
('reads every key it declares' stays GREEN over a declared key nothing reads)
The second is the worse one and it is precisely the defect this file exists to catch: #5447 was a declared-but-unread key, and the file's own docblock explains at length why the DECLARED_BUT_UNREAD registry was deleted so that "a key declared and not read is red the day it lands". A comment mentioning the key restores the exemption silently, with no registry to argue about.
Three assertions match regexes against raw SERVE_SOURCE. Reintroducing the exact regression the file pins, while describing the old call in a trailing comment:
not.toMatch(/checkMultiNodeAllowed\(\s*\)/) -> bad shape present? true (correctly red)
toMatch(/checkMultiNodeAllowed\(\s*[^)\s]/) -> good shape present? true (satisfied by the COMMENT alone)
So the negative assertion still holds, but the positive one is satisfiable by prose. A revert spelled without the bare-call shape — say the call deleted outright and quoted in a comment — passes both.
Its interfaceFields() helper does strip comments, with the naive two-regex form, but only over an already brace-matched export interface body. That is a much narrower blast radius than a whole-file strip and is not what this issue is about; it is noted so a future re-derivation does not read it as the same defect.
Route both scans through maskComments from scripts/js-comment-mask.mjs, as #9445 did for the six gates and #10513 does for the parity scan. Notes for whoever takes it:
maskComments blanks rather than deletes, so both scans' offsets and line numbers survive unchanged.
⚠️ Verify direction before assuming this is a no-op. On today's tree the key set is identical raw / naive / masked, so the swap should move no verdict — but that is a fact to re-measure at the time, not to inherit from this issue. If it does move a verdict, that is a finding, not something to reconcile by editing an assertion.
Filed unassigned from the #10453 dev seat (session
session_019bmVFqoQPq63zhKrxdYG1r, PR #10513). Found by re-deriving that card's file paths at head instead of trusting them.How it surfaced
#10453 names two
packages/clicontract tests as carrying #9367's naivestripComments. Re-derived at5c3faa70dby searching for the strip pattern: only one does.serve-email-config-parity.contract.test.tshas no comment handling at all, andgit log -S stripCommentson that path returns nothing — it never had one. PR #10513 converts the one that really carries it and leaves this alone, because the fix is not the same fix.But "has no strip" is not the same as "is fine". Two scans in that directory read
serve.tsraw, comments included. A naive strip is wrong in the BLINDS direction (real code deleted). Reading raw is wrong in the FABRICATES direction — the onescripts/js-comment-mask.mjs's own header singles out:Same classification #10427 applies to
scanSource's desync. Neither scan is wrong today; both are one ordinary comment away from being wrong.Subject 1 —
packages/cli/src/commands/serve-email-config-parity.contract.test.tskeysReadFromConfigEmail()matches/cfgEmail\.[A-Za-z_$][\w$]*/gover rawSERVE_SOURCE. Its docblock argues the scan is "an exact measure rather than an approximation" because every read ofcfgEmailis a dot access — true, and beside the point: it never considers that a dot access can appear in prose.Both of its assertions break, in opposite directions. Measured on today's
serve.ts:The second is the worse one and it is precisely the defect this file exists to catch: #5447 was a declared-but-unread key, and the file's own docblock explains at length why the
DECLARED_BUT_UNREADregistry was deleted so that "a key declared and not read is red the day it lands". A comment mentioning the key restores the exemption silently, with no registry to argue about.Subject 2 —
packages/cli/src/commands/serve-multi-node-cap-advisory.pin.test.tsThree assertions match regexes against raw
SERVE_SOURCE. Reintroducing the exact regression the file pins, while describing the old call in a trailing comment:So the negative assertion still holds, but the positive one is satisfiable by prose. A revert spelled without the bare-call shape — say the call deleted outright and quoted in a comment — passes both.
Its
interfaceFields()helper does strip comments, with the naive two-regex form, but only over an already brace-matchedexport interfacebody. That is a much narrower blast radius than a whole-file strip and is not what this issue is about; it is noted so a future re-derivation does not read it as the same defect.Not a duplicate
/*inside a string literal blinds — the repo already has the string-aware masker they should use #9367 (closed) converted sixscripts/check-*.mjsgates off the naive strip. Gates, not tests, and about a strip that exists.packages/clicontract TESTS — the fix converted the six gates, not the scans that live besideserve.ts#10453 is the residue of that conversion in a test that has the strip; PR test(cli): put the serve↔bootStack parity scan on the shared comment masker #10513 closes the half that is real. This issue is the other half of that card, re-diagnosed — the card asserted a strip here that has never existed, so its prescribed fix does not apply.scanSource's own nested-template desync. Both subjects here were checked byte-for-byte against@typescript-eslint/parser's comment ranges while working test(cli): put the serve↔bootStack parity scan on the shared comment masker #10513, andserve.tsshows 0 bytes of disagreement in either direction, so the masker is a correct instrument for this file today.maskCommentsimport here would go undeclared silently; relevant to whoever implements this, not the defect itself.Suggested shape
Route both scans through
maskCommentsfromscripts/js-comment-mask.mjs, as #9445 did for the six gates and #10513 does for the parity scan. Notes for whoever takes it:scripts/js-comment-mask.mjstoCROSS_PACKAGE_TEST_INPUTS['@objectstack/cli']and the matching$TURBO_ROOT$input on@objectstack/cli#test; once either it or docs(plugin-audit): document theos serveopt-in, and rule out a config-derived audit options helper #10450 lands there is nothing to add.check:cross-package-test-inputswill not demand it (check-cross-package-test-inputs' literal collector cannot see an escaping relative IMPORT specifier, so a test that imports a module outside its package goes undeclared silently #10452).maskCommentsblanks rather than deletes, so both scans' offsets and line numbers survive unchanged.