fix(docs-sync): intercept revert PRs and calibrate prompts

.github/docs-sync/collect.mjs

@@ -7,22 +7,28 @@
  *
  * Pre-filter drops (triage never sees these):
  *   - PRs labeled auto-docs (this bot's own rolling PRs)
- *   - chore/test/ci/build/docs/style/refactor/revert conventional titles
+ *   - chore/test/ci/build/docs/style/refactor conventional titles
  *   - PRs touching only docs/non-product paths
  *
+ * Revert PRs (conventional `revert(...):` and GitHub-native `Revert "..."` titles)
+ * are intercepted as signals, never added to the digest; affected digest entries
+ * gain `reverted_by` so triage/edit can skip them.
+ *
  * Bot-authored PRs are kept: release/dependency bots ship user-facing
  * changes too, and the label + docs-only guards above prevent loops.
  */
 
 import fs from "node:fs"
 import { api, appendOutput, appendSummary, listPrFiles, searchIssues } from "./lib.mjs"
+import { revertTitleKind, parseRevertTargets, applyRevertAnnotations, unannotatedRevertSignals } from "./reverts.mjs"
 
 const SOURCE_REPOS = ["Kilo-Org/cloud", "Kilo-Org/kilocode"]
 const OUT_DIR = "docs-sync-out"
 const BODY_LIMIT = 2000
 const SLIM_BODY_LIMIT = 300
 const PATCH_LIMIT = 8000
 const FILE_LIMIT = 30
+// Revert PRs (conventional AND GitHub-native) are intercepted below BEFORE this filter; the "revert" alternative here is unreachable and kept only to minimize diff.
 const DROP_TITLE = /^(chore|test|ci|build|docs|style|refactor|revert)(\(.+\))?!?:/i
 const DOCS_ONLY_PATH = /^(packages\/kilo-docs\/|\.github\/docs-sync\/|docs-sync-out\/|docs\/|[^/]+\.md$)/
 
@@ -44,7 +50,8 @@ const since = argSince()
 console.log(`collecting PRs merged since ${since.toISOString()}`)
 
 const digest = []
-const dropped = { label: 0, title: 0, docs_only: 0, fetch_error: 0 }
+const dropped = { label: 0, title: 0, docs_only: 0, fetch_error: 0, revert: 0 }
+const reverts = []
 
 for (const fullRepo of SOURCE_REPOS) {
   const prs = await mergedPrs(fullRepo, since)
@@ -56,6 +63,22 @@ for (const fullRepo of SOURCE_REPOS) {
       dropped.label++
       continue
     }
+    if (revertTitleKind(item.title ?? "")) {
+      try {
+        const pr = await api(`/repos/${fullRepo}/pulls/${item.number}`)
+        const targets = parseRevertTargets(pr.body ?? "", fullRepo)
+        reverts.push({ url: pr.html_url, merged_at: pr.merged_at, targets })
+        if (targets.length === 0) {
+          console.warn(`::warning::revert PR ${fullRepo}#${item.number} has no parseable targets`)
+        }
+        dropped.revert++
+      } catch (err) {
+        // One dead revert PR must not abort the run; its targets just go unannotated.
+        console.warn(`::warning::skipping revert ${fullRepo}#${item.number}: ${err.message}`)
+        dropped.fetch_error++
+      }
+      continue
+    }
     if (DROP_TITLE.test(item.title ?? "")) {
       dropped.title++
       continue
@@ -108,6 +131,9 @@ for (const fullRepo of SOURCE_REPOS) {
   }
 }
 
+const applied = applyRevertAnnotations(digest, reverts)
+const unannotated = unannotatedRevertSignals(reverts, applied)
+
 digest.sort((a, b) => new Date(a.merged_at) - new Date(b.merged_at))
 
 fs.mkdirSync(OUT_DIR, { recursive: true })
@@ -124,14 +150,31 @@ console.log(`kept ${digest.length} PRs, dropped:`, dropped)
 appendOutput("count", digest.length)
 appendOutput("digest", `${OUT_DIR}/digest.json`)
 
-appendSummary(
-  [
-    "### docs-sync collect",
-    "",
-    `- window: since \`${since.toISOString()}\``,
-    `- kept: **${digest.length}** PRs`,
-    `- dropped: ${dropped.label} auto-docs, ${dropped.title} title filter, ${dropped.docs_only} docs-only, ${dropped.fetch_error} fetch errors`,
-    "",
-    ...digest.map((d) => `- [${d.repo}#${d.number}](${d.url}) ${d.title}`),
-  ].join("\n"),
-)
+const summaryLines = [
+  "### docs-sync collect",
+  "",
+  `- window: since \`${since.toISOString()}\``,
+  `- kept: **${digest.length}** PRs`,
+  `- dropped: ${dropped.label} auto-docs, ${dropped.title} title filter, ${dropped.docs_only} docs-only, ${dropped.fetch_error} fetch errors, ${dropped.revert} reverts intercepted`,
+  "",
+  ...digest.map((d) => `- [${d.repo}#${d.number}](${d.url}) ${d.title}`),
+]
+if (applied.length > 0) {
+  summaryLines.push("", "**revert annotations:**", ...applied.map(([target, reverter]) => `- ${target} — reverted by ${reverter}`))
+}
+if (unannotated.missed.length > 0 || unannotated.unparsed.length > 0) {
+  summaryLines.push("", "**revert targets with no in-window annotation:**")
+  for (const m of unannotated.missed) {
+    summaryLines.push(`- ${m.url} — unannotated targets: ${m.targets.join(", ")}`)
+  }
+  for (const url of unannotated.unparsed) {
+    summaryLines.push(`- ${url} (no parseable targets)`)
+  }
+}
+if (unannotated.chains.length > 0) {
+  summaryLines.push("", "**revert chains (not annotated):**")
+  for (const c of unannotated.chains) {
+    summaryLines.push(`- ${c.url}${c.targets.length > 0 ? ` (targets: ${c.targets.join(", ")})` : ""}`)
+  }
+}
+appendSummary(summaryLines.join("\n"))

.github/docs-sync/edit-prompt.md

@@ -4,13 +4,16 @@ Before writing anything:
 
 1. Read `packages/kilo-docs/AGENTS.md` and `packages/kilo-docs/STYLE_GUIDE.md` and follow them exactly: Markdoc custom tags, the `/docs` prefix in image paths, navigation files under `lib/nav/`, redirect rules, and the generated-screenshot policy.
 2. Read the attached batch files: the full-details file (PR title, body, file list, `patch_excerpt` diffs) and the triage file (docs-worthiness verdicts, target sections, priorities).
+3. Verify facts against the current source tree — for Kilo-Org/kilocode PRs. This checkout reflects current kilocode main: before documenting a command, flag, setting, default, or behavior from a kilocode PR, confirm it exists in the current source. Existence alone is not enough: defaults, whether an option is required or optional, and on-by-default behavior must also match the current tree — a symbol that still exists as opt-in does not justify documenting default-on behavior. When the merged diff and the current tree disagree, the current tree wins — the change may have been reverted or superseded; skip it and record why. For Kilo-Org/cloud PRs the source is not in this checkout: rely on the PR diff and body, and on the `reverted_by` field below. Reading any file in the checkout for verification is expected; the hard rule against touching anything outside `packages/kilo-docs/` applies to writing only.
 
-For each PR in the batch, in priority order:
+For each PR in the batch, in priority order, first decide whether it needs documentation at all — skipping is a first-class outcome. For each one that does:
 
 - Find the most relevant existing docs page(s) and make minimal, precise updates in the style of the surrounding content.
 - Create a new page only when no existing page fits; then add it to the matching nav file in `packages/kilo-docs/lib/nav/`.
-- Document only behavior that is actually present in the merged diff. If the PR body or diff shows the feature is behind a flag or otherwise not user-visible yet, skip it and record why.
+- Document only behavior that is actually present in the merged diff and still present in the product now (step 3 above: when they disagree, the current state wins). If the PR body or diff shows the feature is behind a flag or otherwise not user-visible yet, skip it and record why.
 - If a PR turns out not to need documentation, skip it and record why. Trust evidence over the triage verdict.
+- A batch PR with a `reverted_by` field was reverted by that PR; skip it unless there is clear evidence the change is present now — for kilocode PRs verify it in the current source tree (re-land); for cloud PRs require explicit re-land evidence in the entry itself.
+- Skipping is a normal outcome: a batch where every PR is skipped is a valid result. Never write docs just to have something to show.
 
 Hard rules:
 

.github/docs-sync/reverts.mjs

@@ -0,0 +1,153 @@
+// kilocode_change - new file
+
+/**
+ * Pure helpers for intercepting revert PRs during docs-sync collect.
+ * No I/O, no imports — offline-testable title/body/annotation logic only.
+ */
+
+/** Detect revert PR titles. Returns "conventional" | "github-native" | null. */
+export function revertTitleKind(title) {
+  const t = String(title ?? "")
+  if (/^revert(\(.+\))?!?:/i.test(t)) return "conventional"
+  if (/^revert\s+["']/i.test(t)) return "github-native"
+  return null
+}
+
+/**
+ * Parse revert targets from a PR body. `defaultRepo` ("Kilo-Org/kilocode") resolves bare `#N`.
+ * Returns [{ repo, number, url }] with url = `https://github.com/${repo}/pull/${number}`.
+ * Handles the conventional trailer form: a line starting with `Reverts` (case-insensitive),
+ * e.g. `Reverts #12249 and #12481.`, `Reverts Kilo-Org/cloud#42.`, comma-separated lists,
+ * several such lines in one body, and bulleted/quoted single-line trailers
+ * (`- Reverts #12249`, `> Reverts #12249`).
+ * NOT handled (documented limitation): `This reverts commit <sha>.` (no PR number),
+ * mid-sentence forms (`This reverts #5.`), narrative mentions (`Revert the fix in #99999`
+ * is prose, not a trailer — must NOT produce a target), `Revertsomething #5` (prose glued
+ * to the word — `\b` word boundary makes it inert), and multi-line lists
+ * (`Reverts:\n- #1\n- #2`).
+ */
+export function parseRevertTargets(body, defaultRepo) {
+  const text = String(body ?? "")
+  const repoDefault = String(defaultRepo ?? "")
+  const seen = new Set()
+  const out = []
+  const lineRe = /^[ \t>*-]*reverts\b[ \t:]*([^\n]*)/gim
+  let lineMatch
+  while ((lineMatch = lineRe.exec(text)) !== null) {
+    const capture = lineMatch[1] ?? ""
+    const refRe = /(?:([A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+))?#(\d+)/g
+    let refMatch
+    while ((refMatch = refRe.exec(capture)) !== null) {
+      const repo = refMatch[1] || repoDefault
+      const number = Number(refMatch[2])
+      if (!repo || !Number.isInteger(number)) continue
+      const url = `https://github.com/${repo}/pull/${number}`
+      if (seen.has(url)) continue
+      seen.add(url)
+      out.push({ repo, number, url })
+    }
+  }
+  return out
+}
+
+/**
+ * revertSignals: [{ url, merged_at, targets: [{ repo, number, url }] }]
+ * Returns Map<targetUrl, { url, merged_at }> — the annotation for each reverted PR.
+ * Map keys are lowercased target urls; values keep signal.url as authored.
+ * Revert-of-revert: a signal whose own url is itself a target of another signal is
+ * dropped entirely (a re-land: net effect zero — one set lookup, no chain walking).
+ */
+export function computeRevertAnnotations(revertSignals) {
+  const signals = Array.isArray(revertSignals) ? revertSignals : []
+  const revertedUrls = new Set()
+  for (const signal of signals) {
+    for (const t of signal?.targets ?? []) {
+      if (t?.url) revertedUrls.add(t.url.toLowerCase())
+    }
+  }
+  const annotations = new Map()
+  for (const signal of signals) {
+    if (!signal?.url || revertedUrls.has(signal.url.toLowerCase())) continue
+    for (const t of signal.targets ?? []) {
+      if (!t?.url) continue
+      annotations.set(t.url.toLowerCase(), { url: signal.url, merged_at: signal.merged_at })
+    }
+  }
+  return annotations
+}
+
+/**
+ * Applies computeRevertAnnotations to digest entries in place: an entry whose url
+ * was reverted gains `reverted_by: { url, merged_at }` (the reverter).
+ * Lookup is case-insensitive; applied pairs still report entry.url (canonical).
+ * Returns applied [targetUrl, reverterUrl] pairs (digest entries only) for reporting.
+ */
+export function applyRevertAnnotations(digest, revertSignals) {
+  const annotations = computeRevertAnnotations(revertSignals)
+  const applied = []
+  for (const entry of digest ?? []) {
+    if (!entry?.url) continue
+    const ann = annotations.get(entry.url.toLowerCase())
+    if (!ann) continue
+    entry.reverted_by = { url: ann.url, merged_at: ann.merged_at }
+    applied.push([entry.url, ann.url])
+  }
+  return applied
+}
+
+/**
+ * Reports intercepted reverts whose targets received no digest annotation, for the
+ * step summary. `signals` is the same shape as computeRevertAnnotations takes;
+ * `appliedPairs` is applyRevertAnnotations' return ([targetUrl, reverterUrl]).
+ * Returns { missed, unparsed, chains }:
+ * - missed: [{ url, targets: [targetUrl, ...] }] — one entry per signal with at least
+ *   one unannotated plain target, listing exactly the targets that missed (a partially-covered
+ *   revert still surfaces its uncovered targets). Plain = not itself an intercepted revert.
+ * - unparsed: [url, ...] — urls of signals with zero targets (they already warn at
+ *   intercept time).
+ * - chains: [{ url, targets: [targetUrl, ...] }] — signals involved in a revert chain,
+ *   reported for human visibility only; the pipeline never resolves a chain's net effect.
+ *   A signal lands here when EITHER its own url is a target of another signal (cancelled
+ *   by computeRevertAnnotations — lists ALL its targets), OR at least one of its targets
+ *   is itself an intercepted revert (re-land chain link — lists ONLY those chain-link
+ *   targets; plain targets still flow through missed).
+ * Cancelled signals skip missed/unparsed. Chain-linked (non-cancelled) signals may also
+ * appear in missed for their unannotated plain targets. Chain-link targets never appear
+ * in missed. All url comparisons are case-insensitive.
+ */
+export function unannotatedRevertSignals(signals, appliedPairs) {
+  const list = Array.isArray(signals) ? signals : []
+  const annotated = new Set((appliedPairs ?? []).map(([target]) => String(target ?? "").toLowerCase()))
+  const signalUrls = new Set(list.map((s) => String(s?.url ?? "").toLowerCase()))
+  const cancelledUrls = new Set()
+  for (const s of list) {
+    for (const t of s?.targets ?? []) {
+      if (t?.url) cancelledUrls.add(t.url.toLowerCase())
+    }
+  }
+  const missed = []
+  const unparsed = []
+  const chains = []
+  for (const s of list) {
+    const url = s?.url
+    if (!url) continue
+    const targets = (s.targets ?? []).map((t) => t?.url).filter(Boolean)
+    const cancelled = cancelledUrls.has(url.toLowerCase())
+    const chainTargets = targets.filter((u) => signalUrls.has(u.toLowerCase()))
+    if (cancelled) {
+      chains.push({ url, targets })
+      continue
+    }
+    if (chainTargets.length > 0) {
+      chains.push({ url, targets: chainTargets })
+    }
+    if (targets.length === 0) {
+      unparsed.push(url)
+      continue
+    }
+    const plainTargets = targets.filter((u) => !signalUrls.has(u.toLowerCase()))
+    const missing = plainTargets.filter((u) => !annotated.has(u.toLowerCase()))
+    if (missing.length > 0) missed.push({ url, targets: missing })
+  }
+  return { missed, unparsed, chains }
+}