Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a non-trivial auth/transfer state machine in the background script and the PR itself notes remaining manual Firefox/OAuth acceptance verification.
Pull request overview
This PR improves the extension’s background authentication/transfer flow so a link selected while signed out is durably saved and then resumed after sign-in, including MV3 worker restarts, while avoiding duplicate transfer POSTs in ambiguous failure scenarios.
Changes:
- Reworks
src/background.jsto persist a singlependingTransferaction with phases (ready/sending/uncertain) and adds notification-driven recovery paths. - Adds deterministic Node-based background-flow tests (mocked browser + HTTP boundaries) and wires them into
pnpm run check/pnpm test. - Extends English locale strings and contributor docs to cover the new recovery behavior and verification steps.
File summaries
| File | Description |
|---|---|
| tests/background.test.mjs | Adds a comprehensive Node test harness covering durable pending-transfer/auth and restart scenarios (Chrome + Firefox namespaces). |
| src/background.js | Implements durable pending-transfer state machine, serialized operations, bounded auth retry, and notification-based recovery/cleanup. |
| src/_locales/en/messages.json | Adds new notification strings for auth unavailability, pending transfers, auth cancellation, and uncertain transfer outcomes. |
| package.json | Runs Node tests as part of check and adds a dedicated test script. |
| CONTRIBUTING.md | Documents the new deterministic background-flow test suite and manual authentication recovery checks. |
Review details
Suppressed comments (3)
src/_locales/en/messages.json:52
- New locale messages were added without "description" fields; keeping descriptions consistent helps translation and avoids undocumented UI strings.
"pendingTransferTitle": {
"message": "Finish your saved download"
},
"pendingTransferMessage": {
"message": "A link is already waiting. Click to retry that link before choosing another."
src/_locales/en/messages.json:58
- These new locale entries are missing "description" fields; the rest of this file includes descriptions for every message.
"authCancelledTitle": {
"message": "Sign-in was not completed"
},
"authCancelledMessage": {
"message": "No transfer was started. Use the context menu to try again."
src/_locales/en/messages.json:64
- These new locale entries are missing "description" fields; adding them keeps the locale file self-documenting like the existing messages.
"transferUncertainTitle": {
"message": "Check your transfers"
},
"transferUncertainMessage": {
"message": "This download may already have started. Click to check your transfers before trying again."
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: c548a21e50
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f9e0943a2
ℹ️ 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".
Classify the transfer POST once in startTransfer, share the failure notifications between the direct and recovery send paths, and hoist the pending-record validity check and startup staleness guard. Tests share the outage fixtures and label each variant. CONTRIBUTING lists the manual checks instead of restating the state machine.
Summary
Selecting a link while signed out saves that one link, and completing sign-in sends it once. Recovery survives a validation outage after OAuth by keeping the provisional token, so an explicit retry validates it instead of reopening sign-in. Cancellation, rejection, completion, and a 15-minute expiry clear the saved link.
Signed-in downloads still overlap. A transfer POST interrupted by a worker restart is never resent; its notification opens the transfers page and clears the saved link. Startup validation cannot discard a token obtained by a newer user action.
Verification
pnpm run check(lint plus 31 background-flow tests with mocked browser and HTTP boundaries) and both extension builds passNot verified
Manual Firefox behavior and real OAuth/context-menu acceptance. Firefox loaded the temporary MV2 build but inspection stopped on its
script.getRealmsinternal error.