fix: a finished all-run is not a run to resume
`/pygienium-all` loaded the previous run-state and skipped every check whose entry was terminal, so a second invocation dispatched nothing at all. The one check that did run was whichever an interrupted run had left non-terminal — registry order made that `comments`, which read as "comments ran, everything after it was skipped". Resumability belongs to unfinished work: continue only when the stored run is not complete (`in_progress`, `partial`, `failed`), so completed checks are carried forward cheaply and the interrupted one picks up where it stopped. A completed run means the next invocation is a new scan, which is what the command name promises; `--fresh` and `/pygienium-resume` keep their explicit jobs. Settle a terminal check's phases on the way past too: an interrupted run can leave a phase `in_progress` behind a check that reads `complete`, which made the state claim both finished and still running. Tests: reverted src/ fails "a completed run is not resumed", "settles a phase an interrupted run left in_progress", and the integration test "an unfinished run-state is continued, not restarted"; all pass with the fix. Suite 164 pass, 1 pre-existing worktree-only failure (MISC-46). bun run typecheck clean; port-to-omp regenerates and typechecks. MISC-47 Co-Authored-By: nw-flash-flex <noreply@neuralwatt.com>
This commit is contained in:
@@ -43,6 +43,8 @@ import {
|
|||||||
reconcileRunStatus,
|
reconcileRunStatus,
|
||||||
resetCheckEntry,
|
resetCheckEntry,
|
||||||
saveRunState,
|
saveRunState,
|
||||||
|
settleTerminalPhases,
|
||||||
|
shouldResumeRun,
|
||||||
shouldRunOnResume,
|
shouldRunOnResume,
|
||||||
stateDir,
|
stateDir,
|
||||||
type RunState,
|
type RunState,
|
||||||
@@ -282,9 +284,14 @@ export async function runAllChecks(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// --- Init / resume the single shared run-state --------------------------
|
// --- 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 =
|
let state =
|
||||||
(await loadRunState(cwd)) ??
|
stored && shouldResumeRun(stored)
|
||||||
initRunState(
|
? stored
|
||||||
|
: initRunState(
|
||||||
cwd,
|
cwd,
|
||||||
selected.map((c) => ({ name: c.name, label: c.label, fix })),
|
selected.map((c) => ({ name: c.name, label: c.label, fix })),
|
||||||
);
|
);
|
||||||
@@ -363,6 +370,11 @@ export async function runAllChecks(
|
|||||||
// Resumability: skip terminal checks unless --fresh.
|
// Resumability: skip terminal checks unless --fresh.
|
||||||
if (entry && !shouldRunOnResume(entry, fresh)) {
|
if (entry && !shouldRunOnResume(entry, fresh)) {
|
||||||
skipped.push(check.name);
|
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(
|
footerSettle(
|
||||||
check.name,
|
check.name,
|
||||||
entry.status === "complete" ? "skipped" : "skipped",
|
entry.status === "complete" ? "skipped" : "skipped",
|
||||||
|
|||||||
@@ -269,6 +269,36 @@ export function shouldRunOnResume(check: CheckRun, fresh: boolean): boolean {
|
|||||||
return fresh || !isCheckTerminal(check);
|
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
|
* Reset a single check entry back to `pending` with fresh phases. Used by
|
||||||
* `/pygienium-resume --fresh` so that previously-complete checks are
|
* `/pygienium-resume --fresh` so that previously-complete checks are
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ import {
|
|||||||
type AgentRunner,
|
type AgentRunner,
|
||||||
} from "../src/agent-runner.js";
|
} from "../src/agent-runner.js";
|
||||||
import { handleAllCommand, type PygieniumCtx } from "../src/commands.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";
|
import { canonicalChecksRoot } from "../src/export.js";
|
||||||
|
|
||||||
/** Stub check whose fake-runner task writes artifacts + echoes a line. */
|
/** 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");
|
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"));
|
registerCheck(fakeCheck("alpha"));
|
||||||
|
// A completed run is finished work: the next call starts a new run-state.
|
||||||
await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd)));
|
await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd)));
|
||||||
const first = await loadRunState(cwd);
|
const completed = await loadRunState(cwd);
|
||||||
const firstStarted = first!.startedAt;
|
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)));
|
await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd)));
|
||||||
const second = await loadRunState(cwd);
|
const resumed = await loadRunState(cwd);
|
||||||
expect(second!.startedAt).toBe(firstStarted);
|
expect(resumed!.startedAt).toBe(before);
|
||||||
expect(second!.status).toBe("complete");
|
expect(resumed!.status).toBe("complete");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -39,6 +39,9 @@ import {
|
|||||||
loadRunState,
|
loadRunState,
|
||||||
markCheckStatus,
|
markCheckStatus,
|
||||||
applyPhaseStatus,
|
applyPhaseStatus,
|
||||||
|
initRunState,
|
||||||
|
markRunStatus,
|
||||||
|
saveRunState,
|
||||||
PHASE_RECON,
|
PHASE_RECON,
|
||||||
PHASE_ANALYSIS,
|
PHASE_ANALYSIS,
|
||||||
PHASE_FIX,
|
PHASE_FIX,
|
||||||
@@ -229,41 +232,100 @@ describe("/pygienium-all orchestrator (task 12)", () => {
|
|||||||
expect(summary).not.toContain("## beta");
|
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("alpha"));
|
||||||
registerCheck(fakeCheck("beta"));
|
registerCheck(fakeCheck("beta"));
|
||||||
|
|
||||||
// First run: both complete.
|
// First run: both complete.
|
||||||
await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd)));
|
await captureStdout(() => handleAllCommand("--fix", stubCtx(cwd)));
|
||||||
expect(track.dispatched.length).toBe(4); // 2 checks × 2 phases
|
expect(track.dispatched).toEqual(["alpha", "alpha", "beta", "beta"]);
|
||||||
const firstAlphaFix = await readFile(
|
|
||||||
join(cwd, ".pygienium", "checks", "alpha", "changes.md"),
|
|
||||||
"utf8",
|
|
||||||
);
|
|
||||||
|
|
||||||
// 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;
|
track.dispatched.length = 0;
|
||||||
const out = await captureStdout(() =>
|
const out = await captureStdout(() =>
|
||||||
handleAllCommand("--fix", stubCtx(cwd)),
|
handleAllCommand("--fix", stubCtx(cwd)),
|
||||||
);
|
);
|
||||||
expect(track.dispatched).toHaveLength(0);
|
expect(track.dispatched).toEqual(["alpha", "alpha", "beta", "beta"]);
|
||||||
expect(out.join("\n")).toContain("skipping");
|
expect(out.join("\n")).not.toContain("skipping");
|
||||||
const state2 = await loadRunState(cwd);
|
const state = await loadRunState(cwd);
|
||||||
expect(state2?.status).toBe("complete");
|
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;
|
track.dispatched.length = 0;
|
||||||
await captureStdout(() => handleAllCommand("--fix --fresh", stubCtx(cwd)));
|
await captureStdout(() => handleAllCommand("--fix --fresh", stubCtx(cwd)));
|
||||||
expect(track.dispatched).toEqual(["alpha", "alpha", "beta", "beta"]);
|
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).
|
it("resumes an unfinished run: terminal checks skip, the rest re-dispatch", async () => {
|
||||||
const alphaFix2 = await readFile(
|
registerCheck(fakeCheck("alpha"));
|
||||||
join(cwd, ".pygienium", "checks", "alpha", "changes.md"),
|
registerCheck(fakeCheck("beta"));
|
||||||
"utf8",
|
|
||||||
|
// 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");
|
expect(track.dispatched).toEqual(["beta", "beta"]);
|
||||||
void firstAlphaFix;
|
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 () => {
|
it("resumes an interrupted run (re-dispatches non-terminal checks only)", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user