From c4549dece2b5972c6870f45077c92095650d7cce Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 12 Jun 2026 02:37:38 -0700 Subject: [PATCH] fix(review): resolve lint-gate errors in mission-verification - Drop the unnecessary regex escape in the shell-metacharacter class. - Move the R17 git-clean post-condition out of finally (no-unsafe-finally): a dirty tree now fails closed to an inconclusive verdict instead of a finally-throw that masked the verdict. Test updated to the new behavior. --- .../__tests__/mission-verification.test.ts | 9 +++-- packages/engine/src/mission-verification.ts | 35 +++++++++++++------ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/packages/engine/src/__tests__/mission-verification.test.ts b/packages/engine/src/__tests__/mission-verification.test.ts index 7b32226daa..c6b71a04c6 100644 --- a/packages/engine/src/__tests__/mission-verification.test.ts +++ b/packages/engine/src/__tests__/mission-verification.test.ts @@ -333,7 +333,7 @@ describe("TestExecutionVerificationCapability", () => { expect(materializer.cleanCalls).toBe(1); }); - it("throws if the source tree is dirty after a run (R17 post-condition)", async () => { + it("fails closed to inconclusive if the source tree is dirty after a run (R17 post-condition)", async () => { const materializer = makeFakeMaterializer(); materializer.setDirty(true); const cap = new TestExecutionVerificationCapability({ @@ -345,6 +345,11 @@ describe("TestExecutionVerificationCapability", () => { backendFactory: () => makeScriptedBackend([{ outcome: "success", stdout: "ok", stderr: "", bufferOverflow: false }]), }); - await expect(cap.verifyBehavioralAssertion(baseRequest())).rejects.toThrow(/git-clean/); + // A dirty tree means verification mutated the source — never trust the + // verdict; fail closed to inconclusive (the post-condition is checked + // outside finally so it cannot mask the verdict via an unsafe throw). + const outcome = await cap.verifyBehavioralAssertion(baseRequest()); + expect(outcome.verdict).toBe("inconclusive"); + expect(outcome.reason).toMatch(/git-clean/); }); }); diff --git a/packages/engine/src/mission-verification.ts b/packages/engine/src/mission-verification.ts index 065f8e516c..445addf079 100644 --- a/packages/engine/src/mission-verification.ts +++ b/packages/engine/src/mission-verification.ts @@ -179,7 +179,7 @@ export interface VerificationCapability { * additional shell behavior. Agent-supplied test paths containing any of these * are rejected before execution. */ -const SHELL_METACHARACTERS = /[;&|`$(){}<>!*?\[\]\\"'\n\r\t\0]/; +const SHELL_METACHARACTERS = /[;&|`$(){}<>!*?[\]\\"'\n\r\t\0]/; /** * Validate an agent-supplied test-file path. Returns the normalized path when @@ -445,6 +445,9 @@ export class TestExecutionVerificationCapability implements VerificationCapabili if (!request.integrationSha) { return this.inconclusive(assertionId, "no integration SHA available to materialize a trusted checkout"); } + // Capture the narrowed (string) value: property-access narrowing does not + // carry into the nested async IIFE below, so reference this const there. + const integrationSha = request.integrationSha; // R19: validate any agent-supplied proof path BEFORE doing any work. let validatedTestPath: string | undefined; @@ -481,8 +484,10 @@ export class TestExecutionVerificationCapability implements VerificationCapabili let implCheckout: DisposableCheckout | undefined; let baselineCheckout: DisposableCheckout | undefined; + let outcome: VerificationOutcome; try { - implCheckout = await this.materializer.materialize(this.rootDir, request.integrationSha); + outcome = await (async (): Promise => { + implCheckout = await this.materializer.materialize(this.rootDir, integrationSha); const implResult = await runVerificationCommand( this.store, @@ -546,23 +551,33 @@ export class TestExecutionVerificationCapability implements VerificationCapabili reason: "verification suite failed on the implementation checkout; behavior not confirmed", detail: implResult.stderr || implResult.stdout || undefined, }; + })(); } catch (err) { const message = err instanceof Error ? err.message : String(err); // R9: any setup/exec failure (timeout, abort, materialization error) is a // non-pass; we route it to inconclusive (infra, not behavioral). - return this.inconclusive(assertionId, `verification run could not complete: ${message}`); + outcome = this.inconclusive(assertionId, `verification run could not complete: ${message}`); } finally { restoreBackend(); await implCheckout?.dispose(); await baselineCheckout?.dispose(); - // R17: the source tree feeding diff/merge must be byte-clean afterwards. - try { - await this.materializer.assertSourceClean(this.rootDir); - } catch (cleanErr) { - verifyLog.error("Verification post-condition violated (source not git-clean):", cleanErr); - throw cleanErr instanceof Error ? cleanErr : new Error(String(cleanErr)); - } } + + // R17 post-condition — checked OUTSIDE finally so it never masks the verdict + // via an unsafe finally-throw. The source tree feeding diff/merge must be + // byte-clean afterwards; a violation means verification mutated the source, so + // we fail closed to inconclusive rather than trusting the verdict. + try { + await this.materializer.assertSourceClean(this.rootDir); + } catch (cleanErr) { + const message = cleanErr instanceof Error ? cleanErr.message : String(cleanErr); + verifyLog.error("Verification post-condition violated (source not git-clean):", cleanErr); + return this.inconclusive( + assertionId, + `verification post-condition violated: source tree not git-clean after run: ${message}`, + ); + } + return outcome; } private inconclusive(assertionId: string, reason: string): VerificationOutcome {