Skip to content

fix: recover a wedged plugin tab; a search read must be our own search (0.3.3) - #4

Merged
mabry1985 merged 2 commits into
mainfrom
fix/own-tab-followups
Sep 13, 2026
Merged

fix: recover a wedged plugin tab; a search read must be our own search (0.3.3)#4
mabry1985 merged 2 commits into
mainfrom
fix/own-tab-followups

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Follow-ups from review round 2 on #3 (non-blocking, confirmed with stubs)

  • Wedged own tab. If the labelled ebaytab exists but won't take focus (hung/crashed), tab new --label is refused "already used" and every navigation failed until the operator closed it by hand. Now the plugin closes its own labelled tab and creates a fresh one.
  • Wrong-search reads. The host check let an operator's own eBay search (a cmd-click in the shared window) answer ours — ok: true with prices for the operator's query. For a search request, a landed search page that states terms must state the same terms (_nkw on eBay /sch/, k on Amazon /s; normalized for case, +/%20, spacing, parameter order), and an item/product page answering a search is a hijack. Sign-in and challenge hops, and search pages that state no terms, are not judged.
  • Docstring states the residual sub-second race honestly (0.27.1's open cannot name a tab; the read-side check catches it).

Tests

168 host-free (was 156): wedged-tab recovery closes only the labelled tab; reclaim through a blocked navigation with an operator Gmail and an operator eBay search open closes only the Gemini panel (locks in #3's round-1 fix through the real path); 10 same-search cases including eBay's re-encoded/reordered queries and a term-less rewrite.

Live context

0.3.2 (#3) is deployed on merchantAgent and verified: one ebay_session_status created tab t3 labelled ebaytab, navigated only that tab, returned signed_in: true; the operator's pre-existing tab kept its URL.

🤖 Generated with Claude Code

https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ

mabry1985 and others added 2 commits September 13, 2026 02:09
…h (0.3.3)

Review round 2 on #3 (MERGE, no blockers) confirmed two gaps with stubs; closed here.

- Wedged own tab: if the labelled tab exists but will not take focus (hung/crashed),
  `tab new --label` is refused "already used" and every navigation failed until the
  operator closed it by hand. Now: close OUR labelled tab (the only one carrying the
  label) and create a fresh one ("recreated").
- Wrong-search reads: the host check let an operator's own eBay search (a cmd-click in
  the shared window) answer ours — `ok: true` with prices for the operator's query.
  For a search request, the landed page must be the same host and, when it states
  search terms (`_nkw` on eBay /sch/, `k` on Amazon /s), the SAME terms (normalized:
  case, +/%20, spacing, parameter order). An item/product page answering a search is a
  hijack. Sign-in and challenge hops on subdomains, and search pages that state no
  terms (eBay rewrites), are not judged here.
- The module docstring now states the residual sub-second race honestly (0.27.1's
  `open` cannot name a tab; the read-side check catches the result).
- Tests: wedged-tab recovery closes only the labelled tab; reclaim through a blocked
  navigation with an operator Gmail and an operator eBay search in the list closes only
  the panel (locks in round 1's fix via the real path); 10 same-search cases.
  168 host-free tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ
…ves `domain: ebay.com`

Review of #4 (MERGE, no blockers) — two confirmed follow-ups folded in:
- FP2: `_is_undecided` treated any hijack as settled, so a same-site page flagged by the
  same-search rule that was ALSO still loading skipped the settle loop and failed as
  "could not find the results list". Now only `_foreign_host` (a different site) is settled.
- FN1: the same-search check compared the raw hostname, so with `domain: ebay.com` (no www)
  eBay's redirect to www silently disabled it on every read. Compare `_site(host)`.
- Tests: www-less domain in both directions, a search landing on a sign-in subdomain, and the
  settle behaviour (the reviewer's surviving mutant M1 now fails). 172 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review-structural · head 0c72054fa839 · formal

⚠️ PR advanced 1 commit(s) during this round (0c72054fa839724cab0b351a); 0 finding(s) in the delta were demoted to possibly addressed.

Overall risk is low: the diff is purely additive (new "recreated" return path in focus_own_tab, a _foreign_host split from _hijacked, expanded hijack-detection logic, and corresponding tests). No defects were flagged by any finder, and the verifier confirmed the characterization with no annotations to apply. Nothing to fix first. The panel did not disagree — there was nothing to disagree about. The structural verification pass ran but had an empty input to verify, so it contributed no independent signal; if the diff grows in subsequent rounds, a fresh structural pass would be worthwhile.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

@mabry1985

Copy link
Copy Markdown
Member Author

Gate for 724cab0: CI test pass on this head; Vera PASS on 0c72054 with the delta to 724cab0 covered in the same round ("PR advanced 1 commit during this round; 0 findings in the delta"); in-session adversarial review: MERGE, no blockers — its two confirmed follow-ups (only a different site counts as settled; compare normalised sites so domain: ebay.com keeps the same-search check) are the 724cab0 delta.

@mabry1985
mabry1985 merged commit 396086b into main Sep 13, 2026
1 check passed
@mabry1985
mabry1985 deleted the fix/own-tab-followups branch September 13, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant