diff --git a/.gittensory.yml.example b/.gittensory.yml.example index b6b5a1b321..ab94e80503 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -1009,11 +1009,22 @@ settings: # claude_effort: null # Overrides CLAUDE_AI_EFFORT for this repo. String or null. Default (env unset): medium. # codex_model: null # Overrides CODEX_AI_MODEL for this repo. String or null. # codex_effort: null # Overrides CODEX_AI_EFFORT for this repo. String or null. Default (env unset): medium. +# ollama_model: null # Overrides OLLAMA_AI_MODEL for this repo's ollama reviewer. String or null. (#3902) +# openai_model: null # Overrides OPENAI_AI_MODEL for this repo's openai reviewer. String or null. (#3902) +# openai_compatible_model: null # Overrides OPENAI_COMPATIBLE_AI_MODEL for this repo. String or null. (#3902) +# anthropic_model: null # Overrides ANTHROPIC_AI_MODEL for this repo's BYOK Messages API reviewer. String or null. (#3902) # # Per-repo before/after screenshot-capture config (#3609 preview / #3610 routes). Only takes effect when # # the operator has ALSO enabled GITTENSORY_REVIEW_SCREENSHOTS + this repo's cutover allowlist -- this # # config narrows/redirects that feature, it never turns it on by itself. All-null/empty/default ⇒ # # byte-identical to today (GitHub-native preview discovery, automatic file-to-route inference). # visual: +# # The repo's "before" production URL -- e.g. "https://metagraph.sh" for a repo whose live site differs +# # from the operator's own PUBLIC_SITE_ORIGIN env var (a single GLOBAL value with no per-repo awareness, +# # correct for at most one repo on a multi-repo self-host instance). ALWAYS wins over PUBLIC_SITE_ORIGIN +# # when set, mirroring preview.url_template's precedence over GitHub-native discovery below. Must resolve +# # to a valid HTTPS URL targeting a public host (same SSRF guard as preview.url_template). String or null. +# # Default: null (falls back to PUBLIC_SITE_ORIGIN). +# production_url: "https://example.com" # preview: # # The repo's "after" preview URL, with {number}/{head_sha}/{head_sha_short} placeholders substituted # # at capture time. ALWAYS wins over GitHub-native preview discovery (Deployments API / commit checks / diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index e7c87c9d3c..1667cbb52c 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -1022,11 +1022,22 @@ settings: # claude_effort: null # Overrides CLAUDE_AI_EFFORT for this repo. String or null. Default (env unset): medium. # codex_model: null # Overrides CODEX_AI_MODEL for this repo. String or null. # codex_effort: null # Overrides CODEX_AI_EFFORT for this repo. String or null. Default (env unset): medium. +# ollama_model: null # Overrides OLLAMA_AI_MODEL for this repo's ollama reviewer. String or null. (#3902) +# openai_model: null # Overrides OPENAI_AI_MODEL for this repo's openai reviewer. String or null. (#3902) +# openai_compatible_model: null # Overrides OPENAI_COMPATIBLE_AI_MODEL for this repo. String or null. (#3902) +# anthropic_model: null # Overrides ANTHROPIC_AI_MODEL for this repo's BYOK Messages API reviewer. String or null. (#3902) # # Per-repo before/after screenshot-capture config (#3609 preview / #3610 routes). Only takes effect when # # the operator has ALSO enabled GITTENSORY_REVIEW_SCREENSHOTS + this repo's cutover allowlist -- this # # config narrows/redirects that feature, it never turns it on by itself. All-null/empty/default ⇒ # # byte-identical to today (GitHub-native preview discovery, automatic file-to-route inference). # visual: +# # The repo's "before" production URL -- e.g. "https://metagraph.sh" for a repo whose live site differs +# # from the operator's own PUBLIC_SITE_ORIGIN env var (a single GLOBAL value with no per-repo awareness, +# # correct for at most one repo on a multi-repo self-host instance). ALWAYS wins over PUBLIC_SITE_ORIGIN +# # when set, mirroring preview.url_template's precedence over GitHub-native discovery below. Must resolve +# # to a valid HTTPS URL targeting a public host (same SSRF guard as preview.url_template). String or null. +# # Default: null (falls back to PUBLIC_SITE_ORIGIN). +# production_url: "https://example.com" # preview: # # The repo's "after" preview URL, with {number}/{head_sha}/{head_sha_short} placeholders substituted # # at capture time. ALWAYS wins over GitHub-native preview discovery (Deployments API / commit checks / diff --git a/scripts/check-docs-drift.d.mts b/scripts/check-docs-drift.d.mts index 0e6074b5ea..f399208c58 100644 --- a/scripts/check-docs-drift.d.mts +++ b/scripts/check-docs-drift.d.mts @@ -4,14 +4,30 @@ export function extractCatalogIds(sourceText: string, catalogConstName: string): export function extractGateModeFields(typesText: string): string[]; +export function extractRepositorySettingsFields(typesText: string): string[]; + +export function extractFocusManifestFields(focusManifestText: string): string[]; + export type GateModeManifestRow = { field: string; aliases: string[]; pages: string[] }; export const GATE_MODE_MANIFEST: GateModeManifestRow[]; +export type AliasManifestRow = { field: string; aliases: string[] }; + +export const SETTINGS_ALIAS_MANIFEST: AliasManifestRow[]; + +export const FOCUS_MANIFEST_ALIAS_MANIFEST: AliasManifestRow[]; + export function checkDocsDrift(options: { root: string; readFile?: (root: string, relativePath: string) => string; }): { failures: string[]; - counts: { flags: number; commands: number; gateModes: number }; + counts: { + flags: number; + commands: number; + gateModes: number; + settingsFields: number; + focusManifestFields: number; + }; }; diff --git a/scripts/check-docs-drift.mjs b/scripts/check-docs-drift.mjs index 4c35899dee..3bb86680cf 100644 --- a/scripts/check-docs-drift.mjs +++ b/scripts/check-docs-drift.mjs @@ -1,10 +1,14 @@ #!/usr/bin/env node -// Cross-checks three enumerable "surfaces" that each have a single code source of truth but are also -// meant to be documented EXHAUSTIVELY on specific docs pages: feature flags (src/env.d.ts's -// GITTENSORY_REVIEW_* family), @gittensory commands (src/github/commands.ts's two command catalogs), and -// gate-mode dimensions (src/types.ts's *GateMode fields on RepositorySettings). Nothing else in CI catches a -// docs page silently falling behind when a new flag/command/gate-mode field is added to source but the docs -// page enumerating that surface is never updated -- a reviewer has to notice by eye, and often doesn't. +// Cross-checks five enumerable "surfaces" that each have a single code source of truth but are also meant to +// be documented EXHAUSTIVELY somewhere: feature flags (src/env.d.ts's GITTENSORY_REVIEW_* family), +// @gittensory commands (src/github/commands.ts's two command catalogs), gate-mode dimensions (src/types.ts's +// *GateMode fields on RepositorySettings) against specific docs pages, and -- the widened part (#4617) -- the +// FULL RepositorySettings field surface plus every parseable FocusManifest field (packages/gittensory-engine) +// against .gittensory.yml.example. Nothing else in CI catches a docs page/example silently falling behind when +// a new flag/command/gate-mode/settings/manifest field is added to source but the place documenting that +// surface is never updated -- a reviewer has to notice by eye, and often doesn't (#4617's own audit found +// `agentGlobalFreezeOverride` and `review.visual.production_url` this way: both fully live in code, neither +// mentioned anywhere a maintainer would think to look). import { readFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -42,6 +46,177 @@ export function extractGateModeFields(typesText) { return [...new Set([...matches].map((match) => match[0]))]; } +/** Shared brace-depth-slicing field extractor for any `export type = { ... };` object-literal in + * `text`. Walks forward from the matching open brace counting `{`/`}` so the type's OWN closing brace is found + * regardless of nested object/generic braces inside a field's type (e.g. `ReadonlyArray<{ model: string; ... + * }>`), then applies a line-anchored field regex to the sliced body -- safe because every field in every type + * this script reads (`RepositorySettings`, `FocusManifest` and its nested config types) is written one-per-line + * by this repo's Prettier config; verified for both by their complete absence of a multi-line inline-object + * field type as of this writing. Returns `null` when `typeName` has no such declaration in `text` (so callers + * can tell "not a local object-literal type" from "declared but empty"), else `[{name, typeText}]` pairs in + * declaration order -- `typeText` is the field's own declared type (the text after its `:` up to its + * terminating top-level `;`), used by `extractFocusManifestFields` to detect a bare reference to another local + * type worth recursing into. */ +function extractTypeLiteralFieldEntries(text, typeName) { + const declPattern = new RegExp(`export type ${typeName}\\s*=\\s*\\{`); + const declMatch = declPattern.exec(text); + if (!declMatch) return null; + const bodyStart = declMatch.index + declMatch[0].length; + let depth = 1; + let index = bodyStart; + for (; index < text.length && depth > 0; index++) { + if (text[index] === "{") depth++; + else if (text[index] === "}") depth--; + } + const body = text.slice(bodyStart, index - 1); + const fields = []; + for (const line of body.split("\n")) { + const fieldMatch = /^\s*([a-zA-Z_][a-zA-Z0-9_]*)\??:\s*(.+);\s*$/.exec(line); + if (fieldMatch) fields.push({ name: fieldMatch[1], typeText: fieldMatch[2].trim() }); + } + return fields; +} + +/** Every top-level field name DECLARED directly on the `RepositorySettings` object-literal type in src/types.ts + * (#4617) -- the FULL ~100-field surface, not just the `*GateMode` subset `extractGateModeFields` above + * targets. Unlike that regex (which matches a NAME SHAPE anywhere in the file), this is anchored on the type's + * own brace boundary via `extractTypeLiteralFieldEntries`, so it can never pick up an unrelated same-shaped + * field from a different type declared later in the file. Returns every field, unfiltered, in declaration + * order -- judging which fields are genuinely yml-configurable vs internal bookkeeping is `checkDocsDrift`'s + * job (`NOT_YML_CONFIGURABLE_SETTINGS_FIELDS` below), mirroring how `extractGateModeFields` also returns every + * match unfiltered and leaves the GATE_MODE_MANIFEST cross-reference to the caller. */ +export function extractRepositorySettingsFields(typesText) { + return (extractTypeLiteralFieldEntries(typesText, "RepositorySettings") ?? []).map((entry) => entry.name); +} + +/** RepositorySettings fields deliberately excluded from the "every field must have SOME + * `.gittensory.yml.example` mention" check below, for two distinct reasons -- flagging either as "undocumented" + * would be a false drift signal, not a real gap: + * - Not a maintainer-settable knob at all: `repoFullName` is the row's own identity key (set once at + * creation, the opposite of something a maintainer overrides via config); `createdAt`/`updatedAt` are + * DB-row bookkeeping timestamps. + * - `agentGlobalFreezeOverride`: genuinely settable, but DELIBERATELY never documented in the PUBLIC + * `.gittensory.yml.example` -- it is settable only from the self-host operator's own PRIVATE config + * (`source: "api_record"` in `parseSettingsOverride`, packages/gittensory-engine/src/focus-manifest.ts), + * never from a repo's own committed, maintainer-owned manifest (#4391's scope-leak fix). Documenting it in + * the public example would misleadingly suggest a repo maintainer can set it themselves -- see the same + * exclusion, with the same rationale, in `SETTINGS_OPERATOR_ONLY_FIELDS` in + * test/unit/focus-manifest.test.ts's `.gittensory.yml.example field-exhaustiveness` suite. (An #4617 audit + * pass first flagged this field as an undocumented gap without that context; cross-checking the existing + * exhaustiveness suite before "fixing" it here caught the false positive.) */ +const NOT_YML_CONFIGURABLE_SETTINGS_FIELDS = new Set(["repoFullName", "createdAt", "updatedAt", "agentGlobalFreezeOverride"]); + +/** RepositorySettings fields whose `.gittensory.yml.example` documentation exists under a DIFFERENT, shorter + * name than the field itself -- almost always because the yml groups several sibling fields under one named + * block (`gate.aiReview.*`, `gate.cla.*`, `gate.slop.*`, `gate.copycat.*`, `gate.readiness.*`) and so drops the + * shared prefix the flat RepositorySettings field name carries to distinguish it from its siblings (e.g. + * `aiReviewCloseConfidence` is documented as just `closeConfidence`, nested under the `aiReview:` block -- + * verified against the real `.gittensory.yml.example` for every row below). A field landing here is a + * deliberate, reviewed judgment call that it IS genuinely documented, just not findable by a literal name + * match -- unlike GATE_MODE_MANIFEST (checked against specific docs ROUTE pages), `aliases` here is checked + * against the WHOLE `.gittensory.yml.example` file, matching #4617's "SOME mention" ask, so one representative + * alias per row is enough. Any `*GateMode` field is deliberately absent from this manifest even though its own + * yml key is ALSO renamed the same way -- GATE_MODE_MANIFEST above already owns that exhaustive cross-check. */ +export const SETTINGS_ALIAS_MANIFEST = [ + { field: "gateCheckMode", aliases: ["checkMode"] }, + { field: "reviewCheckMode", aliases: ["checkMode"] }, + { field: "gatePack", aliases: ["pack:"] }, + { field: "qualityGateMinScore", aliases: ["minScore"] }, + { field: "slopGateMinScore", aliases: ["minScore"] }, + { field: "copycatGateMinScore", aliases: ["minScore"] }, + { field: "claConsentPhrase", aliases: ["consentPhrase"] }, + { field: "claCheckRunName", aliases: ["checkRunName"] }, + { field: "claCheckRunAppSlug", aliases: ["checkRunAppSlug"] }, + { field: "gateDryRun", aliases: ["dryRun"] }, + { field: "slopAiAdvisory", aliases: ["aiAdvisory"] }, + { field: "aiReviewMode", aliases: ["aiReview:"] }, + { field: "aiReviewByok", aliases: ["byok"] }, + { field: "aiReviewProvider", aliases: ["aiReview:"] }, + { field: "aiReviewModel", aliases: ["aiReview:"] }, + { field: "aiReviewAllAuthors", aliases: ["allAuthors"] }, + { field: "aiReviewCloseConfidence", aliases: ["closeConfidence"] }, + { field: "aiReviewLowConfidenceDisposition", aliases: ["lowConfidenceDisposition"] }, + { field: "aiReviewCombine", aliases: ["aiReview:"] }, + { field: "aiReviewOnMerge", aliases: ["onMerge"] }, + { field: "aiReviewReviewers", aliases: ["reviewers:"] }, + { field: "requireFreshRebaseWindowMinutes", aliases: ["requireFreshRebaseWindow"] }, +]; + +/** camelCase -> snake_case, matching the casing convention `.gittensory.yml`'s `review:` block (and everything + * nested under it, e.g. `review.visual.*`) uses for its own keys -- e.g. `productionUrl` -> `production_url`. + * Every OTHER FocusManifest-reachable block keeps its source field's camelCase spelling verbatim in the yml + * (matching the top-level manifest fields and the `gate:`/`settings:` blocks), for which this is a harmless + * no-op: a name with no lower-to-upper case boundary is unchanged by the transform. */ +function toSnakeCase(name) { + return name.replace(/([a-z0-9])([A-Z])/g, "$1_$2").toLowerCase(); +} + +/** Leaf fields never worth flagging as "undocumented" even though they're syntactically object-literal fields + * on a FocusManifest-reachable type: parser-computed bookkeeping the yml author never sets. `present`/`source`/ + * `warnings` record whether/how a block was configured (an OUTPUT of parsing, not an input); `sharedConfigSource` + * is explicitly documented as runtime-only provenance by its own doc comment ("Never parsed from maintainer + * YAML -- set by the private-config loader only"). */ +const FOCUS_MANIFEST_BOOKKEEPING_FIELDS = new Set(["present", "source", "warnings", "sharedConfigSource"]); + +/** Top-level FocusManifest fields deliberately NOT walked by `extractFocusManifestFields`: `gate` + * (`FocusManifestGateConfig`) and `settings` (`FocusManifestSettings`) are the config-as-code MIRROR of + * RepositorySettings' `gate*Mode`/`settings:`-block fields -- the SAME underlying settings, reached through a + * second, yml-shaped parsing path (`parseGateConfig` / `parseSettingsOverride` in this same file) that renames + * several fields yet again (e.g. RepositorySettings' `slopGateMinScore` is FocusManifestGateConfig's + * `slopMinScore`, itself yml `gate.slop.minScore`). Walking them here too would re-flag the exact same knobs + * under a THIRD set of names for zero new coverage; the RepositorySettings-based checks above already own that + * surface. `review`/`features`/`contentLane`/`repoDocGeneration`/`reviewRecap`/`maintainerRecap` have no + * RepositorySettings counterpart at all -- config-as-code-only surfaces this script had zero coverage of + * before #4617. */ +const FOCUS_MANIFEST_SKIP_TOP_LEVEL_FIELDS = new Set(["gate", "settings"]); + +/** Every leaf (non-recursable) field reachable from the `FocusManifest` type in `focusManifestText` (#4617), + * returned as dotted paths built from the SOURCE field names (e.g. `"review.visual.productionUrl"` -- + * `checkDocsDrift` derives the yml-cased spelling via `toSnakeCase` for the actual doc lookup). Recurses into + * any field whose OWN declared type is a bare reference to another local `export type = { ... }` in the + * same file (e.g. `review: FocusManifestReviewConfig`, then `visual: VisualConfig` inside THAT), so a knob + * nested three levels deep like `review.visual.production_url` is enumerated exactly like a top-level one -- + * unlike RepositorySettings, FocusManifest's real config surface is NOT flat. A field typed as an array/union/ + * Record/generic (e.g. `pathInstructions: ReviewPathInstruction[]`, `fields: Partial>`) is treated as ONE leaf itself rather than recursed into -- it is documented (or not) as a single + * structured knob, matching how the rest of this script treats `aiReviewReviewers`'s array-of-objects shape. */ +export function extractFocusManifestFields(focusManifestText) { + const leaves = []; + const visitedTypes = new Set(); + + function walk(typeName, pathPrefix) { + if (visitedTypes.has(typeName)) return; // guards a hypothetical future cycle; no real cycle exists today + visitedTypes.add(typeName); + const entries = extractTypeLiteralFieldEntries(focusManifestText, typeName); + if (!entries) return; + for (const { name, typeText } of entries) { + if (FOCUS_MANIFEST_BOOKKEEPING_FIELDS.has(name)) continue; + if (pathPrefix.length === 0 && FOCUS_MANIFEST_SKIP_TOP_LEVEL_FIELDS.has(name)) continue; + const path = [...pathPrefix, name]; + const referencedType = /^[A-Z][a-zA-Z0-9]*$/.test(typeText) ? typeText : null; + if (referencedType && extractTypeLiteralFieldEntries(focusManifestText, referencedType)) { + walk(referencedType, path); + } else { + leaves.push(path.join(".")); + } + } + } + + walk("FocusManifest", []); + return leaves; +} + +/** FocusManifest leaf fields (dotted paths, same shape `extractFocusManifestFields` returns) whose + * `.gittensory.yml.example` documentation exists under a shorter name than their own doc comment's dotted-path + * tag would suggest -- e.g. `review.footerText`'s own field carries no `` `review.footer.text` `` tag at all + * (unlike most of its siblings), and is in fact documented as just `footer:` (a nested `text:` sub-key). + * Mirrors SETTINGS_ALIAS_MANIFEST's reasoning exactly, one level down. */ +export const FOCUS_MANIFEST_ALIAS_MANIFEST = [ + { field: "review.footerText", aliases: ["footer:"] }, + { field: "review.enrichmentAnalyzers", aliases: ["enrichment:"] }, + { field: "review.reviewMemory", aliases: ["memory:"] }, +]; + // The real current *GateMode fields on RepositorySettings in src/types.ts. Each row maps the field to its // .gittensory.yml alias(es) (the field's own DB/settings name, plus any config-as-code YAML path it is also // known by) and the docs route filenames (relative to apps/gittensory-ui/src/routes/) that must document it. @@ -71,11 +246,12 @@ function defaultReadFile(root, relativePath) { } /** - * Cross-check feature flags, @gittensory commands, and gate-mode dimensions between their code source of - * truth and the docs pages meant to document them exhaustively. `readFile(root, relativePath)` is injectable - * so tests can simulate a broken/incomplete docs page or source file without touching the real filesystem. - * Returns `{ failures, counts }` -- pure given its inputs, no process.exit/console side effects of its own - * (those live in main()). + * Cross-check feature flags, @gittensory commands, gate-mode dimensions, the full RepositorySettings surface, + * and every parseable FocusManifest field between their code source of truth and wherever they're meant to be + * documented exhaustively (specific docs pages for the first three; `.gittensory.yml.example` for the last + * two, #4617). `readFile(root, relativePath)` is injectable so tests can simulate a broken/incomplete docs + * page or source file without touching the real filesystem. Returns `{ failures, counts }` -- pure given its + * inputs, no process.exit/console side effects of its own (those live in main()). */ export function checkDocsDrift({ root, readFile = defaultReadFile }) { const failures = []; @@ -147,9 +323,66 @@ export function checkDocsDrift({ root, readFile = defaultReadFile }) { } } + // 4. The FULL RepositorySettings surface (#4617): every field, not just *GateMode, vs .gittensory.yml.example. + // A field passes when its literal name appears anywhere in the example file, when it's already covered + // exhaustively by GATE_MODE_MANIFEST above (checked against docs pages, not repeated here), when it's judged + // not yml-configurable at all (NOT_YML_CONFIGURABLE_SETTINGS_FIELDS), or when SETTINGS_ALIAS_MANIFEST records + // it as documented under a different name. + const repositorySettingsFields = extractRepositorySettingsFields(typesText); + if (repositorySettingsFields.length < 20) { + failures.push( + `src/types.ts: extraction found only ${repositorySettingsFields.length} RepositorySettings fields -- expected 20+; the extraction regex may be broken`, + ); + } else { + const gateModeManifestFields = new Set(GATE_MODE_MANIFEST.map((row) => row.field)); + const settingsAliases = new Map(SETTINGS_ALIAS_MANIFEST.map((row) => [row.field, row.aliases])); + const ymlExampleText = read(".gittensory.yml.example"); + for (const field of repositorySettingsFields) { + if (NOT_YML_CONFIGURABLE_SETTINGS_FIELDS.has(field)) continue; + if (gateModeManifestFields.has(field)) continue; + if (ymlExampleText.includes(field)) continue; + const aliases = settingsAliases.get(field); + if (aliases?.some((alias) => ymlExampleText.includes(alias))) continue; + failures.push( + `.gittensory.yml.example: missing any mention of RepositorySettings field "${field}" -- document it there (or the relevant reference doc), or add a SETTINGS_ALIAS_MANIFEST row in scripts/check-docs-drift.mjs if it's already documented under a different yml key name`, + ); + } + } + + // 5. Every parseable FocusManifest field (#4617), excluding gate:/settings: (already exhaustively covered by + // step 4 above through their RepositorySettings mirror), vs .gittensory.yml.example. + const focusManifestText = read("packages/gittensory-engine/src/focus-manifest.ts"); + const focusManifestFields = extractFocusManifestFields(focusManifestText); + if (focusManifestFields.length < 15) { + failures.push( + `packages/gittensory-engine/src/focus-manifest.ts: extraction found only ${focusManifestFields.length} FocusManifest leaf fields -- expected 15+; the extraction regex may be broken`, + ); + } else { + const focusManifestAliases = new Map(FOCUS_MANIFEST_ALIAS_MANIFEST.map((row) => [row.field, row.aliases])); + const ymlExampleText = read(".gittensory.yml.example"); + for (const path of focusManifestFields) { + const segments = path.split("."); + const leaf = segments[segments.length - 1]; + const snakeLeaf = toSnakeCase(leaf); + if (ymlExampleText.includes(leaf) || ymlExampleText.includes(snakeLeaf)) continue; + const aliases = focusManifestAliases.get(path); + if (aliases?.some((alias) => ymlExampleText.includes(alias))) continue; + const prettyPath = segments.map(toSnakeCase).join("."); + failures.push( + `.gittensory.yml.example: missing any mention of FocusManifest field "${prettyPath}" -- document it there, or add a FOCUS_MANIFEST_ALIAS_MANIFEST row in scripts/check-docs-drift.mjs if it's already documented under a different yml key name`, + ); + } + } + return { failures, - counts: { flags: flags.length, commands: allCommandIds.length, gateModes: gateModeFields.length }, + counts: { + flags: flags.length, + commands: allCommandIds.length, + gateModes: gateModeFields.length, + settingsFields: repositorySettingsFields.length, + focusManifestFields: focusManifestFields.length, + }, }; } @@ -162,7 +395,10 @@ function main() { process.exit(1); } - console.log(`Docs-drift check ok: ${counts.flags} feature flags, ${counts.commands} commands, ${counts.gateModes} gate-mode fields all documented.`); + console.log( + `Docs-drift check ok: ${counts.flags} feature flags, ${counts.commands} commands, ${counts.gateModes} gate-mode fields, ` + + `${counts.settingsFields} RepositorySettings fields, ${counts.focusManifestFields} FocusManifest fields all documented.`, + ); } // Guard so importing this module for its pure exports (tests) never triggers the file-read/exit side effects. diff --git a/test/unit/check-docs-drift-script.test.ts b/test/unit/check-docs-drift-script.test.ts index 4626b436f0..0175458c4b 100644 --- a/test/unit/check-docs-drift-script.test.ts +++ b/test/unit/check-docs-drift-script.test.ts @@ -3,9 +3,13 @@ import { describe, expect, it } from "vitest"; import { checkDocsDrift, extractCatalogIds, + extractFocusManifestFields, extractGateModeFields, extractGittensoryReviewFlags, + extractRepositorySettingsFields, + FOCUS_MANIFEST_ALIAS_MANIFEST, GATE_MODE_MANIFEST, + SETTINGS_ALIAS_MANIFEST, } from "../../scripts/check-docs-drift.mjs"; describe("check-docs-drift script", () => { @@ -82,6 +86,138 @@ describe("check-docs-drift script", () => { }); }); + describe("extractRepositorySettingsFields (#4617)", () => { + it("extracts a plain field extractGateModeFields would never find (#4617's own gap: a field not shaped like *GateMode)", () => { + const fixture = ` + export type RepositorySettings = { + repoFullName: string; + agentGlobalFreezeOverride?: boolean | undefined; + linkedIssueGateMode: GateRuleMode; + }; + `; + + // The OLD, narrow check: invisible to a plain boolean field with no "GateMode" in its name -- this is + // exactly the shape of gap #4617 was filed over (agentGlobalFreezeOverride was live in source code but + // had zero automated documentation guarantee, because it isn't a *GateMode field). + expect(extractGateModeFields(fixture)).toEqual(["linkedIssueGateMode"]); + expect(extractGateModeFields(fixture)).not.toContain("agentGlobalFreezeOverride"); + + // The WIDENED check: sees every field on the type, regardless of shape. + const fields = extractRepositorySettingsFields(fixture); + expect(fields).toEqual(["repoFullName", "agentGlobalFreezeOverride", "linkedIssueGateMode"]); + }); + + it("is anchored on the RepositorySettings type's own brace boundary, not a bare name match elsewhere in the file", () => { + const fixture = ` + export type SomeUnrelatedType = { + decoyField: string; + }; + export type RepositorySettings = { + realField: string; + }; + `; + + expect(extractRepositorySettingsFields(fixture)).toEqual(["realField"]); + }); + + it("returns [] when RepositorySettings has no declaration in the text", () => { + expect(extractRepositorySettingsFields("export type SomethingElse = { x: string };")).toEqual([]); + }); + }); + + describe("extractFocusManifestFields (#4617)", () => { + it("recurses into a nested named config type, producing the dotted path a flat top-level-only check would miss", () => { + const fixture = ` + export type FocusManifest = { + present: boolean; + review: FocusManifestReviewConfig; + }; + export type FocusManifestReviewConfig = { + present: boolean; + visual: VisualConfig; + }; + export type VisualConfig = { + productionUrl: string | null; + }; + `; + + // This is exactly #4617's own concrete gap, reproduced structurally: `review.visual.production_url` is + // three levels deep, in a type the top-level FocusManifest declaration never mentions by name. + expect(extractFocusManifestFields(fixture)).toEqual(["review.visual.productionUrl"]); + }); + + it("skips the gate/settings top-level fields (already exhaustively covered elsewhere, see FOCUS_MANIFEST_SKIP_TOP_LEVEL_FIELDS)", () => { + const fixture = ` + export type FocusManifest = { + present: boolean; + gate: FocusManifestGateConfig; + settings: FocusManifestSettings; + review: FocusManifestReviewConfig; + }; + export type FocusManifestGateConfig = { + present: boolean; + someGateField: string; + }; + export type FocusManifestSettings = { + present: boolean; + someSettingsField: string; + }; + export type FocusManifestReviewConfig = { + present: boolean; + someReviewField: string; + }; + `; + + const fields = extractFocusManifestFields(fixture); + + expect(fields).toEqual(["review.someReviewField"]); + expect(fields).not.toContain("gate.someGateField"); + expect(fields).not.toContain("settings.someSettingsField"); + }); + + it("excludes parser-computed bookkeeping fields (present/source/warnings/sharedConfigSource) at every nesting level", () => { + const fixture = ` + export type FocusManifest = { + present: boolean; + source: string; + warnings: string[]; + review: FocusManifestReviewConfig; + }; + export type FocusManifestReviewConfig = { + present: boolean; + sharedConfigSource: string | null; + realField: string; + }; + `; + + expect(extractFocusManifestFields(fixture)).toEqual(["review.realField"]); + }); + + it("treats an array/union/generic-typed field as one leaf rather than recursing into its element shape", () => { + const fixture = ` + export type FocusManifest = { + present: boolean; + review: FocusManifestReviewConfig; + }; + export type FocusManifestReviewConfig = { + present: boolean; + pathInstructions: ReviewPathInstruction[]; + maxFindings: MaxFindingsConfig | null; + }; + export type ReviewPathInstruction = { path: string; instructions: string }; + export type MaxFindingsConfig = { blockers: number | null; nits: number | null }; + `; + + // `ReviewPathInstruction[]` is an array type (not a bare identifier) so it's a leaf; `MaxFindingsConfig | + // null` is a union (not a bare identifier either), also a leaf -- neither recurses into its element shape. + expect(extractFocusManifestFields(fixture)).toEqual(["review.pathInstructions", "review.maxFindings"]); + }); + + it("returns [] when FocusManifest has no declaration in the text", () => { + expect(extractFocusManifestFields("export type SomethingElse = { x: string };")).toEqual([]); + }); + }); + describe("checkDocsDrift", () => { // A minimal set of fixtures that satisfies every check EXCEPT the one under test in each case below. const baseFlags = Array.from({ length: 10 }, (_, i) => `GITTENSORY_REVIEW_FLAG_${i}?: string;`).join("\n"); @@ -98,6 +234,12 @@ describe("check-docs-drift script", () => { ...Array.from({ length: 9 }, (_, i) => `maint-${i}`), ]; const baseFlagNames = Array.from({ length: 10 }, (_, i) => `GITTENSORY_REVIEW_FLAG_${i}`); + // Extra plain (non-*GateMode-shaped) RepositorySettings fields -- proves check 4 covers the FULL surface, + // not just what extractGateModeFields already saw via GATE_MODE_MANIFEST. + const baseSettingsExtraFields = Array.from({ length: 20 }, (_, i) => `settingsField${i}`); + // Extra FocusManifestReviewConfig fields, plus a nested `visual.productionUrl` -- the nesting reproduces + // #4617's own concrete gap shape (a field 2+ levels below the top-level FocusManifest type). + const baseFocusManifestReviewFields = Array.from({ length: 18 }, (_, i) => `reviewField${i}`); function buildDocsPageText(commandIds: string[]) { return commandIds.map((id) => `@gittensory ${id}`).join("\n"); @@ -111,11 +253,55 @@ describe("check-docs-drift script", () => { return GATE_MODE_MANIFEST.flatMap((row) => row.aliases).join("\n"); } + function buildRepositorySettingsSource(extraFieldNames: string[]) { + return [ + "export type RepositorySettings = {", + ...GATE_MODE_MANIFEST.map((row) => ` ${row.field}: GateRuleMode;`), + ...extraFieldNames.map((name) => ` ${name}: string;`), + "};", + ].join("\n"); + } + + function buildFocusManifestSource(reviewFieldNames: string[]) { + return [ + "export type FocusManifest = {", + " present: boolean;", + " gate: FocusManifestGateConfig;", + " settings: FocusManifestSettings;", + " review: FocusManifestReviewConfig;", + "};", + "export type FocusManifestGateConfig = {", + " present: boolean;", + " someGateField: string;", + "};", + "export type FocusManifestSettings = {", + " present: boolean;", + " someSettingsField: string;", + "};", + "export type FocusManifestReviewConfig = {", + " present: boolean;", + ...reviewFieldNames.map((name) => ` ${name}: string;`), + " visual: VisualConfig;", + "};", + "export type VisualConfig = {", + " productionUrl: string | null;", + "};", + ].join("\n"); + } + + function buildYmlExampleText(settingsFieldNames: string[], reviewFieldNames: string[]) { + return [...settingsFieldNames.map((name) => `${name}: null`), ...reviewFieldNames.map((name) => `${name}: null`), "production_url: null"].join( + "\n", + ); + } + function baseFixtures(): Record { const files: Record = { "src/env.d.ts": baseFlags, "src/github/commands.ts": baseCommandsSource, - "src/types.ts": GATE_MODE_MANIFEST.map((row) => `${row.field}: GateRuleMode;`).join("\n"), + "src/types.ts": buildRepositorySettingsSource(baseSettingsExtraFields), + "packages/gittensory-engine/src/focus-manifest.ts": buildFocusManifestSource(baseFocusManifestReviewFields), + ".gittensory.yml.example": buildYmlExampleText(baseSettingsExtraFields, baseFocusManifestReviewFields), "apps/gittensory-ui/src/routes/docs.tuning.tsx": [buildFlagsPageText(baseFlagNames), buildGateModePageText()].join("\n"), "apps/gittensory-ui/src/routes/docs.privacy-security.tsx": buildFlagsPageText(baseFlagNames), "apps/gittensory-ui/src/routes/docs.maintainer-workflow.tsx": buildDocsPageText(allBaseCommandIds), @@ -142,7 +328,9 @@ describe("check-docs-drift script", () => { expect(result.failures).toEqual([]); // gateModes bumped 12 -> 13 for copycatGateMode (#1969, currently inert config scaffold). - expect(result.counts).toEqual({ flags: 10, commands: 19, gateModes: 13 }); + // settingsFields = 13 GATE_MODE_MANIFEST fields + 20 synthetic extras; focusManifestFields = 18 + // synthetic review fields + the nested review.visual.productionUrl leaf (#4617). + expect(result.counts).toEqual({ flags: 10, commands: 19, gateModes: 13, settingsFields: 33, focusManifestFields: 19 }); }); it("catches an unmapped *GateMode field missing from GATE_MODE_MANIFEST", () => { @@ -243,6 +431,111 @@ describe("check-docs-drift script", () => { expect(hit).toBeDefined(); }); + it("self-defends against a broken RepositorySettings-extraction (fewer than 20 fields found, #4617)", () => { + const files = baseFixtures(); + // No "export type RepositorySettings = {" wrapper at all -- extractRepositorySettingsFields finds nothing. + files["src/types.ts"] = "someField: string;"; + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + const hit = result.failures.find( + (failure) => failure.includes("src/types.ts") && failure.includes("RepositorySettings fields") && failure.includes("extraction regex may be broken"), + ); + expect(hit).toBeDefined(); + }); + + it("self-defends against a broken FocusManifest-extraction (fewer than 15 leaf fields found, #4617)", () => { + const files = baseFixtures(); + files["packages/gittensory-engine/src/focus-manifest.ts"] = "export type FocusManifest = { present: boolean; onlyOneField: string; };"; + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + const hit = result.failures.find( + (failure) => + failure.includes("packages/gittensory-engine/src/focus-manifest.ts") && + failure.includes("FocusManifest leaf fields") && + failure.includes("extraction regex may be broken"), + ); + expect(hit).toBeDefined(); + }); + + it("catches a RepositorySettings field with zero .gittensory.yml.example mention and no alias/exclude entry (#4617)", () => { + const files = baseFixtures(); + files["src/types.ts"] = files["src/types.ts"]!.replace("};", " totallyUndocumentedField: boolean;\n};"); + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + const hit = result.failures.find( + (failure) => failure.includes(".gittensory.yml.example") && failure.includes("totallyUndocumentedField"), + ); + expect(hit).toBeDefined(); + // This exact shape -- a plain, non-*GateMode-named field -- is invisible to the OLD narrow check: it + // extracts nothing beyond GATE_MODE_MANIFEST's own 13 rows, so it could never have raised this failure. + expect(extractGateModeFields(files["src/types.ts"])).not.toContain("totallyUndocumentedField"); + }); + + it("does not flag a RepositorySettings field whose real yml key is recorded in SETTINGS_ALIAS_MANIFEST", () => { + // Every real alias-manifest row, exercised directly: add the field to the synthetic RepositorySettings, + // do NOT mention its literal name anywhere, but DO include its recorded alias -- must pass cleanly + // (baseFixtures() alone is already a clean pass, so any failure here can only be this new field). + for (const row of SETTINGS_ALIAS_MANIFEST) { + const files = baseFixtures(); + files["src/types.ts"] = files["src/types.ts"]!.replace("};", ` ${row.field}: string;\n};`); + files[".gittensory.yml.example"] += `\n${row.aliases[0]}`; + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + expect(result.failures, `${row.field} should pass via alias ${row.aliases[0]}`).toEqual([]); + } + }); + + it("treats NOT_YML_CONFIGURABLE_SETTINGS_FIELDS members as excluded even with zero yml mention (repoFullName, createdAt, updatedAt, agentGlobalFreezeOverride)", () => { + const files = baseFixtures(); + files["src/types.ts"] = files["src/types.ts"]!.replace( + "};", + " repoFullName: string;\n createdAt?: string | null | undefined;\n updatedAt?: string | null | undefined;\n agentGlobalFreezeOverride?: boolean | undefined;\n};", + ); + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + for (const field of ["repoFullName", "createdAt", "updatedAt", "agentGlobalFreezeOverride"]) { + expect(result.failures.find((failure) => failure.includes(field))).toBeUndefined(); + } + }); + + it("catches a FocusManifest field nested inside another config type with zero yml mention -- the exact review.visual.production_url shape (#4617)", () => { + const files = baseFixtures(); + // A SECOND VisualConfig-shaped leaf that the synthetic .gittensory.yml.example never mentions. + files["packages/gittensory-engine/src/focus-manifest.ts"] = files["packages/gittensory-engine/src/focus-manifest.ts"]!.replace( + "productionUrl: string | null;", + "productionUrl: string | null;\n totallyUndocumentedNestedField: string | null;", + ); + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + const hit = result.failures.find( + (failure) => failure.includes(".gittensory.yml.example") && failure.includes("review.visual.totally_undocumented_nested_field"), + ); + expect(hit).toBeDefined(); + // A check that only enumerated FocusManifest's own TOP-LEVEL fields (never recursing into `review`, let + // alone `review.visual`) could never have produced this path -- proving the recursion is load-bearing, + // not just a nice-to-have, for catching #4617's own concrete gap shape. + expect(extractFocusManifestFields(files["packages/gittensory-engine/src/focus-manifest.ts"])).toContain( + "review.visual.totallyUndocumentedNestedField", + ); + }); + + it("does not flag a FocusManifest field whose real yml key is recorded in FOCUS_MANIFEST_ALIAS_MANIFEST", () => { + // Same shape as the SETTINGS_ALIAS_MANIFEST case above: baseFixtures() alone is already a clean pass, so + // any failure here can only be this new field failing to resolve through its recorded alias. + for (const row of FOCUS_MANIFEST_ALIAS_MANIFEST) { + const files = baseFixtures(); + const leafName = row.field.split(".").pop(); + files["packages/gittensory-engine/src/focus-manifest.ts"] = files["packages/gittensory-engine/src/focus-manifest.ts"]!.replace( + "visual: VisualConfig;", + `${leafName}: string | null;\n visual: VisualConfig;`, + ); + files[".gittensory.yml.example"] += `\n${row.aliases[0]}`; + const result = checkDocsDrift({ root: "/fake", readFile: makeReadFile(files) }); + + expect(result.failures, `${row.field} should pass via alias ${row.aliases[0]}`).toEqual([]); + } + }); + // Most important regression test in this file: proves the REAL current repo state (source files + // docs pages) passes cleanly, using the real filesystem reader against the real repo root. If this // fails, either a real doc gap exists or the extraction logic is broken -- either way, the check must @@ -256,7 +549,9 @@ describe("check-docs-drift script", () => { it("prints a clean summary and exits 0 for the real repo state when run as a subprocess", () => { const output = execFileSync("node", ["scripts/check-docs-drift.mjs"], { encoding: "utf8" }); - expect(output).toMatch(/Docs-drift check ok: \d+ feature flags, \d+ commands, \d+ gate-mode fields all documented\./); + expect(output).toMatch( + /Docs-drift check ok: \d+ feature flags, \d+ commands, \d+ gate-mode fields, \d+ RepositorySettings fields, \d+ FocusManifest fields all documented\./, + ); }); }); });