From c5d2aa2ac58fcd02dfbcc0b432bd7f8e869bb84b Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:17:48 -0700 Subject: [PATCH 01/15] docs: design spec for CX-3425 reference sync, slugs, and linting fixes Co-Authored-By: Claude Opus 4.8 (1M context) --- ...425-reference-sync-slugs-linting-design.md | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md diff --git a/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md b/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md new file mode 100644 index 0000000..57bc7ff --- /dev/null +++ b/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md @@ -0,0 +1,164 @@ +# CX-3425 — Reference sync, slugs, and linting fixes + +**Ticket:** [CX-3425](https://linear.app/readme-io/issue/CX-3425/misc-cli-issues-around-reference-sync-slugs-and-linting) +**Date:** 2026-06-17 +**Branch:** `jesse/cx-3425-misc-cli-issues-around-reference-sync-slugs-and-linting` + +## Background + +Feedback on the ReadMe CLI surfaced five follow-up issues across reference syncing, +slug handling, and linting. OpenAPI specs are now the source of truth for `reference/` +content, but the CLI still validates and rewrites Markdown frontmatter as if the +Markdown owned the title/excerpt. Separately, the linter produces false positives for +duplicate slugs and unknown components, the `-1` slug renamer misses files and emits no +redirects, and stale `_order.yaml` entries go uncaught. + +This spec covers all five items. There is currently **no test infrastructure** in the +repo (`"test": "echo \"No tests yet\""`); a lightweight `node:test` setup is added as +part of this work. + +## Goals + +1. Stop the CLI from owning `title`/`excerpt` on OAS-backed reference pages. +2. Improve `-1` slug rename coverage and emit a redirect file. +3. Stop flagging slugs shared across `docs/` and `reference/` as duplicates. +4. Stop failing CI on unknown (global/Enterprise) custom blocks. +5. Flag stale `_order.yaml` entries that no longer exist on disk. + +## Non-goals + +- Redirecting `-1` URLs to the base slug in the **conflict case** (base already exists, + so no rename happens). The `bidi_remove_-1.js` script does not handle this either; + tracked as a follow-up. +- Fetching global custom blocks from the ReadMe API, or adding an allowlist config. +- Stripping pre-existing `title`/`excerpt` from reference Markdown (non-destructive). + +--- + +## Item 1 — Reference is OAS-owned (drop title/excerpt ownership) + +OAS is the source of truth for `reference/`. The CLI should stop writing, requiring, +and sync-checking `title`/`excerpt` on api-backed reference pages. They render from the +spec. + +### `src/commands/oas-sync.js` +- `buildPageContent`: emit frontmatter with **only** `api.file` + `api.operationId`. + Remove the `title` and `excerpt` fields. +- `syncOneOas`: remove the title/excerpt **update** branch (the `else if (!skipUpdates)` + block). Keep add and delete behavior. Remove the now-unused `skipUpdates`/ + `isReadMeConfig` plumbing for updates. +- Existing pages are not modified beyond add/delete — this is non-destructive for any + `title`/`excerpt` already present. + +### `src/validators/frontmatter.js` +- For pages where `relativePath` starts with `reference/` **and** parsed frontmatter has + `api.file`, suppress the missing-`title` schema error (filter AJV errors where + `err.keyword === 'required'` and `err.params.missingProperty === 'title'`). All other + frontmatter validation is unchanged. + +### `src/validators/oas-reference.js` +- Remove the two out-of-sync checks (`title`, `excerpt`) and the now-moot + `isReadMeConfig` skip block. +- Keep: *OAS file not found*, *operation not found*, *missing page*. + +**Effect:** The reported errors disappear: +`Invalid frontmatter: must have required property 'title'`, +`Out of sync: title is "undefined"...`, +`Out of sync: excerpt does not match spec description...`. + +--- + +## Item 2 — `-1` slugs → redirects, folded into `numbering.js` + +Port the useful behavior of `bidi_remove_-1.js` into the existing +`src/validators/numbering.js` (which already renames `-N` files/dirs and updates +`_order.yaml` on `--fix`), rather than adding a parallel command. + +### `src/validators/numbering.js` +- **Narrow the suffix match to a single digit**: `/-(\d)$/` instead of `/-(\d+)$/`, + matching `bidi_remove_-1.js` and ReadMe's auto-dedup behavior (`foo-1`, not `foo-123`). + This is a deliberate narrowing of current coverage, approved. +- On `--fix`, when a rename is applied, record redirect lines in the script's + **bidirectional** format, for both sections regardless of the page's location: + - `/docs/ -> /docs/` + - `/reference/ -> /reference/` +- After all renames, write the redirect file to + `~/Desktop/_redirect.txt` (matching the script's output location), + one mapping per line. +- Redirect file is only written when renames are actually applied (i.e. on `--fix` with + confirmed renames). No file on a plain lint run. + +**Coverage win:** `numbering.js` walks the filesystem rather than `_order.yaml` entries, +so it catches the files the original script missed. + +**Out of scope:** the conflict case (base slug already exists) — `numbering.js` already +declines to rename it, and no redirect is emitted, matching the script. + +--- + +## Item 3 — Duplicate slugs scoped per section + +Slugs may legitimately repeat across `docs/` and `reference/`. Uniqueness should be +enforced within each section, not globally. + +### `src/validators/duplicates.js` +- Key the slug map by `":"` rather than bare `slug`. +- A slug appearing in both `docs/` and `reference/` is no longer flagged; duplicates + within the same section still are. +- The existing `ReadMeConfig/` skip is preserved. + +--- + +## Item 4 — Unknown components → warning + +Enterprise projects rely on global custom blocks (e.g. ``) defined in the +ReadMe app, not in the repo. These should not fail CI. + +### `src/validators/components.js` +- In `validateAll`, set `severity: 'warning'` on the "Unknown component" result. +- Per-file `custom_blocks/` validation is unchanged. +- No allowlist / config (kept simple by decision). + +--- + +## Item 5 — Stale `_order.yaml` entries flagged + +`ordering.js` only checks files *missing from* `_order.yaml`. It should also flag entries +*present in* `_order.yaml` with no matching file/folder on disk. + +### `src/validators/ordering.js` +- Add a new check, independent of `ORDERED_DIRS`: for every `_order.yaml` under content + dirs (`docs`, `reference`, `recipes`, `custom_pages`), flag any entry with no matching + `.md`, `.mdx`, or `/` directory in that folder. + - Severity: `warning`. Fixable: on `--fix`, remove the stale entry from `_order.yaml`. +- Also flag `index` / `index.md` entries appearing in `_order.yaml` (they should not be + listed) — directly answering the ticket's open question. +- The existing "missing from `_order.yaml`" behavior (scoped to `ORDERED_DIRS`) is + untouched, so adding `reference` to the stale check does not introduce + "missing from order" noise for OAS-managed reference pages. + +--- + +## Testing + +Add a minimal test harness using Node's built-in test runner (zero new dependencies): + +- `package.json`: `"test": "node --test"`. +- Fixture-based tests under `test/` (or `src/validators/__tests__/`) covering the five + changed validators, each with a small temp-dir fixture: + - reference page with only `api` frontmatter → no missing-title / out-of-sync errors. + - same slug in `docs/` and `reference/` → no duplicate error; same-section dup → error. + - unknown component → warning (not error). + - `_order.yaml` with a stale entry and an `index` entry → warnings; `--fix` removes them. + - `numbering.js`: `foo-1.md` with no `foo.md` → rename + bidirectional redirect lines; + `foo-12.md` → not matched. + +### End-to-end verification +Run `lint` and `oas:sync` against the ticket's example repo +[`production-lightcast-api-7a9ba2da223798f9c29a`](https://github.com/readme-internal-sync/production-lightcast-api-7a9ba2da223798f9c29a/tree/v1.0) +and confirm the previously-reported errors no longer appear. + +## Rollout + +Single PR against `main` covering all five items plus tests. Bump the CLI version so a +release can ship the fixes (CastAI / Lightcast are blocked on these). From 67e7d8fe6a568babec856750b934e1f9b79b6c5b Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:18:30 -0700 Subject: [PATCH 02/15] docs: lock test location to test/ for CX-3425 spec Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-17-cx-3425-reference-sync-slugs-linting-design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md b/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md index 57bc7ff..13d6756 100644 --- a/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md +++ b/docs/superpowers/specs/2026-06-17-cx-3425-reference-sync-slugs-linting-design.md @@ -144,8 +144,8 @@ ReadMe app, not in the repo. These should not fail CI. Add a minimal test harness using Node's built-in test runner (zero new dependencies): - `package.json`: `"test": "node --test"`. -- Fixture-based tests under `test/` (or `src/validators/__tests__/`) covering the five - changed validators, each with a small temp-dir fixture: +- Fixture-based tests under `test/` covering the five changed validators, each with a + small temp-dir fixture: - reference page with only `api` frontmatter → no missing-title / out-of-sync errors. - same slug in `docs/` and `reference/` → no duplicate error; same-section dup → error. - unknown component → warning (not error). From 2c7a875b7417de8c592ae4563745a3717e3e392e Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:24:57 -0700 Subject: [PATCH 03/15] docs: implementation plan for CX-3425 fixes Co-Authored-By: Claude Opus 4.8 (1M context) --- ...17-cx-3425-reference-sync-slugs-linting.md | 1163 +++++++++++++++++ 1 file changed, 1163 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-17-cx-3425-reference-sync-slugs-linting.md diff --git a/docs/superpowers/plans/2026-06-17-cx-3425-reference-sync-slugs-linting.md b/docs/superpowers/plans/2026-06-17-cx-3425-reference-sync-slugs-linting.md new file mode 100644 index 0000000..ee6ee59 --- /dev/null +++ b/docs/superpowers/plans/2026-06-17-cx-3425-reference-sync-slugs-linting.md @@ -0,0 +1,1163 @@ +# CX-3425 Reference Sync, Slugs, and Linting Fixes — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix five CLI issues from CX-3425 — reference pages no longer carry CLI-owned title/excerpt, `-1` slugs rename with redirect output, cross-section duplicate slugs and unknown global components stop being false errors, and stale `_order.yaml` entries get flagged. + +**Architecture:** All changes are localized to the existing validators in `src/validators/` and the `src/commands/oas-sync.js` command. The linter auto-discovers validators via `src/utils/lint.js`; each validator exports `validate()` (per-file) and/or `validateAll()` (cross-file) and returns result objects `{ file, rule, message, severity?, fixable? }` (severity defaults to `error`). A new `test/` directory using Node's built-in test runner covers each change with temp-dir fixtures. + +**Tech Stack:** Node.js (ESM), `node:test` runner (built-in, no new deps), `gray-matter`, `ajv`, `js-yaml`, `@readme/markdown`. + +## Global Constraints + +- Node `>=18` (per `package.json` `engines`). +- ESM modules (`"type": "module"`) — use `import`/`export`, not `require` (except via `createRequire` as existing code does). +- No new runtime dependencies. Tests use the built-in `node:test` runner only. +- Validator result objects: `{ file, rule, message }` plus optional `severity` (`'error'` default | `'warning'`) and `fixable` (boolean). A result with `severity !== 'warning'` counts as an error and fails CI. +- Do not strip pre-existing `title`/`excerpt` from reference Markdown — changes are non-destructive to existing files. + +--- + +## Prerequisite (one-time, before Task 1) + +- [ ] Ensure dependencies are installed: + +Run: `npm install` +Expected: completes without error; `node_modules/` present. + +--- + +### Task 1: Test harness setup + +**Files:** +- Modify: `package.json` (the `scripts.test` field) +- Create: `test/helpers.js` +- Create: `test/smoke.test.js` + +**Interfaces:** +- Produces: `makeRepo(filesObject) → string` (absolute temp repo root) and `rmRepo(root) → void`, imported by every later test file. + +- [ ] **Step 1: Add the test script** + +In `package.json`, replace the `test` script: + +```json + "scripts": { + "start": "node bin/readme.js", + "test": "node --test" + }, +``` + +- [ ] **Step 2: Create the test helper** + +Create `test/helpers.js`: + +```js +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +/** + * Write a set of files into a fresh temp directory and return its absolute root. + * @param {Record} files Map of repo-relative path -> file content. + */ +export function makeRepo(files) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'rdme-cli-test-')); + for (const [rel, content] of Object.entries(files)) { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + } + return root; +} + +export function rmRepo(root) { + fs.rmSync(root, { recursive: true, force: true }); +} +``` + +- [ ] **Step 3: Write the smoke test** + +Create `test/smoke.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('makeRepo writes files and rmRepo cleans up', () => { + const root = makeRepo({ 'docs/a.md': 'hello' }); + assert.equal(fs.readFileSync(path.join(root, 'docs/a.md'), 'utf-8'), 'hello'); + rmRepo(root); + assert.equal(fs.existsSync(root), false); +}); +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `npm test` +Expected: PASS — `smoke.test.js` reports 1 passing test, exit 0. + +- [ ] **Step 5: Commit** + +```bash +git add package.json test/helpers.js test/smoke.test.js +git commit -m "test: add node:test harness and temp-repo helper" +``` + +--- + +### Task 2: Item 3 — scope duplicate slugs per section + +**Files:** +- Modify: `src/validators/duplicates.js` +- Test: `test/duplicates.test.js` + +**Interfaces:** +- Consumes: `validateAll(files: string[]) → results[] | null` (signature unchanged). + +- [ ] **Step 1: Write the failing test** + +Create `test/duplicates.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { validateAll } from '../src/validators/duplicates.js'; + +test('same slug across docs and reference is allowed', () => { + const res = validateAll(['docs/intro.md', 'reference/intro.md']); + assert.equal(res, null); +}); + +test('same slug within docs (different subdirs) is flagged', () => { + const res = validateAll(['docs/a/intro.md', 'docs/b/intro.md']); + assert.ok(Array.isArray(res) && res.length >= 1); + assert.match(res[0].message, /Duplicate slug: "intro"/); + assert.equal(res[0].severity, 'error'); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/duplicates.test.js` +Expected: FAIL — "same slug across docs and reference is allowed" fails because the current global slug map flags `intro` across the two dirs. + +- [ ] **Step 3: Implement the change** + +In `src/validators/duplicates.js`, change the grouping to key by `:` and derive the slug back out in the report loop. Replace the body of `validateAll`: + +```js +export function validateAll(files) { + const results = []; + + // Group files by ":" so slugs only collide within the same section. + const slugMap = new Map(); + for (const relPath of files) { + const topDir = relPath.split('/')[0]; + if (!CHECKED_DIRS.includes(topDir)) continue; + + const filename = path.basename(relPath); + if (filename === 'index.md' || filename === 'index.mdx') continue; + + const slug = filename.replace(/\.(md|mdx)$/, ''); + const key = `${topDir}:${slug}`; + if (!slugMap.has(key)) slugMap.set(key, []); + slugMap.get(key).push(relPath); + } + + for (const [key, paths] of slugMap) { + if (paths.length < 2) continue; + + // TODO: figure out why ReadMeConfig causes duplicate slugs and handle properly. + // For now, skip any duplicate set that involves a ReadMeConfig path. + if (paths.some((p) => p.includes('ReadMeConfig/'))) continue; + + const slug = key.slice(key.indexOf(':') + 1); + const others = paths.slice(1); + for (const relPath of others) { + const otherLocations = paths.filter((p) => p !== relPath).map((p) => path.dirname(p)).join(', '); + results.push({ + file: relPath, + rule: name, + severity: 'error', + message: `Duplicate slug: "${slug}" also exists in ${otherLocations}`, + }); + } + } + + return results.length > 0 ? results : null; +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `node --test test/duplicates.test.js` +Expected: PASS — both tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/validators/duplicates.js test/duplicates.test.js +git commit -m "fix(lint): scope duplicate slug check per top-level section" +``` + +--- + +### Task 3: Item 4 — unknown components become warnings + +**Files:** +- Modify: `src/validators/components.js` (the `validateAll` unknown-component push) +- Test: `test/components.test.js` + +**Interfaces:** +- Consumes: `validateAll(files: string[], gitRoot: string) → results[]` (signature unchanged). + +- [ ] **Step 1: Write the failing test** + +Create `test/components.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/components.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('unknown component is reported as a warning, not an error', () => { + const root = makeRepo({ + 'docs/page.md': '---\ntitle: Page\n---\n\n\n', + }); + try { + const res = validateAll(collectFiles(root), root); + const unknown = res.find((r) => r.message.includes('Unknown component')); + assert.ok(unknown, 'expected an unknown-component result'); + assert.equal(unknown.severity, 'warning'); + } finally { + rmRepo(root); + } +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/components.test.js` +Expected: FAIL — `unknown.severity` is `undefined` (defaults to error), not `'warning'`. + +- [ ] **Step 3: Implement the change** + +In `src/validators/components.js`, inside `validateAll`, add `severity: 'warning'` to the unknown-component result: + +```js + if (!available.has(comp)) { + results.push({ + file: relPath, + rule: name, + severity: 'warning', + message: `Unknown component: <${comp}> is not a built-in or custom block`, + }); + } +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `node --test test/components.test.js` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/validators/components.js test/components.test.js +git commit -m "fix(lint): downgrade unknown component to warning for global blocks" +``` + +--- + +### Task 4: Item 1a — `oas:sync` stops writing/updating title & excerpt + +**Files:** +- Modify: `src/commands/oas-sync.js` (`buildPageContent`, `syncOneOas`, remove `isReadMeConfig`) +- Test: `test/oas-sync.test.js` + +**Interfaces:** +- Consumes: `syncOas(gitRoot: string) → changes[] | null` (signature unchanged). +- Produces: generated reference `.md` files whose frontmatter contains only `api.file` + `api.operationId`. + +- [ ] **Step 1: Write the failing test** + +Create `test/oas-sync.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import matter from 'gray-matter'; +import { syncOas } from '../src/commands/oas-sync.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +const SPEC = JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { + get: { operationId: 'listPets', summary: 'List pets', description: 'Returns pets' }, + }, + }, +}); + +test('generated reference page has only api frontmatter (no title/excerpt)', () => { + const root = makeRepo({ 'reference/pets.json': SPEC }); + try { + syncOas(root); + const page = path.join(root, 'reference/Pets/Other/listPets.md'); + assert.ok(fs.existsSync(page), 'expected generated page'); + const { data } = matter(fs.readFileSync(page, 'utf-8')); + assert.equal(data.api.file, 'pets.json'); + assert.equal(data.api.operationId, 'listPets'); + assert.equal('title' in data, false); + assert.equal('excerpt' in data, false); + } finally { + rmRepo(root); + } +}); + +test('existing reference page title is not overwritten by sync', () => { + const root = makeRepo({ + 'reference/pets.json': SPEC, + 'reference/Pets/Other/listPets.md': + '---\ntitle: My custom title\napi:\n file: pets.json\n operationId: listPets\n---\n', + 'reference/Pets/Other/_order.yaml': '- listPets\n', + 'reference/Pets/_order.yaml': '- Other\n', + }); + try { + syncOas(root); + const { data } = matter( + fs.readFileSync(path.join(root, 'reference/Pets/Other/listPets.md'), 'utf-8'), + ); + assert.equal(data.title, 'My custom title'); + } finally { + rmRepo(root); + } +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/oas-sync.test.js` +Expected: FAIL — first test fails (`title`/`excerpt` are written); second fails (sync overwrites title to "List pets"). + +- [ ] **Step 3: Simplify `buildPageContent`** + +In `src/commands/oas-sync.js`, replace `buildPageContent`: + +```js +function buildPageContent({ oasFilename, operationId }) { + const frontmatter = { + api: { + file: oasFilename, + operationId, + }, + }; + + return matter.stringify('', frontmatter); +} +``` + +- [ ] **Step 4: Remove the update branch and the `skipUpdates`/`isReadMeConfig` plumbing** + +In `src/commands/oas-sync.js`, delete the `isReadMeConfig` function (lines defining it near the top) and rewrite `syncOneOas` so it only adds and deletes (no updates): + +```js +function syncOneOas(refDir, oasFilename, spec) { + const specOps = extractOperations(spec); + const infoTitle = spec.info?.title || path.basename(oasFilename, path.extname(oasFilename)); + + const existingPages = collectExistingPages(refDir).filter( + (p) => p.data.api.file === oasFilename, + ); + + const pagesByOpId = new Map(); + for (const page of existingPages) { + pagesByOpId.set(page.data.api.operationId, page); + } + + const changes = { added: [], deleted: [], updated: [] }; + + // Deletes: pages referencing operations that no longer exist. + for (const [opId, page] of pagesByOpId) { + if (!specOps.has(opId)) { + fs.unlinkSync(page.filePath); + + const pageDir = path.dirname(page.filePath); + const slug = path.basename(page.filePath, '.md'); + removeFromOrder(path.join(pageDir, '_order.yaml'), slug); + + changes.deleted.push(page.relativePath); + } + } + + // Adds: operations with no page yet. Title/excerpt are owned by the OAS spec + // at render time, so generated pages carry only the api reference. + for (const [opId, op] of specOps) { + if (pagesByOpId.has(opId)) continue; + + const tag = op.tag || 'Other'; + const pageDir = path.join(refDir, infoTitle, tag); + fs.mkdirSync(pageDir, { recursive: true }); + + const pagePath = path.join(pageDir, `${opId}.md`); + const content = buildPageContent({ oasFilename, operationId: opId }); + fs.writeFileSync(pagePath, content); + + addToOrder(path.join(pageDir, '_order.yaml'), opId); + addToOrder(path.join(refDir, infoTitle, '_order.yaml'), tag); + + changes.added.push(path.relative(refDir, pagePath)); + } + + return changes; +} +``` + +> Note: `changes.updated` stays in the object (always empty now) so the `run()` summary code in this file is untouched. + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `node --test test/oas-sync.test.js` +Expected: PASS — both tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add src/commands/oas-sync.js test/oas-sync.test.js +git commit -m "fix(oas): stop writing/updating title & excerpt on reference pages" +``` + +--- + +### Task 5: Item 1b — frontmatter no longer requires `title` on api-backed reference pages + +**Files:** +- Modify: `src/validators/frontmatter.js` (the schema-error loop in `validate`) +- Test: `test/frontmatter.test.js` + +**Interfaces:** +- Consumes: `validate({ content, filePath, relativePath, fix }) → result | result[] | null` (signature unchanged). + +- [ ] **Step 1: Write the failing test** + +Create `test/frontmatter.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { validate } from '../src/validators/frontmatter.js'; + +function messages(result) { + if (!result) return []; + return (Array.isArray(result) ? result : [result]).map((r) => r.message); +} + +test('reference page with api but no title is not flagged for missing title', () => { + const content = '---\napi:\n file: pets.json\n operationId: listPets\n---\n'; + const result = validate({ + content, + relativePath: 'reference/Pets/Other/listPets.md', + filePath: '/tmp/listPets.md', + }); + assert.ok( + !messages(result).some((m) => m.includes("must have required property 'title'")), + 'missing-title error should be suppressed for api-backed reference pages', + ); +}); + +test('reference page WITHOUT api still requires title', () => { + const content = '---\nexcerpt: just an excerpt\n---\n'; + const result = validate({ + content, + relativePath: 'reference/loose.md', + filePath: '/tmp/loose.md', + }); + assert.ok( + messages(result).some((m) => m.includes("must have required property 'title'")), + 'non-api reference pages should still require a title', + ); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/frontmatter.test.js` +Expected: FAIL — first test fails because the missing-title error is currently emitted for the api-backed page. + +- [ ] **Step 3: Implement the suppression** + +In `src/validators/frontmatter.js`, inside `validate`, add a guard at the top of the `for (const err of validateFn.errors)` loop (before the existing `if (err.keyword === 'not' ...)` block): + +```js + for (const err of validateFn.errors) { + // Reference pages are OAS-backed: title/excerpt come from the spec, so a + // missing title is not an error for pages that declare an api.file. + if ( + dir === 'reference' && + data?.api?.file && + err.keyword === 'required' && + err.params?.missingProperty === 'title' + ) { + continue; + } + + if (err.keyword === 'not' && err.schema?.properties) { +``` + +(The rest of the loop body is unchanged.) + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `node --test test/frontmatter.test.js` +Expected: PASS — both tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/validators/frontmatter.js test/frontmatter.test.js +git commit -m "fix(lint): don't require title on api-backed reference pages" +``` + +--- + +### Task 6: Item 1c — drop title/excerpt out-of-sync checks + +**Files:** +- Modify: `src/validators/oas-reference.js` (remove the sync checks) +- Test: `test/oas-reference.test.js` + +**Interfaces:** +- Consumes: `validateAll(files: string[], gitRoot: string, { fix }) → results[]` (signature unchanged). + +- [ ] **Step 1: Write the failing test** + +Create `test/oas-reference.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/oas-reference.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +const SPEC = JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { + get: { operationId: 'listPets', summary: 'List pets', description: 'Returns pets' }, + }, + }, +}); + +test('mismatched title/excerpt no longer reported as out of sync', () => { + const root = makeRepo({ + 'reference/pets.json': SPEC, + 'reference/Pets/Other/listPets.md': + '---\ntitle: Totally different\nexcerpt: nope\napi:\n file: pets.json\n operationId: listPets\n---\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(!res.some((r) => r.message.includes('Out of sync')), 'no out-of-sync results'); + } finally { + rmRepo(root); + } +}); + +test('operation not found is still reported', () => { + const root = makeRepo({ + 'reference/pets.json': SPEC, + 'reference/Pets/Other/ghost.md': + '---\napi:\n file: pets.json\n operationId: ghostOp\n---\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(res.some((r) => r.message.includes('Operation not found'))); + } finally { + rmRepo(root); + } +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/oas-reference.test.js` +Expected: FAIL — first test fails because the title/excerpt out-of-sync warnings are still emitted. + +- [ ] **Step 3: Remove the sync checks** + +In `src/validators/oas-reference.js`, replace the per-page loop body so that after the operation-existence check it does nothing further. The full loop becomes: + +```js + for (const relPath of refPages) { + const filePath = path.join(gitRoot, relPath); + let data; + try { + ({ data } = matter(fs.readFileSync(filePath, 'utf-8'))); + } catch { + continue; + } + + if (!data.api || !data.api.file) continue; + + const oasFilename = data.api.file; + const operationId = data.api.operationId; + const oas = oasMap.get(oasFilename); + + // Check: OAS file doesn't exist. + if (!oas) { + results.push({ + file: relPath, + rule: name, + message: `OAS file not found: "${oasFilename}" does not exist in reference/`, + fixable: false, + }); + continue; + } + + if (!operationId) continue; + + // Check: operationId doesn't exist in the spec. + if (!oas.ops.has(operationId)) { + results.push({ + file: relPath, + rule: name, + message: `Operation not found: "${operationId}" does not exist in "${oasFilename}"`, + fixable: true, + }); + continue; + } + + // Title/excerpt are owned by the OAS spec at render time — no sync check here. + } +``` + +(The `extractOperations`/`collectExistingPages` imports, the missing-page loop below, and the `fix` block are unchanged.) + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `node --test test/oas-reference.test.js` +Expected: PASS — both tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/validators/oas-reference.js test/oas-reference.test.js +git commit -m "fix(lint): drop title/excerpt out-of-sync checks for reference" +``` + +--- + +### Task 7: Item 5 — flag stale `_order.yaml` entries + +**Files:** +- Modify: `src/validators/ordering.js` (full rewrite of `validateAll` plus two helpers) +- Test: `test/ordering.test.js` + +**Interfaces:** +- Consumes: `validateAll(files: string[], gitRoot: string, { fix }) → results[]` (signature unchanged). + +- [ ] **Step 1: Write the failing test** + +Create `test/ordering.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/ordering.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('stale and index entries in _order.yaml are flagged', () => { + const root = makeRepo({ + 'docs/foo.md': '---\ntitle: Foo\n---\n', + 'docs/_order.yaml': '- foo\n- ghost\n- index\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(res.some((r) => r.message.includes('Stale entry: "ghost"')), 'flags ghost'); + assert.ok(res.some((r) => r.message.includes('Invalid entry: "index"')), 'flags index'); + assert.ok(!res.some((r) => r.message.includes('foo')), 'does not flag valid foo'); + } finally { + rmRepo(root); + } +}); + +test('--fix removes stale and index entries', () => { + const root = makeRepo({ + 'docs/foo.md': '---\ntitle: Foo\n---\n', + 'docs/_order.yaml': '- foo\n- ghost\n- index\n', + }); + try { + validateAll(collectFiles(root), root, { fix: true }); + const after = fs.readFileSync(path.join(root, 'docs/_order.yaml'), 'utf-8').trim(); + assert.equal(after, '- foo'); + } finally { + rmRepo(root); + } +}); + +test('stale entries are caught in reference too', () => { + const root = makeRepo({ + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\n---\n', + 'reference/Pets/Other/_order.yaml': '- listPets\n- deleted-op\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(res.some((r) => r.message.includes('Stale entry: "deleted-op"'))); + } finally { + rmRepo(root); + } +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/ordering.test.js` +Expected: FAIL — no "Stale entry"/"Invalid entry" results are produced today. + +- [ ] **Step 3: Rewrite `ordering.js`** + +Replace the entire contents of `src/validators/ordering.js`: + +```js +import fs from 'node:fs'; +import path from 'node:path'; + +export const name = 'ordering'; + +// Content directories where _order.yaml is expected (for the "missing from order" check). +const ORDERED_DIRS = ['docs', 'recipes', 'custom_pages']; + +// Content directories scanned for stale _order.yaml entries. +const STALE_SCAN_DIRS = ['docs', 'reference', 'recipes', 'custom_pages']; + +// Values that YAML interprets as non-strings and need quoting in _order.yaml. +const YAML_UNSAFE = /^(?:\d+\.?\d*|true|false|yes|no|on|off|null|~)$/i; +function yamlSafeSlug(slug) { + return YAML_UNSAFE.test(slug) ? `"${slug}"` : slug; +} + +function slugFromFile(filename) { + return filename.replace(/\.(md|mdx)$/, ''); +} + +function parseOrderYaml(content) { + return content + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.startsWith('- ')) + .map((line) => line.slice(2).trim().replace(/^(['"])(.*)\1$/, '$2')); +} + +// Recursively find every _order.yaml under a top-level content dir. +function findOrderFiles(gitRoot, dir) { + const base = path.join(gitRoot, dir); + if (!fs.existsSync(base)) return []; + const found = []; + const stack = [base]; + while (stack.length) { + const cur = stack.pop(); + for (const entry of fs.readdirSync(cur, { withFileTypes: true })) { + const full = path.join(cur, entry.name); + if (entry.isDirectory()) stack.push(full); + else if (entry.name === '_order.yaml') found.push(full); + } + } + return found; +} + +// True if `/` resolves to a page file or a subdirectory. +function entryExists(dir, entry) { + if (fs.existsSync(path.join(dir, `${entry}.md`))) return true; + if (fs.existsSync(path.join(dir, `${entry}.mdx`))) return true; + const folder = path.join(dir, entry); + return fs.existsSync(folder) && fs.statSync(folder).isDirectory(); +} + +export function validateAll(files, gitRoot, { fix } = {}) { + const results = []; + + // ---- Check 1: files/subdirs missing FROM _order.yaml ---- + const dirContents = new Map(); + for (const relPath of files) { + const topDir = relPath.split('/')[0]; + if (!ORDERED_DIRS.includes(topDir)) continue; + + const dir = path.dirname(relPath); + if (!dirContents.has(dir)) dirContents.set(dir, { files: [], subdirs: new Set() }); + + const filename = path.basename(relPath); + dirContents.get(dir).files.push(filename); + + const parts = relPath.split('/'); + if (parts.length > 2) { + const parentDir = parts.slice(0, -2).join('/'); + const subdir = parts[parts.length - 2]; + if (!dirContents.has(parentDir)) dirContents.set(parentDir, { files: [], subdirs: new Set() }); + dirContents.get(parentDir).subdirs.add(subdir); + } + } + + for (const [dir, { files: dirFiles, subdirs }] of dirContents) { + const orderPath = path.join(gitRoot, dir, '_order.yaml'); + + const expectedSlugs = []; + for (const f of dirFiles) { + if (f === 'index.md' || f === 'index.mdx') continue; + expectedSlugs.push(slugFromFile(f)); + } + for (const sub of subdirs) { + expectedSlugs.push(sub); + } + + if (!fs.existsSync(orderPath)) { + if (expectedSlugs.length === 0) continue; + + results.push({ + file: path.join(dir, '_order.yaml'), + rule: name, + severity: 'warning', + fixable: true, + message: `Missing order: _order.yaml not found (${expectedSlugs.length} ${expectedSlugs.length === 1 ? 'entry needs' : 'entries need'} ordering)`, + _fixAdd: { orderPath, missing: expectedSlugs }, + }); + continue; + } + + const content = fs.readFileSync(orderPath, 'utf-8'); + const ordered = new Set(parseOrderYaml(content)); + const missing = expectedSlugs.filter((slug) => !ordered.has(slug)); + + if (missing.length > 0) { + results.push({ + file: path.join(dir, '_order.yaml'), + rule: name, + severity: 'warning', + fixable: true, + message: `Missing from _order.yaml: ${missing.join(', ')}`, + _fixAdd: { orderPath, missing }, + }); + } + } + + // ---- Check 2: entries present IN _order.yaml but absent on disk ---- + const seenOrderFiles = new Set(); + for (const dir of STALE_SCAN_DIRS) { + for (const orderPath of findOrderFiles(gitRoot, dir)) { + if (seenOrderFiles.has(orderPath)) continue; + seenOrderFiles.add(orderPath); + + const orderDir = path.dirname(orderPath); + const relOrder = path.relative(gitRoot, orderPath); + const entries = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')); + + for (const entry of entries) { + if (entry === 'index' || entry === 'index.md') { + results.push({ + file: relOrder, + rule: name, + severity: 'warning', + fixable: true, + message: `Invalid entry: "${entry}" should not be listed in _order.yaml`, + _fixRemove: { orderPath, entry }, + }); + continue; + } + if (!entryExists(orderDir, entry)) { + results.push({ + file: relOrder, + rule: name, + severity: 'warning', + fixable: true, + message: `Stale entry: "${entry}" in _order.yaml has no matching file or folder`, + _fixRemove: { orderPath, entry }, + }); + } + } + } + } + + // ---- Apply fixes ---- + if (fix) { + // Additions first. + for (const r of results) { + if (!r._fixAdd) continue; + const { orderPath, missing } = r._fixAdd; + const newEntries = missing.map((slug) => `- ${yamlSafeSlug(slug)}`).join('\n'); + + if (fs.existsSync(orderPath)) { + const existing = fs.readFileSync(orderPath, 'utf-8'); + const sep = existing.endsWith('\n') ? '' : '\n'; + fs.writeFileSync(orderPath, `${existing}${sep}${newEntries}\n`); + } else { + fs.mkdirSync(path.dirname(orderPath), { recursive: true }); + fs.writeFileSync(orderPath, `${newEntries}\n`); + } + r.message += ' (fixed)'; + } + + // Removals: group stale/index entries by order file, then rewrite each once. + const removalsByPath = new Map(); + for (const r of results) { + if (!r._fixRemove) continue; + const { orderPath, entry } = r._fixRemove; + if (!removalsByPath.has(orderPath)) removalsByPath.set(orderPath, new Set()); + removalsByPath.get(orderPath).add(entry); + } + for (const [orderPath, toRemove] of removalsByPath) { + if (!fs.existsSync(orderPath)) continue; + const kept = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')).filter((e) => !toRemove.has(e)); + if (kept.length > 0) { + fs.writeFileSync(orderPath, kept.map((s) => `- ${yamlSafeSlug(s)}`).join('\n') + '\n'); + } else { + fs.unlinkSync(orderPath); + } + } + for (const r of results) { + if (r._fixRemove) r.message += ' (fixed)'; + } + } + + // Strip internal fix data before returning. + for (const r of results) { + delete r._fixAdd; + delete r._fixRemove; + } + return results; +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `node --test test/ordering.test.js` +Expected: PASS — all three tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/validators/ordering.js test/ordering.test.js +git commit -m "feat(lint): flag stale and index entries in _order.yaml" +``` + +--- + +### Task 8: Item 2 — single-digit `-N` renames with redirect output + +**Files:** +- Modify: `src/validators/numbering.js` (suffix regex, redirect generation, new option) +- Test: `test/numbering.test.js` + +**Interfaces:** +- Consumes: `validateAll(files: string[], gitRoot: string, { fix, nonInteractive, redirectDir }) → results[] | null`. +- New option `redirectDir` (string, optional): directory for the redirect file. Defaults to `~/Desktop`. Used so tests don't write to the real Desktop. + +- [ ] **Step 1: Write the failing test** + +Create `test/numbering.test.js`: + +```js +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/numbering.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('renames single-digit -N file and writes bidirectional redirects', async () => { + const root = makeRepo({ + 'docs/foo-1.md': '---\ntitle: Foo\n---\n', + 'docs/_order.yaml': '- foo-1\n', + }); + const redirectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'rdme-redir-')); + try { + await validateAll(collectFiles(root), root, { fix: true, nonInteractive: true, redirectDir }); + + assert.ok(fs.existsSync(path.join(root, 'docs/foo.md')), 'renamed to foo.md'); + assert.ok(!fs.existsSync(path.join(root, 'docs/foo-1.md')), 'old file gone'); + assert.equal(fs.readFileSync(path.join(root, 'docs/_order.yaml'), 'utf-8').trim(), '- foo'); + + const redirect = fs.readFileSync( + path.join(redirectDir, `${path.basename(root)}_redirect.txt`), + 'utf-8', + ); + assert.match(redirect, /\/docs\/foo-1 -> \/docs\/foo/); + assert.match(redirect, /\/reference\/foo-1 -> \/reference\/foo/); + } finally { + rmRepo(root); + rmRepo(redirectDir); + } +}); + +test('multi-digit suffix is not treated as unnecessary', async () => { + const root = makeRepo({ 'docs/bar-12.md': '---\ntitle: Bar\n---\n' }); + try { + const res = await validateAll(collectFiles(root), root, {}); + assert.equal(res, null); + } finally { + rmRepo(root); + } +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `node --test test/numbering.test.js` +Expected: FAIL — no redirect file is written today (and `bar-12` would currently be flagged, failing the second test). + +- [ ] **Step 3: Update the suffix regex and `os` import** + +In `src/validators/numbering.js`, add the `os` import and narrow the suffix regex: + +```js +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import readline from 'node:readline' +import * as styles from '../utils/styles.js' + +export const name = 'numbering' + +// ReadMe auto-dedupes slugs with a single-digit suffix (foo, foo-1, foo-2…). +// Only those single-digit suffixes are treated as unnecessary. +const SUFFIX_RE = /-(\d)$/ +``` + +- [ ] **Step 4: Update the signature and the rename/redirect block** + +In `src/validators/numbering.js`, change the `validateAll` signature to accept `redirectDir`: + +```js +export async function validateAll(files, gitRoot, { fix, nonInteractive, redirectDir } = {}) { +``` + +Then replace the interactive-rename block (the `if (fix && renames.length > 0) { ... }` section) with: + +```js + // Interactive rename when --fix is passed. + if (fix && renames.length > 0) { + console.log() + console.log(` The following will be renamed:`) + for (const r of renames) { + console.log(` ${styles.dim(r.label)}`) + } + console.log() + console.log(` ${styles.warn('Note:')} Renaming changes slugs, which could break existing URLs.`) + console.log() + + // assume yes if non-interactive, otherwise prompt for confirmation + const answer = nonInteractive ? 'yes' : await prompt(` Rename ${renames.length} ${renames.length === 1 ? 'path' : 'paths'}? (y/N) `) + + if (answer === 'y' || answer === 'yes') { + // Sort longest path first so nested dirs get renamed before parents. + renames.sort((a, b) => b.from.length - a.from.length) + + const redirectLines = [] + for (const r of renames) { + fs.renameSync(r.from, r.to) + updateOrderYaml(r.from, r.to) + + // Emit bidirectional redirects (docs + reference) for each renamed slug, + // matching the bidi_remove_-1.js behavior from CX-3425. + const oldSlug = path.basename(r.from).replace(/\.(md|mdx)$/, '') + const newSlug = path.basename(r.to).replace(/\.(md|mdx)$/, '') + redirectLines.push(`/docs/${oldSlug} -> /docs/${newSlug}`) + redirectLines.push(`/reference/${oldSlug} -> /reference/${newSlug}`) + } + for (const r of results) { + r.message += ' (fixed)' + } + + if (redirectLines.length > 0) { + const outDir = redirectDir || path.join(os.homedir(), 'Desktop') + const redirectFile = path.join(outDir, `${path.basename(gitRoot)}_redirect.txt`) + fs.mkdirSync(outDir, { recursive: true }) + fs.writeFileSync(redirectFile, redirectLines.join('\n') + '\n') + console.log(` ${styles.success('✔')} Redirects written to ${styles.dim(redirectFile)}`) + } + } + } +``` + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `node --test test/numbering.test.js` +Expected: PASS — both tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add src/validators/numbering.js test/numbering.test.js +git commit -m "feat(lint): single-digit -N renames emit bidirectional redirects" +``` + +--- + +### Task 9: Full suite, version bump, and end-to-end verification + +**Files:** +- Modify: `package.json` (version) + +- [ ] **Step 1: Run the whole test suite** + +Run: `npm test` +Expected: PASS — all test files pass, exit 0. + +- [ ] **Step 2: Verify against the example repo** + +Run (in a scratch directory): + +```bash +git clone https://github.com/readme-internal-sync/production-lightcast-api-7a9ba2da223798f9c29a.git /tmp/cx3425-verify || echo "clone failed — verify manually with any reference-heavy repo" +cd /tmp/cx3425-verify 2>/dev/null && git checkout v1.0 2>/dev/null; node /Users/jyowell/cli/bin/readme.js lint +``` + +Expected: the previously-reported errors are gone — no `Invalid frontmatter: must have required property 'title'` and no `Out of sync: title…/excerpt…` for `reference/` Markdown; duplicate slugs shared across docs/reference are not flagged; ``-style components appear as warnings, not errors; stale `_order.yaml` entries appear as warnings. + +> If the repo is inaccessible (private/internal), verify against any reference-heavy ReadMe repo and note the limitation in the PR description. + +- [ ] **Step 3: Bump the version** + +In `package.json`, bump the version so a release can ship: + +```json + "version": "0.0.29", +``` + +- [ ] **Step 4: Commit** + +```bash +git add package.json +git commit -m "chore: bump version to 0.0.29 for CX-3425 fixes" +``` + +--- + +## Self-Review + +**Spec coverage:** +- Item 1 (reference OAS-owned): Task 4 (oas-sync stops writing/updating), Task 5 (frontmatter title not required), Task 6 (oas-reference sync checks removed). ✓ +- Item 2 (`-1` renames + redirects): Task 8. ✓ +- Item 3 (cross-section duplicates): Task 2. ✓ +- Item 4 (unknown components → warning): Task 3. ✓ +- Item 5 (stale `_order.yaml`, including index entries): Task 7. ✓ +- Tests (`node:test`, `test/`): Task 1 + per-task tests. ✓ +- E2E verification + version bump: Task 9. ✓ + +**Type/name consistency:** Result objects use `{ file, rule, message, severity?, fixable? }` throughout. `redirectDir` option added in Task 8 only. Internal `_fixAdd`/`_fixRemove` names are confined to Task 7's `ordering.js`. `makeRepo`/`rmRepo`/`collectFiles` used consistently across test files. + +**Known limitations (documented, out of scope):** conflict-case redirects (base slug already exists) are not generated; `_order.yaml` removal rewrites re-stringify the file (normalizes quoting of untouched entries). From ccfb41eb3da70f3554fc65f3d8eea7faf8a20ba3 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:26:23 -0700 Subject: [PATCH 04/15] test: add node:test harness and temp-repo helper --- package.json | 2 +- test/helpers.js | 21 +++++++++++++++++++++ test/smoke.test.js | 12 ++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/helpers.js create mode 100644 test/smoke.test.js diff --git a/package.json b/package.json index 8501e9d..a8bc044 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ ], "scripts": { "start": "node bin/readme.js", - "test": "echo \"No tests yet\"" + "test": "node --test" }, "engines": { "node": ">=18" diff --git a/test/helpers.js b/test/helpers.js new file mode 100644 index 0000000..a139d60 --- /dev/null +++ b/test/helpers.js @@ -0,0 +1,21 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +/** + * Write a set of files into a fresh temp directory and return its absolute root. + * @param {Record} files Map of repo-relative path -> file content. + */ +export function makeRepo(files) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'rdme-cli-test-')); + for (const [rel, content] of Object.entries(files)) { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + } + return root; +} + +export function rmRepo(root) { + fs.rmSync(root, { recursive: true, force: true }); +} diff --git a/test/smoke.test.js b/test/smoke.test.js new file mode 100644 index 0000000..d902c56 --- /dev/null +++ b/test/smoke.test.js @@ -0,0 +1,12 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('makeRepo writes files and rmRepo cleans up', () => { + const root = makeRepo({ 'docs/a.md': 'hello' }); + assert.equal(fs.readFileSync(path.join(root, 'docs/a.md'), 'utf-8'), 'hello'); + rmRepo(root); + assert.equal(fs.existsSync(root), false); +}); From f38814792aee8a17f76a95fc7705ba5246ac197d Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:29:49 -0700 Subject: [PATCH 05/15] fix(lint): scope duplicate slug check per top-level section --- src/validators/duplicates.js | 10 ++++++---- test/duplicates.test.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 test/duplicates.test.js diff --git a/src/validators/duplicates.js b/src/validators/duplicates.js index 78a59d1..926d5da 100644 --- a/src/validators/duplicates.js +++ b/src/validators/duplicates.js @@ -8,7 +8,7 @@ const CHECKED_DIRS = ['docs', 'reference']; export function validateAll(files) { const results = []; - // Group files by slug. + // Group files by ":" so slugs only collide within the same section. const slugMap = new Map(); for (const relPath of files) { const topDir = relPath.split('/')[0]; @@ -18,17 +18,19 @@ export function validateAll(files) { if (filename === 'index.md' || filename === 'index.mdx') continue; const slug = filename.replace(/\.(md|mdx)$/, ''); - if (!slugMap.has(slug)) slugMap.set(slug, []); - slugMap.get(slug).push(relPath); + const key = `${topDir}:${slug}`; + if (!slugMap.has(key)) slugMap.set(key, []); + slugMap.get(key).push(relPath); } - for (const [slug, paths] of slugMap) { + for (const [key, paths] of slugMap) { if (paths.length < 2) continue; // TODO: figure out why ReadMeConfig causes duplicate slugs and handle properly. // For now, skip any duplicate set that involves a ReadMeConfig path. if (paths.some((p) => p.includes('ReadMeConfig/'))) continue; + const slug = key.slice(key.indexOf(':') + 1); const others = paths.slice(1); for (const relPath of others) { const otherLocations = paths.filter((p) => p !== relPath).map((p) => path.dirname(p)).join(', '); diff --git a/test/duplicates.test.js b/test/duplicates.test.js new file mode 100644 index 0000000..436ae17 --- /dev/null +++ b/test/duplicates.test.js @@ -0,0 +1,15 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { validateAll } from '../src/validators/duplicates.js'; + +test('same slug across docs and reference is allowed', () => { + const res = validateAll(['docs/intro.md', 'reference/intro.md']); + assert.equal(res, null); +}); + +test('same slug within docs (different subdirs) is flagged', () => { + const res = validateAll(['docs/a/intro.md', 'docs/b/intro.md']); + assert.ok(Array.isArray(res) && res.length >= 1); + assert.match(res[0].message, /Duplicate slug: "intro"/); + assert.equal(res[0].severity, 'error'); +}); From fff265896a5f7159d22009f19d309781c19836ef Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:32:26 -0700 Subject: [PATCH 06/15] fix(lint): downgrade unknown component to warning for global blocks --- src/validators/components.js | 1 + test/components.test.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 test/components.test.js diff --git a/src/validators/components.js b/src/validators/components.js index 076eefe..0b3c025 100644 --- a/src/validators/components.js +++ b/src/validators/components.js @@ -220,6 +220,7 @@ export function validateAll(files, gitRoot) { results.push({ file: relPath, rule: name, + severity: 'warning', message: `Unknown component: <${comp}> is not a built-in or custom block`, }); } diff --git a/test/components.test.js b/test/components.test.js new file mode 100644 index 0000000..4423d4c --- /dev/null +++ b/test/components.test.js @@ -0,0 +1,19 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/components.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('unknown component is reported as a warning, not an error', () => { + const root = makeRepo({ + 'docs/page.md': '---\ntitle: Page\n---\n\n\n', + }); + try { + const res = validateAll(collectFiles(root), root); + const unknown = res.find((r) => r.message.includes('Unknown component')); + assert.ok(unknown, 'expected an unknown-component result'); + assert.equal(unknown.severity, 'warning'); + } finally { + rmRepo(root); + } +}); From a180dbe9408a3d960944e533dd72cf46983deac8 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:37:50 -0700 Subject: [PATCH 07/15] fix(oas): stop writing/updating title & excerpt on reference pages --- src/commands/oas-sync.js | 76 +++++++--------------------------------- test/oas-sync.test.js | 52 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 63 deletions(-) create mode 100644 test/oas-sync.test.js diff --git a/src/commands/oas-sync.js b/src/commands/oas-sync.js index 02d3531..398cd71 100644 --- a/src/commands/oas-sync.js +++ b/src/commands/oas-sync.js @@ -14,12 +14,6 @@ export const description = 'Sync reference pages with OpenAPI specs'; const HTTP_METHODS = new Set(['get', 'post', 'put', 'patch', 'delete', 'options', 'head', 'trace']); -/** - * Check if this is a ReadMeConfig spec (internal ReadMe pages — skip title/excerpt updates). - */ -function isReadMeConfig(spec) { - return spec.info?.title === 'ReadMeConfig'; -} /** * Find OAS files at the root of reference/ (JSON or YAML). @@ -168,19 +162,14 @@ function removeFromOrder(orderPath, slug) { } } -function buildPageContent({ oasFilename, operationId, summary, description }) { +function buildPageContent({ oasFilename, operationId }) { const frontmatter = { - title: summary || operationId, api: { file: oasFilename, operationId, }, }; - if (description) { - frontmatter.excerpt = description; - } - return matter.stringify('', frontmatter); } @@ -190,7 +179,6 @@ function buildPageContent({ oasFilename, operationId, summary, description }) { function syncOneOas(refDir, oasFilename, spec) { const specOps = extractOperations(spec); const infoTitle = spec.info?.title || path.basename(oasFilename, path.extname(oasFilename)); - const skipUpdates = isReadMeConfig(spec); const existingPages = collectExistingPages(refDir).filter( (p) => p.data.api.file === oasFilename, @@ -216,61 +204,23 @@ function syncOneOas(refDir, oasFilename, spec) { } } - // Adds + Updates. + // Adds: operations with no page yet. Title/excerpt are owned by the OAS spec + // at render time, so generated pages carry only the api reference. for (const [opId, op] of specOps) { - const existing = pagesByOpId.get(opId); - const tag = op.tag || 'Other'; - - if (!existing) { - const pageDir = path.join(refDir, infoTitle, tag); - fs.mkdirSync(pageDir, { recursive: true }); - - const pagePath = path.join(pageDir, `${opId}.md`); - const content = buildPageContent({ - oasFilename, - operationId: opId, - summary: op.summary, - description: op.description, - }); - fs.writeFileSync(pagePath, content); + if (pagesByOpId.has(opId)) continue; - addToOrder(path.join(pageDir, '_order.yaml'), opId); - addToOrder(path.join(refDir, infoTitle, '_order.yaml'), tag); - - changes.added.push(path.relative(refDir, pagePath)); - } else if (!skipUpdates) { - const expectedTitle = op.summary || opId; - const expectedExcerpt = op.description || null; - const currentTitle = existing.data.title; - const currentExcerpt = existing.data.excerpt || null; + const tag = op.tag || 'Other'; + const pageDir = path.join(refDir, infoTitle, tag); + fs.mkdirSync(pageDir, { recursive: true }); - const titleChanged = currentTitle !== expectedTitle; - const excerptChanged = currentExcerpt !== expectedExcerpt; + const pagePath = path.join(pageDir, `${opId}.md`); + const content = buildPageContent({ oasFilename, operationId: opId }); + fs.writeFileSync(pagePath, content); - if (titleChanged || excerptChanged) { - const updated = { ...existing.data }; - const updateDetails = []; + addToOrder(path.join(pageDir, '_order.yaml'), opId); + addToOrder(path.join(refDir, infoTitle, '_order.yaml'), tag); - if (titleChanged) { - updated.title = expectedTitle; - updateDetails.push('title'); - } - if (excerptChanged) { - if (expectedExcerpt) { - updated.excerpt = expectedExcerpt; - } else { - delete updated.excerpt; - } - updateDetails.push('excerpt'); - } - - const body = matter(existing.content).content; - const newContent = matter.stringify(body, updated); - fs.writeFileSync(existing.filePath, newContent); - - changes.updated.push(`${existing.relativePath} (${updateDetails.join(', ')})`); - } - } + changes.added.push(path.relative(refDir, pagePath)); } return changes; diff --git a/test/oas-sync.test.js b/test/oas-sync.test.js new file mode 100644 index 0000000..1d91fdc --- /dev/null +++ b/test/oas-sync.test.js @@ -0,0 +1,52 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import matter from 'gray-matter'; +import { syncOas } from '../src/commands/oas-sync.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +const SPEC = JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { + get: { operationId: 'listPets', summary: 'List pets', description: 'Returns pets' }, + }, + }, +}); + +test('generated reference page has only api frontmatter (no title/excerpt)', () => { + const root = makeRepo({ 'reference/pets.json': SPEC }); + try { + syncOas(root); + const page = path.join(root, 'reference/Pets/Other/listPets.md'); + assert.ok(fs.existsSync(page), 'expected generated page'); + const { data } = matter(fs.readFileSync(page, 'utf-8')); + assert.equal(data.api.file, 'pets.json'); + assert.equal(data.api.operationId, 'listPets'); + assert.equal('title' in data, false); + assert.equal('excerpt' in data, false); + } finally { + rmRepo(root); + } +}); + +test('existing reference page title is not overwritten by sync', () => { + const root = makeRepo({ + 'reference/pets.json': SPEC, + 'reference/Pets/Other/listPets.md': + '---\ntitle: My custom title\napi:\n file: pets.json\n operationId: listPets\n---\n', + 'reference/Pets/Other/_order.yaml': '- listPets\n', + 'reference/Pets/_order.yaml': '- Other\n', + }); + try { + syncOas(root); + const { data } = matter( + fs.readFileSync(path.join(root, 'reference/Pets/Other/listPets.md'), 'utf-8'), + ); + assert.equal(data.title, 'My custom title'); + } finally { + rmRepo(root); + } +}); From d7d8c54ce0a044a862179fc4190e1d042f2b9b75 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:43:18 -0700 Subject: [PATCH 08/15] fix(lint): don't require title on api-backed reference pages --- src/validators/frontmatter.js | 11 +++++++++++ test/frontmatter.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 test/frontmatter.test.js diff --git a/src/validators/frontmatter.js b/src/validators/frontmatter.js index f1ad761..2dd5e82 100644 --- a/src/validators/frontmatter.js +++ b/src/validators/frontmatter.js @@ -113,6 +113,17 @@ export function validate({ content, filePath, relativePath, fix }) { const valid = validateFn(data); if (!valid) { for (const err of validateFn.errors) { + // Reference pages are OAS-backed: title/excerpt come from the spec, so a + // missing title is not an error for pages that declare an api.file. + if ( + dir === 'reference' && + data?.api?.file && + err.keyword === 'required' && + err.params?.missingProperty === 'title' + ) { + continue; + } + if (err.keyword === 'not' && err.schema?.properties) { const target = err.instancePath ? getNestedValue(data, err.instancePath) : data; if (!target || !Object.keys(err.schema.properties).some((k) => k in target)) continue; diff --git a/test/frontmatter.test.js b/test/frontmatter.test.js new file mode 100644 index 0000000..3b78533 --- /dev/null +++ b/test/frontmatter.test.js @@ -0,0 +1,34 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { validate } from '../src/validators/frontmatter.js'; + +function messages(result) { + if (!result) return []; + return (Array.isArray(result) ? result : [result]).map((r) => r.message); +} + +test('reference page with api but no title is not flagged for missing title', () => { + const content = '---\napi:\n file: pets.json\n operationId: listPets\n---\n'; + const result = validate({ + content, + relativePath: 'reference/Pets/Other/listPets.md', + filePath: '/tmp/listPets.md', + }); + assert.ok( + !messages(result).some((m) => m.includes("must have required property 'title'")), + 'missing-title error should be suppressed for api-backed reference pages', + ); +}); + +test('reference page WITHOUT api still requires title', () => { + const content = '---\nexcerpt: just an excerpt\n---\n'; + const result = validate({ + content, + relativePath: 'reference/loose.md', + filePath: '/tmp/loose.md', + }); + assert.ok( + messages(result).some((m) => m.includes("must have required property 'title'")), + 'non-api reference pages should still require a title', + ); +}); From 8adbf48c06d8f7b1c7b16a9bd33374140a53999e Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:51:05 -0700 Subject: [PATCH 09/15] fix(lint): drop title/excerpt out-of-sync checks for reference --- src/validators/oas-reference.js | 32 +----------------------- test/oas-reference.test.js | 43 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 31 deletions(-) create mode 100644 test/oas-reference.test.js diff --git a/src/validators/oas-reference.js b/src/validators/oas-reference.js index 59e3793..409450a 100644 --- a/src/validators/oas-reference.js +++ b/src/validators/oas-reference.js @@ -60,37 +60,7 @@ export function validateAll(files, gitRoot, { fix } = {}) { continue; } - // Skip title/excerpt sync checks for ReadMeConfig (internal ReadMe pages). - // Check both the spec title and the page's directory path. - const isReadMeConfig = oas.spec.info?.title === 'ReadMeConfig' - || relPath.startsWith('reference/ReadMeConfig/'); - if (isReadMeConfig) continue; - - // Check: title or excerpt out of sync. - const op = oas.ops.get(operationId); - const expectedTitle = op.summary || operationId; - const expectedExcerpt = op.description || null; - - if (data.title !== expectedTitle) { - results.push({ - file: relPath, - rule: name, - severity: 'warning', - message: `Out of sync: title is "${data.title}" but spec summary is "${expectedTitle}"`, - fixable: true, - }); - } - - const currentExcerpt = data.excerpt || null; - if (currentExcerpt !== expectedExcerpt) { - results.push({ - file: relPath, - rule: name, - severity: 'warning', - message: `Out of sync: excerpt does not match spec description for "${operationId}"`, - fixable: true, - }); - } + // Title/excerpt are owned by the OAS spec at render time — no sync check here. } // Check for missing pages: operations in the spec with no corresponding page. diff --git a/test/oas-reference.test.js b/test/oas-reference.test.js new file mode 100644 index 0000000..9ac2991 --- /dev/null +++ b/test/oas-reference.test.js @@ -0,0 +1,43 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/oas-reference.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +const SPEC = JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Pets' }, + paths: { + '/pets': { + get: { operationId: 'listPets', summary: 'List pets', description: 'Returns pets' }, + }, + }, +}); + +test('mismatched title/excerpt no longer reported as out of sync', () => { + const root = makeRepo({ + 'reference/pets.json': SPEC, + 'reference/Pets/Other/listPets.md': + '---\ntitle: Totally different\nexcerpt: nope\napi:\n file: pets.json\n operationId: listPets\n---\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(!res.some((r) => r.message.includes('Out of sync')), 'no out-of-sync results'); + } finally { + rmRepo(root); + } +}); + +test('operation not found is still reported', () => { + const root = makeRepo({ + 'reference/pets.json': SPEC, + 'reference/Pets/Other/ghost.md': + '---\napi:\n file: pets.json\n operationId: ghostOp\n---\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(res.some((r) => r.message.includes('Operation not found'))); + } finally { + rmRepo(root); + } +}); From 678a5ac93cc83b4e3e2643cf255852aed837f0b1 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 16:57:47 -0700 Subject: [PATCH 10/15] feat(lint): flag stale and index entries in _order.yaml --- src/validators/ordering.js | 114 ++++++++++++++++++++++++++++++++----- test/ordering.test.js | 50 ++++++++++++++++ 2 files changed, 149 insertions(+), 15 deletions(-) create mode 100644 test/ordering.test.js diff --git a/src/validators/ordering.js b/src/validators/ordering.js index ff25cf3..821cb94 100644 --- a/src/validators/ordering.js +++ b/src/validators/ordering.js @@ -3,9 +3,12 @@ import path from 'node:path'; export const name = 'ordering'; -// Content directories where _order.yaml is expected. +// Content directories where _order.yaml is expected (for the "missing from order" check). const ORDERED_DIRS = ['docs', 'recipes', 'custom_pages']; +// Content directories scanned for stale _order.yaml entries. +const STALE_SCAN_DIRS = ['docs', 'reference', 'recipes', 'custom_pages']; + // Values that YAML interprets as non-strings and need quoting in _order.yaml. const YAML_UNSAFE = /^(?:\d+\.?\d*|true|false|yes|no|on|off|null|~)$/i; function yamlSafeSlug(slug) { @@ -24,13 +27,35 @@ function parseOrderYaml(content) { .map((line) => line.slice(2).trim().replace(/^(['"])(.*)\1$/, '$2')); } -/** - * Find all directories that contain content and check their _order.yaml. - */ +// Recursively find every _order.yaml under a top-level content dir. +function findOrderFiles(gitRoot, dir) { + const base = path.join(gitRoot, dir); + if (!fs.existsSync(base)) return []; + const found = []; + const stack = [base]; + while (stack.length) { + const cur = stack.pop(); + for (const entry of fs.readdirSync(cur, { withFileTypes: true })) { + const full = path.join(cur, entry.name); + if (entry.isDirectory()) stack.push(full); + else if (entry.name === '_order.yaml') found.push(full); + } + } + return found; +} + +// True if `/` resolves to a page file or a subdirectory. +function entryExists(dir, entry) { + if (fs.existsSync(path.join(dir, `${entry}.md`))) return true; + if (fs.existsSync(path.join(dir, `${entry}.mdx`))) return true; + const folder = path.join(dir, entry); + return fs.existsSync(folder) && fs.statSync(folder).isDirectory(); +} + export function validateAll(files, gitRoot, { fix } = {}) { const results = []; - // Group files by their immediate parent directory. + // ---- Check 1: files/subdirs missing FROM _order.yaml ---- const dirContents = new Map(); for (const relPath of files) { const topDir = relPath.split('/')[0]; @@ -42,7 +67,6 @@ export function validateAll(files, gitRoot, { fix } = {}) { const filename = path.basename(relPath); dirContents.get(dir).files.push(filename); - // Track subdirectories in the parent so we know about categories/nested dirs. const parts = relPath.split('/'); if (parts.length > 2) { const parentDir = parts.slice(0, -2).join('/'); @@ -55,7 +79,6 @@ export function validateAll(files, gitRoot, { fix } = {}) { for (const [dir, { files: dirFiles, subdirs }] of dirContents) { const orderPath = path.join(gitRoot, dir, '_order.yaml'); - // Collect expected slugs: files (excluding index.md) + subdirectories. const expectedSlugs = []; for (const f of dirFiles) { if (f === 'index.md' || f === 'index.mdx') continue; @@ -74,7 +97,7 @@ export function validateAll(files, gitRoot, { fix } = {}) { severity: 'warning', fixable: true, message: `Missing order: _order.yaml not found (${expectedSlugs.length} ${expectedSlugs.length === 1 ? 'entry needs' : 'entries need'} ordering)`, - _fix: { orderPath, missing: expectedSlugs }, + _fixAdd: { orderPath, missing: expectedSlugs }, }); continue; } @@ -90,16 +113,54 @@ export function validateAll(files, gitRoot, { fix } = {}) { severity: 'warning', fixable: true, message: `Missing from _order.yaml: ${missing.join(', ')}`, - _fix: { orderPath, missing }, + _fixAdd: { orderPath, missing }, }); } } - // Apply fixes if requested. + // ---- Check 2: entries present IN _order.yaml but absent on disk ---- + const seenOrderFiles = new Set(); + for (const dir of STALE_SCAN_DIRS) { + for (const orderPath of findOrderFiles(gitRoot, dir)) { + if (seenOrderFiles.has(orderPath)) continue; + seenOrderFiles.add(orderPath); + + const orderDir = path.dirname(orderPath); + const relOrder = path.relative(gitRoot, orderPath); + const entries = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')); + + for (const entry of entries) { + if (entry === 'index' || entry === 'index.md') { + results.push({ + file: relOrder, + rule: name, + severity: 'warning', + fixable: true, + message: `Invalid entry: "${entry}" should not be listed in _order.yaml`, + _fixRemove: { orderPath, entry }, + }); + continue; + } + if (!entryExists(orderDir, entry)) { + results.push({ + file: relOrder, + rule: name, + severity: 'warning', + fixable: true, + message: `Stale entry: "${entry}" in _order.yaml has no matching file or folder`, + _fixRemove: { orderPath, entry }, + }); + } + } + } + } + + // ---- Apply fixes ---- if (fix) { + // Additions first. for (const r of results) { - if (!r._fix) continue; - const { orderPath, missing } = r._fix; + if (!r._fixAdd) continue; + const { orderPath, missing } = r._fixAdd; const newEntries = missing.map((slug) => `- ${yamlSafeSlug(slug)}`).join('\n'); if (fs.existsSync(orderPath)) { @@ -110,12 +171,35 @@ export function validateAll(files, gitRoot, { fix } = {}) { fs.mkdirSync(path.dirname(orderPath), { recursive: true }); fs.writeFileSync(orderPath, `${newEntries}\n`); } - r.message += ' (fixed)'; } + + // Removals: group stale/index entries by order file, then rewrite each once. + const removalsByPath = new Map(); + for (const r of results) { + if (!r._fixRemove) continue; + const { orderPath, entry } = r._fixRemove; + if (!removalsByPath.has(orderPath)) removalsByPath.set(orderPath, new Set()); + removalsByPath.get(orderPath).add(entry); + } + for (const [orderPath, toRemove] of removalsByPath) { + if (!fs.existsSync(orderPath)) continue; + const kept = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')).filter((e) => !toRemove.has(e)); + if (kept.length > 0) { + fs.writeFileSync(orderPath, kept.map((s) => `- ${yamlSafeSlug(s)}`).join('\n') + '\n'); + } else { + fs.unlinkSync(orderPath); + } + } + for (const r of results) { + if (r._fixRemove) r.message += ' (fixed)'; + } } - // Strip internal _fix data before returning. - for (const r of results) delete r._fix; + // Strip internal fix data before returning. + for (const r of results) { + delete r._fixAdd; + delete r._fixRemove; + } return results; } diff --git a/test/ordering.test.js b/test/ordering.test.js new file mode 100644 index 0000000..daf2182 --- /dev/null +++ b/test/ordering.test.js @@ -0,0 +1,50 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/ordering.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('stale and index entries in _order.yaml are flagged', () => { + const root = makeRepo({ + 'docs/foo.md': '---\ntitle: Foo\n---\n', + 'docs/_order.yaml': '- foo\n- ghost\n- index\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(res.some((r) => r.message.includes('Stale entry: "ghost"')), 'flags ghost'); + assert.ok(res.some((r) => r.message.includes('Invalid entry: "index"')), 'flags index'); + assert.ok(!res.some((r) => r.message.includes('foo')), 'does not flag valid foo'); + } finally { + rmRepo(root); + } +}); + +test('--fix removes stale and index entries', () => { + const root = makeRepo({ + 'docs/foo.md': '---\ntitle: Foo\n---\n', + 'docs/_order.yaml': '- foo\n- ghost\n- index\n', + }); + try { + validateAll(collectFiles(root), root, { fix: true }); + const after = fs.readFileSync(path.join(root, 'docs/_order.yaml'), 'utf-8').trim(); + assert.equal(after, '- foo'); + } finally { + rmRepo(root); + } +}); + +test('stale entries are caught in reference too', () => { + const root = makeRepo({ + 'reference/Pets/Other/listPets.md': + '---\napi:\n file: pets.json\n operationId: listPets\n---\n', + 'reference/Pets/Other/_order.yaml': '- listPets\n- deleted-op\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok(res.some((r) => r.message.includes('Stale entry: "deleted-op"'))); + } finally { + rmRepo(root); + } +}); From 7910c2b5cec3ebcc510f6c3b35170dd60e154307 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 17:10:09 -0700 Subject: [PATCH 11/15] feat(lint): single-digit -N renames emit bidirectional redirects --- src/validators/numbering.js | 24 +++++++++++++++++++-- test/numbering.test.js | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 test/numbering.test.js diff --git a/src/validators/numbering.js b/src/validators/numbering.js index 2ac9aff..fd3fd37 100644 --- a/src/validators/numbering.js +++ b/src/validators/numbering.js @@ -1,11 +1,14 @@ import fs from 'node:fs' +import os from 'node:os' import path from 'node:path' import readline from 'node:readline' import * as styles from '../utils/styles.js' export const name = 'numbering' -const SUFFIX_RE = /-(\d+)$/ +// ReadMe auto-dedupes slugs with a single-digit suffix (foo, foo-1, foo-2…). +// Only those single-digit suffixes are treated as unnecessary. +const SUFFIX_RE = /-(\d)$/ function prompt(question) { const rl = readline.createInterface({ input: process.stdin, output: process.stdout }) @@ -33,7 +36,7 @@ function updateOrderYaml(fromPath, toPath) { } } -export async function validateAll(files, gitRoot, { fix, nonInteractive } = {}) { +export async function validateAll(files, gitRoot, { fix, nonInteractive, redirectDir } = {}) { const results = [] const renames = [] @@ -120,13 +123,30 @@ export async function validateAll(files, gitRoot, { fix, nonInteractive } = {}) if (answer === 'y' || answer === 'yes') { // Sort longest path first so nested dirs get renamed before parents. renames.sort((a, b) => b.from.length - a.from.length) + + const redirectLines = [] for (const r of renames) { fs.renameSync(r.from, r.to) updateOrderYaml(r.from, r.to) + + // Emit bidirectional redirects (docs + reference) for each renamed slug, + // matching the bidi_remove_-1.js behavior from CX-3425. + const oldSlug = path.basename(r.from).replace(/\.(md|mdx)$/, '') + const newSlug = path.basename(r.to).replace(/\.(md|mdx)$/, '') + redirectLines.push(`/docs/${oldSlug} -> /docs/${newSlug}`) + redirectLines.push(`/reference/${oldSlug} -> /reference/${newSlug}`) } for (const r of results) { r.message += ' (fixed)' } + + if (redirectLines.length > 0) { + const outDir = redirectDir || path.join(os.homedir(), 'Desktop') + const redirectFile = path.join(outDir, `${path.basename(gitRoot)}_redirect.txt`) + fs.mkdirSync(outDir, { recursive: true }) + fs.writeFileSync(redirectFile, redirectLines.join('\n') + '\n') + console.log(` ${styles.success('✔')} Redirects written to ${styles.dim(redirectFile)}`) + } } } diff --git a/test/numbering.test.js b/test/numbering.test.js new file mode 100644 index 0000000..3a0d2a2 --- /dev/null +++ b/test/numbering.test.js @@ -0,0 +1,43 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { collectFiles } from '../src/utils/lint.js'; +import { validateAll } from '../src/validators/numbering.js'; +import { makeRepo, rmRepo } from './helpers.js'; + +test('renames single-digit -N file and writes bidirectional redirects', async () => { + const root = makeRepo({ + 'docs/foo-1.md': '---\ntitle: Foo\n---\n', + 'docs/_order.yaml': '- foo-1\n', + }); + const redirectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'rdme-redir-')); + try { + await validateAll(collectFiles(root), root, { fix: true, nonInteractive: true, redirectDir }); + + assert.ok(fs.existsSync(path.join(root, 'docs/foo.md')), 'renamed to foo.md'); + assert.ok(!fs.existsSync(path.join(root, 'docs/foo-1.md')), 'old file gone'); + assert.equal(fs.readFileSync(path.join(root, 'docs/_order.yaml'), 'utf-8').trim(), '- foo'); + + const redirect = fs.readFileSync( + path.join(redirectDir, `${path.basename(root)}_redirect.txt`), + 'utf-8', + ); + assert.match(redirect, /\/docs\/foo-1 -> \/docs\/foo/); + assert.match(redirect, /\/reference\/foo-1 -> \/reference\/foo/); + } finally { + rmRepo(root); + rmRepo(redirectDir); + } +}); + +test('multi-digit suffix is not treated as unnecessary', async () => { + const root = makeRepo({ 'docs/bar-12.md': '---\ntitle: Bar\n---\n' }); + try { + const res = await validateAll(collectFiles(root), root, {}); + assert.equal(res, null); + } finally { + rmRepo(root); + } +}); From 3b1addfe18f0074ccda23757efdd7da7bcaa64dd Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 17:15:07 -0700 Subject: [PATCH 12/15] chore: bump version to 0.0.29 for CX-3425 fixes --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a8bc044..e3412d7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@readme/cli", - "version": "0.0.28", + "version": "0.0.29", "description": "The ReadMe CLI", "type": "module", "bin": { From ebf722fbe22876e92611c6e18a8aae7ff5ad6ca1 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Wed, 17 Jun 2026 20:27:51 -0700 Subject: [PATCH 13/15] test+fix(lint): close review coverage gaps; harden _order.yaml entry check Co-Authored-By: Claude Sonnet 4.6 --- src/validators/ordering.js | 9 ++++++--- test/frontmatter.test.js | 13 +++++++++++++ test/numbering.test.js | 36 ++++++++++++++++++++++++++++++++++++ test/oas-sync.test.js | 2 ++ test/ordering.test.js | 16 ++++++++++++++++ 5 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/validators/ordering.js b/src/validators/ordering.js index 821cb94..9c25a47 100644 --- a/src/validators/ordering.js +++ b/src/validators/ordering.js @@ -48,8 +48,11 @@ function findOrderFiles(gitRoot, dir) { function entryExists(dir, entry) { if (fs.existsSync(path.join(dir, `${entry}.md`))) return true; if (fs.existsSync(path.join(dir, `${entry}.mdx`))) return true; - const folder = path.join(dir, entry); - return fs.existsSync(folder) && fs.statSync(folder).isDirectory(); + try { + return fs.statSync(path.join(dir, entry)).isDirectory(); + } catch { + return false; + } } export function validateAll(files, gitRoot, { fix } = {}) { @@ -130,7 +133,7 @@ export function validateAll(files, gitRoot, { fix } = {}) { const entries = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')); for (const entry of entries) { - if (entry === 'index' || entry === 'index.md') { + if (entry === 'index' || entry === 'index.md' || entry === 'index.mdx') { results.push({ file: relOrder, rule: name, diff --git a/test/frontmatter.test.js b/test/frontmatter.test.js index 3b78533..1f492e6 100644 --- a/test/frontmatter.test.js +++ b/test/frontmatter.test.js @@ -32,3 +32,16 @@ test('reference page WITHOUT api still requires title', () => { 'non-api reference pages should still require a title', ); }); + +test('docs page without title is still flagged', () => { + const content = '---\nexcerpt: just an excerpt\n---\n'; + const result = validate({ + content, + relativePath: 'docs/some-page.md', + filePath: '/tmp/some-page.md', + }); + assert.ok( + messages(result).some((m) => m.includes("must have required property 'title'")), + 'docs pages should still require a title', + ); +}); diff --git a/test/numbering.test.js b/test/numbering.test.js index 3a0d2a2..364d402 100644 --- a/test/numbering.test.js +++ b/test/numbering.test.js @@ -41,3 +41,39 @@ test('multi-digit suffix is not treated as unnecessary', async () => { rmRepo(root); } }); + +test('directory single-digit suffix renames with flat redirects (matches script)', async () => { + const root = makeRepo({ 'docs/guides-1/intro.md': '---\ntitle: Intro\n---\n' }); + const redirectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'rdme-redir-')); + try { + await validateAll(collectFiles(root), root, { fix: true, nonInteractive: true, redirectDir }); + assert.ok(fs.existsSync(path.join(root, 'docs/guides/intro.md')), 'folder renamed'); + assert.ok(!fs.existsSync(path.join(root, 'docs/guides-1')), 'old folder gone'); + const redirect = fs.readFileSync( + path.join(redirectDir, `${path.basename(root)}_redirect.txt`), + 'utf-8', + ); + assert.match(redirect, /\/docs\/guides-1 -> \/docs\/guides/); + assert.match(redirect, /\/reference\/guides-1 -> \/reference\/guides/); + } finally { + rmRepo(root); + rmRepo(redirectDir); + } +}); + +test('plain lint run (no fix) writes no redirect file and does not rename', async () => { + const root = makeRepo({ 'docs/foo-1.md': '---\ntitle: Foo\n---\n' }); + const redirectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'rdme-redir-')); + try { + const res = await validateAll(collectFiles(root), root, { redirectDir }); + assert.ok(Array.isArray(res) && res.some((r) => r.message.includes('Unnecessary suffix'))); + assert.equal( + fs.existsSync(path.join(redirectDir, `${path.basename(root)}_redirect.txt`)), + false, + ); + assert.ok(fs.existsSync(path.join(root, 'docs/foo-1.md')), 'file not renamed without fix'); + } finally { + rmRepo(root); + rmRepo(redirectDir); + } +}); diff --git a/test/oas-sync.test.js b/test/oas-sync.test.js index 1d91fdc..00f8900 100644 --- a/test/oas-sync.test.js +++ b/test/oas-sync.test.js @@ -46,6 +46,8 @@ test('existing reference page title is not overwritten by sync', () => { fs.readFileSync(path.join(root, 'reference/Pets/Other/listPets.md'), 'utf-8'), ); assert.equal(data.title, 'My custom title'); + assert.equal(data.api.file, 'pets.json'); + assert.equal(data.api.operationId, 'listPets'); } finally { rmRepo(root); } diff --git a/test/ordering.test.js b/test/ordering.test.js index daf2182..6f00976 100644 --- a/test/ordering.test.js +++ b/test/ordering.test.js @@ -48,3 +48,19 @@ test('stale entries are caught in reference too', () => { rmRepo(root); } }); + +test('--fix adds a file missing from _order.yaml', () => { + const root = makeRepo({ + 'docs/foo.md': '---\ntitle: Foo\n---\n', + 'docs/bar.md': '---\ntitle: Bar\n---\n', + 'docs/_order.yaml': '- foo\n', + }); + try { + validateAll(collectFiles(root), root, { fix: true }); + const after = fs.readFileSync(path.join(root, 'docs/_order.yaml'), 'utf-8'); + assert.match(after, /- bar/); + assert.match(after, /- foo/); + } finally { + rmRepo(root); + } +}); From 9ea1b6e982aec5533a8b1c508a9e6842d8645aa5 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Thu, 18 Jun 2026 09:07:31 -0700 Subject: [PATCH 14/15] refactor(oas): drop dead changes.updated tracking from sync The reference title/excerpt update path was removed in CX-3425, leaving changes.updated permanently empty. Remove it from the changes object, the syncOas JSDoc, and the oas:sync run() summary. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/oas-sync.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/commands/oas-sync.js b/src/commands/oas-sync.js index 398cd71..561fb99 100644 --- a/src/commands/oas-sync.js +++ b/src/commands/oas-sync.js @@ -189,7 +189,7 @@ function syncOneOas(refDir, oasFilename, spec) { pagesByOpId.set(page.data.api.operationId, page); } - const changes = { added: [], deleted: [], updated: [] }; + const changes = { added: [], deleted: [] }; // Deletes: pages referencing operations that no longer exist. for (const [opId, page] of pagesByOpId) { @@ -235,7 +235,7 @@ function syncOneOas(refDir, oasFilename, spec) { * * @param {string | { cwd?: string }} input Repo root path, or `{ cwd }` object. * @returns {null | Array<{ filename: string, spec: object, opCount: number, - * changes: { added: string[], deleted: string[], updated: string[] } }>} + * changes: { added: string[], deleted: string[] } }>} * Returns null if there's no reference/ dir or no specs. */ export function syncOas(input) { @@ -275,11 +275,10 @@ export async function run(_options, _cmd, ctx) { let totalAdded = 0; let totalDeleted = 0; - let totalUpdated = 0; for (const { filename, spec, opCount, changes } of results) { const title = spec.info?.title || filename; - const hasChanges = changes.added.length + changes.deleted.length + changes.updated.length > 0; + const hasChanges = changes.added.length + changes.deleted.length > 0; const dot = hasChanges ? styles.warn('●') : styles.success('●'); console.log(); @@ -295,20 +294,16 @@ export async function run(_options, _cmd, ctx) { for (const file of changes.deleted) { console.log(` ${styles.err('−')} Deleted ${file}`); } - for (const file of changes.updated) { - console.log(` ${styles.warn('~')} Updated ${file}`); - } totalAdded += changes.added.length; totalDeleted += changes.deleted.length; - totalUpdated += changes.updated.length; } console.log(); - const total = totalAdded + totalDeleted + totalUpdated; + const total = totalAdded + totalDeleted; if (total === 0) { styles.ok('Reference pages are already in sync.'); } else { - styles.ok(`Synced: ${totalAdded} added, ${totalDeleted} deleted, ${totalUpdated} updated.`); + styles.ok(`Synced: ${totalAdded} added, ${totalDeleted} deleted.`); } } From 498939fb5a5e65bd6cad81503f07901dae056ae2 Mon Sep 17 00:00:00 2001 From: Jesse Yowell Date: Thu, 18 Jun 2026 09:08:51 -0700 Subject: [PATCH 15/15] refactor(lint): collapse ordering checks into a single per-directory pass Replace the two-pass design (files-derived 'missing from order' + a second filesystem walk for stale entries) with one walk per content dir that diffs the on-disk slug set against the _order.yaml entries: missing = onDisk - ordered, stale = ordered - onDisk. Removes findOrderFiles, entryExists, and the dirContents/seenOrderFiles bookkeeping. Behavior unchanged; adds a test that a folder entry with no markdown children is not flagged stale. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/validators/ordering.js | 254 +++++++++++++++++-------------------- test/ordering.test.js | 16 +++ 2 files changed, 133 insertions(+), 137 deletions(-) diff --git a/src/validators/ordering.js b/src/validators/ordering.js index 9c25a47..e4afd6a 100644 --- a/src/validators/ordering.js +++ b/src/validators/ordering.js @@ -3,11 +3,13 @@ import path from 'node:path'; export const name = 'ordering'; -// Content directories where _order.yaml is expected (for the "missing from order" check). -const ORDERED_DIRS = ['docs', 'recipes', 'custom_pages']; +// Top-level content directories that use _order.yaml. +const CONTENT_DIRS = ['docs', 'reference', 'recipes', 'custom_pages']; -// Content directories scanned for stale _order.yaml entries. -const STALE_SCAN_DIRS = ['docs', 'reference', 'recipes', 'custom_pages']; +// Of those, the dirs that must list every page/folder. reference ordering is +// OAS-managed, so we never flag pages "missing from" a reference _order.yaml — +// but stale entries are flagged everywhere. +const REQUIRE_ORDER = new Set(['docs', 'recipes', 'custom_pages']); // Values that YAML interprets as non-strings and need quoting in _order.yaml. const YAML_UNSAFE = /^(?:\d+\.?\d*|true|false|yes|no|on|off|null|~)$/i; @@ -15,10 +17,6 @@ function yamlSafeSlug(slug) { return YAML_UNSAFE.test(slug) ? `"${slug}"` : slug; } -function slugFromFile(filename) { - return filename.replace(/\.(md|mdx)$/, ''); -} - function parseOrderYaml(content) { return content .split('\n') @@ -27,113 +25,94 @@ function parseOrderYaml(content) { .map((line) => line.slice(2).trim().replace(/^(['"])(.*)\1$/, '$2')); } -// Recursively find every _order.yaml under a top-level content dir. -function findOrderFiles(gitRoot, dir) { - const base = path.join(gitRoot, dir); +function isIndex(name) { + return name === 'index' || name === 'index.md' || name === 'index.mdx'; +} + +/** + * Walk a content directory tree once. For every directory, capture the on-disk + * slug set (child pages + child folders) and its _order.yaml entries (or null + * when absent), so the caller can diff the two in a single pass. + */ +function collectDirs(gitRoot, topDir) { + const base = path.join(gitRoot, topDir); if (!fs.existsSync(base)) return []; - const found = []; + + const dirs = []; const stack = [base]; while (stack.length) { const cur = stack.pop(); + const onDisk = new Set(); + let orderEntries = null; + for (const entry of fs.readdirSync(cur, { withFileTypes: true })) { - const full = path.join(cur, entry.name); - if (entry.isDirectory()) stack.push(full); - else if (entry.name === '_order.yaml') found.push(full); + if (entry.isDirectory()) { + onDisk.add(entry.name); + stack.push(path.join(cur, entry.name)); + } else if (entry.name === '_order.yaml') { + orderEntries = parseOrderYaml(fs.readFileSync(path.join(cur, entry.name), 'utf-8')); + } else if (/\.(md|mdx)$/.test(entry.name) && !isIndex(entry.name)) { + onDisk.add(entry.name.replace(/\.(md|mdx)$/, '')); + } } - } - return found; -} -// True if `/` resolves to a page file or a subdirectory. -function entryExists(dir, entry) { - if (fs.existsSync(path.join(dir, `${entry}.md`))) return true; - if (fs.existsSync(path.join(dir, `${entry}.mdx`))) return true; - try { - return fs.statSync(path.join(dir, entry)).isDirectory(); - } catch { - return false; + dirs.push({ dir: cur, onDisk, orderEntries }); } + return dirs; } +/** + * Validate every _order.yaml against the files and folders next to it: + * - on disk but missing from _order.yaml → "Missing from _order.yaml" (required dirs only) + * - in _order.yaml but missing on disk → "Stale entry" + * - an index entry that shouldn't be listed → "Invalid entry" + */ export function validateAll(files, gitRoot, { fix } = {}) { const results = []; - // ---- Check 1: files/subdirs missing FROM _order.yaml ---- - const dirContents = new Map(); - for (const relPath of files) { - const topDir = relPath.split('/')[0]; - if (!ORDERED_DIRS.includes(topDir)) continue; - - const dir = path.dirname(relPath); - if (!dirContents.has(dir)) dirContents.set(dir, { files: [], subdirs: new Set() }); - - const filename = path.basename(relPath); - dirContents.get(dir).files.push(filename); - - const parts = relPath.split('/'); - if (parts.length > 2) { - const parentDir = parts.slice(0, -2).join('/'); - const subdir = parts[parts.length - 2]; - if (!dirContents.has(parentDir)) dirContents.set(parentDir, { files: [], subdirs: new Set() }); - dirContents.get(parentDir).subdirs.add(subdir); - } - } + for (const topDir of CONTENT_DIRS) { + const requireOrder = REQUIRE_ORDER.has(topDir); - for (const [dir, { files: dirFiles, subdirs }] of dirContents) { - const orderPath = path.join(gitRoot, dir, '_order.yaml'); - - const expectedSlugs = []; - for (const f of dirFiles) { - if (f === 'index.md' || f === 'index.mdx') continue; - expectedSlugs.push(slugFromFile(f)); - } - for (const sub of subdirs) { - expectedSlugs.push(sub); - } - - if (!fs.existsSync(orderPath)) { - if (expectedSlugs.length === 0) continue; - - results.push({ - file: path.join(dir, '_order.yaml'), - rule: name, - severity: 'warning', - fixable: true, - message: `Missing order: _order.yaml not found (${expectedSlugs.length} ${expectedSlugs.length === 1 ? 'entry needs' : 'entries need'} ordering)`, - _fixAdd: { orderPath, missing: expectedSlugs }, - }); - continue; - } + for (const { dir, onDisk, orderEntries } of collectDirs(gitRoot, topDir)) { + const orderPath = path.join(dir, '_order.yaml'); + const relOrder = path.relative(gitRoot, orderPath); - const content = fs.readFileSync(orderPath, 'utf-8'); - const ordered = new Set(parseOrderYaml(content)); - const missing = expectedSlugs.filter((slug) => !ordered.has(slug)); - - if (missing.length > 0) { - results.push({ - file: path.join(dir, '_order.yaml'), - rule: name, - severity: 'warning', - fixable: true, - message: `Missing from _order.yaml: ${missing.join(', ')}`, - _fixAdd: { orderPath, missing }, - }); - } - } + // No _order.yaml at all: only required dirs need one. + if (orderEntries === null) { + if (requireOrder && onDisk.size > 0) { + const missing = [...onDisk]; + results.push({ + file: relOrder, + rule: name, + severity: 'warning', + fixable: true, + message: `Missing order: _order.yaml not found (${missing.length} ${missing.length === 1 ? 'entry needs' : 'entries need'} ordering)`, + _fixAdd: { orderPath, missing }, + }); + } + continue; + } - // ---- Check 2: entries present IN _order.yaml but absent on disk ---- - const seenOrderFiles = new Set(); - for (const dir of STALE_SCAN_DIRS) { - for (const orderPath of findOrderFiles(gitRoot, dir)) { - if (seenOrderFiles.has(orderPath)) continue; - seenOrderFiles.add(orderPath); + const ordered = new Set(orderEntries); - const orderDir = path.dirname(orderPath); - const relOrder = path.relative(gitRoot, orderPath); - const entries = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')); + // On disk but not ordered. + if (requireOrder) { + const missing = [...onDisk].filter((slug) => !ordered.has(slug)); + if (missing.length > 0) { + results.push({ + file: relOrder, + rule: name, + severity: 'warning', + fixable: true, + message: `Missing from _order.yaml: ${missing.join(', ')}`, + _fixAdd: { orderPath, missing }, + }); + } + } - for (const entry of entries) { - if (entry === 'index' || entry === 'index.md' || entry === 'index.mdx') { + // Ordered but not on disk (or an index entry that shouldn't be listed). + for (const entry of orderEntries) { + if (isIndex(entry)) { results.push({ file: relOrder, rule: name, @@ -142,9 +121,7 @@ export function validateAll(files, gitRoot, { fix } = {}) { message: `Invalid entry: "${entry}" should not be listed in _order.yaml`, _fixRemove: { orderPath, entry }, }); - continue; - } - if (!entryExists(orderDir, entry)) { + } else if (!onDisk.has(entry)) { results.push({ file: relOrder, rule: name, @@ -158,48 +135,51 @@ export function validateAll(files, gitRoot, { fix } = {}) { } } - // ---- Apply fixes ---- - if (fix) { - // Additions first. - for (const r of results) { - if (!r._fixAdd) continue; - const { orderPath, missing } = r._fixAdd; - const newEntries = missing.map((slug) => `- ${yamlSafeSlug(slug)}`).join('\n'); - - if (fs.existsSync(orderPath)) { - const existing = fs.readFileSync(orderPath, 'utf-8'); - const sep = existing.endsWith('\n') ? '' : '\n'; - fs.writeFileSync(orderPath, `${existing}${sep}${newEntries}\n`); - } else { - fs.mkdirSync(path.dirname(orderPath), { recursive: true }); - fs.writeFileSync(orderPath, `${newEntries}\n`); - } - r.message += ' (fixed)'; - } + if (!fix) return strip(results); - // Removals: group stale/index entries by order file, then rewrite each once. - const removalsByPath = new Map(); - for (const r of results) { - if (!r._fixRemove) continue; - const { orderPath, entry } = r._fixRemove; - if (!removalsByPath.has(orderPath)) removalsByPath.set(orderPath, new Set()); - removalsByPath.get(orderPath).add(entry); - } - for (const [orderPath, toRemove] of removalsByPath) { - if (!fs.existsSync(orderPath)) continue; - const kept = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')).filter((e) => !toRemove.has(e)); - if (kept.length > 0) { - fs.writeFileSync(orderPath, kept.map((s) => `- ${yamlSafeSlug(s)}`).join('\n') + '\n'); - } else { - fs.unlinkSync(orderPath); - } + // Additions: append missing slugs to the _order.yaml. + for (const r of results) { + if (!r._fixAdd) continue; + const { orderPath, missing } = r._fixAdd; + const newEntries = missing.map((slug) => `- ${yamlSafeSlug(slug)}`).join('\n'); + + if (fs.existsSync(orderPath)) { + const existing = fs.readFileSync(orderPath, 'utf-8'); + const sep = existing.endsWith('\n') ? '' : '\n'; + fs.writeFileSync(orderPath, `${existing}${sep}${newEntries}\n`); + } else { + fs.mkdirSync(path.dirname(orderPath), { recursive: true }); + fs.writeFileSync(orderPath, `${newEntries}\n`); } - for (const r of results) { - if (r._fixRemove) r.message += ' (fixed)'; + r.message += ' (fixed)'; + } + + // Removals: group stale/index entries by order file, then rewrite each once. + const removalsByPath = new Map(); + for (const r of results) { + if (!r._fixRemove) continue; + const { orderPath, entry } = r._fixRemove; + if (!removalsByPath.has(orderPath)) removalsByPath.set(orderPath, new Set()); + removalsByPath.get(orderPath).add(entry); + } + for (const [orderPath, toRemove] of removalsByPath) { + if (!fs.existsSync(orderPath)) continue; + const kept = parseOrderYaml(fs.readFileSync(orderPath, 'utf-8')).filter((e) => !toRemove.has(e)); + if (kept.length > 0) { + fs.writeFileSync(orderPath, kept.map((s) => `- ${yamlSafeSlug(s)}`).join('\n') + '\n'); + } else { + fs.unlinkSync(orderPath); } } + for (const r of results) { + if (r._fixRemove) r.message += ' (fixed)'; + } + + return strip(results); +} - // Strip internal fix data before returning. +// Remove internal fix descriptors before returning results to the reporter. +function strip(results) { for (const r of results) { delete r._fixAdd; delete r._fixRemove; diff --git a/test/ordering.test.js b/test/ordering.test.js index 6f00976..485caab 100644 --- a/test/ordering.test.js +++ b/test/ordering.test.js @@ -64,3 +64,19 @@ test('--fix adds a file missing from _order.yaml', () => { rmRepo(root); } }); + +test('a folder entry with no markdown children is not flagged stale', () => { + const root = makeRepo({ + 'docs/cat/sub/.gitkeep': '', + 'docs/cat/_order.yaml': '- sub\n', + }); + try { + const res = validateAll(collectFiles(root), root, {}); + assert.ok( + !res.some((r) => /Stale entry: "sub"/.test(r.message)), + 'a subfolder with no .md children still counts as on-disk', + ); + } finally { + rmRepo(root); + } +});