Skip to content

test(quality): fail loudly when a source-scanning guard is negative-only - #8619

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.49from
MumuTW:test/split-safety-guards
Jul 28, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.49from
MumuTW:test/split-safety-guards

Conversation

@MumuTW

@MumuTW MumuTW commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

First step of the god-file decomposition campaign tracked in #8617. No production code.

The problem

A negative guard passes against an empty string:

const endpoint = readSource("src/.../EndpointPageClient.tsx");
assert.equal(endpoint.includes(">Active Endpoints<"), false);

Extract AvailableEndpointsSection out of that file and the parent no longer contains the
string — the assertion keeps passing while guarding nothing. No test turns red. Positive
assertions (assert.match) fail loudly and are safe; only the negative shape is silent.

#8617 plans ~90 extraction PRs. Each one is precisely the operation that voids these guards,
so the guards need to become loud before the extractions start.

What this adds

tests/unit/source-scanner-guards.test.ts — a hard gate, deliberately with no baseline
and no allowlist
: the violation set is small and every instance is a real hole, so it is
fixed by adding an anchor, not by suppressing it. Every test variable bound to a project
source read must carry at least one positive anchor.

Classification runs on logical statements, with strings, regexes and comments blanked out
before bracket counting — /notify\.error\(/ must not read as an unclosed paren, and a unit
must end when brace depth rises or an entire test body would fold into one blob and the gate
would start inventing violations. That folding is what exposed 3 of the 7 violations; a
line-based scan classifies the bare source, argument line of a multi-line
assert.doesNotMatch(...) as a positive anchor.

tests/_helpers/readSrc.ts — repo-root-relative reader (derived from import.meta.url,
never cwd) that throws on a missing or empty file instead of returning "".
settings-ui-layout-static.test.ts had a byte-equivalent local copy and now imports it.

The 7 violations fixed

file variable anchor
proxy-bypass-scope-guard-3226 chatHelpers export async function executeChatWithBreaker(
proxy-bypass-scope-guard-3226 chatCore export async function handleChatCore(
providers-autosync-ssrf-323 syncInitializeRouteSrc export async function POST(
claude-web-transport executorSource export class ClaudeWebExecutor extends BaseExecutor
dashboard-localization-contract endpoint export default function APIPageClient(
gamification-display-contract tokensSource export default function TokensPage(
settings-ui-layout-static generalStorage export default function SystemStorageTab(

Every anchor is a top-level exported declaration other modules import by name, so it survives
UI copy churn and internal splits.

Two were security scope guards. providers-autosync-ssrf-323 guards /api/sync/initialize
against a client-controlled Origin becoming the credential-bearing model-sync base URL.
proxy-bypass-scope-guard-3226 asserts the NVIDIA-only proxy bypass has not spread to the
chat hot path — and it reads open-sse/handlers/chatCore.ts, a 4,906-line file scheduled for
decomposition in 3.8.53. Both were held only by multi-line negative assertions, so both would
have died silently at exactly the moment they mattered.

Verification

source-scanner-guards            # pass 4  # fail 0
claude-web-transport             # pass 9  # fail 0
dashboard-localization-contract  # pass 14 # fail 0
gamification-display-contract    # pass 4  # fail 0
settings-ui-layout-static        # pass 8  # fail 0
providers-autosync-ssrf-323      # pass 4  # fail 0
proxy-bypass-scope-guard-3226    # pass 2  # fail 0

npm run typecheck:core           exit 0, 0 errors
npx eslint <all touched>         exit 0
npx prettier --check             all matched files clean
check:file-size                  OK (new files 423 / 57 lines, cap 800)
check:dead-code                  OK (222, baseline 227)
check:test-discovery             OK

The gate bites. Removing the chatCore anchor (the multi-line shape) and re-running:

1 source-bound test variable(s) carry ONLY negative guards:

  tests/unit/proxy-bypass-scope-guard-3226.test.ts
    variable "chatCore" is guarded only by negative assertions (line 51)
    fix: add one positive anchor, e.g. assert.match(chatCore, /export default function SomeComponent/);

Two synthetic cases in the gate's own suite lock the multi-line behaviour in both directions:
a multi-line negative guard must be reported, and a multi-line positive anchor must not be —
the second one catches an over-merging folding bug that would invent violations.

Known limitation (deliberate, documented in the file header)

Variables are keyed by name per file, not per scope, so a file declaring const source in two
separate test bodies pools both sets of usages and one anchor can mask the other's negative-only
guard (tests/unit/8395-plugin-hooks-fire.test.ts is the current example). This errs toward
under-reporting — it never invents a violation — and closing it needs scope-aware parsing that
is not worth the complexity here.

Refs #8617

MumuTW added 2 commits July 26, 2026 17:30
A negative guard — assert.doesNotMatch(src, /x/) or src.includes(x) === false —
passes against an empty string. Once the code it guards is extracted into another
file the parent no longer contains the string, so the assertion keeps passing while
protecting nothing. The regression coverage is deleted with no test turning red,
which is exactly the failure mode the god-file decomposition campaign (diegosouzapw#8617) is
about to trigger 90-odd times.

Adds tests/unit/source-scanner-guards.test.ts: a hard gate (no baseline, no
allowlist) requiring every test variable bound to project source to carry at least
one positive anchor. Classification runs on logical statements with strings, regexes
and comments blanked out, so a guard wrapped across lines cannot slip past — that
folding is what exposed 3 of the 7 violations.

Fixes all 7 violations across 6 files with one stable top-level export anchor each.
Two were security scope guards held only by multi-line negative assertions: the SSRF
guards on /api/sync/initialize (diegosouzapw#323) and the proxy-bypass guards on chatHelpers.ts
and chatCore.ts (diegosouzapw#3226) — the latter anchored on handleChatCore precisely because
that file is a decomposition target.

Adds tests/_helpers/readSrc.ts, a repo-root-relative reader that throws on a missing
or empty file instead of returning "".

Refs diegosouzapw#8617
Same tip fix as diegosouzapw#8657 so Merge integrity is green without waiting for
that PR to land. Regenerated via generate-agent-skills --apply.
@diegosouzapw
diegosouzapw merged commit b4776a2 into diegosouzapw:release/v3.8.49 Jul 28, 2026
15 checks passed
diegosouzapw added a commit that referenced this pull request Jul 28, 2026
…e lost credits

Aggregates every pending changelog.d fragment into the [3.8.49] section and
regenerates the contributors table from the reconciled bullets.

Three fixes this surfaced:

- The [3.8.49] section had no `### 📝 Maintenance` heading, so the aggregator's
  findIndex matched the first one in the file — inside [3.8.47] — and would have
  filed 92 maintenance bullets under the wrong release. Added the heading to the
  living section; [3.8.47] stays at its original 234 bullets.

- 46 bullets carried no PR/issue reference. Fragments may keep the number only in
  the filename (`<N>-slug.md`), which the aggregator does not copy into the bullet,
  so the link and the credit were dropped on aggregation. Restored, scoped strictly
  to the [3.8.49] range.

- 9 external contributors lost their attribution that way and are credited again:
  @MisileLab (#8566), @MumuTW (#8619), @epsilonode (#8724), @hppsc1215 (#8835),
  @sumanxg (#8837, #8856), @TitoTFP (#8838), @HouMinXi (#8842, #8845).

Contributors table: 84 → 155 entries, no one removed. 42 i18n mirrors synced.
check:changelog-integrity green — no base bullet lost.
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
@MumuTW
MumuTW deleted the test/split-safety-guards branch September 5, 2026 10:19
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nly (diegosouzapw#8619)

* test(quality): fail loudly when a source-scanning guard is negative-only

A negative guard — assert.doesNotMatch(src, /x/) or src.includes(x) === false —
passes against an empty string. Once the code it guards is extracted into another
file the parent no longer contains the string, so the assertion keeps passing while
protecting nothing. The regression coverage is deleted with no test turning red,
which is exactly the failure mode the god-file decomposition campaign (diegosouzapw#8617) is
about to trigger 90-odd times.

Adds tests/unit/source-scanner-guards.test.ts: a hard gate (no baseline, no
allowlist) requiring every test variable bound to project source to carry at least
one positive anchor. Classification runs on logical statements with strings, regexes
and comments blanked out, so a guard wrapped across lines cannot slip past — that
folding is what exposed 3 of the 7 violations.

Fixes all 7 violations across 6 files with one stable top-level export anchor each.
Two were security scope guards held only by multi-line negative assertions: the SSRF
guards on /api/sync/initialize (diegosouzapw#323) and the proxy-bypass guards on chatHelpers.ts
and chatCore.ts (diegosouzapw#3226) — the latter anchored on handleChatCore precisely because
that file is a decomposition target.

Adds tests/_helpers/readSrc.ts, a repo-root-relative reader that throws on a missing
or empty file instead of returning "".

Refs diegosouzapw#8617

* docs(changelog): number the fragment for diegosouzapw#8619

* chore(skills): sync cli-backup-sync SKILL.md with catalog

Same tip fix as diegosouzapw#8657 so Merge integrity is green without waiting for
that PR to land. Regenerated via generate-agent-skills --apply.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…e lost credits

Aggregates every pending changelog.d fragment into the [3.8.49] section and
regenerates the contributors table from the reconciled bullets.

Three fixes this surfaced:

- The [3.8.49] section had no `### 📝 Maintenance` heading, so the aggregator's
  findIndex matched the first one in the file — inside [3.8.47] — and would have
  filed 92 maintenance bullets under the wrong release. Added the heading to the
  living section; [3.8.47] stays at its original 234 bullets.

- 46 bullets carried no PR/issue reference. Fragments may keep the number only in
  the filename (`<N>-slug.md`), which the aggregator does not copy into the bullet,
  so the link and the credit were dropped on aggregation. Restored, scoped strictly
  to the [3.8.49] range.

- 9 external contributors lost their attribution that way and are credited again:
  @MisileLab (diegosouzapw#8566), @MumuTW (diegosouzapw#8619), @epsilonode (diegosouzapw#8724), @hppsc1215 (diegosouzapw#8835),
  @sumanxg (diegosouzapw#8837, diegosouzapw#8856), @TitoTFP (diegosouzapw#8838), @HouMinXi (diegosouzapw#8842, diegosouzapw#8845).

Contributors table: 84 → 155 entries, no one removed. 42 i18n mirrors synced.
check:changelog-integrity green — no base bullet lost.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants