Add skill-gated cross-tab OTP email reader - #332
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. |
![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: 7cc8be227a
ℹ️ 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 tokens = key.split(' ').filter(token => token.length >= 2 && !ignored.has(token)); | ||
| return [...new Set([key, ...tokens].filter(Boolean))]; |
There was a problem hiding this comment.
For multiword issuers, every token of length two or more is accepted and later matched with some, so common tokens such as of in “Bank of America” or me in “ID.me” classify unrelated inbox rows as service matches. Those rows' previews—and potentially unrelated verification codes—are then returned to the model, risking both incorrect OTP selection and disclosure of unrelated email content; require the complete normalized identity or at least one sufficiently discriminative token.
Useful? React with 👍 / 👎.
| const directTree = await this._otpEmailMessageTree(mailboxTab.id, observed.tree); | ||
| if (observed.tree?.conversationRootRefId) { |
There was a problem hiding this comment.
When the selected Outlook, Yahoo, Proton, Fastmail, Zoho, Yandex, or iCloud tab already displays the requested message, this branch never recognizes it as a message because conversationRootRefId is only emitted by the Gmail-specific detector in src/chrome/src/content/accessibility-tree.js. The visible message is consequently parsed as an inbox: it either reports no matching message or clones the same message URL and tries to click a non-navigating article/link before timing out. Add provider-neutral trusted message-view detection or directly extract an already-open supported-provider message.
Useful? React with 👍 / 👎.
7cc8be2 to
817d4e8
Compare
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 817d4e82b3
ℹ️ 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 path = url.pathname; | ||
| const hash = url.hash; | ||
| switch (provider) { | ||
| case 'gmail': return /(?:^#|\/)(?:inbox|all|sent|starred|important|trash|spam|search|label\/[^/]+)\/[A-Za-z0-9_-]+/i.test(hash); |
There was a problem hiding this comment.
When Gmail is open on a search-results URL such as #search/github, this regex treats the search query itself as a message ID. _otpEmailMessageTree then sets detected even without a message root, so inspect returns a broad search-results excerpt as stage: "message" instead of presenting opaque candidates; this can expose content from several matching rows and cause an older or unrelated code to be selected. Require an additional path segment containing an actual thread ID for Gmail search routes.
Useful? React with 👍 / 👎.
817d4e8 to
75c042a
Compare
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75c042ab76
ℹ️ 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 rootRef = String(tree?.conversationRootRefId || '').trim() | ||
| || (messageRoute ? otpOpenMessageRootRef(tree?.pageContent, service) : ''); | ||
| if (!rootRef) return { tree, detected: messageRoute, detection: messageRoute ? 'provider_route' : '' }; |
There was a problem hiding this comment.
On a supported non-Gmail message URL where the accessibility tree has no unique article/document root, the new provider-route fallback marks the entire visible mailbox tree as a detected message. The subsequent excerpt search can match the requested service in an inbox/sidebar row and return that row's code or neighboring unrelated email text instead of the open message. A failed subtree read similarly falls back to the broad tree while remaining detected; require a successfully read trusted or unique semantic message root before returning stage: "message".
Useful? React with 👍 / 👎.
| _otpEmailSkillActive(tabId, mode = this._effectiveRunMode(tabId), tier = this._resolvePromptTier()) { | ||
| return this._activeSkillRecords(tabId, mode, tier).some(skill => skill.id === OTP_EMAIL_SKILL_ID); |
There was a problem hiding this comment.
When a restored/imported custom-skill record uses the ID otp-verification-code-helper, this ID-only check activates the privileged cross-tab mailbox reader even if the record is ordinary user or URL content. Skill normalization preserves caller-supplied IDs, and startup skips installing the packaged skill when that ID already exists, so a crafted configuration can replace the bundled instructions yet retain private mailbox access. Bind activation to the verified packaged built-in record and expected source, rather than the ID alone.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75c042ab76
ℹ️ 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 click = await this.executeTool(helperTab.id, 'click_ax', { | ||
| ref_id: helperCandidate.clickRef, | ||
| expectedDocumentToken: helperRead.tree.documentToken, | ||
| expectedPageUrl: helperRead.tree.refScopeUrl || helperRead.liveUrl, | ||
| }); |
There was a problem hiding this comment.
When open_message is used from Ask mode, this direct click_ax invocation bypasses the normal tool-loop capability gate, even though opening an unread message in Gmail, Outlook, and other supported providers commonly marks it read on the server. Using an inactive disposable tab does not isolate that account-level mutation, so a supposedly read-only Ask run can alter the user's mailbox without a click grant; require the normal consequential-action gate or use a genuinely non-mutating message-read path.
Useful? React with 👍 / 👎.
| const subtree = await this.executeTool(targetTabId, 'get_accessibility_tree', { | ||
| ref_id: rootRef, | ||
| filter: 'all', | ||
| maxDepth: 15, | ||
| maxChars: 6000, | ||
| }); |
There was a problem hiding this comment.
When an expanded Gmail conversation exceeds 6,000 characters, this anchored read returns only page 1 with hasMore/nextPage, but _otpEmailMessageTree accepts it as the complete message tree and later returns a successful message result. In a resend thread, an older matching code can be present on page 1 while the newest code is on a later page, causing the agent to select an obsolete OTP despite the skill's newest-resend rule; follow the continuation to completion or fail closed on truncated message trees.
Useful? React with 👍 / 👎.
|
@codex pls review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
open_message now dispatches only against a one-use grant minted by the permission gate, so a call that never passed the capability x host prompt cannot create a helper tab or click in the mailbox. The gate and the handler also derive the session key the same way, from the raw service argument, so astral characters can no longer split them across a UTF-16 slice. Gmail routes are parsed instead of pattern matched: an open message needs a thread-id-shaped tail, which keeps #inbox/p2 and #search/github/p2 out of the message branch that returned a whole listing as one message. Threads opened from a later page still resolve. Mailbox reads peek the abort flag rather than consuming it, so a stop during a read still ends the run. They also refuse to read a tab that is navigating away from the mailbox, which stops a foreign URL from becoming the host the click prompt is charged to. Failures after the helper tab opens now say the session is gone and that inspect has to run again, and provider error strings lose their accessibility refs before they reach the model. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKsX7CYmaEp2rjeVs9EBAL
_otpEmailOpenFailure copied its input with a spread, so any field a helper result happened to carry would have gone to the model along with the error. Nothing reaches it that way today, but the results it wraps are the same shape that carries the accessibility tree, the tab record, and the live mailbox URL, and the model is not allowed to see any of those. It now builds the response from a fixed set of fields instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKsX7CYmaEp2rjeVs9EBAL
1 participant
Summary
read_email_verification_messagetool only while the bundled OTP helper is active on Mid/FullValidation
npm testgit diff --checkDependency
main; after Remove browser tab management tools #331 merges, the displayed diff should narrow to the OTP helper change only.