Repository navigation
feat(tasks): accept side_question commands on task runs - #80023
Conversation
|
😎 Stack merged successfully - details. |
haacked
changed the title
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.35 MiB · 22 files | no change | ███░░░░░░░ 30.0% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.69 MiB · 3,215 files | no change | █████████░ 89.5% 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 |
|---|---|
| 304.7 KiB | ../node_modules/.pnpm/posthog-js@1.418.5/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 |
| 250.7 KiB | ../node_modules/.pnpm/posthog-js@1.418.5/node_modules/posthog-js/dist/module.js |
| 246.9 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.4 KiB | src/lib/api.ts |
| 95.1 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.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 · 17 files | no change | ████░░░░░░ 39.1% of 5.72 MiB |
| Deferred (lazy) | 2.09 MiB · 33 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 |
|---|---|
| 743.3 KiB | dist/toolbar/toolbar-app-EHNBTBYS.css |
| 574.4 KiB | dist/toolbar/chunk-chunk-5UGZG3F7.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-QYFXPLLF.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-Z4DOL7QW.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.2 KiB | dist/toolbar/toolbar-app-4MURQMIW.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-HLGNLH3R.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-JOJIHR7R.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +393 B (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1464.30 MiB · 🔺 +393 B (+0.0%)
✅ ClickHouse migration SQL — none
No ClickHouse migrations in the latest push.
Adds side_question to the task-run command channel: the serializer validates a non-empty question, and the view forwards the command to the sandbox agent server like other sandbox control commands. side_question requires Code access (has_tasks_access): it drives the agent and spends model tokens on the caller's behalf. user_message stays ungated so the Inbox Discuss flow keeps working. Nothing calls this yet. The desktop /btw command that uses it ships separately, so a client on the old build is unaffected and a sandbox predating the command rejects it with an error the caller sees. Generated-By: PostHog Code Task-Id: 9086ddae-4430-49ef-b956-66f90f356b00
733477a to
46b76cc
Compare
🦔 ReviewHog reviewed this pull requestNothing worth raising this time, so here's a calming picture instead:
Resolved comments: 1 left for you |
The gate built its own 403 instead of calling code_access_required_response, so the body omitted the type: permission_denied field every other code-access denial in this viewset carries, and its message said "PostHog Code" where the endpoint's own 403 description says "PostHog Desktop". The proxy test now sends a padded question and asserts the forwarded payload, covering the serializer's strip, and the access test asserts the gate short-circuits before the agent-server call. Claude-Session: https://claude.ai/code/session_01WyZWyiJbQUFSgCfWyM3bCR
Generated-By: PostHog Desktop Task-Id: 78b6d133-c31c-41ba-bf19-64b034eeff4a
The entitlement gate alone let a team over its usage limit, or in a deactivated org, keep spending model tokens through side_question, while user_message returned 429 in the same situation. The endpoint already documented a 429, so the gate now pairs code_access_required_response with usage_limit_response the way the rest of this viewset does. Question length is the direct lever on what a side question costs, so it takes a 10,000 character cap, following MAX_MCP_RESPONSE_PARAMS_BYTES. The three rejection cases move into test_command_rejects_invalid_payloads, where every other method's validation cases live, and gain an oversized-question row alongside them. Claude-Session: https://claude.ai/code/session_01WyZWyiJbQUFSgCfWyM3bCR
…ackend' into posthog-code/btw-side-question-backend
Generated-By: PostHog Desktop Task-Id: e3b1e07a-2d80-441f-be85-011f5e8d59d0
|
Stacked PR 76461 was cancelled: a user cancelled it. |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
![parameterai[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
|
Reviews (1): Last reviewed commit: "Merge branch 'master' into posthog-code/..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Adds support for a new side_question JSON-RPC method on the Tasks “run command” endpoint, enabling one-shot side questions to be forwarded to the sandbox agent server (to support the desktop /btw command stack layer).
Changes:
- Extend the task-run command surface to accept and proxy
side_question, including access + usage-limit gating in the view. - Add serializer validation/normalization for
params.question(required, trimmed, and length-capped). - Regenerate OpenAPI-derived frontend/MCP generated types and add backend API tests covering proxying and gating/validation.
Reviewed changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| services/mcp/src/api/generated.ts | Regenerated MCP API schema enum to include side_question. |
| products/tasks/frontend/generated/api.zod.ts | Regenerated Zod schema docs + enum values to include side_question. |
| products/tasks/frontend/generated/api.ts | Regenerated frontend API client docs to mention side_question. |
| products/tasks/frontend/generated/api.schemas.ts | Regenerated frontend schema types/enums to include side_question. |
| products/tasks/backend/tests/test_api.py | Adds coverage for side_question proxying, access gating, usage limit gating, and invalid payload rejection. |
| products/tasks/backend/presentation/views/api.py | Allows side_question in the command endpoint and applies access/usage gates for it. |
| products/tasks/backend/presentation/serializers.py | Adds side_question to allowed methods and validates/normalizes params.question. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Generated-By: PostHog Desktop Task-Id: 3c69dee1-47cc-4672-8874-f087e999d254
charlesvien
added
the
stamphog
PostHog
|
The diff adds a new agent-driving, token-spending command gated only by a non-atomic usage check, and an unresolved inline review comment from an automated reviewer flags a concrete, unaddressed concurrency/rate-limiting gap that lets a caller burn model spend and hold workers for up to 10 minutes each in bursts — a substantive resource-exhaustion concern in security-sensitive surface, not fixed by the current diff.
Gate mechanics and policy version
|
stamphog
Bot
added
reviewhog
and removed
stamphog
|
I reviewed the current head. The existing concurrency/rate-limiting security finding remains unresolved; I found no additional security issues to report. |
charlesvien
added
the
stamphog
PostHog
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a new sandbox command gated by the same has_tasks_access/usage-limit pattern already used for every other command in this endpoint, with tests covering the proxy path, access denial, usage-limit denial, and invalid payloads; an agent reviewer explicitly reviewed the current head and reported no additional security issues beyond a pre-existing, unrelated finding.
- Author wrote 0% of the modified lines and has 18 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 23L, 2F substantive, 120L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (120L, 7F, two-areas, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ a69fe59 · reviewed head a69fe59 |
|
This PR is the bottom layer of a stack, and it is now the only thing holding the top layer (#76461) from landing. Everything automated on it is already satisfied, so I wanted to pin what is actually left, since it needs a person. State on
It still reports What clears it is a human approving review on this PR. Once that lands, the stack can go through the merge queue bottom-first and #76461 unblocks with it. 🦉 via talyn.dev |
Bottom layer of a stack with #76461. Split out so desktop and backend halves ship separately, as the desktop backend coupling check requires.
Problem
The desktop
/btwcommand (#76461) needs a way to ask a cloud session's sandbox a one-shot side question. The task-run command endpoint rejectsside_questionas an unknown method.Changes
side_questionand forwards it to the sandbox agent server, like other sandbox control commands.questionstring and strips it.side_questionrequires Code access (has_tasks_access); without it the endpoint returns 403 with codecode_access_required. It drives the agent and spends model tokens on the caller's behalf.user_messagestays ungated so the Inbox "Discuss" flow keeps working.hogli build:openapi).Nothing calls this yet. The desktop
/btwcommand that uses it ships in #76461, so a client on the old build is unaffected, and a sandbox predating the command rejects it with an error the caller sees.How did you test this code?
New tests in
test_api.pycover the proxy round-trip, rejection of a blank or missing question, and the 403 without Code access. Each catches a regression no existing test did: the command surface in front of the dispatch was previously untested, which let an unregistered method slip through. Not run locally: the rest of the tasks suite; backend CI covers it.Automatic notifications
Docs update
None
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written with PostHog Code. Skills invoked: /stacking-prs, /improving-drf-endpoints, /writing-pr-descriptions, /debugging-ci-failures. Split out of #76461 after the desktop backend coupling check failed on the combined diff. The split surfaced a bug in the original backend half: the access gate called
code_access_required_response, a function that exists nowhere, and wrappeduser_messagein it too — every cloud message would have raisedNameErroronce deployed (backend CI skipped on the desktop PR, so nothing caught it). Replaced with ahas_tasks_accesscheck onside_questiononly. Generated types were regenerated from serializers rather than copied, which avoided reverting newer master changes to the same files.Created with PostHog Code