feat(tasks): route slack follow-ups through the queue with per-message actors - #70762
Conversation
e307e5b to
6558def
Compare
08f53e3 to
1135f96
Compare
121398c to
5ebad7d
Compare
🤖 CI report✅ Bundle size — no changeUncompressed size of every built Total: 64.23 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 budgetHow 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
🟢 Largest files eagerly shipped from
|
| 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 |
|---|---|
| 281.3 KiB | ../node_modules/.pnpm/posthog-js@1.404.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 |
| 235.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 223.4 KiB | ../node_modules/.pnpm/posthog-js@1.404.1/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.3 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 |
| 105.8 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 93.2 KiB | ../packages/quill/packages/quill/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
✅ Dist folder size — 🔺 +1.1 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1342.59 MiB · 🔺 +1.1 KiB (+0.0%)
⚠️ Backend coverage — 25.0% of changed backend lines covered — 75 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): █████░░░░░░░░░░░░░░░ 25.0% (25 / 100)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/tasks/backend/temporal/process_task/tests/test_send_followup_to_sandbox.py |
0.0% | 63–65, 86, 133, 166, 191, 210, 233, 247, 249, 251–252, 272, 292, 313, 340, 363, 437, 441–442, 444, 447–448, 450–451, 453, 457–458, 460, 463–464, 470, 475–476, 478, 480 |
products/tasks/backend/temporal/client.py |
0.0% | 480 |
products/tasks/backend/temporal/process_task/activities/send_followup_to_sandbox.py |
11.1% | 131–132, 134–137, 142–148, 151–152, 164, 174–175, 271, 274, 277, 289, 295–296 |
products/tasks/backend/logic/services/run_actor.py |
20.0% | 104–107 |
products/tasks/backend/temporal/slack_relay/activities.py |
25.0% | 365, 417–418 |
products/tasks/backend/temporal/process_task/utils.py |
57.1% | 1026, 1040, 1046 |
products/tasks/backend/facade/api.py |
66.7% | 939, 943 |
products/tasks/backend/temporal/process_task/workflow.py |
75.0% | 1845 |
🤖 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 29731103050 -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 |
batch_exports |
████████░░░░░░░░░░░░ 39.7% |
8,416 / 21,225 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
█████████████░░░░░░░ 67.0% |
25,951 / 38,740 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 79.4% |
19,499 / 24,561 |
data_modeling |
████████████████░░░░ 80.0% |
4,834 / 6,045 |
cdp |
████████████████░░░░ 80.7% |
3,118 / 3,864 |
wizard |
████████████████░░░░ 82.5% |
772 / 936 |
notebooks |
█████████████████░░░ 85.1% |
7,096 / 8,338 |
cohorts |
█████████████████░░░ 86.2% |
4,065 / 4,717 |
agent_platform |
█████████████████░░░ 86.4% |
3,807 / 4,405 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
product_tours |
█████████████████░░░ 87.5% |
1,266 / 1,447 |
exports |
██████████████████░░ 88.4% |
6,943 / 7,853 |
business_knowledge |
██████████████████░░ 88.5% |
4,400 / 4,969 |
conversations |
██████████████████░░ 89.0% |
16,183 / 18,186 |
mcp_analytics |
██████████████████░░ 89.2% |
2,514 / 2,819 |
visual_review |
██████████████████░░ 89.4% |
5,818 / 6,505 |
engineering_analytics |
██████████████████░░ 89.5% |
5,507 / 6,154 |
dashboards |
██████████████████░░ 89.5% |
5,923 / 6,616 |
alerts |
██████████████████░░ 89.9% |
4,054 / 4,508 |
early_access_features |
██████████████████░░ 90.1% |
1,031 / 1,144 |
error_tracking |
██████████████████░░ 90.1% |
9,885 / 10,966 |
streamlit_apps |
██████████████████░░ 90.4% |
2,501 / 2,767 |
slack_app |
██████████████████░░ 90.6% |
8,989 / 9,926 |
links |
██████████████████░░ 90.6% |
183 / 202 |
marketing_analytics |
██████████████████░░ 90.8% |
11,514 / 12,684 |
stamphog |
██████████████████░░ 91.0% |
3,993 / 4,387 |
product_analytics |
██████████████████░░ 91.4% |
5,781 / 6,328 |
mcp_store |
██████████████████░░ 91.8% |
3,685 / 4,012 |
data_warehouse |
██████████████████░░ 92.5% |
18,819 / 20,348 |
notifications |
███████████████████░ 92.7% |
1,031 / 1,112 |
ai_observability |
███████████████████░ 92.7% |
14,921 / 16,093 |
workflows |
███████████████████░ 92.8% |
5,520 / 5,949 |
web_analytics |
███████████████████░ 92.9% |
13,853 / 14,913 |
surveys |
███████████████████░ 93.0% |
5,724 / 6,157 |
posthog_ai |
███████████████████░ 93.2% |
1,325 / 1,421 |
approvals |
███████████████████░ 93.3% |
3,395 / 3,640 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
tracing |
███████████████████░ 93.4% |
2,546 / 2,725 |
managed_migrations |
███████████████████░ 93.8% |
1,220 / 1,300 |
legal_documents |
███████████████████░ 94.1% |
1,568 / 1,667 |
endpoints |
███████████████████░ 94.1% |
8,606 / 9,143 |
messaging |
███████████████████░ 94.2% |
2,647 / 2,810 |
revenue_analytics |
███████████████████░ 94.5% |
3,598 / 3,809 |
skills |
███████████████████░ 94.5% |
2,881 / 3,049 |
review_hog |
███████████████████░ 94.6% |
6,806 / 7,191 |
logs |
███████████████████░ 95.3% |
9,844 / 10,325 |
experiments |
███████████████████░ 95.7% |
24,417 / 25,527 |
replay_vision |
███████████████████░ 95.8% |
13,776 / 14,383 |
growth |
███████████████████░ 95.8% |
2,837 / 2,960 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
feature_flags |
███████████████████░ 96.3% |
16,233 / 16,859 |
user_interviews |
███████████████████░ 96.4% |
2,242 / 2,325 |
warehouse_sources |
███████████████████░ 96.4% |
288,287 / 298,905 |
access_control |
███████████████████░ 96.8% |
849 / 877 |
customer_analytics |
███████████████████░ 97.2% |
7,482 / 7,700 |
data_catalog |
███████████████████░ 97.4% |
2,304 / 2,365 |
analytics_platform |
████████████████████ 98.0% |
2,102 / 2,145 |
metrics |
████████████████████ 98.2% |
2,491 / 2,536 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
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.
5ebad7d to
0490534
Compare
1135f96 to
e36c236
Compare
59a2f58 to
c65e0ef
Compare
07d817a to
a1f105c
Compare
66c6fc0 to
52d0dd0
Compare
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
👀 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 |
| # ACP session to a non-stale OAuth token. Non-fatal: if refresh fails we | ||
| # still deliver the follow-up with the existing (possibly stale) creds. | ||
| _refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token) | ||
| _refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token, actor_user=actor_user, state=state) |
There was a problem hiding this comment.
Refresh MCP credentials when the follow-up actor changes
The per-run refresh marker lets the first actor's six-hour OAuth token and personal MCP server list remain installed for up to three hours. A later authorized Slack participant gets a connection token for themselves, but this call returns early via should_refresh_mcp_token(run_id), so the agent executes that participant's prompt using the earlier participant's PostHog permissions. A lower-privileged team member can therefore induce reads or writes through the task using a higher-privileged participant's MCP credentials.
Prompt To Fix With AI
Make the MCP-refresh cache actor-aware. Persist or cache the user ID that minted the currently installed MCP token/configuration and force `create_oauth_access_token_for_run` plus `send_refresh_session` whenever the resolved follow-up actor differs, even inside the normal refresh interval. Update the startup marker to record its actor too, and add a two-user Slack follow-up test asserting the second delivery refreshes with the second actor's OAuth token and personal MCP configuration.Severity: high | Confidence: 93% | React with 👍 if useful or 👎 if not
|
| if not should_refresh_mcp_token(run_id): | ||
| logger.info("refresh_mcp_skipped_within_interval", run_id=run_id) | ||
| return |
There was a problem hiding this comment.
Run-Wide Gate Retains Prior Actor
The refresh marker is keyed only by run_id, but the refreshed OAuth token and user MCP servers are actor-specific. If Alice refreshes the session and Bob's follow-up arrives within the refresh interval, this return leaves Alice's credentials active while the sandbox processes Bob's message.
Rule Used: When implementing new features, ensure that owners... (source)
Learned From
PostHog/posthog#31236
| handle = client.get_workflow_handle(workflow_id) | ||
| asyncio.run(handle.signal("send_followup_message", args=[message, artifact_ids, message_id])) | ||
| asyncio.run( |
There was a problem hiding this comment.
Signal Payload Breaks Old Workers
This changes every signal from three positional values to five. During a rolling deployment, a new caller can send this payload to a workflow still handled by the old three-argument signal handler, causing its workflow task to fail instead of queueing the follow-up; defaults only protect old histories replayed by new workers, not this reverse direction.
tatoalo
left a comment
There was a problem hiding this comment.
as in the stacked PR, we need patching for the signal task activity or in-roll workflows will fail
|
Did a wrong restack and unintentionally requested review from @PostHog/team-security / @PostHog/hogql / @PostHog/clickhouse . You guys don't need to review, sorry for the noise |
| # ACP session to a non-stale OAuth token. Non-fatal: if refresh fails we | ||
| # still deliver the follow-up with the existing (possibly stale) creds. | ||
| _refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token) | ||
| _refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token, actor_user=actor_user, state=state) |
There was a problem hiding this comment.
Rebind GitHub credentials for each follow-up actor
A cross-user Slack follow-up now resolves and authenticates delivery as its own actor, but it leaves the sandbox's GITHUB_TOKEN, GH_TOKEN, and git remote credential unchanged. Those were created from the workflow's initial actor and its static processing context. Thus a PostHog team member without the starter's GitHub repository access can prompt a full_auto Slack run to push or open a PR using the starter's user GitHub token.
Prompt To Fix With AI
When a Slack follow-up actor differs from the identity currently installed in the sandbox, resolve that actor's GitHub integration for the task repository and atomically refresh the sandbox git remote plus GITHUB_TOKEN/GH_TOKEN before delivering the message. If the actor has no authorized GitHub integration, remove/disable the existing user GitHub credentials and fail closed for repository-writing turns. Make the credential state/cache actor-aware and add a two-user Slack test where the second PostHog member has no access to the first actor's repository, asserting the second turn cannot use the first actor's GitHub token.Severity: high | Confidence: 88% | React with 👍 if useful or 👎 if not
There was a problem hiding this comment.
That's a known issue, follow up PR is doing MCP, and working on github as well
|
Reviews (2): Last reviewed commit: "refactor(tasks): resolve reply mentions ..." | Re-trigger Greptile |
|
Thanks for confirming. Since the current diff still leaves the initial actor’s GitHub credentials in the sandbox for a later participant’s turn, I’ll keep this finding open here until the follow-up fix is available. Please link the GitHub-rebinding PR (or update this branch) when it lands and I can verify and resolve it. |
d9caad1 to
0c1ff7e
Compare
844215b to
4febdf9
Compare
…'s speaker Every consumer of "who sent this" read mutable shared state that the next message overwrites — visibly, agent replies tagged whoever spoke last instead of the person being answered. - The follow-up signal gains actor_user_id and a context dict (its extension point for per-message fields, carrying the sender's Slack id). Additive with defaults: old histories replay unchanged. - Delivery pins credential resolution to the message's sender via a local state overlay, stamps the run-state actor at turn start, and on the turn's ack records the completed turn's actor. - The Slack relay tags the completed turn's actor, so a reply tags the person whose turn it answers even when the next message's delivery has already restamped the live actor. mapping.latest_actor stays as the pre-rollout fallback, marked for removal. - One slack_actor_state_updates builder in run_actor.py serves all writers of the load-bearing state keys.
The agent-server can echo the id of the user message a turn answers (optional relay_message field). Delivery records message_id -> sender when it sends; the relay resolves the tag from that correlation first, with the completed-turn stamp and live actor as fallbacks for agents that do not echo yet.
The mention fallback chain was split between the relay trigger (facade) and the relay activity. The relay input now carries the echoed message_id and the activity resolves the whole chain next to its existing mapping fallbacks — message actor, completed-turn actor, live actor, mapping — which also gives the pending-message relay paths the same semantics. Extract the message sender once per delivery, skip the completed-turn stamp when already current, and name the correlation TTL.
0c1ff7e to
632c21f
Compare
The slack_app forward-followup activity reached into products.tasks.backend.logic.services.run_actor directly, which tach rejects because products.tasks only exposes backend.facade. Re-export slack_actor_state_updates from facade.api and import it from there.
The relay endpoint now forwards message_id to execute_posthog_code_agent_relay_workflow; update the existing relay_message tests to expect message_id=None on the call.

