Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/actions/detect-changes/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -146,6 +153,7 @@ jobs:
- history-check
- contributor-check
- uv-lockfile
- lockfile-diff
- docker-lint
- supply-chain
- osv-scanner
Expand Down
98 changes: 98 additions & 0 deletions .github/workflows/lockfile-diff.yml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions scripts/ci/classify_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* ``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,
Expand Down Expand Up @@ -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),
}
Expand All @@ -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.
Expand Down
180 changes: 180 additions & 0 deletions scripts/ci/lockfile_diff.py
Original file line number Diff line number Diff line change
@@ -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())
7 changes: 5 additions & 2 deletions tests/ci/test_classify_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,21 @@
"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,
"docker_meta": docker_meta,
"site": site,
"scan": scan,
"deps": deps,
"npm_lock": npm_lock,
"mcp_catalog": mcp_catalog,
"ci_review": ci_review,
}
Expand All @@ -53,7 +55,8 @@ def _lanes(python=False, frontend=False, site=False, scan=False, deps=False, mcp
"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.
Expand Down
Loading
Loading