refactor(admission): make local checks change aware #497
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export function shouldRunAdmissionCheck(name: string, files: readonly string[]): boolean; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| const routingAuthority = 'scripts/ci-prepush-check-registry.mjs'; | ||
| const runnerAuthority = 'scripts/ci-prepush-lowend.mjs'; | ||
| const i18nPolicyFiles = new Set([ | ||
| 'scripts/check-i18n-keys.mjs', | ||
| 'scripts/i18n-locales.mjs', | ||
| 'scripts/build-i18n.mjs', | ||
| 'scripts/i18n-quality-report.mjs', | ||
| ]); | ||
|
|
||
| export const admissionCheckRegistry = Object.freeze([ | ||
| { | ||
| name: 'i18n', | ||
| matches: (file) => | ||
| file.startsWith('locales/') || | ||
| file.startsWith('public/locales/') || | ||
| i18nPolicyFiles.has(file), | ||
| implementationFiles: new Set([ | ||
| routingAuthority, | ||
| runnerAuthority, | ||
| 'scripts/ci-prepush-classifier.mjs', | ||
| ]), | ||
| }, | ||
| { | ||
| name: 'contentGuard', | ||
| matches: (file) => | ||
| file === 'scripts/content-guard.mjs' || | ||
| file.startsWith('community-templates/') || | ||
| file.startsWith('public/community-templates/'), | ||
| implementationFiles: new Set([ | ||
| routingAuthority, | ||
| runnerAuthority, | ||
| 'scripts/ci-prepush-classifier.mjs', | ||
| ]), | ||
| }, | ||
| ]); | ||
|
|
||
| export function shouldRunAdmissionCheck(name, files) { | ||
| const entry = admissionCheckRegistry.find((candidate) => candidate.name === name); | ||
| if (!entry) throw new Error(`unknown local admission check: ${name}`); | ||
| return files.some((file) => entry.matches(file) || entry.implementationFiles.has(file)); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| export type ChangeKind = | ||
| | 'NO_CHANGES' | ||
| | 'DOCS_ONLY' | ||
| | 'WORKFLOW_ONLY' | ||
| | 'NON_CODE_ONLY' | ||
| | 'RUST_TAURI' | ||
| | 'TOOLING' | ||
| | 'TEST_ONLY' | ||
| | 'TYPESCRIPT_APPLICATION' | ||
| | 'DEPENDENCY_TOOLCHAIN' | ||
| | 'BUILD_CONFIGURATION' | ||
| | 'AMBIGUOUS' | ||
| | 'MIXED'; | ||
|
|
||
| export interface ChangeClassification { | ||
| readonly kind: ChangeKind; | ||
| readonly categories: readonly string[]; | ||
| readonly files: readonly string[]; | ||
| } | ||
|
|
||
| export function classifyFile(file: string): string; | ||
| export function classifyChangedFiles(files: readonly string[]): ChangeClassification; | ||
| export function requiresTypecheck( | ||
| classification: ChangeClassification, | ||
| options?: { readonly full?: boolean }, | ||
| ): boolean; | ||
| export function manualAdmissionNeedsFullValidation(rangeResolved: boolean): boolean; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| const DOC_FILE = /\.(?:md|mdx)$/i; | ||
| const TS_FILE = /\.(?:c|m)?tsx?$/i; | ||
| const WORKFLOW_ROOTS = ['.github/workflows/', '.github/actions/']; | ||
| const RUST_ROOTS = ['src-tauri/', 'crates/']; | ||
| const TOOLING_ROOTS = ['scripts/']; | ||
| const TOOLING_FILES = new Set(['.gitleaks.toml']); | ||
| const DEPENDENCY_FILES = new Set([ | ||
| 'package.json', | ||
| 'pnpm-lock.yaml', | ||
| 'pnpm-workspace.yaml', | ||
| '.npmrc', | ||
| '.nvmrc', | ||
| 'rust-toolchain', | ||
| 'rust-toolchain.toml', | ||
| ]); | ||
| const BUILD_CONFIG_FILES = new Set([ | ||
| 'biome.json', | ||
| 'index.html', | ||
| 'playwright.config.ts', | ||
| 'postcss.config.js', | ||
| 'postcss.config.mjs', | ||
| 'tailwind.config.js', | ||
| 'tailwind.config.ts', | ||
| 'turbo.json', | ||
| 'vite.config.ts', | ||
| 'vitest.config.ts', | ||
| ]); | ||
|
|
||
| function startsWithRoot(file, roots) { | ||
| return roots.some((root) => file.startsWith(root)); | ||
| } | ||
|
|
||
| function normalizePath(file) { | ||
| return file.replaceAll('\\', '/').replace(/^\.\//, ''); | ||
| } | ||
|
|
||
| function isInstructionFile(file) { | ||
| return ( | ||
| file === 'AGENTS.md' || | ||
| file === 'CLAUDE.md' || | ||
| file === '.cursorrules' || | ||
| file === '.github/copilot-instructions.md' || | ||
| file.startsWith('.cursor/rules/') | ||
| ); | ||
| } | ||
|
|
||
| export function classifyFile(file) { | ||
| const normalized = normalizePath(file); | ||
| const base = normalized.split('/').at(-1) ?? normalized; | ||
|
|
||
| if (startsWithRoot(normalized, WORKFLOW_ROOTS)) return 'WORKFLOW'; | ||
| if (DOC_FILE.test(normalized) || isInstructionFile(normalized)) return 'DOCS'; | ||
| if ( | ||
| RUST_ROOTS.some((root) => normalized.startsWith(root)) || | ||
| /(?:^|\/)(?:Cargo\.toml|Cargo\.lock)$/.test(normalized) || | ||
| normalized.endsWith('.rs') | ||
| ) { | ||
| return 'RUST_TAURI'; | ||
| } | ||
| if (normalized.startsWith('tests/')) | ||
| return TS_FILE.test(normalized) ? 'TYPESCRIPT_APPLICATION' : 'TEST_ONLY'; | ||
| if (TS_FILE.test(normalized)) return 'TYPESCRIPT_APPLICATION'; | ||
| if (TOOLING_FILES.has(normalized) || startsWithRoot(normalized, TOOLING_ROOTS)) return 'TOOLING'; | ||
|
qnbs marked this conversation as resolved.
There was a problem hiding this comment.
When an outgoing change only edits AGENTS.md reference: AGENTS.md:L298-L302 Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Validated on exact E3 head fd697ab914e843fc25b8974893eaa96cf4cef11: this is a current classifier-authority blocker. scripts/coverage-thresholds.json is imported by vitest.config.ts with resolveJsonModule, but scripts/ is broadly TOOLING and requiresTypecheck exempts it. E3 is terminal; no E4/code push is permitted, so this remains open and contributes to STOP_AND_SPLIT_RECOMMENDED. |
||
| if ( | ||
| DEPENDENCY_FILES.has(base) || | ||
| normalized.startsWith('patches/') || | ||
| (normalized.startsWith('packages/') && base === 'package.json') | ||
| ) { | ||
| return 'DEPENDENCY_TOOLCHAIN'; | ||
| } | ||
| if (BUILD_CONFIG_FILES.has(base)) return 'BUILD_CONFIGURATION'; | ||
| return 'UNKNOWN'; | ||
| } | ||
|
|
||
| // QNBS-v3: classify change impact before starting expensive local checks. | ||
| export function classifyChangedFiles(files) { | ||
| const normalizedFiles = [...new Set(files.map(normalizePath).filter(Boolean))].sort(); | ||
| const categories = [...new Set(normalizedFiles.map(classifyFile))]; | ||
|
|
||
| if (normalizedFiles.length === 0) | ||
| return { kind: 'NO_CHANGES', categories, files: normalizedFiles }; | ||
| if (categories.every((category) => category === 'DOCS')) | ||
| return { kind: 'DOCS_ONLY', categories, files: normalizedFiles }; | ||
| if (categories.every((category) => category === 'WORKFLOW')) | ||
| return { kind: 'WORKFLOW_ONLY', categories, files: normalizedFiles }; | ||
| if (categories.length === 1) { | ||
| if (categories[0] === 'UNKNOWN') | ||
| return { kind: 'AMBIGUOUS', categories, files: normalizedFiles }; | ||
| return { kind: categories[0], categories, files: normalizedFiles }; | ||
| } | ||
| if ( | ||
| categories.every((category) => ['DOCS', 'WORKFLOW', 'TOOLING', 'TEST_ONLY'].includes(category)) | ||
| ) | ||
| return { kind: 'NON_CODE_ONLY', categories, files: normalizedFiles }; | ||
| if (categories.includes('UNKNOWN')) | ||
| return { kind: 'AMBIGUOUS', categories, files: normalizedFiles }; | ||
| return { kind: 'MIXED', categories, files: normalizedFiles }; | ||
| } | ||
|
|
||
| export function requiresTypecheck(classification, { full = false } = {}) { | ||
| if (full) return true; | ||
| return ![ | ||
| 'NO_CHANGES', | ||
| 'DOCS_ONLY', | ||
| 'WORKFLOW_ONLY', | ||
| 'NON_CODE_ONLY', | ||
| 'RUST_TAURI', | ||
| 'TOOLING', | ||
| 'TEST_ONLY', | ||
| ].includes(classification.kind); | ||
| } | ||
|
|
||
| export function manualAdmissionNeedsFullValidation(rangeResolved) { | ||
| return !rangeResolved; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,16 @@ | ||
| import { spawnSync } from 'node:child_process'; | ||
| import process from 'node:process'; | ||
| import { shouldRunAdmissionCheck } from './ci-prepush-check-registry.mjs'; | ||
| import { | ||
| classifyChangedFiles, | ||
| manualAdmissionNeedsFullValidation, | ||
| requiresTypecheck, | ||
| } from './ci-prepush-classifier.mjs'; | ||
| import { ensureDependencyState, runLocalBinary, runNodeScript } from './hooks/shared.mjs'; | ||
| import { readPrePushEvidenceFile, resolvePushEvidence } from './signing/signing-core.mjs'; | ||
|
|
||
| const evidenceIndex = process.argv.indexOf('--prepush-evidence-file'); | ||
| let evidenceChangedFiles; | ||
| if (evidenceIndex >= 0) { | ||
| try { | ||
| const evidence = resolvePushEvidence( | ||
|
|
@@ -11,49 +19,118 @@ if (evidenceIndex >= 0) { | |
| ); | ||
| if (evidence.evidenceState !== 'RESOLVED') | ||
| throw new Error(evidence.reason ?? 'invalid evidence'); | ||
| evidenceChangedFiles = evidence.changedFiles; | ||
| } catch (error) { | ||
| console.error(`[local-lowend] outgoing evidence rejected: ${error.message}`); | ||
| process.exit(1); | ||
| } | ||
| } | ||
|
|
||
| const fullRequested = process.argv.includes('--full'); | ||
|
|
||
| function gitRaw(args) { | ||
| const result = spawnSync('git', args, { encoding: 'utf8' }); | ||
| if (result.status !== 0) return ''; | ||
| return result.stdout ?? ''; | ||
| } | ||
|
|
||
| function gitOptional(args) { | ||
| const result = spawnSync('git', args, { encoding: 'utf8' }); | ||
| return result.status === 0 ? (result.stdout ?? '') : null; | ||
| } | ||
|
|
||
| function parseNulDelimitedPaths(output) { | ||
| return output.split('\0').filter(Boolean); | ||
| } | ||
|
|
||
| function changedFilesFromWorkingTree() { | ||
| return parseNulDelimitedPaths( | ||
| gitRaw(['diff', '--no-renames', '--name-only', '-z', 'HEAD']), | ||
| ).concat(parseNulDelimitedPaths(gitRaw(['ls-files', '--others', '--exclude-standard', '-z']))); | ||
|
coderabbitai[bot] marked this conversation as resolved.
qnbs marked this conversation as resolved.
|
||
| } | ||
|
qnbs marked this conversation as resolved.
|
||
|
|
||
| function changedFilesFromManualRange() { | ||
| const upstream = gitOptional(['rev-parse', '--verify', '@{upstream}'])?.trim(); | ||
| if (upstream) | ||
| return { | ||
| files: parseNulDelimitedPaths( | ||
| gitRaw(['diff', '--no-renames', '--name-only', '-z', `${upstream}..HEAD`]), | ||
| ).concat(changedFilesFromWorkingTree()), | ||
| rangeResolved: true, | ||
| }; | ||
|
|
||
| return { files: [], rangeResolved: false }; | ||
|
Comment on lines
+52
to
+62
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n scripts/ci-prepush-lowend.mjs | sed -n '1,180p'
printf '%s\n' '--- bound helpers and callers ---'
rg -n -A18 -B8 'function (gitRaw|gitOptional|changedFilesFromManualRange)|changedFilesFromManualRange\(' scripts/ci-prepush-lowend.mjs
printf '%s\n' '--- rangeResolved consumers and NO_CHANGES handling ---'
rg -n -A20 -B12 'rangeResolved|NO_CHANGES|changedFilesFrom' scripts/ci-prepush-lowend.mjsRepository: qnbs/WorldScript-Studio Length of output: 12100 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- classifier definitions ---'
cat -n scripts/ci-prepush-classifier.mjs | sed -n '1,260p'
printf '%s\n' '--- registry definitions ---'
cat -n scripts/ci-prepush-check-registry.mjs | sed -n '1,220p'
printf '%s\n' '--- focused tests ---'
rg -n -A18 -B8 'manual|rangeResolved|NO_CHANGES|classifyChangedFiles|requiresTypecheck|contentGuard|i18n' scripts test tests 2>/dev/null | head -n 260 || trueRepository: qnbs/WorldScript-Studio Length of output: 26047 Fail closed when the committed Git diff fails. When 🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Validated on exact E3 head fd697ab914e843fc25b8974893eaa96cf4cef11: this is a current fail-closed S1 blocker. When upstream resolves but git diff fails, gitRaw returns an empty list while rangeResolved remains true, allowing NO_CHANGES. E3 is terminal, so no E4/code push is permitted; this remains open and is classified STOP_AND_SPLIT_RECOMMENDED. There was a problem hiding this comment.
If E3 is terminal. No E4 code push should occur. The follow-up must fail closed for a committed-diff failure and add regression coverage before this thread can be addressed. 🐇 You are interacting with an AI system. |
||
| } | ||
|
|
||
| function report(name, status, detail = '') { | ||
| console.log(`[local-admission] ${name.padEnd(26)} ${status}${detail ? ` — ${detail}` : ''}`); | ||
| return status; | ||
| } | ||
|
|
||
| function runCheck(name, run) { | ||
| const status = run(); | ||
| report(name, status === 0 ? 'PASS' : 'FAIL'); | ||
| if (status !== 0) process.exit(status ?? 1); | ||
| } | ||
|
|
||
| const manualEvidence = | ||
| evidenceIndex >= 0 | ||
| ? { files: evidenceChangedFiles, rangeResolved: true } | ||
| : changedFilesFromManualRange(); | ||
|
Comment on lines
+76
to
+79
There was a problem hiding this comment.
When the only outgoing ref is an annotated tag, AGENTS.md reference: AGENTS.md:L386-L391 Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Validated on exact E3 head fd697ab914e843fc25b8974893eaa96cf4cef11: this is a distinct current S3a evidence-consumption blocker. Tag-only PushEvidence resolves with no changedFiles, so local admission can classify NO_CHANGES and skip required checks for a tag target. This is outside the authorized S1 E3 scope; no E4/code push is permitted and the thread remains open for the replacement S3a/S1 follow-up. |
||
| const full = | ||
| fullRequested || (!manualEvidence.rangeResolved && manualAdmissionNeedsFullValidation(false)); | ||
| const files = manualEvidence.files; | ||
| const classification = manualEvidence.rangeResolved | ||
| ? classifyChangedFiles(files) | ||
| : { kind: 'AMBIGUOUS', categories: ['UNKNOWN'], files: [] }; | ||
| const typecheckRequired = requiresTypecheck(classification, { full }); | ||
|
|
||
| console.log(`[local-admission] change class: ${classification.kind}`); | ||
| console.log(`[local-admission] files considered: ${classification.files.length}`); | ||
| if (!manualEvidence.rangeResolved && evidenceIndex < 0) | ||
| console.log( | ||
| '[local-admission] manual committed range unresolved; using conservative full admission', | ||
| ); | ||
|
|
||
| if (!ensureDependencyState()) { | ||
| report('Dependency state', 'FAIL'); | ||
| process.exit(1); | ||
| } | ||
| report('Dependency state', 'PASS'); | ||
|
|
||
| runCheck('Toolchain', () => runNodeScript('scripts/check-pnpm-toolchain.mjs', ['--hook'])); | ||
| runCheck('Docs/release truth', () => runNodeScript('scripts/check-doc-metrics.mjs')); | ||
| runCheck('CSP policy', () => runNodeScript('scripts/check-csp-policy.mjs')); | ||
| runCheck('Desktop import boundary', () => runNodeScript('scripts/check-tauri-import-boundary.mjs')); | ||
| runCheck('Native readiness', () => runNodeScript('scripts/check-native-readiness.mjs')); | ||
|
|
||
| if (shouldRunAdmissionCheck('i18n', classification.files) || full) { | ||
| runCheck('i18n key parity', () => runNodeScript('scripts/check-i18n-keys.mjs')); | ||
| runCheck('i18n bundle rebuild', () => runNodeScript('scripts/build-i18n.mjs')); | ||
| runCheck('i18n translation quality', () => | ||
| runNodeScript('scripts/i18n-quality-report.mjs', [ | ||
| '--strict', | ||
| '--min-coverage', | ||
| '75', | ||
| '--max-length-outliers', | ||
| '8', | ||
| ]), | ||
| ); | ||
| } | ||
|
|
||
| if (shouldRunAdmissionCheck('contentGuard', classification.files) || full) | ||
| runCheck('Content guard', () => runNodeScript('scripts/content-guard.mjs')); | ||
|
|
||
| if (typecheckRequired) { | ||
| runCheck('TypeScript (single checker)', () => | ||
| // QNBS-v3: one checker bounds memory use on constrained developer machines. | ||
| runLocalBinary('tsgo', ['--project', 'tsconfig.tsgo.json', '--noEmit', '--checkers', '1']), | ||
|
qnbs marked this conversation as resolved.
|
||
| ); | ||
| } else { | ||
| report('TypeScript', 'DEFERRED_TO_REQUIRED_CI', 'no TypeScript-impacting changes detected'); | ||
| } | ||
|
|
||
| console.log('\nLOCAL ADMISSION RESULT'); | ||
| console.log('Local checks completed sequentially.'); | ||
| console.log('Cloud validation required YES'); | ||
| console.log(`Classification ${classification.kind}`); | ||
Uh oh!
There was an error while loading. Please reload this page.