Repository navigation
fix(user.js): derive SCRIPT_VERSION from GM_info, enforce @version parity - #295
Merged
Merged
Conversation
The three userscripts are about to read their version from Tampermonkey's
GM_info instead of a hand-maintained literal. The test sandbox in
tests/helpers/browser-shim.js evaluates each whole .user.js through
vm.runInContext and defines no GM_info, so that change would abort every
IIFE before it exports its window.__PDB_TEST__ hooks -- surfacing as a
misleading "did not expose window.<hooksKey>" error across 17 test files.
Land the stub on its own first so the scripts have somewhere to read from
when they switch over. It is inert until then.
Changes:
- Added GM_info: { script: { version: '0.0.0-test' } } to the vm sandbox,
alongside the existing localStorage, fetch and MutationObserver fakes.
- Used a fixed sentinel rather than the real header version, so a later
test can assert the GM_info path is genuinely the one being taken.
Security:
- N/A -- test harness only, never inlined into a generated .user.js.
Testing:
- node --test from user.js/: 470 tests, 469 pass, 1 skipped (live tests
are opt-in). Identical to the pre-commit baseline, as expected for an
addition nothing reads yet.
Backwards Compatibility:
- No production code touched. The sandbox gains a key no existing test
references.
Assisted-by: Claude:claude-opus-5
SCRIPT_VERSION was a hand-maintained string constant that had drifted two patch releases behind the @Version this script actually ships under (2.0.223 against a 2.0.225 header). That is not cosmetic here: CP stamps SCRIPT_VERSION into four persisted audit logs -- orgUpdateAuditLog, ixlanRenumberAuditLog, ixfMemberAuditLog and ixlanConflictResolveAuditLog -- which are the forensic record of every destructive write the IXLAN Renumber, IX-F Merge and Conflict Resolver flows perform. Entries were therefore attributing DELETEs to a build that never ran. Tampermonkey injects GM_info into every userscript regardless of @grant, and GM_info.script.version is bound at install time to the @Version header, so reading it removes the constant as an independent source of truth rather than merely resyncing it. Changes: - Replaced the SCRIPT_VERSION literal with a read of GM_info.script.version, guarded by a typeof check that falls back to "0.0.0-unknown" outside Tampermonkey (Greasemonkey 4 exposes GM.info, not GM_info). - Exported SCRIPT_VERSION on window.__pdbCpTestHooks__ so the value can be asserted from the test suite. - Regenerated peeringdb-cp-consolidated-tools.user.js. - Kept the expression inline rather than extracting a lib/admincom-common.js helper: the @include marker sits below the constant, so a shared helper would depend on function-declaration hoisting across the inlined block. Security: - Improves audit-log integrity. Version attribution on recorded DELETEs is now derived from the installed script rather than a constant nobody was obliged to update. - No new grant, no new network destination, no change to what is persisted beyond the value of an existing field. Testing: - node --test from user.js/: 473 tests, 472 pass, 1 skipped (live tests are opt-in), up from 470/469 with the new assertion. - Added a case asserting hooks.SCRIPT_VERSION equals the sandbox stub, which fails if the literal returns or if the fallback branch silently engages. - Verified the guard is not vacuous: removing GM_info from the shim fails that one case with a legible value mismatch rather than erroring out. - node --check passes on the regenerated .user.js. Backwards Compatibility: - Audit-log entries keep the same "version" field and shape; only the value changes, and only to the correct one. Existing persisted entries are untouched. - No storage key, cache namespace, module ID or route boundary changed. Assisted-by: Claude:claude-opus-5
Same defect as the preceding CP commit: SCRIPT_VERSION was a hand-maintained literal that had drifted four patch releases behind the @Version this script ships under (1.1.34 against a 1.1.38 header). FP surfaces the value in its self-check console warning and its init debug line, so both were reporting a build that never ran -- misleading exactly when someone is reading them to diagnose something. Reading GM_info.script.version removes the constant as an independent source of truth. Tampermonkey injects GM_info regardless of @grant and binds it to the @Version header at install time. Changes: - Replaced the SCRIPT_VERSION literal with a read of GM_info.script.version, guarded by a typeof check falling back to "0.0.0-unknown" outside Tampermonkey. - Exported SCRIPT_VERSION on window.__pdbFpTestHooks__. - Regenerated peeringdb-fp-consolidated-tools.user.js. Security: - N/A -- FP reports the version to the console only; it does not persist it to an audit log or transmit it anywhere. Testing: - node --test from user.js/: 473 tests, 472 pass, 1 skipped (live tests are opt-in). - Added a case asserting hooks.SCRIPT_VERSION equals the sandbox stub. - node --check passes on the regenerated .user.js. Backwards Compatibility: - Console output keeps its existing format; only the version value changes, and only to the correct one. - No storage key, cache namespace, module ID or route boundary changed. Assisted-by: Claude:claude-opus-5
Completes the version-drift fix across all three scripts. DeskPro's case
differs from CP's and FP's: its SCRIPT_VERSION was declared and then never
read anywhere, so nothing was reporting a wrong version -- it was reporting
none at all. The constant had still drifted (1.7.7 against a 1.7.9 header),
which is what a value nobody consumes tends to do.
Rather than delete it, wire it up the way the other two scripts already do,
so all three answer "which build is this?" the same way when someone turns
diagnostics on.
Changes:
- Replaced the SCRIPT_VERSION literal with a read of GM_info.script.version,
guarded by a typeof check falling back to "0.0.0-unknown" outside
Tampermonkey.
- Added a dbg("init", `v${SCRIPT_VERSION}`, { path }) line at the top of
init(), mirroring the equivalent lines in CP and FP. dbg() is already in
scope via the lib/admincom-common.js include, and DP defines both
MODULE_PREFIX and isFeatureEnabled that it depends on.
- Used { path: location.pathname } as context, since DP does no route
dispatch and has no route-context object to draw CP's entity/entityId or
FP's type/id from.
- Exported SCRIPT_VERSION on window.__pdbDpTestHooks__.
- Regenerated peeringdb-deskpro-tools.user.js.
Security:
- N/A -- the new output is console-only and gated behind the debugMode
feature flag plus the pdbAdmincom.debug localStorage flag, both of which
are opt-in. DP's debugMode defaults to false, matching FP.
Testing:
- node --test from user.js/: 473 tests, 472 pass, 1 skipped (live tests are
opt-in).
- Added a case asserting hooks.SCRIPT_VERSION equals the sandbox stub.
- node --check passes on the regenerated .user.js.
Backwards Compatibility:
- Silent by default: the init line produces no output unless a user has
already enabled both debug flags.
- No storage key, cache namespace or DeskPro DOM behavior changed.
Assisted-by: Claude:claude-opus-5
Tampermonkey polls the .meta.js to decide whether an update exists, but installs the .user.js generated from the .src.js. When those two headers disagree the update check and the shipped code describe different versions: admins either never see an update that exists, or install one whose reported version is wrong. AGENTS.md asked a human to bump both and nothing enforced it, so all three scripts had drifted -- which is how the stale SCRIPT_VERSION constants this series started with went unnoticed. With SCRIPT_VERSION now derived from GM_info.script.version, the header is the single source of truth and this pair is the last place drift can hide. All three pairs currently agree, so the check passes on introduction. Changes: - check_version_parity() compares the two headers and returns a message rather than raising, so one run reports every bad pair instead of stopping at the first. It runs on every invocation, not only --check: catching a mismatch only in CI means the person who caused it never sees it. - A missing .meta.js is an error, not a silent skip. A script without a manifest never auto-updates, and skipping would make the check quietly cover less than it appears to. - A header carrying no @Version at all gets its own message -- nothing to compare is a different problem from two values disagreeing, and a shared message would send the reader hunting for a value that is not there. - header_block() limits the search to the ==UserScript== block. A .src.js is ~11,600 lines of JavaScript and a stray `// @version` comment below it should not be a candidate. - VERSION_RE tolerates the differing header alignment between the two files (6 spaces in CP, 9 in DP); a fixed-width slice would have read DP's value as empty, and empty == empty passes. - AGENTS.md and docs/CONCERNS.md record that the rule is now enforced and that SCRIPT_VERSION is derived, never maintained. Security: - N/A -- build-time only. Testing: - node --test: 481 tests, 480 pass, 1 skipped (live tests are opt-in). - New tests/build-version-parity.test.js covers matching versions, a mismatch, both --check and plain invocation, two bad pairs in one run, a missing manifest, a missing @Version line on either side, and a stray @Version below the metadata block. - New tests/helpers/build-script-runner.js holds the spawn harness, so the include-marker guard's tests can share it rather than duplicating. - Verified by hand against the real tree first: a bumped DP manifest, two bad pairs at once, a removed manifest and a removed @Version line each produced the expected message and exit 1. - Guards proven non-vacuous by reverting each: no check (6 failures), missing-manifest skipped (2), whole-file scan (2). The whole-file case initially stayed green -- the first regex match wins and the header precedes any stray line, so a fixture keeping its header proved nothing. Rewritten to omit the header version, where whole-file scanning reports a confident but wrong mismatch. - verify.yml already runs build_userscripts.py --check, so no workflow change is needed. Backwards Compatibility: - Generated output is unchanged; all three .user.js files are byte identical. Only a previously silent inconsistency now fails the build. Assisted-by: Claude:claude-opus-5
netravnen
changed the title
netravnen
marked this pull request as ready for review
August 19, 2026 22:26
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.
Stack created with GitHub Stacks CLI • Give Feedback 💬