fix(review): stop false-positiving on transitive bumps and workspace links

[JSONbored]

Summary

Adversarial review of the already-merged #2676 (issue #2563) found two confirmed, empirically-reproduced false-positive bugs that directly violate the issue's own acceptance criterion ("a normal npm install-driven lockfile update does not false-positive"):

• Transitive dependency bumps misclassified as tamper: packagesWithManifestVersionChange() only scanned package.json's own dependency blocks (direct dependencies). The vast majority of any real package-lock.json's entries are transitive dependencies, which never appear in any package.json at all — so the check misfired on nearly every ordinary npm install/npm update.
• npm workspace-local packages misclassified as off-registry: NPM_REGISTRY_HOST_RE required an exact https://registry.npmjs.org/ prefix. A workspace's own local packages (e.g. this repo's packages/gittensory-mcp, apps/gittensory-ui) have a resolved field that's a relative filesystem path, not a URL at all — it failed the registry-host check trivially and got flagged the moment that entry's resolved line changed (entirely routine).

Fix: replaced the package.json cross-reference with a self-contained signal — every package-lock.json entry (direct AND transitive) carries its own version/resolved/integrity trio, and a genuine install/update always bumps all three together. The tamper signal now compares a resolved/integrity change against that SAME entry's own "version" line within the same diff, which covers transitive and direct dependencies uniformly and is a more specific, self-contained tamper tell (a hand-edited resolved/integrity with the version left unchanged) than the old cross-file check. Also exempted non-http(s):// resolved values from the off-registry check.

Addresses gate-review findings on #2676.

Test plan

• npm run typecheck
• npx vitest run test/unit/lockfile-tamper.test.ts test/unit/lockfile-tamper-wiring.test.ts test/unit/queue.test.ts — 417/417 pass, including 3 new regression tests (transitive-only bump does not trigger, workspace-local resolved path does not trigger, a genuine off-registry URL still triggers) and one existing test updated to reflect the new, more accurate design (a changed integrity with no version bump is now correctly flagged even when the package is also being dropped from package.json — previously silently ignored)

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	gittensory-ui	0160d04	Commit Preview URL Branch Preview URL	Jul 03 2026, 06:28 AM

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.13%. Comparing base (dc90617) to head (9471412).
⚠️ Report is 14 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2692      +/-   ##
==========================================
- Coverage   96.13%   96.13%   -0.01%     
==========================================
  Files         241      241              
  Lines       27033    27032       -1     
  Branches     9822     9816       -6     
==========================================
- Hits        25988    25987       -1     
  Misses        433      433              
  Partials      612      612              

Files with missing lines	Coverage Δ
src/review/lockfile-tamper.ts	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[loopover-orb]

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

_{Review updated: 2026-07-03 06:51:04 UTC}

2 files · 1 AI reviewer · 1 blocker · readiness 93/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

• AI reviewers agree on a likely critical defect: src/review/lockfile-tamper.ts:101 keys candidates by collapsed package name, so package-lock entries like node_modules/foo and node_modules/bar/node_modules/foo are merged and a legitimate version bump in one entry can suppress an unbumped resolved/integrity change in the other. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
The change moves the tamper signal from package.json dependency ranges to each lockfile entry's own version line and correctly exempts non-URL workspace resolved values. That is the right direction for transitive bumps, but the implementation does not actually keep candidates per lockfile entry: it collapses every nested copy of the same package name into one record, which breaks the new same-entry invariant and can hide a real unbumped resolved/integrity edit. The emitted advisory text also still describes the old package.json-based rule.

Blockers

• src/review/lockfile-tamper.ts:101 keys candidates by collapsed package name, so package-lock entries like node_modules/foo and node_modules/bar/node_modules/foo are merged and a legitimate version bump in one entry can suppress an unbumped resolved/integrity change in the other.

Nits — 5 non-blocking

• src/review/lockfile-tamper.ts:193 still says "without a matching package.json version bump" even though the check now compares against the same lockfile entry's own version line.
• test/unit/lockfile-tamper.test.ts lacks coverage for duplicate nested package-lock entries with the same package name but different lockfile paths, which is the key edge case for the new same-entry comparison.
• In src/review/lockfile-tamper.ts, key by the lockfile object key/path and store the display package name separately, or otherwise include the full package-lock entry path in MutableCandidate so version/resolved/integrity are reconciled only within the same entry.
• Add a unit test in test/unit/lockfile-tamper.test.ts with two copies of the same package name where one nested entry bumps version normally and the other changes integrity without a version change; the finding should still include that package.
• Update the finding detail/action text in src/review/lockfile-tamper.ts to say "without a matching lockfile entry version change" so contributors are told the rule the code actually enforces.

Why this is blocked

• src/review/lockfile-tamper.ts:101 keys candidates by collapsed package name, so package-lock entries like node_modules/foo and node_modules/bar/node_modules/foo are merged and a legitimate version bump in one entry can suppress an unbumped resolved/integrity change in the other.

CI checks failing

• codecov/patch — 95.00% of diff hit (target 99.00%)

Signal	Result	Evidence
Code review	❌ 1 blocker	1 reviewer
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 61 registered-repo PR(s), 52 merged, 489 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 61 PR(s), 489 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository registration is not available in the local Gittensory cache.
• Public profile languages: not available
• Official Gittensor activity: 61 PR(s), 489 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Explain no-issue PR.
• No action.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review

[JSONbored]

Both findings investigated:

1. Nested-entry collision (blocker) — confirmed real. scanPackageLockPatch keyed candidates by the bare package name, so two distinct package-lock.json entries for the same package at different node_modules nesting depths (e.g. node_modules/foo and node_modules/bar/node_modules/foo, which npm creates when dependents need incompatible versions) were merged into one shared tamper-tracking record. A legitimate bump on one entry could mask an unbumped, tampered resolved/integrity edit on the other. Fixed by keying the map on the full entry path instead of the bare name; the bare name is still used for display only. Added regression tests covering both processing orders.

2. CI: codecov/patch 95% vs 99% — the gap was a single unreachable branch in versionChanged()'s dead both-undefined guard (mathematically redundant: removed !== added already evaluates to false when both are undefined). Simplified the function; coverage is now 100% lines/branches/statements/functions on the touched file.

3. Separate security-scanner P1 finding (context lines treated as removed versions) — investigated and confirmed a false positive: scanPackageLockPatch already has an earlier unconditional guard in the same loop (if (!currentEntryKey || !currentPackageName || line.sign === " ") continue;) that filters out every context-sign line before the version/resolved/integrity match logic runs, so a context "version" line can never reach the removedVersion assignment. This predates the current PR (present since the original feat(gate): add a lockfile-tamper-risk check #2676). No code change was needed for this one, since the guard already makes it structurally unreachable — confirmed independently by tracing every continue in the loop and by checking patchLines() cannot yield any sign outside "+"/"-"/" ".

All 30 tests in lockfile-tamper.test.ts + lockfile-tamper-wiring.test.ts pass, typecheck is clean, coverage is 100%.