diff --git a/src/health-diagnostic.cts b/src/health-diagnostic.cts index 2410426c9..6079c9a02 100644 --- a/src/health-diagnostic.cts +++ b/src/health-diagnostic.cts @@ -443,6 +443,12 @@ function applyRepairs( ...(outcome.detail ? { detail: outcome.detail } : {}), ...(outcome.error ? { error: outcome.error } : {}), }); + // `applied` means "the repair actually succeeded", not "was attempted" + // — a handler that returns `{success: false}` (or throws, caught + // below) is recorded in `details` with its failure, but must not be + // reported as applied. `refused` is reserved for the DESTRUCTIVE-risk + // gate above; a failed attempt is neither applied nor refused. + if (outcome.success) applied.push(code); } catch (err) { details.push({ code, @@ -451,7 +457,6 @@ function applyRepairs( error: err instanceof Error ? err.message : String(err), }); } - applied.push(code); } return { applied, refused, details }; diff --git a/tests/health-diagnostic.test.cjs b/tests/health-diagnostic.test.cjs index e8621da54..ff12e59ca 100644 --- a/tests/health-diagnostic.test.cjs +++ b/tests/health-diagnostic.test.cjs @@ -350,4 +350,28 @@ describe('applyRepairs — REAL diagnostics (rows 15-16)', () => { const diskConfig = JSON.parse(fs.readFileSync(configPath, 'utf-8')); assert.equal(diskConfig.model_profile, 'balanced'); }); + + // Regression: a repair handler that THROWS (caught by applyRepairs's own + // try/catch) must be recorded in `details` with `success: false` and must + // NOT land in `applied` — `applied` means "succeeded", not "attempted". + // Forced here via ADD_NYQUIST_KEY against a config.json that is genuinely + // absent: `runRepairAction`'s `fs.readFileSync(configPath, ...)` throws + // ENOENT. + test('regression: a repair handler that throws is recorded in details with success:false and is NOT pushed to applied', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + setupHealthyProject(tmpDir); + fs.unlinkSync(path.join(tmpDir, '.planning', 'config.json')); + assert.equal(fs.existsSync(path.join(tmpDir, '.planning', 'config.json')), false); + + const diagnostics = [fakeDiagnostic('W008', REMEDY_ACTION.ADD_NYQUIST_KEY, REMEDY_RISK.NONE)]; + const result = applyRepairs(tmpDir, diagnostics, true, false); + + assert.ok(!result.applied.includes('W008'), 'W008 must not be applied — the handler threw'); + assert.ok(!result.refused.includes('W008'), 'a thrown handler is not a DESTRUCTIVE-risk refusal either'); + const detail = result.details.find((d) => d.code === 'W008'); + assert.ok(detail, 'a details row must still be recorded for the failed attempt'); + assert.equal(detail.success, false); + assert.ok(detail.error, 'the details row must carry the thrown error message'); + }); });