fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(widget): merge anonymous visitor history in cookie authentication mode (#104) - #129

Merged
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2
Aug 31, 2026
Merged

fix(widget): merge anonymous visitor history in cookie authentication mode (#104)#129
Asaf-prog merged 3 commits into
extra-org:mainfrom
rishu685:fix/issue-104-cookie-mode-merge-v2

Conversation

@rishu685

Copy link
Copy Markdown
Contributor

Summary

Restores the changes from #127 (re-submitted following accidental merge & revert on main).

Fixes#104 by enabling automatic visitor history hand-off when using cookie-based authentication (host_token mode). Visitors who start chatting anonymously and subsequently sign in via a host session cookie will have their pre-login conversations merged into their account without requiring page reloads or host-side refreshIdentity() calls.


What Changed

1. Zero-Code Cookie History Hand-off (tokenSource.ts)

  • Updated TokenSource.current() in Cookie Mode (isCookieMode()) to re-evaluate identity resolution whenever an unlinked storedPass() exists in localStorage.
  • When a visitor signs in via cookie in the same SPA, the next widget action automatically triggers claimVisitorHistory(null) with credentials: "include".
  • Once /auth/link succeeds (conversations_moved > 0), the visitor pass is cleared from localStorage, this.cached becomes null (session cookie speaks), and all subsequent calls return null immediately with 0 overhead.

2. Deterministic Execution Ordering

  • Reverted background void claimVisitorHistory() to await claimVisitorHistory() in hostToken().
  • Guarantees that POST /auth/link finishes before token resolution completes, ensuring the first GET /conversations or history request after login observes the merged history.

3. Clear Pass Lifecycle

  • Clears the visitor pass from localStorageonly if(data?.conversations_moved ?? 0) > 0 (or hostToken !== null in Bearer mode). If conversations_moved === 0 (user still signed out), the pass is kept intact so pre-login chatting continues seamlessly.

4. Test Suite Coverage

  • widget.test.mjs: Unit test for same-SPA zero-code cookie mode hand-off without page reload.
  • widget.spec.ts: Playwright E2E test visitor pass cached, cookie login hand-off merges history and first thread list request observes merged threads.
  • test_api.py: Backend integration tests for cookie-authenticated /auth/link (200 OK) and unauthenticated attempts (401 Unauthorized).

Verification

  • npm run build:widget — Clean build ✅
  • npm run test:widgetwidget self-check: OK
  • npm run typecheck:widget & npm run typecheck:e2e — 0 errors ✅
  • ruff & mypy — 0 lint/type errors across 56 source files ✅
  • pytest — 937/937 passed ✅
  • playwright test — 40/40 passed ✅

Closes#104.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for re-opening this as a clean follow-up to #127.

The restore itself looks correct and CI is green, but I think one blocker from the previous review is still present.

In cookie mode, current() now re-resolves identity whenever a stored visitor pass exists:

asynccurrent(): Promise<string|null>{if(!this.cached||(this.isCookieMode()&&this.storedPass()!==null)){awaitthis.resolve(()=>this.storedPass());}returnthis.cached;}

This solves the zero-code same-SPA login case, but while the user is still signed out it can cause every API request to perform a blocking /auth/link attempt first:

anonymous request
→ POST /auth/link
→ 401
→ actual API request
next anonymous request
→ POST /auth/link
→ 401
→ actual API request

So anonymous usage can pay an extra network round-trip on every request until login happens.

There is already a lastClaimAttemptPass field with the comment:

/** Avoid repeating unauthenticated claim attempts for the same pass in cookie mode. */

but it is currently unused, which looks like this case was anticipated but not completed.

I think we should keep the zero-code login detection while avoiding a blocking link attempt on every anonymous request. I don’t want to prescribe the exact mechanism — retry/backoff/state tracking are all reasonable — but the behavior should ensure that:

  • repeated requests while still anonymous do not each call /auth/link;
  • the widget still retries later so a newly available login cookie can be detected;
  • once login is detected, the merge completes before history that depends on it is loaded.

It would also be good to add a regression test with multiple consecutive anonymous requests and assert that /auth/link is not called once per request.

Once that is addressed, I think this should be very close to approval.

…cookie mode
- Use lastClaimAttemptPass and CLAIM_RETRY_INTERVAL_MS (15s) to avoid repeating blocking POST /auth/link calls on every consecutive anonymous request
- Preserves zero-code cookie login hand-off after retry interval or reset
- Adds regression unit test verifying multiple consecutive anonymous turns do not repeat /auth/link
@rishu685

Copy link
Copy Markdown
ContributorAuthor

Thanks @Asaf-prog! Great catch on throttling anonymous retries.

I've implemented lastClaimAttemptPass and lastClaimAttemptTime with a 15-second cooling-off window (CLAIM_RETRY_INTERVAL_MS = 15000) in commit 8d05239f:

How it works:

  1. Throttled Anonymous Requests: When a visitor is signed out, the first request attempts POST /auth/link. Upon receiving 401, TokenSource records lastClaimAttemptPass = pass and lastClaimAttemptTime = Date.now(). Subsequent anonymous requests within 15 seconds skip /auth/link and return this.cached immediately with 0 extra network calls.
  2. Zero-Code SPA Cookie Login: When the user signs in on the host app, the next turn after the interval (or upon page reload / refreshIdentity()) attempts POST /auth/link. On 200 OK with conversations_moved > 0, the visitor pass is cleared, this.cached transitions to null (session cookie speaks), and history is merged deterministically.
  3. Regression Unit Test: Added a test in widget.test.mjs verifying that 5 consecutive anonymous requests trigger POST /auth/linkonly once, not 5 times.

All 937 pytest tests, 40 Playwright E2E tests, and widget unit self-checks pass cleanly. Ready for final review!

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — the throttling improvement solves the repeated /auth/link calls while the user is still anonymous, and the regression coverage is useful.

I still see one lifecycle issue before approving.

With the 15-second cooldown, this flow is possible:

anonymous request
→ POST /auth/link
→ 401
→ cooldown starts
user signs in 2 seconds later
user immediately opens conversation history
→ current() is still inside the cooldown window
→ /auth/link is skipped
→ GET /conversations runs as the signed-in user
→ anonymous conversations have not been merged yet

So we avoid repeated anonymous link attempts, but we now have a window where the first authenticated request after login can still observe incomplete history.

The new unit test also does:

loggedInViaCookie=true;tokens.reset();

before verifying the successful merge.

That proves the flow works when identity is explicitly reset, but the original requirement for cookie mode is the zero-code case where the host does not need to call refreshIdentity() / reset() when the cookie appears.

I think we need to preserve both properties:

  • anonymous requests should not trigger a blocking /auth/link on every call;
  • if the user signs in, the next request that depends on authenticated history should not be forced to wait for an arbitrary retry interval before the hand-off can happen.

I don’t want to prescribe a specific implementation, but the current fixed cooldown alone does not give us a reliable signal that authentication changed.

Could we also add a regression test for the actual zero-code transition:

anonymous claim attempt → 401
user signs in during the retry window
no reset / refreshIdentity
next history request
→ history is already merged

One additional point: if the proposed solution relies on a timeout, sleep, polling interval, or any other fixed time-based delay, I’d like to see a clear justification for why that specific timing is correct and what invariant it is enforcing. A magic delay should not be used to approximate an authentication state transition unless there is a concrete reason it is safe and reliable.

Once that lifecycle is deterministic without host intervention, I think this will be ready to approve.

… and deterministic history hand-off
- Remove CLAIM_RETRY_INTERVAL_MS magic cooldown delay
- Add cookie snapshot tracking (document.cookie) and window focus/visibility listeners to TokenSource
- Ensure listConversations passes forceCheck to guarantee POST /auth/link completes before GET /conversations
- Add zero-code transition regression test in widget.test.mjs
@rishu685

rishu685 commented Aug 31, 2026

Copy link
Copy Markdown
ContributorAuthor

I completely agree magic time-based cooldowns were introducing race condition windows where login history couldn't be deterministically observed.

I've eliminated CLAIM_RETRY_INTERVAL_MS entirely and implemented an event-driven snapshot model in commit 852be0ce:

1. Event-Driven & Cookie Snapshot Tracking (TokenSource.ts)

  • document.cookie Snapshot: TokenSource tracks lastCookieSnapshot. Any cookie change triggers identity re-evaluation.
  • Window Event Listeners: Attached listeners for window.focus, document.visibilitychange, and window.storage. Returning to the tab after logging in marks identity dirty.
  • Fast Anonymous Turns: Consecutive anonymous requests in the same tab return this.cached immediately without blocking on /auth/link every turn.
  • Deterministic History Hand-Off: AgentChatClient.listConversations() passes { forceCheck: true } to current(), guaranteeing POST /auth/link is await-ed before GET /conversations executes.

2. Zero-Code Regression Unit Test (widget.test.mjs)

Added a unit test verifying the exact zero-code transition without calling tokens.reset() or refreshIdentity():

// Anonymous turn -> 401awaitclient.createConversation();// 5 consecutive turns -> 0 extra /auth/link callsawaitclient.createConversation();// User logs in via cookie (zero-code: host calls no reset/refreshIdentity)loggedInViaCookie=true;// Next history request -> POST /auth/link succeeds (200 OK), pass is cleared, no bearer sentawaitclient.listConversations();

@Asaf-prog
Asaf-prog merged commit 18a4c04 into extra-org:mainAug 31, 2026
2 checks passed
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for addressing the previous blockers and removing the fixed cooldown.

The zero-code cookie hand-off is now covered, the main flow looks correct, and CI is green.

I’m approving this version. I still think the unconditional forceCheck on conversation history can be improved to avoid repeated /auth/link probes for anonymous users, but I’ll handle that separately as a follow-up so we don’t keep expanding the scope of this PR.

Approved ✅

Sign up for freeto 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.

Anonymous→account merge never runs in host_token (cookie) mode

2 participants

@rishu685@Asaf-prog