Repository navigation
feat(signals): lock a scout's pause, resume, and delete to its owner - #106109
trunk-io[bot] merged 2 commits into
Conversation
|
😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details. |
PostHog Review · Deep · reviewed
|
| Step | Status | Result |
|---|---|---|
| Prepare the diff | done | 25 files, 3 chunks |
| Pick perspectives | done | 3 perspectives |
| Run review passes | done | 11 issues (+2 blind-spot) |
| Merge overlapping findings | done | 13 → 8 |
| Validate findings | done | 6 kept, 2 dismissed |
| Publish | done | 6 posted (view review) |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 23 minutes. View limit detailsLimit details: You’ve used all 12 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (16)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughScout configurations gain a persisted Priority: ➖ Normal Severity of issue fixed: Medium 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Comment |
🤖 CI report
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
ScoutConfigForm |
products/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.tsx:100 |
27 | 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 — 9% of added code lines are comments (50 of 549)
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 |
|---|---|---|
products/signals/backend/test/test_scout_harness_api.py |
22 | 165 |
products/signals/backend/models.py |
8 | 9 |
products/signals/backend/scout_harness/views.py |
7 | 102 |
products/skills/backend/api/archive_guards.py |
3 | 35 |
products/signals/backend/facade/api.py |
2 | 3 |
products/signals/backend/scout_harness/lifecycle_lock.py |
2 | 131 |
products/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.test.tsx |
2 | 8 |
products/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.tsx |
2 | 19 |
This check does not block merging. It updates on every push and clears when the share drops.
✅ Bundle size — no change
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 75.58 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.67 MiB · 23 files | no change | █████████░ 90.7% 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.83 MiB · 675 files | no change | █████████░ 95.0% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.79 MiB · 2,463 files | no change | █████████░ 93.4% of 8.34 MiB |
dashboard scenesrc/scenes/dashboard/Dashboard.tsx |
9.95 MiB · 3,529 files | no change | █████████░ 91.1% of 10.92 MiB |
today home pathsrc/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx |
7.80 MiB · 2,473 files | no change | █████████░ 90.9% of 8.58 MiB |
events scenesrc/scenes/activity/explore/EventsScene.tsx |
9.55 MiB · 3,373 files | no change | █████████░ 90.9% of 10.51 MiB |
replay detail scenesrc/scenes/session-recordings/detail/SessionRecordingDetail.tsx |
12.36 MiB · 4,204 files | no change | ████████░░ 78.6% of 15.72 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ 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
🟢 products/dashboards/frontend/widgets/previews/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 zod/v4/locales/de.js stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/queries/Query/Query.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/activity/explore/EventsScene.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/activity/explore/EventsScene.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 |
| 839 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 317.7 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 220.8 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 |
| 101.6 KiB | src/products.tsx |
| 99.6 KiB | src/lib/api.ts |
| 69.0 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.7 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 |
| 29.0 KiB | ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 317.7 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.8 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.8 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 |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 101.6 KiB | src/products.tsx |
| 99.6 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 |
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
| Size | File |
|---|---|
| 317.7 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.8 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.8 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 |
| 181.9 KiB | src/queries/validators.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 |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 101.6 KiB | src/products.tsx |
| 99.6 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
| Size | File |
|---|---|
| 317.7 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.8 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.8 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 |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 101.6 KiB | src/products.tsx |
| 99.6 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 |
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
| Size | File |
|---|---|
| 317.7 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.8 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.8 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 |
| 181.9 KiB | src/queries/validators.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 |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 101.6 KiB | src/products.tsx |
| 99.6 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
| Size | File |
|---|---|
| 317.7 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 316.0 KiB | ../node_modules/.pnpm/posthog-js@1.438.4_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 280.8 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.8 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 |
| 181.9 KiB | src/queries/validators.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 |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 101.6 KiB | src/products.tsx |
| 99.6 KiB | src/lib/api.ts |
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.24 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.24 MiB · 19 files | no change | ████░░░░░░ 39.2% of 5.72 MiB |
| Deferred (lazy) | 2.19 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 |
|---|---|
| 866.3 KiB | dist/toolbar/toolbar-app-XPFARN22.css |
| 671.1 KiB | dist/toolbar/chunk-chunk-L74JAGOA.js |
| 259.5 KiB | dist/toolbar/chunk-chunk-K2FSNWIW.js |
| 138.9 KiB | dist/toolbar/chunk-chunk-QRZBNVWO.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-6GJ26EEZ.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-2A27PFLG.js |
| 21.7 KiB | dist/toolbar/chunk-chunk-CQDVTV3N.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 — 🔺 +21.7 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1007.87 MiB · 🔺 +21.7 KiB (+0.0%)
ℹ️ MCP UI apps size — 32 app(s), 17262.8 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 203.4 KB |
| action | 454.2 KB | 203.4 KB |
| action-list | 564.3 KB | 203.4 KB |
| cohort | 453.2 KB | 203.4 KB |
| cohort-list | 563.3 KB | 203.4 KB |
| email-template | 453.0 KB | 203.4 KB |
| error-details | 469.7 KB | 203.4 KB |
| error-issue | 454.6 KB | 203.4 KB |
| error-issue-list | 564.9 KB | 203.4 KB |
| experiment | 561.4 KB | 203.4 KB |
| experiment-list | 565.0 KB | 203.4 KB |
| experiment-results | 566.4 KB | 203.4 KB |
| feature-flag | 566.9 KB | 203.4 KB |
| feature-flag-list | 570.6 KB | 203.4 KB |
| feature-flag-testing | 457.4 KB | 203.4 KB |
| inline-scan | 453.7 KB | 203.4 KB |
| insight-actors | 562.4 KB | 203.4 KB |
| llm-costs | 559.4 KB | 203.4 KB |
| session-recording | 455.4 KB | 203.4 KB |
| survey | 454.8 KB | 203.4 KB |
| survey-global-stats | 562.0 KB | 203.4 KB |
| survey-list | 565.0 KB | 203.4 KB |
| survey-stats | 562.0 KB | 203.4 KB |
| trace-span | 453.6 KB | 203.4 KB |
| trace-span-list | 564.2 KB | 203.4 KB |
| vision-observation-list | 563.4 KB | 203.4 KB |
| workflow | 453.5 KB | 203.4 KB |
| workflow-list | 563.6 KB | 203.4 KB |
| loops-review | 457.9 KB | 203.4 KB |
| query-results | 813.9 KB | 203.4 KB |
| render-ui | 897.5 KB | 203.4 KB |
| visual-review-snapshots | 458.0 KB | 203.4 KB |
ℹ️ MCP agent API — agent-facing tool changes
What this PR changes for agents, from the tool schema snapshots and tool definitions.
Tools changed (6):
| Tool | Params | Scopes | Annotations | Schema chars |
|---|---|---|---|---|
scout-config-create |
+lifecycle_locked |
10,634 -> 11,316 | ||
scout-config-update |
+lifecycle_locked |
10,795 -> 11,477 | ||
scout-create |
11,722 -> 12,404 | |||
signals-scout-config-create |
+lifecycle_locked |
10,634 -> 11,316 | ||
signals-scout-config-update |
+lifecycle_locked |
10,795 -> 11,477 | ||
vision-scanners-scouts-create |
11,439 -> 12,121 |
⚠️ Playwright — 1 flaky
🎭 Playwright report · View test results →
- Creating a dashboard from a template populates tiles (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 — 97.0% of changed backend lines covered — 6 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ███████████████████░ 97.0% (220 / 226)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/signals/backend/facade/api.py |
0.0% | 1231 |
products/skills/backend/api/skills.py |
60.0% | 1657, 1659 |
products/skills/backend/tools/skills.py |
75.0% | 513 |
products/signals/backend/scout_harness/lifecycle_lock.py |
95.8% | 58, 60 |
🤖 Agents: add a test only if an uncovered line exposes a realistic regression that existing tests miss. Otherwise explain why no new test is needed under "How did you test this code?". Gap list: the patch-coverage artifact on this run (gh run download 121082578414478 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
demo |
███████████░░░░░░░░░ 55.9% |
1,508 / 2,700 |
data_tools |
████████████░░░░░░░░ 61.2% |
90 / 147 |
warehouse_sources_queue |
█████████████░░░░░░░ 66.8% |
1,921 / 2,875 |
aeo |
███████████████░░░░░ 76.3% |
617 / 809 |
batch_exports |
████████████████░░░░ 80.1% |
21,382 / 26,689 |
apm |
█████████████████░░░ 84.1% |
1,306 / 1,553 |
engineering_analytics |
█████████████████░░░ 84.9% |
11,355 / 13,374 |
mcp_analytics |
█████████████████░░░ 85.7% |
4,844 / 5,651 |
ml_inference |
█████████████████░░░ 86.3% |
524 / 607 |
warehouse_suggestions |
█████████████████░░░ 87.3% |
2,252 / 2,580 |
posthog_ai |
██████████████████░░ 87.7% |
4,226 / 4,817 |
notebooks |
██████████████████░░ 88.4% |
15,201 / 17,196 |
today |
██████████████████░░ 88.6% |
2,866 / 3,233 |
cdp |
██████████████████░░ 88.7% |
5,081 / 5,729 |
mcp_registry |
██████████████████░░ 89.0% |
1,596 / 1,794 |
product_tours |
██████████████████░░ 89.1% |
1,337 / 1,500 |
webmcp |
██████████████████░░ 89.6% |
240 / 268 |
cohorts |
██████████████████░░ 89.6% |
8,458 / 9,438 |
dashboards |
██████████████████░░ 89.8% |
7,107 / 7,911 |
signals |
██████████████████░░ 89.9% |
68,257 / 75,933 |
data_modeling |
██████████████████░░ 90.1% |
10,856 / 12,043 |
ai_training |
██████████████████░░ 90.4% |
349 / 386 |
tasks |
██████████████████░░ 90.6% |
80,611 / 89,007 |
visual_review |
██████████████████░░ 90.7% |
9,842 / 10,856 |
data_warehouse |
██████████████████░░ 90.8% |
14,831 / 16,339 |
canvas |
██████████████████░░ 90.9% |
7,748 / 8,522 |
exports |
██████████████████░░ 91.0% |
9,698 / 10,663 |
business_knowledge |
██████████████████░░ 91.0% |
8,791 / 9,656 |
autoresearch |
██████████████████░░ 91.1% |
10,139 / 11,125 |
managed_warehouse |
██████████████████░░ 91.2% |
11,053 / 12,114 |
wizard |
██████████████████░░ 91.8% |
6,008 / 6,548 |
streamlit_apps |
██████████████████░░ 91.8% |
3,087 / 3,362 |
stamphog |
██████████████████░░ 92.0% |
8,243 / 8,964 |
conversations |
██████████████████░░ 92.0% |
29,714 / 32,284 |
managed_migrations |
██████████████████░░ 92.5% |
1,600 / 1,730 |
early_access_features |
███████████████████░ 92.6% |
1,339 / 1,446 |
alerts |
███████████████████░ 92.9% |
7,246 / 7,797 |
review_hog |
███████████████████░ 93.0% |
15,717 / 16,904 |
surveys |
███████████████████░ 93.1% |
6,626 / 7,118 |
web_analytics |
███████████████████░ 93.1% |
24,096 / 25,877 |
notifications |
███████████████████░ 93.2% |
1,152 / 1,236 |
approvals |
███████████████████░ 93.4% |
4,526 / 4,848 |
cross_project_dashboards |
███████████████████░ 93.4% |
880 / 942 |
slack_app |
███████████████████░ 93.5% |
14,854 / 15,882 |
context_layer |
███████████████████░ 93.7% |
3,409 / 3,640 |
workflows |
███████████████████░ 93.7% |
16,423 / 17,518 |
marketing_analytics |
███████████████████░ 93.8% |
20,074 / 21,402 |
messaging |
███████████████████░ 93.8% |
4,468 / 4,762 |
billing_alerts |
███████████████████░ 93.9% |
2,090 / 2,226 |
customer_analytics |
███████████████████░ 93.9% |
26,079 / 27,773 |
mcp_store |
███████████████████░ 93.9% |
9,008 / 9,593 |
experiments |
███████████████████░ 94.2% |
33,676 / 35,760 |
ai_observability |
███████████████████░ 94.2% |
26,434 / 28,069 |
replay_vision |
███████████████████░ 94.3% |
29,048 / 30,807 |
legal_documents |
███████████████████░ 94.3% |
2,289 / 2,427 |
actions |
███████████████████░ 94.6% |
973 / 1,029 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
endpoints |
███████████████████░ 94.8% |
9,333 / 9,847 |
growth |
███████████████████░ 94.8% |
11,268 / 11,880 |
skills |
███████████████████░ 94.9% |
7,049 / 7,431 |
annotations |
███████████████████░ 95.0% |
816 / 859 |
tracing |
███████████████████░ 95.2% |
3,631 / 3,815 |
data_catalog |
███████████████████░ 95.5% |
4,420 / 4,628 |
access_control |
███████████████████░ 95.5% |
7,779 / 8,143 |
product_analytics |
███████████████████░ 95.7% |
28,676 / 29,952 |
alerts_platform |
███████████████████░ 95.8% |
5,012 / 5,234 |
revenue_analytics |
███████████████████░ 95.9% |
1,878 / 1,959 |
data_quality |
███████████████████░ 95.9% |
8,569 / 8,933 |
feature_flags |
███████████████████░ 96.2% |
26,870 / 27,935 |
warehouse_sources |
███████████████████░ 96.4% |
453,890 / 470,901 |
security |
███████████████████░ 96.7% |
1,452 / 1,501 |
pulse |
███████████████████░ 97.4% |
2,023 / 2,078 |
metrics |
████████████████████ 97.7% |
4,406 / 4,509 |
analytics_platform |
████████████████████ 98.0% |
2,775 / 2,833 |
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.
⚠️ Django migration SQL — 1 new migration to review
We've detected new migrations on this PR. Review the SQL output for each migration:
products/signals/backend/migrations/0170_signalscoutconfig_lifecycle_locked.py
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
return result
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
ed = p(logger, meth_name, ed) # type: ignore[arg-type]
2026-10-10T23:07:32.147815Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=5912 tid=139698282740608
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
return (ssh_host,
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Add field lifecycle_locked to signalscoutconfig
--
ALTER TABLE "signals_signalscoutconfig" ADD COLUMN "lifecycle_locked" boolean DEFAULT false NOT NULL;
COMMIT;Last updated: 2026-10-10 23:08 UTC (e342a78)
✅ Django migration risk — migration analysis complete
We've analyzed your migrations for potential risks.
Summary: 1 Safe | 0 Needs Review | 0 Blocked
✅ Safe
Brief or no lock, backwards compatible
signals.0170_signalscoutconfig_lifecycle_locked
└─ #1 ✅ AddField
Adding NOT NULL field with constant default (safe in PG11+)
model: signalscoutconfig, field: lifecycle_locked
Last updated: 2026-10-10 23:08 UTC (e342a78)
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 206L, 8F substantive, 401L/21F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (401L, 21F, two-areas, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ 6d3fb6f · reviewed head 6d3fb6f |
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 218L, 8F substantive, 413L/21F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (413L, 21F, two-areas, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ b49430f · reviewed head b49430f |
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 218L, 8F substantive, 451L/24F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (451L, 24F, cross-cutting, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ b384af3 · reviewed head b384af3 |
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
| if config.lifecycle_locked: | ||
| assert_can_change_scout_lifecycle( | ||
| request=request, | ||
| team=_canonical_team(self), | ||
| skill_name=config.skill_name, | ||
| config=config, | ||
| action="destroy", | ||
| ) |
There was a problem hiding this comment.
Archiving a skill bypasses the scout deletion lock
Issue description
The new check protects config deletion, but it does not protect skill archival. The Delete scout action archives a custom scout's skill before it calls this endpoint. A non-owner with skill write access can therefore archive a locked scout. Config deletion then returns 403, but the scout has already stopped running.
Why we think it's a valid issue
- Checked: The frontend delete flow,
deleteScoutinscoutFleetLogic.ts:1593-1676, and its confirm dialog,confirmDeleteScoutinScoutConfigControls.tsx:440-458. Also the lock switch copy that this PR adds (ScoutConfigControls.tsx:307-323), the configdestroygate (views.py:3614-3621),archive_skill(skill_services.py:900-921), and the skillarchiveaction (skills.py:1583-1623). - Found: For a custom scout, the Delete button calls
llmSkillsNameArchiveCreatefirst (scoutFleetLogic.ts:1627). Only after that does it callsignalsScoutConfigDestroy(scoutFleetLogic.ts:1640). The archive action checksllm_skill:writeand object access, but it never readsSignalScoutConfig.lifecycle_locked. Onlydestroyrunsassert_can_change_scout_lifecycle. - Found: The flow was checked for a non-owner, non-admin member with skill editor access on a locked custom scout. The archive commits and sets
deleted=Trueon every version (skill_services.py:913-917). The configdestroythen returns 403, so the user gets the error toast (scoutFleetLogic.ts:1666). The skill is already gone, andload_skill_for_runraisesSkillNotFoundError, so the scout never runs again. The skills API has no restore or unarchive endpoint, so the owner cannot undo the archive. - Found: This PR adds switch copy that says only the person the scout runs as, or a project admin, can delete it (
ScoutConfigControls.tsx:310-313). The product's own Delete button breaks that promise. - Impact: A non-owner can delete a locked scout through the first-party UI in one click, and the owner cannot reverse it. The UI reports a failure while the scout is in fact dead, and an orphaned locked config stays in the list. The flow is real and reachable, it defeats the feature the PR adds, and it causes a loss that cannot be undone, so keep it.
must_fixfits.
Suggested fix
Check the locked config before the skill archive can commit. For scout deletion, perform the owner check, skill archive, and config deletion in one server-side transaction. Also protect direct calls to the skill archive endpoint.
Prompt to fix with AI (copy-paste)
## Context
@products/signals/backend/scout_harness/views.py#L3602-3609
<issue_description>
The new check protects config deletion, but it does not protect skill archival. The Delete scout action archives a custom scout's skill before it calls this endpoint. A non-owner with skill write access can therefore archive a locked scout. Config deletion then returns 403, but the scout has already stopped running.
</issue_description>
<issue_validation>
- **Checked:** The frontend delete flow, `deleteScout` in `scoutFleetLogic.ts:1593-1676`, and its confirm dialog, `confirmDeleteScout` in `ScoutConfigControls.tsx:440-458`. Also the lock switch copy that this PR adds (`ScoutConfigControls.tsx:307-323`), the config `destroy` gate (`views.py:3614-3621`), `archive_skill` (`skill_services.py:900-921`), and the skill `archive` action (`skills.py:1583-1623`).
- **Found:** For a custom scout, the Delete button calls `llmSkillsNameArchiveCreate` first (`scoutFleetLogic.ts:1627`). Only after that does it call `signalsScoutConfigDestroy` (`scoutFleetLogic.ts:1640`). The archive action checks `llm_skill:write` and object access, but it never reads `SignalScoutConfig.lifecycle_locked`. Only `destroy` runs `assert_can_change_scout_lifecycle`.
- **Found:** The flow was checked for a non-owner, non-admin member with skill editor access on a locked custom scout. The archive commits and sets `deleted=True` on every version (`skill_services.py:913-917`). The config `destroy` then returns 403, so the user gets the error toast (`scoutFleetLogic.ts:1666`). The skill is already gone, and `load_skill_for_run` raises `SkillNotFoundError`, so the scout never runs again. The skills API has no restore or unarchive endpoint, so the owner cannot undo the archive.
- **Found:** This PR adds switch copy that says only the person the scout runs as, or a project admin, can delete it (`ScoutConfigControls.tsx:310-313`). The product's own Delete button breaks that promise.
- **Impact:** A non-owner can delete a locked scout through the first-party UI in one click, and the owner cannot reverse it. The UI reports a failure while the scout is in fact dead, and an orphaned locked config stays in the list. The flow is real and reachable, it defeats the feature the PR adds, and it causes a loss that cannot be undone, so keep it. `must_fix` fits.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Check the locked config before the skill archive can commit. For scout deletion, perform the owner check, skill archive, and config deletion in one server-side transaction. Also protect direct calls to the skill archive endpoint.
</potential_solution>
There was a problem hiding this comment.
Escalated: the bug is real, but every fix changes who may archive a skill, so a person must choose.
- The Delete button archives the scout's skill before it deletes the config. The skill archive endpoint never checks the lock, so a non-owner can kill a locked scout, and there is no unarchive.
- A direct call to the skill archive endpoint (API or MCP) gets past the lock in the same way.
- Option A: the skill archive endpoint refuses a locked scout for a non-owner. This is a permission check across the skills and signals products.
- Option B: one server-side delete runs the check, the archive, and the config delete in one transaction. This is a new endpoint, and the frontend moves to it.
More detail
- I did not reorder the frontend calls. A failed archive after the config delete lets the coordinator bring the scout back enabled and unlocked.
How this was verified
No code change, so nothing was run. I traced the delete flow in scoutFleetLogic.ts, the config destroy gate in views.py, the skills archive action in skills.py, and archive_skill in skill_services.py at the current head.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
products/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.tsx-319-319 (1)
319-319: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the lock switch editable while the scout is paused.
controlsDisabledReasonreturns 'Enable the scout first' for a paused scout. The lock also protects the resume, so a user must be able to lock a paused scout. For a locked scout that is paused, the owner cannot unlock it from the UI. Use the samedisabledReasonas the emit and network-access controls.Proposed fix
- disabledReason={controlsDisabledReason} + disabledReason={updating ? 'Saving scout settings' : undefined}products/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.tsx-309-313 (1)
309-313: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not call the permitted person "the owner".
The config read shape exposes
ownersfromLLMSkillOwner. The backend gate deliberately does not consultLLMSkillOwner, and it usesresolve_scout_acting_user_id. A person listed inownerswho is not the acting user gets a 403 from a switch labeled "Only the owner". Name the acting user in the label and in thearia-label.Proposed fix
- <span className="text-xs text-default">Only the owner can pause or delete</span> + <span className="text-xs text-default">Lock pausing and deleting</span>- aria-label={`${config.skill_name} only the owner can pause or delete`} + aria-label={`${config.skill_name} lock pausing and deleting`}
🧹 Nitpick comments (1)
products/signals/backend/scout_harness/views.py (1)
2990-2995: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared admin-or-acting-user check.
assert_can_change_scout_lifecyclerepeats the admin-level andresolve_scout_acting_user_idcheck fromassert_can_grant_scout_write_scopes. The docstring says the two gates must resolve the claim "the same way". Two copies can drift apart. Move the check into one helper and call it from both gates.Proposed refactor
def _is_admin_or_acting_user(user: User, team: Team, skill_name: str, config: SignalScoutConfig | None) -> bool: level = UserPermissions(user=user, team=team).current_team.effective_membership_level if level is not None and level >= OrganizationMembership.Level.ADMIN: return True return resolve_scout_acting_user_id(team, skill_name, config) == user.pkuser = cast(User, request.user) - level = UserPermissions(user=user, team=team).current_team.effective_membership_level - if level is not None and level >= OrganizationMembership.Level.ADMIN: - return - if resolve_scout_acting_user_id(team, skill_name, config) == user.pk: + if _is_admin_or_acting_user(user, team, skill_name, config): returnAs 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: b2f2e341-d96d-44f7-99f1-15ad6cc183aa
⛔ Files ignored due to path filters (6)
products/replay_vision/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/replay_vision/frontend/generated/api.zod.tsis excluded by!**/generated/**products/signals/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/signals/frontend/generated/api.zod.tsis excluded by!**/generated/**services/mcp/src/generated/signals/api.tsis excluded by!**/generated/**services/mcp/src/tools/generated/signals.tsis excluded by!**/generated/**
📒 Files selected for processing (18)
posthog/models/activity_logging/activity_log.pyproducts/signals/backend/migrations/0155_signalscoutconfig_lifecycle_locked.pyproducts/signals/backend/migrations/max_migration.txtproducts/signals/backend/models.pyproducts/signals/backend/scout_harness/serializers.pyproducts/signals/backend/scout_harness/views.pyproducts/signals/backend/test/test_scout_harness_api.pyproducts/signals/frontend/inbox/__mocks__/scoutConfigs.tsproducts/signals/frontend/inbox/components/config/scouts/ScoutCadenceLabel.test.tsxproducts/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.test.tsxproducts/signals/frontend/inbox/components/config/scouts/ScoutConfigControls.tsxproducts/signals/frontend/inbox/components/config/scouts/ScoutNextRunLabel.test.tsxproducts/signals/frontend/inbox/components/config/scouts/ScoutTags.stories.tsxproducts/signals/frontend/inbox/components/config/scouts/ScoutWriteAccessSection.test.tsxproducts/signals/frontend/inbox/logics/scoutCreateModalLogic.test.tsproducts/signals/frontend/inbox/logics/scoutFleetLogic.test.tsproducts/signals/frontend/inbox/logics/scoutSuggestionsLogic.test.tsservices/mcp/src/api/generated.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 220L, 8F substantive, 464L/24F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (464L, 24F, cross-cutting, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ c33a973 · reviewed head c33a973 |
|
|
|
✅ Visual changes approved by @andrewm4894 — baseline updated in 12 changed, 2 new. |
|
/trunk merge |
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
The Migration risk check has not finished for this commit, so stamphog cannot tell a safe migration from a risky one yet. The review runs again on the next push, or you can re-request it once the check reports.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: migrations |
| size | ✓ | 248L, 9F substantive, 492L/25F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (492L, 25F, cross-cutting, feat) |
| stamphog 2.1.0 | .stamphog/policy.yml @ 48e1912 · reviewed head 48e1912 |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
b9767d2 to
bb111ba
Compare
|
Risk: No findings Follow-up review of the delta since b9767d2: the branch was rebased, so that diff is mostly master noise; the only substantive delta is regenerated MCP tool-schema snapshots plus small views.py additions (suggestion attribution after the partial_update save, an opt-out capture on destroy, a tool-catalogue action). The lifecycle-lock gates added by this PR are consistently wired and fail closed on every entrypoint I traced (partial_update, config create/upsert, create_scout adoption, destroy, skill archive via the guard registry covering REST, MCP, and system paths), and PUT is unrouted on the config viewset. No new vulnerability is introduced by the delta itself; two leads in the PR's earlier (already-reviewed, unchanged) code could not be anchored on a delta-changed line and are handed off below. Sentinel reviewed |
|
✅ Visual changes approved by @andrewm4894 — baseline updated in 14 changed. Install the Visual Review Chrome extension to see visual review results at the top of your pull requests. |
cf1b33b to
ce1dd5d
Compare
|
/trunk merge |
| def assert_skill_archive_allowed( | ||
| *, | ||
| team: Team, | ||
| skill_name: str, | ||
| user: User | None, | ||
| authenticator: object = None, | ||
| ) -> None: | ||
| """Run every registered guard, raising `SkillArchiveRefused` on the first refusal. | ||
|
|
||
| A caller with no acting user is a system path (seeding, a management command, a test fixture) | ||
| rather than someone asking to remove a skill, so it is not gated. | ||
| """ | ||
| if user is None: | ||
| return | ||
| for guard in _archive_guards: | ||
| guard(team, skill_name, user, authenticator) |
There was a problem hiding this comment.
P1 · Prevent rename from disconnecting a locked scout
The new guard runs only for archive_skill. The config API permits scout skills without a reserved name prefix. rename_skill permits renaming those skills without updating their configs. A skill editor can rename a locked scout's skill, leaving the config pointed at a name the coordinator cannot load. Have rename_skill check whether the old name has a scout config. Reject the rename, or check the lifecycle claim and move the config in the same transaction.
| /** Opt-in guard on this scout's lifecycle. Off by default, so anyone with scout write access may pause, resume, switch the scout to dry run, or delete it. On, only the person the scout's runs act as or a project admin may do any of those, or change this flag. Use it on a scout whose output people depend on: `signal_scout:write` is a project-wide scope held by people and by unattended agents alike, and a resume has to pass the project's enabled-scout maximum that a pause does not, so a bulk pause is not undone in one step. The lock never stops an automatic pause, such as the inactivity sweep or the repeated-failure breaker. */ | ||
| lifecycle_locked?: boolean |
There was a problem hiding this comment.
P2 · Check ownership when creating a locked scout config
This new create field lets a skill editor register another person's existing skill with lifecycle_locked=true. When no config row exists, guard_scout_lifecycle_fields skips the owner check. _upsert_scout_config then stores the lock. A non-owner can set a lock that the API describes as owner-only. Check the scout's acting user before accepting lifecycle_locked=true on first registration. Recheck if get_or_create finds a row created by a concurrent request. Add a test with a skill authored by another member and no config row.
| # Before the select_for_update below, so a refusal costs no row locks. | ||
| assert_skill_archive_allowed(team=team, skill_name=skill_name, user=acting_user) |
There was a problem hiding this comment.
P1 · Guard renames that stop a locked custom scout
The new guard covers archiving, but a skill editor can rename a locked custom scout whose name lacks the signals-scout- prefix. rename_skill changes the skill name but leaves SignalScoutConfig.skill_name unchanged. The coordinator can no longer find the skill, so a non-owner can stop the scout without passing the lifecycle check. Check the lifecycle claim before renaming a skill linked to a locked scout. Pass the acting user into rename_skill and keep the config locked through the rename.
| if config.lifecycle_locked: | ||
| assert_can_change_scout_lifecycle( | ||
| request=request, | ||
| team=_canonical_team(self), | ||
| skill_name=config.skill_name, | ||
| config=config, | ||
| action="destroy", | ||
| ) |
There was a problem hiding this comment.
P2 · Keep the owner's delete claim after skill archive
The UI archives a custom scout's skill before it calls this delete endpoint. resolve_scout_acting_user_id ignores archived skill versions. An auto-registered config can have no created_by or enabled_by. Its author can archive the locked skill, but this check then returns 403 and leaves a config with no runnable skill. Make deletion one server operation. Check the owner's claim before archiving the skill, then archive the skill and delete the config in one transaction.
| # Before the select_for_update below, so a refusal costs no row locks. | ||
| assert_skill_archive_allowed(team=team, skill_name=skill_name, user=acting_user) |
There was a problem hiding this comment.
P3 · Pass the authenticator to the archive refusal counter
assert_skill_archive_allowed accepts an authenticator, but archive_skill does not pass one. Each denied REST archive therefore records auth_kind as other. The counter cannot distinguish a browser session from a scoped token for this action. Pass request.successful_authenticator from the REST action through archive_skill to assert_skill_archive_allowed. Keep None for callers without a request.
| <span className="text-xs text-default">Only the owner can pause or delete</span> | ||
| <span className="text-[11.5px] text-muted"> | ||
| Turn this on for a scout people rely on. Anyone with scout access can still change its other | ||
| settings, but only the person it runs as, or a project admin, can pause it, put it in dry run, | ||
| or delete it. |
There was a problem hiding this comment.
P3 · Describe both directions of the protected changes
The label and description say the lock restricts pausing, entering dry run, and deleting. The API also restricts resuming and leaving dry run. A user may enable the lock expecting a teammate to restore a paused scout or its inbox output, then find that the API refuses the change. Update the visible text and aria-label to name both directions. State that only the person the scout runs as or a project admin can manually pause or resume it, change dry-run mode, or delete it.
Problem
signal_scout:writeis project-wide.enabled,emit, and delete pass on that scope alone, with no per-scout check (views.py).Closes #106000
Origin
Changes
enabled,emit, deletion, and the switch itself need the scout's acting user or a project admin. Every other setting keeps the plain scope bar.resolve_scout_acting_user_id, neverLLMSkillOwner. Any skill editor can rewrite the owner list, so reading it here would let an editor appoint themselves.enabledstays an ordinary edit.signals_scout_lifecycle_writes_refused, labelled by action and auth kind, and logs the same.lifecycle_lockedfield added to frontend test fixtures.Every path that stops a scout is gated, not just
PATCH:partial_updateenabledandemitpassed on the project scope.createcreate_scoutdestroyarchiveArchiving is gated by inversion, because the dependency runs signals → skills and cannot run both ways:
flowchart LR A["skills<br/>archive_skill"] -->|"asks registered guards"| B["skills<br/>archive_guards"] C["signals<br/>AppConfig.ready"] -.->|"registers at startup"| B B -->|"SkillArchiveRefused"| Aarchive_skilltakesacting_useras a required keyword with no default, so a new caller has to say whether a person or a system path is asking. Product-owned retirement (a scanner deleting its own scout) passesNoneand stays ungated.warehouse_sourcesalready uses for the signal-emission gate.Note
The switch defaults off, so no existing project changes behavior until someone turns it on.
How did you test this code?
TestScoutHarnessConfigLifecycleLockAPIintest_scout_harness_api.py, 18 cases. Each group names a regression no existing test caught:destroy, through the create upsert, and through the skill archive, so no single entrypoint is left open;products/skills/backendsuite (1055 cases with the signals scout API, create, and status suites), the MCP unit suite,hogli lint:tach,hogli product:lint --all, and mypy over every changed module.Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Fully autonomous
Agent: Claude Code, claude-opus-5
/django-migrations,/writing-tests,/writing-user-facing-copy,/writing-pr-descriptions,/writing-simplified-technical-english,/simplify.b9767d2is titled as an MCP snapshot refresh but carries the archive guard too. The tooling here signs commits server-side and rejects plaingit commit, so the two could not be split after the fact without force-pushing the whole branch and stranding the open review thread. The PR title and this body describe what actually landed.Product tests (signals (2/2), field-notes)is red, and the twoDjango Tests Passentries only relay it. It is not from this branch:66deebf2,ba7e9c1band74299d1d, after passing at24692a95. CI tests a PR merged with master, so this branch inherits it.products/signals/backend/testsuite gives the same result with this branch, without it, and with current master merged in: 0 failures each time, and the same 34 pre-existing errors, which pass on their own and come from this sandbox rather than the code.Product tests (logs, customer-analytics)andMCP Tests Passare both green now. The MCP one was a real gap — generated sources carriedlifecycle_lockedbut the tool-schema snapshots were never regenerated. The logs one was a flake: it died in 114s when those suites need ~363s locally, left no test output, and passed on the next run untouched.Scout config changedanalytics event does not say which field moved. The frontend already sends asettingproperty per field, so nothing was changed there.Created with PostHog Desktop from this inbox report, addressing #106000.