refactor: add reusable extensions core plumbing#101
Conversation
|
Posting context for reviewers: This PR is the base of a larger stacked multi-source FT extension series. Please review this PR as source-agnostic core plumbing only:
The dependent feature layers currently live as stacked PRs in my fork because I don’t have push rights to create the intermediate base branches on upstream:
So for upstream review, this PR should be treated as the base branch for that stack. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e8f60b6. Configure here.
| } | ||
| } | ||
|
|
||
| return `\n${chunks.join('\n')}`; |
There was a problem hiding this comment.
Leading newline and padding break formatBookmarkStatus output
High Severity
renderStatusSections unconditionally prepends \n to its output, and formatStatusLine pads labels to 14 characters. This changes the output of formatBookmarkStatus from starting with "Bookmarks" to starting with "\nBookmarks", and from "bookmarks: 99" to "bookmarks: 99" (extra padding). The existing test in bookmarks-service.test.ts asserts assert.match(text, /^Bookmarks/) and assert.match(text, /bookmarks: 99/), both of which will now fail. The ft status CLI output also gets an unexpected leading blank line.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e8f60b6. Configure here.
| return { | ||
| cookieHeader: `ct0=${ct0}; auth_token=${authToken}`, | ||
| csrfToken: ct0, | ||
| }; |
There was a problem hiding this comment.
Cookie validation and trimming removed from Firefox path
Medium Severity
The refactored extractFirefoxXCookies no longer trims or validates cookie values against the printable-ASCII range (/^[\x21-\x7E]+$/). The Chrome path still calls sanitizeCookieValue for this purpose. Cookie values with leading/trailing whitespace or non-printable characters will now be embedded directly into the cookieHeader, potentially causing malformed HTTP headers or silent auth failures.
Reviewed by Cursor Bugbot for commit e8f60b6. Configure here.
|
Was dropping the The old code had a Separately: |
Accidentally dropped validateCookie() during the auth_token required-check refactor. Restore printable-ASCII validation and whitespace trimming for both ct0 and auth_token before building the cookie header. Addresses review feedback on PR afar1#101.
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Good catch on both — sorry for the delay getting back to you. ct0 validation: Accidental drop. When I rewrote the return block to add the explicit auth_token → required: Intentional. A partial/expired X session (ct0 present, auth_token missing) produces a confusing downstream failure if we silently skip it — the explicit error with actionable fix steps felt strictly better than the old optional behavior. I can add a note to the PR description calling this out if that helps reviewers. |
3 participants
Summary
What changed
--limitsupport and portable Firefox config test coverageKey commits
9e0b3f5feat: add classify limit and portable firefox config testf809793refactor: improve shared firefox profile discoverye8f60b6refactor: add reusable core status and path helpersVerification
npx tsx --test tests/config.test.ts tests/engine.test.ts tests/firefox-cookies.test.ts tests/firefox-profile-detection.test.ts tests/namespace-helpers.test.tsnpm run buildNotes