fix(recovery): persist direct download completion proof - #135
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc4349c987
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (returnType === "GSTR-2B" && artifactType === "JSON") { | ||
| return endpointClass === "gstr2b-main-world-json-captured-download"; |
There was a problem hiding this comment.
With this stricter compatibility branch, GSTR-2B JSON is only valid with gstr2b-main-world-json-captured-download, but the exact-ID retry path still rebuilds captured single-artifact diagnostics through withFiledReturnsDownloadDiagnostic(), whose GSTR-2B captured branch emits gstr2b-portal-blob-captured-download. If the worker stops after a standalone GSTR-2B JSON download ID is saved but before the summary persists, reconcileFiledReturnsTargetDownload() observes the completed ID and then persistCanonicalSinglePeriodCompletion() drops the reconstructed flow step as non-canonical, leaving the target in review instead of completing. Keep the runtime endpoint mapping in the same canonical rule as this predicate.
AGENTS.md reference: AGENTS.md:L41-L47
Useful? React with 👍 / 👎.
| if (!(await persistSingleArtifactRecoveryDownloadId(scope, downloadId, deps))) { | ||
| throw new Error("single-artifact download ID checkpoint failed"); |
There was a problem hiding this comment.
If the storage.session write immediately above fails after chrome.downloads.download() returned an ID, this later durable write is never reached; downloadAcquiredArtifact() reports checkpoint-failed and the durable target review remains at the intent phase with no downloadId. After an extension reload or browser restart clears the session checkpoint, Pack can no longer reconcile the completed browser item even though the exact ID was available. Record the local target-review ID before depending on the session write outcome.
AGENTS.md reference: AGENTS.md:L117-L118
Useful? React with 👍 / 👎.
| flowStep: directCapturedArtifactFlowStep({ | ||
| artifactType, | ||
| downloadId: delivery.downloadId, | ||
| requestId, |
There was a problem hiding this comment.
This GSTR-3B success path now reports completion from delivery.downloadId, but both GSTR-3B direct branches still save that ID only in storage.session and never create the storage.local target-review attempt used by the new single-artifact recovery path below. If the service worker stops after the browser download finishes but before the summary is persisted, an extension reload/browser restart clears the only checkpoint; the next start has no retained review or exact ID to reconcile and can repeat a completed GSTR-3B PDF/JSON download. Persist the same durable target-review intent and download ID for these direct branches before relying on the session checkpoint.
AGENTS.md reference: AGENTS.md:L117-L118
Useful? React with 👍 / 👎.
| // download needs this exact-ID checkpoint for recovery. | ||
| const tracksBrowserDownload = !deps.stageCapturedDownloads; | ||
| if (tracksBrowserDownload) { | ||
| if (!(await persistSingleArtifactRecoveryIntent(scope, artifactType, requestId, deps))) { |
There was a problem hiding this comment.
Persisting this target-review intent before acquisition means failures that happen before any browser download is created (for example a GSTR-2B JSON fetch/preflight failure or a portal-blob scope mismatch) leave a durable download-intent-persisted review behind, while the finally path only clears the session checkpoint when retainCheckpointForRecovery is false. The next start then stops on that stale review and asks the user to cancel or manually review a download that Pack already determined did not need recovery. Clear the matching target review whenever the non-retained failure path clears the acquisition checkpoint.
Useful? React with 👍 / 👎.
1 participant
Summary
Make single-period GST return downloads durable across repeated downloads and extension reloads. This recovery lane now covers GSTR-3B PDF/JSON, GSTR-1 PDF/Excel, and GSTR-2B JSON without weakening exact download proof.
Root Cause / Decision Record
downloadAttemptcontract, persists opaque intent before browser creation, persists the exact ID after creation, and removes it only after canonical completion.Scope
Pack Workflow Preflight
pnpm workflow:preflightwas run before editing/push.Sanchika Adoption Gate
@sanchika/*packages or copied Sanchika guidance.../sanchika,sanchika/packages/*/src, or parent source paths.Privacy And Data-Flow Impact
Sensitive Surface Review
Chrome Web Store Impact
Verification
pnpm exec prettier --check .pnpm exec eslint . --max-warnings 0pnpm exec tsc --noEmitpnpm exec vitest runpassed in current-head Pack CI; local full suite remains blocked only by the missing pinned Chromium asset in two store-image tests.pnpm exec wxt buildnode scripts/verify-extension-package.mjs .output/chrome-mv3git diff --checkArtifact Evidence
dc4349c987ed76832e15878716732312f65a1ed5.PR Review Follow-Up
chatgpt-codex-connectorreview fordc4349cScreenshots
No visual redesign. Authenticated portal QA was completed separately; no live evidence is included.
DCO
Signed-off-by:trailers.