Repository navigation
feat(support): give Crisp agents the account context they keep asking for - #2851
Conversation
… for Follow-up to #2736. The agent sidebar carries identity, wallet links and verification state; it carries nothing about what the user actually has or just did, so every thread still opens with three rounds of "what's your balance / what did you last do / which build are you on". None of this needs database access — Crisp never had any. The client pushes `session:data`, so the ceiling was never "DB vs no DB", it was "what does the client already know". It knows all of this already. New sidebar rows, all derived from the /get-user read-models and warm react-query caches: balance spendable total, wallet and card halves reported separately account_stats points, streak, tenure, milestone, invite provenance, badges latest_activity kind/provider, status, amount, age, failReason, uuid limits_remaining per-provider fiat headroom card application state, collateral, issued cards linked_accounts rail shapes and countries app_context platform, all three build layers, locale, route, connectivity sentry_issues Sentry search scoped to this user Plus Crisp segments (platform, kyc-*, verification-blocked, zero-balance, offline…) for the boolean half — those are what the inbox filters and routes on, and keeping them out of session:data is what stops the sidebar becoming a wall of yes/no rows. Three things worth calling out: - **No new requests.** SupportDrawer mounts this hook app-wide, for guests too. A useWallet()/useLimits()/useRainCardOverview() call inside it would start a 30s RPC poll and two API requests for every user on every screen — the shape that made /rain/cards the most-called endpoint in the app. Reads go through support-cache, which only looks at what is already cached, and a test asserts the hook leaves the query cache empty. - **Unreadable is never zero.** rainCentsToUsdcUnits(undefined) is 0n, so "no answer" and "genuinely empty" are arithmetically identical (PEANUT-UI-QD5). An agent reading "$0.00" tells a funded user they have no money, so each half is either a figure or the word `unavailable`, and the total only appears when both halves are answers. - **The user's own state, never a counterparty's.** HistoryEntry carries the other party's username and full name; a support console is the wrong place to accumulate third parties' payment records. Activity summaries name the kind, status and amount, and the uuid is enough to look the rest up. Also fixes a drift the new fields would have widened: the three sinks (web widget, proxy iframe, native setString) each wrote the field list by hand, and native had fallen to two keys where web sent seven — so the agents helping *app* users saw the least. There is now one `supportSessionFields` definition feeding all three. Native holds a single segment (assignment, not append), so it gets the most actionable one and the full list rides as a data row. The privacy policy's Crisp row still describes "messages, email address, and basic device data" and needs widening to match — that lives in peanut-content and ships as its own PR.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Code-analysis diffPainscore total: 7643.75 → 7669.11 (+25.36) 🆕 New findings (41)
…and 21 more. ✅ Resolved (33)
…and 13 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…dCrispUserData The assembler had grown to 71 lines declaring values used far below their declaration. buildSupportLinks and buildAppContext are the two clusters that were pure string construction with no reason to sit inline.
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review: Changes requested
Request changes: Crisp segments accumulate stale routing flags, and the expanded financial and verification payload is not covered by the pinned privacy disclosure.
Findings
-
MAJOR · src/utils/crisp.ts:126 · Replace segments instead of appending them
setCrispUserData runs again whenever the snapshot changes, but Crisp appends session segments unless the overwrite argument is true. A user who is briefly offline or balance-unavailable therefore keeps that routing tag after recovery and can accumulate contradictory tags, so inbox filters and routes act on stale state. Pass the overwrite flag and add a regression test covering two successive snapshots. -
MAJOR · src/utils/crisp.ts:66 · Update the disclosure before expanding the Crisp payload
These new rows send balance, account stats, latest activity and failure reason, limits, card state, and linked-account metadata to Crisp. The pinned PR description states that the current privacy policy discloses only messages, email, and basic device data, while this head leaves the content pointer unchanged. Deploying it would begin broader third-party processing whenever support opens without an accurate published disclosure. Gate the new fields until the updated policy is live and processor scope and retention are confirmed, or ship the disclosure change atomically.
Checked clean
- Exact head, base, merge base, trusted author, title, and description
- Cache-only balance, card, limits, and transaction-history reads
- Unknown-balance handling, in-transit arithmetic, and counterparty redaction
- Web, proxy, and native field parity plus logout reset paths
- Platform, build, route, connectivity, and notification context
- Targeted support-context suite: 25 tests passed
- Diff hygiene: git diff --check passed
Second opinion skipped: openrouter-timeout.
Exact head: 3b9acb4bbcc1 · Context: repo, pull_request_description, crisp_official_docs
Crisp appends session segments unless the second push argument is true, and setCrispUserData runs again on every snapshot change. So the segment set only ever grew: a user who was briefly offline kept routing as `offline` after recovery, and `kyc-pending` outlived their approval. The inbox would then filter and route on state the user had already left — worse than shipping no segments at all, because a stale tag reads as current. Native is unaffected: its setSegment is an assignment (CrispSDK.session.segment = …), so it already replaces. Two regression tests cover it — one pinning the overwrite flag, one pushing two successive snapshots and asserting the superseded segment is gone. Both fail without the flag. Found by Chip on #2851.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: the new route latch can retrigger the native Crisp open flow and duplicate prefilled messages. Both earlier findings are fixed.
Findings
- MAJOR · src/hooks/useSupportClientContext.ts:92 · Prevent the route latch from opening native Crisp twice
On the first Capacitor support open,isSupportModalOpenbecomes true and this passive effect updatesrouteOnOpenafter the same commit has already started SupportDrawer's async native open effect. The route change rebuildsuserData; because that object is an effect dependency and there is no in-flight/open-cycle guard, a second chain can callsendMessageandopenMessengerbefore the first finishes, duplicating any prefilled support message. Capture the route without a post-open state transition, or guard/split the native sync/open effect so each open cycle sends and opens exactly once; cover this case in a native SupportDrawer test.
Checked clean
- Exact head, supplied base SHA, and merge base verified.
- Earlier segment finding FIXED: session:segments now uses the overwrite flag and has regression coverage.
- Earlier disclosure finding FIXED: the production privacy copy now discloses the account-context snapshot; publish PR #2855 is merged.
- Exact-head CI is green across unit, e2e, typecheck, eslint, format, analyze, and deployment checks.
- Reviewed cache-only balance, card, limits, and activity reads plus web, proxy, and native Crisp sinks; no new query subscription was introduced.
- Reviewed support-data trust boundaries, session reset behavior, postMessage origin/source checks, and omission of transaction counterparties and bank identifiers.
- No issue comments or review comments were fetched or read.
Second opinion skipped: openrouter-timeout.
Exact head: dcfcc098f087 · Context: repo, content, peanut-api-ts
The route latch set state in an effect, so it landed after the commit that opened support — and on native that commit has already started SupportDrawer's async open chain. The extra render rebuilt `userData`, which is a dependency of that effect, so a second chain reached sendMessage/openMessenger before the first finished and a prefilled support message was sent twice. Two changes, because the latch was the trigger but not the whole hazard: - The route is now latched DURING RENDER. React re-runs the component before anything commits, so the first snapshot support ever sees already carries the route — no post-open transition at all. This also keeps `route:` in the native app_context row, which an open-cycle guard alone would have dropped. - The native effect takes an open-cycle guard. `userData` is a live snapshot: a balance landing from the cache changes its identity too, and any such change during the async window had the same double-send shape. The guard closes it for every cause. Reset on close and in the catch, so the next open and a retry after a failed open both still run. Tests: a prefilled message survives a mid-open snapshot change exactly once (fails without the guard — 3 sends), and the token-still-resolving path still opens, so the guard cannot latch a waiting cycle shut. Found by Chip on #2851.
There was a problem hiding this comment.
Chip review — changes requested
The native messenger now opens once, but the latch sends the account snapshot captured before asynchronous setup instead of the latest state.
Findings
- MAJOR · src/components/Global/SupportDrawer/index.tsx:215 · Send the latest snapshot through the native open latch
IfuserDatachanges while Crisp configuration or camera permission is pending (the added test modelsbalancechanging from unavailable to $100), the latch prevents a second effect but this continuation still closes over the first object. NativesetStringandsetSegmenttherefore publish the stale balance andbalance-unavailablerouting even though the newer snapshot exists before the messenger opens. Keep the latest user data in a ref and read it after the await, while using the latch only to gatesendMessageandopenMessenger.
Checked clean
- Exact detached head and supplied base SHA verified; the worktree is clean.
- PR author, base ref, base SHA, and head SHA match the supplied trusted metadata.
- Earlier native duplicate-open finding: FIXED; the new latch and regression test keep
sendMessageandopenMessengerto one call per open cycle. - Earlier segment accumulation finding: FIXED; web session segments now use Crisp's overwrite flag and tests pin replacement behavior.
- Earlier disclosure finding: FIXED; canonical content now discloses the account-context snapshot in the Crisp processor row.
- Cache readers remain read-only and do not subscribe to or create TanStack queries.
- Balance unknown-state handling, third-party activity redaction, support link construction, session-field sink parity, and logout cache clearing were reviewed.
- All reported CI checks at the exact head completed successfully; the skipped manual check is non-gating.
Second opinion skipped: openrouter-timeout.
Exact head: 2e2eeb594559 · Context: repo, content
…efore setup
The open-cycle guard from the previous commit closed the double-send window but
opened a staleness one. The chain awaits Crisp configuration and the camera
permission; the guard means a snapshot landing during that window gets no second
chance to publish, and the continuation still read `userData` from the effect
closure. So native published whatever was true when support was tapped — an
agent could open the sidebar on a balance the user no longer has, and route on
a `balance-unavailable` segment they had already left.
The chain now reads the payload ref after its awaits. The ref already existed
for the proxy handshake and is refreshed on every commit, so it is renamed
`latestPayloadRef` to say what it is and both sinks read the same latest state.
The token and the prefill come from it too — the same window applied to them,
and a stale token is the session-isolation bug this component's own gate exists
to prevent.
Test asserts the later balance and the later segment reach native, and fails
against the closure ("wallet unavailable" instead of "$100.00 spendable").
Found by Chip on #2851.
There was a problem hiding this comment.
Chip review — changes requested
Two major issues remain: cache updates after the open render do not refresh the support snapshot, and account_stats sends an inviter's username despite the user-only privacy boundary.
Findings
-
MAJOR · src/hooks/useCrispUserData.ts:79 · Subscribe the open snapshot to cache updates
These imperative cache reads are not subscriptions: useQueryClient() stays stable when setQueryData or a query completion updates the cache. If support opens while balance, card, or history data is pending, that completion does not re-render SupportDrawer, so latestPayloadRef remains at unavailable and Crisp receives stale values and routing segments; the new test only observes an update after explicitly rerendering. Subscribe read-only to the relevant cache changes, ideally only while support is open and without enabling any fetches, or otherwise force a snapshot recompute before publishing. -
MAJOR · src/utils/support-context.ts:91 · Do not send the inviter's username to Crisp
IUserProfile.invitedBy is explicitly another person's username, so opening support as a referred user now copies a third party's identifier into Crisp. That contradicts this module's user-only invariant and the updated disclosure that Crisp receives the current user's account context. Omit the username or reduce this field to a non-identifying boolean such as referred:true. -
MAJOR · src/utils/support-cache.ts:53 · [moonshotai/kimi-k3] readLatestHistoryEntry scans other users' cached history (targetUsername variants) and pushes it as the user's own activity
The scan uses findAll({ queryKey: [TRANSACTIONS] }) and aggregates entries from every cached variant, including keys like [TRANSACTIONS, 'latest', { limit, targetUsername: 'bob' }] — the docblock explicitly acknowledges targetUsername variants exist but only guards against limit differences. Concrete failure: the user views another user's activity (targetUsername set), then opens support; if that entry is the newest in the cache, buildLatestActivity serializes its kind/status/amount/failReason/uuid into latest_activity and it is pushed to Crisp as THIS user's latest movement. That both misleads the agent (they troubleshoot a transaction the user never made) and replicates a third party's payment record into the Crisp console — exactly what the PR's stated invariant #2 ('the user's own state, never a counterparty's') and the removal of counterparty names were designed to prevent. Fix: while scanning, skip queries whose key carries a targetUsername different from the logged-in user (e.g. inspect query.queryKey[2]?.targetUsername), or restrict the scan to keys with no targetUsername. -
MINOR · src/components/Global/SupportDrawer/index.tsx:195 · [moonshotai/kimi-k3] Native open latch resets on close while the in-flight chain is still awaiting, so close/reopen still double-sends the prefilled message
nativeOpenStartedRef is set when the chain starts and reset to false the moment isSupportModalOpen flips false, but the already-started chain (awaiting ensureNativeCrispConfigured / ensureNativeCameraPermission) is not invalidated. Concrete failure: user opens support, the chain parks on the camera-permission await, the user dismisses the drawer (ref resets) and reopens it — a second chain starts, both chains complete, and sendMessage/openMessenger each run twice, the exact duplicate-message bug this PR sets out to kill ('closes that window for every cause'). The same gap means a chain started before a close still opens the messenger and posts the prefilled message after the user has dismissed support, since nothing is re-checked after the awaits. Fix: use a generation counter instead of a boolean — increment it on close, capture it at chain start, and bail out after the awaits (before setUser/sendMessage/openMessenger) if the captured generation no longer matches. -
MINOR · src/hooks/useSupportClientContext.ts:105 · [moonshotai/kimi-k3] routeOnOpen latches the raw pathname, which can embed a counterparty identifier from dynamic route segments
routeOnOpen is latched from usePathname() verbatim and pushed to Crisp inside app_context. For any route with a user-identifying dynamic segment (e.g. a payment-link or profile path such as /pay/ or /), the route row names the third party the user was paying or viewing when they opened support — the same class of counterparty leak the PR deliberately removed from latest_activity ('a support console is the wrong place to accumulate third parties' payment records'). Concrete failure: user opens a payment link to another user, hits an error, opens support from that page; the agent sidebar now shows route:/pay/. Fix: normalize the pathname before latching — map it against known route patterns and replace dynamic segments with a placeholder (e.g. route:/pay/:user), or whitelist static route prefixes.
Checked clean
- Pinned head, base SHA, base ref, trusted author, and merge base all matched the supplied values.
- Exact-head CI is green across unit, typecheck, eslint, format, e2e, and aggregate gates; a focused local rerun could not start because dependencies are not installed in the detached worktree.
- Earlier useSupportClientContext route-latch finding: FIXED; the route is latched before commit and the native open chain is guarded per open cycle.
- Earlier SupportDrawer stale-effect-closure finding: FIXED; the native continuation reads the latest committed payload after its awaits.
- Reviewed token gating, logout cache clearing, balance-unavailable handling, session-field parity, segment replacement, native primary-segment selection, query-key alignment, and proxy update flow.
- git diff --check passed.
Second opinion by moonshotai/kimi-k3: 3 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: f5f8aee9dbbc · Context: repo, product
Four findings from Chip's fourth pass. Subscribe the open snapshot to cache updates. The cache reads are not subscriptions, so a query resolving after support opened never reached the sidebar: open while the balance is in flight and the agent read `unavailable` for the whole conversation, routing on a `balance-unavailable` segment — accurate at the instant it was read, wrong a second later. The hook now watches the query cache, read-only and only while support is open. It never triggers a fetch, which is the point of reading the cache rather than subscribing with useQuery, and the signature memo bounds the re-render churn: an event that changes no value leaves the snapshot's identity alone. Stop sending the inviter's username. `invitedBy` is another person's handle, and shipping it contradicted this module's own stated invariant. That the user was referred is useful to an agent; who referred them is not ours to forward. Reduced to `referred:yes`. Normalize the route before latching it. `/pay/bob` named the counterparty the user was paying — the same leak the activity row was built to avoid. Segments under identifier-bearing prefixes become `:id`, and because the root `[...recipient]` catch-all makes any unlisted single segment a possible handle, anything unrecognised degrades to `/:recipient`. Fail-safe by construction: forgetting to list a new static route costs precision, never privacy. `/withdraw/manteca` still names the rail, which names nobody. Replace the native open latch with a generation counter. The boolean assumed the chain could not outlive its cycle. It can: close the drawer while it is parked on the camera-permission await and reopen, and the cleared flag lets a second chain start — the duplicate prefill was back. The same gap let a chain from a dismissed cycle open the messenger after the user walked away. Closing now increments a generation, the chain captures it, and everything after the awaits is conditional on it still being current. One finding not taken: the history scan reading other users' rows. /users/history is always scoped to the authenticated userId — targetUsername resolves to a filter, not a subject (peanut-api-ts src/routes/user/history.ts:167-173). Every cached variant holds this user's own transactions. Reasoning recorded on the PR.
…e guards Four rounds of review on this PR produced four defects in one place, and each fix seeded the next: a duplicate prefill, a stale payload, a duplicate again across close/reopen. That is a shape, not bad luck. The effect both published live data and performed a one-shot action, and it listed the data it publishes in its dependencies — so every snapshot change re-entered a function whose tail sends a message and opens a window, and each guard bolted on to stop that introduced the next gap. The effect now depends only on what defines an open cycle: the drawer being open, and the token being ready. `userData`, `crispTokenId` and `prefilledMessage` are read from `latestPayloadRef` at the moment the chain publishes, which is where they were already being read from, so they never belonged in the deps. That deletes both generation refs and the boolean before them. What is left is React's own idiom: one cycle runs the chain once, cleanup sets `cancelled`, and a chain from a cycle the user has left stops before it can open a messenger they walked away from. Four coordinating mechanisms become two — the token gate and cancellation — and the file loses 24 lines net. All 23 SupportDrawer tests pass untouched, including every regression from the four rounds: single prefill under a mid-open snapshot change, latest-snapshot publication, close/reopen, dismissed cycle, and the token-resolution path. That they still pass against a different mechanism is the point — they pin behaviour, not implementation. One behaviour change worth naming: a failed configure no longer retries on the next unrelated re-render. That retry was incidental, not designed — it only ever fired if some dependency happened to change. Reopening support retries properly, and ensureNativeCrispConfigured clears its memo on failure so the retry really re-configures.
Follow-up: the native open effect, simplified (cda30e5)Four rounds of review produced four defects in one place — duplicate prefill → stale payload → duplicate again across close/reopen — and each fix seeded the next. That is a shape, not bad luck, so rather than adding a fifth guard I went after the cause. The effect both published live data and performed a one-shot action, and it listed the data it publishes in its dependency array. So every snapshot change re-entered a function whose tail calls It now depends only on what defines an open cycle — the drawer being open, and the token being ready. That deletes both generation refs and the boolean before them. What remains is React's own idiom: useEffect(() => {
if (!isSupportModalOpen || !isCapacitor() || isAwaitingToken) return
let cancelled = false
ensureNativeCrispConfigured().then(async ({ CapacitorCrisp }) => {
await ensureNativeCameraPermission()
if (cancelled) return
const { userData: snapshot, tokenId, prefilledMessage: prefill } = latestPayloadRef.current
...
})
return () => { cancelled = true }
}, [isSupportModalOpen, isAwaitingToken, setIsSupportModalOpen, clearSupportBadge])Four coordinating mechanisms become two — the token gate and cancellation. Net −24 lines, and the All 23 SupportDrawer tests pass untouched, including every regression from the four review rounds: single prefill under a mid-open snapshot change, latest-snapshot publication, close/reopen, dismissed cycle, token resolution. They were written against the guards and still pass against a different mechanism — which is the point, they pin behaviour rather than implementation. One behaviour change worth naming rather than burying: a failed configure no longer retries on the next unrelated re-render. That retry was incidental — it only ever fired if some dependency happened to change — not designed. Reopening support retries properly, and Gate: typecheck, eslint (0 errors, 0 warnings in this file), prettier, 1532 tests / 100 suites. |
There was a problem hiding this comment.
Chip review — changes requested
One major privacy defect remains: a guest snapshot can carry cached activity and limits from the prior authenticated session. All three previously reported defects are fixed.
Findings
-
MAJOR · src/hooks/useCrispUserData.ts:194 · Do not publish authenticated cache entries for guests
When/users/mereturns 401, auth becomes null but the existing[transactions]and[limits]queries remain cached. SupportDrawer is also mounted on the setup/guest layout, so opening support before those entries are garbage-collected serializes the prior user's transaction UUID/amount and limits here even thoughuserIdis absent. Gate these fields (and preferably the underlying cache reads) on the current authenticated user, and cover the auth-to-guest transition in the hook test. -
MAJOR · src/utils/support-cache.ts:53 · [moonshotai/kimi-k3] latest_activity can report another user's transaction (targetUsername cache variants are scanned)
readLatestHistoryEntry scans client.getQueryCache().findAll({ queryKey: [TRANSACTIONS] }), which matches every cached history query, keyed as [TRANSACTIONS, 'latest', { limit, targetUsername }] and infinite variants. The file's own comment acknowledges targetUsername is part of the key, but the scan does not filter on it. Reachable path: the user views someone else's profile/activity (the targetUsername variant exists for exactly that), that user's entries are cached, the user then opens support, and the newest entry across ALL variants wins — so buildLatestActivity summarizes a third party's transaction (kind, status, amount, failReason, uuid) and pushes it to the Crisp sidebar as the current user's own latest activity. That is precisely the counterparty-payment-record leak the module's header says it prevents, in attribution form, and it also misleads the agent ('your last transaction was X' when it wasn't theirs). Fix: in the findAll loop, skip any query whose key params carry a targetUsername that is defined and not the current user (e.g. const params = query.queryKey[2]; if (params?.targetUsername) continue), or restrict the scan to keys explicitly owned by the self-history queries.
Checked clean
- FIXED: the open support snapshot now observes relevant React Query cache updates without starting requests.
- FIXED: account stats report referral presence without including the inviter's username.
- FIXED: the native open cycle publishes the latest committed snapshot after its awaits and cancels a dismissed cycle.
- Exact-head CI completed successfully, including lint, typecheck, unit, e2e, format, and analysis gates.
- Checked cache key scoping, passive auth-expiry behavior, guest SupportDrawer reachability, route redaction, sink parity, and native open-cycle concurrency.
- Local targeted Jest could not run because the detached worktree has no installed Jest binary; exact-head unit CI is green.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: cda30e57cd42 · Context: repo, ci
…d user An explicit logout clears the query cache (authContext cancels then clears it), but a session that simply EXPIRES does not: /users/me 401s, `user` becomes null, and `[limits]` and `[transactions]` stay warm behind keys that carry no user id. SupportDrawer is mounted on the guest and setup layouts too, so the next person to open support on that device published the previous user's limits and last transaction — uuid, kind, amount, status — under an empty `user_id`. Worse than the counterparty leak this module was built to prevent: not a name attached to the user's own transaction, but another person's state presented as nobody's. All four cache reads are now gated on an authenticated user. Balance and card were already keyed by wallet address and user id, so this makes the rule uniform rather than incidental — no authenticated cache entry is read without an authenticated user, whatever the key looks like. Test covers the auth-to-guest transition with a warm cache: nothing from it reaches the snapshot, and the previous user's transaction id appears nowhere in the serialized payload. Fails without the gate. Also fixes phrasing spotted in that test's output: a transaction from seconds ago read "just now ago". Found by Chip on #2851.
There was a problem hiding this comment.
Chip review — changes requested
Two major issues remain: userless warm caches can cross an account switch, and Android appends the dynamic primary Crisp segment instead of replacing stale routing state.
Findings
-
MAJOR · src/hooks/useCrispUserData.ts:128 · Scope warm caches to the authenticated account
The new guest gate only suppresses cache data while auth is absent. After a passive /users/me 401, the userless [limits] and [transactions] entries remain warm; if a different account logs in on that device, isAuthenticated becomes true and these reads immediately publish the previous account's limits and latest transaction into the new account's Crisp session before refetch. Key these caches and readers by userId (or remove them whenever the authenticated user ID changes), and add an expiry-to-different-login test. -
MAJOR · src/components/Global/SupportDrawer/index.tsx:249 · Overwrite Android segments instead of appending
The pinned @capgo/capacitor-crisp 8.0.27 Android bridge implements setSegment with Crisp.setSessionSegment(segment), whose one-argument form keeps the SDK's default append behavior; only iOS assigns a single replacing segment. Reopening support after balance-unavailable, offline, or kyc-pending has cleared therefore adds the new primary segment but leaves the stale one routing the conversation. Expose the SDK's overwrite overload/setSessionSegments(..., true), or omit dynamic native routing until it can be replaced, and test the Android bridge semantics rather than only the JS mock call.
Checked clean
- FIXED: authenticated cache entries are no longer published while the current snapshot is a guest.
- FIXED: the open support snapshot now observes relevant React Query cache updates without starting requests.
- FIXED: account stats report referral presence without including the inviter's username.
- Pinned head, base SHA, base ref, trusted author, and merge base all matched the supplied values.
- Exact-head CI completed successfully, including unit, typecheck, eslint, format, e2e, preview, and analysis gates.
- The widened Crisp disclosure is present in the canonical privacy content, and its production content PR is merged.
- Reviewed cache ownership across passive expiry and account switching, cache subscriptions, balance unknown-vs-zero handling, counterparty and route redaction, session-field parity, proxy updates, native open-cycle concurrency, and web/native segment semantics.
- A local focused run passed all 32 pure support-context tests; suites requiring the detached worktree's missing dependency links could not resolve next-intl, while exact-head unit CI is green.
Second opinion skipped: openrouter-empty-reply.
Exact head: 560bfdd6b115 · Context: repo, ci, product, provider
…oid segment Two findings from Chip's sixth pass. Warm caches could cross an account switch. The guest gate from the last commit only suppressed cache data while auth was absent; it did not stop the previous account's rows reaching the NEXT one. After a passive 401 the unscoped `[limits]` and `[transactions]` entries stay warm, so signing a different account in on that device served them until each refetch landed. The fix is in authContext, not here, because the leak is not the support sidebar's: react-query serves that stale data to whatever reads it, so the new user briefly saw someone else's Activity as well. An explicit logout already clears the cache there; this makes an account CHANGE do the same. It fires only between two different accounts — a first sign-in keeps what a guest legitimately prefetched, e.g. a claim link opened before signing in. Keying `[limits]` and `[transactions]` by user id would be the better end state, but `[TRANSACTIONS]` is a prefix for eight invalidation sites and is used directly in two more; that refactor deserves its own PR and review, not a ride along in a support change. Dropped the native Crisp segment. The plugin exposes only a one-argument `setSegment`, which on Android calls `Crisp.setSessionSegment` with no overwrite flag, so segments append: a stale `offline` or `balance-unavailable` keeps routing a conversation after the user has left that state. A routing tag that is wrong is worse than one that is missing. Nothing reaches the agent differently — the `segments` data row carries the whole list and `setString` assigns, so it replaces cleanly on every open. `primarySupportSegment` existed only to pick that one segment and is removed with it, along with the docblock asserting Android assigned like iOS — which was the wrong assumption behind the bug.
…never could `CapacitorCrisp.sendMessage` is declared `unimplemented` in this plugin on BOTH iOS and Android — it is the only unimplemented method on iOS. So every "contact support about X" entry point lost its context in the app: the composer stayed empty and the agent opened the conversation blind, on both platforms, not just Android as first reported. It failed silently twice over. The call was never awaited, so `unimplemented` rejected into nothing — an unhandled "Not implemented on ios" rejection behind every native support open that carried a topic. That is a plausible second source of the "plugin not implemented on ios" Sentry events previously put down entirely to the /crisp-proxy iframe. The topic now rides in a `support_topic` session-data row, through `setString`, which both platforms do implement. It goes through the same `supportSessionFields` definition as everything else, so all three sinks carry it: web keeps the prefilled composer AND gains the row, which survives a user clearing the text before sending. The native composer still opens empty — the SDK gives no way to fill it — so the user writes their own first message. What changes is that the agent already knows what brought them there. JS only: this ships over OTA, with no native release. One existing test asserted `sendMessage` was called once per open cycle. It was pinning a no-op; it now pins the data row that actually carries the topic.
Native support prefill — fixed here after all (b73f642)Correcting what I said in the thread above: this is not Android-only. // ios/Sources/CapacitorCrispPlugin/CapacitorCrispPlugin.swift:143
@objc func sendMessage(_ call: CAPPluginCall) {
call.unimplemented("Not implemented on iOS.")
}// android/.../CapacitorCrispPlugin.java:215
public void sendMessage(PluginCall call) {
call.unimplemented("Not implemented on Android.");
}So every "contact support about X" entry point has been losing its context in the app on both platforms, and it failed silently twice over: the call was never awaited, so The fix routes the topic through Being precise about what this does and does not restore: the native composer still opens empty, because the SDK offers no way to fill it. The user writes their own first message. What changes is that the agent is no longer blind to what brought them there. JS only, so it ships over OTA with no native release. One existing test asserted Gate: typecheck, eslint clean, prettier, full suite 3644 passed / 292 suites. |
There was a problem hiding this comment.
Chip review — changes requested
One previously reported major remains: unscoped warm caches can still be published under a newly authenticated account before the effect-based cleanup runs. The guest-cache and Android-segment findings are fixed.
Findings
-
MAJOR · src/hooks/useCrispUserData.ts:128 · Keep cached support data scoped to its account
These readers still accept the global [LIMITS] entry and every [TRANSACTIONS] entry without proving they belong to userId. If account A expires while support is open and account B authenticates, B's render reads A's warm data here; SupportDrawer's child effect can publish it before AuthProvider's parent effect calls queryClient.clear(), and cache removals do not trigger this hook's watcher. Key these queries/readers by userId, or store and verify an owner before returning cached values; a post-render clear is not a privacy boundary. -
MINOR · src/context/authContext.tsx:121 · [moonshotai/kimi-k3] Cross-account cache clear runs after SupportDrawer publishes, leaving a one-commit leak window
The new effect clears the query cache when the signed-in account changes, but it lives in AuthProvider, the parent of SupportDrawer. React runs child effects before parent effects, so on the commit where user B's profile lands after user A's session expired passively (explicit logout already clears; expiry does not — the code's own comment calls out that[limits]and[transactions]carry no user id and stay warm), the ordering is: (1) SupportDrawer's child effect postMessages the freshuserDatato the proxy iframe / answers a handshake pull fromlatestPayloadRef, computed withisAuthenticated === truefor B but reading A's still-warm[LIMITS]and[TRANSACTIONS]cache entries viareadCachedLimits/readLatestHistoryEntry; (2) only then does AuthProvider's effect runqueryClient.clear(). The result is A's fiat limits and last transaction (kind/status/amount/uuid) pushed to the third-party Crisp console attributed to B's user_id — exactly the cross-account exposure this PR builds theisAuthenticatedgate and this clear to prevent; the gate cannot help here because B is authenticated at that moment. Preconditions narrow the window (passive expiry, then sign-in as a different account while the support proxy iframe is alive, which the PR notes is possible since SupportDrawer mounts on guest/setup layouts), but it is reachable. Fix: clear before children render with the new id — e.g. run the clear in the login-success path that sets the new user (before any commit carrying B's data), or use React's adjust-state-during-render pattern in AuthProvider to detect the id change during render (parents render before children) and clear there, or haveuseCrispUserDatatreat cache-derived fields as invalid on the first render after a userId change.
Checked clean
- Confirmed detached HEAD and merge base exactly match the supplied head and base SHAs.
- Earlier guest-cache finding FIXED: authenticated cache reads are gated when user/userId are absent and covered by an expired-session test.
- Earlier Android segment finding FIXED: native setSegment calls were removed and the current list is assigned through the segments data row.
- Earlier account-scope finding STILL PRESENT: LIMITS and TRANSACTIONS cache keys remain account-agnostic and cleanup happens in a later effect.
- Reviewed balance unknown-vs-zero handling, counterparty scrubbing, route normalization, sink field parity, native open-cycle behavior, and cache-only reads.
- Exact-head CI is green for unit, typecheck, eslint, format, e2e, analyze, and deploy preview.
- Local pure support-context suite passed 30 tests; four additional local suites could not resolve next-intl from the shared dependency install, so exact-head CI supplied the complete test evidence.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: b73f6426db64 · Context: repo
Drops `limits_remaining` and `latest_activity`, and reverts the cross-account cache clear that was added to make them safe. Those two fields were the only ones reading `[limits]` and `[transactions]`, whose keys carry no user id. That is the root behind the last three review findings — the guest leak, the cross-account leak, and the effect-ordering window — each a different angle on the same hole, and each patch narrowed it without closing it. React runs child effects before parent effects, so the clear in AuthProvider could not protect a child that publishes in the same commit: a post-render clear is not a privacy boundary. Balance and card were never part of it. Their keys name the identity they belong to — wallet address, user id — so a cached entry can be proved to belong to the person support is open for. That is now the stated rule in support-cache: only read a key that names its owner, and do not add a reader that cannot answer "whose data is this?" from the key alone. The two fields come back once `[limits]` and `[transactions]` are user-scoped. That refactor belongs in its own PR: `[TRANSACTIONS]` is a prefix for eight invalidation sites and used directly in two more, and scoping it also fixes the home screen serving a previous account's Activity — a bug that has nothing to do with support and should not be fixed inside a support change. The cache clear is reverted for the same reason. It was app-wide, it was only here to protect fields that no longer exist, and it carried a known ordering flaw. The bug it addressed is real and is recorded for that PR. The sidebar keeps balance, account stats, card, linked accounts, app context, sentry link, verification state and support topic. Full suite green: 3636 passed across 292 suites.
Crisp segments are a field people write by hand, and one of them backs an OKR. Agents tag translation reports `translation-issue`, and the monthly count only sees a conversation that still carries the tag — untagged reports are invisible to it (mono/ops/playbook/translation-issue-tag.md, TASK-20776). Ops tags incidents the same way. Crisp has no partial write: a set replaces the whole list. So the app's write erased whatever a human had put there, and it runs on every snapshot change rather than only on open — an agent could tag a conversation and watch the tag vanish while the user was still in it. Appending instead is not a fix. That was the original behaviour; it leaves the app's own flags stale, so a user who was briefly offline routes as `offline` for good. Neither mode is safe while humans and the app share one field, so the app writes none. Nothing is lost to the agent. The same flags already ride as the `segments` row in the sidebar, which is where they were readable anyway. What the app gives up is filtering the inbox by its own state — a capability that never existed and that nobody has asked for, against an OKR count that exists and is load-bearing. Deliberately a deletion, not a redesign: the `segments` row, buildSupportSegments and every other field are untouched. The design question is TASK-21968.
Segment write removed (1d87998)Closing the last open item before merge. The app no longer writes Crisp segments at all. Why, in one line: segments are a field people write by hand, and one of them backs an OKR. Agents tag translation reports Crisp has no partial write — a set replaces the whole list — so the app's write erased whatever a human had put there. And it ran on every snapshot change, not just on open, so an agent could tag a conversation and watch the tag vanish while the user was still in it. Appending is not the alternative: that was the original behaviour, and it leaves the app's own flags stale ( Nothing is lost to the agent. The same flags already ride as the Deliberately a deletion rather than a redesign: the Worth recording how this was found, because it is not a code defect. A reviewer reading a diff against the codebase cannot see how the product is operated — Follow-ups filed: TASK-21968 (the segments design question) and TASK-21970 (scope Full suite green: 3635 passed across 292 suites. |
There was a problem hiding this comment.
Chip review — changes requested
Request changes: support-topic metadata has no stable open-cycle lifecycle, and an in-transit balance edge emits contradictory zero-balance context. Both prior account-scoping findings no longer apply at this head.
Findings
-
MAJOR · src/utils/crisp.ts:83 · Scope support_topic to one open cycle
On the proxy, later snapshot pushes deliberately call setCrispUserData without the unchanged composer prefill; this full session:data write then replaces support_topic with an empty string, so a balance or verification update can erase the topic that was meant to survive composer edits. In the other direction, ModalsContext never clears its prefill, so a later generic support open can publish the previous topic. Give the topic its own per-open-cycle value and pass it to session data independently of whether message:text should be refreshed. -
MINOR · src/hooks/useCrispUserData.ts:203 · Count in-transit funds before flagging zero balance
buildBalanceSummary includes inTransitToCollateralCents in the displayed spendable total, but this zero test ignores it. During the smart-to-Rain handoff, smartBalance=0n, spendingPower=0, and a positive inTransitToCollateralCents produce a funded balance row together with a zero-balance segments row. Compute zero from the same display total so the sidebar cannot contradict itself.
Checked clean
- Verified the detached worktree head, supplied base SHA, PR author, and dev base ref.
- Earlier major finding in useCrispUserData: NO LONGER APPLIES; limits and latest activity were removed, and remaining cache reads are keyed by wallet address or user ID.
- Earlier blocking finding in authContext: NO LONGER APPLIES; the post-render cache clear was reverted together with the unscoped observers it could not safely protect.
- Reviewed web proxy handshake, native Crisp sink parity, account-switch isolation, route redaction, unread-badge behavior, and cache-only query behavior.
- Exact-head CI passed unit, e2e, typecheck, eslint, format, deploy-preview, and analyze checks.
- The current privacy disclosure includes the Crisp account-context snapshot wording before this code reaches production.
- The pure support-context suite passed locally; other focused suites were not runnable from the detached worktree because dependencies were not installed there, while exact-head CI passed them.
Second opinion skipped: openrouter-timeout.
Exact head: 1d87998fa6f3 · Context: repo, content
Two ways the topic went wrong, in opposite directions. It was erased. A routine metadata refresh deliberately omits the composer prefill — re-pushing it would overwrite what the user is typing — but the same call writes the whole session:data block, so `support_topic` went back to empty. A balance landing mid-conversation wiped the reason the user opened support. The topic is now its own argument to setCrispUserData, defaulting to the composer value, and the proxy passes it on every push while still guarding `message:text`. It also outlived its cycle. Nothing ever cleared `supportPrefilledMessage`, so after one "contact support about X" entry point every later open — the nav button included — reopened with X still in the composer, and reported X as the topic. Closing the drawer now clears it, which is where a prefill's life should have ended all along. That half was already true of the composer before this PR; the topic row only made it visible. Also: decide zero balance from the same total the balance row prints. During the smart-to-collateral handoff the funds are in neither bucket and only `inTransitToCollateralCents` accounts for them, so reading the two halves directly put a `zero-balance` flag next to a funded balance row. Found by Chip on #2851.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: the new support_topic row can automatically disclose a send-link bearer secret. Both earlier findings are fixed, and exact-head CI is green.
Findings
- BLOCKING · src/utils/crisp.ts:83 · Redact send-link secrets before publishing the support topic
Opening support from ClaimErrorView supplies window.location.href as the prefill. Claim URLs keep the bearer password in the #p= fragment, and that password deterministically derives the private claim key. This new session:data row publishes the raw prefill to Crisp immediately, before the user sends the composer; a transient NOT_FOUND state can therefore disclose an unclaimed link credential to Crisp/support. Strip fragments and sensitive query parameters before materializing support_topic, or pass a sanitized claim identifier instead.
Checked clean
- Earlier major finding (scope support_topic to one open cycle): FIXED; close now clears the prefill, while metadata refreshes preserve the topic during the active cycle.
- Earlier minor finding (count in-transit funds before zero-balance): FIXED; the segment now uses computeDisplaySpendable and has an in-transit regression test.
- Correctness: reviewed authenticated cache ownership, unavailable-vs-zero balance handling, live cache refreshes, topic lifecycle, and native/web/proxy field parity.
- Security: reviewed postMessage source/origin guards, identity isolation, route redaction, linked-account minimization, and support-prefill trust boundaries.
- Legal disclosure context: mono privacy content includes the widened Crisp account-context disclosure.
- Exact-head CI is green: unit, typecheck, eslint, format, e2e, analyze, review, and deploy-preview checks succeeded.
- Local targeted Jest execution was unavailable because this detached worktree has no installed Jest binary; exact-head unit CI succeeded instead.
- Slop pass: no additional actionable duplication, dead code, misleading naming, or architecture drift survived verification.
Second opinion skipped: openrouter-timeout.
Exact head: 18e76b422b42 · Context: repo, product
BLOCKING. A claim link is /claim?c=...&v=...&i=...#p=<password> (history.utils), and that fragment is a bearer credential - it derives the private claim key, so whoever holds it can take the funds. ClaimErrorView hands support window.location.href, fragment included. Before this PR that URL only sat in the composer, where the user chose whether to send it. The support_topic row changed that: the app publishes it to Crisp the moment the drawer opens. So a transient NOT_FOUND on a claim page disclosed an unclaimed link's credential to a third party with no user action at all. Redacted at the setter, not at either sink. Both paths - the composer and the topic row - read the same stored prefill, and every call site goes through openSupportWithMessage, so one place covers both and covers call sites nobody has written yet. not-found.tsx already carries a comment about exactly this hazard and sends a bare pathname, and two later call sites (ClaimErrorView, Error.validation.view) were added passing the full href anyway. Per-call-site discipline was tried and drifted. Only the fragment goes. The query identifies the deposit on-chain and is what lets an agent actually find the link, so stripping it would trade a real security fix for a support regression. Two tests, both failing without the fix: the redactor keeps ?c=&v=&i= while dropping #p=, and the context never stores a password whatever the call site passes. Found by Chip on #2851.
There was a problem hiding this comment.
Chip review — changes requested
The three earlier findings are fixed, but the web proxy keeps publishing the widened account snapshot after support closes.
Findings
-
BLOCKING · src/components/Global/SupportDrawer/index.tsx:80 · Stop publishing account snapshots after support closes
After the first web open, the proxy iframe remains mounted, and this effect forwards every lateruserDatachange without checkingisSupportModalOpen. For example, after the user closes support, a connectivity change or auth-context rerender can pick up a new cached balance/card overview and post that widened financial and verification snapshot to Crisp while chat is closed. This contradicts the privacy disclosure that the snapshot is shared only when support chat is opened. Gate proxy data updates to the current open cycle (while clearingsupport_topicseparately on close), or stop/unmount the proxy when closed. -
MAJOR · src/utils/support-context.ts:335 · [moonshotai/kimi-k3] redactSupportText keeps query-string credentials; only fragments are stripped
ModalsContext routes every support prefill through redactSupportText, and call sites hand it window.location.href (ClaimErrorView, Error.validation.view per the in-module comment). The regexhttps?:\/\/\S+only cuts at '#', so any credential living in the query survives into the composer text and thesupport_topicrow that the app pushes to Crisp servers before the user sends anything. The module's own comment concedes this class of secret exists ("arbitrary pages can carry magic-link and OAuth tokens in the query too") but relies on individual call sites like not-found.tsx to pre-redact to a bare pathname — a new or existing call site passing href off an OAuth callback (?code=) or magic-link (?token=) URL leaks a bearer credential to Crisp. Fix: parse each matched URL and, unless the origin+path matches an explicit whitelist (e.g. /claim, whose c/v/i params an agent needs), drop the query and keep origin+pathname only; otherwise strip the query universally except whitelisted params.
Checked clean
- Earlier blocking finding fixed: openSupportWithMessage now strips URL fragments before claim-page support text reaches the composer or support_topic.
- Earlier major finding fixed: closing the drawer clears the prefill, and the next open writes an empty support_topic.
- Earlier minor finding fixed: zero-balance uses computeDisplaySpendable, including inTransitToCollateralCents.
- Cache readers are user-scoped, remain fetch-free, and suppress cached account data after authentication expires.
- Web, proxy, and native session fields share one definition; native no longer calls the unimplemented sendMessage path or writes stale Crisp segments.
- Exact-head CI: typecheck, unit, eslint, e2e, format, review, and analyze succeeded; report and Deploy-Preview were still in progress when checked.
- Mono privacy disclosure says the account-context snapshot is shared only when support chat is opened.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: 455883244b06 · Context: repo, product
… query secrets Two findings, both about data leaving without the user asking. The proxy pushed after close. The iframe stays mounted once support has been opened, and the update effect forwarded every later userData change without checking whether the drawer was open. A connectivity flip or an auth re-render therefore posted a fresh balance, card and verification snapshot to Crisp with the chat closed. That is not a performance detail: the privacy policy this PR ships alongside says the snapshot is shared only when the user opens support chat, and this made that sentence untrue. Gated on isSupportModalOpen. The last push of a cycle stands until the next open replaces it, which is that conversation's own context and correct to leave. The redactor only cut fragments. That covered the claim password but not the other place a credential hides: ?code= on an OAuth callback, ?token= on a magic link. not-found.tsx already sends a bare pathname for exactly that reason, and its comment names the class. Call sites hand over window.location.href, so the next one added off such a page would have leaked a bearer credential to Crisp before the user sent anything. The query is now dropped by default and kept only for paths known to identify rather than authorize - /claim, whose c/v/i locate the deposit on-chain and are what let an agent find the link at all. A deny-list of secret-looking parameter names was the alternative and was rejected: it has to be right about every page that exists now and every one added later. This is wrong in the safe direction, so an unlisted page loses context, never a credential. Both tests verified to fail without their fix. The closed-drawer one needed rewriting first: the naive version opened nothing, so the iframe never mounted and it passed either way - it now opens, closes, then lands a balance.
There was a problem hiding this comment.
Chip review — changes requested
The send-link secret finding is fixed, but the earlier closed-drawer publication blocker remains reachable through proxy initialization.
Findings
- BLOCKING · src/components/Global/SupportDrawer/index.tsx:340 · Gate proxy initialization after support closes
STILL PRESENT from the prior review. If a logged-in user opens support while the Crisp token is resolving, closes it, and the token then resolves,hasBeenOpenedstill allows the hidden iframe to mount. ItsCRISP_PROXY_REQUEST_INITreaches this unconditional reply, which publishes the latest account snapshot after the drawer closed despite the open-only disclosure. Guard the init reply with the current open state (using a ref because this listener is registered once), or defer proxy mount/boot until the next open; cover close-before-token-resolution and closed remounts in the test.
Checked clean
- Earlier send-link secret finding: FIXED; support topics are redacted before storage, claim fragments are stripped, and non-claim queries are dropped.
- Cache-only snapshot reads use user-scoped balance and card keys, require authenticated state, and watch cache updates only while support is open.
- Account-switch isolation, native open-cycle cancellation, sink field parity, counterparty-safe route normalization, and unknown-balance handling were reviewed.
- The widened Crisp disclosure is present on mono origin/main.
- Exact-head CI completed successfully: analyze, eslint, typecheck, e2e, unit, format, preview, and aggregate gates are green; git diff --check is clean.
Second opinion skipped: openrouter-timeout.
Exact head: 216513a39fa3 · Context: repo, mono
Gating the update effect covered one of two ways the snapshot leaves. Two more paths reached Crisp with the chat closed. The iframe could first mount while closed. hasBeenOpened latched on the drawer alone, but the iframe is also gated on the token being ready - so opening support while a logged-in user's token resolves, then closing, left the latch true and mounted the hidden iframe when the token landed. It booted, handshook, and published outside an open cycle. The latch now needs both. The handshake reply answered unconditionally. A token or locale change remounts an already-mounted iframe via its key, and that can happen with the drawer shut; the fresh proxy asks for its payload and got it. The listener is registered once so it cannot close over the open state - it reads a ref instead. Staying silent is safe rather than fatal: the proxy re-asks every 250ms until it boots, and the update effect posts the payload the moment the drawer opens, which boots it directly. If the 8s watchdog fires first the parent shows the mail fallback and the later CRISP_READY clears it. Both tests verified failing without their fix - which took three attempts and is worth recording. The first never mounted the iframe, so nothing posted either way. The second built a MessageEvent whose source jsdom drops, so the identity check skipped the branch; the file's own requestInit helper sets it via defineProperty. The third looked right but prettier had folded the guard into 'if (...) return // comment', so the probe that deleted it never matched and the guard was present the whole time. A test that cannot fail proves nothing.
There was a problem hiding this comment.
Chip review — changes requested
Both earlier closed-support blockers are fixed, but the current head still has a credential-redaction hole and two support snapshot lifecycle defects.
Findings
-
BLOCKING · src/utils/support-context.ts:354 · Allowlist the claim query before publishing it
The/claimexemption keepsurl.searchwholesale, although onlyc,v, andiare declared safe. A claim URL containing an extra credential-bearing parameter or an encoded nested link therefore retains that credential after the fragment is stripped, andClaimErrorViewpublishes the result to Crisp as soon as support opens. Rebuild the retained query from an explicit safe-parameter allowlist (and add a case with an unsafe extra/nested parameter) instead of forwarding the original search string. -
MAJOR · src/hooks/useCrispUserData.ts:93 · Close the cache subscription setup race
Opening support reads the cache during render and subscribes only in a passive effect. If the in-flight balance or card query resolves after that render but before this effect attaches, its onlyupdatedevent is missed and the conversation keeps the initialunavailablesnapshot and routing flag until some unrelated render or later cache event. Re-read once immediately after subscribing, or use a subscription primitive that rechecks the snapshot after registration; cover the update-between-render-and-effect case. -
MAJOR · src/app/crisp-proxy/page.tsx:39 · Clear the old web composer on reopen
Closing support now clears the context prefill, but the iframe is intentionally kept mounted and this update passes an empty prefill tosetCrispUserData, whose truthy guard emits nomessage:textcommand. After opening support from an error CTA, closing without sending, and reopening from navigation, Crisp still shows the previous error text in the composer even though the new open cycle has no prefill. WhenprefillChangedis true and the next value is empty, explicitly set the composer to an empty string; keep routine metadata refreshes untouched.
Checked clean
- Exact worktree HEAD and merge base match the supplied head and base SHAs.
- Earlier account-snapshot push after close: FIXED by the open-state gate and regression test.
- Earlier proxy initialization after close: FIXED by token-ready latching plus the current-open handshake gate and regression tests.
- Exact-head format, typecheck, eslint, unit, e2e, analyze, preview, and aggregate CI checks completed successfully.
- Cache readers remain fetch-free and only use identity-scoped balance and card keys; unscoped activity and limits fields were removed.
- Canonical privacy content contains the disclosed account-context wording; production publication order remains outside this code head.
- Crisp web SDK behavior for
message:textwas checked against the provider documentation.
Second opinion skipped: openrouter-timeout.
Exact head: c566cc5f67bc · Context: repo, product
…e subscribe race Three findings, all confirmed against the code first. Naming the path was not enough. The /claim exemption forwarded url.search wholesale, so anything else riding on a claim URL - a parameter added later, an encoded nested link - survived the fragment strip and reached Crisp as soon as support opened. Only c, v and i were ever vetted, and the query is now rebuilt from those names, so an unvetted parameter is dropped rather than published. The composer kept the previous cycle's text. The iframe stays mounted between opens, and clearing the context prefill made the next payload carry an empty string - which a truthy guard skipped. Open support from an error CTA, close without sending, reopen from the nav, and the old error text was still in the box. undefined now means leave the composer alone (a routine metadata refresh must not overwrite typing); an empty string is a real instruction to clear it. Same distinction applied to the proxy's anonymous branch. The cache subscription could miss its only event. Values are read during render and the subscription attaches in a passive effect, so a balance or card query resolving in that window emitted its update with nobody listening, and the conversation kept the initial unavailable snapshot - and the routing flag derived from it. The effect now re-reads once after attaching. Each test verified failing without its own fix, restored between probes. The race test uses a sibling layout effect, which lands exactly in the window: React runs layout effects after render and before passive effects. 1550 tests across 103 suites green.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The four earlier findings are fixed, and no actionable defect remains at this exact head.
Checked clean
- Earlier claim-query finding: FIXED; /claim now rebuilds the query from the explicit c/v/i allowlist and tests drop extra and nested parameters.
- Earlier cache-subscription race: FIXED; the hook re-reads immediately after attaching the cache listener and covers the render-to-effect update window.
- Earlier stale web composer finding: FIXED; an empty prefill now emits message:text with an empty string while routine metadata refreshes still leave typed text alone.
- Earlier closed-drawer proxy initialization finding: FIXED; token-ready mounting and handshake replies both require an open support cycle, with close-before-token and closed-handshake regressions covered.
- Correctness, security, adversarial, and slop passes covered cache ownership, account switching, balance unknown/zero handling, native open-cycle cancellation, support-topic redaction, route normalization, and field parity across Crisp sinks.
- The canonical privacy text discloses the account-context snapshot as shared only when support chat is opened.
- Exact-head CI is green for unit, e2e, typecheck, eslint, format, analyze, preview, review, report, and aggregate gates; git diff --check is clean. Local targeted Jest execution was unavailable because this detached worktree has no installed jest binary.
Second opinion skipped: openrouter-timeout.
Exact head: 889717db810b · Context: repo, mono
This branch was successfully deployed
1 participant
Follow-up to #2736. That PR gave the agent sidebar identity, wallet links and verification state. It gave it nothing about what the user actually has or just did, so a thread still opens with three rounds of "what's your balance / what did you last do / which build are you on".
Worth stating plainly, because it framed the whole design: none of this needs database access, and none of it grants any. Crisp never had a connection to our data — the client pushes
session:data. The ceiling was never "DB vs no DB", it was "what does the client already know". It already knows all of this.New sidebar rows
Every value comes from the
/get-userread-models already inauthContextplus warm react-query caches. No new endpoint, no backend change.balanceaccount_statslatest_activityfailReason, uuidlimits_remainingcardlinked_accountsapp_contextsentry_issuesPlus Crisp segments for the boolean half —
ios-native,kyc-pending,verification-blocked,zero-balance,balance-unavailable,offline,guest,card-holder. Segments are what the inbox filters and routes on, which is the right home for yes/no facts, and keeping them out ofsession:datais what stops the sidebar becoming a wall ofyes/norows nobody scrolls past.app_contextis the one I'd single out as under-rated: an agent currently sees "Chrome 150 on Linux" and nothing about our app. Three version layers can disagree — the web bundle (a document can outlive arbitrarily many deploys, seeuseStaleDeploymentReload), the native binary, and the Capgo OTA bundle on top of it. An agent who can see all three says "update the app" instead of debugging a bug that was fixed a fortnight ago.Three things worth reviewing carefully
No new requests.
SupportDrawermounts this hook app-wide — every screen, guests included. AuseWallet()/useLimits()/useRainCardOverview()call inside it would switch on a 30s RPC poll plus two API requests for every user, which is precisely the shape that made/rain/cardsthe most-called endpoint in the app. All reads go throughsupport-cache.ts, which only looks at what is already in the query cache, anduseCrispUserData.test.tsxasserts the hook leaves the cache empty.Unreadable is never zero.
rainCentsToUsdcUnits(undefined)is0n, so "no answer" and "genuinely empty" are arithmetically identical — the $0-balance bug (PEANUT-UI-QD5) in its support-facing form. An agent reading$0.00tells a funded user they have no money, which is worse than telling them nothing. Each half is either a figure or the wordunavailable, and the total only appears when both halves are answers. A user with no card application is still a real zero and prints as one.The user's own state, never a counterparty's.
HistoryEntrycarries the other party's username and full name. A support console is the wrong place to accumulate third parties' payment records, so activity summaries name the kind, status and amount — the uuid is enough for an agent to look up the rest. Same rule forlinked_accounts:iban:DE, never the IBAN.Drift fix
The three sinks (web widget, proxy iframe, native
setString) each wrote the field list by hand, and native had fallen to two keys where web sent seven — so the agents helping app users saw the least. There is now onesupportSessionFieldsdefinition feeding all three, with a test pinning it. Native holds a single segment (CrispSDK.session.segment = …is an assignment, not an append — same on Android), so it gets the most actionable one viaprimarySupportSegmentand the full list rides along as a data row.Tests
60 passing across the touched suites — 25 on the pure builders, 4 on the cache readers, 5 on sink parity, 4 on the hook (including the no-fetch invariant), plus the existing Crisp/SupportDrawer/verification suites unchanged.
The privacy policy's third-party table described Crisp as receiving "your messages, email address, and basic device data". This PR widens that considerably, so the disclosure had to move first.
It has.
mono/content/legal/privacy/en.mdnow reads:peanutprotocol/mono@26bd1817,
last_updatedbumped. The mirror synced and the publish PR is open: #2855 (basemain).It cannot ship atomically with this PR — the policy lives in the
src/contentsubmodule and the repo rule is that content and code never travel together — so the guarantee is merge order instead:No change to processor scope: Crisp was already a named processor in that same row, and this widens what it receives inside an existing relationship rather than adding one. Retention is Crisp-side and untouched.
The gap that let this nearly slip is now closed at the source —
skills/peanut-prphase 6b never namedcontent/legal/in its docs-impact search scope, so the subagent would not have opened the privacy policy. It now asks the legal question separately and requires quoting the sentence that is wrong (peanutprotocol/mono@0a08a271).