chore(#2453): resolve the uniform-router-param conflict and clear the warning floor (#2489)

npx eslint . reported 0 errors, 53 warnings. The floor eroded the signal: a
genuinely new warning had to be spotted against noise, so 'lint is clean' was not
usable as a check. This takes it to zero.

Fifty of the 53 were one category in gsd-core/bin/gsd-tools.cjs — all unused
ARGUMENTS (error x30, cwd x10, raw x7, args x3), never unused variables. They are
the Command Routing Hub's uniform handler signature, function routeX({ args, cwd,
raw, error }), declared identically whether or not a handler uses all four
members. argsIgnorePattern: '^_' is structurally in conflict with that convention:
satisfying it would mean _-prefixing ~50 parameters and making the signature
non-uniform across the table. Option 1 of #2453: disable args checking for that
file only, keeping varsIgnorePattern intact so genuinely dead variables (the #2379
class) still surface — verified by injecting an unused variable, which still warns.

This is the config decision #732 explicitly deferred ('Severities stay warn (no
config change in this pass)').

The remaining three predate the issue's count of 51 and are real defects, not
suppressions:

- tests/workflow-compat.test.cjs — the step-9 lookahead used (?=\*\*10\.|\z).
  \z is a Perl/Ruby end-of-input anchor with NO meaning in JavaScript; it matched
  a literal 'z', so the lazy span silently stopped at the first z whenever **10.
  was absent, truncating the captured step and letting the assertion pass against
  a partial block. Corrected to $.
- tests/debugger-prevention.test.cjs — an unused RegExp built one line above the
  one actually used. Removed.
- tests/installer-migrations.test.cjs — try/finally inside a test body, which
  CONTRIBUTING prohibits ('verbose, masks test failures, not an approved
  pattern'); the unused t was the symptom. Converted to t.after().

Closes #2453

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-07-21 18:38:51 -04:00
committed by GitHub
parent 360b3ebc5a
commit c19d3d7bda
4 changed files with 59 additions and 29 deletions

View File

@@ -419,4 +419,33 @@ export default tseslint.config(
languageOptions: { sourceType: 'commonjs', globals: { ...globals.node } }, languageOptions: { sourceType: 'commonjs', globals: { ...globals.node } },
rules: { 'local/no-source-grep': 'error' }, rules: { 'local/no-source-grep': 'error' },
}, },
// ── #2453 Command Routing Hub: uniform handler signature ────────────────────
// Every route handler in gsd-tools.cjs is declared with the SAME destructured
// signature — `function routeX({ args, cwd, raw, error })` — whether or not it
// uses all four members. That uniformity is the point: it is the dispatch
// contract, so a handler can be moved or added without re-deriving which
// members exist.
//
// `argsIgnorePattern: '^_'` is structurally in conflict with that convention:
// satisfying it would mean `_`-prefixing ~50 parameters, which makes the
// signature non-uniform across the table and defeats the contract. So args
// checking is disabled HERE ONLY.
//
// `varsIgnorePattern` is deliberately left intact: genuinely dead *variables*
// (the #2379 case — unused `require()` results) must still surface. This
// narrows the exemption to the one category the convention actually forces.
//
// Decision deferred by #732 ("Severities stay `warn` (no config change in this
// pass)"), resolved by #2453 option 1.
{
files: ['gsd-core/bin/gsd-tools.cjs'],
rules: {
'no-unused-vars': ['warn', {
args: 'none',
varsIgnorePattern: '^_',
caughtErrors: 'none',
}],
},
},
); );

View File

@@ -96,7 +96,6 @@ describe('prevention / blameless-postmortem output (#1963, epic #1957 Phase 3B)'
const content = fs.readFileSync(AGENT, 'utf8'); const content = fs.readFileSync(AGENT, 'utf8');
const requiredFields = ['Date', 'Error patterns', 'Root cause', 'Fix', 'Files changed', 'Why not caught', 'Recurrence guard']; const requiredFields = ['Date', 'Error patterns', 'Root cause', 'Fix', 'Files changed', 'Why not caught', 'Recurrence guard'];
for (const field of requiredFields) { for (const field of requiredFields) {
const re = new RegExp(`\\*\\*${field}`, 'i');
const matches = content.match(new RegExp(`\\*\\*${field}`, 'gi')); const matches = content.match(new RegExp(`\\*\\*${field}`, 'gi'));
assert.ok( assert.ok(
matches && matches.length >= 2, matches && matches.length >= 2,

View File

@@ -2654,36 +2654,34 @@ describe('migration.plan()', () => {
test('a regular managed file is still backed up by content (no behavior change)', (t) => { test('a regular managed file is still backed up by content (no behavior change)', (t) => {
const configDir = createTempInstall(); const configDir = createTempInstall();
try { t.after(() => cleanup(configDir));
writeFile(configDir, 'extensions/gsd.cjs', 'locally patched extension\n');
writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') });
const result = runInstallerMigrations({ writeFile(configDir, 'extensions/gsd.cjs', 'locally patched extension\n');
configDir, writeManifest(configDir, { 'extensions/gsd.cjs': sha256('the original extension\n') });
runtime: 'pi',
scope: 'global',
migrations: [piExtensionMigration],
now: () => '2026-07-20T00:00:00.000Z',
});
const backupAction = result.plan.actions.find((a) => a.type === 'backup-and-remove'); const result = runInstallerMigrations({
assert.ok(backupAction, 'expected backup-and-remove for the locally patched file'); configDir,
runtime: 'pi',
scope: 'global',
migrations: [piExtensionMigration],
now: () => '2026-07-20T00:00:00.000Z',
});
// The PLAN carries backupRelPath: null — the concrete backup location is const backupAction = result.plan.actions.find((a) => a.type === 'backup-and-remove');
// chosen during apply and recorded in the journal, so read it from there. assert.ok(backupAction, 'expected backup-and-remove for the locally patched file');
const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8'));
const journalled = journal.actions.find((a) => a.backupRelPath); // The PLAN carries backupRelPath: null — the concrete backup location is
assert.ok(journalled, 'apply must record the backup path in the journal for the user'); // chosen during apply and recorded in the journal, so read it from there.
const backupPath = path.join(configDir, journalled.backupRelPath); const journal = JSON.parse(fs.readFileSync(path.join(configDir, result.journalRelPath), 'utf8'));
assert.equal( const journalled = journal.actions.find((a) => a.backupRelPath);
fs.readFileSync(backupPath, 'utf8'), assert.ok(journalled, 'apply must record the backup path in the journal for the user');
'locally patched extension\n', const backupPath = path.join(configDir, journalled.backupRelPath);
'a real file must still be backed up by content so the user can recover it', assert.equal(
); fs.readFileSync(backupPath, 'utf8'),
assert.ok(!fs.existsSync(path.join(configDir, 'extensions', 'gsd.cjs'))); 'locally patched extension\n',
} finally { 'a real file must still be backed up by content so the user can recover it',
cleanup(configDir); );
} assert.ok(!fs.existsSync(path.join(configDir, 'extensions', 'gsd.cjs')));
}); });
// In-flight failure recovery: when a later step of the SAME apply() attempt // In-flight failure recovery: when a later step of the SAME apply() attempt

View File

@@ -161,7 +161,11 @@ describe('feat-41: ship.md TDD Audit gate_status extraction', () => {
test('#2431: step 9 (aggregate trailer) is also gated on real values existing', () => { test('#2431: step 9 (aggregate trailer) is also gated on real values existing', () => {
// The aggregate gate_status trailer is the companion to the TDD Audit // The aggregate gate_status trailer is the companion to the TDD Audit
// section; both must be skipped together when no real gate_status exists. // section; both must be skipped together when no real gate_status exists.
const step9 = workflow.match(/\*\*9\.\s*Aggregate gate_status trailer[\s\S]*?(?=\*\*10\.|\z)/); // `\z` is a Perl/Ruby end-of-input anchor with NO meaning in JavaScript — it
// matched a literal `z`, so the lazy span silently stopped at the first `z`
// whenever `**10.` was absent, truncating the captured step. `$` (no /m flag)
// is the JS end-of-input anchor.
const step9 = workflow.match(/\*\*9\.\s*Aggregate gate_status trailer[\s\S]*?(?=\*\*10\.|$)/);
assert.ok(step9, 'step 9 must exist in the workflow'); assert.ok(step9, 'step 9 must exist in the workflow');
assert.match(step9[0], /step 8|at least one|real/i, assert.match(step9[0], /step 8|at least one|real/i,
'step 9 must reference step 8 or require at least one real gate_status value (#2431)'); 'step 9 must reference step 8 or require at least one real gate_status value (#2431)');