feat(signals): split scratchpad write scope for pipeline agents - #91364
Conversation
The report pipeline's research and implementation runs could not write the shared scratchpad without also taking the scout write surface (emit_signal, record_output), so what a run judged died with it. Adds a scratchpad-only internal scope, `signals_research` / `signals_implementation` token presets, and a `pipeline:*` writer identity resolved server-side from the token's bound task so pipeline entries are attributable. Generated-By: PostHog Desktop Task-Id: 4593bc90-98ce-4bed-b689-efa49868b9ef
|
😎 Merged successfully - details. |
Generated-By: PostHog Desktop Task-Id: 4593bc90-98ce-4bed-b689-efa49868b9ef
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.37 MiB · 22 files | no change | ███░░░░░░░ 30.3% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.82 MiB · 3,245 files | no change | █████████░ 90.8% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 307.0 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 263.5 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js |
| 252.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.7 KiB | src/lib/api.ts |
| 95.2 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.26 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.26 MiB · 18 files | no change | ████░░░░░░ 39.5% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 45 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 749.4 KiB | dist/toolbar/toolbar-app-MMYGYX5P.css |
| 588.2 KiB | dist/toolbar/chunk-chunk-KKLTKMYJ.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-K5PYML3Q.js |
| 133.8 KiB | dist/toolbar/chunk-chunk-4O6IE5JE.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-3JUPG5ME.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-5VQMW5YZ.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-YDOZYESM.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 — 🔺 +3.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1435.50 MiB · 🔺 +3.4 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17662.9 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 600.0 KB | 195.2 KB |
| action | 458.2 KB | 195.2 KB |
| action-list | 564.9 KB | 195.2 KB |
| cohort | 457.2 KB | 195.2 KB |
| cohort-list | 563.8 KB | 195.2 KB |
| email-template | 457.0 KB | 195.2 KB |
| error-details | 472.9 KB | 195.2 KB |
| error-issue | 457.9 KB | 195.2 KB |
| error-issue-list | 564.7 KB | 195.2 KB |
| experiment | 562.0 KB | 195.2 KB |
| experiment-list | 565.6 KB | 195.2 KB |
| experiment-results | 567.1 KB | 195.2 KB |
| feature-flag | 567.6 KB | 195.2 KB |
| feature-flag-list | 571.4 KB | 195.2 KB |
| feature-flag-testing | 461.4 KB | 195.2 KB |
| inline-scan | 457.7 KB | 195.2 KB |
| insight-actors | 563.0 KB | 195.2 KB |
| invite-email-preview | 456.4 KB | 195.2 KB |
| llm-costs | 560.0 KB | 195.2 KB |
| session-recording | 459.0 KB | 195.2 KB |
| survey | 458.8 KB | 195.2 KB |
| survey-global-stats | 562.7 KB | 195.2 KB |
| survey-list | 565.5 KB | 195.2 KB |
| survey-stats | 562.7 KB | 195.2 KB |
| trace-span | 457.6 KB | 195.2 KB |
| trace-span-list | 564.7 KB | 195.2 KB |
| vision-observation-list | 563.9 KB | 195.2 KB |
| workflow | 457.5 KB | 195.2 KB |
| workflow-list | 564.2 KB | 195.2 KB |
| loops-review | 461.9 KB | 195.2 KB |
| query-results | 755.2 KB | 195.2 KB |
| render-ui | 838.3 KB | 195.2 KB |
| visual-review-snapshots | 462.0 KB | 195.2 KB |
⚠️ Playwright — 1 failed
🎭 Playwright report · View test results →
❌ 1 failed test:
- Hover chart to see tooltip with data point values (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ 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/0108_signalscratchpad_created_by_identity.py
BEGIN;
--
-- Add field created_by_identity to signalscratchpad
--
ALTER TABLE "signals_signalscratchpad" ADD COLUMN "created_by_identity" varchar(64) NULL;
COMMIT;Last updated: 2026-08-30 11:42 UTC (6a30d10)
✅ 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.0108_signalscratchpad_created_by_identity
└─ #1 ✅ AddField
Adding nullable field requires brief lock
model: signalscratchpad, field: created_by_identity
📚 How to Deploy These Changes Safely
AddField:
This operation acquires a brief lock but doesn't rewrite the table.
Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up.
Last updated: 2026-08-30 11:42 UTC (6a30d10)
andrewm4894
left a comment
There was a problem hiding this comment.
Reviewed against #91357's acceptance criteria and traced the two things the description flags. Nothing blocking.
Acceptance criteria
- Resolved-scope shape:
signals_research=read_onlyresolution +signal_scratchpad_internal:write−task:write;signals_implementation=full+ the scratchpad scope; neither carriessignal_scout_internal:writeorsignal_scout_report:write; bothsignals_scout*presets keep the scratchpad scope. Tests pin all of it. - Toolset:
TestSignalsResearchToolsetreads the generated catalog and asserts the only reachable write tools are the scratchpad pair (plus their deprecated aliases). That is a better test than the one the issue asked for, because it can't drift from what the MCP server ships. - Identity: derived server-side from the token's bound task (
sandbox_task_id, whichcreate_oauth_access_tokensets totask.id) viaai_stage, which the pipeline stamps once at run creation. Stored on create only, so an upsert keeps the original author. Cross-team lookup returnsNone. Good call not to accept it as a tool argument. - Scouts unchanged:
SCOUT_INTERNAL_SCOPEScarries the new scope, soremember/forgetkeep working the moment the tools re-scope.
On withholding task:write from signals_research
Agree with the reasoning, and I checked the part the description could not: nothing the research sandbox does outside MCP needs it. Its result comes back through the session and the pipeline persists the artefacts; charts are stored as JSON on the report, not through staged_artifacts; presence / staged_artifacts / send_followup_to_sandbox are interactive-run paths a research run never takes. Nothing in the tasks or signals backends branches on the literal "read_only" either, so the rename is inert there.
Two things to know, neither for this PR
- The user-pressed Create PR path (
facade/api.py,run_source→full/read_only) is untouched, so a person-started implementation run gets no scratchpad write. The issue scoped this PR to autostart, which is right; I have noted it on #91359 so the implementation protocol either widens that path or states the limitation. pipeline:implementationis a writer identity but not a note audience. Fine for now: the implementation run gets its steering rendered into the task description. If it ever needs to be addressable,PIPELINE_AUDIENCESis one line.
Migration 0108 is additive and master is at 0107. CI is green so far with the backend matrix still running.
A bare tuple of preset strings widens to `str`, which mypy rejects against the `McpScopePreset` literal on `resolve_scopes`. Generated-By: PostHog Desktop Task-Id: 4593bc90-98ce-4bed-b689-efa49868b9ef
|
✅ Visual changes approved by @andrewm4894. All visual changes in this run were quarantined or tolerated. |
`hasScope` short-circuits on `*` for every scope object not on SERVER_MINT_ONLY_SCOPE_OBJECTS. The scratchpad write scope was split out of `signal_scout_internal`, which is on that list, so moving the tools without moving the object let a user-consented `*` token pass the MCP scope filter for `scout-scratchpad-remember` / `scout-scratchpad-forget`. The Django permission layer still rejects those calls, so this restores the outer of the two gates. Also adds the scope pair to the OAUTH_SCOPES_SUPPORTED completeness test's server-mint-only set, which is the check that caught this. Generated-By: PostHog Desktop Task-Id: 4593bc90-98ce-4bed-b689-efa49868b9ef
The access-control logics inline an expanded `APIScopeObject` union that kea-typegen writes back into the source, so a new scope object leaves them stale. Output of `typegen:write` plus oxfmt; the only change is the added union member. Generated-By: PostHog Desktop Task-Id: 4593bc90-98ce-4bed-b689-efa49868b9ef
|
/trunk merge |
1 similar comment
|
/trunk merge |
The scope split in #91364 narrowed the scratchpad write path to `signal_scratchpad_internal:write` and stopped minting the legacy `signal_scout_internal:write` for it. Sandbox tokens live 6 hours, so scout runs already in flight at deploy time carried only the legacy scope and their `remember` / `forget` calls returned 403, silently dropping durable memory. Resolve the required scratchpad-write scope per request: enforce the new scope, but accept the legacy one when the token carries only that. Both objects are internal, so neither is reachable via a user key or `*` consent. Drop the legacy branch a token lifetime after the split. Generated-By: PostHog Desktop Task-Id: dd7b40fe-19a0-4053-8f3e-3f7f4ac4e65f
Problem
A team gets the same finding re-derived from scratch on every self-driving run. The report pipeline's research and implementation agents are told to read the shared scratchpad for the entities they touch, but neither can write to it, so a judgment or a learning dies with the run.
Both write tools sit behind
signal_scout_internal:write, which also unlocksscout-emit-signalandscout-record-output. Granting memory today means granting the scout's whole write surface to two agents that are not scouts.Closes #91357
Changes
scout-scratchpad-searchresults name the pipeline stage that wrote an entry (pipeline:report-research,pipeline:implementation), so a reader can tell pipeline memory from a scout's.signal_scratchpad_internal:writeis a new internal scope carryingscout-scratchpad-remember/scout-scratchpad-forgetalone. Scouts keep both tools:SCOUT_INTERNAL_SCOPEScarries the new scope too.read_onlytosignals_research, autostart implementation fromfulltosignals_implementation. No otherfullcaller changes.signals_researchwithholdstask:write, which theread_onlyresolution grants. This is the one place worth a reviewer's attention, and the reason is below.created_by_identitycolumn, regenerated MCP tool definitions and OpenAPI types, and the frontend scope-object mirror.Why
signals_researchdropstask:writeRead-only mode is a tool-annotation filter, not a scope filter. The posture has to turn it off or the MCP server strips the two scratchpad tools it exists to grant.
task:writereaches every posture throughINTERNAL_SCOPESand was inert only because that header hid it.Leaving it in place would hand the research stage 13 more write tools, including
inbox-reports-set-stateandinbox-reports-update. The stage reads data and returns findings; the pipeline persists them after the session. So the scope comes back out, andRESEARCH_WITHHELD_SCOPESrecords why.Note
This changes what two server-minted tokens can reach. It grants no new capability to any user-grantable credential: the new scope object is in
INTERNAL_API_SCOPE_OBJECTS, so the consent flow, the personal-API-key UI, and RBAC all reject it, and*does not satisfy it.How did you test this code?
Automated tests only. No manual run of the pipeline: the sandbox has no agent server, so neither posture was exercised end to end against a real MCP session.
New coverage, and the regression each group catches:
posthog/temporal/tests/test_oauth.py): a preset silently regainingsignal_scout_internal:writeorsignal_scout_report:write, and the split costing scouts their scratchpad scope.signal_scratchpad_internal:write, or a scope creeping back into the posture, widening the write surface read-only mode used to hide. It reads the generated tool catalog the MCP server ships rather than a copy of it.test_pipeline_identity.py): a scout task, a foreign origin naming a pipeline stage, or another team's task resolving to a pipeline identity.test_scout_harness_tools.py): the upsert overwriting the original author, which would attribute an entry to whoever rewrote it last.test_scout_harness_api.py): the viewset no longer deriving the identity, which no unit test would catch.Ran locally: the signals scout harness suites, the scope resolution suite,
posthog/test/test_permissions.py, andfrontend/src/lib/scopes.test.ts.makemigrations --checkreports no drift.Not checked locally: the full backend matrix and the frontend typecheck, both left to CI.
Automatic notifications
Docs update
products/signals/backend/scout_harness/AGENTS.mdcovers the scope split, the withheldtask:write, and how writer identity resolves.🤖 Agent context
Autonomy: Fully autonomous
Written by Claude Code (Opus 4.5) from the linked issue, which carried the implementation sketch. Skills invoked:
/django-migrations,/implementing-mcp-tools,/writing-tests,/writing-pr-descriptions.Two decisions departed from the sketch. It specified
signals_researchas theread_onlyresolution plus the scratchpad scope, but that conflicted with its own acceptance criterion of "no other non-read-only tool" once the read-only header comes off; counting the reachable tools against the generated catalog settled it in favour of withholdingtask:write. The sketch also suggested addingpipeline:implementationtoPIPELINE_AUDIENCES; it is kept out, because that set is the allowlist for note targets and the implementation stage reads no notes. A separatePIPELINE_WRITER_IDENTITIESholds the writer role instead.The
ai_stagestrings the two stages stamp are now shared constants, since the identity lookup reads what those call sites write and a rename would otherwise leave every pipeline entry silently unattributed.Created with PostHog Desktop from an inbox report