Repository navigation
Conversation
A TypeScript/Node CLI (`ghaar`) and composite GitHub Action that scans GitHub Actions workflow run annotations across the latest run per workflow, files dedup-aware GitHub Issues, and auto-closes them when the underlying annotation vanishes. Core domain - Stable sha256 fingerprint over (workflowPath, annotationPath, normalized message); line numbers excluded so refactors don't spawn duplicates. - Severity-aware (notice / warning / error) with `--min-severity` filter and matching `severity/<level>` labels. - History-aware suppression: closed prior issues are honored when any of three signals match — configured label (default `wontfix`), `state_reason: not_planned`, or a regex against the closing comment. - Auto-close with safety guards: requires both a configurable miss counter AND age window, and (by default) the source workflow's latest run to have succeeded. Reopens automatically if the annotation returns. CLI / Action surfaces - `ghaar scan` / `report` / `list` commands with shared options. - `--list-annotations` prints fielded blocks per annotation and adds a full `annotations[]` array to the JSON report. - Phase-based spinner progress on stderr (auto-disabled in non-TTY / `--json` modes; force off with `--no-progress`). - Composite `action.yml` with a hardened bash dispatcher (version allowlist, env-var staging, args array, `mktemp` report path). Scaffolding - Strict TypeScript, ESLint + Prettier, Vitest with ≥90/85/90/90 coverage thresholds; 137 tests passing. - VitePress docs site (guide / recipes / reference) with auto- generated CLI reference via \`pnpm docs:gen-cli\`. - Changeset-driven release pipeline targeting npm trusted publishing, GitHub App auth, and a floating \`v<major>\` tag. - CI workflow, action self-test, weekly self-check, GitHub Pages deploy workflow. - CONTRIBUTING.md, SECURITY.md, full CLAUDE.md project memory.
GitHub Actions runners always inject GITHUB_REPOSITORY, which let resolveRepoFromEnv() succeed and short-circuit the throw the test was asserting. The test passed locally (no env var) but failed in CI. Stub the var to empty for just that test via vi.stubEnv and restore with vi.unstubAllEnvs in a try/finally so the test exercises the "no repo" branch on every host.
… fields Two previously-hidden counters now land in the JSON report's `summary` block, the bash dispatcher's `$GITHUB_OUTPUT`, the @actions/core setOutput path, and the human-readable summary line. `skipped` is new — it tallies create / update / reopen actions that were dropped because `max-issues` was already exhausted on this run. Before this change the cap fired silently: the actions vanished from the loop without contributing to any counter, and a misleading code comment claimed they were "converted to held" when nothing of the sort happened. Users could not tell from the summary whether the cap had bitten. Auto-close is never throttled and so can never contribute to this counter. `auto-close-held` already existed in `ReportSummary` and the JSON report but wasn't surfaced as an `action.yml` output. The CLI's @actions/core path already emitted it, but per CLAUDE.md's pitfall about the CLI erroring after the pipeline runs but before emit, the bash dispatcher should always be able to surface it from the JSON. Add the jq extractor and the matching `outputs:` entry. Documentation: rows added to the outputs table in `docs/guide/use-as-action.md`; `skipped` documented in the JSON schema and sample report in `docs/reference/json-output.md`. Tests: extend the "caps writes at config.maxIssues" case in pipeline-edge.test.ts to assert `summary.skipped === 3` instead of silently passing; add a setOutput assertion for `skipped` and `auto-close-held` in action-outputs.test.ts.
`fs.writeFile(opts.jsonOut, …)` previously failed with a raw `ENOENT`
when an intermediate component of the target path didn't exist —
unhelpful for users who pass paths like `out/reports/today.json`
into a working tree that doesn't have `out/` yet. Wrap the write in
`fs.mkdir(path.dirname(…), { recursive: true })` (no-op when the
directory already exists) so the file just lands.
Test: `tests/unit/io/output.test.ts` gains an "emitResults —
--json-out file handling" describe block that writes a report to a
two-level-deep nested path under `os.tmpdir()` and asserts both
that the file lands and that the human-readable summary still
prints to the injected fake stdout.
…ting `renderAnnotationsList` previously passed `annotation.message` and `annotation.rawDetails` straight through to the terminal. A multi-megabyte stack trace or a binary blob smuggled into `raw_details` would spill unboundedly and could lock up the user's terminal. Cap each field at `MAX_RENDERED_FIELD_BYTES = 4 KB` and append a dim "… (truncated, N more bytes — use --json for the full payload)" marker when truncation fires. JSON output is unaffected — consumers asking for the full payload via `--json` / `outputs.json` see the complete content. The cap is per-field, not per-block, so a long message and a long rawDetails each get their own truncation. Tests: three cases in `tests/unit/io/annotations-list.test.ts` — oversized message truncated with the marker; oversized rawDetails truncated likewise; under-cap fields rendered without any marker.
The `wontfix.commentPattern` config option compiles a user-supplied regex and applies it to the closing comment of arbitrary closed issues. Previously there was no length cap on either side, so a pathological pattern (e.g. `(a+)+b`) combined with a crafted attacker-controlled issue body (`aaaa…aaaa` with no `b`) could trigger catastrophic backtracking and lock up the runner or the local dev machine. Add two caps in `src/core/wontfix-detector.ts`: - `MAX_PATTERN_LENGTH = 1000` — oversized patterns are rejected by `safeCompile`, treated identically to an invalid regex (fail-safe: no match, no suppression). - `MAX_COMMENT_LENGTH = 64 * 1024` — the fetched comment is `slice`d to the cap before `regex.test()`, bounding worst-case evaluation cost. A real "wontfix" marker would land near the top of any reasonable comment anyway, so truncation almost never loses a legitimate signal. Constants exported so tests can reference them at the boundary. Tests (`tests/unit/core/wontfix-detector.test.ts`): - oversized pattern → no suppression (fail-safe) - needle past the cap → not matched (truncation worked) - needle within the cap → still matched (truncation didn't blow it away) CLAUDE.md "Security — high priority" section updated to document the caps as load-bearing.
The recent GITHUB_REPOSITORY-in-CI bug ("throws when the repository
cannot be determined" passing locally / failing on the runner) was
one instance of a broader gotcha: GitHub Actions runners auto-set
a pile of env vars and stream properties that affect code under
test (GITHUB_TOKEN, GITHUB_ACTIONS, RUNNER_TEMP, isTTY, …). Tests
that read these directly or indirectly drift behavior between
local and CI.
Add a "Common pitfalls" bullet listing the typical offenders and
pointing at the `vi.stubEnv` + try/finally pattern already used in
`tests/unit/core/pipeline.test.ts`. Also note that the CLI/command
glue is intentionally excluded from coverage so it never has to be
unit-tested under those conditions.
There was a problem hiding this comment.
Pull request overview
Initial implementation of github-actions-annotations-reporter (CLI ghaar + composite GitHub Action). The PR introduces the full project scaffolding — TypeScript source, ~137 tests with 90/85/90/90 coverage gates, CI/release/docs workflows, a composite action.yml, and a VitePress documentation site. The CLI scans the latest run per workflow, computes line-number-independent fingerprints over annotations, opens/updates GitHub Issues with state persisted in HTML-comment markers in the body, and auto-closes issues whose annotations vanish (gated by miss-counter, age window, and source-run success).
Changes:
- Adds the core CLI/library (
src/cli.ts,src/commands/*,src/core/*,src/io/*) with fingerprinting, reconciler, wontfix detection (with ReDoS guards), and auto-close policy. - Adds comprehensive unit and integration tests, helpers, fixtures, and a Vitest configuration with strict coverage thresholds.
- Adds packaging/release plumbing:
action.yml(composite action with input validation), CI/release/deploy-docs workflows, VitePress docs, and Changesets configuration.
Reviewed changes
Copilot reviewed 93 out of 96 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
tsconfig.json, tsconfig.build.json, vitest.config.ts, eslint.config.js, .prettierrc.json, .editorconfig, .npmrc |
Strict TS / lint / format / test config baseline. |
package.json, pnpm-workspace.yaml, .changeset/* |
Package metadata, workspace, release management. |
src/cli.ts, src/index.ts |
CLI entrypoint and public exports. |
src/commands/{shared,scan,report,list}.ts |
Subcommand glue around the pipeline. |
src/core/{config,auth,fingerprint,reconciler,wontfix-detector,auto-close-policy,annotation-collector,minimatch-lite,types}.ts |
Core domain logic. |
src/core/github/{client,repo,issues,workflows,annotations}.ts |
Octokit-based GitHub API access. |
src/io/{progress,issue-body,action-outputs}.ts, src/io/output/{table,json,formatter,annotations-list}.ts |
Rendering, JSON report, progress spinner, Action outputs. |
src/utils/{paths,markdown}.ts |
POSIX-path and markdown helpers. |
tests/helpers/{fake-octokit,fixtures}.ts, tests/unit/**/*.test.ts |
Test infrastructure and unit coverage. |
.github/workflows/{ci,release,deploy-docs,self-check,action-selftest}.yml, action.yml |
CI matrix, release pipeline, docs deploy, action self-test, composite action with input validation. |
README.md, SECURITY.md, CONTRIBUTING.md, LICENSE, docs/** |
User and contributor documentation, VitePress site, generated CLI reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The 4 KiB per-field cap is measured with `text.length`, which counts UTF-16 code units. Renaming `MAX_RENDERED_FIELD_BYTES → CHARS` and the truncation marker's "more bytes → more characters" makes the units match the measurement. JSON output is still uncapped. Also fix the ANSI-strip regex in the test helper: the previous pattern matched `\[[0-9;]*m` without the leading ESC byte (the file had a literal 0x1B that survived only because `toContain` doesn't compare exactly). Switch to `�\[[0-9;]*m` so the helper survives an exact-match assertion later.
`release.yml`, `self-check.yml`, `deploy-docs.yml`, and `action-selftest.yml` already pin `actions/checkout`, `actions/setup-node`, and `pnpm/action-setup` at `@v6`; only `ci.yml` was on `@v4`. Aligning the majors cuts Dependabot noise and removes a silent inconsistency.
The comment justified clearing GH_TOKEN as well, but only GITHUB_REPOSITORY was stubbed. Stub all three so the "no repo" branch is exercised regardless of the host shell's auth env.
`state.issues.at(-1)?.number + 1` assumed appended-in-order numbering.
Tests that seed issues with hand-picked, out-of-order numbers (e.g.
`{number: 6}` then `{number: 5}`) could silently collide on `create`.
Switch to `Math.max(0, ...state.issues.map(i => i.number)) + 1`.
`IssueState` is re-exported by `src/index.ts` and nothing imports it from this module. Drop the dangling re-export and stale comment.
`String(workflowLatestRun.conclusion)` rendered a null conclusion as the literal text `"null"` inside quotes, which was misleading. Match the listing renderer's convention and substitute `(no conclusion)` instead.
`Number.parseInt('abc', 10)` returns `NaN`, which slipped through the
`typeof n === 'number'` check in `shared.ts` and only surfaced much
later as a generic zod "Expected number, received nan" — far from the
flag that produced it. Throwing `InvalidArgumentError` in the coercer
gives users a precise, locally-attributed error against the offending
flag.
Applies to `--max-issues`, `--auto-close-after-days`, and
`--auto-close-after-misses`.
The old non-greedy `{[^]*?}` capture would stop at the first inner `}`
in the state JSON. If the persisted state ever gains a nested-object
field, JSON.parse trips on the truncated payload, parseIssueState
returns null, and the auto-close policy treats the issue as infinitely
old — eligible for immediate close.
Anchor the marker to a single line and capture everything between the
label and the trailing `-->`. This decouples the parser from any future
shape of the state JSON.
`env['GITHUB_TOKEN'] ?? env['GH_TOKEN']` only fell through on
null/undefined; an empty-string `GITHUB_TOKEN` (a common CI pattern
when a secret resolves but is empty, or `env: { GITHUB_TOKEN: '' }`)
would shadow a valid `GH_TOKEN` and drop the resolver all the way to
`gh auth token` / anonymous.
Trim each candidate independently and pick the first non-empty value
with `||`. Adds a regression test for the empty/whitespace case.
…tion `isGitHubActionsEnv` accepts an optional `env`, but `emitActionOutputs` and `failAction` always called it with no argument — making the parameter dead at the call site and forcing tests to mutate `process.env` directly to exercise the gate. Add an optional `env` field to `EmitActionOutputsArgs` and a matching parameter on `failAction` so the two functions can't drift apart. Behavior is unchanged when callers omit `env` (defaults to `process.env`).
The `MAX_COMMENT_LENGTH` slice counts UTF-16 code units, so a cut that lands inside a surrogate pair (an emoji or any supplementary-plane character at the boundary) would leave an unpaired high surrogate in the bounded string. `RegExp.test` tolerates that, but any downstream JSON serialization would emit a broken string. Back the cut off by one when the last code unit is a high surrogate. Test asserts the bounded prefix stays serializable.
… workflow whitespace contract `--wontfix-labels` parsing used `tr -d '[:space:]'`, which stripped *all* whitespace from each label — silently mangling multi-word labels (GitHub allows them) into one concatenated token. Switch to a `sed`-based leading/trailing-only trim so internal spaces survive. For `workflows` / `reject`, document the whitespace caveat in the input descriptions (action.yml, README/docs): the shell-side `read -r -a` splits on IFS, so individual globs may not contain whitespace. Names with spaces are still matchable via a glob like "My*CI".
The old check built the comparison URL with a template literal:
`new URL(\`file://${process.argv[1]}\`).toString()`. On Windows
`process.argv[1]` is a path like `C:\Users\me\…\cli.js`, which yields
a malformed file URL (no slash before the drive letter, backslashes
instead of forward slashes) — the URL constructor would either throw
(caught, so the CLI silently refuses to run as a script) or normalize
unpredictably.
`pathToFileURL` from `node:url` handles drive letters and separators
correctly on every platform.
Per Copilot review on src/core/annotation-collector.ts:121: the bottom-of-file 'export { type WorkflowInfo } from "./types.js"' was a scaffolding artifact — src/index.ts is the single source of public type re-exports for the package, and nothing imports WorkflowInfo from annotation-collector.js. The equivalent stray re-export on src/core/github/issues.ts was removed in an earlier round; this instance was missed.
Per Copilot review on src/index.ts:30: SerializedAction was re-exported but SerializedAnnotation (also a public, documented part of the JSON report schema — see docs/reference/json-output.md) was not. Downstream consumers writing TypeScript against the annotations[] field had to reach into the deep io/output/json.js path. Added SerializedAnnotation to the barrel for symmetry.
…e and --fail-on-new help Per Copilot review on src/commands/report.ts:6: the bare 'return 2' was a magic number. The exit-code contract (0 / 1 / 2) was documented in CLAUDE.md but nowhere user-facing. Extracted src/commands/exit-codes.ts with EXIT_OK / EXIT_ERROR / EXIT_NEW_ISSUES_FOUND, used the named constants in report.ts, extended the --fail-on-new flag help to mention 'code 2 ... distinct from code 1 for runtime errors', and added an Exit codes section to docs/reference/cli.md above the AUTOGEN block. Shell pipelines branching on the code now have a documented contract instead of source-reading homework.
Per Copilot review on src/commands/shared.ts:122: path.dirname(path.resolve(opts.jsonOut)) was used for mkdir's parent, then fs.writeFile was passed the unresolved (potentially relative) form. Benign today because Node resolves against the same process.cwd() in both calls, but the asymmetry is easy to misread and would have broken subtly if a future caller changed cwd between the two operations. Compute 'const resolved = path.resolve(opts.jsonOut)' once and use it for both mkdir and writeFile. jsonPath trivially equals resolved.
Per Copilot review on docs/.vitepress/config.ts:17: target: 'esnext' disabled down-leveling entirely (every later ES feature gets emitted as-is), which is broader than the original VitePress 1.6 / esbuild 0.27 transpile issue needs. 'es2022' clears the same problematic transform while still down-leveling more recent ES proposals for browsers that don't have them yet. Build is green; the comment now documents the bump-by-one-minor escalation path if a future combination needs more.
…vite 5.4 peer Per Copilot review on package.json:88. The pnpm overrides block was pinning vite to '>=6.4.2' (originally a security-driven floor inherited from the boilerplate), but vitepress 1.6.4 internally pins vite to ^5.4.14 and has no vite peer entry — so forcing the higher floor was a deliberate cross-major upgrade for a peer-bound package. With the override in place pnpm was actually resolving vite 8.0.13 for VitePress, well outside its tested compat window. Removed both the override and the explicit 'vite' devDep. VitePress now resolves vite 5.4.21 (its own tested pin). docs:build remains green; install size shrinks. vitest still pulls vite 8 internally for its own machinery — those live in separate parts of the dep tree, so there's no conflict. VitePress 2.x exists but only as alpha builds, not appropriate for stable use; this path stays inside VitePress 1.x's documented compatibility window.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 98 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tests/helpers/fake-octokit.ts:110
listCommentsdoes not honorsinceor pagination params, and returns all comments on the issue at once.getClosingCommentinsrc/core/github/issues.tspaginates viaoctokit.paginate.iterator(octokit.issues.listComments, ...), so combined with the single-pagepaginate.iteratormock, the test only covers issues whose entire comment history fits in the first page. An issue with >100 comments — common for long-running suppression discussions, exactly the casegetClosingCommenttargets — would silently miss the closing comment in production but pass in tests.
…-page path Per Copilot review on tests/helpers/fake-octokit.ts:79. The previous mock called the underlying method once and yielded all its data as a single page, so production code paths that iterate via 'octokit.paginate.iterator' (listManagedIssues, listJobsForRun, listAnnotationsForCheckRun) were never actually exercising the multi-page traversal loop. Slice the returned data array by params.per_page (default 100, matching @octokit/rest's default) and yield each slice as a separate page. Tests that supply > per_page items now drive the iterator's loop properly. Added a 'iterates across multiple pages' regression in issues.test.ts: seeds 250 issues, asserts all 250 surface and that records at page boundaries (1, 100, 101, 250) are all present.
… mid-run Per Copilot review on .github/workflows/release.yml:44. 'npm install -g npm@latest' pulled whatever the most recent npm release was at publish time. A future npm 12 could change the publish flow, OIDC token format, or pnpm interop — and that breakage would only surface mid-release, after the version-bump PR had already merged. Pin to '^11' (current major; npm 11.5+ is what we need for trusted-publishing OIDC). The comment documents that the floor is intentional and should be bumped explicitly after validating the next major locally.
…runcation Per Copilot review on src/io/output/annotations-list.ts:82. The truncate() helper used 'text.slice(0, max)' to bound rendered fields — same UTF-16 surrogate-pair bug class as the wontfix detector fix from an earlier round, with the milder impact of a possibly-orphaned high surrogate in the terminal (most emulators replace with U+FFFD, but the inconsistency was real and the surfaces were drifting apart). Extracted clampToCodePointBoundary to src/utils/clamp-code-point.ts (dedicated test file covers the empty-input, in-bounds, mid-pair, just-past-pair cases). Both wontfix-detector::detectWontfix and annotations-list::truncate now go through the shared helper. Added a regression test in annotations-list.test.ts that builds a message whose MAX-1 boundary lands on the high surrogate of an emoji pair and walks the rendered output to assert no orphan high surrogate survives.
…ot notice
Per Copilot review on src/core/github/annotations.ts:64. The previous fail-open default ('notice') silently downgraded any unknown level — a future GitHub API value (e.g. a hypothetical 'critical' or 'fatal') would be coerced to the lowest severity and could slip past a '--min-severity warning' filter.
Default to 'warning' instead: surfaces the unknown level rather than suppressing it, without overclaiming it's an error. The existing test that asserted 'unknown' → 'notice' is updated to reflect the new behavior, with a comment pointing to the normalizeLevel docstring that explains the choice.
… null Per Copilot review on src/core/github/issues.ts:162. The 'any commenter before close' fallback path (when 'issue.closed_by' was null — deleted account, App-driven closure without an actor) accepted ANY commenter's body as the closing voice. An attacker with comment-write access could plant text matching a configured 'wontfix-comment-pattern' regex on such an issue and silently suppress future filings of the same annotation fingerprint. Fail closed: getClosingComment now returns null when closer === null, and the inner loop only accepts comments authored by the known closer. The wontfix detector then falls back to label / state_reason signals only on those issues. The existing test that documented the 'falls back to any comment' behavior is updated to assert the new fail-closed contract. SECURITY.md gains a corresponding in-scope threat-model entry pointing at the mitigation.
Previously `escapeHtmlCommentBody` only handled the canonical `-->` sequence by replacing it with `-- >`. Two adjacent attack vectors remained: 1. `--!>` — HTML5's "bogus comment" alternative close. Some renderers accept it as a valid terminator, which would let an annotation body containing `--!>` close one of our `<!-- annot-... -->` state markers and corrupt the issue body. 2. `--->` and longer dash runs — parsers that "fix up" extra dashes could collapse the sequence back to `-->`. The new implementation inserts a zero-width space (U+200B) between every adjacent pair of `-` characters. This is a single-pass rule that neutralizes all three sequences at once, leaves single dashes alone, and preserves visual fidelity (ZWSP is non-printing). Tests updated to assert the new escaped output for `-->`, `--!>`, 3+ dash runs, multiple terminators in one input, and benign text. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…oken
`exactOptionalPropertyTypes` is enabled in tsconfig — which means a
field typed `explicitToken?: string` does NOT accept `undefined` as a
value, only an omitted key. Passing `prepared.token ?? ''` worked by
accident (resolveAuth trims and falls through to env on empty), but
the call site's intent was muddled: was the empty string supposed to
mean "no token" or "explicitly empty"?
Switch to the conditional-spread pattern already used elsewhere
(e.g. `...(prepared.repo ? { explicitRepo: prepared.repo } : {})`)
so the call site clearly signals "no explicit token here". Three
spots updated:
- `src/commands/list.ts::runListCommand` — direct `resolveAuth` call.
- `src/commands/shared.ts::executePipeline` — `runPipeline` call.
- `src/core/pipeline.ts::runPipeline` — internal `resolveAuth` call.
Behavior unchanged: `resolveAuth` still resolves the token from env
(GITHUB_TOKEN / GH_TOKEN / gh CLI) when no explicit value is passed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`git push origin "$major" --force` silently clobbers whatever ref the remote currently has for `vN`. In a hotfix-on-older-major scenario — e.g. v1.5.0 ships, then v1.4.1 is released later as a patch — the later run's tag-move would overwrite v1's pointer with v1.4.1 even though v1.5.0 is the higher version. The push had no protection against this race. Use `--force-with-lease=refs/tags/vN:<expected-sha>` instead, where `<expected-sha>` is captured from `git ls-remote` immediately before the push. If a concurrent run has already moved the tag, the push is rejected and we fail loudly. When the tag doesn't exist yet (first release for a new major), we fall through to a plain push. Also document version-pinning options for consumers in `docs/guide/use-as-action.md`: - `@v1` — floating major, gets all v1.x.y updates (most common, what the doc currently recommends). - `@v1.2.3` — exact version, fully reproducible. - `@<40-char-sha>` — most immutable, gets Dependabot bumps. This makes the trade-off explicit: floating tag = automatic security patches; exact pin = reproducibility. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI's `pnpm format:check` step caught a Prettier violation in `src/utils/markdown.ts` — the single-line `replaceAll(...)` call had been wrapped across three lines by a manual edit, but Prettier wants it back on one line because it's under the print-width threshold. Round 10's local verify ran `pnpm format` before the final edits landed, so the formatter never saw the wrapped form. This commit just re-runs Prettier on the file; no behavior change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Initial implementation of
github-actions-annotations-reporter— aTypeScript/Node CLI (
ghaar) and composite GitHub Action that scansworkflow run annotations across the latest run per workflow, files
dedup-aware GitHub Issues, and auto-closes them when the annotation
vanishes.
Highlights
--min-severityandseverity/<level>labels.state_reason: not_planned, closing-comment regex).--list-annotations: fielded blocks per annotation + fullannotations[]in the JSON report.--json).Test plan
Notes for reviewers
dist/is intentionally gitignored; shipped via npm (filesfield inpackage.json).npm-publishenvironment + secrets (see CONTRIBUTING.md).mainis an empty initial commit so this PR shows the full implementation as its diff.🤖 Generated with Claude Code