Skip to content

fix(access-control): load roles page access controls after team loads - #91346

Draft
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixsettings-stop-the-roles-page-80a35f
Draft

fix(access-control): load roles page access controls after team loads#91346
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixsettings-stop-the-roles-page-80a35f

Conversation

@posthog

@posthog posthog Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Problem

  • An admin who opens Settings → Roles directly gets a red "Load resource access controls failed. Not found." toast, and the access control content never renders.
  • roleAccessControlLogic loaded the project-scoped resource_access_controls endpoint in afterMount, before the team was loaded. On an org-level settings page no project sits in the URL, so currentProjectId fell back to the literal @current, the request hit api/projects/@current/resource_access_controls, 404'd, and kea-loaders raised the failure toast.

Changes

  • The roles page now waits for the team before loading its access controls, so the page renders instead of showing a "Not found" error. A currentTeam subscription fires the loader once a real project id exists.
  • The loader now returns early when no project id is available, so it can never send @current (mechanical guard).
  • Removed the racing loadResourceAccessControls() call from afterMount (mechanical).
  • This mirrors the existing pathCleaningSuggestionsLogic, which already gates project-scoped loads on the team through a currentTeam subscription.

How did you test this code?

  • Ran kea-typegen for the logic, tsgo --noEmit, oxlint/oxfmt, and hogli ci:preflight — all clean.
  • No unit test added: the regression is a mount-ordering race, and the kea test harness always pre-loads a team, so a test would exercise the happy path both before and after the fix and would not catch a revert to afterMount loading.
  • Not able to run the full app to reproduce the toast interactively in this environment.

Automatic notifications

  • Publish to changelog?

Docs update

None — no user-facing docs describe this behavior.

🤖 Agent context

Autonomy: Fully autonomous

  • Authored by Claude (PostHog Desktop agent). Invoked the /writing-tests, /writing-simplified-technical-english, and /writing-pr-descriptions skills.
  • The linked report also flagged a second, unrelated backend bug: warm_insight_cache_task raising ValueError: user does not have organization membership (from products/access_control/backend/property_access_control.py) for insights whose creator left the org. That is a separate subsystem in an isolated product, and the warming task already handles the related TableAccessDeniedError case. It is left out of this branch, which is scoped to the visible roles-page symptom, to keep the change reviewable.

Created with PostHog Desktop from this inbox report.

The roles settings page fired its project-scoped resource_access_controls
request in afterMount, before the team was loaded. currentProjectId then
fell back to "@current", the request 404'd, and a "Load resource access
controls failed. Not found." toast appeared with no access control content.

Gate the load on a real project id: subscribe to currentTeam and fire the
loader once the team is loaded, and guard the loader so it never sends
"@current". Mirrors the pattern in pathCleaningSuggestionsLogic.

Generated-By: PostHog Desktop
Task-Id: c97d4903-0b6b-4852-974f-e3255cc46bbc
@posthog posthog Bot added the skip-agent-review Save $$$, skip auto agent reviews (Greptile) — use for trivial or chore PRs label Aug 30, 2026
@trunk-io

trunk-io Bot commented Aug 30, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@posthog

posthog Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

🦔 PostHog Review is reviewing this pull request

Step 4/6 · Merging overlapping findings · 4/4

Specialist review skills read the changed code in parallel each from their own perspective, a blind-spot sweep catches what they missed, and only validated findings are published back to this pull request.

This comment updates as the review progresses.

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

Bundle size — no change

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.17 MiB · no change

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How 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 import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.37 MiB · 22 files no change ███░░░░░░░ 30.3% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.82 MiB · 3,245 files 🔺 +140 B (+0.0%) █████████░ 90.8% of 9.71 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
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
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
307.0 KiB ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
263.5 KiB ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js
252.5 KiB src/taxonomy/core-filter-definitions-by-group.json
154.2 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
104.7 KiB src/lib/api.ts
95.2 KiB ../packages/quill/packages/quill/dist/index.js
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

Toolbar bundle — eager 2.26 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.26 MiB · 18 files no change ████░░░░░░ 39.5% of 5.72 MiB
Deferred (lazy) 2.11 MiB · 45 files no change n/a — loads on demand
Loader dist/toolbar.js 1.1 KiB no change █░░░░░░░░░ 5.8% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
749.4 KiB dist/toolbar/toolbar-app-MMYGYX5P.css
588.2 KiB dist/toolbar/chunk-chunk-KLLX3OJI.js
484.7 KiB dist/toolbar/chunk-chunk-OX4ECFTJ.js
133.8 KiB dist/toolbar/chunk-chunk-KDK7WOUW.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
71.3 KiB dist/toolbar/toolbar-app-DXVMY4J4.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-KKFNQPQW.js
21.0 KiB dist/toolbar/chunk-chunk-TKJC34GA.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

Dist folder size — 🔺 +2.3 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1435.50 MiB · 🔺 +2.3 KiB (+0.0%)

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contained single-file frontend fix that defers an existing API call until team data loads, fixing a race-condition 404 toast; no auth, data-model, or dependency changes.

Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 29L, 1F substantive — within ceiling
tier T1-agent / T1b-small (29L, 1F, single-area, fix)
stamphog 2.0.0b4 .stamphog/policy.yml @ 5382759 · reviewed head 5382759

@posthog

posthog Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

👋 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.

@trunk-io

trunk-io Bot commented Aug 30, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-agent-review Save $$$, skip auto agent reviews (Greptile) — use for trivial or chore PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants