Skip to content

ci(audit): show the remediation result on the run page; judge changes by audited versions - #160

Merged
daften merged 1 commit into
developfrom
claude/confident-bardeen-vzptx9
Oct 30, 2026
Merged

daften merged 1 commit into
developfrom
claude/confident-bardeen-vzptx9

Conversation

@daften

@daften daften commented Oct 30, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to #158/#159 after the first real run of audit-remediate.yml (a manual dry run: it worked, but its result was hard to find).

1. Every outcome is written to the job summary and a notice annotation

The dry run correctly said Nothing to remediate on main or develop., but only as plain log text. The Actions page in the current layout showed just "This job succeeded" (no step list; the only gear option is "Download log archive"), so the answer was found by downloading the log zip. Now each outcome also goes to $GITHUB_STEP_SUMMARY and, where it matters, a ::notice:::

  • nothing to fix: "No fixable moderate or higher advisory on main or develop, so nothing was changed and no PR was opened."
  • skipped because a fix PR (its own or Dependabot's) is already open
  • dry run: that nothing was pushed, the PR title it would use, and the table of updated packages
  • real run: the PR link plus the table of updated packages

2. "Did the update change anything" compares audited versions, not lockfile text

Found while testing: npm 10.9.4 (my sandbox) rewrites unrelated parts of package-lock.json (it drops the libc fields of optional dev entries), so a git diff --quiet check cannot tell churn from a fix. scripts/audit-remediate.mjs changed now compares the versions of the packages the audit flagged, and the step restores the lockfile and opens nothing when none changed. I checked what npm 11 (the runner's major version, 11.21.1) does on the current main lockfile: a no-op update leaves it byte-identical, so churn is not expected on the runner; this is the guard for when it happens.

A correction about an earlier PR

While testing I found that my hand-made hotfix #152 (run with that same npm 10.9.4) also removed all six "libc" lines from package-lock.json, unrelated to the advisories; I had only checked version fields in that diff, so my "only the vulnerable entries changed" statement was incomplete. They are dev-only optional native bindings, CI passed, and main has 4 libc blocks again from later changes, so I expect no effect, but it is unflagged churn that shipped in 1.0.0-alpha.1.

Verification

  • The workflow's actual run: blocks executed under bash -e with the real registry, real npm audit and real git (only gh faked): the nothing-to-fix, PR-already-open, dry-run and real-PR outcomes each write the expected summary and notice; the update step on already-current packages (with npm 10's libc churn present) reports no change, restores the lockfile and opens nothing; the real vulnerable scenario still opens exactly one PR.
  • New tests/automation/audit-remediate-workflow.node-test.mjs runs those steps offline in CI (git, npm, gh mocked; real script and real audit fixture): 10 tests. I broke the workflow three ways (dropped notice, dry run that still pushes, swallowed audit failure) and each was caught by the intended test; the file is restored byte for byte.
  • 101 automation tests pass (was 89), npm run check passes (598 tests), the changelog guard passes against the v1.0.0-alpha.1 baseline.

Not verified

  • Still no run on a real runner of the path that opens a PR; the summary rendering on the Actions page is checked only as Markdown text, not visually.
  • As before, workflows run from main: this takes effect after the companion hotfix PR to main.

Type of change

  • feat (new tool or feature)
  • fix (bug fix)
  • refactor (code cleanup, no behavior change)
  • test (add or update tests)
  • chore (dependencies, config, scaffolding)
  • docs (documentation only)

Checklist

  • Tests added or updated
  • npm test passes locally (598 passed, 29 skipped); node --test tests/automation/*.node-test.mjs 101 passed
  • npx tsc --noEmit passes locally
  • README tool table updated (no tools changed)
  • CLAUDE.md updated (AGENTS.md updated instead)
  • Verified against the Firefly III OpenAPI spec (not API-touching)

Merge as a merge commit, never squash or rebase.


Generated by Claude Code

…anges by audited versions

A run of audit-remediate.yml that found nothing to fix was invisible on the
run page: the result was plain log text, and the new Actions layout shows no
step list, only 'This job succeeded'. Every outcome (nothing to fix, skipped
because a fix PR is open, dry run, PR opened) now also goes to the job
summary and a notice annotation.

'Did npm update change anything' compared the lockfile text. A different npm
version rewrites unrelated fields (npm 10 drops the libc fields of optional
dev entries), so churn alone counted as a fix. It now compares the versions
of the audited packages (audit-remediate.mjs changed) and restores the
lockfile when none changed.

The steps' run blocks are executed offline in
audit-remediate-workflow.node-test.mjs (git, npm and gh mocked, real script
and real audit fixture); three deliberate breakages of the workflow were each
caught by the intended test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ESxuEjCeVsRAAxAb29Yjr
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 95.63% 1227 / 1283
🔵 Statements 95.28% 1393 / 1462
🔵 Functions 96.73% 355 / 367
🔵 Branches 93.07% 712 / 765
File CoverageNo changed files found.
Generated in workflow #377 for commit 4693ad2 by the Vitest Coverage Report Action

@daften
daften merged commit e070870 into develop Oct 30, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants