Skip to content

fix(tooling): count published executable source outside src/ in the changeset presence gate - #6073

Merged
yinlianghui-tw merged 1 commit into
mainfrom
claude/issue-5733-changeset-presence-population
Aug 24, 2026
Merged

yinlianghui-tw merged 1 commit into
mainfrom
claude/issue-5733-changeset-presence-population

Conversation

@yinlianghui-tw

@yinlianghui-tw yinlianghui-tw commented Aug 24, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #5733

Verification union run at 50365b547 (the final commit).

Note on spelling in this body: the package directory is written PKG/… rather than with angle brackets. GitHub's body sanitizer strips short angle-bracket fragments on every edit — even inside backticks — and the first version of this description lost every one of them.

Where the population test actually is

The card cites scripts/check-changeset-presence.mjs:416. That line number is stale — the file has been edited since. Located by code (git grep -n 'path.startsWith'), the population test was at line 467 on a1c41c516, inside classifyChangedPaths:

if (path.startsWith(`${directory}/src/`)) {

The premise, re-verified on current origin/main

Appended one HTML comment to apps/console/index.html on a clean a1c41c516 worktree and ran the gate unchanged:

Compared the working tree with a1c41c516 (merge-base with origin/main): 1 file(s) changed,
0 of them under the src/ of a package the release covers, 0 under a package changesets
ignores, 0 changeset(s) added.
✅  No source of a released package changed in this range, so no changeset is owed.

Exit 0. The premise holds: a shipped, executable file outside a package's src/ changes with no changeset required. That file is compiled into the published dist/ and carries two inline classic scripts that run in every user's browser during parse.

The judgment: what the population should be

Picked by published AND executable — both words load-bearing, because the population has a wrong answer in each direction (too narrow ships undeclared executable code; too wide makes the gate noise people silence with empty changesets). The rule is stated in the file's header in checkable prose and implemented as isPublishedSource, same clause order.

A changed file counts when any of these holds:

  • (a) it is under PKG/src/ — unchanged, and still all-of-it;
  • (b) it is PKG/index.html, the bundler's HTML entry, compiled into the published dist/;
  • (c) the package's own package.json files list publishes it verbatim (apps/console's root plugin.ts, which is that package's main).

…minus documentation and licences (*.md, LICENSE*), which ship in the tarball but are not behaviour. Clause (a) answers first and unconditionally, so the subtraction can never remove something src/ already covered — nothing guarded before is guarded less now.

On this tree the widening adds exactly three files and no fourth: apps/console/index.html, apps/console/plugin.ts, packages/runner/index.html. Measured by running classifyChangedPaths over every tracked file in the repo: 3330 guarded, 3 of them outside src/.

Boundaries stated in the header rather than left as silences — PKG/public/ (published verbatim by Vite, but static, not code), PKG/*-preview.html and PKG/demo/ (dev-server pages Vite never builds), and package.json itself (clause (c) reads it, does not match it). Clause (b) tests the name, not the build script's spelling: deriving it from a script would fail silently the day the spelling changes, which is this card's own defect class.

No convention was invented that the repo has not settled — files is npm's own published-surface declaration and index.html is Vite's documented default build entry; both packages with a root index.html build it (apps/console: tsc && vite build && pnpm build:plugin; packages/runner: vite build).

Non-vacuity — executable, all three legs

Run against throwaway fixture repositories in scripts/__tests__/check-changeset-presence.test.ts (new section 2b), plus the real tree.

  • Positive — apps/console/index.html changes, no changeset ⇒ red naming that file.
  • Negative control 1 — same change with a changeset ⇒ green (declares 1 changeset).
  • Negative control 2 — four non-shipped neighbours in the same package directory (public/manifest.json, preview-gallery.html, README.md, vitest.config.ts) ⇒ green, 4 file(s) changed, 0 of them published source. The widening did not become a blanket.

Plus a clause table over isPublishedSource, a clause-(c) leg on plugin.ts, a leg proving a files-published README.md still does not count, and a real-repo leg pinning the three-files-and-no-fourth answer.

These tests can fail

Ablated isPublishedSource back to the pre-fix return relative.startsWith('src/') (mutation confirmed on disk: ABLATION-5733 marker present ×1, removed text relative === HTML_ENTRY absent ×0) and re-ran from the repo root:

 Test Files  1 failed (1)
      Tests  5 failed | 33 passed (38)
     × POSITIVE — an HTML entry outside src/ changes with no changeset, and the gate goes red naming it
     × NEGATIVE CONTROL 1 — the same change WITH a changeset is green
     × a package-ROOT file the `files` list publishes verbatim counts — clause (c)
     × states the rule as a clause table `isPublishedSource` must satisfy
     × answers THIS repository the same way — the three files the widening adds, and no fourth

The two new tests that stayed green under ablation are both stays-green assertions (negative control 2, and the published-README leg), which is the correct direction for them; the clause table covers those branches and does go red. The script was restored by an EXIT INT TERM trap and verified byte-identical (diff -q clean).

Reverse-verification against apps/console/index.html

Direction predicted before running: red, naming the file. Observed on the real repo at a1c41c516 + this fix:

Compared the working tree with a1c41c516 (merge-base with origin/main): 2 file(s) changed,
1 of them published source of a package the release covers, 0 under a package changesets
ignores, 0 changeset(s) added.

❌  1 source file(s) of 1 released package(s) changed, and this change adds no changeset:

      @object-ui/console
          apps/console/index.html

Exit 1. Mutation proven on disk before each read (grep -c 'objectui-5733-PROBE' = 1, byte count 9460 → 9505). Restored with git checkout HEAD -- apps/console/index.html (not the bare form); restoration proven by empty git diff HEAD for the path, zero probe hits, and byte count back to 9460.

Same legs also run on the real tree for apps/console/plugin.ts (red naming it) and for five non-shipped files at once (green, 0 of them published source).

Gate verdicts — each quoted from its own output

All at 50365b547, run from the repo root:

gate verdict line
scripts/__tests__/check-changeset-presence.test.ts Test Files 1 passed (1) / Tests 38 passed (38)
whole scripts/__tests__/ suite Test Files 64 passed (64) / Tests 1770 passed (1770)
check-changeset-presence.mjs (this PR's own diff) ✅ No source of a released package changed in this range, so no changeset is owed.
check-changeset-fixed.mjs ✅ All workspace packages are in the changeset fixed group.
check-changeset-no-major.mjs ✅ No changeset declares a major bump.
pnpm check:control-bytes ✅ check-control-bytes: OK (scanned 5016 tracked text file(s); skipped 85 binary).
node scripts/check-lint-coverage.mjs ✅ lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).
node scripts/check-type-check-coverage.mjs ✅ type-check coverage: 45/46 via type-check, 0 via their own build, 0 known-broken (0 errors outstanding), 1 not compiled.
pnpm type-check:scripts exit 0, no diagnostics
eslint --no-inline-config on both changed files 2 files linted, 0 errors, 0 warnings

On the eslint reading: this is not a narrowing. Root pnpm lint is turbo run lint, a per-package fan-out; scripts/ is not a package, so pnpm lint does not reach either changed file. The targeted run uses the root eslint.config.js with --no-inline-config and reports 2 files from --format json — the complete lint population for this diff.

node-esm-load-gate.yml is a nightly/dispatch workflow that does a real build and is not a per-PR gate for this diff; it is red in this unbuilt worktree with @object-ui/console: no-build-output — ./plugin.js does not exist after the build, which names a file this diff does not touch (the diff is two files under scripts/ plus a changeset).

Changeset

The gate says none is owed for this diff (scripts/ is no package's published source), so this is not the infinite-regress case. Added an empty-frontmatter declaration anyway, which is objectui's way of saying "ships nothing" — the gate's own documented first-class pass.

The one boundary a reviewer may want to move

PKG/public/ is genuinely published — Vite copies it verbatim into dist/, so apps/console/public/manifest.json reaches every user. It is excluded here because it is not executable, and demanding a declaration for a favicon swap is the friction that gets a gate answered with empty changesets nobody reads. That reasoning is written into the header as a considered exclusion rather than left silent, so moving the line is a one-clause edit if the maintainer reads it the other way.

Out of scope, untouched

check-changeset-fixed, check-changeset-no-major, #5775's lane-job mirror of readChangesetState, and the release workflow. No gate was weakened: the only behavioural change to an existing verdict is the summary line's wording, and clause (a) preserves the prior guarded set exactly.


Generated by Claude Code

…hangeset gate

check-changeset-presence.mjs counted a changed file as a package's source only
when it sat under <pkg>/src/. apps/console/index.html does not: it is the
console's HTML entry, compiled into the published dist/, and it carries two
inline classic scripts that run in every user's browser during parse. A pull
request editing only that file changed shipped console behaviour while the gate
reported "No source of a released package changed in this range".

The population is now the package's published, executable source:

  (a) under <pkg>/src/ — unchanged, and still all-of-it; or
  (b) <pkg>/index.html, the bundler's HTML entry; or
  (c) published verbatim by the package's own package.json `files` list,

minus documentation and licences, which ship in the tarball but are not
behaviour. Clause (a) answers first, so nothing guarded before is guarded less
now. On this tree the widening adds exactly three files —
apps/console/index.html, apps/console/plugin.ts, packages/runner/index.html —
and nothing else in any package directory.

Non-vacuity is executable: the positive leg goes red naming the file, the same
change with a changeset goes green, and four non-shipped neighbours in the same
package directory (public/, a preview page, the README, the vitest config) stay
green so the widening did not become a blanket.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe
@github-actions github-actions Bot added the tests label Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

PM: ACCEPT — ruling A on the open question, and the ablation is the reason this lands without a second look

Ruling: leave PKG/public/ out, as implemented

Your reasoning is the right one and I am adopting it. The population's failure mode in the wide direction is not a missed changeset — it is the gate ceasing to be read: a favicon or logo swap demanding a declaration trains authors to answer with an empty changeset they did not think about, and at that point the gate is decorative for every change including the ones that matter. Against that, manifest.json is one file among four static assets in the only public/ a released package has.

What makes A safe rather than merely convenient is that you argued the exclusion in the gate header instead of leaving it silent. B is now a one-clause edit with the reasoning already on the page for whoever disagrees, and a future reader can see the boundary was considered rather than overlooked. That is the difference between a scoped gate and an arbitrary one. Flagged to the maintainer, not blocking.

The ablation is the part that earns trust

New tests are the easiest thing in the world to write vacuously — they pass, and nobody checks they could fail. You reverted isPublishedSource to return relative.startsWith('src/') and showed 5 of the new assertions go red (Tests 5 failed | 33 passed (38)), with the mutation proven on disk before any result was read (grep -c ABLATION-5733 = 1 injected, grep -c 'relative === HTML_ENTRY' = 0 removed) rather than inferred from an editor's exit code.

And you handled the two that stayed green honestly instead of hiding them: both are stays-green assertions, so green under ablation is the correct direction for them, and their branches are covered by the clause table that did redden. Explaining why a test did not move is worth more than a clean number.

You also correctly noted there is no build leg — the test imports ../check-changeset-presence.mjs by relative path, so no dist/ sits between the edit and the thing under test. Saying why a rebuild was not owed is exactly right; this lane has lost a round to a stale artifact today.

The line number was stale, as predicted

Card said :416; it was at :467 inside classifyChangedPaths. You located it by code, which is what the order asked. Premise re-verified before touching anything: one appended comment in apps/console/index.html against the unmodified gate produced "1 file(s) changed, 0 of them under the src/ of a package the release covers", exit 0 — the hole demonstrated live rather than quoted from the card.

The population rule is defensible because it is borrowed, not invented

The order warned that this card turns on a judgment with a wrong answer in each direction. You resolved it without inventing a convention: files is npm's own published-surface declaration, and index.html is Vite's documented default build entry — and you verified both packages with a root index.html actually build it, rather than assuming. Clause (a) answers first, so nothing previously guarded is guarded less. Documentation and licences excluded because they ship but are not behaviour.

The measured impact is the number that makes this reviewable: 3330 files guarded, exactly three added — apps/console/index.html, apps/console/plugin.ts, packages/runner/index.html. A widening that names its entire delta is one a reviewer can actually check.

Non-vacuity carried all three required legs plus a clause table, a files-published README.md leg proving documentation still does not count, and a real-tree leg pinning three-and-no-fourth. No infinite regress: the gate on this PR's own diff exits 0.

Scope notes, both correct

  • eslint is not a narrowing and you explained why rather than asserting it: root pnpm lint is turbo run lint, a per-package fan-out, and scripts/ is not a package — so pnpm lint reaches neither changed file. The targeted run uses the root config and its own --format json count of 2 is the complete population for this diff.
  • node-esm-load-gate.yml red in an unbuilt worktree, naming @object-ui/console: no-build-output — ./plugin.js does not exist after the build — a file this diff does not touch, and a nightly/dispatch workflow rather than a per-PR gate. Right call to report it with its reason instead of either hiding it or chasing it.

Process

Sanitizer ate the report marker again, and this time also every <pkg> fragment in the PR body even inside backticks — you worked around it by spelling the directory PKG/. That is the sixth dev hit this round. Lane-wide tooling behaviour, tracked with the rest.

Landing

⏳ Six checks still running (four shards, Lint, Type Check); everything completed so far is success/skipped. Not turned ready yet — the self check-in verifies every-check-green and lands it.


Generated by Claude Code

@yinlianghui-tw
yinlianghui-tw marked this pull request as ready for review August 24, 2026 15:52
@yinlianghui-tw
yinlianghui-tw added this pull request to the merge queue Aug 24, 2026
Merged via the queue into main with commit dd8b023 Aug 24, 2026
22 checks passed
@yinlianghui-tw
yinlianghui-tw deleted the claude/issue-5733-changeset-presence-population branch August 24, 2026 16:05
This was referenced Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants