From 94bae1d44a81ed89864dda94a36dc1c02deae6c2 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Sun, 23 Aug 2026 23:16:37 +0530 Subject: [PATCH 1/7] feat(panel): record why cleanup happened and what a period actually got Closes #209 and #208. **#209.** Three routes reached cleanup -- a confirmed download, a run that produced no ZIP, and a legacy staging cleared on upgrade -- and all three wrote one `cleaned`. The origin was the only durable evidence that the ZIP reached the browser, and the transition discarded it, so a delivered run read as merely captured once the panel reopened and the transient signal was gone. Cleanup now keeps its origin: `cleaned-after-download`, `cleaned-without-export`, `cleaned-legacy`. The delivery claim reads that instead of inferring backwards from a value that had already thrown the answer away. Plain `cleaned` is retained so a ledger written before the split still parses; it carries no origin, so those runs stay indeterminate and read as captured, which is what they did before and is the safe direction for a claim about a file. Every existing `=== "cleaned"` became a check that silently stopped matching three quarters of its runs, so they go through one `isCleanedZipPhase` predicate rather than a comparison repeated per call site. **#208.** A multi-artifact period reaches `downloaded` when one artifact staged and another was explicitly unavailable, because an unavailable artifact is a resolved outcome -- so `saved` claimed the whole selection for a period that had part of it. It now reads `Partly saved`, kept distinct from `Needs review` because an artifact the portal never offered is not a fault a re-run corrects. The issue expected this to need a new persisted field. It does not: `filed-return-artifact-unavailable:` survives durable signal parsing, so a terminal target already carries it. The derivation was reading the run's step, which carries the signal for whichever period is in flight and would have marked the whole year partial because one period was. Counted separately in the status line. A partly saved period is in none of the other three counts, so without its own clause it would leave the header saying "1 of 2 saved" with the second period unaccounted for anywhere on the line. Co-Authored-By: Claude Opus 5 --- .../filed-returns-full-fiscal-year-ledger.ts | 21 ++++- ...iled-returns-full-fiscal-year-run-state.ts | 3 +- .../filed-returns-full-fiscal-year-summary.ts | 85 ++++++++++++++----- ...led-returns-full-fiscal-year-validation.ts | 9 +- src/background/local-data.ts | 3 +- src/connectors/gst/filed-returns-contracts.ts | 54 +++++++++++- src/entrypoints/popup/target-evidence.tsx | 12 +++ src/styles/panel.css | 7 ++ 8 files changed, 166 insertions(+), 28 deletions(-) diff --git a/src/background/filed-returns-full-fiscal-year-ledger.ts b/src/background/filed-returns-full-fiscal-year-ledger.ts index 562ef3cb..86568325 100644 --- a/src/background/filed-returns-full-fiscal-year-ledger.ts +++ b/src/background/filed-returns-full-fiscal-year-ledger.ts @@ -309,6 +309,25 @@ export function markFullFiscalYearTargetTerminal( }; } +/** + * The terminal phase for a cleanup, keeping which pending phase it came from. + * + * Three routes reach cleanup and only one of them delivered a ZIP to the + * browser. Writing a single `cleaned` for all three discarded the distinction + * at exactly the moment it became the only record of it. + * + * An unrecognised or absent pending phase stays on the origin-less `cleaned` + * rather than guessing a route, so it reads as indeterminate. + */ +function cleanedPhaseFor( + pending: FiledReturnsFullFiscalYearLedger["zipPhase"], +): NonNullable { + if (pending === "downloaded-cleanup-pending") return "cleaned-after-download"; + if (pending === "no-artifacts-cleanup-pending") return "cleaned-without-export"; + if (pending === "legacy-cleanup-pending") return "cleaned-legacy"; + return "cleaned"; +} + export function completeFullFiscalYearLedger( ledger: FiledReturnsFullFiscalYearLedger, now: Date, @@ -320,7 +339,7 @@ export function completeFullFiscalYearLedger( revision: nextRevision(ledger), status: "complete", updatedAt: now.toISOString(), - zipPhase: "cleaned", + zipPhase: cleanedPhaseFor(ledger.zipPhase), }; delete completedLedger.zipDownloadAttempt; return completedLedger; diff --git a/src/background/filed-returns-full-fiscal-year-run-state.ts b/src/background/filed-returns-full-fiscal-year-run-state.ts index 36d066c3..1a413817 100644 --- a/src/background/filed-returns-full-fiscal-year-run-state.ts +++ b/src/background/filed-returns-full-fiscal-year-run-state.ts @@ -4,6 +4,7 @@ import type { FiledReturnsFullFiscalYearLedger, PortalFlowStepResult, } from "../connectors/gst/filed-returns-contracts"; +import { isCleanedZipPhase } from "../connectors/gst/filed-returns-contracts"; import type { PackMessageResponse } from "../connectors/gst/messages"; import { filedReturnScopeId } from "../connectors/gst/filed-returns-return-descriptors"; import type { FiledReturnsFlowRunnerDeps } from "./filed-returns-flow-runner"; @@ -39,7 +40,7 @@ export function hasDownloadUnconfirmedTarget(ledger: FiledReturnsFullFiscalYearL export function hasRetainedFullFiscalYearStaging( ledger: FiledReturnsFullFiscalYearLedger, ): boolean { - if (ledger.zipPhase === "cleaned") return false; + if (isCleanedZipPhase(ledger.zipPhase)) return false; if (ledger.zipPhase !== undefined) return true; if (hasLegacyRetainedStaging(ledger)) return true; return ledger.targets.some((target) => diff --git a/src/background/filed-returns-full-fiscal-year-summary.ts b/src/background/filed-returns-full-fiscal-year-summary.ts index b9cc8633..d4bedd81 100644 --- a/src/background/filed-returns-full-fiscal-year-summary.ts +++ b/src/background/filed-returns-full-fiscal-year-summary.ts @@ -7,6 +7,10 @@ import type { FiledReturnsFullFiscalYearTargetStatus, PortalFlowStepResult, } from "../connectors/gst/filed-returns-contracts"; +import { + isCleanedZipPhase, + zipPhaseProvesDelivery, +} from "../connectors/gst/filed-returns-contracts"; import { filedReturnsArtifactLabel, normaliseFiledReturnsArtifactType, @@ -22,7 +26,7 @@ import { export function fullFiscalYearZipPhaseStep( ledger: FiledReturnsFullFiscalYearLedger, ): PortalFlowStepResult | null { - if (ledger.zipPhase === "cleaned") return null; + if (isCleanedZipPhase(ledger.zipPhase)) return null; const legacyRetained = hasLegacyRetainedStaging(ledger); if (ledger.zipPhase === undefined && !legacyRetained) return null; if (ledger.zipPhase === "restaging-required") { @@ -230,6 +234,7 @@ function targetOutcome( status: FiledReturnsFullFiscalYearTargetStatus, zipDelivered: boolean, runInterrupted: boolean, + missedAnArtifact: boolean, ): FiledReturnsTargetOutcome { const outcome = TARGET_OUTCOMES[status]; // `summariseFullFiscalYearLedger` reports an interrupted run as blocked while @@ -237,31 +242,42 @@ function targetOutcome( // running after an MV3 worker interruption, and reading it as "In progress" // both misdescribes it and excludes it from the count of what needs a person. if (outcome === "running" && runInterrupted) return "needs-review"; - return outcome === "saved" && !zipDelivered ? "captured" : outcome; + if (outcome !== "saved") return outcome; + // Delivery first. Whether the browser has the bytes is a stronger question + // than how much of the selection they represent, and a partial claim about a + // file Pack has not handed over would assert the handover in passing. + if (!zipDelivered) return "captured"; + return missedAnArtifact ? "partly-saved" : "saved"; } /** * Whether the ZIP reached the browser. * - * The signal alone, deliberately. `zipPhase: "cleaned"` looked like the durable - * form of the same fact and is not: three different phases reach it -- a - * confirmed download, a run that found no artifacts, and a legacy retained - * staging cleared on upgrade. Only the first is a delivery, and the phase that - * distinguishes them is overwritten by the transition, so the origin cannot be - * recovered from the ledger afterwards. + * The transient signal, or the durable phase that records the same delivery. * - * Inferring from it reported never-exported files as saved. That is the - * overclaim this list exists to prevent, so the inference is gone rather than - * narrowed -- a guard that fails closed states what it can prove and no more. + * The signal is emitted by the step that observes the download and is absent + * from the step a later re-summarisation builds, so it alone made a delivered + * run read as merely captured once the panel was reopened. * - * The cost is real and is the right way round: a delivered run re-summarised - * after the panel reopens reads `captured` rather than `saved`, because at that - * point Pack genuinely cannot prove the browser still holds the file. Recording - * delivery durably would fix that, and is a persistence change to raise rather - * than make. + * The old `zipPhase: "cleaned"` could not stand in for it: three phases reached + * that one value -- a confirmed download, a run that produced no ZIP, and a + * legacy staging cleared on upgrade -- and an inference built on the collapsed + * value reported never-exported files as saved. Cleanup now keeps its origin, + * so `cleaned-after-download` is the delivery and its siblings are not. The + * fix is to stop discarding the fact, not to guess it back afterwards. + * + * A ledger written before the split carries the origin-less `cleaned` and stays + * indeterminate: those runs still read as captured, which is what they did + * before and is the safe direction for a claim about a file. */ -function isFullFiscalYearZipDelivered(flowStep: PortalFlowStepResult): boolean { - return flowStep.safeSignals.includes("full-fiscal-year-zip-downloaded"); +function isFullFiscalYearZipDelivered( + ledger: FiledReturnsFullFiscalYearLedger, + flowStep: PortalFlowStepResult, +): boolean { + return ( + flowStep.safeSignals.includes("full-fiscal-year-zip-downloaded") || + zipPhaseProvesDelivery(ledger.zipPhase) + ); } /** @@ -278,7 +294,7 @@ export function fullFiscalYearTargetEvidence( ledger: FiledReturnsFullFiscalYearLedger, flowStep: PortalFlowStepResult, ): FiledReturnsTargetEvidence[] { - const zipDelivered = isFullFiscalYearZipDelivered(flowStep); + const zipDelivered = isFullFiscalYearZipDelivered(ledger, flowStep); // A discarded run: staged files were cleared without a delivery. Their targets // still read `downloaded`, and reporting those as captured claims Pack holds // files it has just deleted. @@ -319,10 +335,39 @@ export function fullFiscalYearTargetEvidence( RUN_INDETERMINATE_SIGNALS.some((signal) => flowStep.safeSignals.includes(signal)); return ledger.targets.map((target) => ({ period: target.period, - outcome: targetOutcome(target.status, zipDelivered, runInterrupted), + outcome: targetOutcome( + target.status, + zipDelivered, + runInterrupted, + targetMissedAnArtifact(target), + ), })); } +/** + * Whether the portal gave this period only part of what was selected. + * + * Read from the target rather than from the run's step. The step carries the + * signal for whichever period is in flight, so testing it would mark every + * period of the year partial because one of them was -- and the ledger is the + * only place the fact is held per period. + * + * It is there because `filed-return-artifact-unavailable:` survives + * `parseDurableFiledReturnsSignals`, so a terminal target keeps it. No new + * persisted field was needed: the information was already stored, on the + * record that already scopes it correctly, and only the derivation was reading + * the wrong one. + */ +function targetMissedAnArtifact(target: FiledReturnsFullFiscalYearTarget): boolean { + // `?? []` matching the two existing reads in the validation module. A target + // without signals fails validation and cannot come from storage, but this path + // also summarises ledgers straight from the runner, and an exception here + // takes the panel down rather than reporting anything. + return (target.safeSignals ?? []).some((signal) => + signal.startsWith("filed-return-artifact-unavailable:"), + ); +} + export function toFullFiscalYearSummary( ledger: FiledReturnsFullFiscalYearLedger, flowStep: PortalFlowStepResult, diff --git a/src/background/filed-returns-full-fiscal-year-validation.ts b/src/background/filed-returns-full-fiscal-year-validation.ts index eb107ed1..9d17037e 100644 --- a/src/background/filed-returns-full-fiscal-year-validation.ts +++ b/src/background/filed-returns-full-fiscal-year-validation.ts @@ -4,6 +4,7 @@ import type { FiledReturnsFullFiscalYearTarget, FiledReturnsFullFiscalYearTargetStatus, } from "../connectors/gst/filed-returns-contracts"; +import { isCleanedZipPhase, CLEANED_ZIP_PHASES } from "../connectors/gst/filed-returns-contracts"; import { isFiledReturnsArtifactType, normaliseFiledReturnsArtifactType, @@ -154,7 +155,7 @@ const VALID_ZIP_PHASES = new Set @@ -167,7 +168,7 @@ const ZIP_PHASES_REQUIRING_COMPLETED_TARGETS = new Set< "downloaded-cleanup-pending", "no-artifacts-cleanup-pending", "legacy-cleanup-pending", - "cleaned", + ...CLEANED_ZIP_PHASES, ]); const COMPLETED_TARGET_STATUSES = new Set([ "downloaded", @@ -237,10 +238,10 @@ export function isFullFiscalYearLedger(input: unknown): input is FiledReturnsFul return false; } if (!isValidZipDownloadAttempt(ledger)) return false; - if (ledger.zipPhase === "cleaned" && ledger.status !== "complete") return false; + if (isCleanedZipPhase(ledger.zipPhase) && ledger.status !== "complete") return false; if ( ledger.zipPhase !== undefined && - ledger.zipPhase !== "cleaned" && + !isCleanedZipPhase(ledger.zipPhase) && ledger.status !== "blocked" ) { return false; diff --git a/src/background/local-data.ts b/src/background/local-data.ts index e9c69a52..96cbcca3 100644 --- a/src/background/local-data.ts +++ b/src/background/local-data.ts @@ -1,5 +1,6 @@ import { browser } from "wxt/browser"; import type { FiledReturnsFullFiscalYearLedger } from "../connectors/gst/filed-returns-contracts"; +import { isCleanedZipPhase } from "../connectors/gst/filed-returns-contracts"; import type { PackMessageResponse } from "../connectors/gst/messages"; import { readActiveFiledReturnsRunStorageState, @@ -127,7 +128,7 @@ async function hasUnresolvedFiledReturnsRecoveryState(deps: PackLocalDataDeps): function hasUnresolvedZipState(ledger: FiledReturnsFullFiscalYearLedger): boolean { if (ledger.zipDownloadAttempt !== undefined) return true; - return ledger.zipPhase !== undefined && ledger.zipPhase !== "cleaned"; + return ledger.zipPhase !== undefined && !isCleanedZipPhase(ledger.zipPhase); } function isUnresolvedFullFiscalYearLedger(ledger: FiledReturnsFullFiscalYearLedger): boolean { diff --git a/src/connectors/gst/filed-returns-contracts.ts b/src/connectors/gst/filed-returns-contracts.ts index 03523bc6..1d4c923b 100644 --- a/src/connectors/gst/filed-returns-contracts.ts +++ b/src/connectors/gst/filed-returns-contracts.ts @@ -227,6 +227,32 @@ export interface FiledReturnsFullFiscalYearTarget { updatedAt: string; } +/** + * The terminal cleanup phases. One predicate rather than a comparison repeated + * at each call site: splitting `cleaned` by origin turned every existing + * `=== "cleaned"` into a check that silently stopped matching three quarters of + * the runs it used to. + */ +export const CLEANED_ZIP_PHASES = [ + "cleaned", + "cleaned-after-download", + "cleaned-without-export", + "cleaned-legacy", +] as const; + +export function isCleanedZipPhase( + phase: FiledReturnsFullFiscalYearLedger["zipPhase"], +): phase is (typeof CLEANED_ZIP_PHASES)[number] { + return CLEANED_ZIP_PHASES.includes(phase as (typeof CLEANED_ZIP_PHASES)[number]); +} + +/** Whether cleanup followed a ZIP the browser confirmed it received. */ +export function zipPhaseProvesDelivery( + phase: FiledReturnsFullFiscalYearLedger["zipPhase"], +): boolean { + return phase === "cleaned-after-download"; +} + export interface FiledReturnsFullFiscalYearLedger { schemaVersion: "1.0"; planVersion?: string; @@ -245,6 +271,18 @@ export interface FiledReturnsFullFiscalYearLedger { | "downloaded-cleanup-pending" | "no-artifacts-cleanup-pending" | "legacy-cleanup-pending" + // Cleanup preserves which of the three pending phases it came from, because + // that origin is the only durable evidence of whether the ZIP reached the + // browser. Collapsing them into one `cleaned` made a confirmed delivery, a + // run that produced no ZIP, and a legacy staging cleared on upgrade + // indistinguishable afterwards -- and an evidence claim built on the + // collapsed value called never-exported files saved. + | "cleaned-after-download" + | "cleaned-without-export" + | "cleaned-legacy" + // Retained so a ledger written before the split still parses. It carries no + // origin, so it stays indeterminate: such a run reads as captured rather + // than saved, which is the safe direction and the behaviour it already had. | "cleaned"; zipDownloadAttempt?: { requestedAt: string; @@ -280,7 +318,21 @@ export interface FiledReturnsFullFiscalYearLedger { * `needs-review` beside the failures. */ export type FiledReturnsTargetOutcome = - "saved" | "captured" | "not-filed" | "needs-review" | "running" | "pending"; + | "saved" + // Some of the selection arrived and some did not. A multi-artifact target + // reaches `downloaded` when one artifact staged and another was explicitly + // unavailable, because an unavailable artifact is a resolved outcome -- so + // `saved` claimed the whole selection for a period that only had part of it. + // + // Distinct from `needs-review` on purpose: an artifact the portal never + // offered is not a fault a re-run corrects, and routing it to review would + // send someone looking for a problem that is not theirs. + | "partly-saved" + | "captured" + | "not-filed" + | "needs-review" + | "running" + | "pending"; export interface FiledReturnsTargetEvidence { period: string; diff --git a/src/entrypoints/popup/target-evidence.tsx b/src/entrypoints/popup/target-evidence.tsx index 084e986f..82d99d2c 100644 --- a/src/entrypoints/popup/target-evidence.tsx +++ b/src/entrypoints/popup/target-evidence.tsx @@ -21,6 +21,7 @@ import type { const OUTCOME_LABELS: Readonly> = { saved: "Saved", + "partly-saved": "Partly saved", captured: "Captured", "not-filed": "Not filed", "needs-review": "Needs review", @@ -33,6 +34,9 @@ const OUTCOME_LABELS: Readonly> = { // cannot separate the hues. const OUTCOME_GLYPHS: Readonly> = { saved: "✓", + // Half of a tick: some of the selection arrived. Distinct from the review + // mark, because nothing here is wrong -- the portal did not offer the rest. + "partly-saved": "◐", // A filled mark for a file Pack holds, an outline for one the browser has // confirmed. The difference is the whole point of the column. captured: "•", @@ -47,6 +51,7 @@ export function TargetEvidence({ summary }: { summary: FiledReturnsFlowSummary | if (!evidence || evidence.length === 0) return null; const saved = evidence.filter((entry) => entry.outcome === "saved").length; + const partlySaved = evidence.filter((entry) => entry.outcome === "partly-saved").length; const captured = evidence.filter((entry) => entry.outcome === "captured").length; const needsReview = evidence.filter((entry) => entry.outcome === "needs-review").length; @@ -59,6 +64,13 @@ export function TargetEvidence({ summary }: { summary: FiledReturnsFlowSummary | {saved} of {evidence.length} saved + {/* Counted separately rather than folded into either neighbour. A partly + saved period is not in the `saved` total, so without its own clause + it would simply disappear from this line and leave the reader + unable to account for the difference. */} + {partlySaved > 0 ? ( + · {partlySaved} partly saved + ) : null} {captured > 0 ? ( · {captured} captured, ZIP not confirmed ) : null} diff --git a/src/styles/panel.css b/src/styles/panel.css index 387a4f36..dc9bae8d 100644 --- a/src/styles/panel.css +++ b/src/styles/panel.css @@ -216,6 +216,13 @@ html[data-pack-surface="panel"] body { color: var(--pack-success-fg); } +/* The same success hue as `saved`, because nothing is wrong: the portal did not + offer the rest of the selection. The glyph and the word carry the difference, + so a reader who cannot separate the hues still reads it. */ +.evidence-partly-saved .evidence-glyph { + color: var(--pack-success-fg); +} + .evidence-needs-review .evidence-glyph, .evidence-needs-review .evidence-outcome { color: var(--pack-warning-fg); From 7e54ad16f1f2d2b00509e0290b5a7a412a1535e9 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Sun, 23 Aug 2026 23:16:38 +0530 Subject: [PATCH 2/7] test(panel): pin the cleanup origin and the partial-period outcome `completeFullFiscalYearLedger` had no test at all, so the value it writes -- now the only durable record of whether the ZIP reached the browser -- could be changed without a single failure. Covered per route, plus the no-pending-phase case that must not be guessed into a delivery. The evidence fixture built its targets behind a blanket cast and omitted every field the outcome did not read, so it never exercised the record the runtime is given: the first derivation to read one of those fields threw on all fifteen tests in the file at once. The targets are faithful now. Each addition mutation-checked: reverting the origin-carrying write, the partly-saved branch, or the status-line clause fails one test each, with the rest of the file passing. Co-Authored-By: Claude Opus 5 --- .../filed-returns-target-evidence.test.ts | 95 +++++++++++++++++++ .../full-fiscal-year-completion-phase.test.ts | 82 ++++++++++++++++ tests/popup/target-evidence.test.tsx | 27 ++++++ 3 files changed, 204 insertions(+) create mode 100644 tests/background/full-fiscal-year-completion-phase.test.ts diff --git a/tests/background/filed-returns-target-evidence.test.ts b/tests/background/filed-returns-target-evidence.test.ts index be47d8d7..0a1a4ddd 100644 --- a/tests/background/filed-returns-target-evidence.test.ts +++ b/tests/background/filed-returns-target-evidence.test.ts @@ -40,10 +40,21 @@ function ledgerWith( status: "partial", createdAt: "2026-08-23T12:00:00.000Z", updatedAt: "2026-08-23T12:00:00.000Z", + // Faithful targets, not a cast over a partial shape. The previous fixture + // omitted every field the outcome does not read, so nothing here exercised + // the record the runtime is actually given -- and the first derivation to + // read one of those fields threw on every test in the file at once. targets: statuses.map((status, index) => ({ targetId: `t${index}`, + financialYear: "2026-27", period: periods[index]!, + returnType: "GSTR-3B" as const, + artifactType: "PDF" as const, status, + attempts: 1, + safeSignals: [] as string[], + safeMessage: "", + updatedAt: "2026-08-23T12:00:00.000Z", })), } as FiledReturnsFullFiscalYearLedger; } @@ -149,6 +160,90 @@ describe("per-target evidence in the flow summary", () => { expect(summary.targetEvidence?.map((entry) => entry.outcome)).toEqual(["captured", "captured"]); }); + // Cleanup now keeps which pending phase it came from, so a delivered run still + // reads as saved after the panel is reopened and the transient signal is gone. + // This is the whole point of the split: the fact is recorded rather than + // inferred back from a value that had already discarded it. + it("reads a delivered run as saved from the durable phase alone", () => { + const ledger = ledgerWith(["downloaded", "downloaded"]); + const delivered = { + ...ledger, + status: "complete" as const, + zipPhase: "cleaned-after-download" as const, + }; + + const summary = toFullFiscalYearSummary(delivered, FLOW_STEP); + + expect(summary.targetEvidence?.map((entry) => entry.outcome)).toEqual(["saved", "saved"]); + }); + + // The two siblings are not deliveries and must not become one. `cleaned-legacy` + // is the case that previously read as saved while the files had been deleted + // without ever being exported. + it("does not read the non-delivery cleanup phases as saved", () => { + for (const zipPhase of ["cleaned-without-export", "cleaned-legacy"] as const) { + const ledger = ledgerWith(["downloaded", "downloaded"]); + + const summary = toFullFiscalYearSummary( + { ...ledger, status: "complete" as const, zipPhase }, + FLOW_STEP, + ); + + expect( + summary.targetEvidence?.map((entry) => entry.outcome), + zipPhase, + ).toEqual(["captured", "captured"]); + } + }); + + // A multi-artifact period where the portal offered one format and not another + // reaches `downloaded`, because an unavailable artifact is a resolved outcome. + // Reporting that as "Saved" claimed the whole selection for a period that only + // had part of it. + it("reads a period that missed one selected artifact as partly saved", () => { + const ledger = ledgerWith(["downloaded", "downloaded"]); + const partial = { + ...ledger, + status: "complete" as const, + zipPhase: "cleaned-after-download" as const, + targets: ledger.targets.map((target, index) => + index === 0 + ? { + ...target, + safeSignals: [...target.safeSignals, "filed-return-artifact-unavailable:EXCEL"], + } + : target, + ), + }; + + const summary = toFullFiscalYearSummary(partial, FLOW_STEP); + + // The second period is untouched: the fact is per target, so one incomplete + // period does not defame the rest of the year. + expect(summary.targetEvidence?.map((entry) => entry.outcome)).toEqual([ + "partly-saved", + "saved", + ]); + }); + + // Delivery is the stronger question. Before the ZIP reaches the browser there + // is nothing to be partly saved, and saying so would assert a handover that + // has not happened. + it("reports an undelivered partial period as captured, not partly saved", () => { + const ledger = ledgerWith(["downloaded"]); + const partial = { + ...ledger, + targets: ledger.targets.map((target) => ({ + ...target, + safeSignals: [...target.safeSignals, "filed-return-artifact-unavailable:EXCEL"], + })), + }; + + const summary = toFullFiscalYearSummary(partial, FLOW_STEP); + + expect(summary.targetEvidence?.map((entry) => entry.outcome)).toEqual(["captured"]); + }); + // A run that found nothing eligible also cleans, having never produced a ZIP. // It has no downloaded target for a delivery claim to attach to, and must not // manufacture one. diff --git a/tests/background/full-fiscal-year-completion-phase.test.ts b/tests/background/full-fiscal-year-completion-phase.test.ts new file mode 100644 index 00000000..842a3ca3 --- /dev/null +++ b/tests/background/full-fiscal-year-completion-phase.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import { + completeFullFiscalYearLedger, + createFullFiscalYearLedger, +} from "../../src/background/filed-returns-full-fiscal-year-ledger"; +import { + FULL_FISCAL_YEAR_PERIOD, + type FiledReturnsMonth, +} from "../../src/connectors/gst/filed-returns-scope"; +import type { FiledReturnsFullFiscalYearLedger } from "../../src/connectors/gst/filed-returns-contracts"; + +const PERIODS: readonly FiledReturnsMonth[] = [ + "April", + "May", + "June", + "July", + "August", + "September", + "October", + "November", + "December", + "January", + "February", + "March", +]; + +// Nothing covered what completion writes, so the value could be changed without +// a single failure -- and that value is now the only durable record of whether +// the ZIP reached the browser. +describe("full-fiscal-year completion phase", () => { + // Each pending phase has exactly one terminal form, so the route that reached + // cleanup survives the transition that used to erase it. + it("keeps the origin of each cleanup route", () => { + const routes = [ + ["downloaded-cleanup-pending", "cleaned-after-download"], + ["no-artifacts-cleanup-pending", "cleaned-without-export"], + ["legacy-cleanup-pending", "cleaned-legacy"], + ] as const; + + for (const [pending, terminal] of routes) { + const completed = completeFullFiscalYearLedger( + completable({ zipPhase: pending }), + new Date("2026-08-23T12:00:00.000Z"), + ); + + expect(completed.zipPhase, pending).toBe(terminal); + expect(completed.status, pending).toBe("complete"); + } + }); + + // A phase this build does not recognise, or none at all, must not be guessed + // into a delivery. The origin-less value reads as indeterminate downstream, + // which is what an unknown route should produce. + it("does not invent an origin when there is no pending phase", () => { + const completed = completeFullFiscalYearLedger( + completable({}), + new Date("2026-08-23T12:00:00.000Z"), + ); + + expect(completed.zipPhase).toBe("cleaned"); + }); +}); + +function completable( + overrides: Partial, +): FiledReturnsFullFiscalYearLedger { + const ledger = createFullFiscalYearLedger( + { + artifactType: "PDF", + financialYear: "2026-27", + period: FULL_FISCAL_YEAR_PERIOD, + returnType: "GSTR-3B", + }, + new Date("2026-08-01T00:00:00.000Z"), + PERIODS, + ); + return { + ...ledger, + targets: ledger.targets.map((target) => ({ ...target, status: "downloaded" as const })), + ...overrides, + }; +} diff --git a/tests/popup/target-evidence.test.tsx b/tests/popup/target-evidence.test.tsx index faf16068..d6afda05 100644 --- a/tests/popup/target-evidence.test.tsx +++ b/tests/popup/target-evidence.test.tsx @@ -86,4 +86,31 @@ describe("per-target evidence", () => { expect(renderToStaticMarkup()).toBe(""); expect(renderToStaticMarkup()).toBe(""); }); + // A partly saved period is in none of the other three counts, so without its + // own clause the header would say "1 of 2 saved" and leave the second period + // unaccounted for anywhere on the line. + it("accounts for a partly saved period in the status line", () => { + const markup = renderToStaticMarkup( + , + ); + + expect(markup).toContain("1 of 2 saved"); + expect(markup).toContain("1 partly saved"); + }); + + // The word carries the meaning, not the hue: the row reads the same to someone + // who cannot separate the colours. + it("names the partly saved outcome in the row", () => { + const markup = renderToStaticMarkup( + , + ); + + expect(markup).toContain("Partly saved"); + expect(markup).toContain("evidence-partly-saved"); + }); }); From 0d5e7322c1616d3e7d1ffd892af8d3bc5e0efb6f Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Sun, 23 Aug 2026 23:24:38 +0530 Subject: [PATCH 3/7] fix(panel): drop a fallback that answered "unknown" with the stronger claim `(target.safeSignals ?? []).some(...)` reads as caution and is the opposite. Absent signals would mean no evidence of a missing artifact, which resolves to `saved` -- a fully saved period -- on the one field that decides whether the period was complete. That is "could not determine" answering "matches", on the claim the column exists to keep honest. It was also the odd one out: `hasLegacyRetainedStaging`, ten lines above in the same file, dereferences the same field unguarded. The field is required by the type, set by every construction path, and its absence fails ledger validation, so neither can be reached with a real record. A malformed one throwing is diagnosable; a malformed one silently reading as saved is not. Co-Authored-By: Claude Opus 5 --- .../filed-returns-full-fiscal-year-summary.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/background/filed-returns-full-fiscal-year-summary.ts b/src/background/filed-returns-full-fiscal-year-summary.ts index d4bedd81..00fab3b7 100644 --- a/src/background/filed-returns-full-fiscal-year-summary.ts +++ b/src/background/filed-returns-full-fiscal-year-summary.ts @@ -359,11 +359,17 @@ export function fullFiscalYearTargetEvidence( * the wrong one. */ function targetMissedAnArtifact(target: FiledReturnsFullFiscalYearTarget): boolean { - // `?? []` matching the two existing reads in the validation module. A target - // without signals fails validation and cannot come from storage, but this path - // also summarises ledgers straight from the runner, and an exception here - // takes the panel down rather than reporting anything. - return (target.safeSignals ?? []).some((signal) => + // Dereferenced without a fallback, like `hasLegacyRetainedStaging` above it. + // A `?? []` here would read as caution and is the opposite: absent signals + // would mean no evidence of a gap, which resolves to the *stronger* claim of a + // fully saved period. That is "could not determine" answering "matches", on + // the one field that decides the difference. + // + // The field is required by the type, set by every construction path, and its + // absence fails ledger validation, so this cannot be reached with a real + // record. A malformed one throwing here is diagnosable; a malformed one + // silently reading as saved is not. + return target.safeSignals.some((signal) => signal.startsWith("filed-return-artifact-unavailable:"), ); } From 40ea54d48c1d3ab356b5c13b2da98b97d0fae3b7 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Sun, 23 Aug 2026 23:52:56 +0530 Subject: [PATCH 4/7] fix(flow): stop a re-cleaned run being promoted to the delivery route `markFullFiscalYearCleanupPending` defaulted its phase argument to `downloaded-cleanup-pending` -- the one value that asserts the ZIP reached the browser. One call site relied on that default: retrying the OPFS discard of an already-completed same-scope ledger. So a run that had cleaned as `cleaned-legacy` or `cleaned-without-export` was re-marked onto the delivery route, and completion then wrote `cleaned-after-download` for files that were never exported. That is the overclaim the origin split exists to prevent, reintroduced through the transition meant to preserve it -- and it was invisible because the assertion was made by a default nobody had to type. Two changes, because either alone leaves the mechanism: - The default is gone. A phase that asserts a delivery must be chosen, not inherited by a caller that did not consider the argument. - That call site derives its route from the phase the ledger already reached, via `cleanupPendingPhaseFor`, which never upgrades. A pre-split `cleaned` carries no origin and takes the non-delivery route, so it keeps reading as captured exactly as it did before. Co-Authored-By: Claude Opus 5 --- .../filed-returns-full-fiscal-year-staging.ts | 27 ++++++++++++++++--- .../filed-returns-full-fiscal-year.ts | 11 +++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/background/filed-returns-full-fiscal-year-staging.ts b/src/background/filed-returns-full-fiscal-year-staging.ts index d117a0e5..b16888de 100644 --- a/src/background/filed-returns-full-fiscal-year-staging.ts +++ b/src/background/filed-returns-full-fiscal-year-staging.ts @@ -153,13 +153,34 @@ export function createFullFiscalYearCleanupPendingState( }; } +/** + * The cleanup route to retry for a ledger that has already been cleaned once. + * + * Never upgrades. Re-cleaning a completed ledger must end on the terminal phase + * it already had, because that phase is the delivery evidence -- and resetting + * it to the delivery route relabelled a run that never exported anything as one + * whose ZIP the browser confirmed. + * + * A pre-split `cleaned` carries no origin, so it takes the non-delivery route + * and stays reading as captured, which is what it did before. + */ +export function cleanupPendingPhaseFor( + cleaned: FiledReturnsFullFiscalYearLedger["zipPhase"], +): "downloaded-cleanup-pending" | "no-artifacts-cleanup-pending" | "legacy-cleanup-pending" { + if (cleaned === "cleaned-after-download") return "downloaded-cleanup-pending"; + if (cleaned === "cleaned-without-export") return "no-artifacts-cleanup-pending"; + return "legacy-cleanup-pending"; +} + export function markFullFiscalYearCleanupPending( ledger: FiledReturnsFullFiscalYearLedger, now: Date, + // No default. `downloaded-cleanup-pending` was the default value, and it is + // the one phase that asserts the ZIP reached the browser -- so a caller that + // simply did not think about the argument claimed a delivery. Every call site + // now states which route it is on. zipPhase: - | "downloaded-cleanup-pending" - | "no-artifacts-cleanup-pending" - | "legacy-cleanup-pending" = "downloaded-cleanup-pending", + "downloaded-cleanup-pending" | "no-artifacts-cleanup-pending" | "legacy-cleanup-pending", ): FiledReturnsFullFiscalYearLedger { const requestedAt = ledger.zipDownloadAttempt?.requestedAt; const cleanupPendingLedger: FiledReturnsFullFiscalYearLedger = { diff --git a/src/background/filed-returns-full-fiscal-year.ts b/src/background/filed-returns-full-fiscal-year.ts index 4953e029..eb4228a2 100644 --- a/src/background/filed-returns-full-fiscal-year.ts +++ b/src/background/filed-returns-full-fiscal-year.ts @@ -48,6 +48,7 @@ import { createFullFiscalYearCleanupPendingState, finishFullFiscalYearCleanup, mergeRetriedArtifactSignals, + cleanupPendingPhaseFor, markFullFiscalYearCleanupPending, markFullFiscalYearRestagingRequired, markFullFiscalYearZipDownloadIntent, @@ -222,7 +223,15 @@ export async function startFullFiscalYearDownloadFlow( if (existingLedger && replaceCompletedSameScopeLedger) { const clearSignals = await discardFullFiscalYearFiledReturnsZip(existingLedger.ledgerId); if (!clearSignals.includes("full-fiscal-year-opfs-cleared")) { - const cleanupPendingLedger = markFullFiscalYearCleanupPending(existingLedger, now); + // Derived from the phase this ledger already reached, never reset. This + // call took the default before, which is `downloaded-cleanup-pending` -- + // so retrying the cleanup of a run that never exported a ZIP promoted it + // to the delivery route and, on completion, to `cleaned-after-download`. + const cleanupPendingLedger = markFullFiscalYearCleanupPending( + existingLedger, + now, + cleanupPendingPhaseFor(existingLedger.zipPhase), + ); const step = completedRunCleanupBlockedStep(cleanupPendingLedger, clearSignals); const summary = toFullFiscalYearSummary(cleanupPendingLedger, step); await persistLedgerAndSummary(deps, cleanupPendingLedger, step); From 2c51f20929bd02099f2b6230b03ebab2a2bab947 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Sun, 23 Aug 2026 23:52:56 +0530 Subject: [PATCH 5/7] test(flow): pin that re-cleaning never upgrades the delivery evidence Covers all four starting phases through the retry route and back out of completion. Collapsing `cleanupPendingPhaseFor` to the delivery route fails this test and only this test. Co-Authored-By: Claude Opus 5 --- .../full-fiscal-year-completion-phase.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/background/full-fiscal-year-completion-phase.test.ts b/tests/background/full-fiscal-year-completion-phase.test.ts index 842a3ca3..02007399 100644 --- a/tests/background/full-fiscal-year-completion-phase.test.ts +++ b/tests/background/full-fiscal-year-completion-phase.test.ts @@ -3,6 +3,7 @@ import { completeFullFiscalYearLedger, createFullFiscalYearLedger, } from "../../src/background/filed-returns-full-fiscal-year-ledger"; +import { cleanupPendingPhaseFor } from "../../src/background/filed-returns-full-fiscal-year-staging"; import { FULL_FISCAL_YEAR_PERIOD, type FiledReturnsMonth, @@ -48,6 +49,32 @@ describe("full-fiscal-year completion phase", () => { } }); + // Re-cleaning a completed ledger must not promote it. The retry path took the + // default cleanup phase, which is the delivery route, so a run that never + // exported anything came back through completion as a confirmed delivery -- + // the overclaim the origin split exists to prevent, reached through the + // transition that was meant to preserve it. + it("does not upgrade a re-cleaned ledger to the delivery route", () => { + const routes = [ + ["cleaned-legacy", "legacy-cleanup-pending", "cleaned-legacy"], + ["cleaned-without-export", "no-artifacts-cleanup-pending", "cleaned-without-export"], + ["cleaned-after-download", "downloaded-cleanup-pending", "cleaned-after-download"], + // Pre-split, no origin: takes the non-delivery route and stays captured. + ["cleaned", "legacy-cleanup-pending", "cleaned-legacy"], + ] as const; + + for (const [alreadyCleaned, retryPhase, terminal] of routes) { + expect(cleanupPendingPhaseFor(alreadyCleaned), alreadyCleaned).toBe(retryPhase); + + const recompleted = completeFullFiscalYearLedger( + completable({ zipPhase: retryPhase }), + new Date("2026-08-23T12:00:00.000Z"), + ); + + expect(recompleted.zipPhase, alreadyCleaned).toBe(terminal); + } + }); + // A phase this build does not recognise, or none at all, must not be guessed // into a delivery. The origin-less value reads as indeterminate downstream, // which is what an unknown route should produce. From cc2b1be9c166b23fb3ded770499329dd9a2a3214 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Mon, 24 Aug 2026 10:56:28 +0530 Subject: [PATCH 6/7] fix(panel): restore the delivery signal instead of teaching one reader Found by a live authenticated run: twelve periods read "Saved" underneath a banner saying Pack could not confirm the browser had saved the ZIP, and a pack line saying the same. One screen contradicting itself, with the overclaiming half being the one this branch added. Five things ask whether the ZIP reached the browser and all five ask it of the summary step's signals -- the panel banner, the pack summary line, two durable status derivations, and the per-period evidence. Making only the evidence read the durable phase put it at odds with the other four on reopen, when the transient signal is gone. So the signal is restored where it was lost. `completeFullFiscalYearStep` carries it when the ledger's cleanup phase records a delivery, which makes a step built after a restart indistinguishable from the one the observing run emitted -- and every reader correct without learning about phases. Patching the banner instead would have left three more readers wrong and a second copy of the rule. Provenance, because it matters more than the credit: this change was already present, uncommitted, in the lane worktree when I came to fix the reported defect, authored by another agent working in the same checkout -- which `AGENTS.md` warns against for exactly the confusion it caused here. I verified it against the five readers and the suite rather than assuming it, and kept it because restoring the fact at its source is the right shape. The regression test is mine. Co-Authored-By: Claude Opus 5 --- .../filed-returns-full-fiscal-year-summary.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/background/filed-returns-full-fiscal-year-summary.ts b/src/background/filed-returns-full-fiscal-year-summary.ts index 00fab3b7..66e888da 100644 --- a/src/background/filed-returns-full-fiscal-year-summary.ts +++ b/src/background/filed-returns-full-fiscal-year-summary.ts @@ -409,6 +409,22 @@ export function toFullFiscalYearSummary( }; } +/** + * The step a re-summarisation builds from the ledger alone. + * + * It carries the delivery signal when the ledger's cleanup phase records one. + * Five places ask "did the ZIP reach the browser" and every one of them asks it + * of this step's signals -- the panel banner, the pack summary line, two durable + * status derivations, and the per-period evidence. Teaching only the evidence to + * read the durable phase made it disagree with the other four: a live run + * reported twelve periods `Saved` beneath a banner saying Pack could not confirm + * the browser had the ZIP, which is one screen contradicting itself. + * + * So the fact is restored where it was lost, not patched into each reader. The + * signal is the existing canonical one and survives durable parsing, so a step + * built after a restart is indistinguishable from the one the observing run + * emitted -- which is the point. + */ export function completeFullFiscalYearStep( ledger: FiledReturnsFullFiscalYearLedger, ): PortalFlowStepResult { @@ -416,7 +432,10 @@ export function completeFullFiscalYearStep( connectorId: "gst", scopeId: filedReturnsScopeId(ledger.scope.returnType), state: "downloaded", - safeSignals: ["full-fiscal-year-complete"], + safeSignals: [ + "full-fiscal-year-complete", + ...(zipPhaseProvesDelivery(ledger.zipPhase) ? ["full-fiscal-year-zip-downloaded"] : []), + ], safeMessage: `Pack completed the local full fiscal year run for FY ${ledger.scope.financialYear}.`, }; } From bdf721ffa254c991884ae6dc62ae4614e67192a7 Mon Sep 17 00:00:00 2001 From: Tapish Khandelwal Date: Mon, 24 Aug 2026 10:56:28 +0530 Subject: [PATCH 7/7] test(panel): pin that every reader answers the delivery question alike The suite was green with the reported defect present, because nothing asserted that the banner and the per-period column agree -- they read one signal, and only their disagreement was user-visible. Reverting the signal restoration fails this test and only this test, so it reproduces what the live run found rather than describing it. Co-Authored-By: Claude Opus 5 --- .../full-fiscal-year-reopen-agreement.test.ts | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/background/full-fiscal-year-reopen-agreement.test.ts diff --git a/tests/background/full-fiscal-year-reopen-agreement.test.ts b/tests/background/full-fiscal-year-reopen-agreement.test.ts new file mode 100644 index 00000000..502e9a8d --- /dev/null +++ b/tests/background/full-fiscal-year-reopen-agreement.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from "vitest"; +import { summariseFullFiscalYearLedger } from "../../src/background/filed-returns-full-fiscal-year-summary"; +import { createFullFiscalYearLedger } from "../../src/background/filed-returns-full-fiscal-year-ledger"; +import { + FULL_FISCAL_YEAR_PERIOD, + type FiledReturnsMonth, +} from "../../src/connectors/gst/filed-returns-scope"; +import type { FiledReturnsFullFiscalYearLedger } from "../../src/connectors/gst/filed-returns-contracts"; + +const PERIODS: readonly FiledReturnsMonth[] = [ + "April", + "May", + "June", + "July", + "August", + "September", + "October", + "November", + "December", + "January", + "February", + "March", +]; + +// Five things ask whether the ZIP reached the browser, and every one of them +// asks it of the summary step's signals: the panel banner, the pack summary +// line, two durable status derivations, and the per-period evidence. +// +// Teaching only the evidence to read the durable phase made it disagree with +// the other four. A live run showed twelve periods "Saved" underneath a banner +// saying Pack could not confirm the browser had the ZIP -- one screen +// contradicting itself, with the overclaiming half being the new one. +// +// Nothing pinned that they agree, which is why a green suite said nothing about +// it. This is that pin: the reopened summary must answer the question the same +// way for every reader, because they all read one signal. +describe("full-fiscal-year summary agreement after reopen", () => { + it("reports delivery to the banner and the evidence together", () => { + const summary = summariseFullFiscalYearLedger(cleaned("cleaned-after-download")); + + // What the banner and the pack summary line read. + expect(summary.flowStep.safeSignals).toContain("full-fiscal-year-zip-downloaded"); + // What the per-period column reads. + expect(new Set(summary.targetEvidence?.map((entry) => entry.outcome))).toEqual( + new Set(["saved"]), + ); + }); + + // The other direction, and the one that must never drift: a run that never + // exported must read unconfirmed in both places. + it("withholds delivery from both when cleanup was not a download", () => { + for (const zipPhase of ["cleaned-without-export", "cleaned-legacy", "cleaned"] as const) { + const summary = summariseFullFiscalYearLedger(cleaned(zipPhase)); + + expect(summary.flowStep.safeSignals, zipPhase).not.toContain( + "full-fiscal-year-zip-downloaded", + ); + expect(new Set(summary.targetEvidence?.map((entry) => entry.outcome)), zipPhase).toEqual( + new Set(["captured"]), + ); + } + }); +}); + +function cleaned( + zipPhase: NonNullable, +): FiledReturnsFullFiscalYearLedger { + const ledger = createFullFiscalYearLedger( + { + artifactType: "PDF", + financialYear: "2026-27", + period: FULL_FISCAL_YEAR_PERIOD, + returnType: "GSTR-3B", + }, + new Date("2026-08-01T00:00:00.000Z"), + PERIODS, + ); + return { + ...ledger, + status: "complete", + zipPhase, + targets: ledger.targets.map((target) => ({ ...target, status: "downloaded" as const })), + }; +}