diff --git a/package.json b/package.json index de81b55..591dd8e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mikefreno/omp-pygienium", - "version": "0.1.3", + "version": "0.1.4", "description": "Code hygiene extension for omp (port of the pi extension) — isolated sub-agent checks that scan a target, apply fixes, and emit a findings+changes report.", "keywords": [ "omp", diff --git a/src/modes/all.ts b/src/modes/all.ts index ce065d3..43e3991 100644 --- a/src/modes/all.ts +++ b/src/modes/all.ts @@ -43,6 +43,8 @@ import { reconcileRunStatus, resetCheckEntry, saveRunState, + settleTerminalPhases, + shouldResumeRun, shouldRunOnResume, stateDir, type RunState, @@ -282,12 +284,17 @@ export async function runAllChecks( } // --- Init / resume the single shared run-state -------------------------- + // A cleanly completed run is finished work, not a run to continue: a new + // invocation is a new scan. Without this, re-running `/pygienium-all` + // dispatched nothing but whatever an interrupted run had left non-terminal. + const stored = await loadRunState(cwd); let state = - (await loadRunState(cwd)) ?? - initRunState( - cwd, - selected.map((c) => ({ name: c.name, label: c.label, fix })), - ); + stored && shouldResumeRun(stored) + ? stored + : initRunState( + cwd, + selected.map((c) => ({ name: c.name, label: c.label, fix })), + ); // Ensure every selected check has an entry (adds any missing on resume). for (const check of selected) { @@ -363,6 +370,11 @@ export async function runAllChecks( // Resumability: skip terminal checks unless --fresh. if (entry && !shouldRunOnResume(entry, fresh)) { skipped.push(check.name); + // The entry may still carry a phase an interrupted run left + // `in_progress`; settle it so the state never reads as both finished + // and still running. + settleTerminalPhases(entry); + await saveRunState(state); footerSettle( check.name, entry.status === "complete" ? "skipped" : "skipped", diff --git a/src/run-state.ts b/src/run-state.ts index 124dd02..f518ee5 100644 --- a/src/run-state.ts +++ b/src/run-state.ts @@ -269,6 +269,36 @@ export function shouldRunOnResume(check: CheckRun, fresh: boolean): boolean { return fresh || !isCheckTerminal(check); } +/** + * Whether a stored run should be continued rather than started over. + * + * Only a cleanly completed run is finished work: asking for another run then + * means a new scan, so its completed entries must not be skipped (doing that + * made the second `/pygienium-all` dispatch nothing at all). Anything else — + * interrupted, partial, failed — is continued, which is what carries completed + * checks forward cheaply while an unfinished one picks up where it stopped. + */ +export function shouldResumeRun(state: RunState): boolean { + return state.status !== "complete"; +} + +/** + * Settle the phases of a terminal check that an interrupted run left + * `in_progress`, so the stored state never reads as finished and still running + * at once. A completed check's phases become `complete`; a skipped check's + * become `skipped`. + */ +export function settleTerminalPhases(check: CheckRun): void { + if (!isCheckTerminal(check)) return; + const settled: PhaseStatus = check.status === "complete" ? "complete" : "skipped"; + for (const phase of check.phases) { + if (phase.status === "in_progress") { + phase.status = settled; + phase.finishedAt ??= check.finishedAt ?? Date.now(); + } + } +} + /** * Reset a single check entry back to `pending` with fresh phases. Used by * `/pygienium-resume --fresh` so that previously-complete checks are diff --git a/tests/all-integration.test.ts b/tests/all-integration.test.ts index e54999d..b4d345c 100644 --- a/tests/all-integration.test.ts +++ b/tests/all-integration.test.ts @@ -22,7 +22,7 @@ import { type AgentRunner, } from "../src/agent-runner.js"; import { handleAllCommand, type PygieniumCtx } from "../src/commands.js"; -import { loadRunState } from "../src/run-state.js"; +import { loadRunState, markCheckStatus, markRunStatus, saveRunState } from "../src/run-state.js"; import { canonicalChecksRoot } from "../src/export.js"; /** Stub check whose fake-runner task writes artifacts + echoes a line. */ @@ -147,16 +147,27 @@ describe("/pygienium-all end-to-end (task 14)", () => { expect(out.join("\n")).toContain("no checks registered"); }); - it("is resumable: a second call reuses the existing run-state", async () => { + it("is resumable: an unfinished run-state is continued, not restarted", async () => { registerCheck(fakeCheck("alpha")); + // A completed run is finished work: the next call starts a new run-state. await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd))); - const first = await loadRunState(cwd); - const firstStarted = first!.startedAt; + const completed = await loadRunState(cwd); + expect(completed!.status).toBe("complete"); + await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd))); + const fresh = await loadRunState(cwd); + expect(fresh!.startedAt).toBeGreaterThan(completed!.startedAt); + + // An unfinished one is continued: same run, same startedAt, and the + // completed check is not dispatched again. + const interrupted = await loadRunState(cwd); + markRunStatus(interrupted!, "in_progress"); + markCheckStatus(interrupted!, "alpha", "in_progress"); + await saveRunState(interrupted!); + const before = (await loadRunState(cwd))!.startedAt; - // Second run reloads the existing run-state (same startedAt). await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd))); - const second = await loadRunState(cwd); - expect(second!.startedAt).toBe(firstStarted); - expect(second!.status).toBe("complete"); + const resumed = await loadRunState(cwd); + expect(resumed!.startedAt).toBe(before); + expect(resumed!.status).toBe("complete"); }); }); diff --git a/tests/all.test.ts b/tests/all.test.ts index 8965fdb..5f8052d 100644 --- a/tests/all.test.ts +++ b/tests/all.test.ts @@ -39,6 +39,9 @@ import { loadRunState, markCheckStatus, applyPhaseStatus, + initRunState, + markRunStatus, + saveRunState, PHASE_RECON, PHASE_ANALYSIS, PHASE_FIX, @@ -229,41 +232,100 @@ describe("/pygienium-all orchestrator (task 12)", () => { expect(summary).not.toContain("## beta"); }); - it("skips terminal checks on re-run; --fresh re-runs them", async () => { + it("a completed run is not resumed: the next invocation runs every check again", async () => { registerCheck(fakeCheck("alpha")); registerCheck(fakeCheck("beta")); // First run: both complete. await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd))); - expect(track.dispatched.length).toBe(4); // 2 checks × 2 phases - const firstAlphaFix = await readFile( - join(cwd, ".pygienium", "checks", "alpha", "changes.md"), - "utf8", - ); + expect(track.dispatched).toEqual(["alpha", "alpha", "beta", "beta"]); - // Second run without --fresh: both already terminal → skipped. + // Second run, no flags: the stored run completed, so this is a new scan. + // Every check dispatches again — the regression this pins dispatched + // nothing but whichever check an interrupted run happened to leave open. track.dispatched.length = 0; const out = await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd)), ); - expect(track.dispatched).toHaveLength(0); - expect(out.join("\n")).toContain("skipping"); - const state2 = await loadRunState(cwd); - expect(state2?.status).toBe("complete"); + expect(track.dispatched).toEqual(["alpha", "alpha", "beta", "beta"]); + expect(out.join("\n")).not.toContain("skipping"); + const state = await loadRunState(cwd); + expect(state?.status).toBe("complete"); + expect(state?.checks.alpha?.status).toBe("complete"); + expect(state?.checks.beta?.status).toBe("complete"); - // Third run with --fresh: both re-dispatched from scratch. + // --fresh remains the explicit reset and dispatches the same way. track.dispatched.length = 0; await captureStdout(() => handleAllCommand("--fix --fresh", stubCtx(cwd))); expect(track.dispatched).toEqual(["alpha", "alpha", "beta", "beta"]); - const state3 = await loadRunState(cwd); - expect(state3?.status).toBe("complete"); - // The fresh re-run overwrote alpha's changes.md (still valid content). - const alphaFix2 = await readFile( - join(cwd, ".pygienium", "checks", "alpha", "changes.md"), - "utf8", + }); + + it("resumes an unfinished run: terminal checks skip, the rest re-dispatch", async () => { + registerCheck(fakeCheck("alpha")); + registerCheck(fakeCheck("beta")); + + // A run that stopped with alpha done and beta failed is unfinished work, + // so it continues instead of starting over. + const state = initRunState(cwd, [ + { name: "alpha", label: "alpha", fix: true }, + { name: "beta", label: "beta", fix: true }, + ]); + state.recon = { + complete: true, + path: join(cwd, ".pygienium", "recon.json"), + finishedAt: Date.now(), + }; + markComplete(state, "alpha"); + markCheckStatus(state, "beta", "failed", "provider reset mid-scan"); + markRunStatus(state, "partial"); + await saveRunState(state); + + const out = await captureStdout(() => + handleAllCommand("--fix", stubCtx(cwd)), ); - expect(alphaFix2).toContain("alpha changes"); - void firstAlphaFix; + expect(track.dispatched).toEqual(["beta", "beta"]); + expect(out.join("\n")).toContain("skipping"); + }); + + it("settles a phase an interrupted run left in_progress on a skipped check", async () => { + registerCheck(fakeCheck("alpha")); + registerCheck(fakeCheck("beta")); + + // alpha finished, but a later interrupted run left its analysis phase open. + const state = initRunState(cwd, [ + { name: "alpha", label: "alpha", fix: false }, + { name: "beta", label: "beta", fix: false }, + ]); + state.recon = { + complete: true, + path: join(cwd, ".pygienium", "recon.json"), + finishedAt: Date.now(), + }; + markComplete(state, "alpha"); + // Directly, because applyPhaseStatus would also move the check itself back + // to in_progress — the corruption here is a phase left open behind a + // check that still reads complete. + const staleAnalysis = state.checks.alpha?.phases.find( + (p) => p.id === PHASE_ANALYSIS, + ); + if (!staleAnalysis) throw new Error("alpha has no analysis phase"); + staleAnalysis.status = "in_progress"; + applyPhaseStatus(state, "beta", PHASE_ANALYSIS, "in_progress"); + await saveRunState(state); + + await captureStdout(() => handleAllCommand("", stubCtx(cwd))); + + const after = await loadRunState(cwd); + // alpha was skipped as already complete: its stale phase settles to + // complete rather than leaving the check finished-but-running. + const alphaPhases = after?.checks.alpha?.phases ?? []; + expect(alphaPhases.every((p) => p.status !== "in_progress")).toBe(true); + expect( + alphaPhases.find((p) => p.id === PHASE_ANALYSIS)?.status, + ).toBe("complete"); + // beta never finished, so the run resumes it and its scan runs again + // (scan-only: this run passes no --fix). + expect(track.dispatched).toEqual(["beta"]); }); it("resumes an interrupted run (re-dispatches non-terminal checks only)", async () => {