fix(download): bind portal blob capture and cancellation to the Pack action - #112
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1221f7c5f
ℹ️ 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".
| } finally { | ||
| await browser.downloads.erase({ id: item.id }); |
There was a problem hiding this comment.
Keep completed portal downloads visible when cancellation fails
When the exact portal download completes before downloads.cancel() runs, cancellation rejects but this finally still erases its DownloadItem. Chrome's downloads.erase() removes only the browser-history entry, not the downloaded file, so a sensitive duplicate remains on disk while the record needed to locate and review it disappears; the listener's outer catch then suppresses the failure. Preserve the entry and route the cancellation failure to explicit review, or delete the confirmed file before erasing it.
AGENTS.md reference: AGENTS.md:L56-L59
Useful? React with 👍 / 👎.
| reason: captured?.reason ?? "generation-timeout", | ||
| safeSignals: captured?.safeSignals ?? [], | ||
| }; | ||
| await safetyNet.bind(captured.blobUrl); |
There was a problem hiding this comment.
Bind the safety net before requiring shim success
When suppression is actually missed—for example, if portal code invokes a cached native anchor click—the onCreated listener queues the Blob download, but the shim never reaches signal() and eventually returns generation-timeout. Both acquisition functions return from their !captured.ok branch without reaching this binding call, then remove the listener, so the uncontrolled portal file continues downloading; binding only after success covers the case where the shim already suppressed the activation. Preserve the recorded Blob identity on this failure path and bind or route the candidate to review before returning.
AGENTS.md reference: AGENTS.md:L56-L59
Useful? React with 👍 / 👎.
| expectedTarget?: { financialYear: string; period: string; returnType: string }; | ||
| timeoutMs?: number; | ||
| }; | ||
| const MAX_PORTAL_BLOB_BYTES = 25 * 1024 * 1024; |
There was a problem hiding this comment.
Derive the Blob size cap from the canonical validator
This new 25 MiB constant duplicates MAX_ARTIFACT_BYTES in src/connectors/gst/artifact-validation.ts. When either limit changes, the MAIN-world capture and canonical validation silently diverge: raising the validator limit still rejects otherwise accepted artifacts here, while lowering it permits unnecessary large serialization before rejection. Export the canonical limit and pass it through PortalBlobShimInput rather than maintaining a second numeric fact.
AGENTS.md reference: AGENTS.md:L41-L43
Useful? React with 👍 / 👎.
Summary
blob:download in the tab.arrayBuffer()/base64 serialization.Root Cause / Decision Record
The old tab-plus-
blob:predicate could cancel and erase an unrelated concurrent download. This is the smallest safe fix: hold same-tab Blob creations only in memory, derive the captured URL fingerprint in extension context, and act only on an exact match. Closes #97 and addresses the twoportal-blob-shim.tsitems in #101.Scope
Privacy And Data-Flow Impact
Sensitive Surface Review
Chrome Web Store Impact
Verification
pnpm install --frozen-lockfilenode scripts/run-dependency-audit.mjspnpm exec wxt preparepnpm exec prettier --check .pnpm exec eslint . --max-warnings 0pnpm exec tsc --noEmitpnpm exec vitest run— 210 files, 1,408 tests passedpnpm exec wxt buildnode scripts/verify-extension-package.mjs .output/chrome-mv3pnpm exec wxt zipnode scripts/verify-extension-zip.mjsgit diff --checkpnpm review:gate ... --wait-head-review-ms 180000 --allow-missing-head-review: executed; no current-head Codex review was returned, recorded below as an audit gap.Artifact Evidence
71f4d54a717cb6b4448e132ac5f9f7c4c1c9318599ba371dae8bf45afa5f322b.d1221f7c5fe164e7e1c67e7a7dd3c6decf697ec3.PR Review Follow-Up
pnpm review:gatewaited 180 seconds and reported no review ford1221f7Screenshots
Not applicable; behavior is background/MAIN-world contract only and tests use synthetic values.
DCO
Pack Workflow Preflight
pnpm workflow:preflightwas run before editing/push, or the skip reason is documented.Sanchika Adoption Gate
@sanchika/*packages or copied Sanchika guidance, Iread
sanchika/docs/adoption-pack.mdin the coordinated parent worktree.Not applicable: this PR consumes no Sanchika packages or guidance.
and records the Sanchika commit or copied guidance used. Not applicable.
../sanchika,sanchika/packages/*/src, or parentsource paths.