feat(ci): semantic package-lock.json diff as an upserted PR comment (#65206)

git diff on a lockfile is unreadable: npm reorders entries, rewrites
integrity hashes, and moves packages between nesting levels, so a
one-line package.json bump produces a thousand-line textual diff.

scripts/ci/lockfile_diff.py instead parses the `packages` map out of
both versions of every tracked package-lock.json (via `git show`),
reduces each to {install path: version}, and set-diffs the maps —
reorder/hash churn vanishes, leaving only actual version movement
(added / removed / updated, with nested dedup copies tracked
separately).

The lockfile-diff workflow posts the result as a Markdown table in a
PR comment gated behind a hidden marker: subsequent pushes PATCH the
existing comment instead of stacking new ones, and a push that reverts
all lockfile changes updates the comment to say so. Advisory only —
never fails on findings; fork PRs (read-only token) degrade to a
warning.

Wired through the ci.yml orchestrator with a new npm_lock lane in
classify_changes.py (fails open on .github/ changes per the existing
contract).

.github/actions/detect-changes/action.yml

@@ -24,6 +24,9 @@ outputs:
   deps:
     description: Check pyproject.toml dependency upper bounds.
     value: ${{ steps.classify.outputs.deps }}
+  npm_lock:
+    description: Post/update the semantic package-lock.json diff PR comment.
+    value: ${{ steps.classify.outputs.npm_lock }}
   mcp_catalog:
     description: Require MCP catalog security review label.
     value: ${{ steps.classify.outputs.mcp_catalog }}

.github/workflows/ci.yml

@@ -41,6 +41,7 @@ jobs:
       site: ${{ steps.classify.outputs.site }}
       scan: ${{ steps.classify.outputs.scan }}
       deps: ${{ steps.classify.outputs.deps }}
+      npm_lock: ${{ steps.classify.outputs.npm_lock }}
       docker_meta: ${{ steps.classify.outputs.docker_meta }}
       mcp_catalog: ${{ steps.classify.outputs.mcp_catalog }}
       ci_review: ${{ steps.classify.outputs.ci_review }}
@@ -101,6 +102,12 @@ jobs:
     needs: detect
     uses: ./.github/workflows/uv-lockfile-check.yml
 
+  lockfile-diff:
+    name: package-lock.json diff
+    needs: detect
+    if: needs.detect.outputs.event_name == 'pull_request' && needs.detect.outputs.npm_lock == 'true'
+    uses: ./.github/workflows/lockfile-diff.yml
+
   docker-lint:
     name: Lint Docker scripts
     needs: detect
@@ -146,6 +153,7 @@ jobs:
       - history-check
       - contributor-check
       - uv-lockfile
+      - lockfile-diff
       - docker-lint
       - supply-chain
       - osv-scanner

.github/workflows/lockfile-diff.yml

@@ -0,0 +1,98 @@
+name: Lockfile diff
+
+# Advisory PR comment showing the *semantic* diff of package-lock.json
+# changes — which packages were added/removed/updated and their versions.
+# The raw textual diff of a lockfile is unreadable (npm reorders entries
+# and rewrites integrity hashes), so scripts/ci/lockfile_diff.py parses
+# the ``packages`` map at the merge base and at HEAD and set-diffs the
+# {install path: version} maps instead.
+#
+# The comment is upserted: the script embeds a hidden HTML marker and the
+# workflow PATCHes the existing comment when one is found, so a PR gets
+# exactly one lockfile-diff comment that tracks the latest push instead
+# of a stack of stale ones. When a later push reverts all lockfile
+# changes, the comment is updated to say so (deleting it would be more
+# surprising than telling the reviewer it's resolved).
+#
+# Never blocking — this is review signal, not enforcement. Exit is 0 even
+# when commenting fails (fork PRs get a read-only GITHUB_TOKEN).
+
+on:
+  workflow_call:
+
+permissions:
+  contents: read
+  pull-requests: write # post/update the diff comment
+
+concurrency:
+  group: lockfile-diff-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  diff:
+    name: package-lock.json semantic diff
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          fetch-depth: 0 # need history for the merge base
+
+      - name: Generate semantic lockfile diff
+        id: diff
+        run: |
+          set -euo pipefail
+          # Three-dot semantics by hand: diff from the merge base with the
+          # target branch to the PR head, so changes that landed on main
+          # after the branch point don't show up as this PR's doing.
+          BASE_SHA=$(git merge-base "origin/${{ github.base_ref }}" HEAD)
+          echo "Merge base: ${BASE_SHA}"
+          python3 scripts/ci/lockfile_diff.py \
+            --base "$BASE_SHA" \
+            --head HEAD \
+            --output /tmp/lockfile-diff.md
+          if [ -s /tmp/lockfile-diff.md ]; then
+            echo "changed=true" >> "$GITHUB_OUTPUT"
+            cat /tmp/lockfile-diff.md >> "$GITHUB_STEP_SUMMARY"
+          else
+            echo "changed=false" >> "$GITHUB_OUTPUT"
+          fi
+
+      - name: Post or update PR comment
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          REPO: ${{ github.repository }}
+          PR: ${{ github.event.pull_request.number }}
+          CHANGED: ${{ steps.diff.outputs.changed }}
+        run: |
+          set -euo pipefail
+          MARKER='<!-- hermes-lockfile-diff -->'
+
+          # Find our previous comment (paginated — busy PRs exceed one page).
+          EXISTING=$(gh api --paginate "repos/${REPO}/issues/${PR}/comments" \
+            --jq ".[] | select(.body | startswith(\"$MARKER\")) | .id" \
+            | head -1 || true)
+
+          if [ "$CHANGED" != "true" ]; then
+            if [ -n "$EXISTING" ]; then
+              # A previous push changed the lockfile but the latest one
+              # doesn't — update the comment rather than leave stale info.
+              printf '%s\n✅ package-lock.json changes from an earlier push have been reverted — locked versions now match the target branch.\n' "$MARKER" > /tmp/lockfile-diff.md
+            else
+              echo "No lockfile changes and no existing comment — nothing to do."
+              exit 0
+            fi
+          fi
+
+          if [ -n "$EXISTING" ]; then
+            echo "Updating existing comment ${EXISTING}"
+            gh api --method PATCH "repos/${REPO}/issues/comments/${EXISTING}" \
+              -F body=@/tmp/lockfile-diff.md > /dev/null \
+              || echo "::warning::Could not update PR comment (expected for fork PRs — GITHUB_TOKEN is read-only)"
+          else
+            echo "Creating new comment"
+            gh api "repos/${REPO}/issues/${PR}/comments" \
+              -F body=@/tmp/lockfile-diff.md > /dev/null \
+              || echo "::warning::Could not post PR comment (expected for fork PRs — GITHUB_TOKEN is read-only)"
+          fi

scripts/ci/classify_changes.py

@@ -14,6 +14,7 @@ Lanes:
 * ``site``        — Docusaurus + generated skill docs.
 * ``scan``        — supply-chain scan (Python files, .pth, setup hooks).
 * ``deps``        — pyproject.toml dependency bounds check.
+* ``npm_lock``    — semantic package-lock.json diff PR comment.
 * ``mcp_catalog`` — bundled MCP catalog / installer review.
 
 Docker is not a lane — it builds on push-to-main and release only,
@@ -98,6 +99,7 @@ def classify(files: list[str]) -> dict[str, bool]:
         "site": any(f.startswith(_SITE) for f in files),
         "scan": any(_is_scan(f) for f in files),
         "deps": any(f == "pyproject.toml" for f in files),
+        "npm_lock": any(f.split("/")[-1] == "package-lock.json" for f in files),
         "mcp_catalog": any(_is_mcp_catalog(f) for f in files),
         "ci_review": any(_is_ci_review(f) for f in files),
     }
