fix(scopes): generate the frontend scope object type from the backend list - #106173
Conversation
|
😎 This pull request was merged. |
🤖 CI report
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
saveGroupedRules |
frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.ts:927 |
26 | 10 |
✅ Duplication (Python) — clean
New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
🚨 Comment density — 27% of added code lines are comments (26 of 95)
This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.
Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.
Files with the most added comment lines:
| File | Comment lines | Added lines |
|---|---|---|
posthog/scopes.py |
11 | 15 |
frontend/src/lib/scopes.tsx |
5 | 6 |
products/access_control/backend/facade/enums.py |
4 | 6 |
frontend/src/lib/scopes.test.ts |
2 | 5 |
frontend/src/types.ts |
2 | 4 |
posthog/settings/web.py |
2 | 3 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +10.9 KiB (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 69.12 MiB · 🔺 +10.9 KiB (+0.0%)
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/signals/frontend/inbox/InboxScene.js |
457.2 KiB | 🔺 +5.2 KiB (+1.1%) |
render-query/src/render-query/render-query.js |
20.19 MiB | 🔺 +4.1 KiB (+0.0%) |
posthog-app/src/scenes/AuthenticatedShell.js |
270.0 KiB | 🔺 +1.8 KiB (+0.7%) |
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.60 MiB · 22 files | 🔺 +964 B (+0.1%) | █████████░ 86.8% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.54 MiB · 629 files | 🔺 +973 B (+0.0%) | █████████░ 87.9% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.42 MiB · 2,367 files | 🔺 +7.0 KiB (+0.1%) | █████████░ 89.0% of 8.34 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/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 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
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts 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
🟢 [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/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 216.9 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 |
| 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 |
| 100.5 KiB | src/lib/api.ts |
| 89.7 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.1 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.5 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 272.6 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 216.9 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 |
| 153.7 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 |
| 100.5 KiB | src/lib/api.ts |
| 98.8 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 |
| 89.7 KiB | src/products.tsx |
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.17 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.17 MiB · 19 files | 🔺 +60 B (+0.0%) | ████░░░░░░ 37.9% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 809.4 KiB | dist/toolbar/toolbar-app-ZVVWD2DU.css |
| 651.8 KiB | dist/toolbar/chunk-chunk-2NAA2EQ5.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-4EKE7GSM.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-P3QHCZAN.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-ILA3BGY4.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-LPZKSTNF.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-MCEZBJKI.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 — 🟢 -32.8 KiB (-0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 950.66 MiB · 🟢 -32.8 KiB (-0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17633.1 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 199.2 KB |
| action | 454.1 KB | 199.2 KB |
| action-list | 564.2 KB | 199.2 KB |
| cohort | 453.1 KB | 199.2 KB |
| cohort-list | 563.2 KB | 199.2 KB |
| email-template | 452.9 KB | 199.2 KB |
| error-details | 469.6 KB | 199.2 KB |
| error-issue | 454.5 KB | 199.2 KB |
| error-issue-list | 564.8 KB | 199.2 KB |
| experiment | 561.3 KB | 199.2 KB |
| experiment-list | 564.9 KB | 199.2 KB |
| experiment-results | 566.3 KB | 199.2 KB |
| feature-flag | 566.8 KB | 199.2 KB |
| feature-flag-list | 570.5 KB | 199.2 KB |
| feature-flag-testing | 457.3 KB | 199.2 KB |
| inline-scan | 453.6 KB | 199.2 KB |
| insight-actors | 562.3 KB | 199.2 KB |
| invite-email-preview | 452.3 KB | 199.2 KB |
| llm-costs | 559.3 KB | 199.2 KB |
| session-recording | 455.3 KB | 199.2 KB |
| survey | 454.7 KB | 199.2 KB |
| survey-global-stats | 561.9 KB | 199.2 KB |
| survey-list | 564.9 KB | 199.2 KB |
| survey-stats | 561.9 KB | 199.2 KB |
| trace-span | 453.5 KB | 199.2 KB |
| trace-span-list | 564.1 KB | 199.2 KB |
| vision-observation-list | 563.3 KB | 199.2 KB |
| workflow | 453.4 KB | 199.2 KB |
| workflow-list | 563.5 KB | 199.2 KB |
| loops-review | 457.8 KB | 199.2 KB |
| query-results | 774.1 KB | 199.2 KB |
| render-ui | 858.1 KB | 199.2 KB |
| visual-review-snapshots | 457.9 KB | 199.2 KB |
⚠️ Playwright — 1 flaky
🎭 Playwright report · View test results →
- Creating a SQL insight with a variable and overriding it on a dashboard (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ Backend coverage — 90.0% of changed backend lines covered — 1 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ██████████████████░░ 90.0% (10 / 11)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/access_control/backend/presentation/access_control.py |
75.0% | 156 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 36709697225 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
warehouse_sources_queue |
██░░░░░░░░░░░░░░░░░░ 10.5% |
187 / 1,777 |
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
demo |
███████████░░░░░░░░░ 52.8% |
1,411 / 2,673 |
data_tools |
████████████░░░░░░░░ 61.2% |
90 / 147 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
aeo |
███████████████░░░░░ 76.3% |
617 / 809 |
batch_exports |
████████████████░░░░ 81.2% |
21,526 / 26,502 |
apm |
█████████████████░░░ 84.1% |
1,306 / 1,553 |
cdp |
██████████████████░░ 88.2% |
4,545 / 5,155 |
ml_inference |
██████████████████░░ 88.8% |
539 / 607 |
mcp_analytics |
██████████████████░░ 89.2% |
5,038 / 5,651 |
product_tours |
██████████████████░░ 89.3% |
1,340 / 1,500 |
dashboards |
██████████████████░░ 89.5% |
6,904 / 7,714 |
data_warehouse |
██████████████████░░ 90.2% |
14,253 / 15,810 |
notebooks |
██████████████████░░ 90.2% |
15,297 / 16,964 |
signals |
██████████████████░░ 90.3% |
58,230 / 64,497 |
cohorts |
██████████████████░░ 90.4% |
8,420 / 9,316 |
streamlit_apps |
██████████████████░░ 90.8% |
2,684 / 2,956 |
tasks |
██████████████████░░ 91.0% |
76,452 / 84,028 |
managed_warehouse |
██████████████████░░ 91.0% |
10,252 / 11,263 |
data_modeling |
██████████████████░░ 91.2% |
10,559 / 11,575 |
exports |
██████████████████░░ 91.6% |
9,680 / 10,562 |
engineering_analytics |
██████████████████░░ 91.7% |
11,032 / 12,030 |
business_knowledge |
██████████████████░░ 92.1% |
7,783 / 8,449 |
ai_training |
██████████████████░░ 92.2% |
356 / 386 |
conversations |
███████████████████░ 92.5% |
28,749 / 31,077 |
early_access_features |
███████████████████░ 92.6% |
1,339 / 1,446 |
managed_migrations |
███████████████████░ 92.7% |
1,581 / 1,705 |
visual_review |
███████████████████░ 92.8% |
9,247 / 9,969 |
stamphog |
███████████████████░ 92.8% |
8,109 / 8,742 |
canvas |
███████████████████░ 92.8% |
7,034 / 7,579 |
approvals |
███████████████████░ 93.0% |
3,974 / 4,271 |
mcp_registry |
███████████████████░ 93.1% |
1,670 / 1,794 |
notifications |
███████████████████░ 93.2% |
1,145 / 1,229 |
error_tracking |
███████████████████░ 93.2% |
16,360 / 17,547 |
surveys |
███████████████████░ 93.4% |
6,584 / 7,053 |
slack_app |
███████████████████░ 93.4% |
14,141 / 15,133 |
autoresearch |
███████████████████░ 93.6% |
8,481 / 9,061 |
web_analytics |
███████████████████░ 93.8% |
22,496 / 23,974 |
context_layer |
███████████████████░ 93.9% |
3,415 / 3,638 |
alerts |
███████████████████░ 94.1% |
8,628 / 9,172 |
billing_alerts |
███████████████████░ 94.1% |
2,094 / 2,226 |
mcp_store |
███████████████████░ 94.3% |
8,958 / 9,500 |
ai_observability |
███████████████████░ 94.5% |
24,865 / 26,300 |
workflows |
███████████████████░ 94.6% |
15,387 / 16,263 |
wizard |
███████████████████░ 94.7% |
6,150 / 6,496 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
review_hog |
███████████████████░ 95.0% |
11,537 / 12,149 |
endpoints |
███████████████████░ 95.1% |
9,206 / 9,681 |
annotations |
███████████████████░ 95.1% |
817 / 859 |
customer_analytics |
███████████████████░ 95.2% |
25,699 / 27,002 |
legal_documents |
███████████████████░ 95.2% |
2,311 / 2,427 |
posthog_ai |
███████████████████░ 95.3% |
2,491 / 2,614 |
marketing_analytics |
███████████████████░ 95.3% |
19,590 / 20,551 |
experiments |
███████████████████░ 95.4% |
32,685 / 34,247 |
actions |
███████████████████░ 95.5% |
756 / 792 |
logs |
███████████████████░ 95.5% |
15,399 / 16,130 |
data_catalog |
███████████████████░ 95.5% |
4,401 / 4,606 |
tracing |
███████████████████░ 95.6% |
3,536 / 3,699 |
replay_vision |
███████████████████░ 95.6% |
28,579 / 29,883 |
growth |
███████████████████░ 95.7% |
11,255 / 11,762 |
messaging |
███████████████████░ 95.8% |
3,798 / 3,963 |
skills |
███████████████████░ 95.8% |
6,972 / 7,274 |
product_analytics |
███████████████████░ 96.0% |
28,484 / 29,662 |
access_control |
███████████████████░ 96.3% |
7,116 / 7,388 |
revenue_analytics |
███████████████████░ 96.4% |
1,876 / 1,946 |
user_interviews |
███████████████████░ 96.5% |
2,867 / 2,971 |
feature_flags |
███████████████████░ 96.6% |
26,324 / 27,245 |
warehouse_sources |
███████████████████░ 97.3% |
463,024 / 475,894 |
data_quality |
████████████████████ 97.5% |
7,701 / 7,895 |
links |
████████████████████ 97.9% |
234 / 239 |
security |
████████████████████ 98.0% |
1,286 / 1,312 |
metrics |
████████████████████ 98.0% |
4,085 / 4,167 |
analytics_platform |
████████████████████ 98.3% |
2,784 / 2,833 |
pulse |
████████████████████ 98.5% |
2,046 / 2,078 |
live_debugger |
████████████████████ 99.2% |
626 / 631 |
field_notes |
████████████████████ 99.4% |
172 / 173 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
[Medium risk] Refactors scope object types to derive from backend definitions. The PR appears safe to merge based on the reviewed changes. Reviews (3) · Last reviewed commit: "chore(access-control): warn against narr..." |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe backend now defines grantable API scope objects and publishes them as the Priority: ➖ Normal Merge Risk: 🔵 Low · up to This change moves the frontend scope and access-control types onto a list generated from the backend. A missing type import in the access-control logic can break the frontend type check. Two smaller follow-ups also remain: the MCP Server preset still includes a write scope that the picker disables, and some generated type blocks are out of date. These are quick fixes, and the change is close to mergeable once they are addressed. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The shared scope list aligns client types with server validation, and no new privilege expansion was established. The MCP registry scope still does not appear in the key picker, despite the stated goal. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Full details: Description checkExplanation The description covers the problem, changes, testing limits, release status, docs status, and agent decisions. It omits required agent-context evidence, including the session link, CodeRabbit CLI result or skip reason, duplicate-PR search, patch-coverage status, new-events-schema assessment, and public-artifact confirmation. It also provides no frontend screenshots or explicit justification for omitting them. Resolution Add the missing agent-context details required by the template. Include the session link, CodeRabbit CLI disposition, duplicate-PR search result, patch-coverage evidence or justification, new-events-schema assessment, and public-artifact confirmation. Add screenshots for the frontend changes, or explicitly state why screenshots are not applicable because the change has no user-visible effect. ✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Comment |
|
|
👋 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. |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
The frontend kept a hand-written copy of the scope objects in posthog/scopes.py, and no check compared the two. Four objects never reached it: mcp_registry, so the personal API key picker did not offer it, and three internal objects. bin/build-scope-objects.py now writes the object list and the internal and OAuth-hidden sets to frontend/src/lib/scopeObjects.generated.ts as part of hogli build:openapi. The picker test then fails for any backend object that the picker neither offers nor omits, and for any internal or hidden object that is not omitted. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
The APIScopeObject union now carries the four objects the generated list added, so the inline logic types of the access-control logics need a regeneration. Also moves the oxfmt ignore entry next to the other frontend generated file. Generated-By: PostHog Desktop Task-Id: 42cb2015-bf44-4d2a-8db5-6b2be7f5a83a
42dba0a to
4542760
Compare
webjunkie
left a comment
There was a problem hiding this comment.
Note
Agent review, actively prompted for and steered.
Requesting changes. The drift is real, but the fix adds a new generator next to OpenAPI. The generated OpenAPI types can carry this list already.
Put the scope object list in the OpenAPI spec
The frontend has no scope object list because every documented field that holds one is a plain string. Two examples are ResolvedAccess.source_resource (products/access_control/backend/presentation/access_control.py:95) and AccessControlObjectRule.resource (products/access_control/backend/presentation/serializers.py:249). Type them as a ChoiceField over the grantable scope objects. The generated types in products/access_control/frontend/generated/ then carry the list as an *EnumApi const.
This was checked. The fields were patched in memory, and manage.py spectacular generated the schema locally. Result: one enum with 111 values, no internal objects. The three access_control_*_objects endpoints reference it.
AccessControlSerializer.resource does not work for this. Its actions are @extend_schema(exclude=True), so the field never reaches the spec.
With the enum in place:
APIScopeObjectinfrontend/src/types.tsderives from the generated enum.- The partition test in
scopes.test.tsreads its runtime list withObject.values(...), asConfigScopeEnumApiconsumers already do. - The drift guard stays. On a PR, the OpenAPI job regenerates the types and commits them (
.github/workflows/ci-backend.yml:2524). A new object inposthog/scopes.pythen fails the partition test until the picker offers it or omits it with a reason.
A single GRANTABLE_API_SCOPE_OBJECTS in posthog/scopes.py can back the field. validate_resource and validate_personal_api_key_scopes compute that same set inline today.
Remove the parallel pipeline
With the enum above, these become unnecessary: bin/build-scope-objects.py, scopeObjects.generated.ts, the build:openapi-scope-objects hogli step, both workflow path-filter edits, and the .oxfmtrc.json entry. The workflow edits also put this PR in the universal Trunk lane.
The new step carries an openapi name and runs inside build:openapi, but it never reads the spec.
Why the codegen stays lean
From the DevEx side, the goal is one path from a serializer to its frontend and MCP types. Contributors already know hogli build:openapi. CI already checks its output and commits it. Every generator outside that path adds its own hogli step, path filter entries, formatter exclusions, and sometimes its own drift check. Someone has to find and maintain each of them.
These scripts also multiply. This one is the third that loads posthog/scopes.py with runpy and writes TypeScript, and it copies bin/build-mcp-oauth-scopes.py. The task model catalog generator names that same script as its pattern. Each new one copies the nearest existing one.
This direction was set before. On #61945, @webjunkie asked for the widget config codegen to move onto the existing OpenAPI path instead of a separate pipeline, and it did. The same reasoning applies here. When the frontend needs the values an API accepts or returns, type the serializer field and use the generated enum. A separate generator is for data rows that no endpoint serves.
Drop the internal objects from the frontend
The generated list includes the server-minted internal objects. As a result, slack_run, interactive_run and the others now appear in the kea typegen unions of four access control logics. The backend rejects all of them as access control resources. The grantable enum does not contain them. So the Internal: rows in API_SCOPES_OMITTED_FROM_MODAL and the new "omits every internal and OAuth-hidden scope object" test can go. Keep the OAuth-hidden rows. Those objects stay grantable to a personal API key.
Confirm the MCP registry row with its owners
MCPRegistryServerViewSet says the feature flag "stays the boundary that keeps the index internal". The new row shows "MCP registry" in the key picker for every user. The grant has no effect without the flag. Whether the row should be visible is a product decision, not a scope-sync question.
Not blocking: the picker row and its omission reasons can land alone first, while the codegen moves. Greptile's preset finding is left out on purpose. The MCP Server preset already maps every row to :write, and mcp_registry has no write actions.
…end-scope-objects # Conflicts: # frontend/src/types.ts
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
frontend/src/lib/scopes.tsx-164-164 (1)
164-164: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winEmit
mcp_registry:readin the MCP Server preset.Selecting
mcp_serversubmits its preset scopes unchanged. The current derivation emitsmcp_registry:write. The backend accepts and stores this scope, and:writecovers:read, so registry reads still work. However, the registry exposes read actions only. Omitting the entry would remove the intended registry access.Use the permitted action when deriving the preset:
Suggested fix
- ).map(({ key }) => `${key}:write`), + ).flatMap(({ key, disabledActions }) => { + const action = disabledActions?.includes('write') ? 'read' : 'write' + return disabledActions?.includes(action) ? [] : [`${key}:${action}`] + }),
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 6468cdc2-cdb2-4bc8-b7a2-df28f8651f29
⛔ Files ignored due to path filters (1)
frontend/src/lib/scopeObjects.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (10)
.depot/workflows/ci-backend.yml.github/workflows/ci-backend.yml.oxfmtrc.jsonfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessDetailLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/addObjectOverrideModalLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/groupedAccessControlRuleModalLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.tsfrontend/src/lib/scopes.tsxfrontend/src/types.ts
💤 Files with no reviewable changes (1)
- frontend/src/types.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
…API enum The frontend kept its own copy of the scope objects, first by hand and then through a side generator that CI wired up beside the OpenAPI flow. The list now reaches the frontend the way every other backend type does. GRANTABLE_API_SCOPE_OBJECTS in posthog/scopes.py holds every object a person can grant, and the two validators that computed that set inline use it. The access control API types its resource fields as a choice field over it, named ScopeObjectEnum in the schema. APIScopeObject derives from the generated enum, and the picker test reads the same enum, so a new backend object fails the frontend until it gets a picker row or a reason to stay out. Internal objects leave the frontend type with the generator, so their omission rows and the test for them go. mcp_registry stays out of the picker until its owners decide whether every user should see the row. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Regenerate the remaining Kea typegen blocks. · groupedAccessControlRuleModalLogic.ts:69-192
frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/groupedAccessControlRuleModalLogic.ts:69-192
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRegenerate the remaining Kea typegen blocks.
The PR updates several resource parameters to
ScopeObjectEnumApi, but these generated declarations still use stale inline unions. They include internal objects that are not inScopeObjectEnumApiand omitoffline_evaluation_ingestionandwizard_run, which the generated enum contains.Regenerate the
saveGroupedRules.resourceLevels,resourceInheritedReasonTooltip, andsetObjectRulepayload types with Kea typegen.Source: Coding guidelines
🧹 Nitpick comments (1)
posthog/scopes.py (1)
228-234: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
GRANTABLE_API_SCOPE_OBJECTSinget_scope_descriptions.
get_scope_descriptions()(lines 296-302) still has its own copy ofobj in API_SCOPE_OBJECTS if obj not in INTERNAL_API_SCOPE_OBJECTS. This PR addsGRANTABLE_API_SCOPE_OBJECTSto own that rule, andALL_SCOPESalready uses it. Update the remaining copy so the grantable set has one definition.return { f"{obj}:{action}": f"{action.capitalize()} access to {obj}" - for obj in API_SCOPE_OBJECTS - if obj not in INTERNAL_API_SCOPE_OBJECTS + for obj in GRANTABLE_API_SCOPE_OBJECTS for action in API_SCOPE_ACTIONS }As per path instructions: "says everything once and only once".
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 8c64b92b-1e6e-48e3-b550-4a882253266f
⛔ Files ignored due to path filters (1)
products/access_control/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (14)
frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessDetailLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/addObjectOverrideModalLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/groupedAccessControlRuleModalLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.tsfrontend/src/lib/scopes.test.tsfrontend/src/lib/scopes.tsxfrontend/src/types.tsposthog/api/personal_api_key.pyposthog/scopes.pyposthog/settings/web.pyproducts/access_control/backend/facade/enums.pyproducts/access_control/backend/presentation/access_control.pyproducts/access_control/backend/presentation/serializers.py
💤 Files with no reviewable changes (3)
- frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessDetailLogic.ts
- frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts
- frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
…end-scope-objects
The inlined scope unions still listed internal objects the enum no longer has, so the typecheck failed. The blocks now name ScopeObjectEnumApi instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: e4a4a92a-9c44-4c31-ad85-4f5c66f157f2
⛔ Files ignored due to path filters (1)
products/access_control/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessDetailLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/addObjectOverrideModalLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/groupedAccessControlRuleModalLogic.tsfrontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.tsfrontend/src/types.tsservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
webjunkie
left a comment
There was a problem hiding this comment.
Note
Agent review, actively prompted for and steered. Findings marked unverified were not reproduced.
Approving. The scope objects now reach the frontend through the OpenAPI flow, and the side generator is gone.
Verified
hogli build:projections --checkfrom #107222 passes with this branch'sposthog/scopes.py. The MCP OAuth scopes output stays byte-identical, so the two PRs can land in either order.- A test merge with #107222 is clean.
Check the response type of resource (non-blocking, unverified)
AccessControlObjectRuleSerializer.resource is now a ChoiceField over GRANTABLE_API_SCOPE_OBJECTS. DRF does not validate values on output. If a stored rule can name a resource outside that list, the generated type is narrower than the data the endpoint returns. Not checked against stored rules.
The frontend logic diffs were not read line by line.
…end-scope-objects # Conflicts: # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessDetailLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/addObjectOverrideModalLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/groupedAccessControlRuleModalLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts # frontend/src/lib/scopes.tsx # frontend/src/types.ts # posthog/scopes.py
The frontend scope object type now comes from ScopeObjectEnumApi, which lists grantable objects only. Drop the seven internal objects from the "Internal tools" group so it type-checks, and point the group and omission comments at the enum instead of the removed array. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
accessControlLogic named ScopeObjectEnumApi in its kea-typegen block without importing it, which failed the frontend typecheck. Import it, and use the products/ path in the other access-control logics instead of the long relative path. Reword the comments on how the scope object type reaches the frontend and what a new scope object still needs there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
|
@webjunkie re the response type of |
|
Stacked PR 106182 failed testing in the merge queue. Please investigate the failure and re-submit the stack. |
…end-scope-objects # Conflicts: # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessControlsLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/accessDetailLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/addObjectOverrideModalLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/ResourceAccessControlsV2/groupedAccessControlRuleModalLogic.ts # frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts # frontend/src/lib/scopes.tsx # frontend/src/types.ts
…ution The merge took this branch's whole copy of types.ts and the five access-control logics, which dropped unrelated additions from master. Rebuild them from master with only this branch's change applied: the generated scope enum replaces the hand-written array and the inline unions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FunWqqcMYpKX4EaFAzwbD
|
This pull request was merged into |
Problem
posthog/scopes.py, and nothing compared the two.mcp_registryandwizard_runwere missed this way.Changes
APIScopeObjecton the frontend is now generated fromposthog/scopes.py, so a new scope object reaches the frontend throughhogli build:openapi. The hand-written array is gone.GRANTABLE_API_SCOPE_OBJECTSinposthog/scopes.py: every object minus the internal ones. The key validators,ALL_SCOPESand the enum all read it.resourcefields of the access control serializers take it as their choices, andENUM_NAME_OVERRIDESpins the enum name toScopeObjectEnum.APIScopeObjectis an alias of the generatedScopeObjectEnumApi, so no call site changed.frontend/src/lib/scopes.tsx. Those are UI decisions.scopes.test.tsfails until a new object has a row or an omission reason, and exactly one group.Note
Access control resources use scope object names by design, which is why their serializer fields carry the scope type. A comment on the choices, and the
/adding-api-scopesskill in #106182, say not to narrow those fields: the scope pickers would lose objects.No user-visible change.
flowchart LR subgraph Before A[posthog/scopes.py]:::phGray B[types.ts hand-written array]:::phRed A -. copied by hand .-> B end classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;flowchart LR subgraph After A[posthog/scopes.py]:::phGray --> B[access control resource fields]:::phBlue B -- hogli build:openapi --> C[ScopeObjectEnumApi]:::phGray C --> D[APIScopeObject alias]:::phYellow end classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000;How did you test this code?
scopes.test.tsreads the generated enum, so a Python object with no picker row or omission reason fails the coverage test, and one with no group fails the group test.Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5 and Claude Fable 5.1
/improving-drf-endpoints,/adopting-generated-api-types,/writing-pr-descriptions.