Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/cli/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
35 changes: 35 additions & 0 deletions project-plans/issue3212/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# 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.

## 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 (too broad), a special CI workflow step, or duplicate settings-generation logic.
4. Add behavioral selector coverage proving both the facade and a modular schema source select scripts via a path-observer reason, and that an unrelated CLI production file does not select scripts, and retain the existing check-mode test as the end-to-end synchronization gate.
5. 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, and on-disk existence of paths/prefixes. Do not use a package-wide observer (too broad) or a hard-coded workflow exception.
3. Run the affected-selector tests, the drift checker, and the generated-settings check-mode test.
4. Keep settings artifacts canonical; prove `bun scripts/generate-settings-doc.ts --check` succeeds.
5. 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.
4 changes: 2 additions & 2 deletions schemas/settings.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
11 changes: 10 additions & 1 deletion scripts/affected-test-shards.data.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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",
Expand Down
107 changes: 92 additions & 15 deletions scripts/affected-test-shards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>;
Expand All @@ -43,6 +57,7 @@ export interface GraphData {
readonly importEdges: Record<string, readonly string[]>;
readonly testOnlyEdges: Record<string, readonly string[]>;
readonly observers: Record<string, readonly ObserverRule[]>;
readonly pathObservers: readonly PathObserverRule[];
readonly sharedInputs: readonly string[];
}

Expand Down Expand Up @@ -83,6 +98,7 @@ export type ReverseGraph = Record<string, readonly string[]>;
interface SelectionContext {
readonly packageToShard: Record<string, string>;
readonly observers: Record<string, readonly ObserverRule[]>;
readonly pathObservers: readonly PathObserverRule[];
readonly reverseGraph: ReverseGraph;
readonly sharedSet: Set<string>;
}
Expand Down Expand Up @@ -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<string>(data.sharedInputs),
};
Expand Down Expand Up @@ -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<string>(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,
Expand All @@ -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);
}

// ---------------------------------------------------------------------------
Expand Down
Loading
Loading