mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
b9f82ed39f
Replace the static comment-pending + comment-results two-job pattern
with a live-updating comment system that polls the GitHub Actions API
every 15s, re-assembles the review comment from whatever results are
available, and upserts it via the <!-- hermes-ci-review-bot --> marker.
The comment updates in real time as each job finishes — no waiting for
the full pipeline.
Every CI job that wants to appear in the review comment emits a
review_status output — a JSON array of objects, each with a source
and a results array:
[
{
"source": "review-label-gate",
"results": [
{"kind": "action_required", "title": "...", "summary": "...",
"how_to_fix": "..."},
{"kind": "info", "title": "...", "summary": "..."}
]
},
{
"source": "ci timing",
"results": [
{"kind": "warning", "title": "CI timings", "summary": "...",
"detail": "...", "link": "..."}
]
}
]
One job can emit multiple results of different kinds. The source field
is used to exclude the corresponding job from the synthesized error
list (case-insensitive, hyphen-normalized matching against GitHub
Actions job display names).
| job | source | kind (on failure) | section |
|----------------------------|--------------------------|---------------------------|----------------------|
| review-labels | review label gate | action_required / info | Action required |
| lockfile-diff | lockfile-diff | action_required | Action required |
| ci-timings | ci timing | warning / info | Warnings |
| supply-chain scan | supply chain | error / (none) | Job failures |
| supply-chain dep-bounds | supply chain | action_required / (none) | Action required |
| osv-scanner | osv scan | warning / (none) | Warnings |
| uv-lockfile-check | uv.lock check | action_required / (none) | Action required |
| history-check | unrelated histories | action_required | Action required |
| contributor-check | contributor attribution | action_required | Action required |
Jobs that find nothing emit [] (empty array) — no noise info items.
A single comment-live job polls the GitHub Actions API every 15s,
classifies jobs into (completed, pending), assembles the comment, and
upserts it. Merges review_status outputs from all needs jobs via
toJSON(needs.*.outputs.review_status), and downloads the ci-timings
artifact when it becomes available. Shows commit SHA + message below
the header.
The assembler has ZERO job-specific knowledge. It just:
1. collect_from_statuses() — flattens all nested status objects into ReviewItems
2. collect_failed_jobs() — synthesizes errors for failed jobs with no declared status
3. _attach_job_urls() — fills in per-job log links for ALL items
4. render_comment() — groups by severity, renders with group headers
Each item shows links inline next to the title: View report (job-emitted
URL) and View job (auto-attached logs link). Each info item is its own
collapsible <details> block.
# ૮ >ﻌ< ა ci review
running on abc1234 — commit message first line
## ❌ Job failures
### {title} · [View job](url)
{summary}
## ⚠️ Action required
### {title} · [View job](url)
{summary}
**How to fix:**
{how_to_fix}
## ⚠️ Warnings
### {title} · [View report](url) · [View job](url)
{summary}
{detail}
<details><summary>{title}</summary>
{content}
</details>
Still running 3 jobs: ci-timings, docker
- test_assemble_review_comment.py (48 tests): collect_from_statuses,
collect_failed_jobs with exclude_sources, _attach_job_urls,
render_comment (group headers, inline links, commit info, per-item
details, pending footer), assemble integration
- test_live_comment.py (16 tests): classify_jobs pure function
- test_timings_report.py (10 tests): generate_review_status nested format
- test_lockfile_diff.py (6 tests)
- test_classify_changes.py (32 tests, pre-existing)
110 lines
4.1 KiB
Python
110 lines
4.1 KiB
Python
"""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_versions_and_nested_display():
|
|
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})
|
|
# Fragment starts directly with the per-lockfile subsection header.
|
|
assert md.startswith("#### `apps/desktop/package-lock.json`")
|
|
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
|