Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 18 additions & 10 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 @@ -428,6 +428,14 @@
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)
Expand All @@ -436,16 +444,16 @@
# 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
Expand All @@ -465,7 +473,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 476 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

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

- name: 'Run linter'
env:
Expand All @@ -482,7 +490,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 493 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

493: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 +535,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 538 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

538: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
3 changes: 2 additions & 1 deletion packages/cli/src/ui/commands/restoreCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
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