fix(max): guard scene-context reads against unmounted scene logic - #70335
Conversation
🤖 CI report✅ Bundle size — 🟢 -5 B (-0.0%)Uncompressed size of every built Total: 64.33 MiB · 🟢 -5 B (-0.0%) No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 668 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 278.6 KiB | ../node_modules/.pnpm/posthog-js@1.399.2/node_modules/posthog-js/dist/rrweb.js |
| 266.9 KiB | ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 234.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 221.5 KiB | ../node_modules/.pnpm/posthog-js@1.399.2/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.9 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
⚠️ Dist folder size — 🔺 +2.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1280.78 MiB · 🔺 +2.4 KiB (+0.0%)
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit |
|
Reviews (1): Last reviewed commit: "fix(max): guard scene-context reads agai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained frontend-only bugfix guarding kea selector reads against unmounted scene logic, with matching unit tests; not in risky territory (no auth/billing/API/data-model/CI surface), and diff matches the description.
- Author wrote 0% of the modified lines and has 5 merged PRs in these paths (familiarity MODERATE).
- greptile-apps[bot] reviewed the current head.
- Greptile flagged an edge case (P2, unresolved) where a null (not just unmounted) dashboard logic now waits up to the 8s cap before sending the first Max message without context, instead of returning immediately — minor UX regression in a rare case, not a crash/data-loss issue, and greptile's own review was COMMENTED (not CHANGES_REQUESTED) with a 👍 reaction on the PR overall. Worth a follow-up but not blocking.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 28L, 4F substantive, 62L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (62L, 7F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 3da362f · reviewed head 3100846 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Contained frontend logic-only fix outside risky territory (no data model, API, auth, billing, or CI changes), with targeted unit tests covering the regression; the one Greptile review comment (null dashboard logic causing full-timeout delay) is already addressed in the diff via the early !activeSceneLogic return-false branch and a matching new test.
- Author wrote 0% of the modified lines and has 5 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 32L, 4F substantive, 91L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (91L, 7F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ e161da7 · reviewed head 470aa07 |
PostHog AI's auto-context collection read the active scene's kea logic without checking it was still mounted. Mid scene-transition (e.g. navigating away from a dashboard) the logic is built but its reducer path is gone from the store, so reading its selectors/values threw a caught "[KEA] Can not find path ... dashboardLogic" error and blanked auto-context. Add a shared activeSceneLogicHasMaxContext type guard that requires isMounted() before reading, and apply it at all three read sites (maxContextLogic, posthogAiContextLogic, maxThreadLogic). The dashboard-wait loop now treats a not-yet-ready scene logic as "still loading" so it keeps waiting for the remount instead of shipping empty context. Generated-By: PostHog Code Task-Id: 0347d6ad-5c42-4113-bdba-6bc8867868e1
Addresses Greptile review feedback on the dashboard-context wait loop. When the dashboard scene is active but its logic is null (its key hasn't resolved or it can't be built), there is nothing to wait on, so the loop should send immediately rather than block for the full wait cap. Only the present-but-not-yet-mounted case (building, or briefly unmounted mid dashboard-to-dashboard navigation) should keep waiting. Split the guard into the two states and add a regression test covering the null-logic case. Generated-By: PostHog Code Task-Id: 0347d6ad-5c42-4113-bdba-6bc8867868e1
470aa07 to
b7efb2f
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Contained frontend logic fix in Max AI's scene-context reads, with unit tests covering the mount-state edge case; the one Greptile inline concern (null dashboard logic stalling the wait) is directly addressed by the new early-return and a matching regression test, and both bot reactions are positive.
- Author wrote 0% of the modified lines and has 5 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 41L, 4F substantive, 97L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (97L, 6F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 7ea5074 · reviewed head b7efb2f |
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
1 participant
Problem
PostHog AI's auto-context collection reads the active scene's kea logic to attach the open dashboard/insight/etc. to the conversation. It read that logic without checking it was still mounted. Mid scene-transition (for example navigating away from a dashboard, or between two dashboards) the logic is built but its reducer path has already been removed from the Redux store, so reading its selectors or
.valuesthrew a caught[KEA] Can not find path ... dashboardLogicerror. That both blanked the auto-collected context and flooded error tracking on dashboard navigation.Raised from a Slack thread in #team-max — see the footer link.
Changes
activeSceneLogicHasMaxContexttype guard inscenes/max/utils.tsthat requiresisMounted()before treating the active scene logic as readable, with a docstring explaining the KEA gotcha and why the check has to happen at read time (mounted state changes without a selector-input change, so it can't be memoized upstream).activeSceneLogic && 'maxContext' in selectors:maxContextLogic(rawSceneContextselector),posthogAiContextLogic(syncSceneAttachmentslistener), andmaxThreadLogic(isDashboardSceneLoading). The third site was reading.values.dashboardon a possibly-unmounted logic with no guard at all — same latent bug.Frontend logic-only change, no UI.
How did you test this code?
Added automated tests; I (the PostHog Slack app agent) did not run the app or do manual testing.
activeSceneLogicHasMaxContextinutils.test.tscovering null/undefined and the mounted × has-maxContext matrix. The regression they catch that nothing else did: if theisMounted()check is dropped, a built-but-unmounted logic would again pass the guard and callers would read a torn-down reducer path, reintroducing the[KEA] Can not find pathflood.activeSceneLogicmocks inmaxContextLogic.test.tsandmaxThreadLogic.test.tsto exposeisMounted: () => true(they represent a mounted dashboard scene) so they stay valid under the new guard.Note
I could not run Jest or the TypeScript check in this environment (no
node_modulesinstall). Please rely on CI for the frontend test + typecheck run.Automatic notifications
Docs update
No docs change needed (internal behavior fix).
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Directed by the requester in a Slack thread who asked to fix the caught KEA scene-context error and then to run
/simplifyand/code-reviewuntil quality reached at least A-. Authored by the PostHog Slack app agent./simplify,/code-review, and/writing-tests(the repo's mandatory test gate) before adding the unit tests.sceneLogic.selectors.activeSceneLogicitself so no consumer ever receives an unmounted logic. I rejected that — that selector is memoized on[activeExportedScene, activeSceneLogicProps], while mount state flips without an input change (the scene component mounts the logic after the selector last recomputed), so baking it in would cache a stalenulland could hide a genuinely-mounted logic from other consumers (breadcrumbs, side panel). Read-time guards are the correct altitude, matching the existinginsightSceneLogicprecedent.maxThreadLogicwait loop bailing early (now keeps waiting), and existing mocks lackingisMounted()(would have thrown under the new guard). The error-reporting paths were deliberately left in place — they still catch a distinct failure (a scene's ownmaxContextselector throwing on still-loading state); only the unmounted-path noise is eliminated, which is the point.Created with PostHog from a Slack thread