From 12f1bd9f9dbb106e9db013c7a7f86873b7ec4dca Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Thu, 1 Oct 2026 18:03:38 +0530 Subject: [PATCH 01/14] Add documentation proposal automation design and plan Record the design and implementation plan for issue #1105. Signed-off-by: dhruv8sh --- .../2026-10-01-docs-proposal-automation.md | 1500 +++++++++++++++++ ...6-10-01-docs-proposal-automation-design.md | 165 ++ 2 files changed, 1665 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-01-docs-proposal-automation.md create mode 100644 docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md diff --git a/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md b/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md new file mode 100644 index 000000000..a6d67b96d --- /dev/null +++ b/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md @@ -0,0 +1,1500 @@ +# Documentation Proposal Automation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** After a code merge to `main`, run GitHub Copilot CLI to propose +documentation edits, and open one pull request per merge whose hunks each carry +a review comment citing the merged source and a suggestion that reverts the +hunk. + +**Architecture:** A two-job workflow. `propose` (read-only token plus +`copilot-requests: write`) runs Copilot and uploads a patch, rationale, and +per-hunk evidence. `publish` (write token, no Copilot) applies the patch, +enforces a path allowlist, runs the docs gates, pushes `docs/auto/`, +opens or edits the pull request, and posts a review built deterministically +from the diff by a dependency-free Node script. + +**Tech Stack:** GitHub Actions, Copilot CLI (`@github/copilot@1.0.90`), Bash, +Node 24 (`node:test`, no npm dependencies), `gh` CLI, VitePress docs gates. + +**Spec:** `docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md` + +## Global Constraints + +- Copilot may only change `docs/guide/**` and `docs/index.md`; `publish.sh` + enforces it with `^docs/(guide/.+|index\.md)$`. +- Branch name: `docs/auto/`. +- One pull request per merge SHA; a closed or merged proposal is never + reopened or recreated. +- Empty diff: no pull request; an open proposal for that SHA is closed. +- Docs gates before any push: `npm ci`, `npm run lint`, `npm run format`, + `npm run build` in `docs/`. +- Copilot never holds a token with `contents: write` or `pull-requests: write`; + checkout in `propose` uses `persist-credentials: false`. +- Action references match the repository's existing tags (`actions/checkout@v4`, + `actions/setup-node@v4`, `actions/upload-artifact@v4`, + `actions/download-artifact@v4`). Workflow steps are single-line; logic lives + in `scripts/docs-proposal/`. +- Merge SHAs are full 40-character lowercase hex and must be reachable from + `origin/main`. +- Comments above code, never inline. Fictional `example.com` values only. +- Commit messages: sentence case, imperative, no prefixes; `git commit -S --signoff`. + +## File Map + +| File | Responsibility | +| -------------------------------------------------- | ------------------------------------------------------------ | +| `scripts/docs-proposal/hunks.mjs` | Parse a unified diff, list hunks with ids, build the review | +| `scripts/docs-proposal/hunks.test.mjs` | `node:test` coverage for `hunks.mjs` | +| `scripts/docs-proposal/lib.sh` | Shared Bash helpers: SHA validation, branch, allowlist, body | +| `scripts/docs-proposal/prompt.md` | Copilot instructions for proposing edits | +| `scripts/docs-proposal/evidence-prompt.md` | Copilot instructions for per-hunk evidence | +| `scripts/docs-proposal/propose.sh` | Job `propose` entry point | +| `scripts/docs-proposal/publish.sh` | Job `publish` entry point | +| `scripts/docs-proposal/test.sh` | Bash tests for `lib.sh` and `publish.sh` with stubbed `gh` | +| `.github/workflows/docs-proposal.yml` | The proposal workflow | +| `.github/workflows/format.yml` | Add a job that lints and tests the proposal scripts | +| `scripts/README.md`, `.gitignore`, the design spec | Prerequisites, ignored work dir, small spec corrections | + +--- + +### Task 1: Diff parser and hunk listing + +**Files:** + +- Create: `scripts/docs-proposal/hunks.mjs` +- Test: `scripts/docs-proposal/hunks.test.mjs` + +**Interfaces:** + +- Produces: + - `parseDiff(text: string): File[]` where + `File = { path: string, status: "modified"|"added"|"deleted", binary: boolean, hunks: Hunk[] }`, + `Hunk = { lines: Line[] }`, + `Line = { type: " "|"-"|"+", text: string, oldNo: number|null, newNo: number|null }`. + - `collectHunks(files: File[]): { id: string, path: string, status: string, hunk: Hunk }[]` + (binary files skipped; ids are `#<8 hex>`, with `-2`, `-3` … on collision). + - `listHunks(files: File[]): { id, path, status, removed: string[], added: string[] }[]`. + - CLI: `node hunks.mjs list < diff` prints `listHunks` JSON. + +- [ ] **Step 1: Write the failing tests** + +```js +// scripts/docs-proposal/hunks.test.mjs +import assert from 'node:assert/strict' +import { test } from 'node:test' + +import { collectHunks, listHunks, parseDiff } from './hunks.mjs' + +const MODIFIED = [ + 'diff --git a/docs/guide/cli.md b/docs/guide/cli.md', + 'index 1111111..2222222 100644', + '--- a/docs/guide/cli.md', + '+++ b/docs/guide/cli.md', + '@@ -10,4 +10,4 @@ heading', + ' keep one', + '-old flag', + '+new flag', + ' keep two', + '', +].join('\n') + +test('parseDiff numbers context, removed, and added lines', () => { + const [file] = parseDiff(MODIFIED) + assert.equal(file.path, 'docs/guide/cli.md', 'should take the b/ path') + assert.equal(file.status, 'modified', 'should default to modified') + assert.deepEqual( + file.hunks[0].lines, + [ + { type: ' ', text: 'keep one', oldNo: 10, newNo: 10 }, + { type: '-', text: 'old flag', oldNo: 11, newNo: null }, + { type: '+', text: 'new flag', oldNo: null, newNo: 11 }, + { type: ' ', text: 'keep two', oldNo: 12, newNo: 12 }, + ], + 'should track old and new line numbers' + ) +}) + +test('parseDiff records added, deleted, and binary files', () => { + const diff = [ + 'diff --git a/docs/guide/new.md b/docs/guide/new.md', + 'new file mode 100644', + '--- /dev/null', + '+++ b/docs/guide/new.md', + '@@ -0,0 +1 @@', + '+hello', + 'diff --git a/docs/guide/gone.md b/docs/guide/gone.md', + 'deleted file mode 100644', + '--- a/docs/guide/gone.md', + '+++ /dev/null', + '@@ -1 +0,0 @@', + '-bye', + 'diff --git a/docs/guide/logo.png b/docs/guide/logo.png', + 'Binary files a/docs/guide/logo.png and b/docs/guide/logo.png differ', + '', + ].join('\n') + const files = parseDiff(diff) + assert.deepEqual( + files.map(({ path, status, binary }) => ({ path, status, binary })), + [ + { path: 'docs/guide/new.md', status: 'added', binary: false }, + { path: 'docs/guide/gone.md', status: 'deleted', binary: false }, + { path: 'docs/guide/logo.png', status: 'modified', binary: true }, + ], + 'should classify each file' + ) + assert.deepEqual( + files[0].hunks[0].lines, + [{ type: '+', text: 'hello', oldNo: null, newNo: 1 }], + 'should start a new file at line 1' + ) +}) + +test('parseDiff skips no-newline markers', () => { + const diff = MODIFIED.replace( + ' keep two\n', + ' keep two\n\\ No newline at end of file\n' + ) + const [file] = parseDiff(diff) + assert.equal(file.hunks[0].lines.length, 4, 'should ignore the marker line') +}) + +test('collectHunks gives stable ids and skips binary files', () => { + const first = collectHunks(parseDiff(MODIFIED)) + const second = collectHunks( + parseDiff(MODIFIED.replace('@@ -10,4 +10,4', '@@ -20,4 +20,4')) + ) + assert.equal(first.length, 1, 'should collect the single hunk') + assert.match( + first[0].id, + /^docs\/guide\/cli\.md#[0-9a-f]{8}$/, + 'should use path#hash' + ) + assert.equal(first[0].id, second[0].id, 'should not depend on line numbers') +}) + +test('collectHunks suffixes duplicate ids', () => { + const twice = MODIFIED + MODIFIED.split('\n').slice(4).join('\n') + const ids = collectHunks(parseDiff(twice)).map((entry) => entry.id) + assert.equal(ids.length, 2, 'should collect both hunks') + assert.equal(ids[1], `${ids[0]}-2`, 'should suffix the repeated id') +}) + +test('listHunks exposes removed and added text', () => { + const [entry] = listHunks(parseDiff(MODIFIED)) + assert.deepEqual(entry.removed, ['old flag'], 'should list removed lines') + assert.deepEqual(entry.added, ['new flag'], 'should list added lines') + assert.equal(entry.status, 'modified', 'should carry the file status') +}) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `node --test scripts/docs-proposal/hunks.test.mjs` +Expected: FAIL with `Cannot find module` for `hunks.mjs`. + +- [ ] **Step 3: Write the implementation** + +```js +// scripts/docs-proposal/hunks.mjs +// Parses a unified documentation diff into hunks and builds the per-hunk +// review that cites Copilot evidence and offers a suggestion reverting each +// hunk. Dependency-free so it runs on any runner with Node. +import { createHash } from 'node:crypto' +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' + +const FILE_HEADER = /^diff --git a\/(.+) b\/(.+)$/ +const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/ + +export function parseDiff(text) { + const files = [] + let file = null + let hunk = null + let oldNo = 0 + let newNo = 0 + for (const raw of text.split('\n')) { + const fileHeader = FILE_HEADER.exec(raw) + if (fileHeader) { + file = { + path: fileHeader[2], + status: 'modified', + binary: false, + hunks: [], + } + files.push(file) + hunk = null + continue + } + if (!file) { + continue + } + const hunkHeader = HUNK_HEADER.exec(raw) + if (hunkHeader) { + oldNo = Number(hunkHeader[1]) + newNo = Number(hunkHeader[2]) + hunk = { lines: [] } + file.hunks.push(hunk) + continue + } + if (!hunk) { + if (raw.startsWith('new file mode')) { + file.status = 'added' + } else if (raw.startsWith('deleted file mode')) { + file.status = 'deleted' + } else if (raw.startsWith('Binary files ')) { + file.binary = true + } + continue + } + const type = raw[0] + const lineText = raw.slice(1) + if (type === ' ') { + hunk.lines.push({ type, text: lineText, oldNo: oldNo++, newNo: newNo++ }) + } else if (type === '-') { + hunk.lines.push({ type, text: lineText, oldNo: oldNo++, newNo: null }) + } else if (type === '+') { + hunk.lines.push({ type, text: lineText, oldNo: null, newNo: newNo++ }) + } + } + return files +} + +function hashHunk(path, hunk) { + const hash = createHash('sha256') + hash.update(path) + for (const line of hunk.lines) { + if (line.type !== ' ') { + hash.update(`\n${line.type}${line.text}`) + } + } + return `${path}#${hash.digest('hex').slice(0, 8)}` +} + +export function collectHunks(files) { + const seen = new Map() + const entries = [] + for (const file of files) { + if (file.binary) { + continue + } + for (const hunk of file.hunks) { + const base = hashHunk(file.path, hunk) + const count = (seen.get(base) ?? 0) + 1 + seen.set(base, count) + const id = count === 1 ? base : `${base}-${count}` + entries.push({ id, path: file.path, status: file.status, hunk }) + } + } + return entries +} + +export function listHunks(files) { + return collectHunks(files).map(({ id, path, status, hunk }) => ({ + id, + path, + status, + removed: hunk.lines + .filter((line) => line.type === '-') + .map((line) => line.text), + added: hunk.lines + .filter((line) => line.type === '+') + .map((line) => line.text), + })) +} + +function main([command]) { + const files = parseDiff(readFileSync(0, 'utf8')) + if (command === 'list') { + process.stdout.write(`${JSON.stringify(listHunks(files), null, 2)}\n`) + return + } + process.stderr.write( + 'usage: hunks.mjs list | review \n' + ) + process.exitCode = 2 +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main(process.argv.slice(2)) +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `node --test scripts/docs-proposal/hunks.test.mjs` +Expected: PASS, 6 tests. + +- [ ] **Step 5: Commit** + +```bash +git add scripts/docs-proposal/hunks.mjs scripts/docs-proposal/hunks.test.mjs +git commit -S --signoff -m "Add documentation diff hunk parser" +``` + +--- + +### Task 2: Per-hunk review builder + +**Files:** + +- Modify: `scripts/docs-proposal/hunks.mjs` +- Test: `scripts/docs-proposal/hunks.test.mjs` + +**Interfaces:** + +- Consumes: `parseDiff`, `collectHunks` from Task 1. +- Produces: + - `revertSuggestion(hunk: Hunk): { startLine: number, line: number, body: string[] } | null`. + - `indexEvidence(raw: unknown): Map`. + - `buildReview(files: File[], evidence: Map, commitId: string): { commit_id, event: "COMMENT", body: string, comments: object[] }`. + - CLI: `node hunks.mjs review < diff` prints the + review payload for `POST /repos/{owner}/{repo}/pulls/{number}/reviews`. A + missing or malformed evidence file is treated as no evidence. + +- [ ] **Step 1: Write the failing tests** (append to `hunks.test.mjs`, and + extend the import to + `import { buildReview, collectHunks, indexEvidence, listHunks, parseDiff, revertSuggestion } from "./hunks.mjs";`) + +`````js +function hunkOf(lines, oldStart = 1, newStart = 1) { + const diff = [ + 'diff --git a/docs/guide/a.md b/docs/guide/a.md', + '--- a/docs/guide/a.md', + '+++ b/docs/guide/a.md', + `@@ -${oldStart} +${newStart} @@`, + ...lines, + '', + ].join('\n') + return parseDiff(diff)[0].hunks[0] +} + +test('revertSuggestion restores replaced lines', () => { + const suggestion = revertSuggestion(hunkOf([' a', '-b', '+B', '+B2', ' c'])) + assert.deepEqual( + suggestion, + { startLine: 2, line: 3, body: ['b'] }, + 'should span the new lines and restore the old one' + ) +}) + +test('revertSuggestion deletes pure insertions', () => { + const suggestion = revertSuggestion(hunkOf([' a', '+new', ' c'])) + assert.deepEqual( + suggestion, + { startLine: 2, line: 2, body: [] }, + 'should suggest an empty body' + ) +}) + +test('revertSuggestion spans several change groups with their context', () => { + const suggestion = revertSuggestion(hunkOf(['-a', '+A', ' mid', '-c', '+C'])) + assert.deepEqual( + suggestion, + { startLine: 1, line: 3, body: ['a', 'mid', 'c'] }, + 'should keep the middle context line' + ) +}) + +test('revertSuggestion anchors pure deletions on the previous context line', () => { + const suggestion = revertSuggestion(hunkOf([' a', '-gone', ' c'], 4, 4)) + assert.deepEqual( + suggestion, + { startLine: 4, line: 4, body: ['a', 'gone'] }, + 'should restore the deleted line after its anchor' + ) +}) + +test('revertSuggestion anchors leading deletions on the next context line', () => { + const suggestion = revertSuggestion(hunkOf(['-gone', ' a'])) + assert.deepEqual( + suggestion, + { startLine: 1, line: 1, body: ['gone', 'a'] }, + 'should restore the deleted line before its anchor' + ) +}) + +test('revertSuggestion returns null without an anchor', () => { + assert.equal( + revertSuggestion(hunkOf(['-only'])), + null, + 'should not invent an anchor' + ) +}) + +test('indexEvidence keeps valid entries and sanitizes text', () => { + const evidence = indexEvidence({ + hunks: [ + { + id: 'x#1', + source: 'crates/a.rs:4', + reason: 'Adds `--flag`\nfor @team', + }, + { id: 7, source: 'ignored' }, + { id: 'x#2' }, + 'junk', + ], + }) + assert.deepEqual( + [...evidence], + [['x#1', { source: 'crates/a.rs:4', reason: 'Adds --flag for @​team' }]], + 'should drop invalid entries and strip backticks, newlines, and mentions' + ) + assert.equal(indexEvidence(null).size, 0, 'should accept missing evidence') +}) + +test('buildReview cites evidence and suggests reverts', () => { + const files = parseDiff( + [ + 'diff --git a/docs/guide/a.md b/docs/guide/a.md', + '--- a/docs/guide/a.md', + '+++ b/docs/guide/a.md', + '@@ -1,3 +1,3 @@', + ' a', + '-uses ```fence```', + '+B', + ' c', + '@@ -10 +10,2 @@', + ' j', + '+k', + '', + ].join('\n') + ) + const [first, second] = collectHunks(files) + const evidence = indexEvidence({ + hunks: [ + { id: first.id, source: 'crates/a.rs:9', reason: 'Renames the flag.' }, + ], + }) + const review = buildReview(files, evidence, 'abc123') + + assert.equal(review.commit_id, 'abc123', 'should pin the reviewed commit') + assert.equal(review.event, 'COMMENT', 'should not approve or request changes') + assert.equal(review.comments.length, 2, 'should comment on every hunk') + assert.deepEqual( + review.comments[0], + { + path: 'docs/guide/a.md', + line: 2, + side: 'RIGHT', + body: [ + '**Source:** `crates/a.rs:9` — Renames the flag.', + '', + 'Apply this suggestion to revert the hunk.', + '', + '````suggestion', + 'uses ```fence```', + '````', + ].join('\n'), + }, + 'should escalate the fence and omit start_line for one line' + ) + assert.match( + review.comments[1].body, + /none cited/, + 'should flag hunks without evidence' + ) + assert.match( + review.comments[1].body, + /```suggestion\n```$/, + 'should suggest deleting an insertion' + ) + assert.equal(second.id, collectHunks(files)[1].id, 'should be deterministic') +}) + +test('buildReview lists binary and unanchored changes in the body', () => { + const files = parseDiff( + [ + 'diff --git a/docs/guide/gone.md b/docs/guide/gone.md', + 'deleted file mode 100644', + '@@ -1 +0,0 @@', + '-bye', + 'diff --git a/docs/guide/logo.png b/docs/guide/logo.png', + 'Binary files a/docs/guide/logo.png and b/docs/guide/logo.png differ', + '', + ].join('\n') + ) + const review = buildReview(files, new Map(), 'abc123') + assert.equal(review.comments.length, 0, 'should not post line comments') + assert.match( + review.body, + /`docs\/guide\/gone\.md`: file deleted/, + 'should note the deletion' + ) + assert.match( + review.body, + /`docs\/guide\/logo\.png`: binary change/, + 'should note the binary file' + ) +}) + +test('buildReview adds start_line for multi-line spans', () => { + const files = parseDiff( + [ + 'diff --git a/docs/index.md b/docs/index.md', + '@@ -1 +1,2 @@', + '-a', + '+b', + '+c', + '', + ].join('\n') + ) + const [comment] = buildReview(files, new Map(), 'abc123').comments + assert.equal(comment.start_line, 1, 'should start at the first new line') + assert.equal( + comment.start_side, + 'RIGHT', + 'should anchor the start on the new side' + ) + assert.equal(comment.line, 2, 'should end at the last new line') +}) +````` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `node --test scripts/docs-proposal/hunks.test.mjs` +Expected: FAIL with `does not provide an export named 'buildReview'`. + +- [ ] **Step 3: Implement** — add to `hunks.mjs` after `listHunks`: + +```js +const MAX_EVIDENCE_LENGTH = 300 + +const REVIEW_INTRO = + 'Copilot evidence for each proposed hunk. Leave a hunk to accept it, or apply its suggestion to revert it. Verify every cited source before merging.' + +export function revertSuggestion(hunk) { + const { lines } = hunk + const first = lines.findIndex((line) => line.type !== ' ') + const last = lines.findLastIndex((line) => line.type !== ' ') + if (first === -1) { + return null + } + const span = lines.slice(first, last + 1) + const newSide = span.filter((line) => line.type !== '-') + const oldText = span + .filter((line) => line.type !== '+') + .map((line) => line.text) + if (newSide.length > 0) { + return { + startLine: newSide[0].newNo, + line: newSide.at(-1).newNo, + body: oldText, + } + } + const before = lines[first - 1] + if (before?.type === ' ') { + return { + startLine: before.newNo, + line: before.newNo, + body: [before.text, ...oldText], + } + } + const after = lines[last + 1] + if (after?.type === ' ') { + return { + startLine: after.newNo, + line: after.newNo, + body: [...oldText, after.text], + } + } + return null +} + +function sanitize(value) { + if (typeof value !== 'string') { + return '' + } + return value + .replace(/[`\r\n]+/g, ' ') + .replace(/@/g, '@​') + .replace(/\s+/g, ' ') + .trim() + .slice(0, MAX_EVIDENCE_LENGTH) +} + +export function indexEvidence(raw) { + const evidence = new Map() + const items = Array.isArray(raw?.hunks) ? raw.hunks : [] + for (const item of items) { + if (typeof item?.id !== 'string') { + continue + } + const source = sanitize(item.source) + const reason = sanitize(item.reason) + if (source || reason) { + evidence.set(item.id, { source, reason }) + } + } + return evidence +} + +function fenceFor(body) { + const longestRun = Math.max( + 0, + ...body.map((text) => + Math.max(0, ...(text.match(/`+/g) ?? []).map((run) => run.length)) + ) + ) + return '`'.repeat(Math.max(3, longestRun + 1)) +} + +function commentBody(cited, body) { + const source = cited?.source + ? `\`${cited.source}\`` + : 'none cited — verify this hunk against the merged code.' + const reason = cited?.reason ? ` — ${cited.reason}` : '' + const fence = fenceFor(body) + const suggestion = + body.length > 0 + ? `${fence}suggestion\n${body.join('\n')}\n${fence}` + : `${fence}suggestion\n${fence}` + return [ + `**Source:** ${source}${reason}`, + '', + 'Apply this suggestion to revert the hunk.', + '', + suggestion, + ].join('\n') +} + +export function buildReview(files, evidence, commitId) { + const comments = [] + const notes = [] + for (const file of files) { + if (file.binary) { + notes.push( + `- \`${file.path}\`: binary change — review the file directly.` + ) + } + } + for (const { id, path, status, hunk } of collectHunks(files)) { + const suggestion = revertSuggestion(hunk) + if (!suggestion) { + const reason = + status === 'deleted' ? 'file deleted' : 'no line to anchor a revert' + notes.push(`- \`${path}\`: ${reason} — revert it manually if needed.`) + continue + } + const comment = { + path, + line: suggestion.line, + side: 'RIGHT', + body: commentBody(evidence.get(id), suggestion.body), + } + if (suggestion.startLine !== suggestion.line) { + comment.start_line = suggestion.startLine + comment.start_side = 'RIGHT' + } + comments.push(comment) + } + const body = + notes.length > 0 + ? `${REVIEW_INTRO}\n\nChanges without a suggestion:\n\n${notes.join('\n')}` + : REVIEW_INTRO + return { commit_id: commitId, event: 'COMMENT', body, comments } +} + +function readEvidence(path) { + try { + return indexEvidence(JSON.parse(readFileSync(path, 'utf8'))) + } catch (error) { + process.stderr.write( + `Ignoring unreadable evidence at ${path}: ${error.message}\n` + ) + return new Map() + } +} +``` + +Note: the `comment` object key order in the test is `path, line, side, body`; +`deepEqual` ignores key order, so the construction order above is fine. + +Then extend `main`: + +```js +function main([command, evidencePath, commitId]) { + const files = parseDiff(readFileSync(0, 'utf8')) + if (command === 'list') { + process.stdout.write(`${JSON.stringify(listHunks(files), null, 2)}\n`) + return + } + if (command === 'review' && evidencePath && commitId) { + const review = buildReview(files, readEvidence(evidencePath), commitId) + process.stdout.write(`${JSON.stringify(review, null, 2)}\n`) + return + } + process.stderr.write( + 'usage: hunks.mjs list | review \n' + ) + process.exitCode = 2 +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `node --test scripts/docs-proposal/hunks.test.mjs` +Expected: PASS, 16 tests. + +- [ ] **Step 5: Commit** + +```bash +git add scripts/docs-proposal/hunks.mjs scripts/docs-proposal/hunks.test.mjs +git commit -S --signoff -m "Build per-hunk revert reviews for documentation proposals" +``` + +--- + +### Task 3: Shared helpers, prompts, and `propose.sh` + +**Files:** + +- Create: `scripts/docs-proposal/lib.sh`, `scripts/docs-proposal/prompt.md`, + `scripts/docs-proposal/evidence-prompt.md`, `scripts/docs-proposal/propose.sh` +- Create: `scripts/docs-proposal/test.sh` (lib tests only in this task) +- Modify: `.gitignore` (add `/.docs-proposal/`) + +**Interfaces:** + +- Consumes: `node hunks.mjs list` from Task 1. +- Produces (sourced by `publish.sh` in Task 4): + - `docs_proposal_require_sha `: exits 1 unless full hex SHA reachable from `origin/main`. + - `docs_proposal_branch ` → prints `docs/auto/`. + - `docs_proposal_disallowed_paths` (stdin: paths) → prints bad paths, returns 1 if any. + - `docs_proposal_pr_body ` → prints body. + - `propose.sh ` writes `/proposal.patch` (possibly empty), + `/rationale.md`, `/evidence.json`. + +- [ ] **Step 1: Write the failing lib tests** + +```bash +#!/usr/bin/env bash +# Tests the documentation proposal helpers and publish flow with stubbed gh +# and npm against a throwaway git repository. +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +failures=0 + +assert_eq() { + if [ "$1" != "$2" ]; then + printf 'FAIL: %s\n expected: %s\n actual: %s\n' "$3" "$2" "$1" + failures=$((failures + 1)) + fi +} + +assert_contains() { + if [[ "$1" != *"$2"* ]]; then + printf 'FAIL: %s\n missing: %s\n' "$3" "$2" + failures=$((failures + 1)) + fi +} + +sha=0123456789abcdef0123456789abcdef01234567 + +assert_eq "$(docs_proposal_branch "$sha")" "docs/auto/0123456789ab" "should derive the branch from the SHA" + +bad="$(printf 'docs/guide/a.md\ndocs/index.md\ndocs/superpowers/x.md\nREADME.md\n' | docs_proposal_disallowed_paths || true)" +assert_eq "$bad" $'docs/superpowers/x.md\nREADME.md' "should print only paths outside the allowlist" + +if printf 'docs/guide/a.md\ndocs/guide/integrations/b.md\n' | docs_proposal_disallowed_paths >/dev/null; then + status=0 +else + status=1 +fi +assert_eq "$status" 0 "should accept guide and nested guide paths" + +rationale="$(mktemp)" +printf -- '- docs/guide/cli.md: documents the new flag.\n' > "$rationale" +body="$(docs_proposal_pr_body "$sha" "Add example flag" 42 "$rationale")" +assert_contains "$body" "" "should embed the SHA marker" +assert_contains "$body" "merged change $sha (#42): Add example flag" "should link the merge and its PR" +assert_contains "$body" "documents the new flag" "should include the rationale" +: > "$rationale" +body="$(docs_proposal_pr_body "$sha" "Add example flag" "" "$rationale")" +assert_contains "$body" "merged change $sha: Add example flag" "should omit a missing origin PR" +assert_contains "$body" "did not provide a rationale" "should note an empty rationale" +rm -f "$rationale" + +if (docs_proposal_require_sha "not-a-sha") 2>/dev/null; then + status=0 +else + status=1 +fi +assert_eq "$status" 1 "should reject a malformed SHA" + +# PUBLISH_TESTS + +if [ "$failures" -gt 0 ]; then + printf '%d failure(s)\n' "$failures" + exit 1 +fi +printf 'All documentation proposal script tests passed.\n' +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `chmod +x scripts/docs-proposal/test.sh && scripts/docs-proposal/test.sh` +Expected: FAIL — `lib.sh: No such file or directory`. + +- [ ] **Step 3: Write `lib.sh`** + +```bash +# shellcheck shell=bash +# Shared helpers for the documentation proposal scripts. Source this file; +# do not execute it. + +DOCS_PROPOSAL_ALLOWED_RE='^docs/(guide/.+|index\.md)$' +DOCS_PROPOSAL_MAX_RATIONALE_BYTES=20000 + +# Exits unless the argument is a full commit SHA reachable from origin/main. +docs_proposal_require_sha() { + local sha="$1" + if ! [[ "$sha" =~ ^[0-9a-f]{40}$ ]]; then + printf '::error::Expected a full 40-character commit SHA, got %s\n' "$sha" >&2 + exit 1 + fi + if ! git merge-base --is-ancestor "$sha" origin/main 2>/dev/null; then + printf '::error::Commit %s is not reachable from origin/main\n' "$sha" >&2 + exit 1 + fi +} + +docs_proposal_branch() { + printf 'docs/auto/%s\n' "${1:0:12}" +} + +# Reads changed paths on stdin, prints those outside the allowlist, and +# returns 1 when any were found. +docs_proposal_disallowed_paths() { + local path + local found=0 + while IFS= read -r path; do + if [ -n "$path" ] && ! [[ "$path" =~ $DOCS_PROPOSAL_ALLOWED_RE ]]; then + printf '%s\n' "$path" + found=1 + fi + done + return "$found" +} + +docs_proposal_pr_body() { + local sha="$1" + local subject="$2" + local origin_pr="$3" + local rationale="$4" + local origin="" + if [ -n "$origin_pr" ]; then + origin=" (#$origin_pr)" + fi + printf '\n' "$sha" + printf 'Proposed documentation update for merged change %s%s: %s\n\n' "$sha" "$origin" "$subject" + printf 'Generated by GitHub Copilot CLI in the docs-proposal workflow. A maintainer must verify every hunk against the merged code before merging. Each hunk has a review comment citing its source and a suggestion that reverts it.\n\n' + printf '## Rationale\n\n' + if [ -s "$rationale" ]; then + head -c "$DOCS_PROPOSAL_MAX_RATIONALE_BYTES" "$rationale" + printf '\n' + else + printf '_Copilot did not provide a rationale._\n' + fi +} +``` + +- [ ] **Step 4: Run lib tests to verify they pass** + +Run: `scripts/docs-proposal/test.sh` +Expected: `All documentation proposal script tests passed.` + +- [ ] **Step 5: Write the prompts** + +`scripts/docs-proposal/prompt.md`: + +```markdown +# Documentation proposal + +You are updating the public Trusted Server documentation after a commit merged +to `main`. The merged commit and the table mapping implementation sources to +documentation pages appear at the end of this file. + +## Task + +1. Inspect the merged change with `git show --stat ` and + `git show `. +2. Decide whether it changes behavior that `docs/index.md` or a page under + `docs/guide/` describes: configuration fields and defaults, routes, adapter + support, integrations, auction behavior, CLI commands and flags, browser + bundles, or verification commands. Use the Sources of truth table to find + the affected pages. +3. If no reader-facing documentation is affected, change nothing and write + `.docs-proposal/rationale.md` containing one sentence explaining why. +4. Otherwise edit only the affected pages, then write + `.docs-proposal/rationale.md` with one bullet per edited page naming the + page, what changed, and the merged source file that justifies it. + +## Rules + +- Describe behavior the merged code ships. Do not document intent, roadmap + items, or unmerged work. +- Read the merged source before writing. Every statement you add must be + verifiable in the code at this commit. +- Make the smallest edit that keeps the page accurate. Do not restyle, + reorder, or rewrite unaffected text. +- Use only fictional values: `example.com` domains and placeholder secrets. + Never copy real domains, customer names, credentials, or service IDs. +- Keep existing links and anchors working, and link to other pages with + relative paths. +- Do not edit files outside `docs/guide/`, `docs/index.md`, and + `.docs-proposal/`. Do not commit, push, or create branches. +``` + +`scripts/docs-proposal/evidence-prompt.md`: + +````markdown +# Documentation proposal evidence + +The hunks at the end of this file are proposed edits to the Trusted Server +documentation for the merged commit named there. For each hunk, find the merged +source that justifies it. + +Write `.docs-proposal/evidence.json` with exactly this shape, and change no +other file: + +```json +{ + "hunks": [ + { "id": "", "source": ":", "reason": "" } + ] +} +``` + +- Copy `id` exactly from the hunk list. +- `source` is a repository-relative path and line at the merged commit that + shows the documented behavior. +- `reason` is one sentence a reviewer can check against that source. +- If no source justifies a hunk, omit it. The reviewer will be told the hunk + has no cited source. +```` + +- [ ] **Step 6: Write `propose.sh`** + +````bash +#!/usr/bin/env bash +# Asks Copilot CLI to propose documentation edits for one merged commit. +# +# Usage: scripts/docs-proposal/propose.sh +# Writes proposal.patch (empty when nothing changes), rationale.md, and +# evidence.json to , which must be inside the repository. +set -euo pipefail + +sha="$1" +work_dir="$2" +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +root="$(git rev-parse --show-toplevel)" +spec="$root/docs/superpowers/specs/2026-08-19-documentation-refresh-design.md" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +cd "$root" +docs_proposal_require_sha "$sha" +mkdir -p "$work_dir" +work_dir="$(cd "$work_dir" && pwd)" +relative_dir="${work_dir#"$root"/}" + +run_copilot() { + copilot -p "Read $relative_dir/$1 and follow its instructions exactly." \ + --no-ask-user \ + --allow-tool=write \ + --allow-tool='shell(git show:*)' \ + --allow-tool='shell(git diff:*)' \ + --allow-tool='shell(git log:*)' \ + --allow-tool='shell(git grep:*)' \ + --deny-tool='shell(git push)' \ + --deny-tool='shell(git commit)' +} + +{ + cat "$here/prompt.md" + printf '\n## Merged change\n\n- Commit: %s\n- Subject: %s\n\n' "$sha" "$(git log -1 --format=%s "$sha")" + printf '## Sources of truth\n\n' + awk '/^## Sources of truth/ { found = 1; next } /^## / { found = 0 } found' "$spec" +} > "$work_dir/prompt.md" +run_copilot prompt.md + +# Normalize Copilot's edits before hunks are identified so ids stay valid +# after the publish job's format check. +(cd docs && npm ci && npm run format:write) + +git add -A -- docs +git diff --cached --no-renames --binary "$sha" -- docs > "$work_dir/proposal.patch" +touch "$work_dir/rationale.md" +if [ ! -s "$work_dir/proposal.patch" ]; then + printf '{}\n' > "$work_dir/evidence.json" + printf 'No documentation changes proposed for %s.\n' "$sha" + exit 0 +fi + +{ + cat "$here/evidence-prompt.md" + printf '\n## Merged change\n\n- Commit: %s\n\n## Hunks\n\n```json\n' "$sha" + git diff --cached --no-renames "$sha" -- docs | node "$here/hunks.mjs" list + printf '```\n' +} > "$work_dir/evidence-prompt.md" +run_copilot evidence-prompt.md +if [ ! -f "$work_dir/evidence.json" ]; then + printf '{}\n' > "$work_dir/evidence.json" +fi +```` + +- [ ] **Step 7: Ignore the work dir, lint, and commit** + +Append `/.docs-proposal/` to `.gitignore` under a comment +`# Documentation proposal workflow scratch space`. + +Run: `chmod +x scripts/docs-proposal/propose.sh && shellcheck scripts/docs-proposal/*.sh` (skip locally if +`shellcheck` is not installed; CI runs it in Task 5). +Run: `scripts/docs-proposal/test.sh` — Expected: pass. + +```bash +git add .gitignore scripts/docs-proposal/lib.sh scripts/docs-proposal/prompt.md scripts/docs-proposal/evidence-prompt.md scripts/docs-proposal/propose.sh scripts/docs-proposal/test.sh +git commit -S --signoff -m "Add Copilot documentation proposal script" +``` + +--- + +### Task 4: `publish.sh` with stubbed integration tests + +**Files:** + +- Create: `scripts/docs-proposal/publish.sh` +- Modify: `scripts/docs-proposal/test.sh` (replace the `# PUBLISH_TESTS` line) + +**Interfaces:** + +- Consumes: `lib.sh` (Task 3), `node hunks.mjs review` (Task 2), files from `propose.sh`. +- Produces: `publish.sh `; env `GH_TOKEN`. `gh` calls used: + - `gh pr list --head --state all --json number,state --jq '.[0] | select(.) | "\(.number) \(.state)"'` + - `gh pr close --comment --delete-branch` + - `gh api repos/{owner}/{repo}/commits//pulls --jq '.[0].number // empty'` + - `gh pr create --base main --head --title --body-file ` (prints URL) + - `gh pr edit --title --body-file ` + - `gh api --method POST repos/{owner}/{repo}/pulls//reviews --input ` + +- [ ] **Step 1: Write the failing publish tests** — replace `# PUBLISH_TESTS` + in `test.sh` with: + +```bash +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 +export GIT_AUTHOR_NAME=Example GIT_AUTHOR_EMAIL=dev@example.com +export GIT_COMMITTER_NAME=Example GIT_COMMITTER_EMAIL=dev@example.com + +mkdir -p "$tmp/bin" +cat > "$tmp/bin/gh" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$STUB_LOG" +case "$1 $2" in + "pr list") printf '%s' "${STUB_PR:-}" ;; + "pr create") printf 'https://github.com/example/repo/pull/7\n' ;; + "api repos/{owner}/{repo}/commits/"*) printf '%s' "${STUB_ORIGIN_PR:-}" ;; + "api --method") + for arg in "$@"; do last="$arg"; done + cp "$last" "$STUB_REVIEW" + ;; +esac +STUB +printf '#!/usr/bin/env bash\nprintf "npm %%s\\n" "$*" >> "$STUB_LOG"\n' > "$tmp/bin/npm" +chmod +x "$tmp/bin/gh" "$tmp/bin/npm" +export PATH="$tmp/bin:$PATH" STUB_LOG="$tmp/gh.log" STUB_REVIEW="$tmp/review.json" + +git init -q --bare -b main "$tmp/origin.git" +git clone -q "$tmp/origin.git" "$tmp/work" 2>/dev/null +cd "$tmp/work" +mkdir -p docs/guide +printf 'line one\nline two\nline three\n' > docs/guide/cli.md +git add docs && git commit -q -m "Seed docs" && git push -q origin HEAD:main +merge_sha="$(git rev-parse HEAD)" +branch="$(docs_proposal_branch "$merge_sha")" + +# Each publish run leaves HEAD on its proposal commit, so every patch starts +# from a clean checkout of the merge commit. +make_patch() { + git reset -q --hard "$merge_sha" && git clean -qfd + rm -rf "$tmp/proposal" && mkdir -p "$tmp/proposal" + if [ -n "$1" ]; then + mkdir -p "$(dirname "$1")" && printf 'proposed\n' >> "$1" + git add -A && git diff --cached --binary "$merge_sha" > "$tmp/proposal/proposal.patch" + git reset -q --hard "$merge_sha" && git clean -qfd + else + : > "$tmp/proposal/proposal.patch" + fi + printf -- '- docs/guide/cli.md: example rationale.\n' > "$tmp/proposal/rationale.md" + printf '{}\n' > "$tmp/proposal/evidence.json" +} + +run_publish() { + : > "$STUB_LOG" + rm -f "$STUB_REVIEW" + if "$here/publish.sh" "$merge_sha" "$tmp/proposal" > "$tmp/out.log" 2>&1; then + printf '0' + else + printf '1' + fi +} + +remote_has_branch() { + if git ls-remote --exit-code -q origin "refs/heads/$branch" >/dev/null; then printf 'yes'; else printf 'no'; fi +} + +make_patch "" +assert_eq "$(STUB_PR="" run_publish)" 0 "should succeed on an empty proposal" +assert_eq "$(grep -c 'pr create\|pr close' "$STUB_LOG" || true)" 0 "should not open or close a PR for an empty diff" + +make_patch "" +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed when closing a stale proposal" +assert_contains "$(cat "$STUB_LOG")" "pr close 7" "should close the open proposal for an empty diff" + +make_patch docs/superpowers/notes.md +assert_eq "$(STUB_PR="" run_publish)" 1 "should fail for a disallowed path" +assert_contains "$(cat "$tmp/out.log")" "docs/superpowers/notes.md" "should name the disallowed path" +assert_eq "$(remote_has_branch)" no "should not push a disallowed proposal" + +make_patch docs/guide/cli.md +assert_eq "$(STUB_PR="" STUB_ORIGIN_PR=42 run_publish)" 0 "should publish a valid proposal" +assert_eq "$(remote_has_branch)" yes "should push the proposal branch" +assert_contains "$(cat "$STUB_LOG")" "pr create --base main --head $branch" "should open a PR from the proposal branch" +assert_contains "$(cat "$STUB_LOG")" "npm run build" "should run the docs build" +assert_contains "$(cat "$STUB_REVIEW")" '"path": "docs/guide/cli.md"' "should post a review for the hunk" + +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed on an unchanged retry" +assert_eq "$(grep -c 'pr edit\|pr create\|reviews' "$STUB_LOG" || true)" 0 "should not re-post an unchanged proposal" + +make_patch docs/index.md +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should update a changed proposal" +assert_contains "$(cat "$STUB_LOG")" "pr edit 7" "should edit the existing PR" +assert_contains "$(cat "$STUB_REVIEW")" '"path": "docs/index.md"' "should review the new hunk" + +assert_eq "$(STUB_PR="7 CLOSED" run_publish)" 0 "should succeed for a closed proposal" +assert_eq "$(grep -c 'pr edit\|pr create' "$STUB_LOG" || true)" 0 "should never reopen or recreate a closed proposal" + +cd "$here" +``` + +Note: `make_patch docs/index.md` creates `docs/index.md` (an added file) so the +"changed proposal" case diffs a different tree than the pushed branch. + +- [ ] **Step 2: Run to verify it fails** + +Run: `scripts/docs-proposal/test.sh` +Expected: FAIL lines such as `should succeed on an empty proposal` because +`publish.sh` does not exist. + +- [ ] **Step 3: Write `publish.sh`** + +```bash +#!/usr/bin/env bash +# Validates a documentation proposal and opens or updates its pull request. +# +# Usage: scripts/docs-proposal/publish.sh +# holds proposal.patch, rationale.md, and evidence.json from +# propose.sh. Requires GH_TOKEN with contents and pull-requests write access. +set -euo pipefail + +sha="$1" +work_dir="$(cd "$2" && pwd)" +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +root="$(git rev-parse --show-toplevel)" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +cd "$root" +docs_proposal_require_sha "$sha" +branch="$(docs_proposal_branch "$sha")" + +existing="$(gh pr list --head "$branch" --state all --json number,state --jq '.[0] | select(.) | "\(.number) \(.state)"')" +pr_number="${existing%% *}" +pr_state="${existing#* }" +if [ -n "$existing" ] && [ "$pr_state" != "OPEN" ]; then + printf 'Proposal #%s for %s is %s; not reopening it.\n' "$pr_number" "$sha" "$pr_state" + exit 0 +fi + +if [ ! -s "$work_dir/proposal.patch" ]; then + printf 'No documentation changes proposed for %s.\n' "$sha" + if [ -n "$existing" ]; then + gh pr close "$pr_number" --delete-branch \ + --comment "A rerun for $sha proposed no documentation changes." + fi + exit 0 +fi + +git switch --quiet --detach "$sha" +git apply --index "$work_dir/proposal.patch" +if ! disallowed="$(git diff --cached --name-only --no-renames "$sha" | docs_proposal_disallowed_paths)"; then + printf '::error::Proposal changes paths outside docs/guide/** and docs/index.md:\n%s\n' "$disallowed" + exit 1 +fi + +(cd docs && npm ci && npm run lint && npm run format && npm run build) + +git -c user.name='github-actions[bot]' \ + -c user.email='41898282+github-actions[bot]@users.noreply.github.com' \ + commit --quiet --no-verify \ + -m "Propose documentation updates for ${sha:0:12}" \ + -m "Generated by the docs-proposal workflow from $sha." +head_sha="$(git rev-parse HEAD)" + +remote_tree="" +if git fetch --quiet origin "refs/heads/$branch" 2>/dev/null; then + remote_tree="$(git rev-parse 'FETCH_HEAD^{tree}')" +fi +if [ -n "$existing" ] && [ "$remote_tree" = "$(git rev-parse 'HEAD^{tree}')" ]; then + printf 'Proposal #%s for %s is unchanged.\n' "$pr_number" "$sha" + exit 0 +fi +git push --quiet --force origin "HEAD:refs/heads/$branch" + +subject="$(git log -1 --format=%s "$sha")" +origin_pr="$(gh api "repos/{owner}/{repo}/commits/$sha/pulls" --jq '.[0].number // empty' || true)" +title="$(printf 'Update documentation for %s' "$subject" | cut -c1-200)" +docs_proposal_pr_body "$sha" "$subject" "$origin_pr" "$work_dir/rationale.md" > "$work_dir/pr-body.md" +if [ -n "$existing" ]; then + gh pr edit "$pr_number" --title "$title" --body-file "$work_dir/pr-body.md" +else + pr_url="$(gh pr create --base main --head "$branch" --title "$title" --body-file "$work_dir/pr-body.md")" + pr_number="${pr_url##*/}" +fi + +git diff --no-renames "$sha" "$head_sha" -- docs \ + | node "$here/hunks.mjs" review "$work_dir/evidence.json" "$head_sha" > "$work_dir/review.json" +gh api --method POST "repos/{owner}/{repo}/pulls/$pr_number/reviews" --input "$work_dir/review.json" > /dev/null +printf 'Published proposal #%s for %s.\n' "$pr_number" "$sha" +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `chmod +x scripts/docs-proposal/publish.sh && scripts/docs-proposal/test.sh` +Expected: `All documentation proposal script tests passed.` + +- [ ] **Step 5: Commit** + +```bash +git add scripts/docs-proposal/publish.sh scripts/docs-proposal/test.sh +git commit -S --signoff -m "Publish documentation proposals as per-merge pull requests" +``` + +--- + +### Task 5: Workflow, CI coverage, and operator docs + +**Files:** + +- Create: `.github/workflows/docs-proposal.yml` +- Modify: `.github/workflows/format.yml` (new job), `scripts/README.md` (table rows + prerequisites), + `docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md` (corrections below) + +**Interfaces:** + +- Consumes: `propose.sh .docs-proposal`, `publish.sh .docs-proposal`. + +- [ ] **Step 1: Write `docs-proposal.yml`** + +```yaml +name: Documentation proposal + +# Proposes documentation edits with Copilot CLI after code merges to main and +# opens one reviewable pull request per merge. See +# docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md. +on: + push: + branches: [main] + paths: + - 'crates/**' + - 'trusted-server.example.toml' + - 'edgezero.toml' + - 'fastly.toml' + - '.cargo/config.toml' + - 'scripts/**' + - '!scripts/docs-proposal/**' + workflow_dispatch: + inputs: + sha: + description: Full SHA of a commit on main to propose documentation for + required: true + type: string + +permissions: {} + +concurrency: + group: docs-proposal + cancel-in-progress: false + +env: + MERGE_SHA: ${{ inputs.sha || github.sha }} + +jobs: + propose: + runs-on: ubuntu-latest + permissions: + contents: read + copilot-requests: write + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ env.MERGE_SHA }} + fetch-depth: 0 + persist-credentials: false + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + cache: 'npm' + cache-dependency-path: docs/package-lock.json + + - name: Install Copilot CLI + run: npm install -g @github/copilot@1.0.90 + + - name: Propose documentation edits + env: + GITHUB_TOKEN: ${{ github.token }} + run: scripts/docs-proposal/propose.sh "$MERGE_SHA" .docs-proposal + + - name: Upload proposal + uses: actions/upload-artifact@v4 + with: + name: docs-proposal + path: .docs-proposal/ + include-hidden-files: true + if-no-files-found: error + + publish: + needs: propose + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ env.MERGE_SHA }} + fetch-depth: 0 + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + cache: 'npm' + cache-dependency-path: docs/package-lock.json + + - name: Download proposal + uses: actions/download-artifact@v4 + with: + name: docs-proposal + path: .docs-proposal + + - name: Publish proposal pull request + env: + GH_TOKEN: ${{ github.token }} + run: scripts/docs-proposal/publish.sh "$MERGE_SHA" .docs-proposal +``` + +- [ ] **Step 2: Add the CI job to `format.yml`** (after `format-docs`) + +```yaml +docs-proposal-scripts: + name: docs-proposal scripts + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT + shell: bash + + - name: Use Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + + - name: Run shellcheck + run: shellcheck scripts/docs-proposal/*.sh + + - name: Test hunk review builder + run: node --test scripts/docs-proposal/hunks.test.mjs + + - name: Test proposal scripts + run: scripts/docs-proposal/test.sh +``` + +- [ ] **Step 3: Document in `scripts/README.md`** — add table rows: + +```markdown +| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, and evidence into the work dir. Leaves the docs edits staged. | +| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Applies the patch, runs docs gates, force-pushes `docs/auto/`, opens or edits its PR, and posts a per-hunk review. Never writes `main`. | +| `docs-proposal/test.sh` | Git, Node | Tests the helpers and publish flow against a temporary repository with stubbed `gh` and `npm`; removes it on exit. | +``` + +Then append after the table: + +```markdown +The `Documentation proposal` workflow runs both proposal scripts after code +merges to `main`. It needs the organization Copilot policy "Allow use of +Copilot CLI billed to the organization" and the repository setting "Allow +GitHub Actions to create and approve pull requests". Pull requests opened with +`GITHUB_TOKEN` do not trigger other workflows, so the publish job runs the docs +gates itself; push to the proposal branch to run regular CI. Retry a merge with +the workflow's manual dispatch and its full commit SHA. +``` + +Run `cd docs && npx prettier --write ../scripts/README.md` only if the docs +Prettier config covers it; otherwise align the table columns by hand. + +- [ ] **Step 4: Correct the spec** in + `docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md`: + +- Per-hunk table, last row: anchor "Listed in the review body", suggestion "None". +- Trigger: add that `scripts/docs-proposal/**` is excluded and that dispatch + requires a full SHA reachable from `origin/main`. +- `publish` step 1: a closed or merged proposal for the SHA is left alone. +- Testing: CI runs the scripts' tests in a new `format.yml` job. + +Run: `cd docs && npx prettier --write superpowers/specs/2026-10-01-docs-proposal-automation-design.md superpowers/plans/2026-10-01-docs-proposal-automation.md` + +- [ ] **Step 5: Validate and commit** + +Run: `actionlint .github/workflows/docs-proposal.yml .github/workflows/format.yml` if +installed, else `python3 -c "import yaml,sys; [yaml.safe_load(open(f)) for f in sys.argv[1:]]" .github/workflows/docs-proposal.yml .github/workflows/format.yml`. +Expected: no errors. + +```bash +git add .github/workflows/docs-proposal.yml .github/workflows/format.yml scripts/README.md docs/superpowers +git commit -S --signoff -m "Add documentation proposal workflow" -m "Closes #1105." +``` + +--- + +### Task 6: Verification + +- [ ] `node --test scripts/docs-proposal/hunks.test.mjs` — all pass. +- [ ] `scripts/docs-proposal/test.sh` — all pass. +- [ ] `cd docs && npm run lint && npm run format && npm run build` — pass. +- [ ] `cargo fmt --all -- --check` — unaffected, pass. +- [ ] After merge: dispatch `Documentation proposal` with a recent merge SHA + that changed a CLI flag; confirm a `docs/auto/` PR with per-hunk + review comments, and that a second dispatch with the same SHA does not + re-post. diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md new file mode 100644 index 000000000..545adb58d --- /dev/null +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -0,0 +1,165 @@ +# Documentation Proposal Automation Design + +- Status: proposed +- Design date: 2026-10-01 +- Issue: #1105 +- Target branch: `main` + +## Purpose + +When a merge to `main` changes behavior that the public documentation +describes, a reviewable pull request with the proposed documentation diff +should appear automatically. Today `deploy-docs.yml` only builds and deploys +the VitePress site after documentation changes land; nothing proposes edits +after a code merge. + +This design adds a proposal path that never writes to `main`. It is a +deliberate, scoped addition to the documentation refresh design +(`2026-08-19-documentation-refresh-design.md`), which excluded automated +tracked-file write paths. The exclusion still holds for `main`: automation +only ever writes to a dedicated proposal branch and a pull request. + +## Decisions + +| Question | Decision | +| -------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| How is prose produced? | GitHub Copilot CLI, run headless in GitHub Actions with the workflow `GITHUB_TOKEN` (`copilot-requests: write`), billed to the org. | +| Which code changes are examined? | Pushes to `main` touching `crates/**`, `trusted-server.example.toml`, `edgezero.toml`, `fastly.toml`, `.cargo/config.toml`, `scripts/**`. | +| Which pages may change? | `docs/guide/**` and `docs/index.md` only — the public site validated by the docs gates. | +| How is prose checked against merged code? | Copilot cites the merged source for every hunk; a per-hunk review exposes the evidence and a one-click revert; a human approves. | +| How do sequential or overlapping merges map? | One pull request per merge commit on branch `docs/auto/`; runs are serialized. | +| What happens on an empty diff? | No pull request. An existing proposal pull request for that SHA is closed. | +| What prevents duplicates on retry? | The branch name is derived from the merge SHA; a retry force-updates the same branch and edits the same pull request. | + +## Workflow shape + +`.github/workflows/docs-proposal.yml` has two jobs. Copilot never holds a +token that can push or open pull requests. + +### Trigger + +- `push` to `main`, filtered to the source paths above. `docs/**` and + `scripts/docs-proposal/**` are not included, so a merged documentation + proposal or a change to this tooling never retriggers the workflow. +- `workflow_dispatch` with a required `sha` input, for retries and for the + first end-to-end validation. Both scripts require a full 40-character SHA + reachable from `origin/main`. +- `concurrency: { group: docs-proposal, cancel-in-progress: false }` so + sequential merges are processed one at a time, in order. + +### Job `propose` + +Permissions: `contents: read`, `copilot-requests: write`. + +1. Check out the merge commit with full history. +2. Install Copilot CLI (`@github/copilot`) at a pinned version. +3. `scripts/docs-proposal/propose.sh .docs-proposal`: + 1. Renders `.docs-proposal/prompt.md` from `scripts/docs-proposal/prompt.md`, + the merge SHA, its subject, and the Sources-of-truth table from the + refresh design. + 2. Runs `copilot -p` with `--no-ask-user`, `--allow-tool=write`, and + `git show`, `git diff`, `git log`, and `git grep` shell tools; `git push` + and `git commit` are denied, and file reads need no permission. Copilot + edits documentation only when the merged change alters reader-facing + behavior, and writes `.docs-proposal/rationale.md`. + 3. Runs `npm ci` and `npm run format:write` in `docs/` so Prettier + normalizes the edits before any hunk is identified. + 4. Writes `proposal.patch` (`git diff --cached --binary -- docs`). An + empty patch ends the job here. + 5. Runs `node scripts/docs-proposal/hunks.mjs list` to enumerate the hunks. + A hunk id is the changed path plus a short hash of the hunk's removed + and added lines, so it is stable across reruns. A second Copilot run + writes `evidence.json`: for each hunk id, the merged source `path:line` + that justifies it and a one-sentence reason. +4. Upload the `.docs-proposal/` directory as an artifact. + +### Job `publish` + +Permissions: `contents: write`, `pull-requests: write`. Copilot does not run. + +`scripts/docs-proposal/publish.sh .docs-proposal`: + +1. If a closed or merged pull request already exists for + `docs/auto/`, leave it alone and exit successfully: a + maintainer's decision is never reopened or recreated. +2. Empty diff: close any open pull request for that branch with a comment, + then exit successfully. Otherwise apply `proposal.patch` to the merge + commit. +3. Reject the proposal if any changed path is outside `docs/guide/**` or + `docs/index.md`. +4. In `docs/`: `npm ci`, `npm run lint`, `npm run format`, `npm run build`. + Any failure fails the job; no pull request is offered. +5. Commit as `github-actions[bot]` on `docs/auto/`. If the remote + branch already exists and its tree equals the new tree, stop: the proposal + is unchanged and nothing is re-posted. Otherwise force-push. +6. Find the originating pull request with + `gh api repos/{owner}/{repo}/commits//pulls`. Create the pull request, + or edit the existing one, with a body that links the merge commit and the + originating pull request, includes `rationale.md`, and states that a human + must verify the prose against the code before merging. +7. `node scripts/docs-proposal/hunks.mjs review` builds review comments and + posts one `COMMENT` review through `gh api`. + +## Per-hunk review + +Each hunk in the pull request receives one review comment containing the +Copilot evidence for that hunk and a `suggestion` block that **reverts** the +hunk. Reviewers accept the prose by leaving it, and reject a hunk by applying +its suggestion. Suggestions are built from the diff, never from model output, +so the revert is exact: + +| Hunk kind | Comment anchor | Suggestion body | +| ------------------------- | --------------------------------------- | ---------------------------------------- | +| Changed or inserted lines | The new lines (right side) | The original lines (empty for insertion) | +| Pure deletion | Adjacent context line (prefer previous) | That context line plus deleted lines | +| Deleted file or binary | Listed in the review body | None | + +Hunks without evidence still receive a revert suggestion, marked "none +cited". Copilot output that names unknown hunk ids is ignored, and evidence +text is stripped of backticks, newlines, and `@` mentions before it is posted. + +GitHub only allows suggestions on lines inside the pull request diff, which is +why the edits are commits and suggestions are reverts. + +## Repository prerequisites + +Documented in `scripts/README.md`: + +- The organization Copilot policy "Allow use of Copilot CLI billed to the + organization" is enabled. +- Repository setting "Allow GitHub Actions to create and approve pull + requests" is enabled. +- Pull requests opened with `GITHUB_TOKEN` do not trigger other workflows. + The `publish` job runs the docs gates itself; maintainers re-run regular CI + by pushing to the branch if needed. + +## Security + +- Copilot runs with a read-only `contents` token and an allowlist of read-only + shell tools; it has no network shell, no push, and no pull request + permissions. +- Input is code already merged to `main`, so no untrusted fork content reaches + the agent. +- The path allowlist is enforced by `publish.sh`, not by the prompt. +- Proposed prose must use fictional `example.com` values, per `CLAUDE.md`; + the reviewer verifies this before merging. + +## Testing + +- `node --test scripts/docs-proposal/hunks.test.mjs` covers hunk listing, + every row of the per-hunk table, unknown evidence ids, and the review payload. +- `scripts/docs-proposal/test.sh` stubs `gh` and `npm` and pushes to a + temporary bare repository to cover the empty diff, closing a stale + proposal, a disallowed path, first publish, an unchanged retry, an updated + proposal, a closed proposal, and PR body rendering. +- `shellcheck` passes for every script. +- The `docs-proposal-scripts` job in `format.yml` runs all three on every + pull request. +- Copilot cannot run locally; the first `workflow_dispatch` run against a + recent merge SHA is the end-to-end test. + +## Out of scope + +- Crate READMEs, `docs/internal/**`, and `docs/superpowers/**`. +- Rolling or merged proposals across several merges. +- A GitHub App token to trigger regular CI on proposal pull requests. From f70ce72181e839b0bd0d2c9a93bfb56a66694c21 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Thu, 1 Oct 2026 18:03:38 +0530 Subject: [PATCH 02/14] Propose documentation updates after code merges to main Add a Documentation proposal workflow that runs GitHub Copilot CLI on each code merge to main, proposes edits to docs/guide or docs/index.md, and opens one pull request per merge on docs/auto/. The publish job runs the docs gates first, skips empty diffs, and never reopens a closed proposal. It also posts a review comment on every hunk that cites the merged source and offers a suggestion reverting the hunk. Closes #1105. Signed-off-by: dhruv8sh --- .github/workflows/docs-proposal.yml | 110 ++++++++ .github/workflows/format.yml | 25 ++ .gitignore | 3 + scripts/README.md | 12 + scripts/docs-proposal/evidence-prompt.md | 23 ++ scripts/docs-proposal/hunks.mjs | 276 ++++++++++++++++++++ scripts/docs-proposal/hunks.test.mjs | 307 +++++++++++++++++++++++ scripts/docs-proposal/lib.sh | 58 +++++ scripts/docs-proposal/prompt.md | 35 +++ scripts/docs-proposal/propose.sh | 68 +++++ scripts/docs-proposal/publish.sh | 81 ++++++ scripts/docs-proposal/test.sh | 166 ++++++++++++ 12 files changed, 1164 insertions(+) create mode 100644 .github/workflows/docs-proposal.yml create mode 100644 scripts/docs-proposal/evidence-prompt.md create mode 100644 scripts/docs-proposal/hunks.mjs create mode 100644 scripts/docs-proposal/hunks.test.mjs create mode 100644 scripts/docs-proposal/lib.sh create mode 100644 scripts/docs-proposal/prompt.md create mode 100755 scripts/docs-proposal/propose.sh create mode 100755 scripts/docs-proposal/publish.sh create mode 100755 scripts/docs-proposal/test.sh diff --git a/.github/workflows/docs-proposal.yml b/.github/workflows/docs-proposal.yml new file mode 100644 index 000000000..956a192dd --- /dev/null +++ b/.github/workflows/docs-proposal.yml @@ -0,0 +1,110 @@ +name: Documentation proposal + +# Proposes documentation edits with Copilot CLI after code merges to main and +# opens one reviewable pull request per merge. See +# docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md. +on: + push: + branches: [main] + paths: + - "crates/**" + - "trusted-server.example.toml" + - "edgezero.toml" + - "fastly.toml" + - ".cargo/config.toml" + - "scripts/**" + - "!scripts/docs-proposal/**" + workflow_dispatch: + inputs: + sha: + description: Full SHA of a commit on main to propose documentation for + required: true + type: string + +permissions: {} + +# Sequential merges are proposed one at a time, in order. +concurrency: + group: docs-proposal + cancel-in-progress: false + +env: + MERGE_SHA: ${{ inputs.sha || github.sha }} + +jobs: + # Copilot runs here with a read-only token; it cannot push or open PRs. + propose: + runs-on: ubuntu-latest + permissions: + contents: read + copilot-requests: write + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ env.MERGE_SHA }} + fetch-depth: 0 + persist-credentials: false + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + cache: "npm" + cache-dependency-path: docs/package-lock.json + + - name: Install Copilot CLI + run: npm install -g @github/copilot@1.0.90 + + - name: Propose documentation edits + env: + GITHUB_TOKEN: ${{ github.token }} + run: scripts/docs-proposal/propose.sh "$MERGE_SHA" .docs-proposal + + - name: Upload proposal + uses: actions/upload-artifact@v4 + with: + name: docs-proposal + path: .docs-proposal/ + include-hidden-files: true + if-no-files-found: error + + # Copilot does not run here; this job validates and publishes the patch. + publish: + needs: propose + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ env.MERGE_SHA }} + fetch-depth: 0 + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + cache: "npm" + cache-dependency-path: docs/package-lock.json + + - name: Download proposal + uses: actions/download-artifact@v4 + with: + name: docs-proposal + path: .docs-proposal + + - name: Publish proposal pull request + env: + GH_TOKEN: ${{ github.token }} + run: scripts/docs-proposal/publish.sh "$MERGE_SHA" .docs-proposal diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 7253b5ea2..57da08e84 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -151,3 +151,28 @@ jobs: - name: Build with VitePress (fails on dead links) run: npm run build + + docs-proposal-scripts: + name: docs-proposal scripts + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT + shell: bash + + - name: Use Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + + - name: Run shellcheck + run: shellcheck scripts/docs-proposal/*.sh + + - name: Test hunk review builder + run: node --test scripts/docs-proposal/hunks.test.mjs + + - name: Test proposal scripts + run: scripts/docs-proposal/test.sh diff --git a/.gitignore b/.gitignore index 24b9e06aa..ce2b3a693 100644 --- a/.gitignore +++ b/.gitignore @@ -63,3 +63,6 @@ src/*.html # leftover local build artifacts (node_modules, target, dist) that remain on disk. /crates/js/ /crates/integration-tests/ + +# Documentation proposal workflow scratch space +/.docs-proposal/ diff --git a/scripts/README.md b/scripts/README.md index ad4ecf926..8c2081776 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -18,7 +18,19 @@ their documented orchestration handles a probe deliberately. | `smoke-common.sh` | Sourced by the four smoke scripts | Defines bounded port, process, config, secret, and response assertions. Do not invoke it as a standalone smoke. | | `template-cache-local-test.sh` | Viceroy, Node, OpenSSL, `curl`, `lsof`; optional ports and mode | Builds local artifacts, generates certificates and fixtures in a temporary directory, stops owned servers, and removes the directory. | | `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | +| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, and evidence into the work dir. Leaves the edits staged. | +| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Applies the patch, runs the docs gates, force-pushes `docs/auto/`, opens or edits its PR, and posts a per-hunk review. Never writes `main`. | +| `docs-proposal/test.sh` | Git and Node | Tests the helpers and publish flow against a temporary repository with stubbed `gh` and `npm`; removes it on exit. | The four adapter smoke contracts are documented in the [deployment guides](../docs/guide/integrations-overview.md#adapter-support). Repository-wide verification policy lives in [TESTING.md](../TESTING.md). + +The `Documentation proposal` workflow runs both proposal scripts after code +merges to `main` and opens one pull request per merge on `docs/auto/`. +It needs the organization Copilot policy "Allow use of Copilot CLI billed to +the organization" and the repository setting "Allow GitHub Actions to create +and approve pull requests". Pull requests opened with `GITHUB_TOKEN` do not +trigger other workflows, so the publish job runs the docs gates itself; push to +the proposal branch to run regular CI. Retry a merge from the workflow's manual +dispatch with its full commit SHA. diff --git a/scripts/docs-proposal/evidence-prompt.md b/scripts/docs-proposal/evidence-prompt.md new file mode 100644 index 000000000..9bb51e35b --- /dev/null +++ b/scripts/docs-proposal/evidence-prompt.md @@ -0,0 +1,23 @@ +# Documentation proposal evidence + +The hunks at the end of this file are proposed edits to the Trusted Server +documentation for the merged commit named there. For each hunk, find the merged +source that justifies it. + +Write `.docs-proposal/evidence.json` with exactly this shape, and change no +other file: + +```json +{ + "hunks": [ + { "id": "", "source": ":", "reason": "" } + ] +} +``` + +- Copy `id` exactly from the hunk list. +- `source` is a repository-relative path and line at the merged commit that + shows the documented behavior. +- `reason` is one sentence a reviewer can check against that source. +- If no source justifies a hunk, omit it. The reviewer will be told the hunk + has no cited source. diff --git a/scripts/docs-proposal/hunks.mjs b/scripts/docs-proposal/hunks.mjs new file mode 100644 index 000000000..5ad98d7c4 --- /dev/null +++ b/scripts/docs-proposal/hunks.mjs @@ -0,0 +1,276 @@ +// Parses a unified documentation diff into hunks and builds the per-hunk +// review that cites Copilot evidence and offers a suggestion reverting each +// hunk. Dependency-free so it runs on any runner with Node. +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +const FILE_HEADER = /^diff --git a\/(.+) b\/(.+)$/; +const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/; + +export function parseDiff(text) { + const files = []; + let file = null; + let hunk = null; + let oldNo = 0; + let newNo = 0; + for (const raw of text.split("\n")) { + const fileHeader = FILE_HEADER.exec(raw); + if (fileHeader) { + file = { + path: fileHeader[2], + status: "modified", + binary: false, + hunks: [], + }; + files.push(file); + hunk = null; + continue; + } + if (!file) { + continue; + } + const hunkHeader = HUNK_HEADER.exec(raw); + if (hunkHeader) { + oldNo = Number(hunkHeader[1]); + newNo = Number(hunkHeader[2]); + hunk = { lines: [] }; + file.hunks.push(hunk); + continue; + } + if (!hunk) { + if (raw.startsWith("new file mode")) { + file.status = "added"; + } else if (raw.startsWith("deleted file mode")) { + file.status = "deleted"; + } else if (raw.startsWith("Binary files ")) { + file.binary = true; + } + continue; + } + const type = raw[0]; + const lineText = raw.slice(1); + if (type === " ") { + hunk.lines.push({ type, text: lineText, oldNo: oldNo++, newNo: newNo++ }); + } else if (type === "-") { + hunk.lines.push({ type, text: lineText, oldNo: oldNo++, newNo: null }); + } else if (type === "+") { + hunk.lines.push({ type, text: lineText, oldNo: null, newNo: newNo++ }); + } + } + return files; +} + +function hashHunk(path, hunk) { + const hash = createHash("sha256"); + hash.update(path); + for (const line of hunk.lines) { + if (line.type !== " ") { + hash.update(`\n${line.type}${line.text}`); + } + } + return `${path}#${hash.digest("hex").slice(0, 8)}`; +} + +export function collectHunks(files) { + const seen = new Map(); + const entries = []; + for (const file of files) { + if (file.binary) { + continue; + } + for (const hunk of file.hunks) { + const base = hashHunk(file.path, hunk); + const count = (seen.get(base) ?? 0) + 1; + seen.set(base, count); + const id = count === 1 ? base : `${base}-${count}`; + entries.push({ id, path: file.path, status: file.status, hunk }); + } + } + return entries; +} + +export function listHunks(files) { + return collectHunks(files).map(({ id, path, status, hunk }) => ({ + id, + path, + status, + removed: hunk.lines + .filter((line) => line.type === "-") + .map((line) => line.text), + added: hunk.lines + .filter((line) => line.type === "+") + .map((line) => line.text), + })); +} + +// Returns the comment anchor and the original text that undoes a hunk, or +// null when the hunk has no new-side line to anchor a suggestion on. +export function revertSuggestion(hunk) { + const { lines } = hunk; + const first = lines.findIndex((line) => line.type !== " "); + const last = lines.findLastIndex((line) => line.type !== " "); + if (first === -1) { + return null; + } + const span = lines.slice(first, last + 1); + const newSide = span.filter((line) => line.type !== "-"); + const oldText = span + .filter((line) => line.type !== "+") + .map((line) => line.text); + if (newSide.length > 0) { + return { + startLine: newSide[0].newNo, + line: newSide.at(-1).newNo, + body: oldText, + }; + } + const before = lines[first - 1]; + if (before?.type === " ") { + return { + startLine: before.newNo, + line: before.newNo, + body: [before.text, ...oldText], + }; + } + const after = lines[last + 1]; + if (after?.type === " ") { + return { + startLine: after.newNo, + line: after.newNo, + body: [...oldText, after.text], + }; + } + return null; +} + +const MAX_EVIDENCE_LENGTH = 300; + +// Model output lands in review comments, so strip anything that could break +// out of the inline code span or ping a user or team. +function sanitize(value) { + if (typeof value !== "string") { + return ""; + } + return value + .replace(/[`\r\n]+/g, " ") + .replace(/@/g, "@​") + .replace(/\s+/g, " ") + .trim() + .slice(0, MAX_EVIDENCE_LENGTH); +} + +export function indexEvidence(raw) { + const evidence = new Map(); + const items = Array.isArray(raw?.hunks) ? raw.hunks : []; + for (const item of items) { + if (typeof item?.id !== "string") { + continue; + } + const source = sanitize(item.source); + const reason = sanitize(item.reason); + if (source || reason) { + evidence.set(item.id, { source, reason }); + } + } + return evidence; +} + +// A suggestion fence must be longer than any backtick run in its body. +function fenceFor(body) { + let longestRun = 0; + for (const text of body) { + for (const run of text.match(/`+/g) ?? []) { + longestRun = Math.max(longestRun, run.length); + } + } + return "`".repeat(Math.max(3, longestRun + 1)); +} + +function commentBody(cited, body) { + const source = cited?.source + ? `\`${cited.source}\`` + : "none cited — verify this hunk against the merged code."; + const reason = cited?.reason ? ` — ${cited.reason}` : ""; + const fence = fenceFor(body); + const content = body.length > 0 ? `${body.join("\n")}\n` : ""; + return [ + `**Source:** ${source}${reason}`, + "", + "Apply this suggestion to revert the hunk.", + "", + `${fence}suggestion\n${content}${fence}`, + ].join("\n"); +} + +const REVIEW_INTRO = + "Copilot evidence for each proposed hunk. Leave a hunk to accept it, or apply its suggestion to revert it. Verify every cited source before merging."; + +export function buildReview(files, evidence, commitId) { + const comments = []; + const notes = []; + for (const file of files) { + if (file.binary) { + notes.push( + `- \`${file.path}\`: binary change — review the file directly.`, + ); + } + } + for (const { id, path, status, hunk } of collectHunks(files)) { + const suggestion = revertSuggestion(hunk); + if (!suggestion) { + const reason = + status === "deleted" ? "file deleted" : "no line to anchor a revert"; + notes.push(`- \`${path}\`: ${reason} — revert it manually if needed.`); + continue; + } + const comment = { + path, + line: suggestion.line, + side: "RIGHT", + body: commentBody(evidence.get(id), suggestion.body), + }; + if (suggestion.startLine !== suggestion.line) { + comment.start_line = suggestion.startLine; + comment.start_side = "RIGHT"; + } + comments.push(comment); + } + const body = + notes.length > 0 + ? `${REVIEW_INTRO}\n\nChanges without a suggestion:\n\n${notes.join("\n")}` + : REVIEW_INTRO; + return { commit_id: commitId, event: "COMMENT", body, comments }; +} + +function readEvidence(path) { + try { + return indexEvidence(JSON.parse(readFileSync(path, "utf8"))); + } catch (error) { + process.stderr.write( + `Ignoring unreadable evidence at ${path}: ${error.message}\n`, + ); + return new Map(); + } +} + +function main([command, evidencePath, commitId]) { + const files = parseDiff(readFileSync(0, "utf8")); + if (command === "list") { + process.stdout.write(`${JSON.stringify(listHunks(files), null, 2)}\n`); + return; + } + if (command === "review" && evidencePath && commitId) { + const review = buildReview(files, readEvidence(evidencePath), commitId); + process.stdout.write(`${JSON.stringify(review, null, 2)}\n`); + return; + } + process.stderr.write( + "usage: hunks.mjs list | review \n", + ); + process.exitCode = 2; +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main(process.argv.slice(2)); +} diff --git a/scripts/docs-proposal/hunks.test.mjs b/scripts/docs-proposal/hunks.test.mjs new file mode 100644 index 000000000..09440708e --- /dev/null +++ b/scripts/docs-proposal/hunks.test.mjs @@ -0,0 +1,307 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + buildReview, + collectHunks, + indexEvidence, + listHunks, + parseDiff, + revertSuggestion, +} from "./hunks.mjs"; + +const MODIFIED = [ + "diff --git a/docs/guide/cli.md b/docs/guide/cli.md", + "index 1111111..2222222 100644", + "--- a/docs/guide/cli.md", + "+++ b/docs/guide/cli.md", + "@@ -10,4 +10,4 @@ heading", + " keep one", + "-old flag", + "+new flag", + " keep two", + "", +].join("\n"); + +test("parseDiff numbers context, removed, and added lines", () => { + const [file] = parseDiff(MODIFIED); + assert.equal(file.path, "docs/guide/cli.md", "should take the b/ path"); + assert.equal(file.status, "modified", "should default to modified"); + assert.deepEqual( + file.hunks[0].lines, + [ + { type: " ", text: "keep one", oldNo: 10, newNo: 10 }, + { type: "-", text: "old flag", oldNo: 11, newNo: null }, + { type: "+", text: "new flag", oldNo: null, newNo: 11 }, + { type: " ", text: "keep two", oldNo: 12, newNo: 12 }, + ], + "should track old and new line numbers", + ); +}); + +test("parseDiff records added, deleted, and binary files", () => { + const diff = [ + "diff --git a/docs/guide/new.md b/docs/guide/new.md", + "new file mode 100644", + "--- /dev/null", + "+++ b/docs/guide/new.md", + "@@ -0,0 +1 @@", + "+hello", + "diff --git a/docs/guide/gone.md b/docs/guide/gone.md", + "deleted file mode 100644", + "--- a/docs/guide/gone.md", + "+++ /dev/null", + "@@ -1 +0,0 @@", + "-bye", + "diff --git a/docs/guide/logo.png b/docs/guide/logo.png", + "Binary files a/docs/guide/logo.png and b/docs/guide/logo.png differ", + "", + ].join("\n"); + const files = parseDiff(diff); + assert.deepEqual( + files.map(({ path, status, binary }) => ({ path, status, binary })), + [ + { path: "docs/guide/new.md", status: "added", binary: false }, + { path: "docs/guide/gone.md", status: "deleted", binary: false }, + { path: "docs/guide/logo.png", status: "modified", binary: true }, + ], + "should classify each file", + ); + assert.deepEqual( + files[0].hunks[0].lines, + [{ type: "+", text: "hello", oldNo: null, newNo: 1 }], + "should start a new file at line 1", + ); +}); + +test("parseDiff skips no-newline markers", () => { + const diff = MODIFIED.replace( + " keep two\n", + " keep two\n\\ No newline at end of file\n", + ); + const [file] = parseDiff(diff); + assert.equal(file.hunks[0].lines.length, 4, "should ignore the marker line"); +}); + +test("collectHunks gives stable ids and skips binary files", () => { + const first = collectHunks(parseDiff(MODIFIED)); + const second = collectHunks( + parseDiff(MODIFIED.replace("@@ -10,4 +10,4", "@@ -20,4 +20,4")), + ); + assert.equal(first.length, 1, "should collect the single hunk"); + assert.match( + first[0].id, + /^docs\/guide\/cli\.md#[0-9a-f]{8}$/, + "should use path#hash", + ); + assert.equal(first[0].id, second[0].id, "should not depend on line numbers"); +}); + +test("collectHunks suffixes duplicate ids", () => { + const twice = MODIFIED + MODIFIED.split("\n").slice(4).join("\n"); + const ids = collectHunks(parseDiff(twice)).map((entry) => entry.id); + assert.equal(ids.length, 2, "should collect both hunks"); + assert.equal(ids[1], `${ids[0]}-2`, "should suffix the repeated id"); +}); + +test("listHunks exposes removed and added text", () => { + const [entry] = listHunks(parseDiff(MODIFIED)); + assert.deepEqual(entry.removed, ["old flag"], "should list removed lines"); + assert.deepEqual(entry.added, ["new flag"], "should list added lines"); + assert.equal(entry.status, "modified", "should carry the file status"); +}); + +function hunkOf(lines, oldStart = 1, newStart = 1) { + const diff = [ + "diff --git a/docs/guide/a.md b/docs/guide/a.md", + "--- a/docs/guide/a.md", + "+++ b/docs/guide/a.md", + `@@ -${oldStart} +${newStart} @@`, + ...lines, + "", + ].join("\n"); + return parseDiff(diff)[0].hunks[0]; +} + +test("revertSuggestion restores replaced lines", () => { + const suggestion = revertSuggestion(hunkOf([" a", "-b", "+B", "+B2", " c"])); + assert.deepEqual( + suggestion, + { startLine: 2, line: 3, body: ["b"] }, + "should span the new lines and restore the old one", + ); +}); + +test("revertSuggestion deletes pure insertions", () => { + const suggestion = revertSuggestion(hunkOf([" a", "+new", " c"])); + assert.deepEqual( + suggestion, + { startLine: 2, line: 2, body: [] }, + "should suggest an empty body", + ); +}); + +test("revertSuggestion spans several change groups with their context", () => { + const suggestion = revertSuggestion(hunkOf(["-a", "+A", " mid", "-c", "+C"])); + assert.deepEqual( + suggestion, + { startLine: 1, line: 3, body: ["a", "mid", "c"] }, + "should keep the middle context line", + ); +}); + +test("revertSuggestion anchors pure deletions on the previous context line", () => { + const suggestion = revertSuggestion(hunkOf([" a", "-gone", " c"], 4, 4)); + assert.deepEqual( + suggestion, + { startLine: 4, line: 4, body: ["a", "gone"] }, + "should restore the deleted line after its anchor", + ); +}); + +test("revertSuggestion anchors leading deletions on the next context line", () => { + const suggestion = revertSuggestion(hunkOf(["-gone", " a"])); + assert.deepEqual( + suggestion, + { startLine: 1, line: 1, body: ["gone", "a"] }, + "should restore the deleted line before its anchor", + ); +}); + +test("revertSuggestion returns null without an anchor", () => { + assert.equal( + revertSuggestion(hunkOf(["-only"])), + null, + "should not invent an anchor", + ); +}); + +test("indexEvidence keeps valid entries and sanitizes text", () => { + const evidence = indexEvidence({ + hunks: [ + { + id: "x#1", + source: "crates/a.rs:4", + reason: "Adds `--flag`\nfor @team", + }, + { id: 7, source: "ignored" }, + { id: "x#2" }, + "junk", + ], + }); + assert.deepEqual( + [...evidence], + [["x#1", { source: "crates/a.rs:4", reason: "Adds --flag for @​team" }]], + "should drop invalid entries and strip backticks, newlines, and mentions", + ); + assert.equal(indexEvidence(null).size, 0, "should accept missing evidence"); +}); + +test("buildReview cites evidence and suggests reverts", () => { + const files = parseDiff( + [ + "diff --git a/docs/guide/a.md b/docs/guide/a.md", + "--- a/docs/guide/a.md", + "+++ b/docs/guide/a.md", + "@@ -1,3 +1,3 @@", + " a", + "-uses ```fence```", + "+B", + " c", + "@@ -10 +10,2 @@", + " j", + "+k", + "", + ].join("\n"), + ); + const [first] = collectHunks(files); + const evidence = indexEvidence({ + hunks: [ + { id: first.id, source: "crates/a.rs:9", reason: "Renames the flag." }, + ], + }); + const review = buildReview(files, evidence, "abc123"); + + assert.equal(review.commit_id, "abc123", "should pin the reviewed commit"); + assert.equal( + review.event, + "COMMENT", + "should not approve or request changes", + ); + assert.equal(review.comments.length, 2, "should comment on every hunk"); + assert.deepEqual( + review.comments[0], + { + path: "docs/guide/a.md", + line: 2, + side: "RIGHT", + body: [ + "**Source:** `crates/a.rs:9` — Renames the flag.", + "", + "Apply this suggestion to revert the hunk.", + "", + "````suggestion", + "uses ```fence```", + "````", + ].join("\n"), + }, + "should escalate the fence and omit start_line for one line", + ); + assert.match( + review.comments[1].body, + /none cited/, + "should flag hunks without evidence", + ); + assert.match( + review.comments[1].body, + /```suggestion\n```$/, + "should suggest deleting an insertion", + ); +}); + +test("buildReview lists binary and unanchored changes in the body", () => { + const files = parseDiff( + [ + "diff --git a/docs/guide/gone.md b/docs/guide/gone.md", + "deleted file mode 100644", + "@@ -1 +0,0 @@", + "-bye", + "diff --git a/docs/guide/logo.png b/docs/guide/logo.png", + "Binary files a/docs/guide/logo.png and b/docs/guide/logo.png differ", + "", + ].join("\n"), + ); + const review = buildReview(files, new Map(), "abc123"); + assert.equal(review.comments.length, 0, "should not post line comments"); + assert.match( + review.body, + /`docs\/guide\/gone\.md`: file deleted/, + "should note the deletion", + ); + assert.match( + review.body, + /`docs\/guide\/logo\.png`: binary change/, + "should note the binary file", + ); +}); + +test("buildReview adds start_line for multi-line spans", () => { + const files = parseDiff( + [ + "diff --git a/docs/index.md b/docs/index.md", + "@@ -1 +1,2 @@", + "-a", + "+b", + "+c", + "", + ].join("\n"), + ); + const [comment] = buildReview(files, new Map(), "abc123").comments; + assert.equal(comment.start_line, 1, "should start at the first new line"); + assert.equal( + comment.start_side, + "RIGHT", + "should anchor the start on the new side", + ); + assert.equal(comment.line, 2, "should end at the last new line"); +}); diff --git a/scripts/docs-proposal/lib.sh b/scripts/docs-proposal/lib.sh new file mode 100644 index 000000000..87fb538d7 --- /dev/null +++ b/scripts/docs-proposal/lib.sh @@ -0,0 +1,58 @@ +# shellcheck shell=bash +# Shared helpers for the documentation proposal scripts. Source this file; +# do not execute it. + +DOCS_PROPOSAL_ALLOWED_RE='^docs/(guide/.+|index\.md)$' +DOCS_PROPOSAL_MAX_RATIONALE_BYTES=20000 + +# Exits unless the argument is a full commit SHA reachable from origin/main. +docs_proposal_require_sha() { + local sha="$1" + if ! [[ "$sha" =~ ^[0-9a-f]{40}$ ]]; then + printf '::error::Expected a full 40-character commit SHA, got %s\n' "$sha" >&2 + exit 1 + fi + if ! git merge-base --is-ancestor "$sha" origin/main 2>/dev/null; then + printf '::error::Commit %s is not reachable from origin/main\n' "$sha" >&2 + exit 1 + fi +} + +docs_proposal_branch() { + printf 'docs/auto/%s\n' "${1:0:12}" +} + +# Reads changed paths on stdin, prints those outside the allowlist, and +# returns 1 when any were found. +docs_proposal_disallowed_paths() { + local path + local found=0 + while IFS= read -r path; do + if [ -n "$path" ] && ! [[ "$path" =~ $DOCS_PROPOSAL_ALLOWED_RE ]]; then + printf '%s\n' "$path" + found=1 + fi + done + return "$found" +} + +docs_proposal_pr_body() { + local sha="$1" + local subject="$2" + local origin_pr="$3" + local rationale="$4" + local origin="" + if [ -n "$origin_pr" ]; then + origin=" (#$origin_pr)" + fi + printf '\n' "$sha" + printf 'Proposed documentation update for merged change %s%s: %s\n\n' "$sha" "$origin" "$subject" + printf 'Generated by GitHub Copilot CLI in the docs-proposal workflow. A maintainer must verify every hunk against the merged code before merging. Each hunk has a review comment citing its source and a suggestion that reverts it.\n\n' + printf '## Rationale\n\n' + if [ -s "$rationale" ]; then + head -c "$DOCS_PROPOSAL_MAX_RATIONALE_BYTES" "$rationale" + printf '\n' + else + printf '_Copilot did not provide a rationale._\n' + fi +} diff --git a/scripts/docs-proposal/prompt.md b/scripts/docs-proposal/prompt.md new file mode 100644 index 000000000..691060fb1 --- /dev/null +++ b/scripts/docs-proposal/prompt.md @@ -0,0 +1,35 @@ +# Documentation proposal + +You are updating the public Trusted Server documentation after a commit merged +to `main`. The merged commit and the table mapping implementation sources to +documentation pages appear at the end of this file. + +## Task + +1. Inspect the merged change with `git show --stat ` and + `git show `. +2. Decide whether it changes behavior that `docs/index.md` or a page under + `docs/guide/` describes: configuration fields and defaults, routes, adapter + support, integrations, auction behavior, CLI commands and flags, browser + bundles, or verification commands. Use the Sources of truth table to find + the affected pages. +3. If no reader-facing documentation is affected, change nothing and write + `.docs-proposal/rationale.md` containing one sentence explaining why. +4. Otherwise edit only the affected pages, then write + `.docs-proposal/rationale.md` with one bullet per edited page naming the + page, what changed, and the merged source file that justifies it. + +## Rules + +- Describe behavior the merged code ships. Do not document intent, roadmap + items, or unmerged work. +- Read the merged source before writing. Every statement you add must be + verifiable in the code at this commit. +- Make the smallest edit that keeps the page accurate. Do not restyle, + reorder, or rewrite unaffected text. +- Use only fictional values: `example.com` domains and placeholder secrets. + Never copy real domains, customer names, credentials, or service IDs. +- Keep existing links and anchors working, and link to other pages with + relative paths. +- Do not edit files outside `docs/guide/`, `docs/index.md`, and + `.docs-proposal/`. Do not commit, push, or create branches. diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh new file mode 100755 index 000000000..6e2b6a0b2 --- /dev/null +++ b/scripts/docs-proposal/propose.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# Asks Copilot CLI to propose documentation edits for one merged commit. +# +# Usage: scripts/docs-proposal/propose.sh +# Writes proposal.patch (empty when nothing changes), rationale.md, and +# evidence.json to , which must be inside the repository so Copilot +# can read the prompts and write its outputs there. +set -euo pipefail + +sha="$1" +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +root="$(git rev-parse --show-toplevel)" +spec="$root/docs/superpowers/specs/2026-08-19-documentation-refresh-design.md" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +cd "$root" +docs_proposal_require_sha "$sha" +mkdir -p "$2" +work_dir="$(cd "$2" && pwd)" +relative_dir="${work_dir#"$root"/}" + +# Reads are always permitted; writes are allowed so Copilot can edit pages, +# and publish.sh enforces which paths may actually change. +run_copilot() { + copilot -p "Read $relative_dir/$1 and follow its instructions exactly." \ + --no-ask-user \ + --allow-tool=write \ + --allow-tool='shell(git show:*)' \ + --allow-tool='shell(git diff:*)' \ + --allow-tool='shell(git log:*)' \ + --allow-tool='shell(git grep:*)' \ + --deny-tool='shell(git push)' \ + --deny-tool='shell(git commit)' +} + +{ + cat "$here/prompt.md" + printf '\n## Merged change\n\n- Commit: %s\n- Subject: %s\n\n' "$sha" "$(git log -1 --format=%s "$sha")" + printf '## Sources of truth\n\n' + awk '/^## Sources of truth/ { found = 1; next } /^## / { found = 0 } found' "$spec" +} > "$work_dir/prompt.md" +run_copilot prompt.md + +# Normalize Copilot's edits before hunks are identified so their ids stay +# valid after the publish job's format check. +(cd docs && npm ci && npm run format:write) + +git add -A -- docs +git diff --cached --no-renames --binary "$sha" -- docs > "$work_dir/proposal.patch" +touch "$work_dir/rationale.md" +if [ ! -s "$work_dir/proposal.patch" ]; then + printf '{}\n' > "$work_dir/evidence.json" + printf 'No documentation changes proposed for %s.\n' "$sha" + exit 0 +fi + +{ + cat "$here/evidence-prompt.md" + printf '\n## Merged change\n\n- Commit: %s\n\n## Hunks\n\n```json\n' "$sha" + git diff --cached --no-renames "$sha" -- docs | node "$here/hunks.mjs" list + printf '```\n' +} > "$work_dir/evidence-prompt.md" +run_copilot evidence-prompt.md +if [ ! -f "$work_dir/evidence.json" ]; then + printf '{}\n' > "$work_dir/evidence.json" +fi +printf 'Proposed documentation changes for %s.\n' "$sha" diff --git a/scripts/docs-proposal/publish.sh b/scripts/docs-proposal/publish.sh new file mode 100755 index 000000000..90fa8c4ff --- /dev/null +++ b/scripts/docs-proposal/publish.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# Validates a documentation proposal and opens or updates its pull request. +# +# Usage: scripts/docs-proposal/publish.sh +# holds proposal.patch, rationale.md, and evidence.json from +# propose.sh. Requires GH_TOKEN with contents and pull-requests write access. +set -euo pipefail + +sha="$1" +work_dir="$(cd "$2" && pwd)" +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +root="$(git rev-parse --show-toplevel)" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +cd "$root" +docs_proposal_require_sha "$sha" +branch="$(docs_proposal_branch "$sha")" + +# A closed or merged proposal is a maintainer decision; never reopen or +# recreate it on a retry. +existing="$(gh pr list --head "$branch" --state all --json number,state --jq '.[0] | select(.) | "\(.number) \(.state)"')" +pr_number="${existing%% *}" +pr_state="${existing#* }" +if [ -n "$existing" ] && [ "$pr_state" != "OPEN" ]; then + printf 'Proposal #%s for %s is %s; not reopening it.\n' "$pr_number" "$sha" "$pr_state" + exit 0 +fi + +if [ ! -s "$work_dir/proposal.patch" ]; then + printf 'No documentation changes proposed for %s.\n' "$sha" + if [ -n "$existing" ]; then + gh pr close "$pr_number" --delete-branch \ + --comment "A rerun for $sha proposed no documentation changes." + fi + exit 0 +fi + +git switch --quiet --detach "$sha" +git apply --index "$work_dir/proposal.patch" +if ! disallowed="$(git diff --cached --name-only --no-renames "$sha" | docs_proposal_disallowed_paths)"; then + printf '::error::Proposal changes paths outside docs/guide/** and docs/index.md:\n%s\n' "$disallowed" + exit 1 +fi + +(cd docs && npm ci && npm run lint && npm run format && npm run build) + +git -c user.name='github-actions[bot]' \ + -c user.email='41898282+github-actions[bot]@users.noreply.github.com' \ + commit --quiet --no-verify \ + -m "Propose documentation updates for ${sha:0:12}" \ + -m "Generated by the docs-proposal workflow from $sha." +head_sha="$(git rev-parse HEAD)" + +# An identical tree means this is a retry of an already-published proposal; +# skip the push and review so reviewers do not see duplicate comments. +remote_tree="" +if git fetch --quiet origin "refs/heads/$branch" 2>/dev/null; then + remote_tree="$(git rev-parse 'FETCH_HEAD^{tree}')" +fi +if [ -n "$existing" ] && [ "$remote_tree" = "$(git rev-parse 'HEAD^{tree}')" ]; then + printf 'Proposal #%s for %s is unchanged.\n' "$pr_number" "$sha" + exit 0 +fi +git push --quiet --force origin "HEAD:refs/heads/$branch" + +subject="$(git log -1 --format=%s "$sha")" +origin_pr="$(gh api "repos/{owner}/{repo}/commits/$sha/pulls" --jq '.[0].number // empty' || true)" +title="$(printf 'Update documentation for %s' "$subject" | cut -c1-200)" +docs_proposal_pr_body "$sha" "$subject" "$origin_pr" "$work_dir/rationale.md" > "$work_dir/pr-body.md" +if [ -n "$existing" ]; then + gh pr edit "$pr_number" --title "$title" --body-file "$work_dir/pr-body.md" +else + pr_url="$(gh pr create --base main --head "$branch" --title "$title" --body-file "$work_dir/pr-body.md")" + pr_number="${pr_url##*/}" +fi + +git diff --no-renames "$sha" "$head_sha" -- docs \ + | node "$here/hunks.mjs" review "$work_dir/evidence.json" "$head_sha" > "$work_dir/review.json" +gh api --method POST "repos/{owner}/{repo}/pulls/$pr_number/reviews" --input "$work_dir/review.json" > /dev/null +printf 'Published proposal #%s for %s.\n' "$pr_number" "$sha" diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh new file mode 100755 index 000000000..6a6eb1181 --- /dev/null +++ b/scripts/docs-proposal/test.sh @@ -0,0 +1,166 @@ +#!/usr/bin/env bash +# Tests the documentation proposal helpers and publish flow with stubbed gh +# and npm against a throwaway git repository. +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +failures=0 + +assert_eq() { + if [ "$1" != "$2" ]; then + printf 'FAIL: %s\n expected: %s\n actual: %s\n' "$3" "$2" "$1" + failures=$((failures + 1)) + fi +} + +assert_contains() { + if [[ "$1" != *"$2"* ]]; then + printf 'FAIL: %s\n missing: %s\n' "$3" "$2" + failures=$((failures + 1)) + fi +} + +sha=0123456789abcdef0123456789abcdef01234567 + +assert_eq "$(docs_proposal_branch "$sha")" "docs/auto/0123456789ab" "should derive the branch from the SHA" + +bad="$(printf 'docs/guide/a.md\ndocs/index.md\ndocs/superpowers/x.md\nREADME.md\n' | docs_proposal_disallowed_paths || true)" +assert_eq "$bad" $'docs/superpowers/x.md\nREADME.md' "should print only paths outside the allowlist" + +if printf 'docs/guide/a.md\ndocs/guide/integrations/b.md\n' | docs_proposal_disallowed_paths >/dev/null; then + status=0 +else + status=1 +fi +assert_eq "$status" 0 "should accept guide and nested guide paths" + +rationale="$(mktemp)" +printf -- '- docs/guide/cli.md: documents the new flag.\n' > "$rationale" +body="$(docs_proposal_pr_body "$sha" "Add example flag" 42 "$rationale")" +assert_contains "$body" "" "should embed the SHA marker" +assert_contains "$body" "merged change $sha (#42): Add example flag" "should link the merge and its PR" +assert_contains "$body" "documents the new flag" "should include the rationale" +: > "$rationale" +body="$(docs_proposal_pr_body "$sha" "Add example flag" "" "$rationale")" +assert_contains "$body" "merged change $sha: Add example flag" "should omit a missing origin PR" +assert_contains "$body" "did not provide a rationale" "should note an empty rationale" +rm -f "$rationale" + +if (docs_proposal_require_sha "not-a-sha") 2>/dev/null; then + status=0 +else + status=1 +fi +assert_eq "$status" 1 "should reject a malformed SHA" + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 +export GIT_AUTHOR_NAME=Example GIT_AUTHOR_EMAIL=dev@example.com +export GIT_COMMITTER_NAME=Example GIT_COMMITTER_EMAIL=dev@example.com + +mkdir -p "$tmp/bin" +cat > "$tmp/bin/gh" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$STUB_LOG" +case "$1 $2" in + "pr list") printf '%s' "${STUB_PR:-}" ;; + "pr create") printf 'https://github.com/example/repo/pull/7\n' ;; + "api repos/{owner}/{repo}/commits/"*) printf '%s' "${STUB_ORIGIN_PR:-}" ;; + "api --method") + for arg in "$@"; do last="$arg"; done + cp "$last" "$STUB_REVIEW" + ;; +esac +STUB +cat > "$tmp/bin/npm" <<'STUB' +#!/usr/bin/env bash +printf 'npm %s\n' "$*" >> "$STUB_LOG" +STUB +chmod +x "$tmp/bin/gh" "$tmp/bin/npm" +export PATH="$tmp/bin:$PATH" STUB_LOG="$tmp/gh.log" STUB_REVIEW="$tmp/review.json" + +git init -q --bare "$tmp/origin.git" +git clone -q "$tmp/origin.git" "$tmp/work" 2>/dev/null +cd "$tmp/work" +mkdir -p docs/guide +printf 'line one\nline two\nline three\n' > docs/guide/cli.md +git add docs && git commit -q -m "Seed docs" && git push -q origin HEAD:main +merge_sha="$(git rev-parse HEAD)" +branch="$(docs_proposal_branch "$merge_sha")" + +# Each publish run leaves HEAD on its proposal commit, so every patch starts +# from a clean checkout of the merge commit. +make_patch() { + git reset -q --hard "$merge_sha" && git clean -qfd + rm -rf "$tmp/proposal" && mkdir -p "$tmp/proposal" + if [ -n "$1" ]; then + mkdir -p "$(dirname "$1")" && printf 'proposed\n' >> "$1" + git add -A && git diff --cached --binary "$merge_sha" > "$tmp/proposal/proposal.patch" + git reset -q --hard "$merge_sha" && git clean -qfd + else + : > "$tmp/proposal/proposal.patch" + fi + printf -- '- docs/guide/cli.md: example rationale.\n' > "$tmp/proposal/rationale.md" + printf '{}\n' > "$tmp/proposal/evidence.json" +} + +run_publish() { + : > "$STUB_LOG" + rm -f "$STUB_REVIEW" + if "$here/publish.sh" "$merge_sha" "$tmp/proposal" > "$tmp/out.log" 2>&1; then + printf '0' + else + printf '1' + fi +} + +remote_has_branch() { + if git ls-remote --exit-code -q origin "refs/heads/$branch" >/dev/null; then + printf 'yes' + else + printf 'no' + fi +} + +make_patch "" +assert_eq "$(STUB_PR="" run_publish)" 0 "should succeed on an empty proposal" +assert_eq "$(grep -c 'pr create\|pr close' "$STUB_LOG" || true)" 0 "should not open or close a PR for an empty diff" + +make_patch "" +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed when closing a stale proposal" +assert_contains "$(cat "$STUB_LOG")" "pr close 7" "should close the open proposal for an empty diff" + +make_patch docs/superpowers/notes.md +assert_eq "$(STUB_PR="" run_publish)" 1 "should fail for a disallowed path" +assert_contains "$(cat "$tmp/out.log")" "docs/superpowers/notes.md" "should name the disallowed path" +assert_eq "$(remote_has_branch)" no "should not push a disallowed proposal" + +make_patch docs/guide/cli.md +assert_eq "$(STUB_PR="" STUB_ORIGIN_PR=42 run_publish)" 0 "should publish a valid proposal" +assert_eq "$(remote_has_branch)" yes "should push the proposal branch" +assert_contains "$(cat "$STUB_LOG")" "pr create --base main --head $branch" "should open a PR from the proposal branch" +assert_contains "$(cat "$STUB_LOG")" "npm run build" "should run the docs build" +assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/guide/cli.md"' "should post a review for the hunk" + +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed on an unchanged retry" +assert_eq "$(grep -c 'pr edit\|pr create\|reviews' "$STUB_LOG" || true)" 0 "should not re-post an unchanged proposal" + +make_patch docs/index.md +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should update a changed proposal" +assert_contains "$(cat "$STUB_LOG")" "pr edit 7" "should edit the existing PR" +assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/index.md"' "should review the new hunk" + +assert_eq "$(STUB_PR="7 CLOSED" run_publish)" 0 "should succeed for a closed proposal" +assert_eq "$(grep -c 'pr edit\|pr create' "$STUB_LOG" || true)" 0 "should never reopen or recreate a closed proposal" + +cd "$here" + +if [ "$failures" -gt 0 ]; then + printf '%d failure(s)\n' "$failures" + exit 1 +fi +printf 'All documentation proposal script tests passed.\n' From f22e366a83c52d8097ef8635985a256103ca85da Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Sat, 3 Oct 2026 17:52:08 +0530 Subject: [PATCH 03/14] Isolate documentation proposal builds from publication credentials Split the docs gates into a validate job with a read-only token and no persisted credentials, since VitePress executes Vue in generated Markdown. The publish job now re-checks the path allowlist on a fresh checkout without building. Every job runs the scripts from the workflow's revision against a separate checkout of the merge commit, so historical merges can be retried. Queue every pending run instead of cancelling all but one, and have Copilot inspect the full pushed range so merge commits and multi-commit pushes are not missed. A retry with an unchanged tree now reuses the pushed commit and posts the review if an earlier run failed before posting it. Signed-off-by: dhruv8sh --- .github/workflows/docs-proposal.yml | 96 ++++++++++++++++--- ...6-10-01-docs-proposal-automation-design.md | 82 +++++++++++----- scripts/README.md | 45 +++++---- scripts/docs-proposal/lib.sh | 14 +++ scripts/docs-proposal/prompt.md | 10 +- scripts/docs-proposal/propose.sh | 19 ++-- scripts/docs-proposal/publish.sh | 40 ++++---- scripts/docs-proposal/test.sh | 48 ++++++++-- scripts/docs-proposal/validate.sh | 27 ++++++ 9 files changed, 288 insertions(+), 93 deletions(-) create mode 100755 scripts/docs-proposal/validate.sh diff --git a/.github/workflows/docs-proposal.yml b/.github/workflows/docs-proposal.yml index 956a192dd..bb5b9cb3d 100644 --- a/.github/workflows/docs-proposal.yml +++ b/.github/workflows/docs-proposal.yml @@ -23,13 +23,20 @@ on: permissions: {} -# Sequential merges are proposed one at a time, in order. +# Sequential merges are proposed one at a time, in order. The default queue +# keeps only one pending run and cancels older ones, so keep them all. concurrency: group: docs-proposal cancel-in-progress: false + queue: max +# Each job runs the proposal scripts from the workflow's own revision in +# `tools/` against a checkout of the merge commit in `source/`, so retrying a +# merge that predates this tooling, or a later fix to it, still works. A push +# that adds several commits is inspected from the previous main head. env: MERGE_SHA: ${{ inputs.sha || github.sha }} + BASE_SHA: ${{ github.event_name == 'push' && github.event.before || '' }} jobs: # Copilot runs here with a read-only token; it cannot push or open PRs. @@ -39,72 +46,133 @@ jobs: contents: read copilot-requests: write steps: - - name: Checkout + - name: Checkout tools + uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + path: tools + persist-credentials: false + + - name: Checkout merge commit uses: actions/checkout@v4 with: ref: ${{ env.MERGE_SHA }} + path: source fetch-depth: 0 persist-credentials: false - name: Retrieve Node.js version id: node-version - run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + run: echo "node-version=$(grep '^nodejs ' tools/.tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: ${{ steps.node-version.outputs.node-version }} cache: "npm" - cache-dependency-path: docs/package-lock.json + cache-dependency-path: source/docs/package-lock.json - name: Install Copilot CLI run: npm install -g @github/copilot@1.0.90 - name: Propose documentation edits + working-directory: source env: GITHUB_TOKEN: ${{ github.token }} - run: scripts/docs-proposal/propose.sh "$MERGE_SHA" .docs-proposal + run: ../tools/scripts/docs-proposal/propose.sh "$MERGE_SHA" .docs-proposal "$BASE_SHA" - name: Upload proposal uses: actions/upload-artifact@v4 with: name: docs-proposal - path: .docs-proposal/ + path: source/.docs-proposal/ include-hidden-files: true if-no-files-found: error - # Copilot does not run here; this job validates and publishes the patch. - publish: + # VitePress executes Vue in Markdown during the build, so the docs gates run + # here with no write token or persisted credentials. + validate: needs: propose runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout tools + uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + path: tools + persist-credentials: false + + - name: Checkout merge commit + uses: actions/checkout@v4 + with: + ref: ${{ env.MERGE_SHA }} + path: source + fetch-depth: 0 + persist-credentials: false + + - name: Retrieve Node.js version + id: node-version + run: echo "node-version=$(grep '^nodejs ' tools/.tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node-version.outputs.node-version }} + cache: "npm" + cache-dependency-path: source/docs/package-lock.json + + - name: Download proposal + uses: actions/download-artifact@v4 + with: + name: docs-proposal + path: source/.docs-proposal + + - name: Validate proposal + working-directory: source + run: ../tools/scripts/docs-proposal/validate.sh "$MERGE_SHA" .docs-proposal + + # Copilot does not run here, and neither does any proposal content: this job + # re-checks the path allowlist on a fresh checkout and publishes the patch. + publish: + needs: validate + runs-on: ubuntu-latest permissions: contents: write pull-requests: write steps: - - name: Checkout + - name: Checkout tools + uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + path: tools + persist-credentials: false + + - name: Checkout merge commit uses: actions/checkout@v4 with: ref: ${{ env.MERGE_SHA }} + path: source fetch-depth: 0 - name: Retrieve Node.js version id: node-version - run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" + run: echo "node-version=$(grep '^nodejs ' tools/.tool-versions | awk '{print $2}')" >> "$GITHUB_OUTPUT" - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: ${{ steps.node-version.outputs.node-version }} - cache: "npm" - cache-dependency-path: docs/package-lock.json - name: Download proposal uses: actions/download-artifact@v4 with: name: docs-proposal - path: .docs-proposal + path: source/.docs-proposal - name: Publish proposal pull request + working-directory: source env: GH_TOKEN: ${{ github.token }} - run: scripts/docs-proposal/publish.sh "$MERGE_SHA" .docs-proposal + run: ../tools/scripts/docs-proposal/publish.sh "$MERGE_SHA" .docs-proposal diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index 545adb58d..a8a3c8f53 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -33,8 +33,14 @@ only ever writes to a dedicated proposal branch and a pull request. ## Workflow shape -`.github/workflows/docs-proposal.yml` has two jobs. Copilot never holds a -token that can push or open pull requests. +`.github/workflows/docs-proposal.yml` has three jobs. Copilot never holds a +token that can push or open pull requests, and proposal content is never built +or executed in a job that can. + +Every job checks out the workflow's own revision into `tools/` and the merge +commit into `source/`, then runs the scripts from `tools/` against `source/`. +Retrying a merge that predates this tooling therefore works, and a retry runs +the current scripts rather than historical ones. ### Trigger @@ -44,19 +50,24 @@ token that can push or open pull requests. - `workflow_dispatch` with a required `sha` input, for retries and for the first end-to-end validation. Both scripts require a full 40-character SHA reachable from `origin/main`. -- `concurrency: { group: docs-proposal, cancel-in-progress: false }` so - sequential merges are processed one at a time, in order. +- `concurrency` with `group: docs-proposal`, `cancel-in-progress: false`, and + `queue: max` so sequential merges are processed one at a time, in order. + The default queue keeps one pending run and cancels the rest, which would + skip merges. ### Job `propose` Permissions: `contents: read`, `copilot-requests: write`. -1. Check out the merge commit with full history. +1. Check out the tools and the merge commit with full history. 2. Install Copilot CLI (`@github/copilot`) at a pinned version. -3. `scripts/docs-proposal/propose.sh .docs-proposal`: +3. `scripts/docs-proposal/propose.sh .docs-proposal `: 1. Renders `.docs-proposal/prompt.md` from `scripts/docs-proposal/prompt.md`, - the merge SHA, its subject, and the Sources-of-truth table from the - refresh design. + the merge SHA, its base, its subject, and the Sources-of-truth table from + the refresh design. The base is the push's previous `main` head, or the + first parent for a manual dispatch, so Copilot inspects + `git diff `: a multi-commit push is covered in full, and a + clean merge commit is not hidden by `git show`'s empty combined diff. 2. Runs `copilot -p` with `--no-ask-user`, `--allow-tool=write`, and `git show`, `git diff`, `git log`, and `git grep` shell tools; `git push` and `git commit` are denied, and file reads need no permission. Copilot @@ -73,9 +84,24 @@ Permissions: `contents: read`, `copilot-requests: write`. that justifies it and a one-sentence reason. 4. Upload the `.docs-proposal/` directory as an artifact. +### Job `validate` + +Permissions: `contents: read`; checkouts do not persist credentials. +VitePress executes Vue in Markdown during the build, so generated content runs +only here. + +`scripts/docs-proposal/validate.sh .docs-proposal`: + +1. Empty diff: exit successfully. +2. Apply `proposal.patch` to the merge commit and reject the proposal if any + changed path is outside `docs/guide/**` or `docs/index.md`. +3. In `docs/`: `npm ci`, `npm run lint`, `npm run format`, `npm run build`. + Any failure fails the workflow; no pull request is offered. + ### Job `publish` -Permissions: `contents: write`, `pull-requests: write`. Copilot does not run. +Permissions: `contents: write`, `pull-requests: write`. Copilot does not run, +and nothing installs, builds, or executes proposal content. `scripts/docs-proposal/publish.sh .docs-proposal`: @@ -83,22 +109,23 @@ Permissions: `contents: write`, `pull-requests: write`. Copilot does not run. `docs/auto/`, leave it alone and exit successfully: a maintainer's decision is never reopened or recreated. 2. Empty diff: close any open pull request for that branch with a comment, - then exit successfully. Otherwise apply `proposal.patch` to the merge - commit. -3. Reject the proposal if any changed path is outside `docs/guide/**` or + then exit successfully. Otherwise apply `proposal.patch` to a fresh + checkout of the merge commit. +3. Reject the proposal again if any changed path is outside `docs/guide/**` or `docs/index.md`. -4. In `docs/`: `npm ci`, `npm run lint`, `npm run format`, `npm run build`. - Any failure fails the job; no pull request is offered. -5. Commit as `github-actions[bot]` on `docs/auto/`. If the remote - branch already exists and its tree equals the new tree, stop: the proposal - is unchanged and nothing is re-posted. Otherwise force-push. -6. Find the originating pull request with +4. Commit as `github-actions[bot]` on `docs/auto/`. If the remote + branch already exists and its tree equals the new tree, reuse the remote + commit and skip the push, so a retry resumes any later step that failed. + Otherwise force-push. +5. Find the originating pull request with `gh api repos/{owner}/{repo}/commits//pulls`. Create the pull request, or edit the existing one, with a body that links the merge commit and the originating pull request, includes `rationale.md`, and states that a human must verify the prose against the code before merging. -7. `node scripts/docs-proposal/hunks.mjs review` builds review comments and - posts one `COMMENT` review through `gh api`. +6. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing + is re-posted. Otherwise `node scripts/docs-proposal/hunks.mjs review` + builds review comments and posts one `COMMENT` review through `gh api` + against the pushed commit. ## Per-hunk review @@ -130,7 +157,7 @@ Documented in `scripts/README.md`: - Repository setting "Allow GitHub Actions to create and approve pull requests" is enabled. - Pull requests opened with `GITHUB_TOKEN` do not trigger other workflows. - The `publish` job runs the docs gates itself; maintainers re-run regular CI + The `validate` job runs the docs gates itself; maintainers re-run regular CI by pushing to the branch if needed. ## Security @@ -140,7 +167,10 @@ Documented in `scripts/README.md`: permissions. - Input is code already merged to `main`, so no untrusted fork content reaches the agent. -- The path allowlist is enforced by `publish.sh`, not by the prompt. +- The path allowlist is enforced by `validate.sh` and again by `publish.sh`, + not by the prompt. +- Generated Markdown is built only in the `validate` job, which has a + read-only token and no persisted credentials. - Proposed prose must use fictional `example.com` values, per `CLAUDE.md`; the reviewer verifies this before merging. @@ -149,9 +179,11 @@ Documented in `scripts/README.md`: - `node --test scripts/docs-proposal/hunks.test.mjs` covers hunk listing, every row of the per-hunk table, unknown evidence ids, and the review payload. - `scripts/docs-proposal/test.sh` stubs `gh` and `npm` and pushes to a - temporary bare repository to cover the empty diff, closing a stale - proposal, a disallowed path, first publish, an unchanged retry, an updated - proposal, a closed proposal, and PR body rendering. + temporary bare repository to cover validation of empty, disallowed, and + allowed proposals, the empty diff, closing a stale proposal, a disallowed + path, first publish without a build, an unchanged retry, an updated proposal + whose review fails and is resumed without a second push, a closed proposal, + and PR body rendering. - `shellcheck` passes for every script. - The `docs-proposal-scripts` job in `format.yml` runs all three on every pull request. diff --git a/scripts/README.md b/scripts/README.md index 8c2081776..2ce1fc5e2 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -3,24 +3,25 @@ Run these scripts from the repository root. They fail on command errors unless their documented orchestration handles a probe deliberately. -| Script | Inputs and prerequisites | Side effects and cleanup | -| ----------------------------------------- | ------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `batch-sync.sh` | Endpoint, bearer token, EC ID, partner UID; `curl` and Python | Sends one authenticated batch-sync request. Its temporary response file is removed on exit. | -| `benchmark.sh` | A running server; `curl`, `bc`, and optionally `hey` | May install `hey` through Homebrew. `--save` writes under `benchmark-results/`; it does not stop the server. | -| `profile.sh` | Fastly CLI, Rust WASM target, `curl`; endpoint and request options | Builds and starts the Fastly app, stops the owned process, and retains a profile under `benchmark-results/profiles/`. `--open` launches the local viewer. | -| `generate-integration-viceroy-configs.sh` | Rust toolchain; optional origin port and artifact directory | Builds the native generator and writes Viceroy config under `target/integration-test-artifacts/`; generated files persist. | -| `integration-tests.sh` | Docker, Viceroy, Rust WASM target, pinned Node | Builds WASM/native artifacts and two Docker images, generates Viceroy config, and runs native integration tests serially. Build products and images persist. | -| `integration-tests-browser.sh` | The integration prerequisites plus npm and Playwright | Installs package/browser dependencies, builds fixtures and images, runs both browser suites, and stops matching test containers on exit. Build and npm artifacts persist. | -| `smoke-axum.sh` | `cargo`, `curl`, Python | Uses an isolated temporary config/store and stub origin; stops owned processes and removes its workspace. | -| `smoke-fastly.sh` | Fastly CLI, `cargo`, `curl`, Python | Uses an isolated Fastly project and application config; stops owned processes and removes its workspace. | -| `smoke-cloudflare.sh` | Pinned Wrangler, `cargo`, `curl`, `jq`, and Python | Uses isolated Wrangler manifests, KV state, ports, and logs; stops owned processes and removes its workspace. | -| `smoke-spin.sh` | Spin CLI, `cargo`, `curl`, and Python | Uses an isolated Spin manifest and SQLite KV store; stops owned processes and removes its workspace. | -| `smoke-common.sh` | Sourced by the four smoke scripts | Defines bounded port, process, config, secret, and response assertions. Do not invoke it as a standalone smoke. | -| `template-cache-local-test.sh` | Viceroy, Node, OpenSSL, `curl`, `lsof`; optional ports and mode | Builds local artifacts, generates certificates and fixtures in a temporary directory, stops owned servers, and removes the directory. | -| `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | -| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, and evidence into the work dir. Leaves the edits staged. | -| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Applies the patch, runs the docs gates, force-pushes `docs/auto/`, opens or edits its PR, and posts a per-hunk review. Never writes `main`. | -| `docs-proposal/test.sh` | Git and Node | Tests the helpers and publish flow against a temporary repository with stubbed `gh` and `npm`; removes it on exit. | +| Script | Inputs and prerequisites | Side effects and cleanup | +| ----------------------------------------- | -------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `batch-sync.sh` | Endpoint, bearer token, EC ID, partner UID; `curl` and Python | Sends one authenticated batch-sync request. Its temporary response file is removed on exit. | +| `benchmark.sh` | A running server; `curl`, `bc`, and optionally `hey` | May install `hey` through Homebrew. `--save` writes under `benchmark-results/`; it does not stop the server. | +| `profile.sh` | Fastly CLI, Rust WASM target, `curl`; endpoint and request options | Builds and starts the Fastly app, stops the owned process, and retains a profile under `benchmark-results/profiles/`. `--open` launches the local viewer. | +| `generate-integration-viceroy-configs.sh` | Rust toolchain; optional origin port and artifact directory | Builds the native generator and writes Viceroy config under `target/integration-test-artifacts/`; generated files persist. | +| `integration-tests.sh` | Docker, Viceroy, Rust WASM target, pinned Node | Builds WASM/native artifacts and two Docker images, generates Viceroy config, and runs native integration tests serially. Build products and images persist. | +| `integration-tests-browser.sh` | The integration prerequisites plus npm and Playwright | Installs package/browser dependencies, builds fixtures and images, runs both browser suites, and stops matching test containers on exit. Build and npm artifacts persist. | +| `smoke-axum.sh` | `cargo`, `curl`, Python | Uses an isolated temporary config/store and stub origin; stops owned processes and removes its workspace. | +| `smoke-fastly.sh` | Fastly CLI, `cargo`, `curl`, Python | Uses an isolated Fastly project and application config; stops owned processes and removes its workspace. | +| `smoke-cloudflare.sh` | Pinned Wrangler, `cargo`, `curl`, `jq`, and Python | Uses isolated Wrangler manifests, KV state, ports, and logs; stops owned processes and removes its workspace. | +| `smoke-spin.sh` | Spin CLI, `cargo`, `curl`, and Python | Uses an isolated Spin manifest and SQLite KV store; stops owned processes and removes its workspace. | +| `smoke-common.sh` | Sourced by the four smoke scripts | Defines bounded port, process, config, secret, and response assertions. Do not invoke it as a standalone smoke. | +| `template-cache-local-test.sh` | Viceroy, Node, OpenSSL, `curl`, `lsof`; optional ports and mode | Builds local artifacts, generates certificates and fixtures in a temporary directory, stops owned servers, and removes the directory. | +| `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | +| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, and evidence into the work dir. Leaves the edits staged. | +| `docs-proposal/validate.sh` | Merge SHA, work dir from `propose.sh`; Node, `npm`; no write credentials | Applies the patch, rejects paths outside the allowlist, and runs the docs gates. Leaves the edits staged. | +| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, force-pushes `docs/auto/`, opens or edits its PR, and posts a per-hunk review unless that commit already has one. Never writes `main`. | +| `docs-proposal/test.sh` | Git and Node | Tests the helpers, validate flow, and publish flow against a temporary repository with stubbed `gh` and `npm`; removes it on exit. | The four adapter smoke contracts are documented in the [deployment guides](../docs/guide/integrations-overview.md#adapter-support). @@ -31,6 +32,8 @@ merges to `main` and opens one pull request per merge on `docs/auto/`. It needs the organization Copilot policy "Allow use of Copilot CLI billed to the organization" and the repository setting "Allow GitHub Actions to create and approve pull requests". Pull requests opened with `GITHUB_TOKEN` do not -trigger other workflows, so the publish job runs the docs gates itself; push to -the proposal branch to run regular CI. Retry a merge from the workflow's manual -dispatch with its full commit SHA. +trigger other workflows, so the workflow's validate job runs the docs gates +itself, without write credentials; push to the proposal branch to run regular +CI. Retry a merge from the workflow's manual dispatch with its full commit SHA; +the scripts always run from the workflow's revision, so any merge on `main` can +be retried. diff --git a/scripts/docs-proposal/lib.sh b/scripts/docs-proposal/lib.sh index 87fb538d7..52f6d52ce 100644 --- a/scripts/docs-proposal/lib.sh +++ b/scripts/docs-proposal/lib.sh @@ -36,6 +36,20 @@ docs_proposal_disallowed_paths() { return "$found" } +# Applies to the index on a detached checkout of , then exits +# when it changes paths outside the allowlist. +docs_proposal_apply() { + local sha="$1" + local patch="$2" + local disallowed + git switch --quiet --detach "$sha" + git apply --index "$patch" + if ! disallowed="$(git diff --cached --name-only --no-renames "$sha" | docs_proposal_disallowed_paths)"; then + printf '::error::Proposal changes paths outside docs/guide/** and docs/index.md:\n%s\n' "$disallowed" + exit 1 + fi +} + docs_proposal_pr_body() { local sha="$1" local subject="$2" diff --git a/scripts/docs-proposal/prompt.md b/scripts/docs-proposal/prompt.md index 691060fb1..1bde920e8 100644 --- a/scripts/docs-proposal/prompt.md +++ b/scripts/docs-proposal/prompt.md @@ -1,13 +1,15 @@ # Documentation proposal You are updating the public Trusted Server documentation after a commit merged -to `main`. The merged commit and the table mapping implementation sources to -documentation pages appear at the end of this file. +to `main`. The merged commit, its base, and the table mapping implementation +sources to documentation pages appear at the end of this file. ## Task -1. Inspect the merged change with `git show --stat ` and - `git show `. +1. Inspect the merged change from its base to its commit with + `git log --oneline ..`, `git diff --stat `, + and `git diff `. The range covers every commit the push + added; `git show` omits the changes of a clean merge commit. 2. Decide whether it changes behavior that `docs/index.md` or a page under `docs/guide/` describes: configuration fields and defaults, routes, adapter support, integrations, auction behavior, CLI commands and flags, browser diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh index 6e2b6a0b2..01106a767 100755 --- a/scripts/docs-proposal/propose.sh +++ b/scripts/docs-proposal/propose.sh @@ -1,21 +1,28 @@ #!/usr/bin/env bash # Asks Copilot CLI to propose documentation edits for one merged commit. # -# Usage: scripts/docs-proposal/propose.sh -# Writes proposal.patch (empty when nothing changes), rationale.md, and -# evidence.json to , which must be inside the repository so Copilot -# can read the prompts and write its outputs there. +# Usage: scripts/docs-proposal/propose.sh [] +# Run from a checkout of ; this script may live in a separate +# checkout. Copilot inspects .., where +# defaults to the first parent and is the previous main head when one push +# added several commits. Writes proposal.patch (empty when nothing changes), +# rationale.md, and evidence.json to , which must be inside the +# repository so Copilot can read the prompts and write its outputs there. set -euo pipefail sha="$1" here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" root="$(git rev-parse --show-toplevel)" -spec="$root/docs/superpowers/specs/2026-08-19-documentation-refresh-design.md" +spec="$here/../../docs/superpowers/specs/2026-08-19-documentation-refresh-design.md" # shellcheck source=scripts/docs-proposal/lib.sh source "$here/lib.sh" cd "$root" docs_proposal_require_sha "$sha" +base="${3:-}" +if [ -z "$base" ] || [ "$base" = "$sha" ] || ! git merge-base --is-ancestor "$base" "$sha" 2>/dev/null; then + base="$(git rev-parse "$sha^1")" +fi mkdir -p "$2" work_dir="$(cd "$2" && pwd)" relative_dir="${work_dir#"$root"/}" @@ -36,7 +43,7 @@ run_copilot() { { cat "$here/prompt.md" - printf '\n## Merged change\n\n- Commit: %s\n- Subject: %s\n\n' "$sha" "$(git log -1 --format=%s "$sha")" + printf '\n## Merged change\n\n- Commit: %s\n- Base: %s\n- Subject: %s\n\n' "$sha" "$base" "$(git log -1 --format=%s "$sha")" printf '## Sources of truth\n\n' awk '/^## Sources of truth/ { found = 1; next } /^## / { found = 0 } found' "$spec" } > "$work_dir/prompt.md" diff --git a/scripts/docs-proposal/publish.sh b/scripts/docs-proposal/publish.sh index 90fa8c4ff..6f05f070f 100755 --- a/scripts/docs-proposal/publish.sh +++ b/scripts/docs-proposal/publish.sh @@ -1,9 +1,11 @@ #!/usr/bin/env bash -# Validates a documentation proposal and opens or updates its pull request. +# Opens or updates the pull request for a validated documentation proposal. # # Usage: scripts/docs-proposal/publish.sh # holds proposal.patch, rationale.md, and evidence.json from # propose.sh. Requires GH_TOKEN with contents and pull-requests write access. +# Run validate.sh first, in a job without write credentials: this script +# re-checks the path allowlist but never builds or executes proposal content. set -euo pipefail sha="$1" @@ -36,14 +38,7 @@ if [ ! -s "$work_dir/proposal.patch" ]; then exit 0 fi -git switch --quiet --detach "$sha" -git apply --index "$work_dir/proposal.patch" -if ! disallowed="$(git diff --cached --name-only --no-renames "$sha" | docs_proposal_disallowed_paths)"; then - printf '::error::Proposal changes paths outside docs/guide/** and docs/index.md:\n%s\n' "$disallowed" - exit 1 -fi - -(cd docs && npm ci && npm run lint && npm run format && npm run build) +docs_proposal_apply "$sha" "$work_dir/proposal.patch" git -c user.name='github-actions[bot]' \ -c user.email='41898282+github-actions[bot]@users.noreply.github.com' \ @@ -52,17 +47,19 @@ git -c user.name='github-actions[bot]' \ -m "Generated by the docs-proposal workflow from $sha." head_sha="$(git rev-parse HEAD)" -# An identical tree means this is a retry of an already-published proposal; -# skip the push and review so reviewers do not see duplicate comments. -remote_tree="" +# An identical tree means a previous run already pushed this proposal. Reuse +# its commit instead of pushing, so a retry resumes any pull request or review +# step that failed after the push. +remote_head="" if git fetch --quiet origin "refs/heads/$branch" 2>/dev/null; then - remote_tree="$(git rev-parse 'FETCH_HEAD^{tree}')" + remote_head="$(git rev-parse FETCH_HEAD)" fi -if [ -n "$existing" ] && [ "$remote_tree" = "$(git rev-parse 'HEAD^{tree}')" ]; then - printf 'Proposal #%s for %s is unchanged.\n' "$pr_number" "$sha" - exit 0 +if [ -n "$remote_head" ] && [ "$(git rev-parse "$remote_head^{tree}")" = "$(git rev-parse 'HEAD^{tree}')" ]; then + head_sha="$remote_head" + printf 'Proposal branch for %s is unchanged; not pushing.\n' "$sha" +else + git push --quiet --force origin "HEAD:refs/heads/$branch" fi -git push --quiet --force origin "HEAD:refs/heads/$branch" subject="$(git log -1 --format=%s "$sha")" origin_pr="$(gh api "repos/{owner}/{repo}/commits/$sha/pulls" --jq '.[0].number // empty' || true)" @@ -75,6 +72,15 @@ else pr_number="${pr_url##*/}" fi +# Skip the review when this commit already has one, so retries never post +# duplicate comments. +posted_review="$(gh api --paginate "repos/{owner}/{repo}/pulls/$pr_number/reviews" \ + --jq ".[] | select(.commit_id == \"$head_sha\" and .user.login == \"github-actions[bot]\") | .id")" +if [ -n "$posted_review" ]; then + printf 'Proposal #%s for %s is already published.\n' "$pr_number" "$sha" + exit 0 +fi + git diff --no-renames "$sha" "$head_sha" -- docs \ | node "$here/hunks.mjs" review "$work_dir/evidence.json" "$head_sha" > "$work_dir/review.json" gh api --method POST "repos/{owner}/{repo}/pulls/$pr_number/reviews" --input "$work_dir/review.json" > /dev/null diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index 6a6eb1181..311f2d678 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -# Tests the documentation proposal helpers and publish flow with stubbed gh -# and npm against a throwaway git repository. +# Tests the documentation proposal helpers, validate flow, and publish flow +# with stubbed gh and npm against a throwaway git repository. set -euo pipefail here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -70,7 +70,11 @@ case "$1 $2" in "pr list") printf '%s' "${STUB_PR:-}" ;; "pr create") printf 'https://github.com/example/repo/pull/7\n' ;; "api repos/{owner}/{repo}/commits/"*) printf '%s' "${STUB_ORIGIN_PR:-}" ;; + "api --paginate") printf '%s' "${STUB_REVIEWS:-}" ;; "api --method") + if [ -n "${STUB_FAIL_REVIEW:-}" ]; then + exit 1 + fi for arg in "$@"; do last="$arg"; done cp "$last" "$STUB_REVIEW" ;; @@ -118,6 +122,19 @@ run_publish() { fi } +run_validate() { + : > "$STUB_LOG" + if "$here/validate.sh" "$merge_sha" "$tmp/proposal" > "$tmp/out.log" 2>&1; then + printf '0' + else + printf '1' + fi +} + +remote_head() { + git ls-remote origin "refs/heads/$branch" | cut -f1 +} + remote_has_branch() { if git ls-remote --exit-code -q origin "refs/heads/$branch" >/dev/null; then printf 'yes' @@ -126,6 +143,18 @@ remote_has_branch() { fi } +make_patch "" +assert_eq "$(run_validate)" 0 "should validate an empty proposal" +assert_eq "$(grep -c 'npm' "$STUB_LOG" || true)" 0 "should not run the docs gates for an empty proposal" + +make_patch docs/superpowers/notes.md +assert_eq "$(run_validate)" 1 "should reject a disallowed path before the docs gates" +assert_eq "$(grep -c 'npm' "$STUB_LOG" || true)" 0 "should not build a disallowed proposal" + +make_patch docs/guide/cli.md +assert_eq "$(run_validate)" 0 "should validate an allowed proposal" +assert_contains "$(cat "$STUB_LOG")" "npm run build" "should run the docs build" + make_patch "" assert_eq "$(STUB_PR="" run_publish)" 0 "should succeed on an empty proposal" assert_eq "$(grep -c 'pr create\|pr close' "$STUB_LOG" || true)" 0 "should not open or close a PR for an empty diff" @@ -143,16 +172,23 @@ make_patch docs/guide/cli.md assert_eq "$(STUB_PR="" STUB_ORIGIN_PR=42 run_publish)" 0 "should publish a valid proposal" assert_eq "$(remote_has_branch)" yes "should push the proposal branch" assert_contains "$(cat "$STUB_LOG")" "pr create --base main --head $branch" "should open a PR from the proposal branch" -assert_contains "$(cat "$STUB_LOG")" "npm run build" "should run the docs build" +assert_eq "$(grep -c '^npm' "$STUB_LOG" || true)" 0 "should never build proposal content while publishing" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/guide/cli.md"' "should post a review for the hunk" -assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed on an unchanged retry" -assert_eq "$(grep -c 'pr edit\|pr create\|reviews' "$STUB_LOG" || true)" 0 "should not re-post an unchanged proposal" +published_head="$(remote_head)" +assert_eq "$(STUB_PR="7 OPEN" STUB_REVIEWS=101 run_publish)" 0 "should succeed on an unchanged retry" +assert_eq "$(remote_head)" "$published_head" "should not push an unchanged proposal" +assert_eq "$(grep -c 'pr create\|--method POST' "$STUB_LOG" || true)" 0 "should not re-post an unchanged proposal" make_patch docs/index.md -assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should update a changed proposal" +assert_eq "$(STUB_PR="7 OPEN" STUB_FAIL_REVIEW=1 run_publish)" 1 "should fail when the review cannot be posted" +pushed_head="$(remote_head)" assert_contains "$(cat "$STUB_LOG")" "pr edit 7" "should edit the existing PR" + +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should resume a proposal whose review failed" +assert_eq "$(remote_head)" "$pushed_head" "should not push again when resuming" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/index.md"' "should review the new hunk" +assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" "\"commit_id\": \"$pushed_head\"" "should review the pushed commit" assert_eq "$(STUB_PR="7 CLOSED" run_publish)" 0 "should succeed for a closed proposal" assert_eq "$(grep -c 'pr edit\|pr create' "$STUB_LOG" || true)" 0 "should never reopen or recreate a closed proposal" diff --git a/scripts/docs-proposal/validate.sh b/scripts/docs-proposal/validate.sh new file mode 100755 index 000000000..08066718f --- /dev/null +++ b/scripts/docs-proposal/validate.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# Applies a documentation proposal and runs the docs gates against it. +# +# Usage: scripts/docs-proposal/validate.sh +# holds proposal.patch from propose.sh. VitePress executes Vue in +# Markdown during the build, so run this only without write credentials; +# publish.sh never executes proposal content. +set -euo pipefail + +sha="$1" +work_dir="$(cd "$2" && pwd)" +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +root="$(git rev-parse --show-toplevel)" +# shellcheck source=scripts/docs-proposal/lib.sh +source "$here/lib.sh" + +cd "$root" +docs_proposal_require_sha "$sha" + +if [ ! -s "$work_dir/proposal.patch" ]; then + printf 'No documentation changes proposed for %s.\n' "$sha" + exit 0 +fi + +docs_proposal_apply "$sha" "$work_dir/proposal.patch" +(cd docs && npm ci && npm run lint && npm run format && npm run build) +printf 'Validated proposal for %s.\n' "$sha" From e26af27478f82f3db30ed5fd99aecc470aa9a761 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:10:03 +0530 Subject: [PATCH 04/14] Keep maintainer commits on proposal branches across retries A retry for the same merge regenerated the proposal and force-pushed it, discarding commits maintainers added on top, including applied revert suggestions. An empty rerun also closed the pull request and deleted that branch. publish.sh now leaves a branch alone when its head is not the generated proposal commit, and pushes with a lease instead of a bare force. Signed-off-by: dhruv8sh --- ...6-10-01-docs-proposal-automation-design.md | 23 +++++++++++-------- scripts/README.md | 2 +- scripts/docs-proposal/publish.sh | 21 ++++++++++++----- scripts/docs-proposal/test.sh | 18 +++++++++++++++ 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index a8a3c8f53..f15b922eb 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -29,7 +29,7 @@ only ever writes to a dedicated proposal branch and a pull request. | How is prose checked against merged code? | Copilot cites the merged source for every hunk; a per-hunk review exposes the evidence and a one-click revert; a human approves. | | How do sequential or overlapping merges map? | One pull request per merge commit on branch `docs/auto/`; runs are serialized. | | What happens on an empty diff? | No pull request. An existing proposal pull request for that SHA is closed. | -| What prevents duplicates on retry? | The branch name is derived from the merge SHA; a retry force-updates the same branch and edits the same pull request. | +| What prevents duplicates on retry? | The branch name is derived from the merge SHA; a retry updates that branch and pull request, but never over maintainer commits. | ## Workflow shape @@ -108,21 +108,25 @@ and nothing installs, builds, or executes proposal content. 1. If a closed or merged pull request already exists for `docs/auto/`, leave it alone and exit successfully: a maintainer's decision is never reopened or recreated. -2. Empty diff: close any open pull request for that branch with a comment, +2. If the remote branch's head is not the generated proposal commit (its + parent is not the merge commit), a maintainer has committed there, for + example by applying a review suggestion. Warn and exit successfully + without pushing, closing, or deleting anything. +3. Empty diff: close any open pull request for that branch with a comment, then exit successfully. Otherwise apply `proposal.patch` to a fresh checkout of the merge commit. -3. Reject the proposal again if any changed path is outside `docs/guide/**` or +4. Reject the proposal again if any changed path is outside `docs/guide/**` or `docs/index.md`. -4. Commit as `github-actions[bot]` on `docs/auto/`. If the remote +5. Commit as `github-actions[bot]` on `docs/auto/`. If the remote branch already exists and its tree equals the new tree, reuse the remote commit and skip the push, so a retry resumes any later step that failed. - Otherwise force-push. -5. Find the originating pull request with + Otherwise push with a lease on the fetched head. +6. Find the originating pull request with `gh api repos/{owner}/{repo}/commits//pulls`. Create the pull request, or edit the existing one, with a body that links the merge commit and the originating pull request, includes `rationale.md`, and states that a human must verify the prose against the code before merging. -6. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing +7. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing is re-posted. Otherwise `node scripts/docs-proposal/hunks.mjs review` builds review comments and posts one `COMMENT` review through `gh api` against the pushed commit. @@ -182,8 +186,9 @@ Documented in `scripts/README.md`: temporary bare repository to cover validation of empty, disallowed, and allowed proposals, the empty diff, closing a stale proposal, a disallowed path, first publish without a build, an unchanged retry, an updated proposal - whose review fails and is resumed without a second push, a closed proposal, - and PR body rendering. + whose review fails and is resumed without a second push, a retry or empty + rerun that leaves maintainer commits in place, a closed proposal, and PR + body rendering. - `shellcheck` passes for every script. - The `docs-proposal-scripts` job in `format.yml` runs all three on every pull request. diff --git a/scripts/README.md b/scripts/README.md index 2ce1fc5e2..62d6ef448 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -20,7 +20,7 @@ their documented orchestration handles a probe deliberately. | `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | | `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, and evidence into the work dir. Leaves the edits staged. | | `docs-proposal/validate.sh` | Merge SHA, work dir from `propose.sh`; Node, `npm`; no write credentials | Applies the patch, rejects paths outside the allowlist, and runs the docs gates. Leaves the edits staged. | -| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, force-pushes `docs/auto/`, opens or edits its PR, and posts a per-hunk review unless that commit already has one. Never writes `main`. | +| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, pushes `docs/auto/` unless a maintainer added to it, opens or edits its PR, and reviews each new commit per hunk. Never writes `main`. | | `docs-proposal/test.sh` | Git and Node | Tests the helpers, validate flow, and publish flow against a temporary repository with stubbed `gh` and `npm`; removes it on exit. | The four adapter smoke contracts are documented in the diff --git a/scripts/docs-proposal/publish.sh b/scripts/docs-proposal/publish.sh index 6f05f070f..7b53ef541 100755 --- a/scripts/docs-proposal/publish.sh +++ b/scripts/docs-proposal/publish.sh @@ -29,6 +29,18 @@ if [ -n "$existing" ] && [ "$pr_state" != "OPEN" ]; then exit 0 fi +# Maintainers push to the proposal branch, and applying a review suggestion +# commits there too. Never overwrite or delete commits beyond the generated +# proposal, whose only parent is the merge commit. +remote_head="" +if git fetch --quiet origin "refs/heads/$branch" 2>/dev/null; then + remote_head="$(git rev-parse FETCH_HEAD)" +fi +if [ -n "$remote_head" ] && [ "$(git rev-parse "$remote_head^")" != "$sha" ]; then + printf '::warning::Proposal branch %s has commits beyond the generated proposal; not overwriting it.\n' "$branch" + exit 0 +fi + if [ ! -s "$work_dir/proposal.patch" ]; then printf 'No documentation changes proposed for %s.\n' "$sha" if [ -n "$existing" ]; then @@ -49,16 +61,13 @@ head_sha="$(git rev-parse HEAD)" # An identical tree means a previous run already pushed this proposal. Reuse # its commit instead of pushing, so a retry resumes any pull request or review -# step that failed after the push. -remote_head="" -if git fetch --quiet origin "refs/heads/$branch" 2>/dev/null; then - remote_head="$(git rev-parse FETCH_HEAD)" -fi +# step that failed after the push. The lease refuses the push if the branch +# moved after it was fetched. if [ -n "$remote_head" ] && [ "$(git rev-parse "$remote_head^{tree}")" = "$(git rev-parse 'HEAD^{tree}')" ]; then head_sha="$remote_head" printf 'Proposal branch for %s is unchanged; not pushing.\n' "$sha" else - git push --quiet --force origin "HEAD:refs/heads/$branch" + git push --quiet --force-with-lease="refs/heads/$branch:$remote_head" origin "HEAD:refs/heads/$branch" fi subject="$(git log -1 --format=%s "$sha")" diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index 311f2d678..876af4e7c 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -190,6 +190,24 @@ assert_eq "$(remote_head)" "$pushed_head" "should not push again when resuming" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/index.md"' "should review the new hunk" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" "\"commit_id\": \"$pushed_head\"" "should review the pushed commit" +git fetch -q origin "refs/heads/$branch" +git switch -q --detach FETCH_HEAD +printf 'maintainer edit\n' >> docs/guide/cli.md +git commit -q -am "Apply suggestion from review" +git push -q origin "HEAD:refs/heads/$branch" +maintainer_head="$(remote_head)" + +make_patch docs/guide/cli.md +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed when a maintainer committed to the proposal" +assert_eq "$(remote_head)" "$maintainer_head" "should not overwrite maintainer commits" +assert_contains "$(cat "$tmp/out.log")" "commits beyond the generated proposal" "should warn about maintainer commits" +assert_eq "$(grep -c 'pr edit\|--method POST' "$STUB_LOG" || true)" 0 "should leave a maintained proposal untouched" + +make_patch "" +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed on an empty rerun of a maintained proposal" +assert_eq "$(grep -c 'pr close' "$STUB_LOG" || true)" 0 "should not close or delete a maintained proposal" +assert_eq "$(remote_head)" "$maintainer_head" "should keep the maintained proposal branch" + assert_eq "$(STUB_PR="7 CLOSED" run_publish)" 0 "should succeed for a closed proposal" assert_eq "$(grep -c 'pr edit\|pr create' "$STUB_LOG" || true)" 0 "should never reopen or recreate a closed proposal" From 8ce9d089e59016b072ce0bcdcaf5680ee7fc9584 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:10:50 +0530 Subject: [PATCH 05/14] Stop allowing Copilot to run git grep git grep --open-files-in-pager (or -O) runs an arbitrary command, so allowing shell(git grep:*) let content in the merged change steer Copilot into executing commands in the propose job. Copilot's built-in file reads cover what git grep was used for. Signed-off-by: dhruv8sh --- .../specs/2026-10-01-docs-proposal-automation-design.md | 5 +++-- scripts/docs-proposal/propose.sh | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index f15b922eb..5053f9616 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -69,8 +69,9 @@ Permissions: `contents: read`, `copilot-requests: write`. `git diff `: a multi-commit push is covered in full, and a clean merge commit is not hidden by `git show`'s empty combined diff. 2. Runs `copilot -p` with `--no-ask-user`, `--allow-tool=write`, and - `git show`, `git diff`, `git log`, and `git grep` shell tools; `git push` - and `git commit` are denied, and file reads need no permission. Copilot + `git show`, `git diff`, and `git log` shell tools; `git push` and + `git commit` are denied, and file reads need no permission. `git grep` + is not allowed because `--open-files-in-pager` runs a command. Copilot edits documentation only when the merged change alters reader-facing behavior, and writes `.docs-proposal/rationale.md`. 3. Runs `npm ci` and `npm run format:write` in `docs/` so Prettier diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh index 01106a767..bf96fcc7c 100755 --- a/scripts/docs-proposal/propose.sh +++ b/scripts/docs-proposal/propose.sh @@ -36,7 +36,6 @@ run_copilot() { --allow-tool='shell(git show:*)' \ --allow-tool='shell(git diff:*)' \ --allow-tool='shell(git log:*)' \ - --allow-tool='shell(git grep:*)' \ --deny-tool='shell(git push)' \ --deny-tool='shell(git commit)' } From d0710fe91a753d70459580e67fba41464a30e2ab Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:11:24 +0530 Subject: [PATCH 06/14] Drop the npm cache from jobs that run untrusted content Copilot can write files in the propose job that propose.sh then executes, and validate builds generated Markdown that VitePress runs as Vue. Either job could therefore save a poisoned main-scoped setup-node cache entry that later runs restore. Neither job restores or saves a dependency cache now, and the design spells out that the job, not the tool allowlist, is the trust boundary. Signed-off-by: dhruv8sh --- .github/workflows/docs-proposal.yml | 8 +++----- .../specs/2026-10-01-docs-proposal-automation-design.md | 7 ++++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docs-proposal.yml b/.github/workflows/docs-proposal.yml index bb5b9cb3d..ab67a9456 100644 --- a/.github/workflows/docs-proposal.yml +++ b/.github/workflows/docs-proposal.yml @@ -40,6 +40,8 @@ env: jobs: # Copilot runs here with a read-only token; it cannot push or open PRs. + # Copilot can write files that later steps execute, so treat this job as + # untrusted: it uses no dependency cache that later runs would restore. propose: runs-on: ubuntu-latest permissions: @@ -69,8 +71,6 @@ jobs: uses: actions/setup-node@v4 with: node-version: ${{ steps.node-version.outputs.node-version }} - cache: "npm" - cache-dependency-path: source/docs/package-lock.json - name: Install Copilot CLI run: npm install -g @github/copilot@1.0.90 @@ -90,7 +90,7 @@ jobs: if-no-files-found: error # VitePress executes Vue in Markdown during the build, so the docs gates run - # here with no write token or persisted credentials. + # here with no write token, persisted credentials, or dependency cache. validate: needs: propose runs-on: ubuntu-latest @@ -120,8 +120,6 @@ jobs: uses: actions/setup-node@v4 with: node-version: ${{ steps.node-version.outputs.node-version }} - cache: "npm" - cache-dependency-path: source/docs/package-lock.json - name: Download proposal uses: actions/download-artifact@v4 diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index 5053f9616..a0c945f02 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -167,9 +167,14 @@ Documented in `scripts/README.md`: ## Security -- Copilot runs with a read-only `contents` token and an allowlist of read-only +- Copilot runs with a read-only `contents` token and an allowlist of git shell tools; it has no network shell, no push, and no pull request permissions. +- The tool allowlist is not the boundary. Copilot can write any file in the + checkout, and `propose.sh` then runs `npm` in `docs/`, so the `propose` job + is treated as able to run arbitrary code. Its token is read-only, neither it + nor `validate` restores or saves a dependency cache, and its artifact is + re-checked by `validate.sh` and `publish.sh`. - Input is code already merged to `main`, so no untrusted fork content reaches the agent. - The path allowlist is enforced by `validate.sh` and again by `publish.sh`, From 5c3fd10478950afc4840fa6bc1ffe4409a4dd683 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:11:37 +0530 Subject: [PATCH 07/14] Bound documentation proposal jobs with timeouts Copilot CLI has no limit of its own, so a hung run inherited the 360-minute default and, with the concurrency queue, delayed every later merge's proposal. The propose, validate, and publish jobs now time out after 30, 15, and 10 minutes. Signed-off-by: dhruv8sh --- .github/workflows/docs-proposal.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs-proposal.yml b/.github/workflows/docs-proposal.yml index ab67a9456..be5fd7a66 100644 --- a/.github/workflows/docs-proposal.yml +++ b/.github/workflows/docs-proposal.yml @@ -24,7 +24,8 @@ on: permissions: {} # Sequential merges are proposed one at a time, in order. The default queue -# keeps only one pending run and cancels older ones, so keep them all. +# keeps only one pending run and cancels older ones, so keep them all. Every +# job has a timeout so a hung run cannot hold the queue for hours. concurrency: group: docs-proposal cancel-in-progress: false @@ -44,6 +45,7 @@ jobs: # untrusted: it uses no dependency cache that later runs would restore. propose: runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: read copilot-requests: write @@ -94,6 +96,7 @@ jobs: validate: needs: propose runs-on: ubuntu-latest + timeout-minutes: 15 permissions: contents: read steps: @@ -136,6 +139,7 @@ jobs: publish: needs: validate runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: write pull-requests: write From 17a09389f129073bc6cebd649a0cbaecdc1f7c95 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:12:41 +0530 Subject: [PATCH 08/14] Neutralize mentions in the proposal rationale Evidence already had its @ mentions broken because it is model output, but up to 20 KB of Copilot's rationale went into the pull request body verbatim, so a rationale naming a team would ping it on every proposal. hunks.mjs gains a sanitize subcommand that inserts a zero width space after each @, which the PR body helper now pipes the rationale through. Signed-off-by: dhruv8sh --- ...6-10-01-docs-proposal-automation-design.md | 5 ++-- scripts/docs-proposal/hunks.mjs | 23 +++++++++++++------ scripts/docs-proposal/hunks.test.mjs | 11 +++++++++ scripts/docs-proposal/lib.sh | 5 +++- scripts/docs-proposal/test.sh | 3 ++- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index a0c945f02..f62be70fc 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -125,8 +125,9 @@ and nothing installs, builds, or executes proposal content. 6. Find the originating pull request with `gh api repos/{owner}/{repo}/commits//pulls`. Create the pull request, or edit the existing one, with a body that links the merge commit and the - originating pull request, includes `rationale.md`, and states that a human - must verify the prose against the code before merging. + originating pull request, includes `rationale.md` with its `@` mentions + neutralized, and states that a human must verify the prose against the + code before merging. 7. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing is re-posted. Otherwise `node scripts/docs-proposal/hunks.mjs review` builds review comments and posts one `COMMENT` review through `gh api` diff --git a/scripts/docs-proposal/hunks.mjs b/scripts/docs-proposal/hunks.mjs index 5ad98d7c4..132b1360f 100644 --- a/scripts/docs-proposal/hunks.mjs +++ b/scripts/docs-proposal/hunks.mjs @@ -146,15 +146,19 @@ export function revertSuggestion(hunk) { const MAX_EVIDENCE_LENGTH = 300; -// Model output lands in review comments, so strip anything that could break -// out of the inline code span or ping a user or team. +// Model output lands in pull request bodies and review comments, so a zero +// width space after every `@` keeps it from pinging a user or team. +export function neutralizeMentions(text) { + return text.replace(/@/g, "@​"); +} + +// Evidence also lands in an inline code span, so strip anything that could +// break out of it. function sanitize(value) { if (typeof value !== "string") { return ""; } - return value - .replace(/[`\r\n]+/g, " ") - .replace(/@/g, "@​") + return neutralizeMentions(value.replace(/[`\r\n]+/g, " ")) .replace(/\s+/g, " ") .trim() .slice(0, MAX_EVIDENCE_LENGTH); @@ -255,7 +259,12 @@ function readEvidence(path) { } function main([command, evidencePath, commitId]) { - const files = parseDiff(readFileSync(0, "utf8")); + const input = readFileSync(0, "utf8"); + if (command === "sanitize") { + process.stdout.write(neutralizeMentions(input)); + return; + } + const files = parseDiff(input); if (command === "list") { process.stdout.write(`${JSON.stringify(listHunks(files), null, 2)}\n`); return; @@ -266,7 +275,7 @@ function main([command, evidencePath, commitId]) { return; } process.stderr.write( - "usage: hunks.mjs list | review \n", + "usage: hunks.mjs list | review | sanitize\n", ); process.exitCode = 2; } diff --git a/scripts/docs-proposal/hunks.test.mjs b/scripts/docs-proposal/hunks.test.mjs index 09440708e..e4a8b2df9 100644 --- a/scripts/docs-proposal/hunks.test.mjs +++ b/scripts/docs-proposal/hunks.test.mjs @@ -6,6 +6,7 @@ import { collectHunks, indexEvidence, listHunks, + neutralizeMentions, parseDiff, revertSuggestion, } from "./hunks.mjs"; @@ -197,6 +198,16 @@ test("indexEvidence keeps valid entries and sanitizes text", () => { assert.equal(indexEvidence(null).size, 0, "should accept missing evidence"); }); +test("neutralizeMentions keeps text but breaks every mention", () => { + assert.equal( + neutralizeMentions( + "- docs/guide/cli.md: ask @example-org/team\nor @user.\n", + ), + "- docs/guide/cli.md: ask @\u200bexample-org/team\nor @\u200buser.\n", + "should insert a zero width space after each @ and keep newlines", + ); +}); + test("buildReview cites evidence and suggests reverts", () => { const files = parseDiff( [ diff --git a/scripts/docs-proposal/lib.sh b/scripts/docs-proposal/lib.sh index 52f6d52ce..ee3d0c60d 100644 --- a/scripts/docs-proposal/lib.sh +++ b/scripts/docs-proposal/lib.sh @@ -2,6 +2,7 @@ # Shared helpers for the documentation proposal scripts. Source this file; # do not execute it. +DOCS_PROPOSAL_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DOCS_PROPOSAL_ALLOWED_RE='^docs/(guide/.+|index\.md)$' DOCS_PROPOSAL_MAX_RATIONALE_BYTES=20000 @@ -64,7 +65,9 @@ docs_proposal_pr_body() { printf 'Generated by GitHub Copilot CLI in the docs-proposal workflow. A maintainer must verify every hunk against the merged code before merging. Each hunk has a review comment citing its source and a suggestion that reverts it.\n\n' printf '## Rationale\n\n' if [ -s "$rationale" ]; then - head -c "$DOCS_PROPOSAL_MAX_RATIONALE_BYTES" "$rationale" + # The rationale is model output; keep it from pinging users or teams. + head -c "$DOCS_PROPOSAL_MAX_RATIONALE_BYTES" "$rationale" \ + | node "$DOCS_PROPOSAL_DIR/hunks.mjs" sanitize printf '\n' else printf '_Copilot did not provide a rationale._\n' diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index 876af4e7c..ff6092351 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -38,11 +38,12 @@ fi assert_eq "$status" 0 "should accept guide and nested guide paths" rationale="$(mktemp)" -printf -- '- docs/guide/cli.md: documents the new flag.\n' > "$rationale" +printf -- '- docs/guide/cli.md: documents the new flag for @example-org/maintainers.\n' > "$rationale" body="$(docs_proposal_pr_body "$sha" "Add example flag" 42 "$rationale")" assert_contains "$body" "" "should embed the SHA marker" assert_contains "$body" "merged change $sha (#42): Add example flag" "should link the merge and its PR" assert_contains "$body" "documents the new flag" "should include the rationale" +assert_contains "$body" $'@\u200bexample-org/maintainers' "should neutralize mentions in the rationale" : > "$rationale" body="$(docs_proposal_pr_body "$sha" "Add example flag" "" "$rationale")" assert_contains "$body" "merged change $sha: Add example flag" "should omit a missing origin PR" From f9f47ee5935d7d42573f80f50e824a6b678a1f24 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:15:30 +0530 Subject: [PATCH 09/14] Refuse proposal retries that inspect a different range A manual dispatch had no base, so retrying a multi-commit push such as a rebase merge narrowed the inspected range to the final commit. The retry could then miss documented behavior, or produce an empty patch that closed and deleted a valid proposal. The dispatch gains an optional base input. propose.sh records the resolved base in the artifact and warns when it discards a supplied base. publish.sh records the base in the pull request body and fails, naming the original base, when an open proposal was generated from a different range. Signed-off-by: dhruv8sh --- .github/workflows/docs-proposal.yml | 11 ++- ...6-10-01-docs-proposal-automation-design.md | 26 +++--- scripts/README.md | 9 +- scripts/docs-proposal/lib.sh | 9 +- scripts/docs-proposal/propose.sh | 10 ++- scripts/docs-proposal/publish.sh | 23 +++-- scripts/docs-proposal/test.sh | 83 ++++++++++++++++--- 7 files changed, 135 insertions(+), 36 deletions(-) diff --git a/.github/workflows/docs-proposal.yml b/.github/workflows/docs-proposal.yml index be5fd7a66..4d8f4aecc 100644 --- a/.github/workflows/docs-proposal.yml +++ b/.github/workflows/docs-proposal.yml @@ -20,6 +20,12 @@ on: description: Full SHA of a commit on main to propose documentation for required: true type: string + base: + description: >- + Full SHA of main before the push that added the commit, when that + push added several commits; defaults to the commit's first parent + required: false + type: string permissions: {} @@ -34,10 +40,11 @@ concurrency: # Each job runs the proposal scripts from the workflow's own revision in # `tools/` against a checkout of the merge commit in `source/`, so retrying a # merge that predates this tooling, or a later fix to it, still works. A push -# that adds several commits is inspected from the previous main head. +# that adds several commits is inspected from the previous main head; a manual +# retry of such a push needs that head as its `base` input. env: MERGE_SHA: ${{ inputs.sha || github.sha }} - BASE_SHA: ${{ github.event_name == 'push' && github.event.before || '' }} + BASE_SHA: ${{ github.event_name == 'push' && github.event.before || inputs.base }} jobs: # Copilot runs here with a read-only token; it cannot push or open PRs. diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index f62be70fc..e70a78c91 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -65,7 +65,8 @@ Permissions: `contents: read`, `copilot-requests: write`. 1. Renders `.docs-proposal/prompt.md` from `scripts/docs-proposal/prompt.md`, the merge SHA, its base, its subject, and the Sources-of-truth table from the refresh design. The base is the push's previous `main` head, or the - first parent for a manual dispatch, so Copilot inspects + manual dispatch's optional `base` input, falling back to the first + parent when it is missing or not an ancestor, so Copilot inspects `git diff `: a multi-commit push is covered in full, and a clean merge commit is not hidden by `git show`'s empty combined diff. 2. Runs `copilot -p` with `--no-ask-user`, `--allow-tool=write`, and @@ -109,26 +110,30 @@ and nothing installs, builds, or executes proposal content. 1. If a closed or merged pull request already exists for `docs/auto/`, leave it alone and exit successfully: a maintainer's decision is never reopened or recreated. -2. If the remote branch's head is not the generated proposal commit (its +2. If an open pull request records a different base in its body, fail: the + retry inspected another range, such as a manual dispatch without the base + of a multi-commit push, and must not narrow or close the proposal. The + error names the recorded base to rerun with. +3. If the remote branch's head is not the generated proposal commit (its parent is not the merge commit), a maintainer has committed there, for example by applying a review suggestion. Warn and exit successfully without pushing, closing, or deleting anything. -3. Empty diff: close any open pull request for that branch with a comment, +4. Empty diff: close any open pull request for that branch with a comment, then exit successfully. Otherwise apply `proposal.patch` to a fresh checkout of the merge commit. -4. Reject the proposal again if any changed path is outside `docs/guide/**` or +5. Reject the proposal again if any changed path is outside `docs/guide/**` or `docs/index.md`. -5. Commit as `github-actions[bot]` on `docs/auto/`. If the remote +6. Commit as `github-actions[bot]` on `docs/auto/`. If the remote branch already exists and its tree equals the new tree, reuse the remote commit and skip the push, so a retry resumes any later step that failed. Otherwise push with a lease on the fetched head. -6. Find the originating pull request with +7. Find the originating pull request with `gh api repos/{owner}/{repo}/commits//pulls`. Create the pull request, or edit the existing one, with a body that links the merge commit and the - originating pull request, includes `rationale.md` with its `@` mentions + originating pull request, records the inspected base, includes `rationale.md` with its `@` mentions neutralized, and states that a human must verify the prose against the code before merging. -7. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing +8. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing is re-posted. Otherwise `node scripts/docs-proposal/hunks.mjs review` builds review comments and posts one `COMMENT` review through `gh api` against the pushed commit. @@ -194,8 +199,9 @@ Documented in `scripts/README.md`: allowed proposals, the empty diff, closing a stale proposal, a disallowed path, first publish without a build, an unchanged retry, an updated proposal whose review fails and is resumed without a second push, a retry or empty - rerun that leaves maintainer commits in place, a closed proposal, and PR - body rendering. + rerun that leaves maintainer commits in place, retries over a different or + unrecorded range, a closed proposal, PR body rendering, and how + `propose.sh` resolves the base of a multi-commit push. - `shellcheck` passes for every script. - The `docs-proposal-scripts` job in `format.yml` runs all three on every pull request. diff --git a/scripts/README.md b/scripts/README.md index 62d6ef448..26ca19b13 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -18,10 +18,10 @@ their documented orchestration handles a probe deliberately. | `smoke-common.sh` | Sourced by the four smoke scripts | Defines bounded port, process, config, secret, and response assertions. Do not invoke it as a standalone smoke. | | `template-cache-local-test.sh` | Viceroy, Node, OpenSSL, `curl`, `lsof`; optional ports and mode | Builds local artifacts, generates certificates and fixtures in a temporary directory, stops owned servers, and removes the directory. | | `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | -| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, and evidence into the work dir. Leaves the edits staged. | +| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, evidence, and base into the work dir. Leaves the edits staged. | | `docs-proposal/validate.sh` | Merge SHA, work dir from `propose.sh`; Node, `npm`; no write credentials | Applies the patch, rejects paths outside the allowlist, and runs the docs gates. Leaves the edits staged. | | `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, pushes `docs/auto/` unless a maintainer added to it, opens or edits its PR, and reviews each new commit per hunk. Never writes `main`. | -| `docs-proposal/test.sh` | Git and Node | Tests the helpers, validate flow, and publish flow against a temporary repository with stubbed `gh` and `npm`; removes it on exit. | +| `docs-proposal/test.sh` | Git and Node | Tests the helpers and the propose, validate, and publish flows against a temporary repository with stubbed `copilot`, `gh`, and `npm`; removes it on exit. | The four adapter smoke contracts are documented in the [deployment guides](../docs/guide/integrations-overview.md#adapter-support). @@ -36,4 +36,7 @@ trigger other workflows, so the workflow's validate job runs the docs gates itself, without write credentials; push to the proposal branch to run regular CI. Retry a merge from the workflow's manual dispatch with its full commit SHA; the scripts always run from the workflow's revision, so any merge on `main` can -be retried. +be retried. When the original push added several commits, as a rebase merge +does, also pass its `base`: the `main` head before that push, shown as +`BASE_SHA` in the original run and recorded in the proposal's body. Publication +refuses a retry whose range differs from an open proposal's. diff --git a/scripts/docs-proposal/lib.sh b/scripts/docs-proposal/lib.sh index ee3d0c60d..f1668987a 100644 --- a/scripts/docs-proposal/lib.sh +++ b/scripts/docs-proposal/lib.sh @@ -53,15 +53,18 @@ docs_proposal_apply() { docs_proposal_pr_body() { local sha="$1" - local subject="$2" - local origin_pr="$3" - local rationale="$4" + local base="$2" + local subject="$3" + local origin_pr="$4" + local rationale="$5" local origin="" if [ -n "$origin_pr" ]; then origin=" (#$origin_pr)" fi printf '\n' "$sha" + printf '\n' "$base" printf 'Proposed documentation update for merged change %s%s: %s\n\n' "$sha" "$origin" "$subject" + printf 'Copilot inspected %s..%s.\n\n' "${base:0:12}" "${sha:0:12}" printf 'Generated by GitHub Copilot CLI in the docs-proposal workflow. A maintainer must verify every hunk against the merged code before merging. Each hunk has a review comment citing its source and a suggestion that reverts it.\n\n' printf '## Rationale\n\n' if [ -s "$rationale" ]; then diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh index bf96fcc7c..b0dbc694f 100755 --- a/scripts/docs-proposal/propose.sh +++ b/scripts/docs-proposal/propose.sh @@ -6,8 +6,9 @@ # checkout. Copilot inspects .., where # defaults to the first parent and is the previous main head when one push # added several commits. Writes proposal.patch (empty when nothing changes), -# rationale.md, and evidence.json to , which must be inside the -# repository so Copilot can read the prompts and write its outputs there. +# rationale.md, evidence.json, and the resolved base to , which must +# be inside the repository so Copilot can read the prompts and write its +# outputs there. set -euo pipefail sha="$1" @@ -21,10 +22,15 @@ cd "$root" docs_proposal_require_sha "$sha" base="${3:-}" if [ -z "$base" ] || [ "$base" = "$sha" ] || ! git merge-base --is-ancestor "$base" "$sha" 2>/dev/null; then + if [ -n "$base" ] && [ "$base" != "0000000000000000000000000000000000000000" ]; then + printf '::warning::Base %s is not an ancestor of %s; using its first parent.\n' "$base" "$sha" + fi base="$(git rev-parse "$sha^1")" fi mkdir -p "$2" work_dir="$(cd "$2" && pwd)" +# publish.sh records the base so a retry over a different range is refused. +printf '%s\n' "$base" > "$work_dir/base" relative_dir="${work_dir#"$root"/}" # Reads are always permitted; writes are allowed so Copilot can edit pages, diff --git a/scripts/docs-proposal/publish.sh b/scripts/docs-proposal/publish.sh index 7b53ef541..eb1ec82b0 100755 --- a/scripts/docs-proposal/publish.sh +++ b/scripts/docs-proposal/publish.sh @@ -2,7 +2,7 @@ # Opens or updates the pull request for a validated documentation proposal. # # Usage: scripts/docs-proposal/publish.sh -# holds proposal.patch, rationale.md, and evidence.json from +# holds proposal.patch, rationale.md, evidence.json, and base from # propose.sh. Requires GH_TOKEN with contents and pull-requests write access. # Run validate.sh first, in a job without write credentials: this script # re-checks the path allowlist but never builds or executes proposal content. @@ -18,17 +18,30 @@ source "$here/lib.sh" cd "$root" docs_proposal_require_sha "$sha" branch="$(docs_proposal_branch "$sha")" +base="$(cat "$work_dir/base")" +if ! [[ "$base" =~ ^[0-9a-f]{40}$ ]]; then + printf '::error::Expected a full base SHA in %s/base, got %s\n' "$work_dir" "$base" >&2 + exit 1 +fi # A closed or merged proposal is a maintainer decision; never reopen or # recreate it on a retry. -existing="$(gh pr list --head "$branch" --state all --json number,state --jq '.[0] | select(.) | "\(.number) \(.state)"')" -pr_number="${existing%% *}" -pr_state="${existing#* }" +existing="$(gh pr list --head "$branch" --state all --json number,state,body \ + --jq '.[0] | select(.) | "\(.number) \(.state) \([.body | match("").captures[0].string] | first // "")"')" +read -r pr_number pr_state pr_base <<< "$existing" || true if [ -n "$existing" ] && [ "$pr_state" != "OPEN" ]; then printf 'Proposal #%s for %s is %s; not reopening it.\n' "$pr_number" "$sha" "$pr_state" exit 0 fi +# A retry that inspected a different range, such as a manual dispatch without +# the base of a multi-commit push, must not narrow or close the proposal. +if [ -n "$existing" ] && [ "$pr_base" != "$base" ]; then + printf '::error::Proposal #%s for %s covers %s..%s, but this run inspected %s..%s. Rerun the workflow with base %s.\n' \ + "$pr_number" "$sha" "${pr_base:-an unrecorded base}" "$sha" "$base" "$sha" "${pr_base:-}" >&2 + exit 1 +fi + # Maintainers push to the proposal branch, and applying a review suggestion # commits there too. Never overwrite or delete commits beyond the generated # proposal, whose only parent is the merge commit. @@ -73,7 +86,7 @@ fi subject="$(git log -1 --format=%s "$sha")" origin_pr="$(gh api "repos/{owner}/{repo}/commits/$sha/pulls" --jq '.[0].number // empty' || true)" title="$(printf 'Update documentation for %s' "$subject" | cut -c1-200)" -docs_proposal_pr_body "$sha" "$subject" "$origin_pr" "$work_dir/rationale.md" > "$work_dir/pr-body.md" +docs_proposal_pr_body "$sha" "$base" "$subject" "$origin_pr" "$work_dir/rationale.md" > "$work_dir/pr-body.md" if [ -n "$existing" ]; then gh pr edit "$pr_number" --title "$title" --body-file "$work_dir/pr-body.md" else diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index ff6092351..0d265d3f4 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash -# Tests the documentation proposal helpers, validate flow, and publish flow -# with stubbed gh and npm against a throwaway git repository. +# Tests the documentation proposal helpers and the propose, validate, and +# publish flows with stubbed copilot, gh, and npm against a throwaway git +# repository. set -euo pipefail here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -24,6 +25,7 @@ assert_contains() { } sha=0123456789abcdef0123456789abcdef01234567 +base_sha=fedcba9876543210fedcba9876543210fedcba98 assert_eq "$(docs_proposal_branch "$sha")" "docs/auto/0123456789ab" "should derive the branch from the SHA" @@ -39,13 +41,15 @@ assert_eq "$status" 0 "should accept guide and nested guide paths" rationale="$(mktemp)" printf -- '- docs/guide/cli.md: documents the new flag for @example-org/maintainers.\n' > "$rationale" -body="$(docs_proposal_pr_body "$sha" "Add example flag" 42 "$rationale")" +body="$(docs_proposal_pr_body "$sha" "$base_sha" "Add example flag" 42 "$rationale")" assert_contains "$body" "" "should embed the SHA marker" +assert_contains "$body" "" "should embed the base marker" +assert_contains "$body" "inspected fedcba987654..0123456789ab" "should show the inspected range" assert_contains "$body" "merged change $sha (#42): Add example flag" "should link the merge and its PR" assert_contains "$body" "documents the new flag" "should include the rationale" assert_contains "$body" $'@\u200bexample-org/maintainers' "should neutralize mentions in the rationale" : > "$rationale" -body="$(docs_proposal_pr_body "$sha" "Add example flag" "" "$rationale")" +body="$(docs_proposal_pr_body "$sha" "$base_sha" "Add example flag" "" "$rationale")" assert_contains "$body" "merged change $sha: Add example flag" "should omit a missing origin PR" assert_contains "$body" "did not provide a rationale" "should note an empty rationale" rm -f "$rationale" @@ -85,7 +89,11 @@ cat > "$tmp/bin/npm" <<'STUB' #!/usr/bin/env bash printf 'npm %s\n' "$*" >> "$STUB_LOG" STUB -chmod +x "$tmp/bin/gh" "$tmp/bin/npm" +cat > "$tmp/bin/copilot" <<'STUB' +#!/usr/bin/env bash +printf 'copilot %s\n' "$*" >> "$STUB_LOG" +STUB +chmod +x "$tmp/bin/gh" "$tmp/bin/npm" "$tmp/bin/copilot" export PATH="$tmp/bin:$PATH" STUB_LOG="$tmp/gh.log" STUB_REVIEW="$tmp/review.json" git init -q --bare "$tmp/origin.git" @@ -111,6 +119,7 @@ make_patch() { fi printf -- '- docs/guide/cli.md: example rationale.\n' > "$tmp/proposal/rationale.md" printf '{}\n' > "$tmp/proposal/evidence.json" + printf '%s\n' "${2:-$base_sha}" > "$tmp/proposal/base" } run_publish() { @@ -161,7 +170,7 @@ assert_eq "$(STUB_PR="" run_publish)" 0 "should succeed on an empty proposal" assert_eq "$(grep -c 'pr create\|pr close' "$STUB_LOG" || true)" 0 "should not open or close a PR for an empty diff" make_patch "" -assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed when closing a stale proposal" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should succeed when closing a stale proposal" assert_contains "$(cat "$STUB_LOG")" "pr close 7" "should close the open proposal for an empty diff" make_patch docs/superpowers/notes.md @@ -177,16 +186,30 @@ assert_eq "$(grep -c '^npm' "$STUB_LOG" || true)" 0 "should never build proposal assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/guide/cli.md"' "should post a review for the hunk" published_head="$(remote_head)" -assert_eq "$(STUB_PR="7 OPEN" STUB_REVIEWS=101 run_publish)" 0 "should succeed on an unchanged retry" +assert_eq "$(STUB_PR="7 OPEN $base_sha" STUB_REVIEWS=101 run_publish)" 0 "should succeed on an unchanged retry" assert_eq "$(remote_head)" "$published_head" "should not push an unchanged proposal" assert_eq "$(grep -c 'pr create\|--method POST' "$STUB_LOG" || true)" 0 "should not re-post an unchanged proposal" +narrow_base=0000000000000000000000000000000000000001 +make_patch "" "$narrow_base" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 1 "should fail an empty retry over a different range" +assert_eq "$(grep -c 'pr close' "$STUB_LOG" || true)" 0 "should not close a proposal from a narrower range" +assert_contains "$(cat "$tmp/out.log")" "Rerun the workflow with base $base_sha" "should name the original base" + +make_patch docs/index.md "$narrow_base" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 1 "should fail a retry over a different range" +assert_eq "$(remote_head)" "$published_head" "should not replace a proposal from a different range" + make_patch docs/index.md -assert_eq "$(STUB_PR="7 OPEN" STUB_FAIL_REVIEW=1 run_publish)" 1 "should fail when the review cannot be posted" +assert_eq "$(STUB_PR="7 OPEN" run_publish)" 1 "should fail a retry of a proposal with no recorded base" +assert_eq "$(remote_head)" "$published_head" "should not replace a proposal with no recorded base" + +make_patch docs/index.md +assert_eq "$(STUB_PR="7 OPEN $base_sha" STUB_FAIL_REVIEW=1 run_publish)" 1 "should fail when the review cannot be posted" pushed_head="$(remote_head)" assert_contains "$(cat "$STUB_LOG")" "pr edit 7" "should edit the existing PR" -assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should resume a proposal whose review failed" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should resume a proposal whose review failed" assert_eq "$(remote_head)" "$pushed_head" "should not push again when resuming" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/index.md"' "should review the new hunk" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" "\"commit_id\": \"$pushed_head\"" "should review the pushed commit" @@ -199,19 +222,57 @@ git push -q origin "HEAD:refs/heads/$branch" maintainer_head="$(remote_head)" make_patch docs/guide/cli.md -assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed when a maintainer committed to the proposal" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should succeed when a maintainer committed to the proposal" assert_eq "$(remote_head)" "$maintainer_head" "should not overwrite maintainer commits" assert_contains "$(cat "$tmp/out.log")" "commits beyond the generated proposal" "should warn about maintainer commits" assert_eq "$(grep -c 'pr edit\|--method POST' "$STUB_LOG" || true)" 0 "should leave a maintained proposal untouched" make_patch "" -assert_eq "$(STUB_PR="7 OPEN" run_publish)" 0 "should succeed on an empty rerun of a maintained proposal" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should succeed on an empty rerun of a maintained proposal" assert_eq "$(grep -c 'pr close' "$STUB_LOG" || true)" 0 "should not close or delete a maintained proposal" assert_eq "$(remote_head)" "$maintainer_head" "should keep the maintained proposal branch" assert_eq "$(STUB_PR="7 CLOSED" run_publish)" 0 "should succeed for a closed proposal" assert_eq "$(grep -c 'pr edit\|pr create' "$STUB_LOG" || true)" 0 "should never reopen or recreate a closed proposal" +# A push that added two commits is inspected from the previous main head; a +# retry without that base falls back to the first parent, which publish.sh +# then refuses to apply over the original proposal. +git switch -q --detach "$merge_sha" +printf 'first\n' > docs/guide/first.md +git add docs && git commit -q -m "Document the first change" +first_sha="$(git rev-parse HEAD)" +printf 'second\n' > notes.txt +git add notes.txt && git commit -q -m "Touch notes" +second_sha="$(git rev-parse HEAD)" +git push -q origin HEAD:main + +run_propose() { + : > "$STUB_LOG" + rm -rf .docs-proposal + if "$here/propose.sh" "$second_sha" .docs-proposal "$1" > "$tmp/out.log" 2>&1; then + printf '0' + else + printf '1' + fi +} + +assert_eq "$(run_propose "$merge_sha")" 0 "should propose for a multi-commit push" +assert_eq "$(cat .docs-proposal/base)" "$merge_sha" "should record the push base" +assert_contains "$(cat .docs-proposal/prompt.md)" "- Base: $merge_sha" "should inspect the whole push" +assert_contains "$(cat "$STUB_LOG")" "copilot -p" "should run Copilot" + +assert_eq "$(run_propose "")" 0 "should propose without a base" +assert_eq "$(cat .docs-proposal/base)" "$first_sha" "should default the base to the first parent" + +assert_eq "$(run_propose 0000000000000000000000000000000000000000)" 0 "should propose for a push that created main" +assert_eq "$(cat .docs-proposal/base)" "$first_sha" "should replace an all-zero base with the first parent" +assert_eq "$(grep -c '::warning::' "$tmp/out.log" || true)" 0 "should not warn about an all-zero base" + +assert_eq "$(run_propose "$published_head")" 0 "should propose with an unrelated base" +assert_eq "$(cat .docs-proposal/base)" "$first_sha" "should replace an unrelated base with the first parent" +assert_contains "$(cat "$tmp/out.log")" "is not an ancestor" "should warn about an unrelated base" + cd "$here" if [ "$failures" -gt 0 ]; then From 947cf4cdc73e98844579421cd1380c67910c3980 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:16:04 +0530 Subject: [PATCH 10/14] Document the proposal scripts CI gate and first-run prerequisites AGENTS.md now lists the docs-proposal scripts job among the CI gates. scripts/README.md names all three scripts the workflow runs, and warns that the run triggered by merging the workflow fails unless the Copilot policy and pull request setting are already enabled. Signed-off-by: dhruv8sh --- AGENTS.md | 1 + scripts/README.md | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 738fcab34..5a3fd9156 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -352,6 +352,7 @@ Every PR must pass: 6. JS format (`cd crates/trusted-server-js/lib && npm run format`) 7. Docs format (`cd docs && npm run format`) 8. Markdown format outside `docs/` (requires `cd docs && npm ci` first): `docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" ".claude/**/*.md" ".github/**/*.md" "crates/**/*.md" "scripts/**/*.md" "tinybird/**/*.md"`; fix with `--write` in place of `--check` +9. Documentation proposal scripts: `shellcheck scripts/docs-proposal/*.sh && node --test scripts/docs-proposal/hunks.test.mjs && scripts/docs-proposal/test.sh` --- diff --git a/scripts/README.md b/scripts/README.md index 26ca19b13..e94925601 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -27,16 +27,18 @@ The four adapter smoke contracts are documented in the [deployment guides](../docs/guide/integrations-overview.md#adapter-support). Repository-wide verification policy lives in [TESTING.md](../TESTING.md). -The `Documentation proposal` workflow runs both proposal scripts after code -merges to `main` and opens one pull request per merge on `docs/auto/`. -It needs the organization Copilot policy "Allow use of Copilot CLI billed to -the organization" and the repository setting "Allow GitHub Actions to create -and approve pull requests". Pull requests opened with `GITHUB_TOKEN` do not -trigger other workflows, so the workflow's validate job runs the docs gates -itself, without write credentials; push to the proposal branch to run regular -CI. Retry a merge from the workflow's manual dispatch with its full commit SHA; -the scripts always run from the workflow's revision, so any merge on `main` can -be retried. When the original push added several commits, as a rebase merge -does, also pass its `base`: the `main` head before that push, shown as -`BASE_SHA` in the original run and recorded in the proposal's body. Publication -refuses a retry whose range differs from an open proposal's. +The `Documentation proposal` workflow runs the propose, validate, and publish +scripts after code merges to `main` and opens one pull request per merge on +`docs/auto/`. Its first run, including the one triggered by merging the +workflow itself, fails unless the organization Copilot policy "Allow use of +Copilot CLI billed to the organization" and the repository setting "Allow GitHub +Actions to create and approve pull requests" are already enabled. Pull requests +opened with `GITHUB_TOKEN` do not trigger other workflows, so the workflow's +validate job runs the docs gates itself, without write credentials; push to the +proposal branch to run regular CI. Retry a merge from the workflow's manual +dispatch with its full commit SHA; the scripts always run from the workflow's +revision, so any merge on `main` can be retried. When the original push added +several commits, as a rebase merge does, also pass its `base`: the `main` head +before that push, shown as `BASE_SHA` in the original run and recorded in the +proposal's body. Publication refuses a retry whose range differs from an open +proposal's. From 2304a9ae88de1b99bc5058ae1d6f63aff36e487e Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 18:16:50 +0530 Subject: [PATCH 11/14] Fill the proposal work directory into Copilot prompts propose.sh accepts any work directory, but both prompts told Copilot to write its outputs under .docs-proposal/. The prompts now use a placeholder that propose.sh replaces with the work directory relative to the repository root. Signed-off-by: dhruv8sh --- scripts/docs-proposal/evidence-prompt.md | 2 +- scripts/docs-proposal/prompt.md | 6 +++--- scripts/docs-proposal/propose.sh | 12 ++++++++++-- scripts/docs-proposal/test.sh | 7 +++++++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/scripts/docs-proposal/evidence-prompt.md b/scripts/docs-proposal/evidence-prompt.md index 9bb51e35b..587268e7d 100644 --- a/scripts/docs-proposal/evidence-prompt.md +++ b/scripts/docs-proposal/evidence-prompt.md @@ -4,7 +4,7 @@ The hunks at the end of this file are proposed edits to the Trusted Server documentation for the merged commit named there. For each hunk, find the merged source that justifies it. -Write `.docs-proposal/evidence.json` with exactly this shape, and change no +Write `/evidence.json` with exactly this shape, and change no other file: ```json diff --git a/scripts/docs-proposal/prompt.md b/scripts/docs-proposal/prompt.md index 1bde920e8..9dfa072d3 100644 --- a/scripts/docs-proposal/prompt.md +++ b/scripts/docs-proposal/prompt.md @@ -16,9 +16,9 @@ sources to documentation pages appear at the end of this file. bundles, or verification commands. Use the Sources of truth table to find the affected pages. 3. If no reader-facing documentation is affected, change nothing and write - `.docs-proposal/rationale.md` containing one sentence explaining why. + `/rationale.md` containing one sentence explaining why. 4. Otherwise edit only the affected pages, then write - `.docs-proposal/rationale.md` with one bullet per edited page naming the + `/rationale.md` with one bullet per edited page naming the page, what changed, and the merged source file that justifies it. ## Rules @@ -34,4 +34,4 @@ sources to documentation pages appear at the end of this file. - Keep existing links and anchors working, and link to other pages with relative paths. - Do not edit files outside `docs/guide/`, `docs/index.md`, and - `.docs-proposal/`. Do not commit, push, or create branches. + `/`. Do not commit, push, or create branches. diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh index b0dbc694f..a300e05b2 100755 --- a/scripts/docs-proposal/propose.sh +++ b/scripts/docs-proposal/propose.sh @@ -33,6 +33,14 @@ work_dir="$(cd "$2" && pwd)" printf '%s\n' "$base" > "$work_dir/base" relative_dir="${work_dir#"$root"/}" +# Prints a prompt template with each `` replaced by the work +# directory, relative to the repository root. +render_prompt() { + local text + text="$(cat "$1")" + printf '%s\n' "${text///"$relative_dir"}" +} + # Reads are always permitted; writes are allowed so Copilot can edit pages, # and publish.sh enforces which paths may actually change. run_copilot() { @@ -47,7 +55,7 @@ run_copilot() { } { - cat "$here/prompt.md" + render_prompt "$here/prompt.md" printf '\n## Merged change\n\n- Commit: %s\n- Base: %s\n- Subject: %s\n\n' "$sha" "$base" "$(git log -1 --format=%s "$sha")" printf '## Sources of truth\n\n' awk '/^## Sources of truth/ { found = 1; next } /^## / { found = 0 } found' "$spec" @@ -68,7 +76,7 @@ if [ ! -s "$work_dir/proposal.patch" ]; then fi { - cat "$here/evidence-prompt.md" + render_prompt "$here/evidence-prompt.md" printf '\n## Merged change\n\n- Commit: %s\n\n## Hunks\n\n```json\n' "$sha" git diff --cached --no-renames "$sha" -- docs | node "$here/hunks.mjs" list printf '```\n' diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index 0d265d3f4..d768df3b0 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -261,6 +261,13 @@ assert_eq "$(run_propose "$merge_sha")" 0 "should propose for a multi-commit pus assert_eq "$(cat .docs-proposal/base)" "$merge_sha" "should record the push base" assert_contains "$(cat .docs-proposal/prompt.md)" "- Base: $merge_sha" "should inspect the whole push" assert_contains "$(cat "$STUB_LOG")" "copilot -p" "should run Copilot" +assert_contains "$(cat .docs-proposal/prompt.md)" ".docs-proposal/rationale.md" "should name the work dir in the prompt" +assert_eq "$(grep -c '' .docs-proposal/prompt.md || true)" 0 "should fill every work dir placeholder" + +rm -rf proposal-out +"$here/propose.sh" "$second_sha" proposal-out "" > "$tmp/out.log" 2>&1 +assert_contains "$(cat proposal-out/prompt.md)" "proposal-out/rationale.md" "should name a custom work dir in the prompt" +rm -rf proposal-out assert_eq "$(run_propose "")" 0 "should propose without a base" assert_eq "$(cat .docs-proposal/base)" "$first_sha" "should default the base to the first parent" From 0bc4bf7a13eca8e68275055996a547fb171b74d0 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Thu, 8 Oct 2026 00:24:04 +0530 Subject: [PATCH 12/14] Guard empty-rerun branch deletion with a lease An empty rerun deleted the proposal branch via gh pr close --delete-branch, so a maintainer push after the head inspection was lost, and a failed fetch hid existing maintainer commits. Look up the branch with git ls-remote --exit-code and fail unless it is verifiably absent, delete it with --force-with-lease on the inspected head, and close the pull request only after the deletion succeeds. Add regressions for a concurrent maintainer push, fetch failures on the empty and push paths, and the lease-guarded deletion. Signed-off-by: dhruv8sh --- ...6-10-01-docs-proposal-automation-design.md | 16 ++++-- scripts/README.md | 38 +++++++------- scripts/docs-proposal/publish.sh | 18 +++++-- scripts/docs-proposal/test.sh | 51 ++++++++++++++++++- 4 files changed, 96 insertions(+), 27 deletions(-) diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index e70a78c91..ba8859dd3 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -114,12 +114,18 @@ and nothing installs, builds, or executes proposal content. retry inspected another range, such as a manual dispatch without the base of a multi-commit push, and must not narrow or close the proposal. The error names the recorded base to rerun with. -3. If the remote branch's head is not the generated proposal commit (its +3. Look up the remote branch with `git ls-remote --exit-code`. Only a + verified absent branch counts as new; any other lookup or fetch failure + fails the run, so an error never hides maintainer commits. If the remote + branch's head is not the generated proposal commit (its parent is not the merge commit), a maintainer has committed there, for example by applying a review suggestion. Warn and exit successfully without pushing, closing, or deleting anything. -4. Empty diff: close any open pull request for that branch with a comment, - then exit successfully. Otherwise apply `proposal.patch` to a fresh +4. Empty diff: if a pull request is open, delete its branch with + `git push --force-with-lease=refs/heads/: --delete`, + so a maintainer push after the inspection makes the deletion fail, and + close the pull request with a comment only after the deletion succeeds. + Then exit successfully. Otherwise apply `proposal.patch` to a fresh checkout of the merge commit. 5. Reject the proposal again if any changed path is outside `docs/guide/**` or `docs/index.md`. @@ -199,7 +205,9 @@ Documented in `scripts/README.md`: allowed proposals, the empty diff, closing a stale proposal, a disallowed path, first publish without a build, an unchanged retry, an updated proposal whose review fails and is resumed without a second push, a retry or empty - rerun that leaves maintainer commits in place, retries over a different or + rerun that leaves maintainer commits in place, an empty rerun that races a + maintainer push, a failed branch fetch on both the empty and push paths, + deleting an unmaintained branch before closing its proposal, retries over a different or unrecorded range, a closed proposal, PR body rendering, and how `propose.sh` resolves the base of a multi-commit push. - `shellcheck` passes for every script. diff --git a/scripts/README.md b/scripts/README.md index e94925601..e43dfd7b5 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -3,25 +3,25 @@ Run these scripts from the repository root. They fail on command errors unless their documented orchestration handles a probe deliberately. -| Script | Inputs and prerequisites | Side effects and cleanup | -| ----------------------------------------- | -------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `batch-sync.sh` | Endpoint, bearer token, EC ID, partner UID; `curl` and Python | Sends one authenticated batch-sync request. Its temporary response file is removed on exit. | -| `benchmark.sh` | A running server; `curl`, `bc`, and optionally `hey` | May install `hey` through Homebrew. `--save` writes under `benchmark-results/`; it does not stop the server. | -| `profile.sh` | Fastly CLI, Rust WASM target, `curl`; endpoint and request options | Builds and starts the Fastly app, stops the owned process, and retains a profile under `benchmark-results/profiles/`. `--open` launches the local viewer. | -| `generate-integration-viceroy-configs.sh` | Rust toolchain; optional origin port and artifact directory | Builds the native generator and writes Viceroy config under `target/integration-test-artifacts/`; generated files persist. | -| `integration-tests.sh` | Docker, Viceroy, Rust WASM target, pinned Node | Builds WASM/native artifacts and two Docker images, generates Viceroy config, and runs native integration tests serially. Build products and images persist. | -| `integration-tests-browser.sh` | The integration prerequisites plus npm and Playwright | Installs package/browser dependencies, builds fixtures and images, runs both browser suites, and stops matching test containers on exit. Build and npm artifacts persist. | -| `smoke-axum.sh` | `cargo`, `curl`, Python | Uses an isolated temporary config/store and stub origin; stops owned processes and removes its workspace. | -| `smoke-fastly.sh` | Fastly CLI, `cargo`, `curl`, Python | Uses an isolated Fastly project and application config; stops owned processes and removes its workspace. | -| `smoke-cloudflare.sh` | Pinned Wrangler, `cargo`, `curl`, `jq`, and Python | Uses isolated Wrangler manifests, KV state, ports, and logs; stops owned processes and removes its workspace. | -| `smoke-spin.sh` | Spin CLI, `cargo`, `curl`, and Python | Uses an isolated Spin manifest and SQLite KV store; stops owned processes and removes its workspace. | -| `smoke-common.sh` | Sourced by the four smoke scripts | Defines bounded port, process, config, secret, and response assertions. Do not invoke it as a standalone smoke. | -| `template-cache-local-test.sh` | Viceroy, Node, OpenSSL, `curl`, `lsof`; optional ports and mode | Builds local artifacts, generates certificates and fixtures in a temporary directory, stops owned servers, and removes the directory. | -| `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | -| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, evidence, and base into the work dir. Leaves the edits staged. | -| `docs-proposal/validate.sh` | Merge SHA, work dir from `propose.sh`; Node, `npm`; no write credentials | Applies the patch, rejects paths outside the allowlist, and runs the docs gates. Leaves the edits staged. | -| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, pushes `docs/auto/` unless a maintainer added to it, opens or edits its PR, and reviews each new commit per hunk. Never writes `main`. | -| `docs-proposal/test.sh` | Git and Node | Tests the helpers and the propose, validate, and publish flows against a temporary repository with stubbed `copilot`, `gh`, and `npm`; removes it on exit. | +| Script | Inputs and prerequisites | Side effects and cleanup | +| ----------------------------------------- | -------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `batch-sync.sh` | Endpoint, bearer token, EC ID, partner UID; `curl` and Python | Sends one authenticated batch-sync request. Its temporary response file is removed on exit. | +| `benchmark.sh` | A running server; `curl`, `bc`, and optionally `hey` | May install `hey` through Homebrew. `--save` writes under `benchmark-results/`; it does not stop the server. | +| `profile.sh` | Fastly CLI, Rust WASM target, `curl`; endpoint and request options | Builds and starts the Fastly app, stops the owned process, and retains a profile under `benchmark-results/profiles/`. `--open` launches the local viewer. | +| `generate-integration-viceroy-configs.sh` | Rust toolchain; optional origin port and artifact directory | Builds the native generator and writes Viceroy config under `target/integration-test-artifacts/`; generated files persist. | +| `integration-tests.sh` | Docker, Viceroy, Rust WASM target, pinned Node | Builds WASM/native artifacts and two Docker images, generates Viceroy config, and runs native integration tests serially. Build products and images persist. | +| `integration-tests-browser.sh` | The integration prerequisites plus npm and Playwright | Installs package/browser dependencies, builds fixtures and images, runs both browser suites, and stops matching test containers on exit. Build and npm artifacts persist. | +| `smoke-axum.sh` | `cargo`, `curl`, Python | Uses an isolated temporary config/store and stub origin; stops owned processes and removes its workspace. | +| `smoke-fastly.sh` | Fastly CLI, `cargo`, `curl`, Python | Uses an isolated Fastly project and application config; stops owned processes and removes its workspace. | +| `smoke-cloudflare.sh` | Pinned Wrangler, `cargo`, `curl`, `jq`, and Python | Uses isolated Wrangler manifests, KV state, ports, and logs; stops owned processes and removes its workspace. | +| `smoke-spin.sh` | Spin CLI, `cargo`, `curl`, and Python | Uses an isolated Spin manifest and SQLite KV store; stops owned processes and removes its workspace. | +| `smoke-common.sh` | Sourced by the four smoke scripts | Defines bounded port, process, config, secret, and response assertions. Do not invoke it as a standalone smoke. | +| `template-cache-local-test.sh` | Viceroy, Node, OpenSSL, `curl`, `lsof`; optional ports and mode | Builds local artifacts, generates certificates and fixtures in a temporary directory, stops owned servers, and removes the directory. | +| `test-cli.sh` | Rustup and the host toolchain; optional host triple | May install the selected Rust target, then runs native CLI and browser-audit tests. Cargo artifacts persist. | +| `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, evidence, and base into the work dir. Leaves the edits staged. | +| `docs-proposal/validate.sh` | Merge SHA, work dir from `propose.sh`; Node, `npm`; no write credentials | Applies the patch, rejects paths outside the allowlist, and runs the docs gates. Leaves the edits staged. | +| `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, pushes `docs/auto/` unless a maintainer added to it, opens or edits its PR, and reviews each new commit per hunk. An empty rerun deletes the branch under a lease, then closes the PR; a failed branch lookup stops the run. Never writes `main`. | +| `docs-proposal/test.sh` | Git and Node | Tests the helpers and the propose, validate, and publish flows against a temporary repository with stubbed `copilot`, `gh`, and `npm`; removes it on exit. | The four adapter smoke contracts are documented in the [deployment guides](../docs/guide/integrations-overview.md#adapter-support). diff --git a/scripts/docs-proposal/publish.sh b/scripts/docs-proposal/publish.sh index eb1ec82b0..06049b8b3 100755 --- a/scripts/docs-proposal/publish.sh +++ b/scripts/docs-proposal/publish.sh @@ -44,10 +44,17 @@ fi # Maintainers push to the proposal branch, and applying a review suggestion # commits there too. Never overwrite or delete commits beyond the generated -# proposal, whose only parent is the merge commit. +# proposal, whose only parent is the merge commit. Only a verified absent +# branch counts as empty; any other lookup or fetch failure stops the run. remote_head="" -if git fetch --quiet origin "refs/heads/$branch" 2>/dev/null; then +ls_status=0 +git ls-remote --exit-code --quiet origin "refs/heads/$branch" > /dev/null || ls_status=$? +if [ "$ls_status" -eq 0 ]; then + git fetch --quiet origin "refs/heads/$branch" remote_head="$(git rev-parse FETCH_HEAD)" +elif [ "$ls_status" -ne 2 ]; then + printf '::error::Could not inspect proposal branch %s.\n' "$branch" >&2 + exit 1 fi if [ -n "$remote_head" ] && [ "$(git rev-parse "$remote_head^")" != "$sha" ]; then printf '::warning::Proposal branch %s has commits beyond the generated proposal; not overwriting it.\n' "$branch" @@ -57,7 +64,12 @@ fi if [ ! -s "$work_dir/proposal.patch" ]; then printf 'No documentation changes proposed for %s.\n' "$sha" if [ -n "$existing" ]; then - gh pr close "$pr_number" --delete-branch \ + # The lease refuses the deletion if a maintainer pushed after the + # inspection above; the pull request is closed only once it succeeds. + if [ -n "$remote_head" ]; then + git push --quiet --force-with-lease="refs/heads/$branch:$remote_head" origin --delete "$branch" + fi + gh pr close "$pr_number" \ --comment "A rerun for $sha proposed no documentation changes." fi exit 0 diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index d768df3b0..e0e0863ef 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -93,7 +93,20 @@ cat > "$tmp/bin/copilot" <<'STUB' #!/usr/bin/env bash printf 'copilot %s\n' "$*" >> "$STUB_LOG" STUB -chmod +x "$tmp/bin/gh" "$tmp/bin/npm" "$tmp/bin/copilot" +# Wraps git so a test can fail the proposal branch fetch, or land a +# maintainer push right after publish.sh inspects the branch head. +real_git="$(command -v git)" +cat > "$tmp/bin/git" <&2 +fi +STUB +chmod +x "$tmp/bin/gh" "$tmp/bin/npm" "$tmp/bin/copilot" "$tmp/bin/git" export PATH="$tmp/bin:$PATH" STUB_LOG="$tmp/gh.log" STUB_REVIEW="$tmp/review.json" git init -q --bare "$tmp/origin.git" @@ -214,6 +227,26 @@ assert_eq "$(remote_head)" "$pushed_head" "should not push again when resuming" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" '"path": "docs/index.md"' "should review the new hunk" assert_contains "$(cat "$STUB_REVIEW" 2>/dev/null)" "\"commit_id\": \"$pushed_head\"" "should review the pushed commit" +# A maintainer push that lands between the head inspection and the deletion +# must survive an empty rerun. +git clone -q "$tmp/origin.git" "$tmp/racer" 2>/dev/null +cat > "$tmp/race-push.sh" <> docs/guide/cli.md +git commit -q -am "Push a maintainer edit during the rerun" +git push -q origin "HEAD:refs/heads/$branch" +RACE +chmod +x "$tmp/race-push.sh" +make_patch "" +assert_eq "$(STUB_PR="7 OPEN $base_sha" STUB_RACE_PUSH="$tmp/race-push.sh" run_publish)" 1 "should fail an empty rerun that races a maintainer push" +assert_eq "$(remote_has_branch)" yes "should keep a branch pushed to during an empty rerun" +assert_eq "$(git -C "$tmp/racer" rev-parse HEAD)" "$(remote_head)" "should keep the racing maintainer commit" +assert_eq "$(grep -c 'pr close' "$STUB_LOG" || true)" 0 "should not close a proposal whose branch moved" + git fetch -q origin "refs/heads/$branch" git switch -q --detach FETCH_HEAD printf 'maintainer edit\n' >> docs/guide/cli.md @@ -232,9 +265,25 @@ assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should succeed on an em assert_eq "$(grep -c 'pr close' "$STUB_LOG" || true)" 0 "should not close or delete a maintained proposal" assert_eq "$(remote_head)" "$maintainer_head" "should keep the maintained proposal branch" +make_patch "" +assert_eq "$(STUB_PR="7 OPEN $base_sha" STUB_FAIL_FETCH=1 run_publish)" 1 "should fail an empty rerun when the branch fetch fails" +assert_eq "$(grep -c 'pr close' "$STUB_LOG" || true)" 0 "should not close a proposal whose branch could not be fetched" +assert_eq "$(remote_head)" "$maintainer_head" "should keep a branch that could not be fetched" + +make_patch docs/guide/cli.md +assert_eq "$(STUB_PR="7 OPEN $base_sha" STUB_FAIL_FETCH=1 run_publish)" 1 "should fail a rerun when the branch fetch fails" +assert_eq "$(remote_head)" "$maintainer_head" "should not push over a branch that could not be fetched" + assert_eq "$(STUB_PR="7 CLOSED" run_publish)" 0 "should succeed for a closed proposal" assert_eq "$(grep -c 'pr edit\|pr create' "$STUB_LOG" || true)" 0 "should never reopen or recreate a closed proposal" +git push -q --force origin "$pushed_head:refs/heads/$branch" +make_patch "" +assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should succeed when an empty rerun deletes the proposal" +assert_eq "$(remote_has_branch)" no "should delete an unmaintained proposal branch" +assert_contains "$(cat "$STUB_LOG")" "pr close 7 --comment" "should close the proposal after deleting its branch" +assert_eq "$(grep -c 'delete-branch' "$STUB_LOG" || true)" 0 "should not let gh delete the branch" + # A push that added two commits is inspected from the previous main head; a # retry without that base falls back to the first parent, which publish.sh # then refuses to apply over the original proposal. From a7ad11dee23fefa4bfcada5bb7ba8ef98244f65c Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Sat, 10 Oct 2026 02:31:05 +0530 Subject: [PATCH 13/14] Harden documentation proposals against forks, symlinks, and live rationale Ignore fork pull requests that reuse the proposal branch name, reject non-regular files such as symlinks, and render the rationale as an inert text block. Comment before closing an emptied proposal, keep a proposal when the evidence run fails, resolve a manual base to a full SHA, and fill prompts portably across bash versions. Ask for deletion instead of an empty suggestion on new pages, drop the npm cache from the Pages deploy, mark the original plan superseded, and cover the evidence half, symlinks, and fork pull requests in the script tests. Signed-off-by: dhruv8sh --- .github/workflows/deploy-docs.yml | 4 +- .../2026-10-01-docs-proposal-automation.md | 6 ++ ...6-10-01-docs-proposal-automation-design.md | 49 ++++++++----- scripts/README.md | 7 +- scripts/docs-proposal/hunks.mjs | 33 +++++++-- scripts/docs-proposal/hunks.test.mjs | 54 ++++++++++++++ scripts/docs-proposal/lib.sh | 16 +++- scripts/docs-proposal/propose.sh | 22 ++++-- scripts/docs-proposal/publish.sh | 13 ++-- scripts/docs-proposal/test.sh | 73 +++++++++++++++++-- 10 files changed, 228 insertions(+), 49 deletions(-) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index 3fa0da9b6..99d73b0dd 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -32,12 +32,12 @@ jobs: id: node-version run: echo "node-version=$(grep '^nodejs ' .tool-versions | awk '{print $2}')" >> $GITHUB_OUTPUT + # No dependency cache: the docs-proposal workflow runs generated content + # on the main ref, where it could save a poisoned entry under this key. - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: ${{ steps.node-version.outputs.node-version }} - cache: "npm" - cache-dependency-path: docs/package-lock.json - name: Setup Pages uses: actions/configure-pages@v4 diff --git a/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md b/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md index a6d67b96d..4a61e3b74 100644 --- a/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md +++ b/docs/superpowers/plans/2026-10-01-docs-proposal-automation.md @@ -1,5 +1,11 @@ # Documentation Proposal Automation Implementation Plan +> **Status: Superseded.** This is the original two-job plan, kept for history. +> Review moved the docs gates into the read-only `validate` job, replaced +> `--force` pushes and `gh pr close --delete-branch` with leased pushes, dropped +> `shell(git grep:*)` and the npm cache, and added base tracking. Do not execute +> it; the design spec and `scripts/docs-proposal/` are the source of truth. + > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** After a code merge to `main`, run GitHub Copilot CLI to propose diff --git a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md index ba8859dd3..abf52e016 100644 --- a/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md +++ b/docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md @@ -47,9 +47,9 @@ the current scripts rather than historical ones. - `push` to `main`, filtered to the source paths above. `docs/**` and `scripts/docs-proposal/**` are not included, so a merged documentation proposal or a change to this tooling never retriggers the workflow. -- `workflow_dispatch` with a required `sha` input, for retries and for the - first end-to-end validation. Both scripts require a full 40-character SHA - reachable from `origin/main`. +- `workflow_dispatch` with a required `sha` input and an optional `base` + input, for retries and for the first end-to-end validation. All three + scripts require a full 40-character SHA reachable from `origin/main`. - `concurrency` with `group: docs-proposal`, `cancel-in-progress: false`, and `queue: max` so sequential merges are processed one at a time, in order. The default queue keeps one pending run and cancels the rest, which would @@ -83,7 +83,8 @@ Permissions: `contents: read`, `copilot-requests: write`. A hunk id is the changed path plus a short hash of the hunk's removed and added lines, so it is stable across reruns. A second Copilot run writes `evidence.json`: for each hunk id, the merged source `path:line` - that justifies it and a one-sentence reason. + that justifies it and a one-sentence reason. Evidence is advisory, so a + failed evidence run only logs a warning and keeps the proposal. 4. Upload the `.docs-proposal/` directory as an artifact. ### Job `validate` @@ -96,7 +97,9 @@ only here. 1. Empty diff: exit successfully. 2. Apply `proposal.patch` to the merge commit and reject the proposal if any - changed path is outside `docs/guide/**` or `docs/index.md`. + changed path is outside `docs/guide/**` or `docs/index.md`, or any changed + entry is not a regular file. A symlink would otherwise pass the path check + and publish its target. 3. In `docs/`: `npm ci`, `npm run lint`, `npm run format`, `npm run build`. Any failure fails the workflow; no pull request is offered. @@ -109,7 +112,8 @@ and nothing installs, builds, or executes proposal content. 1. If a closed or merged pull request already exists for `docs/auto/`, leave it alone and exit successfully: a - maintainer's decision is never reopened or recreated. + maintainer's decision is never reopened or recreated. Pull requests from + forks that reuse the branch name are ignored. 2. If an open pull request records a different base in its body, fail: the retry inspected another range, such as a manual dispatch without the base of a multi-commit push, and must not narrow or close the proposal. The @@ -123,12 +127,14 @@ and nothing installs, builds, or executes proposal content. without pushing, closing, or deleting anything. 4. Empty diff: if a pull request is open, delete its branch with `git push --force-with-lease=refs/heads/: --delete`, - so a maintainer push after the inspection makes the deletion fail, and - close the pull request with a comment only after the deletion succeeds. + so a maintainer push after the inspection makes the deletion fail. Only + after the deletion succeeds, comment on the pull request and then close + it; the deletion may already have closed it, and `gh pr close` skips its + `--comment` on a closed pull request. Then exit successfully. Otherwise apply `proposal.patch` to a fresh checkout of the merge commit. 5. Reject the proposal again if any changed path is outside `docs/guide/**` or - `docs/index.md`. + `docs/index.md`, or any changed entry is not a regular file. 6. Commit as `github-actions[bot]` on `docs/auto/`. If the remote branch already exists and its tree equals the new tree, reuse the remote commit and skip the push, so a retry resumes any later step that failed. @@ -136,9 +142,10 @@ and nothing installs, builds, or executes proposal content. 7. Find the originating pull request with `gh api repos/{owner}/{repo}/commits//pulls`. Create the pull request, or edit the existing one, with a body that links the merge commit and the - originating pull request, records the inspected base, includes `rationale.md` with its `@` mentions - neutralized, and states that a human must verify the prose against the - code before merging. + originating pull request, records the inspected base, includes `rationale.md` as a + fenced `text` block with its `@` mentions neutralized, so links, HTML, + issue references, and closing keywords in it stay inert, and states that + a human must verify the prose against the code before merging. 8. If `github-actions[bot]` already reviewed the pushed commit, stop: nothing is re-posted. Otherwise `node scripts/docs-proposal/hunks.mjs review` builds review comments and posts one `COMMENT` review through `gh api` @@ -156,6 +163,7 @@ so the revert is exact: | ------------------------- | --------------------------------------- | ---------------------------------------- | | Changed or inserted lines | The new lines (right side) | The original lines (empty for insertion) | | Pure deletion | Adjacent context line (prefer previous) | That context line plus deleted lines | +| New file | The new lines (right side) | None; the comment asks to delete it | | Deleted file or binary | Listed in the review body | None | Hunks without evidence still receive a revert suggestion, marked "none @@ -174,8 +182,9 @@ Documented in `scripts/README.md`: - Repository setting "Allow GitHub Actions to create and approve pull requests" is enabled. - Pull requests opened with `GITHUB_TOKEN` do not trigger other workflows. - The `validate` job runs the docs gates itself; maintainers re-run regular CI - by pushing to the branch if needed. + The `validate` job runs the docs gates itself. The checks `main` requires + never start on a proposal, so maintainers close and reopen it, or push to + its branch, before merging. ## Security @@ -187,6 +196,10 @@ Documented in `scripts/README.md`: is treated as able to run arbitrary code. Its token is read-only, neither it nor `validate` restores or saves a dependency cache, and its artifact is re-checked by `validate.sh` and `publish.sh`. +- Both jobs run on the `main` ref, and code running in a job can save cache + entries scoped to `main` without declaring a cache. `deploy-docs.yml`, the + only privileged workflow that restored a cache, therefore installs docs + dependencies without one. - Input is code already merged to `main`, so no untrusted fork content reaches the agent. - The path allowlist is enforced by `validate.sh` and again by `publish.sh`, @@ -201,15 +214,17 @@ Documented in `scripts/README.md`: - `node --test scripts/docs-proposal/hunks.test.mjs` covers hunk listing, every row of the per-hunk table, unknown evidence ids, and the review payload. - `scripts/docs-proposal/test.sh` stubs `gh` and `npm` and pushes to a - temporary bare repository to cover validation of empty, disallowed, and - allowed proposals, the empty diff, closing a stale proposal, a disallowed + temporary bare repository to cover validation of empty, disallowed, symlinked, + and allowed proposals, the empty diff, fork pull requests that reuse the + proposal branch name, closing a stale proposal, a disallowed path, first publish without a build, an unchanged retry, an updated proposal whose review fails and is resumed without a second push, a retry or empty rerun that leaves maintainer commits in place, an empty rerun that races a maintainer push, a failed branch fetch on both the empty and push paths, deleting an unmaintained branch before closing its proposal, retries over a different or unrecorded range, a closed proposal, PR body rendering, and how - `propose.sh` resolves the base of a multi-commit push. + `propose.sh` resolves the base of a multi-commit push, renders the work + dir into prompts, and runs, or survives a failure of, the evidence half. - `shellcheck` passes for every script. - The `docs-proposal-scripts` job in `format.yml` runs all three on every pull request. diff --git a/scripts/README.md b/scripts/README.md index e43dfd7b5..773052da5 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -21,7 +21,7 @@ their documented orchestration handles a probe deliberately. | `docs-proposal/propose.sh` | Merge SHA on `main`, work dir, optional base SHA; Copilot CLI, Node, `npm` | Runs Copilot CLI to edit `docs/guide/**` or `docs/index.md`, formats docs, and writes a patch, rationale, evidence, and base into the work dir. Leaves the edits staged. | | `docs-proposal/validate.sh` | Merge SHA, work dir from `propose.sh`; Node, `npm`; no write credentials | Applies the patch, rejects paths outside the allowlist, and runs the docs gates. Leaves the edits staged. | | `docs-proposal/publish.sh` | Merge SHA, work dir from `propose.sh`; `gh` with write `GH_TOKEN` | Re-checks the allowlist without building, pushes `docs/auto/` unless a maintainer added to it, opens or edits its PR, and reviews each new commit per hunk. An empty rerun deletes the branch under a lease, then closes the PR; a failed branch lookup stops the run. Never writes `main`. | -| `docs-proposal/test.sh` | Git and Node | Tests the helpers and the propose, validate, and publish flows against a temporary repository with stubbed `copilot`, `gh`, and `npm`; removes it on exit. | +| `docs-proposal/test.sh` | Git, Node, and `jq` | Tests the helpers and the propose, validate, and publish flows against a temporary repository with stubbed `copilot`, `gh`, and `npm`; removes it on exit. | The four adapter smoke contracts are documented in the [deployment guides](../docs/guide/integrations-overview.md#adapter-support). @@ -34,8 +34,9 @@ workflow itself, fails unless the organization Copilot policy "Allow use of Copilot CLI billed to the organization" and the repository setting "Allow GitHub Actions to create and approve pull requests" are already enabled. Pull requests opened with `GITHUB_TOKEN` do not trigger other workflows, so the workflow's -validate job runs the docs gates itself, without write credentials; push to the -proposal branch to run regular CI. Retry a merge from the workflow's manual +validate job runs the docs gates itself, without write credentials. The checks +`main` requires never start on a proposal, so close and reopen it, or push to +its branch, before merging. Retry a merge from the workflow's manual dispatch with its full commit SHA; the scripts always run from the workflow's revision, so any merge on `main` can be retried. When the original push added several commits, as a rebase merge does, also pass its `base`: the `main` head diff --git a/scripts/docs-proposal/hunks.mjs b/scripts/docs-proposal/hunks.mjs index 132b1360f..95c81aa4c 100644 --- a/scripts/docs-proposal/hunks.mjs +++ b/scripts/docs-proposal/hunks.mjs @@ -180,7 +180,7 @@ export function indexEvidence(raw) { return evidence; } -// A suggestion fence must be longer than any backtick run in its body. +// A fence must be longer than any backtick run in its body. function fenceFor(body) { let longestRun = 0; for (const text of body) { @@ -191,15 +191,33 @@ function fenceFor(body) { return "`".repeat(Math.max(3, longestRun + 1)); } +// The rationale is model output, so render it as a plain text block: links, +// images, HTML, issue references, and closing keywords in it stay inert. +export function inertBlock(text) { + const body = neutralizeMentions(text).replace(/\n+$/, ""); + const fence = fenceFor(body.split("\n")); + return `${fence}text\n${body}\n${fence}\n`; +} + +// A null body marks a hunk that creates its file: an empty suggestion would +// leave an empty page that VitePress still builds, so ask for a deletion. function commentBody(cited, body) { const source = cited?.source ? `\`${cited.source}\`` : "none cited — verify this hunk against the merged code."; const reason = cited?.reason ? ` — ${cited.reason}` : ""; + const heading = `**Source:** ${source}${reason}`; + if (body === null) { + return [ + heading, + "", + "This hunk creates the file. Delete the file to revert it.", + ].join("\n"); + } const fence = fenceFor(body); const content = body.length > 0 ? `${body.join("\n")}\n` : ""; return [ - `**Source:** ${source}${reason}`, + heading, "", "Apply this suggestion to revert the hunk.", "", @@ -232,7 +250,10 @@ export function buildReview(files, evidence, commitId) { path, line: suggestion.line, side: "RIGHT", - body: commentBody(evidence.get(id), suggestion.body), + body: commentBody( + evidence.get(id), + status === "added" ? null : suggestion.body, + ), }; if (suggestion.startLine !== suggestion.line) { comment.start_line = suggestion.startLine; @@ -260,8 +281,8 @@ function readEvidence(path) { function main([command, evidencePath, commitId]) { const input = readFileSync(0, "utf8"); - if (command === "sanitize") { - process.stdout.write(neutralizeMentions(input)); + if (command === "rationale") { + process.stdout.write(inertBlock(input)); return; } const files = parseDiff(input); @@ -275,7 +296,7 @@ function main([command, evidencePath, commitId]) { return; } process.stderr.write( - "usage: hunks.mjs list | review | sanitize\n", + "usage: hunks.mjs list | review | rationale\n", ); process.exitCode = 2; } diff --git a/scripts/docs-proposal/hunks.test.mjs b/scripts/docs-proposal/hunks.test.mjs index e4a8b2df9..49d952906 100644 --- a/scripts/docs-proposal/hunks.test.mjs +++ b/scripts/docs-proposal/hunks.test.mjs @@ -5,6 +5,7 @@ import { buildReview, collectHunks, indexEvidence, + inertBlock, listHunks, neutralizeMentions, parseDiff, @@ -208,6 +209,24 @@ test("neutralizeMentions keeps text but breaks every mention", () => { ); }); +test("inertBlock fences the rationale beyond any backtick run", () => { + assert.equal( + inertBlock("Closes #12, see [x](https://example.com) and `a` ```b```\n\n"), + [ + "````text", + "Closes #12, see [x](https://example.com) and `a` ```b```", + "````", + "", + ].join("\n"), + "should wrap the text in a longer fence and drop trailing newlines", + ); + assert.equal( + inertBlock("ask @team\n"), + "```text\nask @\u200bteam\n```\n", + "should still neutralize mentions", + ); +}); + test("buildReview cites evidence and suggests reverts", () => { const files = parseDiff( [ @@ -296,6 +315,41 @@ test("buildReview lists binary and unanchored changes in the body", () => { ); }); +test("buildReview asks to delete a new file instead of emptying it", () => { + const files = parseDiff( + [ + "diff --git a/docs/guide/new.md b/docs/guide/new.md", + "new file mode 100644", + "--- /dev/null", + "+++ b/docs/guide/new.md", + "@@ -0,0 +1,2 @@", + "+# New", + "+page", + "", + ].join("\n"), + ); + const [entry] = collectHunks(files); + const evidence = indexEvidence({ + hunks: [{ id: entry.id, source: "crates/a.rs:1", reason: "New page." }], + }); + const [comment] = buildReview(files, evidence, "abc123").comments; + assert.equal( + comment.path, + "docs/guide/new.md", + "should anchor on the new file", + ); + assert.equal(comment.line, 2, "should end at the last new line"); + assert.equal( + comment.body, + [ + "**Source:** `crates/a.rs:1` — New page.", + "", + "This hunk creates the file. Delete the file to revert it.", + ].join("\n"), + "should keep the evidence and offer no empty suggestion", + ); +}); + test("buildReview adds start_line for multi-line spans", () => { const files = parseDiff( [ diff --git a/scripts/docs-proposal/lib.sh b/scripts/docs-proposal/lib.sh index f1668987a..5dc699583 100644 --- a/scripts/docs-proposal/lib.sh +++ b/scripts/docs-proposal/lib.sh @@ -38,7 +38,8 @@ docs_proposal_disallowed_paths() { } # Applies to the index on a detached checkout of , then exits -# when it changes paths outside the allowlist. +# when it changes paths outside the allowlist or entries that are not regular +# files. docs_proposal_apply() { local sha="$1" local patch="$2" @@ -49,6 +50,13 @@ docs_proposal_apply() { printf '::error::Proposal changes paths outside docs/guide/** and docs/index.md:\n%s\n' "$disallowed" exit 1 fi + # A symlink passes the path check, but VitePress builds and publishes its + # target, so a proposal may only add, edit, or delete regular files. + disallowed="$(git diff --cached --raw --no-renames "$sha" | awk '$2 != "100644" && $2 != "000000"')" + if [ -n "$disallowed" ]; then + printf '::error::Proposal changes entries that are not regular files:\n%s\n' "$disallowed" + exit 1 + fi } docs_proposal_pr_body() { @@ -68,10 +76,10 @@ docs_proposal_pr_body() { printf 'Generated by GitHub Copilot CLI in the docs-proposal workflow. A maintainer must verify every hunk against the merged code before merging. Each hunk has a review comment citing its source and a suggestion that reverts it.\n\n' printf '## Rationale\n\n' if [ -s "$rationale" ]; then - # The rationale is model output; keep it from pinging users or teams. + # The rationale is model output; render it inert so it cannot ping users, + # reference or close issues, or embed links, images, or HTML. head -c "$DOCS_PROPOSAL_MAX_RATIONALE_BYTES" "$rationale" \ - | node "$DOCS_PROPOSAL_DIR/hunks.mjs" sanitize - printf '\n' + | node "$DOCS_PROPOSAL_DIR/hunks.mjs" rationale else printf '_Copilot did not provide a rationale._\n' fi diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh index a300e05b2..e3f421348 100755 --- a/scripts/docs-proposal/propose.sh +++ b/scripts/docs-proposal/propose.sh @@ -27,6 +27,8 @@ if [ -z "$base" ] || [ "$base" = "$sha" ] || ! git merge-base --is-ancestor "$ba fi base="$(git rev-parse "$sha^1")" fi +# A manual base such as `main` resolves here, so publish.sh gets a full SHA. +base="$(git rev-parse --verify "$base^{commit}")" mkdir -p "$2" work_dir="$(cd "$2" && pwd)" # publish.sh records the base so a retry over a different range is refused. @@ -34,11 +36,17 @@ printf '%s\n' "$base" > "$work_dir/base" relative_dir="${work_dir#"$root"/}" # Prints a prompt template with each `` replaced by the work -# directory, relative to the repository root. +# directory, relative to the repository root. Splits on the placeholder +# instead of using ${text//pattern/replacement}: bash 3.2 keeps quotes in the +# replacement literally, and bash 5.2 expands an unquoted & in it. render_prompt() { - local text - text="$(cat "$1")" - printf '%s\n' "${text///"$relative_dir"}" + local rest rendered="" + rest="$(cat "$1")" + while [[ "$rest" == *""* ]]; do + rendered+="${rest%%*}$relative_dir" + rest="${rest#*}" + done + printf '%s\n' "$rendered$rest" } # Reads are always permitted; writes are allowed so Copilot can edit pages, @@ -81,7 +89,11 @@ fi git diff --cached --no-renames "$sha" -- docs | node "$here/hunks.mjs" list printf '```\n' } > "$work_dir/evidence-prompt.md" -run_copilot evidence-prompt.md +# Evidence is advisory: hunks without it are posted as "none cited", so a +# failed evidence run must not discard the proposal. +if ! run_copilot evidence-prompt.md; then + printf '::warning::Copilot evidence run failed; hunks will be posted without cited sources.\n' +fi if [ ! -f "$work_dir/evidence.json" ]; then printf '{}\n' > "$work_dir/evidence.json" fi diff --git a/scripts/docs-proposal/publish.sh b/scripts/docs-proposal/publish.sh index 06049b8b3..33d4190c7 100755 --- a/scripts/docs-proposal/publish.sh +++ b/scripts/docs-proposal/publish.sh @@ -25,9 +25,10 @@ if ! [[ "$base" =~ ^[0-9a-f]{40}$ ]]; then fi # A closed or merged proposal is a maintainer decision; never reopen or -# recreate it on a retry. -existing="$(gh pr list --head "$branch" --state all --json number,state,body \ - --jq '.[0] | select(.) | "\(.number) \(.state) \([.body | match("").captures[0].string] | first // "")"')" +# recreate it on a retry. The head filter matches branch names only, so skip +# pull requests from forks that reuse the predictable proposal branch name. +existing="$(gh pr list --head "$branch" --state all --json number,state,body,isCrossRepository \ + --jq '[.[] | select(.isCrossRepository | not)] | .[0] | select(.) | "\(.number) \(.state) \([.body | match("").captures[0].string] | first // "")"')" read -r pr_number pr_state pr_base <<< "$existing" || true if [ -n "$existing" ] && [ "$pr_state" != "OPEN" ]; then printf 'Proposal #%s for %s is %s; not reopening it.\n' "$pr_number" "$sha" "$pr_state" @@ -69,8 +70,10 @@ if [ ! -s "$work_dir/proposal.patch" ]; then if [ -n "$remote_head" ]; then git push --quiet --force-with-lease="refs/heads/$branch:$remote_head" origin --delete "$branch" fi - gh pr close "$pr_number" \ - --comment "A rerun for $sha proposed no documentation changes." + # Deleting the head branch already closes the pull request, and + # gh pr close skips --comment on a closed one, so comment separately. + gh pr comment "$pr_number" --body "A rerun for $sha proposed no documentation changes." + gh pr close "$pr_number" fi exit 0 fi diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index e0e0863ef..139e80bdc 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -47,7 +47,8 @@ assert_contains "$body" "" "should embed t assert_contains "$body" "inspected fedcba987654..0123456789ab" "should show the inspected range" assert_contains "$body" "merged change $sha (#42): Add example flag" "should link the merge and its PR" assert_contains "$body" "documents the new flag" "should include the rationale" -assert_contains "$body" $'@\u200bexample-org/maintainers' "should neutralize mentions in the rationale" +assert_contains "$body" $'## Rationale\n\n```text\n- docs/guide/cli.md' "should render the rationale as an inert text block" +assert_contains "$body" "$(printf '@\342\200\213example-org/maintainers')" "should neutralize mentions in the rationale" : > "$rationale" body="$(docs_proposal_pr_body "$sha" "$base_sha" "Add example flag" "" "$rationale")" assert_contains "$body" "merged change $sha: Add example flag" "should omit a missing origin PR" @@ -72,7 +73,16 @@ cat > "$tmp/bin/gh" <<'STUB' #!/usr/bin/env bash printf '%s\n' "$*" >> "$STUB_LOG" case "$1 $2" in - "pr list") printf '%s' "${STUB_PR:-}" ;; + "pr list") + # STUB_PR_JSON is filtered through the real --jq expression; STUB_PR is + # printed as the already-filtered result. + if [ -n "${STUB_PR_JSON:-}" ]; then + while [ "$#" -gt 0 ] && [ "$1" != --jq ]; do shift; done + jq -r "$2" <<< "$STUB_PR_JSON" + else + printf '%s' "${STUB_PR:-}" + fi + ;; "pr create") printf 'https://github.com/example/repo/pull/7\n' ;; "api repos/{owner}/{repo}/commits/"*) printf '%s' "${STUB_ORIGIN_PR:-}" ;; "api --paginate") printf '%s' "${STUB_REVIEWS:-}" ;; @@ -92,6 +102,13 @@ STUB cat > "$tmp/bin/copilot" <<'STUB' #!/usr/bin/env bash printf 'copilot %s\n' "$*" >> "$STUB_LOG" +# The proposal run edits a page when asked, so the evidence half runs too. +if [ -n "${STUB_COPILOT_EDIT:-}" ] && [[ "$2" == *"/prompt.md "* ]]; then + printf 'proposed\n' >> "$STUB_COPILOT_EDIT" +fi +if [ -n "${STUB_COPILOT_FAIL_EVIDENCE:-}" ] && [[ "$2" == *"/evidence-prompt.md "* ]]; then + exit 1 +fi STUB # Wraps git so a test can fail the proposal branch fetch, or land a # maintainer push right after publish.sh inspects the branch head. @@ -174,6 +191,22 @@ make_patch docs/superpowers/notes.md assert_eq "$(run_validate)" 1 "should reject a disallowed path before the docs gates" assert_eq "$(grep -c 'npm' "$STUB_LOG" || true)" 0 "should not build a disallowed proposal" +# A symlink under docs/guide passes the path check but would publish its +# target, so both validation and publication refuse it. +make_symlink_patch() { + make_patch "" + ln -s ../superpowers/notes.md docs/guide/link.md + git add -A && git diff --cached --binary "$merge_sha" > "$tmp/proposal/proposal.patch" + git reset -q --hard "$merge_sha" && git clean -qfd +} +make_symlink_patch +assert_eq "$(run_validate)" 1 "should reject a symlink under docs/guide" +assert_eq "$(grep -c 'npm' "$STUB_LOG" || true)" 0 "should not build a symlinked proposal" +assert_contains "$(cat "$tmp/out.log")" "not regular files" "should explain why a symlink is rejected" +make_symlink_patch +assert_eq "$(STUB_PR="" run_publish)" 1 "should refuse to publish a symlink" +assert_eq "$(remote_has_branch)" no "should not push a symlinked proposal" + make_patch docs/guide/cli.md assert_eq "$(run_validate)" 0 "should validate an allowed proposal" assert_contains "$(cat "$STUB_LOG")" "npm run build" "should run the docs build" @@ -203,6 +236,17 @@ assert_eq "$(STUB_PR="7 OPEN $base_sha" STUB_REVIEWS=101 run_publish)" 0 "should assert_eq "$(remote_head)" "$published_head" "should not push an unchanged proposal" assert_eq "$(grep -c 'pr create\|--method POST' "$STUB_LOG" || true)" 0 "should not re-post an unchanged proposal" +# A fork can open a pull request from a branch with the predictable proposal +# name; only the same-repository proposal counts. +marker="" +fork_and_own="[{\"number\":9,\"state\":\"CLOSED\",\"body\":\"$marker\",\"isCrossRepository\":true},{\"number\":7,\"state\":\"OPEN\",\"body\":\"$marker\",\"isCrossRepository\":false}]" +assert_eq "$(STUB_PR_JSON="$fork_and_own" STUB_REVIEWS=101 run_publish)" 0 "should skip a newer fork PR with the proposal branch name" +assert_contains "$(cat "$STUB_LOG")" "pr edit 7" "should update the same-repository proposal" +assert_eq "$(grep -c 'pr edit 9' "$STUB_LOG" || true)" 0 "should never edit a fork PR" +only_fork="[{\"number\":9,\"state\":\"OPEN\",\"body\":\"$marker\",\"isCrossRepository\":true}]" +assert_eq "$(STUB_PR_JSON="$only_fork" STUB_REVIEWS=101 run_publish)" 0 "should ignore a fork PR when no proposal exists" +assert_contains "$(cat "$STUB_LOG")" "pr create --base main --head $branch" "should open the proposal despite a fork PR" + narrow_base=0000000000000000000000000000000000000001 make_patch "" "$narrow_base" assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 1 "should fail an empty retry over a different range" @@ -281,7 +325,8 @@ git push -q --force origin "$pushed_head:refs/heads/$branch" make_patch "" assert_eq "$(STUB_PR="7 OPEN $base_sha" run_publish)" 0 "should succeed when an empty rerun deletes the proposal" assert_eq "$(remote_has_branch)" no "should delete an unmaintained proposal branch" -assert_contains "$(cat "$STUB_LOG")" "pr close 7 --comment" "should close the proposal after deleting its branch" +assert_contains "$(cat "$STUB_LOG")" "pr comment 7 --body" "should explain why the proposal closed" +assert_contains "$(cat "$STUB_LOG")" "pr close 7" "should close the proposal after deleting its branch" assert_eq "$(grep -c 'delete-branch' "$STUB_LOG" || true)" 0 "should not let gh delete the branch" # A push that added two commits is inspected from the previous main head; a @@ -313,10 +358,10 @@ assert_contains "$(cat "$STUB_LOG")" "copilot -p" "should run Copilot" assert_contains "$(cat .docs-proposal/prompt.md)" ".docs-proposal/rationale.md" "should name the work dir in the prompt" assert_eq "$(grep -c '' .docs-proposal/prompt.md || true)" 0 "should fill every work dir placeholder" -rm -rf proposal-out -"$here/propose.sh" "$second_sha" proposal-out "" > "$tmp/out.log" 2>&1 -assert_contains "$(cat proposal-out/prompt.md)" "proposal-out/rationale.md" "should name a custom work dir in the prompt" -rm -rf proposal-out +rm -rf 'proposal&out' +"$here/propose.sh" "$second_sha" 'proposal&out' "" > "$tmp/out.log" 2>&1 +assert_contains "$(cat 'proposal&out/prompt.md')" "proposal&out/rationale.md" "should name a custom work dir in the prompt" +rm -rf 'proposal&out' assert_eq "$(run_propose "")" 0 "should propose without a base" assert_eq "$(cat .docs-proposal/base)" "$first_sha" "should default the base to the first parent" @@ -329,6 +374,20 @@ assert_eq "$(run_propose "$published_head")" 0 "should propose with an unrelated assert_eq "$(cat .docs-proposal/base)" "$first_sha" "should replace an unrelated base with the first parent" assert_contains "$(cat "$tmp/out.log")" "is not an ancestor" "should warn about an unrelated base" +assert_eq "$(run_propose "$second_sha~2")" 0 "should propose with a revision expression as the base" +assert_eq "$(cat .docs-proposal/base)" "$merge_sha" "should record the base as a full SHA" + +assert_eq "$(STUB_COPILOT_EDIT=docs/guide/cli.md run_propose "$merge_sha")" 0 "should propose an edited page" +assert_contains "$(cat .docs-proposal/proposal.patch)" "+proposed" "should write the edit to the patch" +assert_contains "$(cat .docs-proposal/evidence-prompt.md)" '"path": "docs/guide/cli.md"' "should list the edited hunk for evidence" +assert_eq "$(grep -c '^copilot' "$STUB_LOG")" 2 "should run Copilot again for evidence" + +git reset -q --hard "$second_sha" +assert_eq "$(STUB_COPILOT_EDIT=docs/guide/cli.md STUB_COPILOT_FAIL_EVIDENCE=1 run_propose "$merge_sha")" 0 "should keep the proposal when the evidence run fails" +assert_contains "$(cat .docs-proposal/proposal.patch)" "+proposed" "should keep the patch when the evidence run fails" +assert_eq "$(cat .docs-proposal/evidence.json)" "{}" "should fall back to no evidence" +assert_contains "$(cat "$tmp/out.log")" "evidence run failed" "should warn that the evidence run failed" + cd "$here" if [ "$failures" -gt 0 ]; then From 80286ec0d1c9da7a25ab606488c2d97c79a55023 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Thu, 15 Oct 2026 14:41:31 +0530 Subject: [PATCH 14/14] Harden docs proposal evidence rendering and test prerequisites Render evidence reasons as inline code so links, images, HTML, and issue references stay inert, tell Copilot new pages need a sidebar entry, fail test.sh early when jq is missing, and document that the propose job allows arbitrary code execution and that privileged workflows must not restore caches. Signed-off-by: dhruv8sh --- .github/workflows/docs-proposal.yml | 2 ++ scripts/docs-proposal/hunks.mjs | 4 +++- scripts/docs-proposal/hunks.test.mjs | 30 ++++++++++++++++++++++++++-- scripts/docs-proposal/prompt.md | 3 +++ scripts/docs-proposal/propose.sh | 6 +++++- scripts/docs-proposal/test.sh | 6 ++++++ 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/.github/workflows/docs-proposal.yml b/.github/workflows/docs-proposal.yml index 4d8f4aecc..04708da74 100644 --- a/.github/workflows/docs-proposal.yml +++ b/.github/workflows/docs-proposal.yml @@ -50,6 +50,8 @@ jobs: # Copilot runs here with a read-only token; it cannot push or open PRs. # Copilot can write files that later steps execute, so treat this job as # untrusted: it uses no dependency cache that later runs would restore. + # Because untrusted code runs on the main ref, it can still write other + # main-scoped caches, so privileged workflows must never restore caches. propose: runs-on: ubuntu-latest timeout-minutes: 30 diff --git a/scripts/docs-proposal/hunks.mjs b/scripts/docs-proposal/hunks.mjs index 95c81aa4c..edd934162 100644 --- a/scripts/docs-proposal/hunks.mjs +++ b/scripts/docs-proposal/hunks.mjs @@ -205,7 +205,9 @@ function commentBody(cited, body) { const source = cited?.source ? `\`${cited.source}\`` : "none cited — verify this hunk against the merged code."; - const reason = cited?.reason ? ` — ${cited.reason}` : ""; + // The reason is model output; inline code keeps its links, images, HTML, + // and issue references inert (sanitize already removed backticks). + const reason = cited?.reason ? ` — \`${cited.reason}\`` : ""; const heading = `**Source:** ${source}${reason}`; if (body === null) { return [ diff --git a/scripts/docs-proposal/hunks.test.mjs b/scripts/docs-proposal/hunks.test.mjs index 49d952906..b42decc07 100644 --- a/scripts/docs-proposal/hunks.test.mjs +++ b/scripts/docs-proposal/hunks.test.mjs @@ -266,7 +266,7 @@ test("buildReview cites evidence and suggests reverts", () => { line: 2, side: "RIGHT", body: [ - "**Source:** `crates/a.rs:9` — Renames the flag.", + "**Source:** `crates/a.rs:9` — `Renames the flag.`", "", "Apply this suggestion to revert the hunk.", "", @@ -342,7 +342,7 @@ test("buildReview asks to delete a new file instead of emptying it", () => { assert.equal( comment.body, [ - "**Source:** `crates/a.rs:1` — New page.", + "**Source:** `crates/a.rs:1` — `New page.`", "", "This hunk creates the file. Delete the file to revert it.", ].join("\n"), @@ -370,3 +370,29 @@ test("buildReview adds start_line for multi-line spans", () => { ); assert.equal(comment.line, 2, "should end at the last new line"); }); + +test("buildReview renders the evidence reason as inline code", () => { + const files = parseDiff( + [ + "diff --git a/docs/guide/a.md b/docs/guide/a.md", + "--- a/docs/guide/a.md", + "+++ b/docs/guide/a.md", + "@@ -1 +1 @@", + "-a", + "+b", + "", + ].join("\n"), + ); + const [hunk] = collectHunks(files); + const reason = + "See #1 and [click](https://example.com) ![](https://example.com/p.png) "; + const evidence = indexEvidence({ + hunks: [{ id: hunk.id, source: "x.rs:1", reason }], + }); + const [comment] = buildReview(files, evidence, "abc123").comments; + + assert.ok( + comment.body.startsWith(`**Source:** \`x.rs:1\` — \`${reason}\`\n`), + "should wrap the reason in inline code so Markdown and HTML stay inert", + ); +}); diff --git a/scripts/docs-proposal/prompt.md b/scripts/docs-proposal/prompt.md index 9dfa072d3..09f8677f5 100644 --- a/scripts/docs-proposal/prompt.md +++ b/scripts/docs-proposal/prompt.md @@ -35,3 +35,6 @@ sources to documentation pages appear at the end of this file. relative paths. - Do not edit files outside `docs/guide/`, `docs/index.md`, and `/`. Do not commit, push, or create branches. +- Prefer extending an existing page. The sidebar is in + `docs/.vitepress/config.mts`, which you cannot edit, so when a new page is + unavoidable, say in the rationale that it needs a sidebar entry. diff --git a/scripts/docs-proposal/propose.sh b/scripts/docs-proposal/propose.sh index e3f421348..033170eb6 100755 --- a/scripts/docs-proposal/propose.sh +++ b/scripts/docs-proposal/propose.sh @@ -50,7 +50,11 @@ render_prompt() { } # Reads are always permitted; writes are allowed so Copilot can edit pages, -# and publish.sh enforces which paths may actually change. +# and publish.sh enforces which paths may actually change. Treat this job as +# allowing arbitrary code execution: `git diff`/`git log --output` can write +# any file, and writes can add diff.external or textconv to .git/config. The +# job therefore holds a read-only token and its artifact is re-checked +# downstream. run_copilot() { copilot -p "Read $relative_dir/$1 and follow its instructions exactly." \ --no-ask-user \ diff --git a/scripts/docs-proposal/test.sh b/scripts/docs-proposal/test.sh index 139e80bdc..5b075feb3 100755 --- a/scripts/docs-proposal/test.sh +++ b/scripts/docs-proposal/test.sh @@ -8,6 +8,12 @@ here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # shellcheck source=scripts/docs-proposal/lib.sh source "$here/lib.sh" +# The gh stub runs the real --jq filters through jq. +if ! command -v jq > /dev/null; then + printf 'test.sh needs jq on PATH for its gh stub.\n' >&2 + exit 1 +fi + failures=0 assert_eq() {