refactor(devops): reconstruct change-aware local admission #492
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export function shouldRunAdmissionCheck(name: string, files: readonly string[]): boolean; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| const routingAuthority = 'scripts/ci-prepush-check-registry.mjs'; | ||
| const i18nPolicyFiles = new Set(['scripts/check-i18n-keys.mjs', 'scripts/i18n-locales.mjs']); | ||
|
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Route all i18n check implementations. Line 2 omits Add both files to 🤖 Prompt for AI AgentsThere was a problem hiding this comment.
If a change touches only AGENTS.md reference: AGENTS.md:L298-L303 Useful? React with 👍 / 👎. |
||
|
|
||
| export const admissionCheckRegistry = Object.freeze([ | ||
|
Comment on lines
+1
to
+4
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add the required QNBS-v3 why-comments.
As per coding guidelines, add “one single-line 📍 Affects 3 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| { | ||
| name: 'i18n', | ||
| matches: (file) => | ||
| file.startsWith('locales/') || | ||
| file.startsWith('public/locales/') || | ||
| i18nPolicyFiles.has(file), | ||
| implementationFiles: new Set([routingAuthority]), | ||
| }, | ||
| { | ||
| name: 'contentGuard', | ||
| matches: (file) => | ||
| file === 'scripts/content-guard.mjs' || | ||
| file.startsWith('community-templates/') || | ||
| file.startsWith('public/community-templates/'), | ||
| implementationFiles: new Set([routingAuthority]), | ||
| }, | ||
| ]); | ||
|
|
||
| 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)); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| 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; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| const DOC_FILE = /\.(?:md|mdx)$/i; | ||
| const TS_FILE = /\.(?:c|m)?tsx?$|\.(?:c|m)?jsx?$/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'; | ||
| } | ||
|
There was a problem hiding this comment.
When a developer changes a TypeScript file under AGENTS.md reference: AGENTS.md:L32-L38 Useful? React with 👍 / 👎. |
||
| if (normalized.startsWith('tests/')) | ||
| return TS_FILE.test(normalized) ? 'TYPESCRIPT_APPLICATION' : 'TEST_ONLY'; | ||
| if (TOOLING_FILES.has(normalized) || startsWithRoot(normalized, TOOLING_ROOTS)) return 'TOOLING'; | ||
|
There was a problem hiding this comment. Suggestion: Files under Severity Level: Major
|
||
| 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'; | ||
| if (TS_FILE.test(normalized)) return 'TYPESCRIPT_APPLICATION'; | ||
| return 'UNKNOWN'; | ||
| } | ||
|
|
||
| // QNBS-v3: classify the outgoing 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); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,83 @@ | ||
| import { spawnSync } from 'node:child_process'; | ||
| import process from 'node:process'; | ||
| import { shouldRunAdmissionCheck } from './ci-prepush-check-registry.mjs'; | ||
| import { classifyChangedFiles, requiresTypecheck } from './ci-prepush-classifier.mjs'; | ||
| import { ensureDependencyState, runLocalBinary, runNodeScript } from './hooks/shared.mjs'; | ||
|
|
||
| const full = process.argv.includes('--full'); | ||
|
|
||
| function gitRaw(args) { | ||
| const result = spawnSync('git', args, { encoding: 'utf8' }); | ||
| if (result.status !== 0) return ''; | ||
| return result.stdout ?? ''; | ||
| } | ||
|
Comment on lines
+9
to
+13
There was a problem hiding this comment. Suggestion: Converting any failed Git command into an empty string makes Git errors indistinguishable from a successful command with no paths. Severity Level: Major
|
||
|
|
||
| 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']))); | ||
|
Comment on lines
+21
to
+22
There was a problem hiding this comment.
On an ordinary push from a clean worktree, this compares AGENTS.md reference: AGENTS.md:L32-L38 Useful? React with 👍 / 👎. |
||
| } | ||
|
Comment on lines
+19
to
+23
There was a problem hiding this comment. Suggestion: The pre-push hook invokes this script after the outgoing commit is already Severity Level: Major
|
||
|
|
||
| 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 files = changedFilesFromWorkingTree(); | ||
| const classification = classifyChangedFiles(files); | ||
| const typecheckRequired = requiresTypecheck(classification, { full }); | ||
|
|
||
| console.log(`[local-admission] change class: ${classification.kind}`); | ||
| console.log(`[local-admission] files considered: ${classification.files.length}`); | ||
|
|
||
| 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)', () => | ||
| runLocalBinary('tsgo', ['--project', 'tsconfig.tsgo.json', '--noEmit', '--checkers', '1']), | ||
| ); | ||
| } 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}`); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The i18n admission route omits
scripts/build-i18n.mjsandscripts/i18n-quality-report.mjs, although the low-end admission gate runs both checks. Modifying either implementation is classified only as generic tooling and therefore skips the i18n checks, allowing changes to the checker or bundle-generation logic to pass local admission without exercising the affected validation. Add all i18n gate implementation files to this policy-file set. [api mismatch]Severity Level: Major⚠️
Prompt for AI Agent 🤖