@@ -108,6 +110,7 @@ def classify(files: list[str]) -> dict[str, bool]:
         ret["site"] = True
         ret["scan"] = True
         ret["deps"] = True
+        ret["npm_lock"] = True
         ret["ci_review"] = True
 
         # explicitly skip mcp catalog here. it's not needed unless those files are modified.

scripts/ci/lockfile_diff.py

@@ -0,0 +1,180 @@
+#!/usr/bin/env python3
+"""Semantic diff of npm ``package-lock.json`` files for PR comments.
+
+``git diff`` on a lockfile is unreadable: npm reorders entries, rewrites
+integrity hashes, and moves packages between nesting levels, so a one-line
+``package.json`` bump can produce a thousand-line textual diff. This script
+ignores the text entirely — it parses the ``packages`` map out of both
+versions of each lockfile (lockfileVersion 2/3), reduces each to
+``{install path: version}``, and set-diffs the two dicts. Reordering and
+hash churn vanish; what's left is the actual dependency change.
+
+Usage (from a checkout that still has the base ref available):
+
+    python scripts/ci/lockfile_diff.py --base <ref> --head <ref> \
+        --output diff.md [--repo-root .]
+
+Reads every ``package-lock.json`` tracked at either ref (top-level and
+nested — the repo has several), diffs each, and writes a Markdown report
+to ``--output``. Exits 0 always; an empty report file means "no version
+changes" (the caller uses that to decide whether to post/update the PR
+comment). The report embeds ``COMMENT_MARKER`` so the workflow can find
+and update its own previous comment instead of stacking new ones.
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import subprocess
+import sys
+
+# Hidden marker used to locate the bot's previous comment for in-place update.
+COMMENT_MARKER = "<!-- hermes-lockfile-diff -->"
+
+
+def parse_lockfile(text: str) -> dict[str, str]:
+    """Reduce lockfile JSON to ``{install path: version}``.
+
+    Keys are the ``packages`` map's keys (e.g. ``node_modules/react`` or
+    ``node_modules/foo/node_modules/react``), so the same package deduped
+    at two versions shows up as two distinct entries. The root entry
+    (``""``, the workspace itself) is skipped, as are versionless link
+    entries.
+    """
+    data = json.loads(text)
+    out: dict[str, str] = {}
+    for path, meta in data.get("packages", {}).items():
+        if not path:
+            continue  # root project entry, not a dependency
+        version = meta.get("version")
+        if version:
+            out[path] = version
+    return out
+
+
+def diff_locks(base: dict[str, str], head: dict[str, str]) -> dict[str, list]:
+    """Set-diff two ``{path: version}`` maps.
+
+    Returns ``added`` / ``removed`` as ``[(path, version)]`` and
+    ``updated`` as ``[(path, base_version, head_version)]``, each sorted
+    by path.
+    """
+    added = sorted((p, v) for p, v in head.items() if p not in base)
+    removed = sorted((p, v) for p, v in base.items() if p not in head)
+    updated = sorted(
+        (p, base[p], head[p]) for p in base.keys() & head.keys() if base[p] != head[p]
+    )
+    return {"added": added, "removed": removed, "updated": updated}
+
+
+def _display_name(path: str) -> str:
+    """``node_modules/foo/node_modules/@scope/bar`` → ``@scope/bar (nested under foo)``."""
+    parts = path.split("node_modules/")
+    name = parts[-1].rstrip("/")
+    if len(parts) > 2:
+        parents = " → ".join(p.rstrip("/") for p in parts[1:-1])
+        return f"{name} *(nested under {parents})*"
+    return name
+
+
+def render_markdown(diffs: dict[str, dict[str, list]]) -> str:
+    """Render per-lockfile diffs as a Markdown PR comment body.
+
+    ``diffs`` maps lockfile repo-path → the output of :func:`diff_locks`.
+    Lockfiles with no version changes are omitted. Returns ``""`` when
+    nothing changed anywhere (caller skips commenting entirely).
+    """
+    sections = []
+    total = 0
+    for lockfile, d in sorted(diffs.items()):
+        added, removed, updated = d["added"], d["removed"], d["updated"]
+        n = len(added) + len(removed) + len(updated)
+        if n == 0:
+            continue
+        total += n
+        lines = [f"### `{lockfile}`", ""]
+        lines.append("| Package | Before | After |")
+        lines.append("| --- | --- | --- |")
+        for path, old, new in updated:
+            lines.append(f"| {_display_name(path)} | `{old}` | `{new}` |")
+        for path, version in added:
+            lines.append(f"| ➕ {_display_name(path)} | — | `{version}` |")
+        for path, version in removed:
+            lines.append(f"| ➖ {_display_name(path)} | `{version}` | — |")
+        sections.append("\n".join(lines))
+
+    if not sections:
+        return ""
+
+    header = (
+        f"{COMMENT_MARKER}\n"
+        f"## ⚠️ `package-lock.json` changes ({total} package"
+        f"{'s' if total != 1 else ''})\n\n"
+        "This PR changes locked npm dependency versions."
+    )
+    return header + "\n" + "\n\n".join(sections) + "\n"
+
+
+def _git_show(ref: str, path: str, repo_root: str) -> str | None:
+    """Contents of ``path`` at ``ref``, or None if it doesn't exist there."""
+    proc = subprocess.run(
+        ["git", "show", f"{ref}:{path}"],
+        capture_output=True,
+        text=True,
+        cwd=repo_root,
+    )
+    return proc.stdout if proc.returncode == 0 else None
+
+
+def _tracked_lockfiles(ref: str, repo_root: str) -> set[str]:
+    proc = subprocess.run(
+        ["git", "ls-tree", "-r", "--name-only", ref],
+        capture_output=True,
+        text=True,
+        cwd=repo_root,
+        check=True,
+    )
+    return {
+        line
+        for line in proc.stdout.splitlines()
+        if line.split("/")[-1] == "package-lock.json"
+    }
+
+
+def diff_refs(base: str, head: str, repo_root: str = ".") -> dict[str, dict[str, list]]:
+    """Diff every package-lock.json tracked at either ref."""
+    lockfiles = _tracked_lockfiles(base, repo_root) | _tracked_lockfiles(head, repo_root)
+    diffs = {}
+    for path in sorted(lockfiles):
+        base_text = _git_show(base, path, repo_root)
+        head_text = _git_show(head, path, repo_root)
+        base_map = parse_lockfile(base_text) if base_text else {}
+        head_map = parse_lockfile(head_text) if head_text else {}
+        diffs[path] = diff_locks(base_map, head_map)
+    return diffs
+
+
+def main() -> int:
+    ap = argparse.ArgumentParser(description=__doc__)
+    ap.add_argument("--base", required=True, help="base git ref (merge base)")
+    ap.add_argument("--head", required=True, help="head git ref")
+    ap.add_argument("--output", required=True, help="markdown output path")
+    ap.add_argument("--repo-root", default=".", help="repository root")
+    args = ap.parse_args()
+
+    diffs = diff_refs(args.base, args.head, args.repo_root)
+    markdown = render_markdown(diffs)
+    with open(args.output, "w", encoding="utf-8") as fh:
+        fh.write(markdown)
+
+    if markdown:
+        changed = sum(len(v) for d in diffs.values() for v in d.values())
+        print(f"{changed} package version change(s) — report written to {args.output}")
+    else:
+        print("No package version changes.")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/ci/test_classify_changes.py

