From ae99843c140083f150af1ff81cbacfb2f70fb310 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Tue, 15 Sep 2026 07:26:32 -0700 Subject: [PATCH] build: discover owned runtime modules in the formatter gate --- CONTRIBUTING.md | 13 +-- docs/CODE-BOUNDARIES.md | 16 +++- scripts/format-runtime.json | 3 + scripts/format-scope.json | 3 +- scripts/format.mjs | 59 ++++++++++++- src/cameraHandoff.test.mjs | 21 ++--- src/data/detectionPresentation.test.mjs | 2 +- src/data/detectionRenderDemand.test.mjs | 8 +- src/firstRunExperience.test.mjs | 8 +- src/hudAltitudeDatum.test.mjs | 2 +- src/radioMarkup.test.mjs | 2 +- src/tooling/format.test.mjs | 90 ++++++++++++++++++++ src/voice/aircraftIdentityNarration.test.mjs | 2 +- 13 files changed, 193 insertions(+), 36 deletions(-) create mode 100644 scripts/format-runtime.json diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b37aa0559..a58227d8e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,12 +64,13 @@ The highest-leverage places to jump in: ## Formatting and reusable components -Run `npm run format` before submitting changes to adopted modules, then -`npm run format:check` and `npm run check:boundaries`. Formatting uses the -explicit file list in `scripts/format-scope.json`; add newly extracted modules -and tests there in a separate mechanical commit. Keep unadopted files consistent -with their surrounding style. CI checks all adopted files and package boundaries -on Linux and Windows. +Run `npm run format` before submitting changes, then `npm run format:check` +and `npm run check:boundaries`. Runtime JavaScript under the owned roots in +`scripts/format-runtime.json` is discovered automatically, including new files. +Tests and other adopted files remain listed in `scripts/format-scope.json`. +Git-ignored files and `.prettierignore` exclusions are not automatically adopted. +Keep mechanical formatting separate from behavioral edits. CI checks formatting +and package boundaries on Linux and Windows. Reusable package exports own their state and receive application operations through explicit callbacks. They must not import the standalone bootstrap or diff --git a/docs/CODE-BOUNDARIES.md b/docs/CODE-BOUNDARIES.md index 41b674996..06e391adf 100644 --- a/docs/CODE-BOUNDARIES.md +++ b/docs/CODE-BOUNDARIES.md @@ -1,16 +1,19 @@ # Formatting and component boundaries -Run `npm run format` to format the files in `scripts/format-scope.json` and +Run `npm run format` to format the files in `scripts/format-scope.json` plus +automatically discovered runtime JavaScript in `scripts/format-runtime.json`, and `npm run format:check` to check that same list without writing. CI checks the -entire adopted list on Linux and Windows. Prettier is pinned in the development +combined scope on Linux and Windows. Prettier is pinned in the development dependencies; use the installed version so local and CI output agree. The shared configuration specifies two spaces, single quotes, semicolons and LF endings. -Add new reusable modules and their tests to the list as they are extracted. +Owned runtime modules are discovered automatically; add their tests and other +non-runtime files to the explicit list as they are adopted. Keep mechanical formatting in its own commit after behavior is stable. Existing source-text regression assertions still apply; investigate failures and preserve their behavioral coverage when a move or line wrap changes a tested shape. -Files outside the list retain their surrounding style until deliberately adopted. +Files outside the runtime roots and explicit list retain their surrounding style +until deliberately adopted. Generated output, local configuration, browser evidence and bundled datasets are excluded. The formatter validates every entry before writing any file. @@ -598,3 +601,8 @@ server adapters without importing search composition. `./sources/overpass` exports the quoted-string/comment lexer. The lexer alone is not a query validator: spatial bounds, timeouts and other policy remain in the server sanitizer. Existing server imports keep their compatibility exports. + +Runtime formatting discovers tracked and new non-ignored `.js`, `.mjs` and `.cjs` +files under the configured owned roots. Tests retain explicit adoption. Git and +Prettier exclusions keep local/generated data out; resolved paths are validated +before any writes. New runtime modules do not need another scope-list entry. diff --git a/scripts/format-runtime.json b/scripts/format-runtime.json new file mode 100644 index 000000000..050b973f8 --- /dev/null +++ b/scripts/format-runtime.json @@ -0,0 +1,3 @@ +{ + "roots": ["src", "server"] +} diff --git a/scripts/format-scope.json b/scripts/format-scope.json index ad13e1b96..7f28a6fbc 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -722,5 +722,6 @@ "scripts/qa-transit-scenes.mjs", "scripts/qa-transit-controls.mjs", "src/tooling/transitQa.test.mjs", - "scripts/qa-transit-browser.mjs" + "scripts/qa-transit-browser.mjs", + "scripts/format-runtime.json" ] diff --git a/scripts/format.mjs b/scripts/format.mjs index b74dfbdcd..25311ba29 100644 --- a/scripts/format.mjs +++ b/scripts/format.mjs @@ -1,9 +1,58 @@ +import { execFileSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; import { readFile, writeFile, realpath, stat } from 'node:fs/promises'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import * as prettier from 'prettier'; -/** Check or format the explicitly adopted files, without scanning other source. */ +/** Discover owned runtime code; tests and other files retain explicit adoption. */ +export async function discoverRuntimeFormatFiles(root) { + const configFile = path.join(root, 'scripts/format-runtime.json'); + if (!existsSync(configFile)) return []; + const { roots } = JSON.parse(await readFile(configFile, 'utf8')); + if ( + !Array.isArray(roots) || + !roots.length || + new Set(roots).size !== roots.length || + roots.some( + (name) => + typeof name !== 'string' || + !/^[a-zA-Z0-9_-]+(?:\/[a-zA-Z0-9_-]+)*$/.test(name), + ) + ) + throw new Error( + 'Runtime formatting roots must be unique repository directory paths', + ); + const candidates = execFileSync( + 'git', + [ + 'ls-files', + '-z', + '--cached', + '--others', + '--exclude-standard', + '--', + ...roots, + ], + { cwd: root, encoding: 'utf8' }, + ).split('\0'); + const files = []; + for (const name of new Set(candidates)) { + if ( + !/\.[cm]?js$/.test(name) || + /\.test\.[cm]?js$/.test(name) || + !existsSync(path.join(root, name)) + ) + continue; + const info = await prettier.getFileInfo(path.join(root, name), { + ignorePath: path.join(root, '.prettierignore'), + }); + if (!info.ignored) files.push(name); + } + return files.sort(); +} + +/** Check or format explicit adoption plus automatically discovered runtime code. */ export async function formatAdoptedFiles(root, mode) { if (!['--check', '--write'].includes(mode)) { throw new Error('Usage: node scripts/format.mjs --check|--write'); @@ -22,9 +71,13 @@ export async function formatAdoptedFiles(root, mode) { ); } + const names = [ + ...new Set([...scope, ...(await discoverRuntimeFormatFiles(root))]), + ]; + // Validate the whole scope before any write, including resolved symlink paths. const files = []; - for (const name of scope) { + for (const name of names) { if ( typeof name !== 'string' || !name || @@ -91,7 +144,7 @@ if (import.meta.url === invoked) { process.exitCode = 1; } else { console.log( - `${mode === '--write' ? 'Formatted' : 'Checked'} ${result.count} adopted files.`, + `${mode === '--write' ? 'Formatted' : 'Checked'} ${result.count} source files.`, ); } } catch (error) { diff --git a/src/cameraHandoff.test.mjs b/src/cameraHandoff.test.mjs index 9ba486f55..4551d830c 100644 --- a/src/cameraHandoff.test.mjs +++ b/src/cameraHandoff.test.mjs @@ -31,7 +31,8 @@ function body(source, pattern, label) { function ordered(source, needles, label) { let previous = -1; for (const needle of needles) { - const index = source.indexOf(needle, previous + 1); + const match = needle instanceof RegExp ? needle.exec(source.slice(previous + 1)) : null; + const index = needle instanceof RegExp ? (match ? previous + 1 + match.index : -1) : source.indexOf(needle, previous + 1); assert.ok(index >= 0, `${label}: missing ${needle}`); assert.ok(index > previous, `${label}: ${needle} is out of order`); previous = index; @@ -104,7 +105,7 @@ test('voice Cockpit entry reaches the camera only through stamping seams', () => 'Cockpit entry transaction', ); ordered(transaction, [ - 'if (!selectedLayer.trackById?.(selectedTarget.id, { origin: selectionOrigin })) {', + /if \(\s*!selectedLayer\.trackById\?\.\(selectedTarget\.id, \{\s*origin: selectionOrigin,?\s*\}\)\s*\) \{/, 'if (!entryError) entered = Boolean(cockpitView.enter());', ], 'Cockpit entry transaction'); // No direct camera control: every mutation goes through a layer tracker or @@ -200,11 +201,11 @@ test('accepted navigation releases through PR15-aware ownership before flight', test('validated voice camera destinations share the UI navigation authority facade', () => { assert.match(ui, /runImmediateNavigation\(noun, navigate, releaseOptions = undefined\) \{\s*return this\._runExplicitNavigation\(noun, navigate, releaseOptions\);/); - assert.match(voice, /runManagedVoiceNavigation\(\s*styleManager, 'camera', 'move_camera', navigate, releaseOptions/); - assert.match(voice, /runManagedVoiceNavigation\(styleManager, 'route', 'fly_route', navigate/); - assert.match(voice, /runManagedVoiceNavigation\(styleManager, 'fire', 'track_entity'/); - assert.match(voice, /runManagedVoiceNavigation\(styleManager, family\.kind, 'track_entity'/); - assert.match(voice, /runManagedVoiceNavigation\(styleManager, 'frame', 'frame_overhead'/); + assert.match(voice, /runManagedVoiceNavigation\(\s*styleManager,\s*'camera',\s*'move_camera',\s*navigate,\s*releaseOptions/); + assert.match(voice, /runManagedVoiceNavigation\(\s*styleManager,\s*'route',\s*'fly_route',\s*navigate/); + assert.match(voice, /runManagedVoiceNavigation\(\s*styleManager,\s*'fire',\s*'track_entity'/); + assert.match(voice, /runManagedVoiceNavigation\(\s*styleManager,\s*family\.kind,\s*'track_entity'/); + assert.match(voice, /runManagedVoiceNavigation\(\s*styleManager,\s*'frame',\s*'frame_overhead'/); const trackedVoice = voice.slice( voice.indexOf('async function trackEntity'), voice.indexOf('async function frameOverhead'), @@ -224,20 +225,20 @@ test('validated voice camera destinations share the UI navigation authority faca ordered(move, [ "if (!['orbit', 'pan', 'tilt', 'rotate'].includes(motion))", 'const start = () => {', - "const preserveCameraFlight = motion === 'orbit'", + /const preserveCameraFlight =\s*motion === 'orbit'/, 'runNavigation(start, { preserveCameraFlight })', ], 'move validation before handoff'); const route = body( cameraVerbs, - /export function flyRoute\(annoList, args = \{\}, floorFn = null, runNavigation = null, warmFn = null\) \{([\s\S]*?)\n\}/, + /export function flyRoute\(\s*annoList,\s*args = \{\},\s*floorFn = null,\s*runNavigation = null,\s*warmFn = null,?\s*\) \{([\s\S]*?)\n\}/, 'fly route', ); ordered(route, [ 'if (!routes.length)', 'const pts = route.path.map', 'const start = () => {', - "return typeof runNavigation === 'function' ? runNavigation(start) : start();", + /return typeof runNavigation === 'function'\s*\? runNavigation\(start\)\s*: start\(\);/, ], 'route validation before handoff'); // The corridor warm is injected the same way the floor READ is — the dolly // never reaches into the data layer itself, and the voice dispatch is the one diff --git a/src/data/detectionPresentation.test.mjs b/src/data/detectionPresentation.test.mjs index ff2102e0d..2ef2f51f9 100644 --- a/src/data/detectionPresentation.test.mjs +++ b/src/data/detectionPresentation.test.mjs @@ -17,7 +17,7 @@ test('Cockpit reduces only the bracket presentation multiplier', () => { test('detection owns and releases the Cockpit lifecycle listener', async () => { const source = await readFile(new URL('./detection.js', import.meta.url), 'utf8'); assert.match(source, /addEventListener\('gev:cockpit-mode-changed', _cockpitModeListener\)/); - assert.match(source, /removeEventListener\('gev:cockpit-mode-changed', _cockpitModeListener\)/); + assert.match(source, /removeEventListener\(\s*'gev:cockpit-mode-changed',\s*_cockpitModeListener,?\s*\)/); assert.match( source, /fade \* entry\.alpha \* bracketPresentationOpacity/, diff --git a/src/data/detectionRenderDemand.test.mjs b/src/data/detectionRenderDemand.test.mjs index d6af9d607..c140bdd22 100644 --- a/src/data/detectionRenderDemand.test.mjs +++ b/src/data/detectionRenderDemand.test.mjs @@ -277,7 +277,7 @@ test('detection holds nothing, and asks for its own frames instead', async () => assert.match(source, /governorRequestRender\('detection-visibility'\)/, 'mode/suspend transitions request their own repaint'); - assert.match(source, /if \(detectionNeedsFollowUpFrame\(\{[\s\S]*?\}\)\) \{\s*\n\s*governorRequestRender\('detection-animation'\);/, + assert.match(source, /if \(\s*detectionNeedsFollowUpFrame\(\{[\s\S]*?\}\)\s*\) \{\s*\n\s*governorRequestRender\('detection-animation'\);/, 'the follow-up frame is gated on the policy, never unconditional'); // The demand call must read the FRAME's timestamp, not a fresh sample. @@ -306,7 +306,7 @@ test('detection holds nothing, and asks for its own frames instead', async () => // the emptiest possible scene. (Caught by the governor gate at 301 renders/5 s // with zero layers; the fix settles the solve there, because with nothing // detectable it is vacuously complete.) - const emptyExit = /\/\/ Drop the replay buffer with it[\s\S]*?return \{ didSolve: false[^\n]*\n/.exec(source)?.[0]; + const emptyExit = /\/\/ Drop the replay buffer with it[\s\S]*?return \{\s*didSolve: false[\s\S]*?\};/.exec(source)?.[0]; assert.ok(emptyExit, 'the zero-objects exit is still identifiable'); assert.match(emptyExit, /_labelSolveDirty = false;/, 'the zero-objects exit settles the solve instead of carrying it forward'); @@ -318,7 +318,7 @@ test('detection holds nothing, and asks for its own frames instead', async () => assert.ok(drawOverlay, 'detection.js still has a draw pass'); assert.doesNotMatch(drawOverlay, /Date\.now\(\)/, 'the draw pass must run on the frame timestamp, not a wall clock'); - assert.match(drawOverlay, /const now = Number\.isFinite\(frame\.timestamp\) \? frame\.timestamp : _nowMs\(\);/); + assert.match(drawOverlay, /const now =\s*Number\.isFinite\(frame\.timestamp\) \? frame\.timestamp : _nowMs\(\);/); assert.match(source, /_enableTime = _nowMs\(\);/, 'the enable stamp shares the monotonic clock the fade is measured against'); @@ -331,7 +331,7 @@ test('detection holds nothing, and asks for its own frames instead', async () => 'the valve asks the shared policy instead of inlining its own threshold'); assert.doesNotMatch(shouldPaint, /_lastPaintMs > 22/, 'the old inline threshold must be gone, not shadowing the policy'); - assert.match(shouldPaint, /if \(decision\.requestFollowUp\) governorRequestRender\('detection-paint-skipped'\)/, + assert.match(shouldPaint, /if \(decision\.requestFollowUp\)\s*governorRequestRender\('detection-paint-skipped'\)/, 'skipping a paint must hand the request forward, not swallow it'); // The scanline must not go back to the frame counter: that is what made a diff --git a/src/firstRunExperience.test.mjs b/src/firstRunExperience.test.mjs index ccaacf7e4..5a501a677 100644 --- a/src/firstRunExperience.test.mjs +++ b/src/firstRunExperience.test.mjs @@ -222,8 +222,8 @@ test('the key handler refuses to act for a card that is not really on screen', ( assert.match(module, /isActive: \(\) => !closing && isTopmost\(\)/); // Real visibility, not just the class: the class survives while CSS hides the // card, which is precisely how a Scene left an invisible ESC handler armed. - assert.match(module, /const isTopmost = \(\) => root\.isConnected/); - assert.match(module, /&& root\.getClientRects\(\)\.length > 0\s*\n\s*&& !coveredByOverlay\(\);/); + assert.match(module, /const isTopmost = \(\) =>\s*root\.isConnected/); + assert.match(module, /&&\s*root\.getClientRects\(\)\.length > 0 &&\s*!coveredByOverlay\(\);/); const keyboard = fs.readFileSync(new URL('./ui/surfaceKeyboard.js', import.meta.url), 'utf8'); const handler = keyboard.slice(keyboard.indexOf('const onKeyDown = (event) => {')); assert.match( @@ -373,7 +373,7 @@ test('the launcher yields on engage and waits when a surface is already up', () assert.match(module, /dismiss\(\{ restoreFocus: false \}\)/); // A cheap attribute watch, not a per-frame poll — the render governor must // not see a new hold because of onboarding chrome. - assert.match(module, /attributes: true, attributeFilter: \['class'\]/); + assert.match(module, /attributes: true,\s*attributeFilter: \['class'\]/); assert.match(module, /surfaceObserver\?\.disconnect\(\)/); assert.doesNotMatch(module, /setInterval|requestAnimationFrame\(function poll/); }); @@ -536,7 +536,7 @@ test('no mission writes a preference the visitor did not choose by picking it', const panelWrites = code.match(/setPanelCollapsed/g) || []; assert.equal(panelWrites.length, 1, 'exactly one panel reveal, on the Context path'); const contextPath = code.slice(code.indexOf('setContextMode: async (mode)'), code.indexOf('setLayerEnabled:')); - assert.match(contextPath, /result\?\.ok[\s\S]*?setPanelCollapsed\?\.\('global-context-panel', false, \{ explicit: true \}\)/); + assert.match(contextPath, /result\?\.ok[\s\S]*?setPanelCollapsed\?\.\('global-context-panel', false, \{\s*explicit: true,?\s*\}\)/); }); test('the decision table is written down where the next editor will read it', () => { diff --git a/src/hudAltitudeDatum.test.mjs b/src/hudAltitudeDatum.test.mjs index 3bb8f1539..eeb407da4 100644 --- a/src/hudAltitudeDatum.test.mjs +++ b/src/hudAltitudeDatum.test.mjs @@ -108,7 +108,7 @@ test('hud.js corrects the camera height to MSL through the geoid module', () => 'the ~2.7 MB grid must be requested once, on demand — not at HUD construction', ); assert.equal( - has(/\.catch\(\(\) => \{ \/\* readout falls back to the uncorrected height \*\/ \}\)/), + has(/\.catch\(\(\) => \{\s*\/\* readout falls back to the uncorrected height \*\/\s*\}\)/), true, 'a failed geoid load must leave the readout uncorrected, not unhandled', ); diff --git a/src/radioMarkup.test.mjs b/src/radioMarkup.test.mjs index 778f89a8c..1a0c27b2c 100644 --- a/src/radioMarkup.test.mjs +++ b/src/radioMarkup.test.mjs @@ -349,5 +349,5 @@ test('successful explicit user playback hands the speaker from voice to Radio', assert.match(radioBindings, /togglePlayback\(\{ origin: 'user' \}\)/); assert.match(radioBindings, /cycleStation\(direction, \{[\s\S]*?origin: 'user'/); assert.match(radioBindings, /commitTuningStation\(station\.id, \{ origin: 'user' \}\)/); - assert.match(realtime, /event\.origin === 'user' && event\.action === 'play' && this\.isActive\(\)[\s\S]*?this\.stop\(\{ preserveRadioPlayback: true \}\)/); + assert.match(realtime, /event\.origin === 'user' &&\s*event\.action === 'play' &&\s*this\.isActive\(\)[\s\S]*?this\.stop\(\{ preserveRadioPlayback: true \}\)/); }); diff --git a/src/tooling/format.test.mjs b/src/tooling/format.test.mjs index f0ab4405c..0f4fb01cb 100644 --- a/src/tooling/format.test.mjs +++ b/src/tooling/format.test.mjs @@ -1,4 +1,5 @@ import test from 'node:test'; +import { execFileSync } from 'node:child_process'; import assert from 'node:assert/strict'; import { mkdtemp, @@ -93,3 +94,92 @@ test( ); }, ); + +test('runtime discovery includes new nested modules and respects Git and formatter exclusions', async (t) => { + const root = await fixture(t); + execFileSync('git', ['init', '-q'], { cwd: root }); + await mkdir(path.join(root, 'src/nested'), { recursive: true }); + await mkdir(path.join(root, 'vendor'), { recursive: true }); + await writeFile( + path.join(root, 'scripts/format-runtime.json'), + JSON.stringify({ roots: ['src'] }), + ); + await writeFile(path.join(root, '.gitignore'), 'src/ignored.js\n'); + await writeFile(path.join(root, '.prettierignore'), 'src/generated.js\n'); + const source = 'export const value="yes"'; + for (const name of [ + 'src/nested/new.js', + 'src/module.mjs', + 'src/check.test.mjs', + 'src/ignored.js', + 'src/generated.js', + 'vendor/other.js', + ]) + await writeFile(path.join(root, name), source); + const expected = ['adopted.js', 'src/module.mjs', 'src/nested/new.js']; + assert.deepEqual( + (await formatAdoptedFiles(root, '--check')).changed, + expected, + ); + await formatAdoptedFiles(root, '--write'); + assert.deepEqual((await formatAdoptedFiles(root, '--check')).changed, []); + for (const name of [ + 'src/check.test.mjs', + 'src/ignored.js', + 'src/generated.js', + 'vendor/other.js', + ]) + assert.equal(await readFile(path.join(root, name), 'utf8'), source); +}); + +test('runtime scope rejects unsafe roots before formatting existing entries', async (t) => { + for (const roots of [ + [], + ['src', 'src'], + ['../outside'], + ['/absolute'], + ['.'], + ['src/*'], + ]) { + const root = await fixture(t); + await writeFile( + path.join(root, 'scripts/format-runtime.json'), + JSON.stringify({ roots }), + ); + await assert.rejects( + formatAdoptedFiles(root, '--write'), + /Runtime formatting roots/, + ); + assert.equal( + await readFile(path.join(root, 'adopted.js'), 'utf8'), + 'const value="yes"\r\n', + ); + } +}); + +test( + 'discovered runtime symlinks cannot escape the repository', + { skip: process.platform === 'win32' }, + async (t) => { + const root = await fixture(t); + const other = await fixture(t); + execFileSync('git', ['init', '-q'], { cwd: root }); + await mkdir(path.join(root, 'src')); + await writeFile( + path.join(root, 'scripts/format-runtime.json'), + JSON.stringify({ roots: ['src'] }), + ); + await symlink( + path.join(other, 'adopted.js'), + path.join(root, 'src/outside.js'), + ); + await assert.rejects( + formatAdoptedFiles(root, '--write'), + /repository files/, + ); + assert.equal( + await readFile(path.join(root, 'adopted.js'), 'utf8'), + 'const value="yes"\r\n', + ); + }, +); diff --git a/src/voice/aircraftIdentityNarration.test.mjs b/src/voice/aircraftIdentityNarration.test.mjs index 8d88af8b3..51a5de3c9 100644 --- a/src/voice/aircraftIdentityNarration.test.mjs +++ b/src/voice/aircraftIdentityNarration.test.mjs @@ -30,7 +30,7 @@ test('aircraft identity narration acknowledges missing enrichment', () => { assert.match(followup, /explicitly cover operator, aircraft type, and route before finishing/); assert.match(followup, /selectedProperties\.operator/); assert.match(followup, /selectedProperties\.type/); - assert.match(followup, /selectedProperties\.route \|\| selectedProperties\.routeOrigin \|\| selectedProperties\.routeDestination/); + assert.match(followup, /selectedProperties\.route \|\|\s*selectedProperties\.routeOrigin \|\|\s*selectedProperties\.routeDestination/); assert.match(followup, /Operator details are unavailable/); assert.match(followup, /Aircraft type is unavailable/); assert.match(followup, /Route details are unavailable/);