Skip to content

Fix escapeAttr, ExtensionAuth double fetch, and candidate page title bugs#246

Closed
bd73-com wants to merge 4 commits intomainfrom
claude/fix-github-issues-69Kwf
Closed

Fix escapeAttr, ExtensionAuth double fetch, and candidate page title bugs#246
bd73-com wants to merge 4 commits intomainfrom
claude/fix-github-issues-69Kwf

Conversation

@bd73-com
Copy link
Owner

@bd73-com bd73-com commented Mar 19, 2026

Summary

Closes #242, closes #243, closes #244

Test plan

  • npm run check passes
  • npm run test passes (1707 tests)
  • Verify extension popup shows correct page title when clicking "Track this" on candidates
  • Verify extension popup handles ampersands in page titles correctly
  • Verify ExtensionAuth page only sends one POST when token endpoint returns error

https://claude.ai/code/session_01AmZyEy5PWRPk34fqgyyUQ9

Summary by CodeRabbit

  • Bug Fixes

    • Prevented unnecessary token regeneration triggered by transient error updates, improving reliability.
  • Improvements

    • Use the active tab's title for page selection to improve accuracy.
    • Added robust HTML attribute escaping and tier sanitization to reduce injection risks and normalize tier inputs.
  • Tests

    • Added unit tests covering attribute escaping and tier sanitization behavior.
  • Chores

    • Extension version bumped to 1.0.3.

…didate page title

- #244: Escape `&` to `&` before other replacements in escapeAttr()
- #243: Remove `error` from useEffect dependency array to prevent
  double POST on token fetch failure
- #242: Use stored tab title from chrome.tabs.query instead of
  popup's document.title for candidate "Track this" button

Closes #242, closes #243, closes #244

https://claude.ai/code/session_01AmZyEy5PWRPk34fqgyyUQ9
@coderabbitai
Copy link

coderabbitai bot commented Mar 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9ce65387-f168-4e96-a015-2f153f1bfc1a

📥 Commits

Reviewing files that changed from the base of the PR and between d60c4dd and d269e63.

⛔ Files ignored due to path filters (1)
  • extension/fetchthechange-extension.zip is excluded by !**/*.zip
📒 Files selected for processing (5)
  • client/src/pages/ExtensionAuth.tsx
  • extension/manifest.json
  • extension/src/popup/popup.ts
  • extension/src/popup/utils.test.ts
  • extension/src/popup/utils.ts

📝 Walkthrough

Walkthrough

Removes error from a token-fetch useEffect dependency to stop duplicate requests, switches the popup to use the active tab's title for created selections, and adds/uses escapeAttr+sanitizeTier utilities (escaping & first) with tests; also bumps extension manifest version.

Changes

Cohort / File(s) Summary
Token fetch control
client/src/pages/ExtensionAuth.tsx
Narrowed the useEffect dependency array by removing error ([user, tokenSent]), preventing the token-generation effect from re-running when error state changes.
Popup UI & tab title
extension/src/popup/popup.ts
Added currentTabTitle state populated from the active tab during init(). Updated selection creation to use currentTabTitle (instead of document.title) for pageTitle.
Popup utilities
extension/src/popup/utils.ts, extension/src/popup/utils.test.ts
Added exported escapeAttr(str) (now escapes && first) and sanitizeTier(tier) with unit tests covering escaping and tier sanitization. popup.ts now imports these from ./utils.
Manifest bump
extension/manifest.json
Incremented extension version from 1.0.21.0.3.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the three main bug fixes in the changeset: escapeAttr ampersand escaping, ExtensionAuth double fetch, and page title issues.
Linked Issues check ✅ Passed All three linked issues (#242, #243, #244) have corresponding code changes that directly address their stated requirements and expected outcomes.
Out of Scope Changes check ✅ Passed All code changes are directly scoped to the three linked issues; no extraneous modifications are present in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-github-issues-69Kwf
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the fix label Mar 19, 2026
claude added 3 commits March 19, 2026 13:02
Move pure utility functions from popup.ts into popup/utils.ts for
testability. Add comprehensive test coverage for escapeAttr (including
ampersand escaping) and sanitizeTier. Move import to top of file
following codebase conventions.

https://claude.ai/code/session_01AmZyEy5PWRPk34fqgyyUQ9
- Add error check to useEffect guard to prevent implicit retries when
  user object reference changes after a failed token fetch
- Add comment documenting KNOWN_TIERS sync dependency on
  shared/models/auth.ts TIER_LIMITS

https://claude.ai/code/session_01AmZyEy5PWRPk34fqgyyUQ9
Version bump for popup bug fixes: escapeAttr ampersand encoding,
candidate page title source, and utils module extraction.

https://claude.ai/code/session_01AmZyEy5PWRPk34fqgyyUQ9
@bd73-com bd73-com closed this Mar 19, 2026
@bd73-com bd73-com deleted the claude/fix-github-issues-69Kwf branch March 22, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants