Repository navigation
fix(extension): sync-first user-script evaluate so CSP-strict pages work in Search - #34
Conversation
…rks in Search Search's userScripts.execute does not await a promise returned by the injected script, so the async wrapper came back with no result on CSP-strict pages (e.g. Todoist). Try a synchronous wrapper first and only fall back to the async one when the expression itself returns a thenable.
Codex Review Summary: ✅ CompletedThis 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7154ad0e7b
ℹ️ 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".
| const [first] = await api.execute<EvalOutcome>({ | ||
| js: [{ code }], | ||
| js: [{ code: asyncCode }], |
There was a problem hiding this comment.
Await the original promise instead of executing the expression again
On CSP-strict pages using the user-script fallback, any promise-returning expression now executes twice: syncCode starts the operation and discards its promise, then this fallback evaluates the same expression again. This affects the async fill/type helpers in cli-js/src/tool.js as well as user evaluations containing mutating requests, potentially duplicating input or server-side changes. Running the two generated wrappers with an async counter increment confirms two increments for one evaluation. Preserve the original promise and retrieve its outcome rather than rerunning the expression; the new test mocks both results without executing the injected code, so it misses this regression.
Useful? React with 👍 / 👎.
Search's
userScripts.executedoesn't await a promise returned by the injected script, so on CSP-strict pages (e.g. Todoist)evaluate/snapshotfailed withuserScripts.execute returned no result.This runs a synchronous wrapper first and only falls back to the async wrapper when the expression itself returns a thenable.
Verified: live in Search on app.todoist.com,
evaluateandsnapshotnow return content.npx vitest runinextension/: 87 passed (updated the userScripts test, added an async-fallback test).Known gap:
typestill fails in Search on these pages because its injected helper is async; follow-up could make it synchronous too.