diff --git a/README.md b/README.md index fbbe6562..c694abfa 100644 --- a/README.md +++ b/README.md @@ -304,8 +304,10 @@ answer where the next decision is actually being made. objections) as Markdown or `QueueReport` v1 JSON; also a managed-issue Action. - **CI comment** — the `@adrkit/ci` GitHub Action surfaces the governing decisions on the PRs that touch or explicitly declare them; pattern matches render as `via` - and PR-authored marker claims as `declared by`. It runs with only the default `GITHUB_TOKEN` and - degrades (never fails the job) on a read-only fork token. + and PR-authored marker claims as `declared by`. The comment also distinguishes + marker files it could not inspect from marker claims it read but could not bind. + Both are advisory: they never fail the job. It runs with only the default + `GITHUB_TOKEN` and degrades (never fails the job) on a read-only fork token. - **MCP server** — let agents retrieve prior decisions, including the rejected ones, before proposing something already tried. diff --git a/packages/ci/dist/index.js b/packages/ci/dist/index.js index 5b330e83..cba9b5e5 100644 --- a/packages/ci/dist/index.js +++ b/packages/ci/dist/index.js @@ -48622,6 +48622,8 @@ var HISTORY_HEADING = "#### Historical records that once covered this change"; var HISTORY_NOTE = "These no longer bind this change, and are listed for context only:"; var MAX_GOVERNING = 50; var MAX_DECLARATIONS = 10; +var MAX_MARKER_PATHS_PER_STATE = 10; +var MAX_MARKER_CLAIMS = 20; var MAX_COMMENT_CHARS = 65536; var TRUNCATION_NOTICE = "- …output truncated to fit GitHub’s comment size limit; run `adr check` locally for the complete result."; var MAX_FINDING_FIELD_CHARS = 256; @@ -48651,6 +48653,51 @@ function renderFindingLine(finding) { const message = boundedDetail(finding.message, MAX_FINDING_MESSAGE_CHARS, "message"); return `- ${where} — ${code(finding.rule)}${field}: ${message}`; } +function markerScanHealthLines(report) { + const states = [ + ["absent", report.absentPaths], + ["unreadable", report.unreadablePaths], + ["out-of-tree", report.outOfTreePaths], + ["skipped at the scan cap", report.skippedPaths] + ]; + const unavailable = states.reduce((total, [, paths]) => total + paths.length, 0); + if (unavailable === 0) + return []; + const lines = [ + "#### Marker scan health", + "", + `Marker scanning could not inspect ${unavailable} changed file${unavailable === 1 ? "" : "s"}:` + ]; + for (const [label, paths] of states) { + if (paths.length === 0) + continue; + const shown = paths.slice(0, MAX_MARKER_PATHS_PER_STATE).map(code).join(", "); + const remaining = paths.length - Math.min(paths.length, MAX_MARKER_PATHS_PER_STATE); + lines.push(`- ${paths.length} ${label}: ${shown}${remaining > 0 ? `, and ${remaining} more` : ""}`); + } + lines.push("", "These files could not be inspected for `@adr` markers; an empty result does not prove that no marker is present."); + return lines; +} +function markerClaimLines(outcome) { + const changed = new Set(outcome.changedFiles); + const claims = outcome.findings.filter((finding) => finding.field === "marker" && finding.path !== undefined && changed.has(finding.path) && finding.rule !== "marker-scan-capped"); + if (claims.length === 0) + return []; + const shown = claims.slice(0, MAX_MARKER_CLAIMS); + const lines = [ + "#### Marker claims not bound", + "", + "Marker scanning was healthy for these changed files, but the claims did not bind to a record in this corpus:" + ]; + for (const finding of shown) { + lines.push(`- ${code(finding.path ?? "(unknown)")} — ${code(boundedDetail(finding.message, MAX_FINDING_MESSAGE_CHARS, "message"))}`); + } + const remaining = claims.length - shown.length; + if (remaining > 0) { + lines.push(`- …and ${remaining} more marker claim${remaining === 1 ? "" : "s"}`); + } + return lines; +} function renderDecisionLines(decision, withStatus) { const status = withStatus ? ` _(${decision.status})_` : ""; const successor = decision.supersededBy ? ` — superseded by **${decision.supersededBy}**` : ""; @@ -48708,6 +48755,14 @@ function renderComment(outcome) { lines.push(renderFindingLine(finding)); lines.push(""); } + if (outcome.markerScan) { + const markerAdvisories = [ + ...markerScanHealthLines(outcome.markerScan), + ...markerClaimLines(outcome) + ]; + if (markerAdvisories.length > 0) + lines.push(...markerAdvisories, ""); + } if (outcome.governedBy.length === 0) { lines.push(EMPTY_STATE); } else if (outcome.governing.length === 0) { diff --git a/packages/ci/src/comment.ts b/packages/ci/src/comment.ts index 989740d9..0de955d8 100644 --- a/packages/ci/src/comment.ts +++ b/packages/ci/src/comment.ts @@ -33,6 +33,8 @@ const MAX_GOVERNING = 50; // window holds ~630 `// @adr 0021` lines, and the path in each is the author's too. // Bounding what is rendered is what keeps that content out of the body budget below. const MAX_DECLARATIONS = 10; +const MAX_MARKER_PATHS_PER_STATE = 10; +const MAX_MARKER_CLAIMS = 20; /** * GitHub rejects a comment body over 65,536 characters with a 422. That is not a @@ -97,6 +99,63 @@ function renderFindingLine(finding: Finding): string { return `- ${where} — ${code(finding.rule)}${field}: ${message}`; } +function markerScanHealthLines(report: NonNullable): string[] { + const states: readonly [string, readonly string[]][] = [ + ['absent', report.absentPaths], + ['unreadable', report.unreadablePaths], + ['out-of-tree', report.outOfTreePaths], + ['skipped at the scan cap', report.skippedPaths], + ]; + const unavailable = states.reduce((total, [, paths]) => total + paths.length, 0); + if (unavailable === 0) return []; + + const lines = [ + '#### Marker scan health', + '', + `Marker scanning could not inspect ${unavailable} changed file${unavailable === 1 ? '' : 's'}:`, + ]; + for (const [label, paths] of states) { + if (paths.length === 0) continue; + const shown = paths.slice(0, MAX_MARKER_PATHS_PER_STATE).map(code).join(', '); + const remaining = paths.length - Math.min(paths.length, MAX_MARKER_PATHS_PER_STATE); + lines.push( + `- ${paths.length} ${label}: ${shown}${remaining > 0 ? `, and ${remaining} more` : ''}`, + ); + } + lines.push( + '', + 'These files could not be inspected for `@adr` markers; an empty result does not prove that no marker is present.', + ); + return lines; +} + +function markerClaimLines(outcome: CheckOutcome): string[] { + const changed = new Set(outcome.changedFiles); + const claims = outcome.findings.filter( + (finding) => + finding.field === 'marker' && + finding.path !== undefined && + changed.has(finding.path) && + finding.rule !== 'marker-scan-capped', + ); + if (claims.length === 0) return []; + + const shown = claims.slice(0, MAX_MARKER_CLAIMS); + const lines = [ + '#### Marker claims not bound', + '', + 'Marker scanning was healthy for these changed files, but the claims did not bind to a record in this corpus:', + ]; + for (const finding of shown) { + lines.push(`- ${code(finding.path ?? '(unknown)')} — ${code(boundedDetail(finding.message, MAX_FINDING_MESSAGE_CHARS, 'message'))}`); + } + const remaining = claims.length - shown.length; + if (remaining > 0) { + lines.push(`- …and ${remaining} more marker claim${remaining === 1 ? '' : 's'}`); + } + return lines; +} + /** * One decision as a bullet, annotated with its status and — for superseded records — * the successor that replaced it, so a reviewer is never shown a bare record id and @@ -180,6 +239,14 @@ export function renderComment(outcome: CheckOutcome): string { lines.push(''); } + if (outcome.markerScan) { + const markerAdvisories = [ + ...markerScanHealthLines(outcome.markerScan), + ...markerClaimLines(outcome), + ]; + if (markerAdvisories.length > 0) lines.push(...markerAdvisories, ''); + } + if (outcome.governedBy.length === 0) { lines.push(EMPTY_STATE); } else if (outcome.governing.length === 0) { diff --git a/packages/ci/test/action.test.ts b/packages/ci/test/action.test.ts index 9e41f8a7..593c0a27 100644 --- a/packages/ci/test/action.test.ts +++ b/packages/ci/test/action.test.ts @@ -156,7 +156,7 @@ describe('runAction (end to end with a fake client)', () => { expect(result.outcome?.governing.map((decision) => decision.recordId)).toEqual(['0001']); }); - test('keeps dangling markers non-failing and out of the focused PR comment', async () => { + test('reports dangling markers without making them failing', async () => { const root = await resetTestDir(DIR_NAME); await mkdir(join(root, 'docs/adr'), { recursive: true }); await writeText(join(root, 'src/dangling.ts'), '// @adr 9999\n'); @@ -167,7 +167,8 @@ describe('runAction (end to end with a fake client)', () => { expect(result.failed).toBe(false); expect(result.outcome?.ok).toBe(true); expect(result.outcome?.findings.map((finding) => finding.rule)).toContain('dangling-marker'); - expect(client.created[0]).not.toContain('dangling-marker'); + expect(client.created[0]).toContain('#### Marker claims not bound'); + expect(client.created[0]).toContain('@adr 9999'); }); test('warns with the exact skipped paths when the marker scan cap is reached', async () => { diff --git a/packages/ci/test/comment-render.test.ts b/packages/ci/test/comment-render.test.ts index aabe68b9..da8be42b 100644 --- a/packages/ci/test/comment-render.test.ts +++ b/packages/ci/test/comment-render.test.ts @@ -79,22 +79,116 @@ describe('renderComment', () => { expect(body).not.toContain('**0002**'); }); - test('keeps marker reference warnings out of the focused PR comment', async () => { + test('separates marker scan health from the empty governing state', async () => { const root = await seed(); - const outcome = await outcomeFor(root, ['docs/adr/0001-api.md']); + const outcome = await outcomeFor(root, ['src/a.ts']); + outcome.markerScan = { + totalCandidates: 1, + limit: 3000, + counts: { scanned: 0, absent: 1, unreadable: 0, 'out-of-tree': 0, truncated: 0, skipped: 0 }, + absentPaths: ['src/a.ts'], + unreadablePaths: [], + outOfTreePaths: [], + truncatedPaths: [], + skippedPaths: [], + }; + + const body = renderComment(outcome); + + expect(body).toContain( + 'no marker is present.\n\nNo governing decisions for the changed files.', + ); + }); + + test('reports a dangling marker after a healthy scan without making it blocking', async () => { + const root = await seed(); + const outcome = await outcomeFor(root, ['src/owned.ts']); outcome.findings.push({ rule: 'dangling-marker', severity: 'warn', - message: 'Source marker does not resolve', - path: 'docs/adr/0001-api.md', + message: 'Source marker "@adr 9999" in src/owned.ts:1 does not resolve to a record in the corpus', + path: 'src/owned.ts', field: 'marker', pattern: '9999', }); + outcome.markerScan = { + totalCandidates: 1, + limit: 3000, + counts: { scanned: 1, absent: 0, unreadable: 0, 'out-of-tree': 0, truncated: 0, skipped: 0 }, + absentPaths: [], + unreadablePaths: [], + outOfTreePaths: [], + truncatedPaths: [], + skippedPaths: [], + }; const body = renderComment(outcome); - expect(body).not.toContain('dangling-marker'); - expect(body).not.toContain('Source marker does not resolve'); + expect(body).toContain('#### Marker claims not bound'); + expect(body).toContain('src/owned.ts'); + expect(body).toContain('@adr 9999'); + expect(body).not.toContain('#### Marker scan health'); + }); + + test('reports files the marker scan could not inspect, with bounded safe paths', async () => { + const root = await seed(); + const outcome = await outcomeFor(root, ['src/a.ts']); + const paths = Array.from({ length: 12 }, (_, index) => `src/${index}-\`[x](https://evil.example).ts`); + outcome.changedFiles = paths; + outcome.markerScan = { + totalCandidates: paths.length, + limit: 3000, + counts: { scanned: 0, absent: 12, unreadable: 0, 'out-of-tree': 0, truncated: 0, skipped: 0 }, + absentPaths: paths, + unreadablePaths: [], + outOfTreePaths: [], + truncatedPaths: [], + skippedPaths: [], + }; + + const body = renderComment(outcome); + + expect(body).toContain('#### Marker scan health'); + expect(body).toContain('could not inspect 12 changed files'); + expect(body).toContain('and 2 more'); + expect(body).toContain('``src/0-'); + expect(body).not.toContain('- `src/0-'); + }); + + test('keeps changed-record validation errors ahead of marker reports under the body budget', async () => { + const root = await seed(); + const outcome = await outcomeFor(root, ['docs/adr/0001-api.md']); + outcome.changedRecords = ['docs/adr/broken.md']; + outcome.findings = [{ + rule: 'frontmatter-parse', + severity: 'error', + message: 'broken record', + path: 'docs/adr/broken.md', + }]; + outcome.markerScan = { + totalCandidates: 1, + limit: 3000, + counts: { scanned: 0, absent: 1, unreadable: 0, 'out-of-tree': 0, truncated: 0, skipped: 0 }, + absentPaths: ['src/a.ts'], + unreadablePaths: [], + outOfTreePaths: [], + truncatedPaths: [], + skippedPaths: [], + }; + outcome.findings.push(...Array.from({ length: 4000 }, (_, index) => ({ + rule: 'dangling-marker', + severity: 'warn' as const, + message: `claim ${index} ${'x'.repeat(60)}`, + path: 'src/a.ts', + field: 'marker', + pattern: '9999', + }))); + + const body = renderComment(outcome); + + expect(body.length).toBeLessThanOrEqual(65536); + expect(body).toContain('Validation errors on changed records'); + expect(body).toContain('docs/adr/broken.md'); }); }); @@ -120,7 +214,26 @@ describe('renderComment status awareness (#39)', () => { test('only accepted records appear under the governing heading', async () => { const root = await seedMixed(); - const body = renderComment(await outcomeFor(root, ['src/api/thing.ts'])); + const outcome = await outcomeFor(root, ['src/api/thing.ts']); + outcome.markerScan = { + totalCandidates: 1, + limit: 3000, + counts: { scanned: 1, absent: 0, unreadable: 0, 'out-of-tree': 0, truncated: 0, skipped: 0 }, + absentPaths: [], + unreadablePaths: [], + outOfTreePaths: [], + truncatedPaths: [], + skippedPaths: [], + }; + outcome.findings.push({ + rule: 'dangling-marker', + severity: 'warn', + message: 'Source marker "@adr 9999" in src/api/thing.ts:1 does not resolve', + path: 'src/api/thing.ts', + field: 'marker', + pattern: '9999', + }); + const body = renderComment(outcome); const governingSection = body.slice( body.indexOf('### Decisions governing this change'), @@ -129,6 +242,9 @@ describe('renderComment status awareness (#39)', () => { expect(governingSection).toContain('**0001** — Accepted record'); expect(governingSection).not.toContain('0002'); expect(governingSection).not.toContain('0003'); + expect(body).toContain( + '- `src/api/thing.ts` — `Source marker "@adr 9999" in src/api/thing.ts:1 does not resolve`\n\n- **0001** — Accepted record', + ); }); test('proposals and history are labelled with their status under their own headings', async () => { diff --git a/site/src/content/docs/ci.mdx b/site/src/content/docs/ci.mdx index 409aca8b..fa97d529 100644 --- a/site/src/content/docs/ci.mdx +++ b/site/src/content/docs/ci.mdx @@ -43,6 +43,17 @@ The Action updates its existing comment on later pushes, so each pull request keeps one current governing-decisions comment. No additional token environment variable is required when using the default `GITHUB_TOKEN`. +The comment reports marker results separately from governing decisions. **Marker +scan health** lists changed files that could not be inspected (`absent`, +`unreadable`, `out-of-tree`, or skipped at the scan cap), so an empty marker +result is not mistaken for a healthy scan. **Marker claims not bound** lists +`@adr` claims that were read successfully but did not resolve in this corpus. +Both sections are bounded and advisory: they never affect the Action's exit +status, and changed-record validation errors retain priority if the comment +reaches GitHub's size limit. Keep the default checkout rooted at +`GITHUB_WORKSPACE`; if a workflow checks out elsewhere, marker health will +identify files the Action could not inspect. + `v0` is a moving major tag, and it now resolves to a `v0.11.0` build. Pin the immutable `v0.11.0` tag or a commit SHA for maximum reproducibility.