feat(action): GitHub Marketplace Action — Slices 1-4 of #73 plan#74
Merged
SutuSebastian merged 10 commits intomainfrom May 5, 2026
Merged
feat(action): GitHub Marketplace Action — Slices 1-4 of #73 plan#74SutuSebastian merged 10 commits intomainfrom
SutuSebastian merged 10 commits intomainfrom
Conversation
First half of Slice 1 from the GitHub Marketplace Action plan. `audit` today only emits `--json`; this adds SARIF 2.1.0 output directly, no JSON→SARIF transform step needed in CI workflows. CLI: - New `--format <text|json|sarif>` flag (default `text`). - `--json` stays as backward-compat shortcut for `--format json`. - `--json` + `--format <other>` rejected as a contradiction with a helpful error message. - `--summary` is a no-op with `--format sarif` (SARIF results are per-row, not counts) and surfaces a stderr warning. - `runAuditCmd` signature updated: `json: boolean` → `format: AuditOutputFormat` (`"text" | "json" | "sarif"`). SARIF shape: - One rule per delta key (`codemap.audit.files-added`, `codemap.audit.dependencies-added`, `codemap.audit.deprecated-added`). - One result per `added` row (severity = `warning`; audit deltas are more actionable than per-recipe `note`). - Locations auto-detected via the existing `file_path`/`path`/`to_path`/`from_path` priority list (same as `query --format sarif`); `line_start`/`line_end` populate the SARIF `region`. - `removed` rows intentionally excluded — SARIF surfaces findings to act on, not cleanups. - Location-only rows (e.g. files-added has only `path`) get a "new files: src/foo.ts" message, not the generic "(no message)" fallback. Tests: - 4 new SARIF-formatter unit tests (rules/results shape; empty deltas; missing location column; line range). - 4 new audit-CLI parser tests (--format sarif/json/=json, unknown format value, --json + --format contradiction, --json + --format json reconcilable). - All existing audit / output-formatters tests updated for the `json: bool` → `format: AuditOutputFormat` field rename. Lockstep updates: - `.agents/rules/codemap.md` + `templates/agents/rules/codemap.md` audit-vs-git-ref row gains `--format sarif` mention. - New changeset (.changeset/audit-format-sarif.md, minor). Slice 1b (`--ci` aggregate flag on `query` + `audit`) lands in the follow-up PR.
🦋 Changeset detectedLatest commit: 80b575b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR introduces SARIF output formatting for ChangesAudit & Query SARIF & CI Output
PR Comment Engine
GitHub Marketplace Action & Package Manager Detection
Sequence DiagramsequenceDiagram
participant GH as GitHub
participant WF as Workflow Runner
participant DetectPM as detect-pm.mjs
participant CLI as codemap CLI
participant Fmt as Output Formatter
participant PCmt as PR Comment
participant API as GitHub API
GH->>WF: Trigger PR/Push
WF->>WF: Set up Node.js
WF->>DetectPM: Run with env vars
DetectPM->>DetectPM: Detect PM (pnpm/bun/npm)
DetectPM->>DetectPM: Resolve codemap cmd
DetectPM-->>WF: Output agent/exec/install_method
WF->>CLI: Run codemap audit --format sarif
CLI->>Fmt: audit deltas → formatAuditSarif
Fmt-->>CLI: SARIF JSON
CLI-->>WF: Exit code (0 or 1 per --ci)
opt If format=sarif
WF->>API: Upload SARIF artifact
API-->>WF: ✓
end
opt If pr-comment enabled
WF->>CLI: Run codemap pr-comment <output.json>
CLI->>PCmt: Detect shape (audit/sarif)
PCmt->>PCmt: Render markdown
PCmt-->>CLI: Markdown body + findings_count
CLI-->>WF: Markdown
WF->>API: gh pr comment --body <markdown>
API-->>GH: Comment posted
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…plan) Second half of Slice 1. `--ci` is the GitHub-Action-shaped CI flag: - Aliases `--format sarif` - Sets process.exitCode = 1 when ≥1 finding/addition surfaced - Suppresses the no-locatable-rows stderr warning (CI templates would surface it as red noise; the row-set itself is the gating signal) Lands on both `query` and `audit` (the two finding-producing verbs). Same parser / resolver semantics on both: - Mutually exclusive with `--json` (different format aliases) - Mutually exclusive with `--format <other>` (contradicts the alias) - `--ci --format sarif` redundant but accepted (consumers may set both for clarity in CI templates) Wiring: - `parseQueryRest` / `parseAuditRest` gain `--ci` token + `ci: boolean` in the run-shape union. - `runQueryCmd` / `runAuditCmd` gain `ci?: boolean` opt; threaded through to `printFormattedQuery` (query) and the post-render exit-code branch (audit). - `query`: exit 1 if `rows.length > 0` after SARIF emit. - `audit`: exit 1 if any delta has `added.length > 0` after SARIF emit. Tests: - 4 new `cmd-query` parser tests (--ci alias; --ci+--json reject; --ci+--format json reject; --ci+--format sarif accept). - 4 new `cmd-audit` parser tests (same matrix). - All existing toEqual tests updated for the `ci: false` field default. Smoke verified end-to-end: - `query --ci` with results → SARIF stdout, exit 1. - `audit --baseline X --ci` against identical baseline → 0 additions, exit 0. Diff with adds → exit 1. - Contradiction tests (`--ci --json`) emit clear error + exit 1.
Composite GitHub Action wrapping codemap CLI for the Marketplace.
~16 declarative inputs per Q1 resolution; package-manager detection +
codemap CLI invocation resolution via package-manager-detector
(antfu/userquin, MIT, 0 transitive deps, 23 kB).
action.yml shape:
- Skip-on-non-PR-events for the headline α default (audit --base
${{ github.base_ref }} --ci). Other events (push, schedule, …)
no-op + log "no PR context, skipping" + exit 0 unless an explicit
command: input is passed.
- 16 declarative inputs across 3 categories:
- WHERE TO RUN: working-directory, package-manager (override),
version (CLI pin), state-dir
- WHAT TO RUN: mode (audit | recipe | aggregate | command), recipe,
params, baseline, audit-base, changed-since, group-by, command
(escape hatch)
- WHAT TO DO WITH OUTPUT: format (default sarif), output-path,
upload-sarif, pr-comment (Slice 3 stub for v1.0), fail-on, token
- Validation precedence: command > mode > defaults; mode='aggregate'
rejected (reserved for v1.x post-Q6 SARIF rule.id de-dup work).
- 4 outputs: agent / exec / install_method (debug breadcrumbs) +
output-file (echoes inputs.output-path).
- Composite steps: gate → setup-node → detect-pm → validate → run →
upload-sarif (if Code Scanning enabled) → pr-comment-stub.
scripts/detect-pm.mjs:
- Wraps `package-manager-detector`'s `detect()` + `resolveCommand()`.
- Implements the Q3 invocation logic:
- VERSION env var set → 'execute' intent (dlx-pinned)
- codemap in devDependencies → 'execute-local'
- else → 'execute' intent (dlx-latest)
- Outputs to $GITHUB_OUTPUT per current Actions convention (set-output
deprecated 2022-10).
- Validates PACKAGE_MANAGER override against known agents.
- 8 unit tests covering: pnpm/bun/npm autodetect, no-lockfile fallback,
execute-local for project-installed, dlx-pinned override, manual PM
override, unknown PM rejection, packageManager-field priority over
lockfile.
New runtime dep: package-manager-detector@1.6.0 (MIT, antfu/userquin,
0 transitive deps).
…plan)
Slice 3 — markdown PR-summary writer for the cases SARIF→Code-Scanning
doesn't cover well: private repos without GHAS, repos that haven't
enabled Code Scanning, aggregate audit deltas without a single
file:line anchor, and bot-context seeding (review bots read PR
conversation, not workflow artifacts).
Architecture (engine + CLI separation, mirrors show / impact / audit):
- src/application/pr-comment-engine.ts — pure transport-agnostic
renderer. Auto-detects input shape (audit envelope vs SARIF doc) +
renders markdown grouped by delta key (audit) or rule id (SARIF).
Lists >50 entries collapse to `… and N more`. Removed rows surface
in their own collapsed section (audit only).
- src/cli/cmd-pr-comment.ts — CLI wrapper. Reads JSON from a file or
stdin (`-`). `--shape audit|sarif` overrides autodetection;
`--json` emits structured envelope `{ markdown, findings_count,
kind }` for action.yml steps.
- src/cli/main.ts + src/cli/bootstrap.ts wire the new `pr-comment`
verb (whitelist + dispatch).
action.yml integration (Slice 2 stub replaced):
- pr-comment toggle now actually invokes `codemap pr-comment` against
the SARIF / JSON output file produced by the run step, then posts
via `gh pr comment <PR> -F -`. Same binary that produced the output
renders the comment — version stream stays coherent.
Tests:
- 12 new pr-comment-engine unit tests (input shape detection,
no-drift / no-findings ✅ rendering, audit summary line + per-delta
sections, SARIF rule grouping, location formatting, >50 collapse).
- Smoke verified: real audit envelope produces markdown with file:
links + delta sections; SARIF doc groups findings by rule id.
Lockstep updates (per docs/README.md Rule 10):
- .agents/rules/codemap.md + templates/agents/rules/codemap.md
gain rows for `--ci` aggregate flag (Slice 1b) and `pr-comment`
renderer (Slice 3).
… plan) Adds an `action-smoke` job to .github/workflows/ci.yml that runs `uses: ./` from this repo on every PR. Validates: - action.yml YAML syntax + composite-step flow - gate step (skip-on-non-PR; command-set-overrides-skip) - setup-node + npm install of package-manager-detector - scripts/detect-pm.mjs execution + GITHUB_OUTPUT writes - detect-pm command resolution (`<pm> dlx codemap@latest --version`) Smoke uses `command: --version` to avoid the real-audit dependency chain (audit baselines etc.) — codemap audit logic is covered by the unit-test suite. The action-smoke validates the wrapper, not the underlying CLI. Non-blocking (`continue-on-error: true`) until v1.0.0 of codemap is published — today the action pulls codemap@latest from npm (0.4.0), which works for `--version` but doesn't validate v1.x flags. Promote to a hard gate when v1.0.0 ships. Bundles the v1.0 changeset: - .changeset/marketplace-action.md describes the full Slice 1b-4 surface (--ci flag, action.yml, pr-comment, dogfood) as one minor release. Slice 5 (Marketplace publish + listing metadata) is post-merge once a v1.0.0 tag exists.
SutuSebastian
changed the title
codemap audit (Slice 1a of #73 plan)
Contributor
Author
|
@coderabbitai please review — all 4 implementation slices for the GitHub Marketplace Action plan are now in this PR. Specific things to verify: (1) audit-engine integration with output-formatters.ts emits valid SARIF 2.1.0 rule.id taxonomy; (2) |
|
Tip For best results, initiate chat on the files or code changes.
Noted that [full_review] ✏️ Learnings added
|
Per .agents/rules/concise-comments.md — re-read every comment authored in this PR and slimmed the ones the code itself already conveys. Net: +68 / -163 lines of comments / docstrings; same information density, fewer words. Cuts/slims (in order of file): - output-formatters.ts: AuditSarifDelta + formatAuditSarif JSDoc; the message-fallback inline comment. - cmd-audit.ts: AUDIT_OUTPUT_FORMATS doc-block; jsonShortcut / ci / Reconcile / --ci-exit / emitAuditError comments. - cmd-query.ts: --ci variable + resolveFormat doc; printFormattedQuery's ci option doc; "no-locatable-rows warning" inline; runQueryCmd's ci option JSDoc; parseQueryRest's ci field JSDoc. - pr-comment-engine.ts: file header; RenderedComment JSDoc; "kind" field doc; detectCommentInputShape doc; renderAuditComment + renderSarifComment JSDocs; "Header summary line" / "Group by ruleId" / "Prefer the most-specific identity columns first." / "Unknown shape — fall through to JSON." inline comments. - cmd-pr-comment.ts: PrCommentOpts.shape doc; printPrCommentCmdHelp JSDoc; runPrCommentCmd JSDoc; "detected here is …" inline; readStdinSync gotcha doc. - scripts/detect-pm.mjs: file header; "Step 1/2/3:" labels; "Local / non-Actions invocation —" comment; codemapInDevDependencies JSDoc. - action.yml: "Build args based on inputs" inline. Docs staleness sweep — fact-checked against the codebase: - docs/architecture.md § "Audit wiring" listed audit's flags but pre-dated --format sarif + --ci. Added both + the formatAuditSarif shape note. - docs/architecture.md § "Output formatters" missed formatAuditSarif. Added (one rule per delta key, severity warning, removed-rows excluded, location-only fallback). - docs/architecture.md § "Query wiring" missed --ci. Added. - docs/architecture.md added a new "PR-comment wiring" section (mirrors the cmd ↔ engine seam pattern). - docs/glossary.md added `--ci` (under C) and `pr-comment` (under P) entries per Rule 9. - README.md CLI examples updated with `audit --format sarif`, `audit --ci`, `query --ci`, and the `pr-comment` pipe-to-`gh pr comment` idiom. Pre-existing comments (preserve-comments rule) untouched.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/bootstrap.ts (1)
8-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
pr-commentis missing fromprintCliUsage.
validateIndexModeArgsnow acceptspr-commentas a first-class verb (line 84) butprintCliUsageoutput has no corresponding entry. Users runningcodemap --helpwon't discover the command.📝 Suggested addition to the help text
Targeted reads (precise lookup by symbol name): codemap show <name> [--kind <k>] [--in <path>] [--json] # metadata: file:line + signature codemap snippet <name> [--kind <k>] [--in <path>] [--json] # source text from disk + stale flag + +PR comment (render audit JSON or SARIF as a markdown PR-summary): + codemap pr-comment <input.json> # or `-` to read from stdin🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/bootstrap.ts` around lines 8 - 61, printCliUsage currently omits the new "pr-comment" verb so users won't see it in --help; update the help string inside the printCliUsage() function to add a short usage line for the pr-comment command (mirror the style of other verb entries), referencing the new validateIndexModeArgs usage of "pr-comment" so the CLI help shows how to invoke codemap pr-comment and any relevant flags.src/cli/cmd-audit.ts (1)
263-267:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the intro paragraph for the new
--cibehavior.This still says audit has “No verdict / threshold / non-zero exit codes,” but
runAuditCmd()now sets a non-zero exit code under--ci. The current help text sends users toward the old--json + jqflow even though the command now has a built-in CI mode.Suggested fix
as a {head, deltas} envelope. Each delta carries its own \`base\` metadata. v1 ships three deltas: -files, dependencies, deprecated. No verdict / threshold / non-zero exit codes — compose --json + jq -for CI exit codes. +files, dependencies, deprecated. Use \`--ci\` for the built-in CI mode +(\`--format sarif\` + non-zero exit when any delta has additions); otherwise +the command remains informational.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/cmd-audit.ts` around lines 263 - 267, The help intro still claims "No verdict / threshold / non-zero exit codes" but runAuditCmd() now implements a CI mode that sets non-zero exit codes when --ci is used; update the intro paragraph text (the string used to build the audit command help/description in cmd-audit.ts) to remove the old claim and clearly state that using --ci enables CI behavior (non-zero exit codes on failures), what constitutes a failing condition, and that the legacy --json + jq flow is still available; ensure the change references the --ci flag and runAuditCmd() behavior so the documentation matches the implementation.
🧹 Nitpick comments (3)
src/application/output-formatters.ts (1)
192-239: ⚡ Quick winExtractable location-building helper — duplicates
formatSarif's region logic.The
physicalLocation+regionconstruction informatAuditSarif(lines 218–234) is nearly identical to the block informatSarif(lines 124–138). Extracting it reduces the surface for region-handling bugs to drift.♻️ Suggested refactor
+/** + * Build a SARIF physicalLocation for `locCol`'s value in `row`. + * Includes a region when line_start / line_end are positive integers. + */ +function buildPhysicalLocation( + row: Record<string, unknown>, + uri: string, +): Record<string, unknown> { + const lineStartRaw = row["line_start"]; + const lineEndRaw = row["line_end"]; + const region: Record<string, number> = {}; + if (typeof lineStartRaw === "number" && lineStartRaw > 0) { + region["startLine"] = lineStartRaw; + } + if (typeof lineEndRaw === "number" && lineEndRaw > 0) { + region["endLine"] = lineEndRaw; + } + const physicalLocation: Record<string, unknown> = { + artifactLocation: { uri }, + }; + if (Object.keys(region).length > 0) { + physicalLocation["region"] = region; + } + return physicalLocation; +}Then in
formatSarifandformatAuditSarifreplace the duplicated block with:- const physicalLocation: Record<string, unknown> = { - artifactLocation: { uri }, - }; - if (Object.keys(region).length > 0) { - physicalLocation["region"] = region; - } + const physicalLocation = buildPhysicalLocation(row, uri);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/application/output-formatters.ts` around lines 192 - 239, The region/physicalLocation construction duplicated in formatAuditSarif and formatSarif should be extracted into a single helper (e.g. buildPhysicalLocationFromRow(row: Record<string, unknown>, locCol: string | null): { physicalLocation?: Record<string, unknown> } | null) that reads uri, line_start and line_end, builds the region only when start/end are valid numbers > 0, and returns the physicalLocation object (or null if no locCol). Replace the inline blocks in formatAuditSarif and formatSarif with calls to this helper and set result.locations = [{ physicalLocation }] only when the helper returns a value; ensure the helper signature and return types align with existing usage of detectLocationColumn and the shape expected by SARIF consumers.package.json (1)
81-81: ⚡ Quick win
package-manager-detectorshould be adevDependency, not adependency.
scripts/detect-pm.mjs(the only consumer of this package) is not included in the published npm package — the"files"field only coversCHANGELOG.md,dist, andtemplates. Publishing it underdependenciesmeans everynpm install@stainless-code/codemap`` forces end users to download a package they'll never use at runtime.🔧 Proposed fix
"dependencies": { "@clack/prompts": "^1.2.0", "@modelcontextprotocol/sdk": "^1.29.0", "better-sqlite3": "^12.9.0", "chokidar": "^5.0.0", "lightningcss": "^1.32.0", "oxc-parser": "^0.127.0", "oxc-resolver": "^11.19.1", - "package-manager-detector": "^1.6.0", "tinyglobby": "^0.2.16", "zod": "^4.3.6" }, "devDependencies": { ... + "package-manager-detector": "^1.6.0", ... }Note: Verify that
action.yml's composite steps don't rely on a--productioninstall that would excludedevDependencies; if they do, the action must installpackage-manager-detectorexplicitly in an earlier step rather than relying onpackage.jsonclassification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 81, Move "package-manager-detector" from dependencies into devDependencies: remove the "package-manager-detector": "^1.6.0" entry under "dependencies" in package.json and add the same entry under "devDependencies" (or run npm install --save-dev package-manager-detector@^1.6.0) so only dev installs pull it in; reference the consumer script scripts/detect-pm.mjs to confirm it's only used at build/dev time and verify action.yml doesn’t expect devDependencies to be skipped by a --production install (if it does, explicitly install package-manager-detector in the action instead).src/cli/cmd-pr-comment.ts (1)
66-132: ⚡ Quick winDocument the exported parser surface.
ParsedPrCommentRestandparsePrCommentRest()are public exports, but they are the only public API in this file without accompanying docs. A short JSDoc covering the return variants and positional-arg contract would keep the CLI surface self-describing.As per coding guidelines, "All public APIs must have accompanying documentation".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/cmd-pr-comment.ts` around lines 66 - 132, Add JSDoc comments for the exported ParsedPrCommentRest type and parsePrCommentRest function: document the three kind variants ("run","help","error") and what fields are present for each variant (e.g., inputPath/shape/json for "run", message for "error"), describe the positional-argument contract (first token must be "pr-comment", single inputPath or "-" for stdin, and supported flags --help/-h, --json, --shape=<audit|sarif>), and note error conditions returned as kind:"error"; attach these comments immediately above the ParsedPrCommentRest declaration and the parsePrCommentRest function to satisfy the public API documentation requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@action.yml`:
- Around line 44-46: The description for the input named audit-base currently
contains the expression `${{ github.base_ref }}` which causes GitHub Actions to
try to evaluate an unavailable context; update the audit-base input description
to remove the interpolation and use a literal example or placeholder (e.g.,
"Default: the base ref of the PR (e.g. main)" or "Default: $GITHUB_BASE_REF") so
the description remains static; locate the audit-base input in action.yml and
replace the description string accordingly.
- Around line 162-181: The validate step currently allows MODE='command' but
doesn't check that the COMMAND input is provided, so ARGS can remain empty and
EXEC is called with no arguments; add a guard similar to the existing recipe
check: when MODE equals "command" and COMMAND is empty (and RECIPE is also empty
if relevant), emit an ::error:: message explaining that mode='command' requires
the 'command' input and exit 1; reference the MODE, COMMAND, RECIPE, ARGS and
EXEC variables and the case statement so the check sits alongside the existing
recipe validation.
In `@scripts/detect-pm.mjs`:
- Around line 101-114: codemapInDevDependencies currently checks for manifest
keys using dot notation for "codemap", which misses the actual package name
"@stainless-code/codemap"; change the checks in codemapInDevDependencies to use
bracket notation and test for the scoped package name
(manifest.dependencies?.['@stainless-code/codemap'] etc.), and for robustness
also accept the unscoped 'codemap' key (e.g., manifest.dependencies &&
(manifest.dependencies['@stainless-code/codemap'] ||
manifest.dependencies['codemap'])), doing the same for devDependencies and
optionalDependencies; keep the try/catch behavior and then update the call site
that chooses the project-installed vs dlx-latest branch to rely on this
corrected codemapInDevDependencies result.
In `@src/application/output-formatters.test.ts`:
- Around line 612-614: The test currently calls expect(run.results.every((r: {
level: string }) => r.level === "warning")) but never chains a matcher, making
the assertion a no-op; update the assertion (in
src/application/output-formatters.test.ts around where `run` and
`formatAuditSarif` are exercised) to actually assert truthiness, e.g. chain a
matcher like .toBe(true) or .toEqual(true) so the check that every result's
level equals "warning" is enforced; locate the assertion referencing
run.results.every(...) and add the appropriate matcher.
In `@src/application/pr-comment-engine.ts`:
- Around line 162-165: The findings_count currently only uses totalAdded which
ignores removals; update the renderer return to compute findings_count as the
combined drift (e.g., totalAdded + totalRemoved) so that removed-only audits are
counted and clean detection (totalAdded + totalRemoved == 0) works
correctly—change the reference in the object returned (findings_count) to use
both totalAdded and totalRemoved rather than totalAdded alone.
- Around line 179-225: The code only reads doc.runs?.[0] causing markdown and
findings_count to reflect only the first SARIF run; update the logic to iterate
all runs (doc.runs) and aggregate every run's results into the same results
array or directly into the byRule Map so that grouping, header summary, per-rule
sections, and findings_count include all runs; ensure you replace usages of the
single-run variable (results, results.length) with the aggregated total and
still cap per-rule display at 50 using the existing formatSarifLine function and
retain the current HTML/details formatting.
In `@src/cli/cmd-pr-comment.ts`:
- Around line 210-224: readStdinSync uses require("node:fs").readSync which
breaks in this ESM module; import readSync from node:fs at the top of the file
and replace the require call with a direct call to readSync in the readStdinSync
function (keep the same buffer/loop logic and return
Buffer.concat(chunks).toString("utf8")); ensure the top-of-file import uses the
named readSync from "node:fs" so the function works at runtime.
In `@templates/agents/rules/codemap.md`:
- Around line 45-46: The table row describing the `--ci` aggregate flag is
inconsistent with the earlier doc text: update the `--ci` row (the line
containing "`--ci` aggregate flag" and the phrase "suppresses no-findings stderr
warning") to state it suppresses the "no-location / no-locatable-rows" warning
instead of "no-findings", keeping the rest of the description (aliases, non-zero
exit on findings/additions, mutual exclusivity with `--json`/`--format`)
unchanged so the terminology matches the earlier rule language.
---
Outside diff comments:
In `@src/cli/bootstrap.ts`:
- Around line 8-61: printCliUsage currently omits the new "pr-comment" verb so
users won't see it in --help; update the help string inside the printCliUsage()
function to add a short usage line for the pr-comment command (mirror the style
of other verb entries), referencing the new validateIndexModeArgs usage of
"pr-comment" so the CLI help shows how to invoke codemap pr-comment and any
relevant flags.
In `@src/cli/cmd-audit.ts`:
- Around line 263-267: The help intro still claims "No verdict / threshold /
non-zero exit codes" but runAuditCmd() now implements a CI mode that sets
non-zero exit codes when --ci is used; update the intro paragraph text (the
string used to build the audit command help/description in cmd-audit.ts) to
remove the old claim and clearly state that using --ci enables CI behavior
(non-zero exit codes on failures), what constitutes a failing condition, and
that the legacy --json + jq flow is still available; ensure the change
references the --ci flag and runAuditCmd() behavior so the documentation matches
the implementation.
---
Nitpick comments:
In `@package.json`:
- Line 81: Move "package-manager-detector" from dependencies into
devDependencies: remove the "package-manager-detector": "^1.6.0" entry under
"dependencies" in package.json and add the same entry under "devDependencies"
(or run npm install --save-dev package-manager-detector@^1.6.0) so only dev
installs pull it in; reference the consumer script scripts/detect-pm.mjs to
confirm it's only used at build/dev time and verify action.yml doesn’t expect
devDependencies to be skipped by a --production install (if it does, explicitly
install package-manager-detector in the action instead).
In `@src/application/output-formatters.ts`:
- Around line 192-239: The region/physicalLocation construction duplicated in
formatAuditSarif and formatSarif should be extracted into a single helper (e.g.
buildPhysicalLocationFromRow(row: Record<string, unknown>, locCol: string |
null): { physicalLocation?: Record<string, unknown> } | null) that reads uri,
line_start and line_end, builds the region only when start/end are valid numbers
> 0, and returns the physicalLocation object (or null if no locCol). Replace the
inline blocks in formatAuditSarif and formatSarif with calls to this helper and
set result.locations = [{ physicalLocation }] only when the helper returns a
value; ensure the helper signature and return types align with existing usage of
detectLocationColumn and the shape expected by SARIF consumers.
In `@src/cli/cmd-pr-comment.ts`:
- Around line 66-132: Add JSDoc comments for the exported ParsedPrCommentRest
type and parsePrCommentRest function: document the three kind variants
("run","help","error") and what fields are present for each variant (e.g.,
inputPath/shape/json for "run", message for "error"), describe the
positional-argument contract (first token must be "pr-comment", single inputPath
or "-" for stdin, and supported flags --help/-h, --json, --shape=<audit|sarif>),
and note error conditions returned as kind:"error"; attach these comments
immediately above the ParsedPrCommentRest declaration and the parsePrCommentRest
function to satisfy the public API documentation requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bcea25af-c57c-49d0-81cc-2426d586efc0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.agents/rules/codemap.md.changeset/audit-format-sarif.md.changeset/marketplace-action.md.github/workflows/ci.ymlaction.ymlpackage.jsonscripts/detect-pm.mjsscripts/detect-pm.test.mjssrc/application/output-formatters.test.tssrc/application/output-formatters.tssrc/application/pr-comment-engine.test.tssrc/application/pr-comment-engine.tssrc/cli/bootstrap.tssrc/cli/cmd-audit.test.tssrc/cli/cmd-audit.tssrc/cli/cmd-pr-comment.tssrc/cli/cmd-query.test.tssrc/cli/cmd-query.tssrc/cli/main.tstemplates/agents/rules/codemap.md
…ons reject expressions in input defaults) CI failure on PR #74's Action smoke job: TemplateValidationException: Unrecognized named-value: 'github'. Located at position 1 within expression: github.base_ref / github.token GitHub composite actions do NOT allow ${{ github.* }} expressions in input defaults. Only `runs:` step expressions can reference the github context. Two inputs were affected: - `audit-base`: now defaults to "". The existing run step already does `BASE="${AUDIT_BASE:-$BASE_REF}"` where `BASE_REF: ${{ github.base_ref }}` is set as an env var (legal in step env blocks), so empty-input → PR base_ref behavior is preserved. - `token`: now defaults to "". Two call sites (`upload-sarif` step's `token:` arg + `pr-comment` step's `GH_TOKEN`) now use `${{ inputs.token != '' && inputs.token || github.token }}` to fall back to `github.token` when unset. Both inputs' descriptions updated to document the empty-falls-back behavior so consumers know what to expect.
Fact-checked each finding against the codebase; all valid. Two were
critical correctness bugs (T2, T3, T5) that would have shipped silently
broken behavior.
T2 (Major) — `scripts/detect-pm.mjs` wrong package-name keys
`codemapInDevDependencies` checked `manifest?.dependencies?.codemap` but
the published package is `@stainless-code/codemap` — the project-installed
branch was dead code for all real consumers; `dlx-pinned` / `dlx-latest`
also pulled the unscoped `codemap` package (a different npm namespace
entirely). Fixed:
- Lookup checks both scoped (`@stainless-code/codemap`) and bare
(`codemap`) keys — the latter for monorepos that alias via
`"codemap": "workspace:*"`.
- `'execute'` (dlx) commandArgs now use the scoped published name so
npm/pnpm/yarn/bun resolve the right registry entry.
- `'execute-local'` keeps `["codemap"]` because that's the bin alias
per `package.json#bin`, regardless of the scoped name.
- Tests updated: scoped-dev-dep / bare-dev-dep / scoped-dlx-pinned
cases. Old tests that asserted dlx-with-unscoped-`codemap@<v>` were
themselves testing a bug.
T3 (Critical) — no-op `expect()` in formatAuditSarif test
`expect(run.results.every(...))` without a chained matcher creates and
discards the expectation. If formatAuditSarif reverted to severity
`note`, the test would still pass. Added `.toBe(true)`.
T5 (Major) — `require()` in ESM module
`readStdinSync` in `cmd-pr-comment.ts` used `require("node:fs").readSync`
but the file loads via `await import()` (ESM); `require` is undefined at
runtime. `codemap pr-comment -` would have crashed for every user. My
unit tests passed file paths, not stdin — caught nothing. Imported
`readSync` from `node:fs` at the top, used directly.
T4 (Major) — pr-comment SARIF renderer dropped runs[1+]
`renderSarifComment` only read `doc.runs?.[0]?.results`. Valid SARIF
allows multiple runs (merged / multi-tool docs). Now flattens via
`(doc.runs ?? []).flatMap(run => run.results ?? [])`. New test
`aggregates results across multi-run SARIF docs (not just runs[0])`.
T1 (Minor) — `mode: command` without `command:` input falls through
The validate step accepted `mode=command` but didn't guard against
empty `command:`. Run step's if/elif only handled `audit` + `recipe`,
so `$EXEC` would invoke with empty `$ARGS`. Added an explicit guard
mirroring the `mode=recipe` pattern.
T6 (Minor) — `--ci` doc said "no-findings warning"; actual is "no-locatable-rows"
Both `.agents/rules/codemap.md` and `templates/agents/rules/codemap.md`
described the suppressed warning incorrectly. Aligned wording with the
implementation in `printFormattedQuery`.
`non-goals-reassessment-2026-05.md` § 1 Shipped table + § 7 Lifted-to table contradicted each other on § 1.5 — § 1 didn't list it (lifted table was last edited before PR #72), § 7 still said "(pending)". Boundary-violations shipped 2026-05 as PR #72; both tables now reflect that, plus the "Pending picks" enumeration drops § 1.5. Per Rule 8, research notes get closed/lifted but factual state may still be corrected when the codebase moves under them. This is hygiene, not extension of the analysis.
Slice 5 (publish + Marketplace listing) is post-merge work — anyone should be able to pick it up without triangulating across the plan's Q7 release-workflow + Q10 listing-metadata + Slice-5 one-liner. Added a "Slice 5 runbook (post-merge)" subsection that: - Surfaces the CLI / Action version stream decoupling explicitly: Action ships at v1 against CLI 0.5.0 (the version PR #74's changeset bumps to). CLI v1.0.0 is not required. - Lists 7 sequenced executable steps from "merge PR #74" through to "delete this plan per Rule 3" (the canonical close-out per docs-governance). - Calls out which docs already hold the durable design decisions (architecture.md / glossary.md / agent rules / MARKETPLACE.md) so the deletion in step 7 is safe. - Documents subsequent-release cadence (changesets-driven; force-push the floating v<major> tag) and the major-bump policy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implementation of Slices 1-4 from the GitHub Marketplace Action plan (PR #73). Slice 5 (Marketplace publish + listing metadata) is post-merge once a v1.0.0 tag exists.
5 commits, kept distinct for review clarity:
b65df6aaudit --format sarif(extendscmd-audit.tsparser +output-formatters.ts)7a81b88--ciaggregate flag onquery+audit(alias for--format sarif+ non-zero exit)9383a95action.yml(composite, ~16 declarative inputs) +scripts/detect-pm.mjs3409d27codemap pr-comment(engine + CLI + action.yml hookup)08dec00action-smokeCI job + bundle changesetSlice 1a:
audit --format sarifauditpreviously only emitted--json; now supports SARIF 2.1.0 directly so CI consumers don't need a JSON→SARIF translation step.--format <text|json|sarif>flag (defaulttext);--jsonstays as backward-compat shortcut.codemap.audit.files-added,codemap.audit.dependencies-added,codemap.audit.deprecated-added); one result peraddedrow; severitywarning. Locations auto-detected via existingfile_path/path/to_path/from_pathpriority list.removedrows excluded from SARIF (findings to act on, not cleanups).Slice 1b:
--ciaggregate flagLands on both
queryandaudit(the two finding-producing verbs). Aliases--format sarif+process.exitCode = 1if any rows/additions + suppresses no-locatable-rows stderr warning. Mutually exclusive with--jsonand--format <other>.cmd-queryparser tests + 4 newcmd-auditparser tests.ci: falsefield default.Slice 2:
action.yml+scripts/detect-pm.mjspull_request:audit --base ${{ github.base_ref }} --ci. Other events no-op + log unless an explicitcommand:input is passed.package-manager-detector(antfu/userquin, MIT, 0 transitive deps, 23 kB).scripts/detect-pm.mjswraps the library'sdetect()+resolveCommand(). Implements Q3's project-installed-first / dlx-fallback logic. Outputs to$GITHUB_OUTPUTper current Actions convention (::set-outputdeprecated 2022-10).Slice 3:
codemap pr-commentMarkdown PR-summary writer for cases SARIF→Code-Scanning doesn't cover: private repos without GHAS, repos that haven't enabled Code Scanning, aggregate audit deltas, bot-context seeding (review bots read PR conversation, not workflow artifacts).
show/impact/audit).pr-comment-engine.tsis pure: input shape detection, audit / SARIF rendering, location formatting, >50-row collapse.cmd-pr-comment.tsreads from a file or stdin (-);--shape audit|sarifoverrides;--jsonenvelope{ markdown, findings_count, kind }for action.yml consumers.action.yml's pr-comment step now invokescodemap pr-commentagainst the run-step output and posts viagh pr comment <PR> -F -.Slice 4: Dogfood
action-smokejob in.github/workflows/ci.ymlrunsuses: ./from this very repo on every PR. Validates the action.yml YAML + composite-step flow + detect-pm + actual CLI invocation.command: --version(avoids the real-audit dependency chain — covered by unit tests).v1.0.0of codemap ships; today the action pullscodemap@latestfrom npm (0.4.0), which has--versionbut not the v1.x flags. Promote to hard gate post-publish.Lockstep updates (per
docs/README.mdRule 10).agents/rules/codemap.md+templates/agents/rules/codemap.mdgain rows for--ciaggregate flag andpr-commentrenderer..changeset/audit-format-sarif.md(Slice 1a) +.changeset/marketplace-action.md(bundles Slices 1b-4).Test plan
bun run typecheck)bun run check) — 750+ tests + golden scenarios all greenaudit --format sarifproduces valid SARIF 2.1.0 (Code-Scanning compatible)query --ciexits 1 on findings; contradiction errors emit clearlyaudit --ciexits 1 only when deltas have additionspr-commentrenders both audit envelopes and SARIF docs as readable markdown--jsonshortcut still works on both verbsFollowups (post-merge)
v1.0.0, push fast-forward@v1, fill Marketplace listing metadata (icon, description, tags) per Q10 resolution. One-time setup; documented in the plan.action-smoketo a hard gate (dropcontinue-on-error).Summary by CodeRabbit
Release Notes
--format sarifoption for audit command (SARIF 2.1.0 output)--ciflag for CI-optimized behavior with exit codes and output formattingcodemap pr-commentcommand to render audit/SARIF findings as PR comments