Problem
Follow-ups are queued now (#70806), but everything that needs "who sent this one" — credential resolution, reply @-tagging, the permission broker — still reads shared mutable state the next message overwrites. Two people messaging back-to-back get replies tagged to whoever spoke last.
Changes
actor_user_id+ acontextdict (extension point, holds the sender's Slack id). Additive with defaults, so old histories replay unchanged.message_id → sender(redis, 2h TTL).message_idfirst (echoed by the agent, see below), falling back to the live actor for pre-rollout runs.mapping.latest_actor_slack_user_idstays as the pre-rollout fallback, marked for removal.slack_actor_state_updatesbuilder inrun_actor.pyfor every writer of the load-bearing keys.Paired with PostHog/code#3434
The agent-server echoes the initiating
message_idonrelay_message(that PR). Neither side hard-depends on the other: without the echo the relay uses its fallbacks (correct except rapid multi-user bursts), and an older backend ignores the unknown field. Exact per-message tagging switches on once both ship, with the agent release (npm + sandbox image) as the gating leg. Verified end-to-end locally with the two PRs together.How did you test this code?
Locally, two-user thread: each reply tags its sender and PostHog attribution follows the speaker. Automated coverage for the pinning, the turn-start actor stamp, and the forwarding/API call shapes.
Automatic notifications
Docs update
Internal behavior only.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Claude Fable 5).