diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a86fbd9102..49581d1cf6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -428,6 +428,14 @@ jobs: run: |- npm run lint:e2e-model-budget + # Runs independently of test shard selection. Path observers only reduce + # test work; this guard provides the settings synchronization guarantee. + # It must run before build:types, whose declaration-only output cannot be + # executed when the schema follows runtime imports across workspaces. + - name: 'Run settings docs/schema sync guard (#3212)' + run: |- + npm run lint:settings-sync + # Build must run BEFORE the type-aware lint: three workspace tsconfigs # map cross-workspace imports to `dist/*.d.ts` — cli -> tools, core -> mcp, # and a2a-server -> settings/storage/tools — so tsserver (projectService) @@ -436,16 +444,16 @@ jobs: # postinstall only symlinks, see scripts/postinstall.cjs) it does not, so # the build is explicit here. # - # `build:types` emits declarations only (issue #2983). Every step in this - # job reads types and none executes application code, so the transpiled - # `.js` was pure cost here. Do not reuse `build:types` in a job that runs - # the CLI or a Bun test suite: Bun honors tsconfig `paths`, so a - # declaration-only `dist` makes cross-package imports resolve to a - # `.d.ts` with no JavaScript behind it. The release path - # (release.yml -> `npm run build:packages`) still does a full emit, - # because every published library workspace declares `main: - # dist/index.js` and ships `dist`, so npm consumers resolving without the - # `bun` export condition load that JavaScript. + # `build:types` emits declarations only (issue #2983). Every later step in + # this job reads types and none executes application code, so the + # transpiled `.js` was pure cost here. Do not reuse `build:types` in a job + # that runs the CLI or a Bun test suite: Bun honors tsconfig `paths`, so a + # declaration-only `dist` makes cross-package imports resolve to a `.d.ts` + # with no JavaScript behind it. The release path (release.yml -> `npm run + # build:packages`) still does a full emit, because every published library + # workspace declares `main: dist/index.js` and ships `dist`, so npm + # consumers resolving without the `bun` export condition load that + # JavaScript. - name: 'Build declarations for type-aware lint' run: |- npm run build:types diff --git a/docs/cli/configuration.md b/docs/cli/configuration.md index 206d5ccb17..ca1812c7ca 100644 --- a/docs/cli/configuration.md +++ b/docs/cli/configuration.md @@ -234,7 +234,7 @@ In addition to a project settings file, a project's `.llxprt` directory can cont - **Requires restart:** No - **`sessionRetention.maxTotalSizeMB`** (number): - - **Description:** Machine-wide aggregate size limit for all session recordings and cold archives, in MiB. Defaults to 4096 (4 GiB). + - **Description:** Machine-wide aggregate size limit for all session recordings and cold archives, in MiB (see the default property). - **Default:** `4096` - **Requires restart:** No diff --git a/package.json b/package.json index 2cb9b52753..a2f30b6579 100644 --- a/package.json +++ b/package.json @@ -110,6 +110,7 @@ "lint:agents-api-surface": "bun scripts/check-agents-api-surface.ts", "lint:test-shards": "bun scripts/check-test-shards.ts", "lint:affected-shards": "bun scripts/check-affected-test-shards.ts", + "lint:settings-sync": "bun ./scripts/generate-settings-doc.ts --check", "lint:e2e-model-budget": "bun scripts/check-e2e-model-budget.ts --validate-budget", "lint:test-file-coverage": "bun scripts/check-test-file-coverage.ts", "affected-shards": "node scripts/affected-test-shards.ts", diff --git a/packages/cli/src/ui/commands/restoreCommand.ts b/packages/cli/src/ui/commands/restoreCommand.ts index 279e795266..9a66f4b8de 100644 --- a/packages/cli/src/ui/commands/restoreCommand.ts +++ b/packages/cli/src/ui/commands/restoreCommand.ts @@ -50,6 +50,7 @@ const restoreSchema: CommandArgumentSchema = [ return files .filter((file) => file.endsWith('.json')) .map((file) => file.replace(/\.json$/, '')) + .sort() .map((name) => ({ value: name, description: checkpointSuggestionDescription, @@ -187,7 +188,7 @@ async function restoreAction( try { await fs.mkdir(checkpointDir, { recursive: true }); const files = await fs.readdir(checkpointDir); - const jsonFiles = files.filter((file) => file.endsWith('.json')); + const jsonFiles = files.filter((file) => file.endsWith('.json')).sort(); if (!args) { return listCheckpoints(jsonFiles); diff --git a/project-plans/issue3212/PLAN.md b/project-plans/issue3212/PLAN.md new file mode 100644 index 0000000000..064ce56e5d --- /dev/null +++ b/project-plans/issue3212/PLAN.md @@ -0,0 +1,40 @@ +# Plan: Prevent stale generated settings artifacts from reaching release (Issue #3212) + +Plan ID: PLAN-20260812-ISSUE3212 +Generated: 2026-08-12 + +## Root cause and evidence + +The scheduled release runs 31447714495 and 31552306338 both failed in `Run Preflight Checks` while `npm run test:ci` executed the scripts shard. The failing behavioral test was `scripts/tests/generate-settings-doc.test.ts`: `generateDocs(['--check'])` reported both `schemas/settings.schema.json` and `docs/cli/configuration.md` stale and set exit code 1. + +The exact stale generated description originated from `packages/cli/src/config/settings-schema/schema-core.ts`, changed by PR #3201 (commit `62f5fcdf6e`, "Bound recorded sessions with a safe global janitor (Fixes #3164)"). It introduced the recording aggregate size-limit description whose generated documentation text drifted out of sync with the source. Separately, PR #3207 (merge commit `e4e6aa77be`) added `telemetry.perf.enabled` and `telemetry.perf.memory` to the facade `packages/cli/src/config/settingsSchema.ts` and committed a changed JSON schema without regenerating the configuration documentation; that change demonstrated the same selection gap and escaped scripts-shard CI before the first release failure, but it was not the exact stale line. + +PR CI remained green in both cases because affected-test selection mapped package-scoped CLI source changes to the CLI/dependent shards but omitted the scripts shard, even though a scripts-shard synchronization test reads the settings schema sources — both the facade `packages/cli/src/config/settingsSchema.ts` and the modular sources under `packages/cli/src/config/settings-schema/` — without importing them. Consequently, the release workflow was the first post-merge workflow to run that check. + +The schema also consumes runtime defaults from outside those observed CLI paths. For example, `schema-security.ts` imports truncation defaults from `packages/core/src/config/configTypes.ts`. A core-only change selects normal package and dependent shards but can still skip scripts. Path observers therefore improve test selection but cannot be the sole synchronization guarantee. + +## Accepted behavior + +1. Regenerate and commit settings artifacts so the current source schema, `schemas/settings.schema.json`, and `docs/cli/configuration.md` agree. +2. Treat both the settings schema facade (`packages/cli/src/config/settingsSchema.ts`) and every modular source under `packages/cli/src/config/settings-schema/` as explicitly observed inputs of the scripts test shard, via a checked-in path-observer rule (exact path + directory prefix). Any PR changing those sources must select the scripts shard, in addition to normal package-owner/reverse-dependency shards and package observers. This is source-specific, not a package-wide observer, so unrelated CLI files do not select scripts. +3. Keep the selector's existing package observer model and fail-closed behavior. Extend the data shape generically with a `pathObservers` field validated by the checker; do not add a package-wide `scripts` observer. +4. Run the existing generated-settings check unconditionally in the fail-closed JavaScript lint job. This protects transitive inputs that path-based test selection cannot infer and keeps path observers as an optimization rather than the correctness boundary. +5. Validate exact observer paths as repository-relative file paths and directory prefixes as repository-relative directory paths before checking their filesystem targets. +6. Add behavioral coverage for selector behavior, observer validation, and unconditional CI wiring. +7. All changed tests use Bun and `bun:test`. Do not add or modify Vitest/Node suites. + +## Test-first sequence + +1. Add failing selector tests: a PR changing the facade `packages/cli/src/config/settingsSchema.ts` selects both `cli` and `scripts` with a `path-observer` reason; a PR changing a modular source `packages/cli/src/config/settings-schema/schema-core.ts` does the same; and an unrelated CLI production file does NOT select scripts. Assert each via the real selector. +2. Add a generic, checked-in path-observer model (exact paths + directory prefixes) to the affected-test-shard selector and its `GraphData` shape, covering both the facade and the modular schema sources. Extend the checker (`validatePathObservers`) to validate observer identity, selected shard, path shape, and file/directory type. Do not use a package-wide observer. +3. Add an unconditional `lint:settings-sync` step to the JavaScript lint job and test its command, placement, and lack of path or shard conditions against the real workflow. +4. Run the affected-selector tests, workflow wiring tests, drift checker, and generated-settings check. +5. Prove `bun scripts/generate-settings-doc.ts --check` succeeds without changing either generated artifact. +6. Run full repository verification: `npm run test`, `npm run lint`, `npm run typecheck`, `npm run format`, `npm run build`, and `bun scripts/start.ts --profile-load stepfun-37 "write me a haiku and nothing else"`. + +## Scope boundaries + +- No change to settings runtime semantics or telemetry behavior. +- No weakening, skipping, or suppression of generated-artifact checks. +- No lint/complexity threshold changes, ignore additions, eslint disables, or TypeScript suppression directives. +- No release workflow bypass and no unrelated project-plan edits. diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index d464a97cf8..a9db8363aa 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -253,8 +253,8 @@ }, "maxTotalSizeMB": { "title": "Max Total Size (MiB)", - "description": "Machine-wide aggregate size limit for all session recordings and cold archives, in MiB. Defaults to 4096 (4 GiB).", - "markdownDescription": "Machine-wide aggregate size limit for all session recordings and cold archives, in MiB. Defaults to 4096 (4 GiB).\n\n- Category: `General`\n- Requires restart: `no`\n- Default: `4096`", + "description": "Machine-wide aggregate size limit for all session recordings and cold archives, in MiB (see the default property).", + "markdownDescription": "Machine-wide aggregate size limit for all session recordings and cold archives, in MiB (see the default property).\n\n- Category: `General`\n- Requires restart: `no`\n- Default: `4096`", "default": 4096, "type": "number" }, diff --git a/scripts/affected-test-shards.data.json b/scripts/affected-test-shards.data.json index 1a21cb1d39..71ec00071f 100644 --- a/scripts/affected-test-shards.data.json +++ b/scripts/affected-test-shards.data.json @@ -1,6 +1,6 @@ { "version": 1, - "description": "Checked-in package import graph, shard map, and observer rules for affected-test-shard selection (issue #2709). Validated by scripts/check-affected-test-shards.ts against real AST imports.", + "description": "Checked-in package import graph, shard map, package observer rules, and path observer rules (exact paths + directory prefixes) for affected-test-shard selection (issue #2709, #3212). Validated by scripts/check-affected-test-shards.ts against real AST imports and on-disk paths.", "packagePrefix": "@vybestack/llxprt-code-", "packageToShard": { "cli": "cli", @@ -107,6 +107,15 @@ } ] }, + "pathObservers": [ + { + "observingPackage": "scripts", + "selectShard": "scripts", + "reason": "scripts/tests/generate-settings-doc.test.ts reads the cli settings schema source without importing it", + "paths": ["packages/cli/src/config/settingsSchema.ts"], + "pathPrefixes": ["packages/cli/src/config/settings-schema/"] + } + ], "sharedInputs": [ "package.json", "package-lock.json", diff --git a/scripts/affected-test-shards.ts b/scripts/affected-test-shards.ts index 1412407dd2..f8c7ae1175 100644 --- a/scripts/affected-test-shards.ts +++ b/scripts/affected-test-shards.ts @@ -35,6 +35,20 @@ interface ObserverRule { readonly reason: string; } +/** + * Path observer rule: a shard whose tests read specific source paths without + * importing them. Unlike package observers (which fire for any change to an + * observed package), a path observer fires only for exact `paths` or files + * beneath a `pathPrefixes` directory, making it source-specific. + */ +interface PathObserverRule { + readonly observingPackage: string; + readonly selectShard: string; + readonly reason: string; + readonly paths: readonly string[]; + readonly pathPrefixes: readonly string[]; +} + /** The checked-in import graph shape (validated by the checker). */ export interface GraphData { readonly packageToShard: Record; @@ -43,6 +57,7 @@ export interface GraphData { readonly importEdges: Record; readonly testOnlyEdges: Record; readonly observers: Record; + readonly pathObservers: readonly PathObserverRule[]; readonly sharedInputs: readonly string[]; } @@ -83,6 +98,7 @@ export type ReverseGraph = Record; interface SelectionContext { readonly packageToShard: Record; readonly observers: Record; + readonly pathObservers: readonly PathObserverRule[]; readonly reverseGraph: ReverseGraph; readonly sharedSet: Set; } @@ -271,6 +287,7 @@ function buildSelectionContext(data: GraphData): SelectionContext { return { packageToShard: data.packageToShard, observers: data.observers, + pathObservers: data.pathObservers, reverseGraph: buildReverseGraph(data.importEdges, data.testOnlyEdges), sharedSet: new Set(data.sharedInputs), }; @@ -522,6 +539,63 @@ function classifyOtherPath(p: string): PathClassification | null { return null; } +/** + * Returns true when `p` is `dir` itself or resides beneath `dir`, compared at + * path-separator granularity. This is boundary-safe: a sibling directory whose + * name merely shares a textual prefix (e.g. `settings-schema` vs + * `settings-schema-extra`) never matches, regardless of whether `dir` carries + * a trailing slash. The canonical data contract requires a trailing slash + * (enforced by the checker), but matching must not silently overmatch if that + * invariant is ever violated. + */ +function isPathInDirectory(p: string, dir: string): boolean { + const base = dir.endsWith('/') ? dir.slice(0, -1) : dir; + return p === base || p.startsWith(`${base}/`); +} + +/** + * Returns true when a path matches a path-observer rule: an exact match in + * `paths`, or the path resides beneath one of the `pathPrefixes` directories. + * Directory matching is boundary-safe (see {@link isPathInDirectory}) so a + * sibling textual prefix can never overmatch. + */ +function pathObserverMatches(p: string, rule: PathObserverRule): boolean { + if (rule.paths.includes(p)) return true; + for (const prefix of rule.pathPrefixes) { + if (isPathInDirectory(p, prefix)) return true; + } + return false; +} + +/** + * Layers source-specific path observers on top of a base classification. Path + * observers run alongside (never replace) the normal package owner/reverse- + * dependent selection: any matching rule adds its shard and an observer + * reason. A full-run classification already selects every shard, so it is + * returned unchanged. + */ +function applyPathObservers( + p: string, + base: PathClassification, + ctx: SelectionContext, +): PathClassification { + if (base.fullRun) return base; + const reasons: string[] = []; + const shards = new Set(base.shards); + for (const rule of ctx.pathObservers) { + if (!pathObserverMatches(p, rule)) continue; + shards.add(rule.selectShard); + reasons.push( + `path-observer '${rule.observingPackage}' (${rule.selectShard}) scans '${p}'`, + ); + } + if (reasons.length === 0) return base; + return selectShards( + [...shards].sort(), + `${base.reason}; ${reasons.join('; ')}`, + ); +} + function selectPathShards( p: string, ctx: SelectionContext, @@ -536,27 +610,30 @@ function selectPathShards( // 2. Package source change. const pkg = packageFromPath(p); + let base: PathClassification; if (pkg) { - return classifyPackageChange(p, pkg, ctx); - } - - // 3. Scripts harness change selects scripts shard. - if (p.startsWith('scripts/')) { - return selectShards( + base = classifyPackageChange(p, pkg, ctx); + } else if (p.startsWith('scripts/')) { + // 3. Scripts harness change selects scripts shard. + base = selectShards( ['scripts'], `scripts harness change selects scripts shard`, ); + } else { + // 4-10. Other known path categories. + const other = classifyOtherPath(p); + base = + other ?? + // 11. Unknown path → fail closed. + fullRun( + `unknown path '${p}' → fail closed`, + `unknown path '${p}' cannot be classified`, + ); } - // 4-10. Other known path categories. - const other = classifyOtherPath(p); - if (other) return other; - - // 11. Unknown path → fail closed. - return fullRun( - `unknown path '${p}' → fail closed`, - `unknown path '${p}' cannot be classified`, - ); + // Source-specific path observers apply to every non-full-run path + // (package and non-package alike) on top of the base classification. + return applyPathObservers(p, base, ctx); } // --------------------------------------------------------------------------- diff --git a/scripts/check-affected-test-shards.ts b/scripts/check-affected-test-shards.ts index ddea1d79ba..efcf8205dc 100644 --- a/scripts/check-affected-test-shards.ts +++ b/scripts/check-affected-test-shards.ts @@ -28,7 +28,7 @@ * Exits 0 on success, 1 on any drift. */ -import { readFileSync, existsSync } from 'node:fs'; +import { readFileSync, existsSync, statSync } from 'node:fs'; import { join, relative, resolve, dirname, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { execFileSync } from 'node:child_process'; @@ -46,19 +46,28 @@ const PACKAGE_PREFIX = '@vybestack/llxprt-code-'; const TEST_PATH_RE = /(__tests__|\.test\.|\.spec\.|\.bun\.ts$|\/tests\/|\/test-bun\/|\/integration-tests\/|\/test\/)/; -interface ObserverRule { +export interface ObserverRule { readonly observingPackage: string; readonly selectShard: string; readonly reason: string; } -interface GraphData { +export interface PathObserverRule { + readonly observingPackage: string; + readonly selectShard: string; + readonly reason: string; + readonly paths: readonly string[]; + readonly pathPrefixes: readonly string[]; +} + +export interface GraphData { readonly packageToShard: Record; readonly shardOrder: readonly string[]; readonly shardTimingsSeconds: Record; readonly importEdges: Record; readonly testOnlyEdges: Record; readonly observers: Record; + readonly pathObservers: readonly PathObserverRule[]; readonly sharedInputs: readonly string[]; } @@ -225,7 +234,7 @@ function extractAllEdges(repoRoot: string): { return { prodEdges, testOnlyEdges }; } -interface DriftIssue { +export interface DriftIssue { readonly kind: string; readonly detail: string; } @@ -296,7 +305,7 @@ function compareEdges( } /** Builds the canonical workspace → shard map from TEST_SHARDS. */ -function buildCanonicalShardMap(): Map { +export function buildCanonicalShardMap(): Map { const map = new Map(); for (const shard of TEST_SHARDS) { if (shard.isScriptsShard) continue; @@ -375,6 +384,143 @@ function validateShardConfig( return issues; } +/** + * Directory-prefix contract for path-observer `pathPrefixes`: + * - repo-relative (no leading separator, no Windows drive); + * - forward-slash separated (no backslash); + * - ends with a trailing '/' (a directory boundary, never a file); + * - no '.' or '..' path segments (no up-level/self traversal). + * + * A prefix that violates this — most dangerously one missing its trailing + * slash — can overmatch sibling directories via textual prefix comparison, so + * it is rejected here rather than silently tolerated. + */ +export function isValidDirectoryPrefix(prefix: string): boolean { + if (!prefix.endsWith('/')) return false; + if (prefix.startsWith('/')) return false; + if (prefix.includes('\\')) return false; + // Reject Windows drive-qualified forward-slash prefixes (e.g. "C:/..."), + // honoring the documented "no Windows drive" contract. + if (/^[a-z]:\//i.test(prefix)) return false; + const segments = prefix.slice(0, -1).split('/'); + return !segments.some( + (segment) => segment === '' || segment === '..' || segment === '.', + ); +} + +/** + * Repo-relative file-path contract for path-observer exact `paths`: + * - nonempty; + * - repo-relative (no leading separator, no Windows drive); + * - forward-slash separated (no backslash); + * - no trailing '/' (an exact path is a file, not a directory); + * - no '.' or '..' path segments (no self/up-level traversal). + * + * A malformed exact path is rejected here, before any filesystem access, so + * the drift report distinguishes "invalid shape" from "valid shape but not a + * file on disk". + */ +export function isValidExactPath(path: string): boolean { + if (path.length === 0) return false; + if (path.endsWith('/')) return false; + if (path.startsWith('/')) return false; + if (path.includes('\\')) return false; + if (/^[a-z]:\//i.test(path)) return false; + return !path + .split('/') + .some((segment) => segment === '' || segment === '..' || segment === '.'); +} + +/** + * Validates path observer rules: each rule's observing identity must be a known + * package or shard (the scripts harness shard owns no workspace but runs + * tests), selectShard must be in shardOrder, every exact path must be a valid + * repo-relative file that exists on disk, every directory prefix must follow + * the directory-prefix contract and exist on disk, and a rule needs at least + * one path or prefix to match. Exact-path shape is validated by + * {@link isValidExactPath} and directory-prefix shape by + * {@link isValidDirectoryPrefix}. + */ +export function validatePathObservers( + data: GraphData, + canonical: Map, + repoRoot: string, +): DriftIssue[] { + const issues: DriftIssue[] = []; + for (const [i, rule] of data.pathObservers.entries()) { + const observingIsKnown = + canonical.has(rule.observingPackage) || + data.shardOrder.includes(rule.observingPackage); + if (!observingIsKnown) { + issues.push({ + kind: 'path-observer-unknown-observer', + detail: `pathObservers[${i}] references observingPackage '${rule.observingPackage}' which is neither a declared workspace nor a shard name.`, + }); + } + if (!data.shardOrder.includes(rule.selectShard)) { + issues.push({ + kind: 'path-observer-unknown-shard', + detail: `pathObservers[${i}] references selectShard '${rule.selectShard}' which is not in shardOrder.`, + }); + } + if (rule.paths.length === 0 && rule.pathPrefixes.length === 0) { + issues.push({ + kind: 'path-observer-empty', + detail: `pathObservers[${i}] has no paths and no pathPrefixes, so it can never match.`, + }); + } + for (const pathEntry of rule.paths) { + if (!isValidExactPath(pathEntry)) { + issues.push({ + kind: 'path-observer-path-invalid', + detail: `pathObservers[${i}] exact path '${pathEntry}' violates the repo-relative file-path contract: it must be nonempty, repo-relative, forward-slash separated, have no trailing slash, and contain no '.'/'..' path segments.`, + }); + continue; + } + const filePath = join(repoRoot, pathEntry); + let isFile = false; + try { + isFile = statSync(filePath).isFile(); + } catch { + // Filesystem failures are reported below as observer drift. + } + if (!isFile) { + issues.push({ + kind: 'path-observer-path-not-file', + detail: `pathObservers[${i}] references exact path '${pathEntry}' which is not an existing file.`, + }); + } + } + for (const prefix of rule.pathPrefixes) { + if (!isValidDirectoryPrefix(prefix)) { + issues.push({ + kind: 'path-observer-prefix-invalid', + detail: `pathObservers[${i}] pathPrefix '${prefix}' violates the directory-prefix contract: it must be repo-relative, forward-slash separated, end with a trailing '/', and contain no '.'/'..' path segments.`, + }); + continue; + } + const dir = join(repoRoot, prefix); + // The filesystem is external input: a single stat in a narrowly scoped + // try/catch covers missing, non-directory, and race conditions. Report + // any of those as drift rather than throwing. + let isDirectory = false; + try { + isDirectory = statSync(dir).isDirectory(); + } catch { + // Missing path, a stat race, or a non-directory entry — all are + // reported below as path-observer-prefix-not-dir drift. + } + if (!isDirectory) { + issues.push({ + kind: 'path-observer-prefix-not-dir', + detail: `pathObservers[${i}] references pathPrefix '${prefix}' which is not an existing directory.`, + }); + } + } + } + return issues; +} + /** Indispensable shared inputs that MUST appear in sharedInputs. */ const REQUIRED_SHARED_INPUTS: readonly string[] = [ 'package.json', @@ -472,6 +618,7 @@ function checkGraph( const canonical = buildCanonicalShardMap(); issues.push(...validateShardMap(data, canonical)); issues.push(...validateShardConfig(data, canonical)); + issues.push(...validatePathObservers(data, canonical, repoRoot)); issues.push(...validateSharedInputs(data, repoRoot)); issues.push(...validateReverseCompleteness(data, canonical)); @@ -482,16 +629,41 @@ function formatIssue(issue: DriftIssue): string { return ` - [${issue.kind}] ${issue.detail}`; } -function main(): void { - const rootArg = process.argv.find((a) => a === '--root'); - let repoRoot = DEFAULT_REPO_ROOT; - if (rootArg !== undefined) { - const idx = process.argv.indexOf('--root'); - if (idx + 1 < process.argv.length) { - repoRoot = resolve(process.argv[idx + 1]); - } +/** + * Recognized `--name value` CLI options. A value argument that is itself one + * of these tokens (e.g. `--root --data file`) is a missing-value error, not a + * path value. + */ +const RECOGNIZED_OPTIONS: ReadonlySet = new Set(['--root', '--data']); + +/** + * Reads the value of a `--name value` option from `argv`, or returns + * `undefined` when the option is absent. Fail-fast: when the option is present + * but its value is missing or is itself another recognized option token, + * prints a clear error and exits nonzero. + */ +function readOptionValue( + name: string, + argv: readonly string[], +): string | undefined { + const idx = argv.indexOf(name); + if (idx === -1) return undefined; + const value = argv[idx + 1]; + if (value === undefined || RECOGNIZED_OPTIONS.has(value)) { + console.error(`error: ${name} requires a value`); + process.exit(1); } - const dataPath = DEFAULT_DATA_PATH; + return value; +} + +function main(): void { + const rootValue = readOptionValue('--root', process.argv); + const repoRoot = + rootValue !== undefined ? resolve(rootValue) : DEFAULT_REPO_ROOT; + + const dataValue = readOptionValue('--data', process.argv); + const dataPath = + dataValue !== undefined ? resolve(dataValue) : DEFAULT_DATA_PATH; console.log( `affected-test-shards drift guard: scanning ${repoRoot} against ${relative(repoRoot, dataPath)}`, diff --git a/scripts/tests/affected-test-shards-prefix-boundary.test.ts b/scripts/tests/affected-test-shards-prefix-boundary.test.ts new file mode 100644 index 0000000000..cd638ef40a --- /dev/null +++ b/scripts/tests/affected-test-shards-prefix-boundary.test.ts @@ -0,0 +1,120 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Behavioral tests for boundary-safe path-observer prefix matching in the + * affected-test-shard selector (issue #3212). + * + * `pathObserverMatches` must not overmatch sibling directories whose names + * merely share a textual prefix, even when a `pathPrefixes` entry is malformed + * (missing its trailing slash). These tests drive the REAL selector with a + * temp copy of the checked-in graph whose prefix lacks the trailing slash, so + * the boundary-safe matching — not the (checker-enforced) data shape — is the + * property under test. + */ + +import { describe, expect, it } from 'bun:test'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(__dirname, '..', '..'); +const SELECTOR_PATH = join(REPO_ROOT, 'scripts', 'affected-test-shards.ts'); +const DATA_PATH = join(REPO_ROOT, 'scripts', 'affected-test-shards.data.json'); + +interface PathReason { + readonly path: string; + readonly reason: string; + readonly shards: readonly string[]; +} + +interface SelectionResult { + readonly selectedShards: readonly string[]; + readonly skippedShards: readonly string[]; + readonly hasTests: boolean; + readonly coverageComplete: boolean; + readonly fullRunReason: string | null; + readonly pathReasons: readonly PathReason[]; +} + +interface SelectorModule { + selectAffectedShards: (params: { + readonly event: string; + readonly changedPaths: readonly string[]; + readonly dataPath?: string; + }) => SelectionResult; +} + +async function loadSelector(): Promise { + return await import(SELECTOR_PATH); +} + +const PR_EVENT = 'pull_request'; + +/** + * Writes a temp graph derived from the checked-in data whose single + * path-observer prefix is malformed (missing its trailing slash), returning the + * temp file path. All other fields are kept intact so only prefix matching is + * under test. + */ +function writeMalformedPrefixData(dir: string): string { + const dataPath = join(dir, 'data.json'); + const raw = JSON.parse(readFileSync(DATA_PATH, 'utf8')) as Record< + string, + unknown + >; + raw.pathObservers = [ + { + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'malformed no-slash prefix', + paths: ['packages/cli/src/config/settingsSchema.ts'], + pathPrefixes: ['packages/cli/src/config/settings-schema'], + }, + ]; + writeFileSync(dataPath, JSON.stringify(raw)); + return dataPath; +} + +describe('affected-test-shards selector — path-observer prefix boundary safety (issue #3212)', () => { + it('does not overmatch a sibling directory for a no-slash prefix', async () => { + const dir = mkdtempSync(join(tmpdir(), 'affected-shards-prefix-')); + try { + const dataPath = writeMalformedPrefixData(dir); + const { selectAffectedShards } = await loadSelector(); + const result = selectAffectedShards({ + event: PR_EVENT, + changedPaths: ['packages/cli/src/config/settings-schema-other/x.ts'], + dataPath, + }); + expect(result.selectedShards).toContain('cli'); + expect(result.selectedShards).not.toContain('scripts'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('still matches a real descendant for a no-slash prefix', async () => { + const dir = mkdtempSync(join(tmpdir(), 'affected-shards-prefix-')); + try { + const dataPath = writeMalformedPrefixData(dir); + const { selectAffectedShards } = await loadSelector(); + const result = selectAffectedShards({ + event: PR_EVENT, + changedPaths: [ + 'packages/cli/src/config/settings-schema/schema-core.ts', + ], + dataPath, + }); + expect(result.selectedShards).toContain('cli'); + expect(result.selectedShards).toContain('scripts'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/scripts/tests/affected-test-shards.test.ts b/scripts/tests/affected-test-shards.test.ts index f3f5a8f338..50827139af 100644 --- a/scripts/tests/affected-test-shards.test.ts +++ b/scripts/tests/affected-test-shards.test.ts @@ -78,6 +78,7 @@ interface SelectorModule { selectAffectedShards: (params: { readonly event: string; readonly changedPaths: readonly string[]; + readonly dataPath?: string; }) => SelectionResult; replayHistory: (params: { readonly count: number; @@ -91,6 +92,24 @@ async function loadSelector(): Promise { return await import(SELECTOR_PATH); } +/** + * Asserts that a changed path's per-path reason in the selection result is + * present and contains all given keywords (case-insensitive). Extracted from + * duplicated assertion blocks in the path-observer tests (issue #3212). + */ +function expectPathReasonKeywords( + result: SelectionResult, + path: string, + ...keywords: readonly string[] +): void { + const pr = result.pathReasons.find((r) => r.path === path); + expect(pr).toBeDefined(); + const reasonLower = pr!.reason.toLowerCase(); + for (const kw of keywords) { + expect(reasonLower).toContain(kw.toLowerCase()); + } +} + const PR_EVENT = 'pull_request'; const ALL_SHARDS = ['cli', 'agents', 'providers', 'core', 'rest', 'scripts']; @@ -236,6 +255,59 @@ describe('affected-test-shards selector — observer rules', () => { expect(result.selectedShards).toContain('cli'); expect(result.selectedShards).toContain('rest'); }); + + it('selects scripts when the cli settings schema facade source changes (issue #3212)', async () => { + const { selectAffectedShards } = await loadSelector(); + // scripts/tests/generate-settings-doc.test.ts reads the cli settings + // schema source without importing it, so a change to that source must run + // the scripts shard (its synchronization gate) in addition to the normal + // cli owner/observer coverage. + const result = selectAffectedShards({ + event: PR_EVENT, + changedPaths: ['packages/cli/src/config/settingsSchema.ts'], + }); + expect(result.selectedShards).toContain('cli'); + expect(result.selectedShards).toContain('scripts'); + expectPathReasonKeywords( + result, + 'packages/cli/src/config/settingsSchema.ts', + 'Path-Observer', + 'Scripts', + ); + }); + + it('selects scripts when a settings-schema modular source changes (issue #3212)', async () => { + const { selectAffectedShards } = await loadSelector(); + // The generated schema is assembled from modular sources under + // packages/cli/src/config/settings-schema/. A change to any of those + // sources can alter the generated artifacts, so it must also select the + // scripts shard via the same path-observer rule. + const result = selectAffectedShards({ + event: PR_EVENT, + changedPaths: ['packages/cli/src/config/settings-schema/schema-core.ts'], + }); + expect(result.selectedShards).toContain('cli'); + expect(result.selectedShards).toContain('scripts'); + expectPathReasonKeywords( + result, + 'packages/cli/src/config/settings-schema/schema-core.ts', + 'path-observer', + 'scripts', + ); + }); + + it('does NOT select scripts for an unrelated cli production file (issue #3212)', async () => { + const { selectAffectedShards } = await loadSelector(); + // The path-observer rule is source-specific: a generic cli source file + // that the settings-doc test does not read must select only the normal + // cli owner + package-observer coverage, never scripts. + const result = selectAffectedShards({ + event: PR_EVENT, + changedPaths: ['packages/cli/src/commands/chat.ts'], + }); + expect(result.selectedShards).toContain('cli'); + expect(result.selectedShards).not.toContain('scripts'); + }); }); describe('affected-test-shards selector — scripts observation', () => { diff --git a/scripts/tests/check-affected-test-shards.test.ts b/scripts/tests/check-affected-test-shards.test.ts new file mode 100644 index 0000000000..cc1fb5c5e2 --- /dev/null +++ b/scripts/tests/check-affected-test-shards.test.ts @@ -0,0 +1,514 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Behavioral tests for the affected-test-shards drift checker + * (`scripts/check-affected-test-shards.ts`), focusing on the path-observer + * directory-prefix and exact-path contracts (issue #3212). + * + * Two complementary test layers: + * - Direct unit tests of the exported `validatePathObservers` / + * `isValidDirectoryPrefix` / `isValidExactPath` functions: fast, precise, + * and do NOT scan the full repository. + * - Subprocess end-to-end tests: run the real checker binary to verify the + * full CLI pipeline (data loading → validation → error output → exit code), + * including fail-fast CLI parsing for missing `--data` / `--root` values. + */ + +import { describe, expect, it } from 'bun:test'; +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + validatePathObservers, + buildCanonicalShardMap, + isValidDirectoryPrefix, + isValidExactPath, + type GraphData, + type PathObserverRule, +} from '../check-affected-test-shards.ts'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(__dirname, '..', '..'); +const CHECKER_PATH = join( + REPO_ROOT, + 'scripts', + 'check-affected-test-shards.ts', +); +const DATA_PATH = join(REPO_ROOT, 'scripts', 'affected-test-shards.data.json'); +const CANONICAL = buildCanonicalShardMap(); +const CHECKER_TIMEOUT_MS = 30_000; + +/** + * A minimal GraphData fixture: only `pathObservers` is populated; all other + * fields are empty. This is sufficient for `validatePathObservers` which only + * reads `pathObservers` and `shardOrder` from the data object, plus disk + * existence via `repoRoot`. + */ +function makeData(rule: PathObserverRule): GraphData { + return { + packageToShard: {}, + shardOrder: ['cli', 'agents', 'providers', 'core', 'rest', 'scripts'], + shardTimingsSeconds: {}, + importEdges: {}, + testOnlyEdges: {}, + observers: {}, + pathObservers: [rule], + sharedInputs: [], + }; +} + +/** + * Runs the checker as a subprocess with a bounded timeout. Throws with a clear + * message on spawn failure or timeout (SIGTERM) so those failures are never + * misreported as status-code assertion mismatches. + */ +function runChecker(args: readonly string[]): { + readonly status: number | null; + readonly stderr: string; + readonly stdout: string; +} { + const run = spawnSync(process.execPath, [CHECKER_PATH, ...args], { + timeout: CHECKER_TIMEOUT_MS, + }); + if (run.error !== undefined) { + throw new Error(`checker failed to spawn: ${run.error.message}`); + } + if (run.signal === 'SIGTERM') { + throw new Error( + `checker timed out after ${CHECKER_TIMEOUT_MS / 1000}s (killed by SIGTERM)`, + ); + } + return { + status: run.status, + stderr: run.stderr?.toString() ?? '', + stdout: run.stdout?.toString() ?? '', + }; +} + +/** + * Writes a temp data file derived from the checked-in graph with its single + * path observer's `pathPrefixes` replaced by the supplied value, returning the + * temp file path. All other fields (edges, shard map, shared inputs) are kept + * intact so only the prefix contract is under test. + */ +function writeDataWithPrefix(dir: string, prefix: string): string { + const dataPath = join(dir, 'data.json'); + const raw = JSON.parse(readFileSync(DATA_PATH, 'utf8')) as Record< + string, + unknown + >; + raw.pathObservers = [ + { + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'temp prefix for contract test', + paths: ['packages/cli/src/config/settingsSchema.ts'], + pathPrefixes: [prefix], + }, + ]; + writeFileSync(dataPath, JSON.stringify(raw)); + return dataPath; +} + +/** + * Writes a temp data file derived from the checked-in graph with its single + * path observer's `paths` replaced by the supplied value (and no prefixes), + * returning the temp file path. All other fields are kept intact so only the + * exact-path contract is under test. + */ +function writeDataWithExactPath(dir: string, exactPath: string): string { + const dataPath = join(dir, 'data.json'); + const raw = JSON.parse(readFileSync(DATA_PATH, 'utf8')) as Record< + string, + unknown + >; + raw.pathObservers = [ + { + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'temp exact path for contract test', + paths: [exactPath], + pathPrefixes: [], + }, + ]; + writeFileSync(dataPath, JSON.stringify(raw)); + return dataPath; +} + +describe('check-affected-test-shards — isValidDirectoryPrefix (issue #3212)', () => { + it('rejects a prefix missing its trailing slash', () => { + expect( + isValidDirectoryPrefix('packages/cli/src/config/settings-schema'), + ).toBe(false); + }); + + it('accepts a well-formed prefix with a trailing slash', () => { + expect( + isValidDirectoryPrefix('packages/cli/src/config/settings-schema/'), + ).toBe(true); + }); + + it('rejects an absolute prefix', () => { + expect(isValidDirectoryPrefix('/packages/cli/')).toBe(false); + }); + + it('rejects a prefix with backslash separators', () => { + expect(isValidDirectoryPrefix('packages\\cli\\')).toBe(false); + }); + + it('rejects a prefix with duplicate separators', () => { + expect(isValidDirectoryPrefix('packages//cli/')).toBe(false); + }); + + it('rejects a prefix with parent-directory traversal', () => { + expect(isValidDirectoryPrefix('packages/../other/')).toBe(false); + }); + + it('rejects a drive-qualified forward-slash prefix (Windows drive)', () => { + expect(isValidDirectoryPrefix('C:/packages/cli/')).toBe(false); + }); +}); + +describe('check-affected-test-shards — validatePathObservers prefix contract (issue #3212)', () => { + it('reports a path-observer-prefix-invalid issue for a prefix missing its trailing slash', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'malformed prefix', + paths: ['packages/cli/src/config/settingsSchema.ts'], + pathPrefixes: ['packages/cli/src/config/settings-schema'], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const invalid = issues.filter( + (i) => i.kind === 'path-observer-prefix-invalid', + ); + expect(invalid.length).toBe(1); + expect(invalid[0].detail).toContain("trailing '/'"); + }); + + it('produces no issues for a well-formed prefix that exists on disk', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'well-formed prefix', + paths: ['packages/cli/src/config/settingsSchema.ts'], + pathPrefixes: ['packages/cli/src/config/settings-schema/'], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + expect(issues).toEqual([]); + }); + + it('reports path-observer-prefix-not-dir for a prefix that does not exist on disk', () => { + const dir = mkdtempSync(join(tmpdir(), 'checker-nonexistent-')); + try { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'nonexistent prefix', + paths: [], + pathPrefixes: ['packages/cli/src/config/does-not-exist/'], + }); + const issues = validatePathObservers(data, CANONICAL, dir); + const notDir = issues.filter( + (i) => i.kind === 'path-observer-prefix-not-dir', + ); + expect(notDir.length).toBe(1); + expect(notDir[0].detail).toContain('does-not-exist'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('reports path-observer-prefix-not-dir for a prefix pointing at an existing file', () => { + const dir = mkdtempSync(join(tmpdir(), 'checker-nondir-')); + try { + writeFileSync(join(dir, 'afile'), 'x'); + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'file prefix', + paths: [], + pathPrefixes: ['afile/'], + }); + const issues = validatePathObservers(data, CANONICAL, dir); + const notDir = issues.filter( + (i) => i.kind === 'path-observer-prefix-not-dir', + ); + expect(notDir.length).toBe(1); + expect(notDir[0].detail).toContain('afile'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('check-affected-test-shards — isValidExactPath (issue #3212)', () => { + it('accepts a well-formed repo-relative file path', () => { + expect(isValidExactPath('packages/cli/src/config/settingsSchema.ts')).toBe( + true, + ); + }); + + it('accepts a short file path with one segment', () => { + expect(isValidExactPath('package.json')).toBe(true); + }); + + it('accepts a nested file path', () => { + expect(isValidExactPath('a/b/c/d.ts')).toBe(true); + }); + + it('rejects an empty string', () => { + expect(isValidExactPath('')).toBe(false); + }); + + it('rejects a trailing slash (exact path is a file)', () => { + expect(isValidExactPath('packages/cli/')).toBe(false); + }); + + it('rejects an absolute path', () => { + expect(isValidExactPath('/packages/cli/x.ts')).toBe(false); + }); + + it('rejects a Windows drive prefix', () => { + expect(isValidExactPath('C:/packages/cli/x.ts')).toBe(false); + }); + + it('rejects backslash separators', () => { + expect(isValidExactPath('packages\\cli\\x.ts')).toBe(false); + }); + + it('rejects duplicate separators', () => { + expect(isValidExactPath('packages//cli/x.ts')).toBe(false); + }); + + it('rejects parent-directory traversal', () => { + expect(isValidExactPath('packages/../other/x.ts')).toBe(false); + }); + + it('rejects a current-directory segment', () => { + expect(isValidExactPath('./packages/x.ts')).toBe(false); + }); +}); + +describe('check-affected-test-shards — validatePathObservers exact-path contract (issue #3212)', () => { + it('produces no issues for a valid exact path that exists as a file', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'valid exact path', + paths: ['packages/cli/src/config/settingsSchema.ts'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + expect(issues).toEqual([]); + }); + + it('produces no issues for an actual file in a temp root', () => { + const dir = mkdtempSync(join(tmpdir(), 'checker-exact-realfile-')); + try { + writeFileSync(join(dir, 'realfile.ts'), 'x'); + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'real file', + paths: ['realfile.ts'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, dir); + expect(issues).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('reports path-observer-path-invalid for an absolute exact path', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'absolute exact path', + paths: ['/etc/passwd'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const invalid = issues.filter( + (i) => i.kind === 'path-observer-path-invalid', + ); + expect(invalid.length).toBe(1); + }); + + it('reports path-observer-path-invalid for a trailing-slash exact path', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'trailing slash exact path', + paths: ['packages/cli/'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const invalid = issues.filter( + (i) => i.kind === 'path-observer-path-invalid', + ); + expect(invalid.length).toBe(1); + }); + + it('reports path-observer-path-invalid for a backslash exact path', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'backslash exact path', + paths: ['packages\\cli\\x.ts'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const invalid = issues.filter( + (i) => i.kind === 'path-observer-path-invalid', + ); + expect(invalid.length).toBe(1); + }); + + it('reports path-observer-path-invalid for a traversal exact path', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'traversal exact path', + paths: ['packages/../other/x.ts'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const invalid = issues.filter( + (i) => i.kind === 'path-observer-path-invalid', + ); + expect(invalid.length).toBe(1); + }); + + it('reports path-observer-path-not-file for a valid exact path that is a directory', () => { + const dir = mkdtempSync(join(tmpdir(), 'checker-exact-isdir-')); + try { + mkdirSync(join(dir, 'subdir'), { recursive: true }); + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'exact path is a directory', + paths: ['subdir'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, dir); + const notFile = issues.filter( + (i) => i.kind === 'path-observer-path-not-file', + ); + expect(notFile.length).toBe(1); + expect(notFile[0].detail).toContain('subdir'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('reports path-observer-path-not-file for a valid exact path that is missing', () => { + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'missing exact path', + paths: ['does/not/exist.ts'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const notFile = issues.filter( + (i) => i.kind === 'path-observer-path-not-file', + ); + expect(notFile.length).toBe(1); + expect(notFile[0].detail).toContain('does/not/exist.ts'); + }); + + it('does not access the filesystem for an invalid exact path shape', () => { + // A malformed path must be reported as invalid shape without any + // filesystem check, so the issue kind is path-observer-path-invalid + // (never path-observer-path-not-file). + const data = makeData({ + observingPackage: 'scripts', + selectShard: 'scripts', + reason: 'malformed path', + paths: ['/absolute.ts', 'packages/../x.ts'], + pathPrefixes: [], + }); + const issues = validatePathObservers(data, CANONICAL, REPO_ROOT); + const invalid = issues.filter( + (i) => i.kind === 'path-observer-path-invalid', + ); + const notFile = issues.filter( + (i) => i.kind === 'path-observer-path-not-file', + ); + expect(invalid.length).toBe(2); + expect(notFile.length).toBe(0); + }); +}); + +describe('check-affected-test-shards — end-to-end subprocess (issue #3212)', () => { + it('rejects a pathPrefix missing its trailing slash via --data', () => { + const dir = mkdtempSync(join(tmpdir(), 'checker-prefix-contract-')); + try { + // The real directory exists on disk, so an existence-only check would + // accept it; only the directory-prefix shape check catches the missing + // slash. + const dataPath = writeDataWithPrefix( + dir, + 'packages/cli/src/config/settings-schema', + ); + const { status, stderr } = runChecker(['--data', dataPath]); + expect(status).toBe(1); + expect(stderr).toContain('path-observer-prefix-invalid'); + expect(stderr).toContain("trailing '/'"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('rejects a malformed exact path (absolute) via --data', () => { + const dir = mkdtempSync(join(tmpdir(), 'checker-exactpath-contract-')); + try { + const dataPath = writeDataWithExactPath(dir, '/absolute/path.ts'); + const { status, stderr } = runChecker(['--data', dataPath]); + expect(status).toBe(1); + expect(stderr).toContain('path-observer-path-invalid'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('check-affected-test-shards — fail-fast CLI parsing', () => { + it('exits nonzero when --data has no following value', () => { + const { status, stderr } = runChecker(['--data']); + expect(status).toBe(1); + expect(stderr).toContain('--data requires a value'); + }); + + it('exits nonzero when --root has no following value', () => { + const { status, stderr } = runChecker(['--root']); + expect(status).toBe(1); + expect(stderr).toContain('--root requires a value'); + }); + + it('exits nonzero when --root value is another recognized option', () => { + const { status, stderr } = runChecker(['--root', '--data', 'file.json']); + expect(status).toBe(1); + expect(stderr).toContain('--root requires a value'); + }); + + it('exits nonzero when --data value is another recognized option', () => { + const { status, stderr } = runChecker(['--data', '--root', 'repo']); + expect(status).toBe(1); + expect(stderr).toContain('--data requires a value'); + }); +}); diff --git a/scripts/tests/settings-sync-gate.test.ts b/scripts/tests/settings-sync-gate.test.ts new file mode 100644 index 0000000000..003b2b898f --- /dev/null +++ b/scripts/tests/settings-sync-gate.test.ts @@ -0,0 +1,121 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Behavioral test for the settings docs/schema synchronization correctness + * gate (issue #3212). + * + * The settings-sync gate runs the real `generate-settings-doc.ts` in `--check` + * mode so that any drift between the checked-in settings source/schema and the + * generated docs/schema fails CI unconditionally — independent of shard + * selection. Path observers remain an optimization for *skipping* shards; they + * must never be the sole correctness guard. + * + * This test reads the REAL package.json and ci.yml and asserts: + * 1. package.json exposes `lint:settings-sync` executing + * `bun ./scripts/generate-settings-doc.ts --check`. + * 2. The `lint_javascript` job runs `lint:settings-sync` as an unconditional + * step (no path-filtering `if`) BEFORE the declaration-only build. + * + * YAML is parsed with the established `parseWorkflowYaml` convention rather + * than fragile raw-substring ordering. + */ + +import { describe, expect, it } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + parseWorkflowYaml, + jobSteps, + type WorkflowDocument, + type WorkflowStep, +} from './typed-test-helpers.ts'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(__dirname, '..', '..'); + +function readRepoFile(relativePath: string): string { + return readFileSync(join(REPO_ROOT, relativePath), 'utf-8'); +} + +function rootScripts(): Record { + const pkg = JSON.parse(readRepoFile('package.json')) as { + scripts?: Record; + }; + return pkg.scripts ?? {}; +} + +function ciDocument(): WorkflowDocument { + return parseWorkflowYaml(readRepoFile('.github/workflows/ci.yml')); +} + +function lintJavascriptSteps(): WorkflowStep[] { + const doc = ciDocument(); + const job = doc.jobs?.['lint_javascript']; + if (job === undefined) { + throw new Error('ci.yml must contain a lint_javascript job'); + } + return jobSteps(job); +} + +describe('issue #3212 — settings-sync correctness gate', () => { + it('exposes lint:settings-sync running the generator in check mode', () => { + const scripts = rootScripts(); + const sync = scripts['lint:settings-sync']; + expect( + sync, + 'package.json must define a lint:settings-sync script', + ).toBeDefined(); + expect(sync).toBe('bun ./scripts/generate-settings-doc.ts --check'); + }); + + it('runs lint:settings-sync inside the lint_javascript job', () => { + const steps = lintJavascriptSteps(); + const syncSteps = steps.filter((step) => + String(step.run ?? '').includes('npm run lint:settings-sync'), + ); + expect( + syncSteps.length, + 'lint_javascript must invoke npm run lint:settings-sync', + ).toBe(1); + }); + + it('runs lint:settings-sync before the declaration-only build step', () => { + const steps = lintJavascriptSteps(); + const buildIndex = steps.findIndex((step) => + String(step.run ?? '').includes('npm run build:types'), + ); + expect( + buildIndex, + 'lint_javascript must contain an npm run build:types step', + ).toBeGreaterThanOrEqual(0); + + const syncIndex = steps.findIndex((step) => + String(step.run ?? '').includes('npm run lint:settings-sync'), + ); + expect( + syncIndex, + 'lint_javascript must contain an npm run lint:settings-sync step', + ).toBeGreaterThanOrEqual(0); + + expect( + syncIndex, + 'lint:settings-sync must run before build:types replaces runtime output with declarations', + ).toBeLessThan(buildIndex); + }); + + it('runs lint:settings-sync unconditionally (no path-filtering gate)', () => { + const steps = lintJavascriptSteps(); + const syncStep = steps.find((step) => + String(step.run ?? '').includes('npm run lint:settings-sync'), + ); + expect(syncStep, 'lint:settings-sync step must exist').toBeDefined(); + // An unconditional step has no `if:` condition that could gate it on + // changed paths or shard selection. + expect(syncStep?.if).toBeUndefined(); + }); +});