Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
- 'release/**'
merge_group:
workflow_dispatch:
# Manual trigger - no inputs needed, uses repository variables

Check warning on line 16 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

16:5 [comments-indentation] comment not indented like content

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand Down Expand Up @@ -247,14 +247,14 @@
- name: 'Install shellcheck' # Actionlint uses shellcheck
run: |-
mkdir -p "${RUNNER_TEMP}/shellcheck"
curl -sSLo "${RUNNER_TEMP}/.shellcheck.txz" "https://github.com/koalaman/shellcheck/releases/download/v${SHELLCHECK_VERSION}/shellcheck-v${SHELLCHECK_VERSION}.linux.x86_64.tar.xz"

Check warning on line 250 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

250:121 [line-length] line too long (189 > 120 characters)
tar -xf "${RUNNER_TEMP}/.shellcheck.txz" -C "${RUNNER_TEMP}/shellcheck" --strip-components=1
echo "${RUNNER_TEMP}/shellcheck" >> "${GITHUB_PATH}"

- name: 'Install actionlint'
run: |-
mkdir -p "${RUNNER_TEMP}/actionlint"
curl -sSLo "${RUNNER_TEMP}/.actionlint.tgz" "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz"

Check warning on line 257 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

257:121 [line-length] line too long (184 > 120 characters)
tar -xzf "${RUNNER_TEMP}/.actionlint.tgz" -C "${RUNNER_TEMP}/actionlint"
echo "${RUNNER_TEMP}/actionlint" >> "${GITHUB_PATH}"

Expand All @@ -264,7 +264,7 @@
run: |-
actionlint \
-color \
-format "{{range \$err := .}}::error file={{\$err.Filepath}},line={{\$err.Line}},col={{\$err.Column}}::{{\$err.Filepath}}@{{\$err.Line}} {{\$err.Message}}%0A\`\`\`%0A{{replace \$err.Snippet \"\\\\n\" \"%0A\"}}%0A\`\`\`\\n{{end}}" \

Check warning on line 267 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

267:121 [line-length] line too long (243 > 120 characters)
-ignore 'SC2002:' \
-ignore 'SC2016:info' \
-ignore 'SC2129:' \
Expand Down Expand Up @@ -450,6 +450,12 @@
run: |-
npm run build:types

# Runs independently of test shard selection. Path observers only reduce
# test work; this guard provides the settings synchronization guarantee.
- name: 'Run settings docs/schema sync guard (#3212)'
run: |-
npm run lint:settings-sync

- name: 'Run agents API-surface guard'
run: |-
npm run lint:agents-api-surface
Expand All @@ -465,7 +471,7 @@
uses: 'actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9' # ratchet:actions/cache@v6
with:
path: 'node_modules/.cache/eslint'
key: ${{ runner.os }}-eslint-${{ hashFiles('packages/*/src/**', 'packages/*/package.json', 'packages/*/tsconfig.json', 'scripts/**', 'evals/**', 'integration-tests/**', 'eslint.config.js', 'eslint-rules/**', 'package.json', 'package-lock.json', 'bun.lock', 'tsconfig.json', 'tsconfig.scripts.json', '*.js', 'test-setup/**', '.github/workflows/**', '.github/scripts/**') }}

Check warning on line 474 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

474:121 [line-length] line too long (382 > 120 characters)

- name: 'Run linter'
env:
Expand All @@ -482,7 +488,7 @@
if [[ -n "$lint_full_run_reason" ]]; then
echo "::notice::Lint running full: $lint_full_run_reason"
fi
if [[ "$selector_result" == "success" && "$lint_full_run" != "true" && -n "$lint_targets" && "$lint_targets" != "null" ]]; then

Check warning on line 491 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

491:121 [line-length] line too long (137 > 120 characters)
# Scoped run: the runner derives --cache args centrally from the
# LLXPRT_LINT_CACHE env (it also reads the target list from env),
# so no cache flags are passed on the CLI.
Expand Down Expand Up @@ -527,7 +533,7 @@
- name: 'Install shellcheck'
run: |-
mkdir -p "${RUNNER_TEMP}/shellcheck"
curl -sSLo "${RUNNER_TEMP}/.shellcheck.txz" "https://github.com/koalaman/shellcheck/releases/download/v${SHELLCHECK_VERSION}/shellcheck-v${SHELLCHECK_VERSION}.linux.x86_64.tar.xz"

Check warning on line 536 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

536:121 [line-length] line too long (189 > 120 characters)
tar -xf "${RUNNER_TEMP}/.shellcheck.txz" -C "${RUNNER_TEMP}/shellcheck" --strip-components=1
echo "${RUNNER_TEMP}/shellcheck" >> "${GITHUB_PATH}"

Expand Down
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
40 changes: 40 additions & 0 deletions project-plans/issue3212/PLAN.md
Original file line number Diff line number Diff line change
@@ -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.
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