From 5427f6b024fd0c79638c7bd552f7e94283ff06a5 Mon Sep 17 00:00:00 2001 From: Eva Sarafianou Date: Mon, 28 Sep 2026 18:18:51 +0300 Subject: [PATCH] Draft docs from merged Docs/Needed via author personas (#38764) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: draft pages from merged Docs/Needed via author personas When a PR merges still carrying Docs/Needed, read the gap sticky comment, author MDX with the matching audience personas (pre-reviewed in-loop), and open docs/pr-. Closing that draft flips Docs/Needed to Docs/Done. Retires the cross-repo docs-needed.yml workflow. Co-authored-by: Cursor * fix(docs-ai): harden writer fence parse, version check, and brief fallback Match CodeRabbit findings that still hold: outer fences of equal length so nested code samples survive; require the milestone version (not any vX.Y); fall back to PR evidence when the gap sticky is missing; narrow the App token. Keep pull_request (not pull_request_target) to match the gap pipeline's fork posture. Co-authored-by: Cursor * fix(docs-ai): tighten author path scope, ERROR halt, and sync provenance Restrict each author pass to its group targets; fail the loop on review ERROR instead of opening a half-reviewed draft; require same-repo head and DOCS_AI_BOT_LOGIN before flipping Docs/Done. Also cover additionsDiff edge cases and longest-prefix author matching. Co-authored-by: Cursor * docs: lift observability and parity guidance into docs-writer prompt Carry the useful bits from draft_docs.md without restoring triage: parity vs net-new as a one-line scope rule, plus when to document logs/metrics. Co-authored-by: Cursor * fix(docs-ai): require author passes to cover every group target Emitted ⊆ allowlist was not enough — an incomplete pass could drop a named page. Reject initial omissions; keep prior groupFiles when a revision fails to re-emit every target. Co-authored-by: Cursor * fix(docs-ai): harden revision limit, impact paths, and anchor reject Validate DOCS_AI_MAX_REVISIONS as a non-negative integer; filter gap impact locations to allowed .md/.mdx before grouping; keep the prior pass when a revision drops the milestone version anchor. Co-authored-by: Cursor * fix(docs-ai): drop AI-Drafted label; frame unknown versions Identify drafts by App authorship instead of Docs/AI-Drafted. Require "From Mattermost [NOT PRESENT — …]" for unknown releases, and replace the DEBUG observability example with admin-facing cases. Co-authored-by: Cursor * fix(docs-ai): address writer review on sync, paths, and author context Require draft merge before Docs/Done, filter deny-listed paths at extraction, prefer branch provenance, pass existing pages into the author prompt, and tighten workflow input/credential handling. Co-authored-by: Cursor * fix(docs-ai): keep full page context on author and revision passes Reject oversized pages instead of truncating them, feed the in-memory draft into revision prompts, and tighten line-based MDX import/export checks. Co-authored-by: Cursor --------- Co-authored-by: Cursor --- .github/prompts/conventions.md | 10 +- .github/prompts/docs-writer.md | 88 +++ .github/scripts/docs-ai/README.md | 62 +- .github/scripts/docs-ai/lib/diff.mjs | 30 + .github/scripts/docs-ai/lib/diff.test.mjs | 52 ++ .github/scripts/docs-ai/lib/personas.mjs | 58 ++ .github/scripts/docs-ai/package.json | 2 +- .github/scripts/docs-ai/prompts.test.mjs | 20 +- .../scripts/docs-ai/review/persona-review.mjs | 83 ++- .../scripts/docs-ai/writer/author-loop.mjs | 404 +++++++++++ .github/scripts/docs-ai/writer/files.mjs | 107 +++ .github/scripts/docs-ai/writer/gap-brief.mjs | 73 ++ .github/scripts/docs-ai/writer/paths.mjs | 48 ++ .github/scripts/docs-ai/writer/run.mjs | 124 ++++ .github/scripts/docs-ai/writer/sync.mjs | 78 +++ .../scripts/docs-ai/writer/writer.test.mjs | 487 ++++++++++++++ .github/workflows/docs-needed.yml | 628 ------------------ .github/workflows/docs-writer-merged-pr.yml | 302 +++++++++ 18 files changed, 1969 insertions(+), 687 deletions(-) create mode 100644 .github/prompts/docs-writer.md create mode 100644 .github/scripts/docs-ai/lib/diff.mjs create mode 100644 .github/scripts/docs-ai/lib/diff.test.mjs create mode 100644 .github/scripts/docs-ai/writer/author-loop.mjs create mode 100644 .github/scripts/docs-ai/writer/files.mjs create mode 100644 .github/scripts/docs-ai/writer/gap-brief.mjs create mode 100644 .github/scripts/docs-ai/writer/paths.mjs create mode 100644 .github/scripts/docs-ai/writer/run.mjs create mode 100644 .github/scripts/docs-ai/writer/sync.mjs create mode 100644 .github/scripts/docs-ai/writer/writer.test.mjs delete mode 100644 .github/workflows/docs-needed.yml create mode 100644 .github/workflows/docs-writer-merged-pr.yml diff --git a/.github/prompts/conventions.md b/.github/prompts/conventions.md index 5770b0c4b35..1fe97bf22f4 100644 --- a/.github/prompts/conventions.md +++ b/.github/prompts/conventions.md @@ -14,8 +14,9 @@ to, in this form: **Authoring.** Never invent a version or infer one from a branch name or date; a wrong version is worse than none, because readers believe it. Take it from the PR or issue -milestone. With no milestone, write `[NOT PRESENT — REQUIRES HUMAN JUDGMENT]` and leave -it for a human. +milestone. With no milestone, write `From Mattermost [NOT PRESENT — REQUIRES HUMAN JUDGMENT]` +and leave it for a human — keep the "From Mattermost …" frame so readers see a version +is missing. **Reviewing.** Check the anchor, never supply one. @@ -23,8 +24,9 @@ it for a human. knows when their own work ships and you do not. - Present — accept it. You have no release schedule to check it against, so a version is not suspect merely because you cannot verify it. -- `[NOT PRESENT — REQUIRES HUMAN JUDGMENT]` still in the content — always a finding, and - one only a human can clear. Report it; never try to resolve it by supplying a version. +- `From Mattermost [NOT PRESENT — REQUIRES HUMAN JUDGMENT]` still in the content — always + a finding, and one only a human can clear. Report it; never try to resolve it by + supplying a version. ## Frontmatter diff --git a/.github/prompts/docs-writer.md b/.github/prompts/docs-writer.md new file mode 100644 index 00000000000..7e4caf25956 --- /dev/null +++ b/.github/prompts/docs-writer.md @@ -0,0 +1,88 @@ +# Docs writer + +You draft Mattermost documentation pages on behalf of the technical writing team. +You receive untrusted PR evidence and a gap brief; you emit MDX file blocks only. + +## Role + +Write the pages named in the brief so a reader in your audience lens can act on the +merged change. Prefer updating an existing page over inventing a new one when the +brief names a path that already exists. + +Gap analysis already decided that documentation is needed and named targets. Your job +is to close that brief, not re-triage the PR. + +## Scope + +Parity (closing a gap or removing a limitation) → smallest update to existing pages, +often a version line and a short behavioural note. Net-new capability → a new page only +when the brief asks for one; otherwise extend the named existing page. + +Ask: what can the reader do now that they could not before? Answer in one sentence and +write only what that sentence requires. + +## Hard rules + +- Claims must map to evidence in the supplied PR metadata, description, code diff, or + gap brief. If evidence is absent, write `[NOT PRESENT IN PR — REQUIRES HUMAN JUDGMENT]` + at the claim site — never invent behaviour, UI labels, config keys, or defaults. +- Version anchoring comes only from `milestone.version` in the brief. When present, use + `From Mattermost vX.Y` for new or changed capability. When absent, write + `From Mattermost [NOT PRESENT — REQUIRES HUMAN JUDGMENT]` — never guess a version. +- Do not hand-author `docs/api/reference/**`, `docs/site/**`, `docs/styles/**`, + `docs/pdf/**`, `docs/vendor/**`, or `docs/main/agents/docs/**`. +- Do not edit changelogs, important upgrade notes, or version-archive pages unless the + brief explicitly requires it. +- Follow `conventions.md` for frontmatter, plan availability, callouts, links, MDX + escaping, heading case, and voice. + +## Observability and diagnostics + +When the PR adds logging, metrics, monitoring events, or diagnostic output: + +Document when the product already has logging/metrics/observability reference docs, when +the message helps admins troubleshoot or understand system behaviour, or when new log +levels, categories, configuration options, or audit events appear. + +Do not document internal debug or trace noise with no admin troubleshooting value, or +logging changes when the product has no observability documentation for that surface +(implementation-only logs). + +Examples to document: an Info/Warn log an admin uses to diagnose a failed job; a new +metric such as `api_request_duration_seconds`; a new audit event such as +`USER_PASSWORD_CHANGED`; a config option that changes what is logged. + +Examples not to document: "added DEBUG/trace logging in `processWidgets()`"; "improved +log formatting" when the meaning of the output is unchanged. + +## Anti-patterns + +- Do not document implementation details (code structure, internal algorithms) unless + an admin must operate on them (config keys, logs, metrics, CLI flags). +- Do not treat PR size as a proxy for docs scope. +- Prefer the smallest change that closes the gap. + +## Output format + +Return one or more fenced code blocks, each preceded by a `path=` attribute naming the +destination relative to the repository root. No prose between blocks. + +Use a **four-backtick** outer fence so any three-backtick code samples inside the page +do not close the block early: + +````mdx path=docs/main/administration-guide/configure/example.mdx +--- +title: "Example setting" +--- + +From Mattermost v11.7, … + +```bash +mmctl config get ServiceSettings.SiteURL +``` + +… +```` + +Include only files you create or modify. Every `path=` must be under `docs/main/`, +`docs/develop/`, or be exactly `docs/api/examples.mdx` / `docs/api/index.mdx`. diff --git a/.github/scripts/docs-ai/README.md b/.github/scripts/docs-ai/README.md index b0fd0de4080..548dd7f496c 100644 --- a/.github/scripts/docs-ai/README.md +++ b/.github/scripts/docs-ai/README.md @@ -1,15 +1,15 @@ # Docs AI -Two pipelines, one persona registry, one package. +Three pipelines, one persona registry, one package. | Workflow | Question | Runs on | | --- | --- | --- | | `docs-review.yml` | Is this prose right? | Manual `workflow_dispatch` with a PR number (while tuning) | | `docs-gap.yml` | Is a page missing? | Every PR | +| `docs-writer-merged-pr.yml` | Write the missing pages | PR merged with `Docs/Needed` | -Both are advisory. No verdict blocks a merge. `docs/api` is out of scope for review — its -reference pages are generated from the OpenAPI spec, so corrections belong in -`api/v4/source/` — and out of scope as a gap action for the same reason. +Review and gap are advisory. No verdict blocks a merge. `docs/api` reference pages are +generated from OpenAPI — out of scope for review and as a gap action. ## Layout @@ -17,7 +17,8 @@ reference pages are generated from the OpenAPI spec, so corrections belong in docs-ai/ review/ # persona review entry points (router, persona-review, report) gap/ # gap analysis entry points (prepare, report) + lifecycle helpers - lib/ # shared (anthropic, github, untrusted, personas) + writer/ # author loop + merged-PR entry + Docs/Done sync + lib/ # shared (anthropic, github, untrusted, personas, diff) package.json README.md ``` @@ -26,7 +27,7 @@ One `package.json`. Separate packages would only duplicate the lockfile and CI i ## How to run it -The workflow is manual while we tune personas. From the Actions tab, run +The review workflow is manual while we tune personas. From the Actions tab, run **Docs AI Review** with the target PR number (same-repo branches only). That posts one sticky comment on the PR and produces a single workflow run — no checks appear on other PRs. When the output is trustworthy, add a `pull_request` trigger back onto the same @@ -43,9 +44,11 @@ single job so it becomes one check rather than a matrix of them. | `review/report.mjs` | Upserts the sticky review comment | | `gap/prepare.mjs` | Collects diffs and renders the gap prompt | | `gap/report.mjs` | Posts the sticky gap comment and owns `Docs/Needed` | +| `writer/run.mjs` | Reads the gap sticky, runs the author loop, writes MDX | +| `writer/sync.mjs` | On merge of `docs/pr-*`, flips `Docs/Needed` → `Docs/Done` | `npm test` (run from this directory) is the registry validator the workflow also runs -before reviewing. +before reviewing or writing. ## Adding or changing a persona @@ -73,15 +76,16 @@ Every key is required except `code_signals`, which is required only with `impact Everything below the closing `---` is the persona's system prompt, sent verbatim. Write it as the brief you would give a human reviewer: who they are, what they are accountable -for, what to look for and in what order. +for, what to look for and in what order. The writer reframes the same body as an audience +lens when `scope` includes `author`. | Scope | Used by | | --- | --- | | `review` | Reviewing a docs PR | -| `author` | Writing a page | +| `author` | Writing a page (path → persona via `docs_paths`) | | `impact` | Deciding whether a code change needs docs | -`brand-voice` is always selected and cannot be dropped by the router. +`brand-voice` is always selected and cannot be dropped by the router. It never authors. An `impact` persona's `docs_paths` and `code_signals` are rendered into the gap prompt by `gap/prompt.mjs`, so the two pipelines cannot disagree about who reads what. Moving a @@ -89,8 +93,8 @@ persona's paths updates both in one edit. ## The Docs/Needed lifecycle -`gap/report.mjs` is the only writer of the label. It reads prior state back out of the -comment it wrote last time, so it can tell its own label from a human's: +`gap/report.mjs` is the only writer of the label on open PRs. It reads prior state back +out of the comment it wrote last time, so it can tell its own label from a human's: ``` @@ -104,7 +108,12 @@ comment it wrote last time, so it can tell its own label from a human's: | label, no state (human-applied) | `none` | leave it, note the override | | `Docs/Not Needed` or `Docs/Done` | any | skip entirely | -Three constraints to preserve if you change this: +When a PR **merges** still carrying `Docs/Needed`, `docs-writer-merged-pr.yml` reads the +sticky comment's recommended actions, authors MDX with the matching `author` personas, +pre-reviews in-loop, and opens `docs/pr-` as a draft (authored by the Docs AI App). +Closing that draft flips `Docs/Needed` → `Docs/Done` on the originating PR. + +Three constraints to preserve if you change the gap label logic: - **The label follows the verdict, not the presence of a docs diff.** A push that edits an unrelated page must not clear it, or the self-clearing behaviour is an escape hatch. @@ -113,13 +122,27 @@ Three constraints to preserve if you change this: - **A failed run keeps the prior state block.** Dropping it has the same effect as writing a label with no state. -Every branch renders locally without touching a PR: +Every gap branch renders locally without touching a PR: ```bash node gap/report.mjs --dry-run --result-file ./result.json \ --labels 'Docs/Needed' --prior-state '{"applied_by":"bot"}' ``` +## Writer credentials + +Opening a branch and PR needs a GitHub App (`DOCS_AI_APP_CLIENT_ID` / +`DOCS_AI_APP_PRIVATE_KEY`). The default `GITHUB_TOKEN` cannot push under org policy, and +PRs it opens would not re-trigger `docs-review.yml`. Soft-skip when unset. + +The milestone on the merged PR is mandatory: it is the only source of the +`From Mattermost vX.Y` version anchor. + +`docs-writer-merged-pr.yml`'s sync job also needs `DOCS_AI_BOT_LOGIN` (the App's +`{slug}[bot]` login). Label flips run only when the closed PR's head repo is this +repository and its author matches that login — branch name and body markers are not +provenance. + ## Where a rule belongs Anything that does not vary by audience goes in the shared prompts, stated once: @@ -128,10 +151,12 @@ Anything that does not vary by audience goes in the shared prompts, stated once: - `.github/prompts/review-contract.md` — JSON output contract and verdict semantics. Reviewers. - `.github/prompts/docs-gap-analysis.md` — the gap prompt. Gap analysis only. +- `.github/prompts/docs-writer.md` — authoring output contract and hard rules. Writers. -The first two are sent as a byte-identical prefix across every persona, so nothing in them -may be templated per persona. The gap prompt carries `{{DATA_NOTICE}}`, `{{INPUTS}}` and -`{{PERSONAS}}`; renaming or dropping one fails the render rather than reaching the model. +The conventions + contract (or conventions + writer prompt) prefixes are sent as a +byte-identical cacheable prefix across calls. The gap prompt carries `{{DATA_NOTICE}}`, +`{{INPUTS}}` and `{{PERSONAS}}`; renaming or dropping one fails the render rather than +reaching the model. ## Untrusted input @@ -142,4 +167,5 @@ unbounded monorepo diff out of the workflow's expression context. Model output is untrusted on the way back out. `clampResult()` strips HTML comment delimiters from every string before it reaches a comment — without that, a summary could -forge the `docs-gap-state` block the next run reads. +forge the `docs-gap-state` block the next run reads. The writer aborts when it sees no +`path=` blocks or when a milestone was set and a page lacks a version anchor. diff --git a/.github/scripts/docs-ai/lib/diff.mjs b/.github/scripts/docs-ai/lib/diff.mjs new file mode 100644 index 00000000000..f17f2b18f63 --- /dev/null +++ b/.github/scripts/docs-ai/lib/diff.mjs @@ -0,0 +1,30 @@ +/* + * Synthesise an all-additions unified diff so pre-open persona review takes the + * same prompt shape as docs-review.yml's git diff. + */ + +export function additionsDiff(files) { + const chunks = []; + for (const {path, content} of files) { + const lines = content === '' + ? [] + : (() => { + const body = content.endsWith('\n') ? content : `${content}\n`; + const parts = body.split('\n'); + // Trailing empty from the final newline — drop it so +++ counts match. + if (parts.at(-1) === '') parts.pop(); + return parts; + })(); + + const n = lines.length; + chunks.push( + `diff --git a/${path} b/${path}`, + `new file mode 100644`, + `--- /dev/null`, + `+++ b/${path}`, + n === 0 ? `@@ -0,0 +0,0 @@` : `@@ -0,0 +1,${n} @@`, + ...lines.map((l) => `+${l}`), + ); + } + return chunks.join('\n') + (chunks.length ? '\n' : ''); +} diff --git a/.github/scripts/docs-ai/lib/diff.test.mjs b/.github/scripts/docs-ai/lib/diff.test.mjs new file mode 100644 index 00000000000..dda45fb6078 --- /dev/null +++ b/.github/scripts/docs-ai/lib/diff.test.mjs @@ -0,0 +1,52 @@ +import {test} from 'node:test'; +import assert from 'node:assert/strict'; +import {additionsDiff} from './diff.mjs'; + +function hunkHeader(diff) { + const m = diff.match(/@@ -0,0 \+(\d+)(?:,(\d+))? @@/); + if (!m) return null; + // +0,0 → count 0; +1,N → count N; +N alone is not used here. + if (m[2] !== undefined) return Number(m[2]); + return Number(m[1]); +} + +test('additionsDiff normalises a file with no trailing newline', () => { + const diff = additionsDiff([{path: 'docs/main/a.mdx', content: 'line1\nline2'}]); + assert.ok(diff.endsWith('\n')); + assert.equal(hunkHeader(diff), 2); + assert.match(diff, /\+line1\n\+line2\n/); + assert.doesNotMatch(diff, /\+line2\n\+\n/); +}); + +test('additionsDiff drops the phantom empty line after a trailing newline', () => { + const diff = additionsDiff([{path: 'docs/main/a.mdx', content: 'line1\nline2\n'}]); + assert.equal(hunkHeader(diff), 2); + assert.match(diff, /\+line1\n\+line2\n$/); +}); + +test('additionsDiff handles empty content without crashing', () => { + const diff = additionsDiff([{path: 'docs/main/empty.mdx', content: ''}]); + assert.match(diff, /diff --git a\/docs\/main\/empty\.mdx/); + assert.match(diff, /new file mode 100644/); + assert.match(diff, /@@ -0,0 \+0,0 @@/); + assert.equal(hunkHeader(diff), 0); + // No content lines after the hunk header (+++ file header still uses +). + assert.equal((diff.split(/@@[^\n]*\n/)[1] ?? '').trim(), ''); + assert.ok(diff.endsWith('\n')); +}); + +test('additionsDiff joins multiple files as separate chunks', () => { + const diff = additionsDiff([ + {path: 'docs/main/a.mdx', content: 'A\n'}, + {path: 'docs/main/b.mdx', content: 'B\n'}, + ]); + const a = diff.indexOf('+++ b/docs/main/a.mdx'); + const b = diff.indexOf('+++ b/docs/main/b.mdx'); + assert.ok(a !== -1 && b !== -1 && a < b); + assert.match(diff, /\+A\n/); + assert.match(diff, /\+B\n/); +}); + +test('additionsDiff returns empty string for no files', () => { + assert.equal(additionsDiff([]), ''); +}); diff --git a/.github/scripts/docs-ai/lib/personas.mjs b/.github/scripts/docs-ai/lib/personas.mjs index c079aa41c1d..c2ada4c1551 100644 --- a/.github/scripts/docs-ai/lib/personas.mjs +++ b/.github/scripts/docs-ai/lib/personas.mjs @@ -153,4 +153,62 @@ export function reviewSystemBlocks(id) { ]; } +export function writerPrompt() { + return readFileSync(join(PROMPTS_DIR, 'docs-writer.md'), 'utf8').trim(); +} + +// Authoring persona body is a review rubric; the writer prompt reframes it as +// the audience lens. Same cache prefix (conventions) as reviewers. +export function authorSystemBlocks(id) { + const persona = getPersona(id); + return [ + {type: 'text', text: conventions(), cache_control: {type: 'ephemeral'}}, + {type: 'text', text: writerPrompt(), cache_control: {type: 'ephemeral'}}, + { + type: 'text', + text: `## Audience lens: ${persona.label}\n\nThe brief below describes the reader you write for. Author for them; do not emit review JSON.\n\n${persona.prompt}`, + cache_control: {type: 'ephemeral'}, + }, + ]; +} + +export function neutralAuthorSystemBlocks() { + return [ + {type: 'text', text: conventions(), cache_control: {type: 'ephemeral'}}, + {type: 'text', text: writerPrompt(), cache_control: {type: 'ephemeral'}}, + { + type: 'text', + text: '## Audience lens: general technical writer\n\nNo path-matched authoring persona applied. Write clear operator-facing documentation.', + cache_control: {type: 'ephemeral'}, + }, + ]; +} + +/** Longest matching docs_paths prefix wins among author-scoped personas. */ +export function authorForPath(relPath) { + const authors = personasWithScope('author'); + let best = null; + let bestLen = -1; + for (const p of authors) { + for (const prefix of p.docsPaths) { + if ((relPath === prefix || relPath.startsWith(`${prefix}/`)) && prefix.length > bestLen) { + best = p; + bestLen = prefix.length; + } + } + } + return best; +} + +export function groupPathsByAuthor(paths) { + const groups = new Map(); // personaId | 'neutral' → paths[] + for (const path of paths) { + const persona = authorForPath(path); + const key = persona?.id ?? 'neutral'; + if (!groups.has(key)) groups.set(key, {personaId: persona?.id ?? null, paths: []}); + groups.get(key).paths.push(path); + } + return [...groups.values()]; +} + export {PERSONAS_DIR, PROMPTS_DIR, REPO_ROOT}; diff --git a/.github/scripts/docs-ai/package.json b/.github/scripts/docs-ai/package.json index 710acf3aeb5..79a2ea288fb 100644 --- a/.github/scripts/docs-ai/package.json +++ b/.github/scripts/docs-ai/package.json @@ -3,7 +3,7 @@ "version": "1.0.0", "private": true, "type": "module", - "description": "AI docs pipelines: persona review (review/) and gap analysis (gap/), with shared lib/.", + "description": "AI docs pipelines: persona review (review/), gap analysis (gap/), and writer (writer/), with shared lib/.", "scripts": { "test": "node --test" }, diff --git a/.github/scripts/docs-ai/prompts.test.mjs b/.github/scripts/docs-ai/prompts.test.mjs index 33a8ec921aa..19fde7ba8b5 100644 --- a/.github/scripts/docs-ai/prompts.test.mjs +++ b/.github/scripts/docs-ai/prompts.test.mjs @@ -10,7 +10,7 @@ import {test} from 'node:test'; import assert from 'node:assert/strict'; -import {conventions, reviewContract} from './lib/personas.mjs'; +import {conventions, reviewContract, writerPrompt} from './lib/personas.mjs'; test('conventions state the version anchoring rule', () => { // The requirement most likely to be silently dropped in an edit, and the @@ -63,3 +63,21 @@ test('review contract names the fields the reviewer parses', () => { } assert.match(contract, /At most three findings/); }); + +test('writer prompt states the path= output contract and version rule', () => { + const prompt = writerPrompt(); + assert.match(prompt, /path=/); + assert.match(prompt, /From Mattermost vX\.Y/); + assert.match(prompt, /From Mattermost \[NOT PRESENT — REQUIRES HUMAN JUDGMENT\]/); + assert.match(prompt, /milestone\.version/); + assert.match(prompt, /docs\/api\/reference/); +}); + +test('writer prompt states scope and observability guidance', () => { + const prompt = writerPrompt(); + assert.match(prompt, /Parity/); + assert.match(prompt, /Net-new/); + assert.match(prompt, /Observability and diagnostics/); + assert.match(prompt, /USER_PASSWORD_CHANGED/); + assert.match(prompt, /processWidgets/); +}); diff --git a/.github/scripts/docs-ai/review/persona-review.mjs b/.github/scripts/docs-ai/review/persona-review.mjs index 324051cb963..8b3fc97b52b 100644 --- a/.github/scripts/docs-ai/review/persona-review.mjs +++ b/.github/scripts/docs-ai/review/persona-review.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node import {readFileSync, writeFileSync, mkdirSync, existsSync} from 'node:fs'; import {dirname} from 'node:path'; +import {fileURLToPath} from 'node:url'; import {complete, parseJson, usageLine} from '../lib/anthropic.mjs'; import {getPersona, reviewSystemBlocks} from '../lib/personas.mjs'; import {DATA_NOTICE, block} from '../lib/untrusted.mjs'; @@ -19,7 +20,7 @@ function readIfSet(name) { return path && existsSync(path) ? readFileSync(path, 'utf8') : null; } -function buildUserPrompt({diff, prTitle, prBody}) { +export function buildReviewUserPrompt({diff, prTitle, prBody}) { const parts = [DATA_NOTICE, '']; if (prTitle) parts.push(block('pull-request-title', prTitle, {maxChars: 500}), ''); @@ -34,7 +35,7 @@ function buildUserPrompt({diff, prTitle, prBody}) { return parts.join('\n'); } -function normalize(parsed, persona) { +export function normalizeReview(parsed, persona, model = MODEL) { const verdict = VERDICTS.includes(parsed?.verdict) ? parsed.verdict : 'COMMENT'; const feedback = (Array.isArray(parsed?.feedback) ? parsed.feedback : []) @@ -47,7 +48,39 @@ function normalize(parsed, persona) { ? parsed.summary.trim().slice(0, 500) : 'No summary returned.'; - return {persona: persona.id, label: persona.label, verdict, summary, feedback, model: MODEL}; + return {persona: persona.id, label: persona.label, verdict, summary, feedback, model}; +} + +export async function reviewPersona({ + personaId, + diff, + prTitle, + prBody, + model = MODEL, + completeFn = complete, +}) { + const persona = getPersona(personaId); + try { + const {text, usage} = await completeFn({ + model, + system: reviewSystemBlocks(personaId), + userPrompt: buildReviewUserPrompt({diff, prTitle, prBody}), + maxTokens: 2048, + temperature: 0.2, + }); + console.error(`[${personaId}] ${model} ${usageLine(usage)}`); + return normalizeReview(parseJson(text), persona, model); + } catch (e) { + console.error(`[${personaId}] review failed: ${e.message}`); + return { + persona: persona.id, + label: persona.label, + verdict: 'ERROR', + summary: `Review did not complete: ${e.message}`.slice(0, 500), + feedback: [], + model, + }; + } } async function main() { @@ -58,41 +91,21 @@ async function main() { const out = arg('out'); if (!out) throw new Error('--out is required'); - const persona = getPersona(id); - let result; - - try { - const {text, usage} = await complete({ - model: MODEL, - system: reviewSystemBlocks(id), - userPrompt: buildUserPrompt({ - diff: readFileSync(diffPath, 'utf8'), - prTitle: arg('pr-title'), - prBody: readIfSet('pr-body-file'), - }), - maxTokens: 2048, - temperature: 0.2, - }); - console.error(`[${id}] ${MODEL} ${usageLine(usage)}`); - result = normalize(parseJson(text), persona); - } catch (e) { - console.error(`[${id}] review failed: ${e.message}`); - result = { - persona: persona.id, - label: persona.label, - verdict: 'ERROR', - summary: `Review did not complete: ${e.message}`.slice(0, 500), - feedback: [], - model: MODEL, - }; - } + const result = await reviewPersona({ + personaId: id, + diff: readFileSync(diffPath, 'utf8'), + prTitle: arg('pr-title'), + prBody: readIfSet('pr-body-file'), + }); mkdirSync(dirname(out), {recursive: true}); writeFileSync(out, `${JSON.stringify(result, null, 2)}\n`); console.error(`[${id}] ${result.verdict} — ${result.summary}`); } -main().catch((e) => { - console.error(e); - process.exit(1); -}); +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main().catch((e) => { + console.error(e); + process.exit(1); + }); +} diff --git a/.github/scripts/docs-ai/writer/author-loop.mjs b/.github/scripts/docs-ai/writer/author-loop.mjs new file mode 100644 index 00000000000..fd800b57f97 --- /dev/null +++ b/.github/scripts/docs-ai/writer/author-loop.mjs @@ -0,0 +1,404 @@ +import {existsSync, readFileSync} from 'node:fs'; +import {join} from 'node:path'; +import {complete, usageLine} from '../lib/anthropic.mjs'; +import {additionsDiff} from '../lib/diff.mjs'; +import { + alwaysOnPersonaIds, + authorSystemBlocks, + groupPathsByAuthor, + neutralAuthorSystemBlocks, + personasWithScope, + REPO_ROOT as DEFAULT_ROOT, +} from '../lib/personas.mjs'; +import {DATA_NOTICE, block} from '../lib/untrusted.mjs'; +import {reviewPersona} from '../review/persona-review.mjs'; +import {assertSafeMdx, assertVersionAnchors, mergeFiles, parseFileBlocks} from './files.mjs'; +import {isAllowedPath} from './paths.mjs'; + +const WRITER_MODEL = process.env.DOCS_AI_WRITER_MODEL || 'claude-sonnet-4-6'; +const REVIEW_MODEL = process.env.DOCS_AI_REVIEW_MODEL || 'claude-sonnet-4-5-20250929'; +const ROUTER_MODEL = process.env.DOCS_AI_ROUTER_MODEL || 'claude-haiku-4-5-20251001'; + +/** Accept only non-negative integers; fall back when missing or malformed. */ +export function parseMaxRevisions(raw, fallback = 2) { + if (raw == null || raw === '') return fallback; + const n = Number(raw); + return Number.isInteger(n) && n >= 0 ? n : fallback; +} + +const MAX_REVISIONS = parseMaxRevisions(process.env.DOCS_AI_MAX_REVISIONS); + +export function normalizeDocsPath(p) { + return String(p || '') + .trim() + .replace(/^\.\//, '') + .replace(/\/+$/, ''); +} + +/** Concrete docs/ targets for this author group; empty means "no path filter". */ +export function groupPathAllowlist(groupPaths) { + return new Set( + (groupPaths || []) + .map(normalizeDocsPath) + .filter((p) => p.startsWith('docs/')), + ); +} + +/** Targets in the group allowlist that are absent from the authored files. */ +export function missingGroupTargets(files, groupPaths) { + const allow = groupPathAllowlist(groupPaths); + if (allow.size === 0) return []; + const have = new Set((files || []).map((f) => normalizeDocsPath(f.path))); + return [...allow].filter((t) => !have.has(t)).sort(); +} + +/** Load on-disk pages for targets so updates edit the real page, not a blank rewrite. */ +export function loadExistingPages(repoRoot, groupPaths) { + const root = repoRoot || DEFAULT_ROOT; + const pages = []; + for (const raw of groupPaths || []) { + const path = normalizeDocsPath(raw); + if (!path.startsWith('docs/') || !isAllowedPath(path)) continue; + const abs = join(root, path); + if (!existsSync(abs)) continue; + pages.push({path, content: readFileSync(abs, 'utf8')}); + } + return pages; +} + +/** Refuse pages too large to fit in the prompt without truncation (would risk deleting omitted content). */ +export const MAX_PAGE_PROMPT_CHARS = 100_000; + +export function assertPagesFitPrompt(pages, maxChars = MAX_PAGE_PROMPT_CHARS) { + for (const p of pages || []) { + const n = String(p.content ?? '').length; + if (n > maxChars) { + throw new Error( + `${p.path}: page is ${n} characters (limit ${maxChars}); refusing to author a replacement that might omit content`, + ); + } + } +} + +function buildAuthorUserPrompt({brief, groupPaths, input, revisionFeedback, existingPages}) { + const parts = [ + DATA_NOTICE, + '', + block('gap-brief', JSON.stringify(brief, null, 2), {maxChars: 20_000}), + '', + ]; + + if (input.prTitle) parts.push(block('pull-request-title', input.prTitle, {maxChars: 500}), ''); + if (input.prBody) parts.push(block('pull-request-description', input.prBody, {maxChars: 6000}), ''); + if (input.milestoneTitle) { + parts.push( + block( + 'pr-metadata', + [ + `PR URL: ${input.prUrl || '(unknown)'}`, + `milestone.title: ${input.milestoneTitle}`, + `milestone.version: ${input.milestoneVersion || '(none)'}`, + ].join('\n'), + {maxChars: 2000}, + ), + '', + ); + } + if (input.codeDiff) parts.push(block('code_diff', input.codeDiff, {maxChars: 40_000}), ''); + + parts.push( + block('target-paths', groupPaths.join('\n') || '(derive from gap brief)'), + '', + ); + + if (existingPages?.length) { + // Full page only (no truncation): assertPagesFitPrompt already rejected oversized files. + const pageTag = revisionFeedback?.length ? 'current-draft' : 'existing-page'; + for (const page of existingPages) { + parts.push( + block(`${pageTag} path=${page.path}`, page.content, {maxChars: Number.MAX_SAFE_INTEGER}), + '', + ); + } + parts.push( + revisionFeedback?.length + ? 'For paths with a current-draft block, revise that draft in place. Output the full updated file.' + : 'For paths with an existing-page block, edit that page in place (smallest change that closes the gap). Output the full updated file.', + '', + ); + } + + if (revisionFeedback?.length) { + parts.push( + block('reviewer-feedback', JSON.stringify(revisionFeedback, null, 2), {maxChars: 12_000}), + '', + 'Revise the pages to address REQUEST_CHANGES feedback. Output the full file blocks again.', + ); + } else { + parts.push( + 'Draft the MDX file(s) that close the documentation gap for the target paths. Output only the file blocks.', + ); + } + + return parts.join('\n'); +} + +async function authorPass({personaId, groupPaths, brief, input, revisionFeedback, basePages, completeFn}) { + const system = personaId ? authorSystemBlocks(personaId) : neutralAuthorSystemBlocks(); + // Revisions use the in-memory draft; initial passes load the on-disk page when present. + const existingPages = basePages ?? loadExistingPages(input.repoRoot, groupPaths); + assertPagesFitPrompt(existingPages); + const {text, usage} = await completeFn({ + model: WRITER_MODEL, + system, + userPrompt: buildAuthorUserPrompt({ + brief, + groupPaths, + input, + revisionFeedback, + existingPages, + }), + maxTokens: 8192, + temperature: 0.3, + }); + console.error(`[author:${personaId ?? 'neutral'}] ${WRITER_MODEL} ${usageLine(usage)}`); + + const allow = groupPathAllowlist(groupPaths); + const files = parseFileBlocks(text) + .map((f) => ({...f, path: normalizeDocsPath(f.path)})) + .filter((f) => { + if (!isAllowedPath(f.path)) { + console.error(`[author] dropping disallowed path ${f.path}`); + return false; + } + if (allow.size > 0 && !allow.has(f.path)) { + console.error(`[author] dropping path outside group targets: ${f.path}`); + return false; + } + return true; + }); + + if (files.length === 0) { + throw new Error(`author ${personaId ?? 'neutral'} emitted no allowed file blocks`); + } + return files; +} + +async function selectReviewers({diff, completeFn}) { + const alwaysOn = alwaysOnPersonaIds(); + const candidates = personasWithScope('review').filter((p) => !alwaysOn.includes(p.id)); + const validIds = new Set(candidates.map((p) => p.id)); + + try { + const menu = candidates + .map((p) => `- ${p.id} (${p.label})\n Applies to: ${p.routerHints}`) + .join('\n'); + const {text, usage} = await completeFn({ + model: ROUTER_MODEL, + system: `You route Mattermost documentation changes to reviewers.\n\n${DATA_NOTICE}\n\nAvailable reviewers:\n\n${menu}\n\nReturn strict JSON: {"personas": ["id"]}`, + userPrompt: `${block('diff', diff)}\n\nWhich reviewers apply?`, + maxTokens: 512, + temperature: 0, + }); + console.error(`[router] ${ROUTER_MODEL} ${usageLine(usage)}`); + const raw = JSON.parse(text.match(/\{[\s\S]*\}/)?.[0] ?? text).personas; + const selected = (Array.isArray(raw) ? raw : []).filter((id) => validIds.has(id)); + return [...new Set([...selected, ...alwaysOn])].sort(); + } catch (e) { + console.error(`[router] failed (${e.message}); using all reviewers`); + return [...new Set([...candidates.map((p) => p.id), ...alwaysOn])].sort(); + } +} + +async function reviewFiles({files, input, completeFn}) { + const diff = additionsDiff(files); + const reviewers = await selectReviewers({diff, completeFn}); + const results = await Promise.all( + reviewers.map((personaId) => + reviewPersona({ + personaId, + diff, + prTitle: input.prTitle, + prBody: input.prBody, + model: REVIEW_MODEL, + completeFn, + }), + ), + ); + return {diff, reviewers, results}; +} + +/** + * Author → review → revise loop. Returns files + trail for the draft PR body. + */ +export async function authorLoop({brief, input, completeFn = complete} = {}) { + const targets = + brief.targetPaths?.length > 0 + ? brief.targetPaths + : (brief.actions ?? []).length + ? brief.actions + : ['(unspecified — derive from the gap brief)']; + + // When we only have action strings, groupPathsByAuthor still needs path-like targets. + const pathTargets = targets.filter((t) => t.startsWith('docs/')); + const groups = + pathTargets.length > 0 + ? groupPathsByAuthor(pathTargets) + : [{personaId: null, paths: targets}]; + + let files = []; + const trail = {iterations: [], openConcerns: []}; + + for (const group of groups) { + let groupFiles = await authorPass({ + personaId: group.personaId, + groupPaths: group.paths, + brief, + input, + completeFn, + }); + const initialMissing = missingGroupTargets(groupFiles, group.paths); + if (initialMissing.length) { + throw new Error( + `author ${group.personaId ?? 'neutral'} omitted required targets: ${initialMissing.join(', ')}`, + ); + } + assertVersionAnchors(groupFiles, {milestoneVersion: input.milestoneVersion}); + assertSafeMdx(groupFiles); + + let revision = 0; + let lastResults = []; + for (;;) { + const {results} = await reviewFiles({files: groupFiles, input, completeFn}); + lastResults = results; + trail.iterations.push({ + author: group.personaId ?? 'neutral', + revision, + paths: groupFiles.map((f) => f.path), + reviews: results.map((r) => ({ + persona: r.persona, + verdict: r.verdict, + summary: r.summary, + feedback: r.feedback, + })), + }); + + const errors = results.filter((r) => r.verdict === 'ERROR'); + if (errors.length) { + throw new Error( + `pre-open review failed (${errors.map((r) => r.persona).join(', ')}): ${errors.map((r) => r.summary).join('; ')}`, + ); + } + + const blocking = results.filter((r) => r.verdict === 'REQUEST_CHANGES'); + if (blocking.length === 0 || revision >= MAX_REVISIONS) { + if (blocking.length) { + trail.openConcerns.push( + ...blocking.map((r) => ({ + persona: r.persona, + summary: r.summary, + feedback: r.feedback, + })), + ); + } + break; + } + + revision += 1; + const revised = await authorPass({ + personaId: group.personaId, + groupPaths: group.paths, + brief, + input, + revisionFeedback: blocking, + basePages: groupFiles, + completeFn, + }); + const revisedMissing = missingGroupTargets(revised, group.paths); + if (revisedMissing.length) { + // Incomplete revision must not replace a complete prior pass. + console.error( + `[author-loop] revision omitted targets (${revisedMissing.join(', ')}); keeping prior files`, + ); + trail.openConcerns.push({ + persona: group.personaId ?? 'neutral', + summary: `Revision omitted required targets: ${revisedMissing.join(', ')}`, + feedback: [], + }); + break; + } + try { + assertVersionAnchors(revised, {milestoneVersion: input.milestoneVersion}); + assertSafeMdx(revised); + } catch (e) { + // Dropped anchor / unsafe MDX must not replace a complete prior pass. + console.error( + `[author-loop] revision failed validation (${e.message}); keeping prior files`, + ); + trail.openConcerns.push({ + persona: group.personaId ?? 'neutral', + summary: `Revision rejected: ${e.message}`, + feedback: [], + }); + break; + } + groupFiles = revised; + } + + files = mergeFiles(files, groupFiles); + console.error( + `[author-loop] ${group.personaId ?? 'neutral'}: ${groupFiles.map((f) => f.path).join(', ')} ` + + `(${lastResults.map((r) => `${r.persona}:${r.verdict}`).join(', ')})`, + ); + } + + return {files, trail}; +} + +export function renderPrBody({sourcePr, trail, brief}) { + const lines = [ + '', + ``, + '', + `AI-drafted documentation for #${sourcePr}.`, + '', + 'Authored by audience personas from the gap analysis sticky comment, then reviewed', + 'in-loop before this PR opened. Treat as a draft pending human editorial review.', + '', + '## Gap brief', + '', + brief.summary || '_No summary recovered from the sticky comment._', + '', + ]; + + if (brief.actions?.length) { + lines.push('### Recommended actions', '', ...brief.actions.map((a) => `- [ ] ${a}`), ''); + } + + lines.push('## Pre-open review trail', ''); + for (const iter of trail.iterations) { + lines.push( + `### Author \`${iter.author}\` — revision ${iter.revision}`, + '', + `Paths: ${iter.paths.map((p) => `\`${p}\``).join(', ')}`, + '', + ); + for (const r of iter.reviews) { + lines.push(`- **${r.persona}** — \`${r.verdict}\`: ${r.summary}`); + for (const f of r.feedback ?? []) lines.push(` - ${f}`); + } + lines.push(''); + } + + if (trail.openConcerns?.length) { + lines.push('## Open concerns after revision cap', ''); + for (const c of trail.openConcerns) { + lines.push(`- **${c.persona}**: ${c.summary}`); + for (const f of c.feedback ?? []) lines.push(` - ${f}`); + } + lines.push(''); + } + + lines.push(`Closes the docs gap filed against #${sourcePr}.`); + return `${lines.join('\n')}\n`; +} diff --git a/.github/scripts/docs-ai/writer/files.mjs b/.github/scripts/docs-ai/writer/files.mjs new file mode 100644 index 00000000000..b7e83c71aff --- /dev/null +++ b/.github/scripts/docs-ai/writer/files.mjs @@ -0,0 +1,107 @@ +import {mkdirSync, writeFileSync} from 'node:fs'; +import {dirname} from 'node:path'; +import {resolveAllowed} from './paths.mjs'; + +// Outer fence length must match so nested ``` code samples do not truncate the page. +const BLOCK_RE = /^(`{3,})(?:\w+)?[ \t]+path=([^\s`]+)[ \t]*\n([\s\S]*?)\n\1[ \t]*$/gm; + +export const VERSION_ANCHOR_RE = /From Mattermost v\d+\.\d+/; +export const HUMAN_JUDGMENT_MARKER = '[NOT PRESENT — REQUIRES HUMAN JUDGMENT]'; +/** Escape hatch when the release is unknown — keeps the "From Mattermost …" frame. */ +export const HUMAN_JUDGMENT_VERSION_ANCHOR = `From Mattermost ${HUMAN_JUDGMENT_MARKER}`; + +// Line-start import; capture module. Optional "X from" covers `import X from 'm'` and `import 'm'`. +const MDX_IMPORT_RE = /^import\s+(?:.*?\s+from\s+)?['"]([^'"]+)['"]\s*;?\s*$/; +// Word boundary so `export{…}` / `export*` match, not only `export `. +const MDX_EXPORT_RE = /^export\b/; + +/** Import sources AI-authored MDX may use. Everything else is rejected before write. */ +const SAFE_IMPORT_RE = + /^(?:@theme\/|@docusaurus\/|\.\.?\/).+$/; + +/** Drop a trailing // comment so `import X from 'y'; // note` is still checked. */ +function stripLineComment(line) { + // Require whitespace before // so https:// inside a string is left alone. + return line.replace(/\s+\/\/.*$/, ''); +} + +export function parseFileBlocks(text) { + const blocks = []; + let m; + const re = new RegExp(BLOCK_RE); + while ((m = re.exec(text)) !== null) { + blocks.push({path: m[2].trim(), content: m[3]}); + } + return blocks; +} + +export function versionFromMilestone(title) { + if (!title) return null; + const m = String(title).match(/v(\d+\.\d+)/i); + return m ? `v${m[1]}` : null; +} + +export function hasVersionAnchor(content, version) { + if (content.includes(HUMAN_JUDGMENT_VERSION_ANCHOR)) return true; + if (version) return content.includes(`From Mattermost ${version}`); + return VERSION_ANCHOR_RE.test(content); +} + +// Reject pages that document capability without an anchor when a milestone was supplied. +export function assertVersionAnchors(files, {milestoneVersion, required = true} = {}) { + if (!required || !milestoneVersion) return; + for (const f of files) { + if (!hasVersionAnchor(f.content, milestoneVersion)) { + throw new Error( + `${f.path}: missing version anchor (expected "From Mattermost ${milestoneVersion}" or "${HUMAN_JUDGMENT_VERSION_ANCHOR}")`, + ); + } + } +} + +/** Drop fenced code samples so prose gates do not false-positive on examples. */ +function withoutFencedCode(content) { + return String(content).replace(/^(`{3,}).*?\n[\s\S]*?\n\1[ \t]*$/gm, ''); +} + +/** + * Structural MDX gate before write: known import sources only, no export. + * Line-based on fenced-stripped content (no MDX parser dependency). House-style + * JSX is allowed; model protections + docs CI + human review cover the rest. + */ +export function assertSafeMdx(files) { + for (const f of files) { + const content = withoutFencedCode(f.content ?? ''); + for (const rawLine of content.split('\n')) { + const line = stripLineComment(rawLine).trimEnd(); + if (!line) continue; + if (MDX_EXPORT_RE.test(line)) { + throw new Error(`${f.path}: MDX export statements are not allowed in AI drafts`); + } + const m = line.match(MDX_IMPORT_RE); + if (m && !SAFE_IMPORT_RE.test(m[1])) { + throw new Error(`${f.path}: disallowed MDX import source "${m[1]}"`); + } + } + } +} + +export function writeFiles(repoRoot, files) { + assertSafeMdx(files); + const written = []; + for (const f of files) { + const {abs, rel} = resolveAllowed(repoRoot, f.path); + mkdirSync(dirname(abs), {recursive: true}); + const body = f.content.endsWith('\n') ? f.content : `${f.content}\n`; + writeFileSync(abs, body); + written.push({path: rel, bytes: body.length}); + console.error(`[writer] wrote ${rel} (${body.length} chars)`); + } + return written; +} + +export function mergeFiles(existing, incoming) { + const byPath = new Map(existing.map((f) => [f.path, f])); + for (const f of incoming) byPath.set(f.path, f); + return [...byPath.values()]; +} diff --git a/.github/scripts/docs-ai/writer/gap-brief.mjs b/.github/scripts/docs-ai/writer/gap-brief.mjs new file mode 100644 index 00000000000..29d18dabd21 --- /dev/null +++ b/.github/scripts/docs-ai/writer/gap-brief.mjs @@ -0,0 +1,73 @@ +/* + * Recover a writer brief from the gap sticky comment. + * Prefer machine-readable fields when present; otherwise parse the rendered markdown. + */ + +import {MARKER, parseState} from '../gap/gap.mjs'; +import {extractDocsPaths, isAllowedPath} from './paths.mjs'; + +const ACTIONS_RE = /^\- \[ \] (.+)$/gm; + +export function parseGapBrief(body) { + if (!body || !body.includes(MARKER)) return null; + + const state = parseState(body); + const actions = []; + let m; + const actionRe = new RegExp(ACTIONS_RE.source, 'gm'); + while ((m = actionRe.exec(body)) !== null) { + actions.push(m[1].trim()); + } + + const paths = [ + ...extractDocsPaths(actions.join('\n')), + ...extractDocsPaths(body), + ]; + + // Summary sits between the blank line after the verdict and the next section. + let summary = ''; + const summaryMatch = body.match( + /\*\*[^*]+\.\*\*[^\n]*\n\n([\s\S]*?)(?=\n
|\n\*\*Recommended actions\*\*|\n---\n)/, + ); + if (summaryMatch) summary = summaryMatch[1].replace(/\s+/g, ' ').trim(); + + return { + state, + assessment: state?.verdict ?? null, + summary, + actions, + targetPaths: [...new Set(paths)], + }; +} + +export function briefFromGapResult(result) { + const actions = result.actions ?? []; + const fromImpacts = (result.impacts ?? []) + .map((r) => r.docsLocation || r.docs_location) + .filter((p) => typeof p === 'string') + .map((p) => p.trim().replace(/^\.\//, '')) + .filter((p) => /\.(?:md|mdx)$/.test(p) && isAllowedPath(p)); + return { + state: null, + assessment: result.assessment, + summary: result.summary ?? '', + actions, + targetPaths: [...new Set([...extractDocsPaths(actions.join('\n')), ...fromImpacts])], + }; +} + +// Human-applied Docs/Needed (or a missing sticky) — draft from PR evidence alone. +export function briefFromPrEvidence({prTitle, prBody} = {}) { + const blob = [prTitle, prBody].filter(Boolean).join('\n'); + return { + state: null, + assessment: 'required', + summary: + (prTitle && String(prTitle).trim()) || + 'Docs/Needed was present without a docs-gap sticky comment; draft from the merged PR evidence.', + actions: [ + 'Draft documentation for the merged change using the PR title, description, and code diff as evidence.', + ], + targetPaths: extractDocsPaths(blob), + }; +} diff --git a/.github/scripts/docs-ai/writer/paths.mjs b/.github/scripts/docs-ai/writer/paths.mjs new file mode 100644 index 00000000000..9bc040cf0e7 --- /dev/null +++ b/.github/scripts/docs-ai/writer/paths.mjs @@ -0,0 +1,48 @@ +/* + * Path allow/deny for AI-authored content. Deny wins. + * docs/api/reference/ is OpenAPI-generated and must never be hand-authored. + */ + +import {isAbsolute, relative, resolve} from 'node:path'; + +export const ALLOW_PREFIXES = ['docs/main/', 'docs/develop/']; +export const ALLOW_EXACT = ['docs/api/examples.mdx', 'docs/api/index.mdx']; +export const DENY_PREFIXES = [ + 'docs/site/', + 'docs/styles/', + 'docs/pdf/', + 'docs/vendor/', + 'docs/api/reference/', + 'docs/main/agents/docs/', +]; + +// Paths the gap prompt and sticky comment are allowed to name. +export const DOCS_PATH_RE = + /docs\/(?:main|develop)\/[A-Za-z0-9_./-]+\.(?:mdx|md)|docs\/api\/(?:examples|index)\.mdx/g; + +export function extractDocsPaths(text) { + if (!text) return []; + const found = String(text).match(DOCS_PATH_RE) ?? []; + // Deny list (e.g. docs/main/agents/docs/) can still match DOCS_PATH_RE. + return [...new Set(found)].filter(isAllowedPath); +} + +export function isAllowedPath(relPath) { + const rel = relPath.replace(/^\.\//, ''); + if (rel.startsWith('/') || rel.includes('\0') || rel.includes('..')) return false; + if (DENY_PREFIXES.some((p) => rel.startsWith(p))) return false; + if (ALLOW_EXACT.includes(rel)) return true; + return ALLOW_PREFIXES.some((p) => rel.startsWith(p)); +} + +export function resolveAllowed(repoRoot, relPath) { + const abs = resolve(repoRoot, relPath); + const rel = relative(repoRoot, abs); + if (rel.startsWith('..') || isAbsolute(rel)) { + throw new Error(`refusing to write outside repo root: ${relPath}`); + } + if (!isAllowedPath(rel)) { + throw new Error(`refusing to write outside content allowlist: ${rel}`); + } + return {abs, rel}; +} diff --git a/.github/scripts/docs-ai/writer/run.mjs b/.github/scripts/docs-ai/writer/run.mjs new file mode 100644 index 00000000000..5f18fa78aab --- /dev/null +++ b/.github/scripts/docs-ai/writer/run.mjs @@ -0,0 +1,124 @@ +#!/usr/bin/env node +/* + * Merged-PR writer entry point. + * + * node writer/run.mjs + * + * Env: + * GITHUB_TOKEN, GITHUB_REPOSITORY, PR_NUMBER + * REPO_ROOT (default cwd) + * Optional: PR_TITLE, PR_BODY_FILE, CODE_DIFF_FILE, MILESTONE_TITLE, + * GAP_COMMENT_FILE (skip GitHub sticky fetch), DRY_RUN=1 + * + * Writes docs files into REPO_ROOT and emits: + * .docs-ai/writer/trail.json + * .docs-ai/writer/pr-body.md + * .docs-ai/writer/written.json + */ + +import {mkdirSync, readFileSync, writeFileSync, existsSync} from 'node:fs'; +import {join} from 'node:path'; +import {findStickyComment} from '../lib/github.mjs'; +import {MARKER} from '../gap/gap.mjs'; +import {REPO_ROOT as DEFAULT_ROOT} from '../lib/personas.mjs'; +import {authorLoop, renderPrBody} from './author-loop.mjs'; +import {versionFromMilestone, writeFiles} from './files.mjs'; +import {briefFromGapResult, briefFromPrEvidence, parseGapBrief} from './gap-brief.mjs'; + +function arg(name) { + const i = process.argv.indexOf(`--${name}`); + return i === -1 ? null : process.argv[i + 1]; +} + +function readPrEvidence() { + const prBodyFile = process.env.PR_BODY_FILE; + return { + prTitle: process.env.PR_TITLE || '', + prBody: prBodyFile && existsSync(prBodyFile) ? readFileSync(prBodyFile, 'utf8') : '', + }; +} + +async function loadBrief({repo, pr}) { + const file = arg('brief-file') || process.env.GAP_COMMENT_FILE; + if (file) { + const raw = readFileSync(file, 'utf8'); + // Accept either a sticky comment body or a clampResult JSON object. + if (raw.trimStart().startsWith('{')) { + return briefFromGapResult(JSON.parse(raw)); + } + const brief = parseGapBrief(raw); + if (!brief) throw new Error(`no gap sticky marker in ${file}`); + return brief; + } + + const sticky = await findStickyComment(repo, pr, {marker: MARKER}); + if (!sticky?.body) { + console.error( + `[writer] no docs-gap sticky on #${pr}; falling back to PR title/body/diff as the brief`, + ); + return briefFromPrEvidence(readPrEvidence()); + } + const brief = parseGapBrief(sticky.body); + if (!brief) { + console.error(`[writer] gap sticky on #${pr} unparseable; falling back to PR evidence`); + return briefFromPrEvidence(readPrEvidence()); + } + if (!brief.actions.length && !brief.targetPaths.length) { + console.error(`[writer] gap sticky on #${pr} has no actions/paths; falling back to PR evidence`); + return briefFromPrEvidence(readPrEvidence()); + } + return brief; +} + +async function main() { + const repo = process.env.GITHUB_REPOSITORY; + const pr = process.env.PR_NUMBER || arg('pr'); + const repoRoot = process.env.REPO_ROOT || DEFAULT_ROOT; + const outDir = join(repoRoot, '.docs-ai', 'writer'); + mkdirSync(outDir, {recursive: true}); + + if (!repo || !pr) throw new Error('GITHUB_REPOSITORY and PR_NUMBER are required'); + + const milestoneTitle = process.env.MILESTONE_TITLE || ''; + const milestoneVersion = versionFromMilestone(milestoneTitle); + if (!milestoneVersion) { + throw new Error('milestone title must contain vMAJOR.MINOR (version anchor source)'); + } + + const brief = await loadBrief({repo, pr}); + console.error( + `[writer] assessment=${brief.assessment} actions=${brief.actions.length} paths=${brief.targetPaths.join(', ') || '(none)'}`, + ); + + const prBodyFile = process.env.PR_BODY_FILE; + const codeDiffFile = process.env.CODE_DIFF_FILE; + const input = { + prTitle: process.env.PR_TITLE || '', + prBody: prBodyFile && existsSync(prBodyFile) ? readFileSync(prBodyFile, 'utf8') : '', + codeDiff: codeDiffFile && existsSync(codeDiffFile) ? readFileSync(codeDiffFile, 'utf8') : '', + milestoneTitle, + milestoneVersion, + prUrl: `https://github.com/${repo}/pull/${pr}`, + repoRoot, + }; + + const {files, trail} = await authorLoop({brief, input}); + + writeFileSync(join(outDir, 'trail.json'), `${JSON.stringify(trail, null, 2)}\n`); + const body = renderPrBody({sourcePr: pr, trail, brief}); + writeFileSync(join(outDir, 'pr-body.md'), body); + + if (process.env.DRY_RUN === '1') { + writeFileSync(join(outDir, 'files.json'), `${JSON.stringify(files, null, 2)}\n`); + console.error(`[writer] dry-run: ${files.length} file(s) not written`); + return; + } + + const written = writeFiles(repoRoot, files); + writeFileSync(join(outDir, 'written.json'), `${JSON.stringify(written, null, 2)}\n`); +} + +main().catch((e) => { + console.error(e); + process.exit(1); +}); diff --git a/.github/scripts/docs-ai/writer/sync.mjs b/.github/scripts/docs-ai/writer/sync.mjs new file mode 100644 index 00000000000..5b3a727f363 --- /dev/null +++ b/.github/scripts/docs-ai/writer/sync.mjs @@ -0,0 +1,78 @@ +#!/usr/bin/env node +/* + * When a docs/pr- draft merges, flip Docs/Needed → Docs/Done on the source PR. + * + * node writer/sync.mjs + * + * Env: + * GITHUB_TOKEN, GITHUB_REPOSITORY + * HEAD_REPO, PR_USER — trusted pull_request event fields (provenance) + * DOCS_AI_BOT_LOGIN — exact App bot login that opened the draft + * SOURCE_PR or HEAD_REF / PR_BODY_FILE — resolve which source PR to update + */ + +import {readFileSync, existsSync} from 'node:fs'; +import {fileURLToPath} from 'node:url'; +import {addLabel, removeLabel} from '../lib/github.mjs'; + +const NEEDED = 'Docs/Needed'; +const DONE = 'Docs/Done'; +const BRANCH_RE = /^docs\/pr-(\d+)$/; +const BODY_RE = //; + +export function sourcePrFrom({headRef, body, explicit}) { + if (explicit) return String(explicit); + // Prefer branch name: body markers are easy to edit; head.ref is event metadata. + const fromBranch = headRef?.match(BRANCH_RE); + if (fromBranch) return fromBranch[1]; + const fromBody = body?.match(BODY_RE); + if (fromBody) return fromBody[1]; + return null; +} + +/** Provenance from trusted event metadata only — not branch name, body, or labels. */ +export function assertWriterProvenance({repo, headRepo, prUser, botLogin}) { + if (!repo) throw new Error('GITHUB_REPOSITORY is required'); + if (!headRepo || headRepo !== repo) { + throw new Error(`refusing sync: head repo "${headRepo || '(missing)'}" is not ${repo}`); + } + if (!botLogin) { + throw new Error('DOCS_AI_BOT_LOGIN is required to verify writer provenance'); + } + if (!prUser || prUser !== botLogin) { + throw new Error(`refusing sync: PR author "${prUser || '(missing)'}" is not ${botLogin}`); + } +} + +async function main() { + const repo = process.env.GITHUB_REPOSITORY; + assertWriterProvenance({ + repo, + headRepo: process.env.HEAD_REPO, + prUser: process.env.PR_USER, + botLogin: process.env.DOCS_AI_BOT_LOGIN, + }); + + const bodyFile = process.env.PR_BODY_FILE; + const body = bodyFile && existsSync(bodyFile) ? readFileSync(bodyFile, 'utf8') : process.env.PR_BODY || ''; + const source = sourcePrFrom({ + headRef: process.env.HEAD_REF, + body, + explicit: process.env.SOURCE_PR, + }); + + if (!source) { + throw new Error('could not resolve source PR from HEAD_REF, body marker, or SOURCE_PR'); + } + + console.error(`[sync] #${source}: ${NEEDED} → ${DONE}`); + await removeLabel(repo, source, NEEDED); + await addLabel(repo, source, DONE); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main().catch((e) => { + console.error(e); + process.exit(1); + }); +} diff --git a/.github/scripts/docs-ai/writer/writer.test.mjs b/.github/scripts/docs-ai/writer/writer.test.mjs new file mode 100644 index 00000000000..d430182ea4a --- /dev/null +++ b/.github/scripts/docs-ai/writer/writer.test.mjs @@ -0,0 +1,487 @@ +import {test} from 'node:test'; +import assert from 'node:assert/strict'; +import {additionsDiff} from '../lib/diff.mjs'; +import {authorForPath, groupPathsByAuthor, REPO_ROOT} from '../lib/personas.mjs'; +import { + assertSafeMdx, + assertVersionAnchors, + hasVersionAnchor, + parseFileBlocks, + versionFromMilestone, +} from './files.mjs'; +import {briefFromGapResult, briefFromPrEvidence, parseGapBrief} from './gap-brief.mjs'; +import {DENY_PREFIXES, extractDocsPaths, isAllowedPath} from './paths.mjs'; +import {sourcePrFrom, assertWriterProvenance} from './sync.mjs'; +import { + assertPagesFitPrompt, + groupPathAllowlist, + loadExistingPages, + MAX_PAGE_PROMPT_CHARS, + missingGroupTargets, + normalizeDocsPath, + parseMaxRevisions, + authorLoop, +} from './author-loop.mjs'; +import {MARKER, buildComment, decide} from '../gap/gap.mjs'; + +test('allowlist accepts hand-authored content roots', () => { + assert.equal(isAllowedPath('docs/main/administration-guide/foo.mdx'), true); + assert.equal(isAllowedPath('docs/develop/contribute/bar.mdx'), true); + assert.equal(isAllowedPath('docs/api/examples.mdx'), true); + assert.equal(isAllowedPath('docs/api/index.mdx'), true); +}); + +test('deny list wins over allow prefixes', () => { + for (const prefix of DENY_PREFIXES) { + assert.equal(isAllowedPath(`${prefix}x.mdx`), false, prefix); + } + assert.equal(isAllowedPath('docs/api/reference/users.mdx'), false); + assert.equal(isAllowedPath('docs/site/docusaurus.config.js'), false); + assert.equal(isAllowedPath('../etc/passwd'), false); +}); + +test('extractDocsPaths pulls exact content paths from prose', () => { + const text = + 'Update docs/main/administration-guide/configure/x.mdx and docs/api/examples.mdx; ignore docs/site/foo.'; + assert.deepEqual(extractDocsPaths(text).sort(), [ + 'docs/api/examples.mdx', + 'docs/main/administration-guide/configure/x.mdx', + ]); +}); + +test('extractDocsPaths drops deny-listed paths that match the path regex', () => { + const text = + 'See docs/main/agents/docs/foo.mdx and docs/main/administration-guide/configure/x.mdx'; + assert.deepEqual(extractDocsPaths(text), ['docs/main/administration-guide/configure/x.mdx']); +}); + +test('versionFromMilestone reads vMAJOR.MINOR', () => { + assert.equal(versionFromMilestone('v11.7.0'), 'v11.7'); + assert.equal(versionFromMilestone('Mattermost v10.12'), 'v10.12'); + assert.equal(versionFromMilestone('no version here'), null); +}); + +test('hasVersionAnchor accepts the convention or the framed human marker', () => { + assert.equal(hasVersionAnchor('From Mattermost v11.7, users can…'), true); + assert.equal( + hasVersionAnchor('From Mattermost [NOT PRESENT — REQUIRES HUMAN JUDGMENT], users can…'), + true, + ); + assert.equal(hasVersionAnchor('See [NOT PRESENT — REQUIRES HUMAN JUDGMENT].'), false); + assert.equal(hasVersionAnchor('No version at all.'), false); +}); + +test('hasVersionAnchor requires the milestone version when supplied', () => { + assert.equal(hasVersionAnchor('From Mattermost v9.5, legacy…', 'v11.7'), false); + assert.equal(hasVersionAnchor('From Mattermost v11.7, new…', 'v11.7'), true); + assert.equal( + hasVersionAnchor('From Mattermost [NOT PRESENT — REQUIRES HUMAN JUDGMENT]', 'v11.7'), + true, + ); + assert.equal(hasVersionAnchor('[NOT PRESENT — REQUIRES HUMAN JUDGMENT]', 'v11.7'), false); +}); + +test('assertVersionAnchors rejects capability pages without an anchor', () => { + assert.throws( + () => + assertVersionAnchors( + [{path: 'docs/main/x.mdx', content: '---\ntitle: X\n---\n\nHello.\n'}], + {milestoneVersion: 'v11.7'}, + ), + /missing version anchor/, + ); +}); + +test('assertVersionAnchors rejects a stale anchor that is not the milestone', () => { + assert.throws( + () => + assertVersionAnchors( + [{path: 'docs/main/x.mdx', content: '---\ntitle: X\n---\n\nFrom Mattermost v9.5, …\n'}], + {milestoneVersion: 'v11.7'}, + ), + /missing version anchor/, + ); +}); + +test('parseFileBlocks reads path= fences', () => { + const text = [ + '```mdx path=docs/main/end-user-guide/a.mdx', + '---', + 'title: A', + '---', + '', + 'From Mattermost v11.7, a.', + '```', + '', + '```mdx path=docs/main/administration-guide/b.mdx', + '---', + 'title: B', + '---', + '', + 'From Mattermost v11.7, b.', + '```', + ].join('\n'); + const files = parseFileBlocks(text); + assert.equal(files.length, 2); + assert.equal(files[0].path, 'docs/main/end-user-guide/a.mdx'); + assert.match(files[1].content, /title: B/); +}); + +test('parseFileBlocks preserves nested three-backtick samples inside a longer fence', () => { + const text = [ + '````mdx path=docs/main/administration-guide/example.mdx', + '---', + 'title: Example', + '---', + '', + 'From Mattermost v11.7, run:', + '', + '```bash', + 'mmctl config get ServiceSettings.SiteURL', + '```', + '', + 'Done.', + '````', + ].join('\n'); + const files = parseFileBlocks(text); + assert.equal(files.length, 1); + assert.equal(files[0].path, 'docs/main/administration-guide/example.mdx'); + assert.match(files[0].content, /```bash/); + assert.match(files[0].content, /mmctl config get/); + assert.match(files[0].content, /Done\./); +}); + +test('additionsDiff synthesises a reviewable unified diff', () => { + const diff = additionsDiff([ + {path: 'docs/main/x.mdx', content: '---\ntitle: X\n---\n\nBody\n'}, + ]); + assert.match(diff, /diff --git a\/docs\/main\/x\.mdx/); + assert.match(diff, /\+---/); + assert.match(diff, /\+Body/); +}); + +test('authorForPath matches an exact docs_paths prefix and paths under it', () => { + assert.equal(authorForPath('docs/main/administration-guide')?.id, 'system-admin'); + assert.equal(authorForPath('docs/main/administration-guide/configure/x.mdx')?.id, 'system-admin'); + assert.equal(authorForPath('docs/main/end-user-guide/collaborate/y.mdx')?.id, 'end-user'); + assert.equal(authorForPath('docs/develop/api/foo.mdx')?.id, 'developer-dx'); +}); + +test('authorForPath prefers the longest matching docs_paths prefix', () => { + // security-compliance owns administration-guide/comply; system-admin owns administration-guide. + assert.equal( + authorForPath('docs/main/administration-guide/comply/compliance-export.mdx')?.id, + 'security-compliance', + ); + assert.equal( + authorForPath('docs/main/administration-guide/onboard/ad-ldap.mdx')?.id, + 'security-compliance', + ); +}); + +test('authorForPath returns null when nothing matches', () => { + assert.equal(authorForPath('docs/main/unknown-section/x.mdx'), null); + assert.equal(authorForPath('server/channels/app/foo.go'), null); +}); + +test('groupPathsByAuthor batches by persona and keeps unmatched paths neutral', () => { + const groups = groupPathsByAuthor([ + 'docs/main/administration-guide/a.mdx', + 'docs/main/deployment-guide/b.mdx', + 'docs/main/end-user-guide/c.mdx', + 'docs/main/orphan/page.mdx', + ]); + const byId = Object.fromEntries(groups.map((g) => [g.personaId, g.paths])); + assert.deepEqual(byId['system-admin'], [ + 'docs/main/administration-guide/a.mdx', + 'docs/main/deployment-guide/b.mdx', + ]); + assert.deepEqual(byId['end-user'], ['docs/main/end-user-guide/c.mdx']); + assert.deepEqual(byId[null], ['docs/main/orphan/page.mdx']); +}); + +test('parseGapBrief recovers actions and paths from a sticky comment', () => { + const comment = buildComment({ + result: { + assessment: 'required', + summary: 'Adds EnableFoo.', + confidence: 'high', + impacts: [ + { + change: 'Config', + files: 'config.go', + audiences: 'Admin', + action: 'Document', + docsLocation: 'docs/main/administration-guide/configure/foo.mdx', + }, + ], + actions: ['Document EnableFoo in docs/main/administration-guide/configure/foo.mdx'], + }, + decision: decide({assessment: 'required', labels: [], priorState: null}), + sha: 'abc1234', + runUrl: 'https://example.test/run', + }); + + assert.match(comment, new RegExp(MARKER)); + const brief = parseGapBrief(comment); + assert.equal(brief.assessment, 'required'); + assert.equal(brief.actions.length, 1); + assert.ok(brief.targetPaths.includes('docs/main/administration-guide/configure/foo.mdx')); + assert.match(brief.summary, /EnableFoo/); +}); + +test('briefFromGapResult uses impact docs_location', () => { + const brief = briefFromGapResult({ + assessment: 'recommended', + summary: 's', + actions: [], + impacts: [{docs_location: 'docs/main/security-guide/x.mdx'}], + }); + assert.deepEqual(brief.targetPaths, ['docs/main/security-guide/x.mdx']); +}); + +test('briefFromGapResult drops directory, denied, and non-string impact locations', () => { + const brief = briefFromGapResult({ + assessment: 'required', + summary: 's', + actions: [], + impacts: [ + {docs_location: 'docs/main/administration-guide/'}, + {docs_location: 'docs/site/foo.mdx'}, + {docs_location: 'docs/api/reference/openapi.mdx'}, + {docsLocation: 42}, + {docs_location: ' ./docs/main/administration-guide/configure/foo.mdx '}, + {docs_location: null}, + ], + }); + assert.deepEqual(brief.targetPaths, ['docs/main/administration-guide/configure/foo.mdx']); +}); + +test('parseMaxRevisions accepts non-negative integers and rejects malformed values', () => { + assert.equal(parseMaxRevisions(undefined), 2); + assert.equal(parseMaxRevisions(''), 2); + assert.equal(parseMaxRevisions('two'), 2); + assert.equal(parseMaxRevisions('2.5'), 2); + assert.equal(parseMaxRevisions('2abc'), 2); + assert.equal(parseMaxRevisions('0'), 0); + assert.equal(parseMaxRevisions('3'), 3); + assert.equal(parseMaxRevisions('-1'), 2); + assert.equal(parseMaxRevisions(null, 5), 5); +}); + +test('briefFromPrEvidence drafts from title/body when no sticky exists', () => { + const brief = briefFromPrEvidence({ + prTitle: 'Add EnableFoo', + prBody: 'See docs/main/administration-guide/configure/foo.mdx', + }); + assert.equal(brief.assessment, 'required'); + assert.equal(brief.actions.length, 1); + assert.ok(brief.targetPaths.includes('docs/main/administration-guide/configure/foo.mdx')); + assert.match(brief.summary, /EnableFoo/); +}); + +test('sourcePrFrom reads branch and body marker', () => { + assert.equal(sourcePrFrom({headRef: 'docs/pr-38177'}), '38177'); + assert.equal(sourcePrFrom({body: '\nhello'}), '99'); + assert.equal(sourcePrFrom({explicit: '12', headRef: 'docs/pr-99'}), '12'); + assert.equal(sourcePrFrom({headRef: 'feature/x'}), null); +}); + +test('sourcePrFrom prefers branch over editable body marker', () => { + assert.equal( + sourcePrFrom({ + headRef: 'docs/pr-38177', + body: '\nforged', + }), + '38177', + ); +}); + +test('assertSafeMdx allows theme imports and house JSX; rejects odd imports and exports', () => { + assert.doesNotThrow(() => + assertSafeMdx([ + { + path: 'docs/main/x.mdx', + content: [ + '---', + 'title: X', + '---', + '', + "import Tabs from '@theme/Tabs';", + "import Help from './help.mdx';", + '', + '', + '', + 'From Mattermost v11.7, …', + '', + '```js', + 'export default function demo() {}', + 'import fs from "fs";', + '```', + '', + ].join('\n'), + }, + ]), + ); + assert.throws( + () => + assertSafeMdx([ + { + path: 'docs/main/x.mdx', + content: "import fs from 'fs';\n\nHello.\n", + }, + ]), + /disallowed MDX import source/, + ); + assert.throws( + () => + assertSafeMdx([ + { + path: 'docs/main/x.mdx', + content: "export const meta = {};\n\nHello.\n", + }, + ]), + /export statements/, + ); + assert.throws( + () => + assertSafeMdx([ + { + path: 'docs/main/x.mdx', + content: "export{meta};\n\nHello.\n", + }, + ]), + /export statements/, + ); + assert.throws( + () => + assertSafeMdx([ + { + path: 'docs/main/x.mdx', + content: "import fs from 'fs'; // ignored\n\nHello.\n", + }, + ]), + /disallowed MDX import source/, + ); +}); + +test('assertPagesFitPrompt rejects oversized pages', () => { + assert.throws( + () => + assertPagesFitPrompt([{path: 'docs/main/huge.mdx', content: 'x'.repeat(MAX_PAGE_PROMPT_CHARS + 1)}]), + /refusing to author/, + ); + assert.doesNotThrow(() => + assertPagesFitPrompt([{path: 'docs/main/ok.mdx', content: 'x'.repeat(100)}]), + ); +}); + +test('assertWriterProvenance requires same-repo head and exact bot author', () => { + assert.throws( + () => assertWriterProvenance({repo: 'o/r', headRepo: 'fork/r', prUser: 'bot[bot]', botLogin: 'bot[bot]'}), + /head repo/, + ); + assert.throws( + () => assertWriterProvenance({repo: 'o/r', headRepo: 'o/r', prUser: 'human', botLogin: 'bot[bot]'}), + /PR author/, + ); + assert.throws( + () => assertWriterProvenance({repo: 'o/r', headRepo: 'o/r', prUser: 'bot[bot]', botLogin: ''}), + /DOCS_AI_BOT_LOGIN/, + ); + assert.doesNotThrow(() => + assertWriterProvenance({repo: 'o/r', headRepo: 'o/r', prUser: 'bot[bot]', botLogin: 'bot[bot]'}), + ); +}); + +test('groupPathAllowlist keeps only concrete docs/ targets', () => { + const allow = groupPathAllowlist([ + 'docs/main/administration-guide/a.mdx', + './docs/main/b.mdx/', + '(unspecified — derive from the gap brief)', + ]); + assert.equal(allow.size, 2); + assert.ok(allow.has('docs/main/administration-guide/a.mdx')); + assert.ok(allow.has('docs/main/b.mdx')); + assert.equal(normalizeDocsPath('./docs/main/x.mdx'), 'docs/main/x.mdx'); +}); + +test('loadExistingPages reads allowed on-disk targets only', () => { + const pages = loadExistingPages(REPO_ROOT, [ + 'docs/api/index.mdx', + 'docs/main/agents/docs/missing.mdx', + 'docs/main/does-not-exist-ai-writer-test.mdx', + ]); + assert.deepEqual( + pages.map((p) => p.path), + ['docs/api/index.mdx'], + ); + assert.ok(pages[0].content.length > 0); +}); + +test('missingGroupTargets requires every concrete target to be authored', () => { + const targets = [ + 'docs/main/administration-guide/a.mdx', + 'docs/main/administration-guide/b.mdx', + ]; + assert.deepEqual( + missingGroupTargets([{path: 'docs/main/administration-guide/a.mdx', content: 'x'}], targets), + ['docs/main/administration-guide/b.mdx'], + ); + assert.deepEqual( + missingGroupTargets( + [ + {path: 'docs/main/administration-guide/a.mdx', content: 'x'}, + {path: 'docs/main/administration-guide/b.mdx', content: 'y'}, + ], + targets, + ), + [], + ); + assert.deepEqual(missingGroupTargets([{path: 'docs/main/a.mdx', content: 'x'}], ['(unspecified)']), []); +}); + +test('authorLoop keeps prior files when a revision drops the version anchor', async () => { + const path = 'docs/main/administration-guide/configure/foo.mdx'; + const fileBlock = (body) => + ['````mdx path=' + path, '---', 'title: Foo', '---', '', body, '````'].join('\n'); + + let authorCalls = 0; + let sawCurrentDraft = false; + const completeFn = async ({userPrompt}) => { + const prompt = String(userPrompt || ''); + if (prompt.includes('Which reviewers apply?')) { + return {text: '{"personas": ["system-admin"]}', usage: {}}; + } + if (prompt.includes('Review the diff above')) { + return { + text: JSON.stringify({ + verdict: 'REQUEST_CHANGES', + summary: 'Needs a clearer lead.', + feedback: ['Clarify the lead sentence.'], + }), + usage: {}, + }; + } + authorCalls += 1; + if (prompt.includes('Revise the pages')) { + if (prompt.includes('current-draft path=' + path) && prompt.includes('From Mattermost v11.7')) { + sawCurrentDraft = true; + } + return {text: fileBlock('Foo is available.'), usage: {}}; + } + return {text: fileBlock('From Mattermost v11.7, Foo is available.'), usage: {}}; + }; + + const {files, trail} = await authorLoop({ + brief: {summary: 's', actions: [], targetPaths: [path]}, + input: {milestoneVersion: 'v11.7', milestoneTitle: 'v11.7.0', prTitle: 'Add Foo'}, + completeFn, + }); + + assert.equal(authorCalls, 2); + assert.equal(sawCurrentDraft, true); + assert.equal(files.length, 1); + assert.match(files[0].content, /From Mattermost v11\.7/); + assert.ok(trail.openConcerns.some((c) => /Revision rejected/.test(c.summary))); +}); diff --git a/.github/workflows/docs-needed.yml b/.github/workflows/docs-needed.yml deleted file mode 100644 index 2683f565386..00000000000 --- a/.github/workflows/docs-needed.yml +++ /dev/null @@ -1,628 +0,0 @@ -# .github/workflows/docs-needed.yml -# -# Place this file in ALL of: -# mattermost/mattermost -> .github/workflows/docs-needed.yml -# mattermost/mattermost-mobile -> .github/workflows/docs-needed.yml -# mattermost/desktop -> .github/workflows/docs-needed.yml -# -# Authentication — reuses the two-app pattern from changelog automation: -# -# Read app (vars.CHANGELOG_READ_CLIENT_ID / secrets.CHANGELOG_READ_PRIVATE_KEY) -# Installed on mattermost/mattermost with contents: read. -# Used only to look up milestones (step 3). -# -# Write app (vars.CHANGELOG_WRITE_CLIENT_ID / secrets.CHANGELOG_WRITE_PRIVATE_KEY) -# Installed on mattermost/docs with contents: write + pull-requests: write. -# Used to push the docs branch, open the draft PR, and manage milestones/reviewers. -# changelog-automation-docs[bot] must be on the CLA ignore list in mattermost/docs. -# -# Other required secrets (set in each repo's Settings -> Secrets and Variables): -# secrets.ANTHROPIC_API_KEY - Claude API key for doc generation -# vars.CHANGELOG_GIT_USERNAME - Git display name for automated commits -# vars.CHANGELOG_GIT_EMAIL - Git email for automated commits -# -# Behaviour: -# When Docs/Needed is added to a non-draft PR from within the same repo, -# opens one docs PR in mattermost/docs: -# Branch: docs/-pr- -# e.g. docs/mattermost-pr-1234, docs/desktop-pr-42 -# Targets the versioned release branch (e.g. v11.7-documentation) if it -# exists in mattermost/docs; falls back to master with a warning. -# Draft doc generated by Claude using the draft_docs.md prompt. -# Milestone on the docs PR mirrors the dev PR milestone. -# Dev PR author added as reviewer on the docs PR. -# Comment posted on the dev PR linking to the new docs PR. -# If Docs/Needed is re-applied after removal, a fresh run creates or -# updates the same branch (idempotent via the exists check). - -name: "docs/needed - Open Docs PR" - -on: - pull_request: - types: [labeled] - workflow_dispatch: - inputs: - pr_number: - description: >- - PR number to process. Intended use cases: - (1) smoke-test the workflow after merging without having to apply a label; - (2) re-run after a transient failure (e.g. Claude API timeout, push error) - without bouncing the Docs/Needed label; - (3) validate the workflow against a known PR during development or review. - required: true - type: string - -# One run at a time per dev PR prevents a race if the label bounces quickly. -# cancel-in-progress is false because a mid-run cancellation can leave the -# docs branch in a partial state (e.g. commit pushed but PR not yet opened). -# Queuing is safer; the idempotency checks in each step handle duplicates. -concurrency: - group: >- - docs-needed-${{ github.repository }}-${{ github.event.pull_request.number || inputs.pr_number }} - cancel-in-progress: false - -jobs: - open-docs-pr: - name: "Open Docs PR" - runs-on: ubuntu-latest - # Fork guard: only run on PRs opened from within this repo, not forks. - # Draft guard: skip draft PRs - apply Docs/Needed when the PR is ready. - # workflow_dispatch bypasses both guards for manual testing. - if: | - github.event_name == 'workflow_dispatch' || - (github.event.label.name == 'Docs/Needed' && - github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.draft == false) - permissions: - contents: read - pull-requests: write - - steps: - # 0. Generate short-lived installation tokens — one per app. - # Tokens expire automatically after 1 hour and are not tied to any - # individual user account. - # - # read-token: scoped to mattermost/mattermost for milestone lookups - # write-token: scoped to mattermost/docs for branch push, PR creation, - # milestone sync, and reviewer assignment - - name: Generate read token - id: read-token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 - with: - client-id: ${{ vars.CHANGELOG_READ_CLIENT_ID }} - private-key: ${{ secrets.CHANGELOG_READ_PRIVATE_KEY }} - repositories: mattermost - - - name: Generate write token - id: write-token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 - with: - client-id: ${{ vars.CHANGELOG_WRITE_CLIENT_ID }} - private-key: ${{ secrets.CHANGELOG_WRITE_PRIVATE_KEY }} - repositories: docs - - # 0b. Resolve PR data — normalises pull_request event fields and - # workflow_dispatch inputs into one set of outputs so all subsequent - # steps are trigger-agnostic. For workflow_dispatch the PR is fetched - # by number from this repo using the built-in GITHUB_TOKEN. - - name: Resolve PR data - id: pr-data - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - INPUT_PR_NUMBER: ${{ inputs.pr_number }} - EVENT_PR_NUMBER: ${{ github.event.pull_request.number }} - EVENT_MILESTONE_NUMBER: ${{ github.event.pull_request.milestone.number }} - EVENT_MILESTONE_TITLE: ${{ github.event.pull_request.milestone.title }} - EVENT_MILESTONE_DUE: ${{ github.event.pull_request.milestone.due_on }} - EVENT_PR_TITLE: ${{ github.event.pull_request.title }} - EVENT_PR_AUTHOR: ${{ github.event.pull_request.user.login }} - run: | - if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then - DATA=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$INPUT_PR_NUMBER") - echo "pr_number=$INPUT_PR_NUMBER" >> "$GITHUB_OUTPUT" - echo "milestone_number=$(echo "$DATA" | jq -r '.milestone.number // ""')" >> "$GITHUB_OUTPUT" - echo "milestone_title=$(echo "$DATA" | jq -r '.milestone.title // ""')" >> "$GITHUB_OUTPUT" - echo "milestone_due=$(echo "$DATA" | jq -r '.milestone.due_on // ""')" >> "$GITHUB_OUTPUT" - echo "pr_title=$(echo "$DATA" | jq -r '.title')" >> "$GITHUB_OUTPUT" - echo "pr_author=$(echo "$DATA" | jq -r '.user.login')" >> "$GITHUB_OUTPUT" - else - echo "pr_number=$EVENT_PR_NUMBER" >> "$GITHUB_OUTPUT" - echo "milestone_number=$EVENT_MILESTONE_NUMBER" >> "$GITHUB_OUTPUT" - echo "milestone_title=$EVENT_MILESTONE_TITLE" >> "$GITHUB_OUTPUT" - echo "milestone_due=$EVENT_MILESTONE_DUE" >> "$GITHUB_OUTPUT" - echo "pr_title=$EVENT_PR_TITLE" >> "$GITHUB_OUTPUT" - echo "pr_author=$EVENT_PR_AUTHOR" >> "$GITHUB_OUTPUT" - fi - - # 1. Guard: milestone is required - - name: Require milestone - env: - MILESTONE: ${{ steps.pr-data.outputs.milestone_number }} - run: | - if [ -z "$MILESTONE" ]; then - echo "::error title=Missing milestone::Set a milestone on this PR before applying Docs/Needed." - exit 1 - fi - - # 2. Compute shared variables - - name: Set variables - id: vars - env: - MILESTONE_TITLE: ${{ steps.pr-data.outputs.milestone_title }} - MILESTONE_NUMBER: ${{ steps.pr-data.outputs.milestone_number }} - PR_NUMBER: ${{ steps.pr-data.outputs.pr_number }} - PR_TITLE: ${{ steps.pr-data.outputs.pr_title }} - PR_AUTHOR: ${{ steps.pr-data.outputs.pr_author }} - SOURCE_REPO: ${{ github.repository }} - run: | - REPO_NAME="${SOURCE_REPO##*/}" - BRANCH="docs/${REPO_NAME}-pr-${PR_NUMBER}" - DOCS_FILE="source/about/drafts/${REPO_NAME}-pr-${PR_NUMBER}.md" - - echo "repo_name=$REPO_NAME" >> "$GITHUB_OUTPUT" - echo "branch=$BRANCH" >> "$GITHUB_OUTPUT" - echo "docs_file=$DOCS_FILE" >> "$GITHUB_OUTPUT" - echo "milestone_title=$MILESTONE_TITLE" >> "$GITHUB_OUTPUT" - echo "milestone_number=$MILESTONE_NUMBER" >> "$GITHUB_OUTPUT" - echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT" - echo "pr_title=$PR_TITLE" >> "$GITHUB_OUTPUT" - echo "pr_author=$PR_AUTHOR" >> "$GITHUB_OUTPUT" - echo "source_repo=$SOURCE_REPO" >> "$GITHUB_OUTPUT" - - # 3. Resolve the versioned base branch in mattermost/docs - # mattermost/mattermost milestones (e.g. "v11.7.0") map directly to - # the docs branch (e.g. "v11.7-documentation"). - # mattermost-mobile and desktop use different versioning schemes, but - # releases are coordinated with mattermost/mattermost by due date. - # mobile v2.40.0 due 2025-03-15 == mattermost v11.7.0 due 2025-03-15 - # => docs branch v11.7-documentation. - - name: Resolve docs base branch - id: base-branch - env: - GH_TOKEN: ${{ steps.read-token.outputs.token }} - MILESTONE_TITLE: ${{ steps.vars.outputs.milestone_title }} - MILESTONE_DUE: ${{ steps.pr-data.outputs.milestone_due }} - REPO_NAME: ${{ steps.vars.outputs.repo_name }} - run: | - derive_branch_from_title() { - local title="$1" - local version - version=$(echo "$title" | grep -oE 'v[0-9]+\.[0-9]+' | head -1) - echo "$version" - } - - if [ "$REPO_NAME" == "mattermost" ]; then - VERSION=$(derive_branch_from_title "$MILESTONE_TITLE") - if [ -z "$VERSION" ]; then - echo "::warning::Could not parse vMAJOR.MINOR from milestone" \ - "'${MILESTONE_TITLE}' - targeting master." - echo "base=master" >> "$GITHUB_OUTPUT" - exit 0 - fi - echo "Derived docs branch: ${VERSION}-documentation" - - else - if [ -z "$MILESTONE_DUE" ] || [ "$MILESTONE_DUE" == "null" ]; then - echo "::warning::Milestone '${MILESTONE_TITLE}' has no due date" \ - "- cannot match to a mattermost/mattermost release. Targeting master." - echo "base=master" >> "$GITHUB_OUTPUT" - exit 0 - fi - - MILESTONE_DATE="${MILESTONE_DUE%%T*}" - MM_MILESTONE=$(gh api repos/mattermost/mattermost/milestones \ - --paginate \ - | jq -r --arg date "$MILESTONE_DATE" \ - '.[] | select(((.due_on // "")[0:10]) == $date) | .title' \ - | head -1) - - if [ -z "$MM_MILESTONE" ]; then - echo "::warning::No mattermost/mattermost milestone found with" \ - "due date '${MILESTONE_DUE}'. Targeting master." - echo "base=master" >> "$GITHUB_OUTPUT" - exit 0 - fi - - VERSION=$(derive_branch_from_title "$MM_MILESTONE") - if [ -z "$VERSION" ]; then - echo "::warning::Could not parse vMAJOR.MINOR from matched" \ - "milestone '${MM_MILESTONE}'. Targeting master." - echo "base=master" >> "$GITHUB_OUTPUT" - exit 0 - fi - - echo "Matched '${MILESTONE_TITLE}' to '${MM_MILESTONE}'" \ - "=> ${VERSION}-documentation" - fi - - DOCS_BRANCH="${VERSION}-documentation" - - EXISTS=$(gh api "repos/mattermost/docs/branches/${DOCS_BRANCH}" \ - --jq '.name' 2>/dev/null || echo "") - - if [ -n "$EXISTS" ]; then - echo "base=$DOCS_BRANCH" >> "$GITHUB_OUTPUT" - echo "Docs branch '${DOCS_BRANCH}' found." - else - echo "base=master" >> "$GITHUB_OUTPUT" - echo "::warning::Branch '${DOCS_BRANCH}' not found in" \ - "mattermost/docs - targeting master." \ - "Ensure docs-branch-create.yml has run for this milestone." - fi - - # 4. Check for an existing docs PR (idempotency) - - name: Check for existing docs PR - id: existing - env: - GH_TOKEN: ${{ steps.write-token.outputs.token }} - BRANCH: ${{ steps.vars.outputs.branch }} - run: | - RESULT=$(gh pr list \ - --repo mattermost/docs \ - --head "$BRANCH" \ - --state open \ - --json number,url \ - --jq 'first // empty') - - if [ -n "$RESULT" ]; then - echo "exists=true" >> "$GITHUB_OUTPUT" - echo "number=$(echo "$RESULT" | jq -r .number)" >> "$GITHUB_OUTPUT" - echo "url=$(echo "$RESULT" | jq -r .url)" >> "$GITHUB_OUTPUT" - EXISTING_URL=$(echo "$RESULT" | jq -r .url) - echo "::notice::Docs PR already open at ${EXISTING_URL} - updating content only." - else - echo "exists=false" >> "$GITHUB_OUTPUT" - fi - - # 5. Checkout docs repo - # Pinned to a specific commit SHA for supply-chain security. - # To update: find the SHA for the desired tag at - # https://github.com/actions/checkout/releases - - name: Checkout docs repo - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - repository: mattermost/docs - token: ${{ steps.write-token.outputs.token }} - path: docs-repo - fetch-depth: 1 - - - name: Configure git identity - working-directory: docs-repo - run: | - git config user.name "${{ vars.CHANGELOG_GIT_USERNAME }}" - git config user.email "${{ vars.CHANGELOG_GIT_EMAIL }}" - - - name: Set up branch - working-directory: docs-repo - env: - BRANCH: ${{ steps.vars.outputs.branch }} - BASE_BRANCH: ${{ steps.base-branch.outputs.base }} - run: | - if git ls-remote --exit-code origin "$BRANCH" > /dev/null 2>&1; then - git fetch origin "$BRANCH" - git checkout "$BRANCH" - else - git fetch origin "$BASE_BRANCH" - git checkout -b "$BRANCH" "origin/$BASE_BRANCH" - fi - - # 6. Fetch PR content for Claude - - name: Fetch PR diff and description - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ steps.vars.outputs.pr_number }} - SOURCE_REPO: ${{ steps.vars.outputs.source_repo }} - run: | - gh pr diff "$PR_NUMBER" --repo "$SOURCE_REPO" 2>/dev/null \ - | head -c 20000 > /tmp/pr-diff.txt || true - - gh pr view "$PR_NUMBER" --repo "$SOURCE_REPO" \ - --json body --jq '.body // ""' > /tmp/pr-body.txt - - # 7. Generate documentation draft via Claude - # - # System prompt is read from mattermost/docs at runtime so it can be - # updated independently of this workflow file. - # File location in mattermost/docs: .github/prompts/draft_docs.md - # - # Raw API call (not anthropics/claude-code-action) because: - # - We supply a custom system prompt read from a file in the docs repo - # - We HTML-escape untrusted PR content before insertion (security) - # - We validate the response structure before writing any file - # claude-code-action is designed for code-execution tasks and does not - # support file-based system prompts or the output validation required - # by the security model here. - # - # Model: claude-sonnet-4-6 balances quality and cost for doc drafting - # and is consistent with other Mattermost docs automation workflows. - - name: Generate documentation draft - env: - ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} - PR_TITLE: ${{ steps.vars.outputs.pr_title }} - PR_NUMBER: ${{ steps.vars.outputs.pr_number }} - SOURCE_REPO: ${{ steps.vars.outputs.source_repo }} - MILESTONE_TITLE: ${{ steps.vars.outputs.milestone_title }} - run: | - PROMPT_FILE="docs-repo/.github/prompts/draft_docs.md" - if [ ! -f "$PROMPT_FILE" ]; then - echo "::error::Prompt file not found: $PROMPT_FILE" - echo "Add draft_docs.md to mattermost/docs at .github/prompts/draft_docs.md" - exit 1 - fi - cp "$PROMPT_FILE" /tmp/system_prompt.txt - - # Untrusted PR content is HTML-escaped then wrapped in XML tags. - # Escaping converts < > & to < > & so that an attacker - # cannot inject a closing tag (e.g. ) to break out - # of the data section and inject arbitrary instructions. - escape_xml() { - python3 -c \ - 'import sys,html;print(html.escape(sys.stdin.read(),quote=False),end="")' - } - - escape_xml < /tmp/pr-body.txt > /tmp/pr-body-safe.txt - escape_xml < /tmp/pr-diff.txt > /tmp/pr-diff-safe.txt - - # Escape user-controlled metadata fields for the prompt. - # PR title is set by the PR author; milestone title is set by - # maintainers but escaped here for defence-in-depth. - PR_TITLE_SAFE=$(printf '%s' "$PR_TITLE" | escape_xml) - MILESTONE_SAFE=$(printf '%s' "$MILESTONE_TITLE" | escape_xml) - - PR_URL="https://github.com/${SOURCE_REPO}/pull/${PR_NUMBER}" - { - echo "" - echo "PR URL: ${PR_URL}" - echo "PR Title: ${PR_TITLE_SAFE}" - echo "Source Repository: ${SOURCE_REPO}" - echo "milestone.title: ${MILESTONE_SAFE}" - echo "" - echo "" - echo "" - cat /tmp/pr-body-safe.txt - echo "" - echo "" - echo "" - cat /tmp/pr-diff-safe.txt - echo "" - } > /tmp/user_message.txt - - PAYLOAD=$(jq -n \ - --rawfile system /tmp/system_prompt.txt \ - --rawfile content /tmp/user_message.txt \ - '{ - model: "claude-sonnet-4-6", - max_tokens: 2048, - system: $system, - messages: [{ - role: "user", - content: $content - }] - }') - - # --max-time 60: fail fast if the API is unresponsive. - # Capture the full response before piping so we can inspect it. - RESPONSE=$(curl -sf --max-time 60 \ - https://api.anthropic.com/v1/messages \ - -H "x-api-key: $ANTHROPIC_API_KEY" \ - -H "anthropic-version: 2023-06-01" \ - -H "content-type: application/json" \ - -d "$PAYLOAD") || { - echo "::error::Claude API call failed (timeout or HTTP error)." - exit 1 - } - - echo "$RESPONSE" | jq -r '.content[0].text' > /tmp/generated-docs.md - - # Fail immediately if the API returned null or an empty body. - # Log only the error type and message from the JSON response - not - # the full body - to avoid exposing API metadata in workflow logs. - if [ ! -s /tmp/generated-docs.md ] || \ - [ "$(cat /tmp/generated-docs.md)" = "null" ]; then - ERROR_INFO=$(echo "$RESPONSE" | jq -r ' - if .error then "\(.error.type): \(.error.message)" - else "unexpected null or empty content" - end' 2>/dev/null || echo "response could not be parsed") - echo "::error::Claude API returned empty/null content. ${ERROR_INFO}" - exit 1 - fi - - # Validate the output matches the required structure defined in the - # system prompt. Missing sections indicate the model was steered away - # from its instructions - a signal of possible prompt injection. - if ! grep -qF '=== CAPABILITY SUMMARY ===' /tmp/generated-docs.md; then - echo "::error::Output missing CAPABILITY SUMMARY section." \ - "Possible prompt injection. Aborting." - exit 1 - fi - if ! grep -qF '=== DOCUMENTATION DRAFT ===' /tmp/generated-docs.md; then - echo "::error::Output missing DOCUMENTATION DRAFT section." \ - "Possible prompt injection. Aborting." - exit 1 - fi - - # 8. Write docs file - - name: Write docs file - working-directory: docs-repo - env: - DOCS_FILE: ${{ steps.vars.outputs.docs_file }} - SOURCE_REPO: ${{ steps.vars.outputs.source_repo }} - PR_NUMBER: ${{ steps.vars.outputs.pr_number }} - PR_TITLE: ${{ steps.vars.outputs.pr_title }} - PR_AUTHOR: ${{ steps.vars.outputs.pr_author }} - run: | - mkdir -p "$(dirname "$DOCS_FILE")" - - # Sanitize PR title for the Markdown heading to prevent XSS if the - # docs are rendered as HTML (e.g. < > become < >). - PR_TITLE_SAFE=$(printf '%s' "$PR_TITLE" | \ - python3 -c \ - 'import sys,html;print(html.escape(sys.stdin.read(),quote=False),end="")') - - { - printf '# %s\n\n' "$PR_TITLE_SAFE" - printf '> **Status:** Draft - auto-generated. Requires editorial review.\n' - printf '> **Source:** [%s#%s](https://github.com/%s/pull/%s)\n' \ - "$SOURCE_REPO" "$PR_NUMBER" "$SOURCE_REPO" "$PR_NUMBER" - printf '> **Dev reviewer:** @%s\n\n' "$PR_AUTHOR" - printf -- '---\n\n' - cat /tmp/generated-docs.md - } > "$DOCS_FILE" - - # 9. Commit and push - - name: Commit and push - working-directory: docs-repo - env: - BRANCH: ${{ steps.vars.outputs.branch }} - DOCS_FILE: ${{ steps.vars.outputs.docs_file }} - SOURCE_REPO: ${{ steps.vars.outputs.source_repo }} - PR_NUMBER: ${{ steps.vars.outputs.pr_number }} - PR_TITLE: ${{ steps.vars.outputs.pr_title }} - run: | - # Stage only the specific docs file - not the entire working tree. - git add "$DOCS_FILE" - if git diff --cached --quiet; then - echo "No content changes - skipping commit." - else - # Sanitize PR title for commit message: PR_TITLE is author-controlled - # and may contain newlines, quotes, or be very long. Strip control - # characters and truncate to 72 chars (git convention). - PR_TITLE_SHORT=$(printf '%s' "$PR_TITLE" | tr -d '\n\r' | cut -c1-72) - # Use printf with a literal format string and positional arguments, - # then pipe to -F - so the message is never interpolated into a - # shell command string. This avoids any risk of shell metacharacters - # in PR_TITLE_SHORT being evaluated. - printf 'docs: draft for %s#%s - %s\n' \ - "$SOURCE_REPO" "$PR_NUMBER" "$PR_TITLE_SHORT" \ - | git commit -F - - git push origin "$BRANCH" - fi - - # 10. Sync milestone to mattermost/docs - - name: Sync milestone - id: docs-milestone - env: - GH_TOKEN: ${{ steps.write-token.outputs.token }} - MILESTONE_TITLE: ${{ steps.vars.outputs.milestone_title }} - run: | - # Pipe to standalone jq with --arg to pass the title safely. - # gh api does not support jq variable bindings directly. - NUM=$(gh api 'repos/mattermost/docs/milestones?state=all' \ - --paginate \ - | jq -r --arg title "$MILESTONE_TITLE" \ - '.[] | select(.title == $title) | .number' \ - | head -1) - - if [ -z "$NUM" ]; then - NUM=$(gh api repos/mattermost/docs/milestones \ - --method POST \ - -f title="$MILESTONE_TITLE" \ - --jq '.number') - echo "::notice::Created milestone '$MILESTONE_TITLE' (#$NUM) in mattermost/docs." - fi - - echo "number=$NUM" >> "$GITHUB_OUTPUT" - - # 11a. Create docs PR - - name: Create docs PR - id: create-pr - if: steps.existing.outputs.exists == 'false' - env: - GH_TOKEN: ${{ steps.write-token.outputs.token }} - BRANCH: ${{ steps.vars.outputs.branch }} - MILESTONE_TITLE: ${{ steps.vars.outputs.milestone_title }} - SOURCE_REPO: ${{ steps.vars.outputs.source_repo }} - PR_NUMBER: ${{ steps.vars.outputs.pr_number }} - PR_TITLE: ${{ steps.vars.outputs.pr_title }} - PR_AUTHOR: ${{ steps.vars.outputs.pr_author }} - run: | - PR_LINK="[${SOURCE_REPO}#${PR_NUMBER}]" - PR_LINK="${PR_LINK}(https://github.com/${SOURCE_REPO}/pull/${PR_NUMBER})" - BODY=$(printf '%s\n\n%s\n\n%s\n%s\n%s\n%s\n%s\n\n%s' \ - "Documentation draft for ${PR_LINK}." \ - "> Auto-generated by Claude - requires editorial review before merging." \ - "| Field | Value |" \ - "|---|---|" \ - "| **Source PR** | ${PR_LINK} |" \ - "| **Dev reviewer** | @${PR_AUTHOR} |" \ - "| **Milestone** | ${MILESTONE_TITLE} |" \ - "") - - # Sanitize PR title for the docs PR title: same rules as commit message. - PR_TITLE_SHORT=$(printf '%s' "$PR_TITLE" | tr -d '\n\r' | cut -c1-72) - - # Open as a draft so a human must explicitly un-draft before merging. - # This is the primary control against auto-committing injected content. - # gh pr create --milestone takes a title string, not a numeric ID. - URL=$(gh pr create \ - --repo mattermost/docs \ - --head "$BRANCH" \ - --base "${{ steps.base-branch.outputs.base }}" \ - --title "docs: ${PR_TITLE_SHORT}" \ - --body "$BODY" \ - --milestone "$MILESTONE_TITLE" \ - --draft) - - echo "number=$(echo "$URL" | grep -oE '[0-9]+$')" >> "$GITHUB_OUTPUT" - echo "url=$URL" >> "$GITHUB_OUTPUT" - - # 11b. Update existing docs PR milestone (re-trigger path) - - name: Update existing docs PR - if: steps.existing.outputs.exists == 'true' - env: - GH_TOKEN: ${{ steps.write-token.outputs.token }} - DOCS_PR: ${{ steps.existing.outputs.number }} - MILESTONE_TITLE: ${{ steps.vars.outputs.milestone_title }} - run: | - gh pr edit "$DOCS_PR" \ - --repo mattermost/docs \ - --milestone "$MILESTONE_TITLE" - - # 12. Resolve final docs PR reference - - name: Resolve docs PR reference - id: resolve - run: | - if [ "${{ steps.existing.outputs.exists }}" == "true" ]; then - echo "number=${{ steps.existing.outputs.number }}" >> "$GITHUB_OUTPUT" - echo "url=${{ steps.existing.outputs.url }}" >> "$GITHUB_OUTPUT" - else - echo "number=${{ steps.create-pr.outputs.number }}" >> "$GITHUB_OUTPUT" - echo "url=${{ steps.create-pr.outputs.url }}" >> "$GITHUB_OUTPUT" - fi - - # 13. Add dev PR author as reviewer - - name: Add reviewer - env: - GH_TOKEN: ${{ steps.write-token.outputs.token }} - PR_AUTHOR: ${{ steps.vars.outputs.pr_author }} - run: | - gh pr edit "${{ steps.resolve.outputs.number }}" \ - --repo mattermost/docs \ - --add-reviewer "$PR_AUTHOR" \ - || echo "::warning::Could not add @${PR_AUTHOR} as reviewer" \ - "- they may need docs repo access first." - - # 14. Comment on the dev PR - - name: Comment on dev PR - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ steps.vars.outputs.pr_number }} - SOURCE_REPO: ${{ steps.vars.outputs.source_repo }} - DOCS_PR_NUMBER: ${{ steps.resolve.outputs.number }} - DOCS_PR_URL: ${{ steps.resolve.outputs.url }} - run: | - if [ "${{ steps.existing.outputs.exists }}" == "true" ]; then - ACTION="refreshed at" - else - ACTION="opened at" - fi - - DOCS_LINK="[mattermost/docs#${DOCS_PR_NUMBER}](${DOCS_PR_URL})" - COMMENT=$(printf '%s\n\n%s\n\n%s' \ - "A documentation draft has been ${ACTION} ${DOCS_LINK}." \ - "The docs PR is opened as a **draft**. Please review the AI-generated content, edit as needed, then mark it ready for review before merging." \ - "The Docs/Needed label will be replaced with Docs/Done once the docs PR is merged or closed.") - - gh pr comment "$PR_NUMBER" \ - --repo "$SOURCE_REPO" \ - --body "$COMMENT" diff --git a/.github/workflows/docs-writer-merged-pr.yml b/.github/workflows/docs-writer-merged-pr.yml new file mode 100644 index 00000000000..e72d4f338a6 --- /dev/null +++ b/.github/workflows/docs-writer-merged-pr.yml @@ -0,0 +1,302 @@ +name: Docs Writer (Merged PR) + +# When a PR merges still carrying Docs/Needed, draft the missing pages in this +# repo from the gap sticky comment, then open a draft PR. Closing that draft +# flips Docs/Needed → Docs/Done on the originating PR. +# +# Requires DOCS_AI_APP_CLIENT_ID (variable) + DOCS_AI_APP_PRIVATE_KEY (secret) +# so the opened PR triggers docs-review.yml (GITHUB_TOKEN PRs do not). +# +# Fork merges soft-skip: pull_request from a fork gets no secrets (same posture +# as docs-gap.yml). Re-run via workflow_dispatch after merge if needed. Do not +# switch this to pull_request_target — the sync job must not run on forged +# fork heads named docs/pr-*. + +on: + pull_request: + types: [closed] + workflow_dispatch: + inputs: + pr_number: + description: Merged PR number that still carries Docs/Needed + required: true + type: string + +permissions: + contents: read + pull-requests: write + issues: write + +concurrency: + group: >- + docs-writer-${{ github.event.pull_request.number || inputs.pr_number || github.event.pull_request.head.ref }} + cancel-in-progress: false + +jobs: + draft: + name: Draft docs PR + runs-on: ubuntu-24.04 + if: | + github.event_name == 'workflow_dispatch' || + (github.event.pull_request.merged == true && + contains(github.event.pull_request.labels.*.name, 'Docs/Needed') && + !startsWith(github.event.pull_request.head.ref, 'docs/pr-')) + env: + HAS_ANTHROPIC_KEY: ${{ secrets.ANTHROPIC_API_KEY != '' }} + HAS_DOCS_AI_APP: ${{ vars.DOCS_AI_APP_CLIENT_ID != '' && secrets.DOCS_AI_APP_PRIVATE_KEY != '' }} + steps: + - name: Soft-skip when secrets are unset + id: gate + run: | + set -euo pipefail + if [ "$HAS_ANTHROPIC_KEY" != "true" ]; then + echo "::warning::ANTHROPIC_API_KEY unset; skipping docs writer." + echo "run=false" >> "$GITHUB_OUTPUT" + elif [ "$HAS_DOCS_AI_APP" != "true" ]; then + echo "::warning::DOCS_AI_APP credentials unset; skipping docs writer." + echo "run=false" >> "$GITHUB_OUTPUT" + else + echo "run=true" >> "$GITHUB_OUTPUT" + fi + + - name: Generate App token + if: steps.gate.outputs.run == 'true' + id: app-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.DOCS_AI_APP_CLIENT_ID }} + private-key: ${{ secrets.DOCS_AI_APP_PRIVATE_KEY }} + repositories: mattermost + # Narrow below the App installation grant (cannot exceed it). + permission-contents: write + permission-pull-requests: write + permission-issues: write + + - name: Checkout + if: steps.gate.outputs.run == 'true' + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: master + token: ${{ steps.app-token.outputs.token }} + fetch-depth: 0 + persist-credentials: false + + - name: Resolve source PR + if: steps.gate.outputs.run == 'true' + id: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + INPUT_PR: ${{ inputs.pr_number }} + EVENT_PR: ${{ github.event.pull_request.number }} + EVENT_TITLE: ${{ github.event.pull_request.title }} + EVENT_AUTHOR: ${{ github.event.pull_request.user.login }} + EVENT_MILESTONE_TITLE: ${{ github.event.pull_request.milestone.title }} + EVENT_MILESTONE_NUMBER: ${{ github.event.pull_request.milestone.number }} + run: | + set -euo pipefail + mkdir -p .docs-ai/writer + + if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then + if [[ ! "$INPUT_PR" =~ ^[0-9]+$ ]]; then + echo "::error::pr_number must be a positive integer (got '${INPUT_PR}')." + exit 1 + fi + DATA=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${INPUT_PR}") + PR_NUMBER="$INPUT_PR" + TITLE=$(echo "$DATA" | jq -r '.title') + AUTHOR=$(echo "$DATA" | jq -r '.user.login') + MILESTONE_TITLE=$(echo "$DATA" | jq -r '.milestone.title // ""') + MILESTONE_NUMBER=$(echo "$DATA" | jq -r '.milestone.number // ""') + MERGED=$(echo "$DATA" | jq -r '.merged') + LABELS=$(echo "$DATA" | jq -r '[.labels[].name] | join(",")') + if [ "$MERGED" != "true" ]; then + echo "::error::PR #${PR_NUMBER} is not merged." + exit 1 + fi + if [[ ",$LABELS," != *",Docs/Needed,"* ]]; then + echo "::error::PR #${PR_NUMBER} does not carry Docs/Needed." + exit 1 + fi + else + PR_NUMBER="$EVENT_PR" + TITLE="$EVENT_TITLE" + AUTHOR="$EVENT_AUTHOR" + MILESTONE_TITLE="$EVENT_MILESTONE_TITLE" + MILESTONE_NUMBER="$EVENT_MILESTONE_NUMBER" + fi + + if [ -z "$MILESTONE_NUMBER" ] || [ "$MILESTONE_NUMBER" = "null" ]; then + echo "::error title=Missing milestone::Set a milestone on #${PR_NUMBER} before Docs/Needed can draft pages (version anchor source)." + exit 1 + fi + + printf '%s' "$TITLE" > .docs-ai/writer/pr-title.txt + gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.body // ""' \ + > .docs-ai/writer/pr-body.txt + + { + echo "pr_number=${PR_NUMBER}" + echo "pr_author=${AUTHOR}" + echo "milestone_title=${MILESTONE_TITLE}" + echo "branch=docs/pr-${PR_NUMBER}" + } >> "$GITHUB_OUTPUT" + + - name: Collect merged code diff + if: steps.gate.outputs.run == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + run: | + set -euo pipefail + # Cap so an unbounded monorepo diff cannot overwhelm the prompt. + gh pr diff "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" 2>/dev/null \ + | head -c 80000 > .docs-ai/writer/code.diff || true + + - name: Set up Node + if: steps.gate.outputs.run == 'true' + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: .nvmrc + cache: npm + cache-dependency-path: .github/scripts/docs-ai/package-lock.json + + - name: Install dependencies + if: steps.gate.outputs.run == 'true' + working-directory: .github/scripts/docs-ai + run: npm ci --ignore-scripts + + - name: Validate persona registry + if: steps.gate.outputs.run == 'true' + working-directory: .github/scripts/docs-ai + run: npm test + + - name: Author docs + if: steps.gate.outputs.run == 'true' + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + MILESTONE_TITLE: ${{ steps.pr.outputs.milestone_title }} + PR_BODY_FILE: .docs-ai/writer/pr-body.txt + CODE_DIFF_FILE: .docs-ai/writer/code.diff + DOCS_AI_WRITER_MODEL: ${{ vars.DOCS_AI_WRITER_MODEL }} + DOCS_AI_REVIEW_MODEL: ${{ vars.DOCS_AI_REVIEW_MODEL }} + DOCS_AI_ROUTER_MODEL: ${{ vars.DOCS_AI_ROUTER_MODEL }} + DOCS_AI_MAX_REVISIONS: ${{ vars.DOCS_AI_MAX_REVISIONS }} + REPO_ROOT: ${{ github.workspace }} + run: | + set -euo pipefail + PR_TITLE=$(cat .docs-ai/writer/pr-title.txt) + export PR_TITLE + node .github/scripts/docs-ai/writer/run.mjs + + - name: Configure git identity + if: steps.gate.outputs.run == 'true' + env: + GIT_USERNAME: ${{ vars.DOCS_AI_GIT_USERNAME || 'mattermost-docs-ai[bot]' }} + GIT_EMAIL: ${{ vars.DOCS_AI_GIT_EMAIL || '41898282+github-actions[bot]@users.noreply.github.com' }} + run: | + git config user.name "$GIT_USERNAME" + git config user.email "$GIT_EMAIL" + + - name: Commit and push + if: steps.gate.outputs.run == 'true' + id: push + env: + BRANCH: ${{ steps.pr.outputs.branch }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + GH_TOKEN: ${{ steps.app-token.outputs.token }} + run: | + set -euo pipefail + git remote set-url origin "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" + git checkout -B "$BRANCH" + git add docs/main docs/develop docs/api/examples.mdx docs/api/index.mdx 2>/dev/null || true + if git diff --cached --quiet; then + echo "::error::Writer produced no file changes." + exit 1 + fi + printf 'docs: AI draft for #%s\n' "$PR_NUMBER" | git commit -F - + if git ls-remote --exit-code origin "refs/heads/${BRANCH}" >/dev/null 2>&1; then + git push origin "HEAD:refs/heads/${BRANCH}" --force-with-lease + else + git push -u origin "HEAD:refs/heads/${BRANCH}" + fi + + - name: Open or update draft PR + if: steps.gate.outputs.run == 'true' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + BRANCH: ${{ steps.pr.outputs.branch }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + PR_AUTHOR: ${{ steps.pr.outputs.pr_author }} + run: | + set -euo pipefail + EXISTING=$(gh pr list --head "$BRANCH" --state open --json number,url --jq 'first // empty') + TITLE="docs: AI draft for #${PR_NUMBER}" + + if [ -n "$EXISTING" ]; then + NUM=$(echo "$EXISTING" | jq -r .number) + URL=$(echo "$EXISTING" | jq -r .url) + gh pr edit "$NUM" --body-file .docs-ai/writer/pr-body.md + else + URL=$(gh pr create \ + --base master \ + --head "$BRANCH" \ + --title "$TITLE" \ + --body-file .docs-ai/writer/pr-body.md \ + --draft) + NUM=$(gh pr view "$BRANCH" --json number --jq .number) + gh pr edit "$NUM" --add-reviewer "$PR_AUTHOR" || true + fi + + { + echo "## Docs writer" + echo "" + echo "Draft PR: ${URL}" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Comment on source PR + if: steps.gate.outputs.run == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.pr.outputs.pr_number }} + BRANCH: ${{ steps.pr.outputs.branch }} + run: | + set -euo pipefail + URL=$(gh pr view "$BRANCH" --json url --jq .url) + gh pr comment "$PR_NUMBER" --body "Draft documentation PR opened: ${URL}" + + sync: + name: Mark Docs/Done + runs-on: ubuntu-24.04 + if: | + github.event_name == 'pull_request' && + github.event.action == 'closed' && + github.event.pull_request.merged == true && + startsWith(github.event.pull_request.head.ref, 'docs/pr-') + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: .nvmrc + + - name: Sync labels on source PR + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Provenance: trusted event fields only (not branch/body/label). + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + PR_USER: ${{ github.event.pull_request.user.login }} + DOCS_AI_BOT_LOGIN: ${{ vars.DOCS_AI_BOT_LOGIN }} + HEAD_REF: ${{ github.event.pull_request.head.ref }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + mkdir -p .docs-ai/writer + gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq '.body // ""' \ + > .docs-ai/writer/sync-body.txt + PR_BODY_FILE=.docs-ai/writer/sync-body.txt node .github/scripts/docs-ai/writer/sync.mjs