@@ -27,12 +27,13 @@ DEFAULT = {
     "site": True,
     "scan": True,
     "deps": True,
+    "npm_lock": True,
     "mcp_catalog": False,
     "ci_review": True,
 }
 
 
-def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, mcp_catalog=False, docker_meta=False, ci_review=False) -> dict[str, bool]:
+def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, npm_lock=False, mcp_catalog=False, docker_meta=False, ci_review=False) -> dict[str, bool]:
     return {
         "python": python,
         "frontend": frontend,
@@ -40,6 +41,7 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, mcp
         "site": site,
         "scan": scan,
         "deps": deps,
+        "npm_lock": npm_lock,
         "mcp_catalog": mcp_catalog,
         "ci_review": ci_review,
     }
@@ -53,7 +55,8 @@ CASES = {
     "ts package → frontend": (["apps/desktop/src/app.tsx"], _lanes(frontend=True)),
     "ui-tui → frontend": (["ui-tui/src/entry.ts"], _lanes(frontend=True)),
     # Lockfile bump shifts every TS package's tree, but not the Python suite.
-    "root lockfile → frontend, not python": (["package-lock.json"], _lanes(frontend=True)),
+    "root lockfile → frontend, not python": (["package-lock.json"], _lanes(frontend=True, npm_lock=True)),
+    "nested lockfile → npm_lock": (["website/package-lock.json"], _lanes(site=True, npm_lock=True)),
     "website → site": (["website/docs/intro.md"], _lanes(site=True)),
     # SKILL.md reads like docs, but the skill-doc tests read skills/, so a
     # skill edit must still run Python.

tests/ci/test_lockfile_diff.py

@@ -0,0 +1,111 @@
+"""Tests for scripts/ci/lockfile_diff.py.
+
+The differ's job is semantic comparison: reordering and integrity-hash
+churn in the lockfile text must produce an empty diff, while actual
+version movement must show up as added/removed/updated regardless of
+where in the file it appears.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import json
+from pathlib import Path
+
+_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "lockfile_diff.py"
+_spec = importlib.util.spec_from_file_location("lockfile_diff", _PATH)
+if _spec is None or _spec.loader is None:
+    raise ImportError("Failed to load lockfile_diff.py")
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
+
+def _lock(packages: dict[str, dict]) -> str:
+    return json.dumps(
+        {
+            "name": "hermes",
+            "lockfileVersion": 3,
+            "packages": {"": {"name": "hermes"}, **packages},
+        }
+    )
+
+
+BASE = _lock(
+    {
+        "node_modules/react": {"version": "18.2.0", "integrity": "sha512-aaa"},
+        "node_modules/left-pad": {"version": "1.3.0", "integrity": "sha512-bbb"},
+        "node_modules/foo/node_modules/react": {"version": "17.0.2"},
+    }
+)
+
+
+def test_parse_skips_root_and_versionless():
+    text = _lock({"node_modules/linked": {"link": True}})
+    parsed = _mod.parse_lockfile(text)
+    assert parsed == {}  # root entry and versionless link both skipped
+
+
+def test_reorder_and_hash_churn_is_empty_diff():
+    # Same packages, reordered, different integrity hashes — the exact noise
+    # that makes textual lockfile diffs unreadable.
+    reordered = _lock(
+        {
+            "node_modules/foo/node_modules/react": {"version": "17.0.2"},
+            "node_modules/left-pad": {"version": "1.3.0", "integrity": "sha512-XYZ"},
+            "node_modules/react": {"version": "18.2.0", "integrity": "sha512-ZZZ"},
+        }
+    )
+    d = _mod.diff_locks(_mod.parse_lockfile(BASE), _mod.parse_lockfile(reordered))
+    assert d == {"added": [], "removed": [], "updated": []}
+    assert _mod.render_markdown({"package-lock.json": d}) == ""
+
+
+def test_add_remove_update_all_detected():
+    head = _lock(
+        {
+            "node_modules/react": {"version": "18.3.1"},  # updated
+            "node_modules/is-even": {"version": "1.0.0"},  # added
+            "node_modules/foo/node_modules/react": {"version": "17.0.2"},  # unchanged
+            # left-pad removed
+        }
+    )
+    d = _mod.diff_locks(_mod.parse_lockfile(BASE), _mod.parse_lockfile(head))
+    assert d["added"] == [("node_modules/is-even", "1.0.0")]
+    assert d["removed"] == [("node_modules/left-pad", "1.3.0")]
+    assert d["updated"] == [("node_modules/react", "18.2.0", "18.3.1")]
+
+
+def test_nested_dedup_is_distinct_entry():
+    # The same package at two nesting levels must be tracked separately —
+    # bumping only the nested copy must not look like a top-level change.
+    head = _lock(
+        {
+            "node_modules/react": {"version": "18.2.0"},
+            "node_modules/left-pad": {"version": "1.3.0"},
+            "node_modules/foo/node_modules/react": {"version": "17.0.3"},
+        }
+    )
+    d = _mod.diff_locks(_mod.parse_lockfile(BASE), _mod.parse_lockfile(head))
+    assert d["updated"] == [("node_modules/foo/node_modules/react", "17.0.2", "17.0.3")]
+
+
+def test_render_markdown_contains_marker_and_versions():
+    d = _mod.diff_locks(
+        _mod.parse_lockfile(BASE),
+        _mod.parse_lockfile(_lock({"node_modules/react": {"version": "19.0.0"}})),
+    )
+    md = _mod.render_markdown({"apps/desktop/package-lock.json": d})
+    assert md.startswith(_mod.COMMENT_MARKER)  # workflow finds its comment by prefix
+    assert "⚠️" in md
+    assert "`apps/desktop/package-lock.json`" in md
+    assert "`18.2.0`" in md and "`19.0.0`" in md
+    # nested display name keeps the parent chain visible
+    assert "nested under foo" in md
+
+
+def test_render_markdown_omits_unchanged_lockfiles():
+    changed = _mod.diff_locks({}, {"node_modules/x": "1.0.0"})
+    unchanged = _mod.diff_locks({}, {})
+    md = _mod.render_markdown({"a/package-lock.json": changed, "b/package-lock.json": unchanged})
+    assert "a/package-lock.json" in md
+    assert "b/package-lock.json" not in md