Skip to content

fix(tasks): stop stamping hedgehog reaction on interim slack relays#59858

Merged
richardsolomou merged 2 commits into
masterfrom
posthog-code/fix-slack-relay-hedgehog-reaction
May 28, 2026
Merged

fix(tasks): stop stamping hedgehog reaction on interim slack relays#59858
richardsolomou merged 2 commits into
masterfrom
posthog-code/fix-slack-relay-hedgehog-reaction

Conversation

@richardsolomou
Copy link
Copy Markdown
Member

Problem

When a Slack-launched task runs, the bot manages a reaction on the user's mention message as a status indicator:

Reaction Meaning
:seedling: task received / queued
:eyes: agent working
:hedgehog: task completed (or PR opened)
:x: task failed

post_slack_update is the intended single source of truth for terminal reactions — it only stamps :hedgehog: when the run reaches COMPLETED, CANCELLED, or a PR is opened.

However, every interim agent message posted back into the Slack thread via the relay_message endpoint was also flipping the reaction to :hedgehog:. The endpoint by design only fires for non-terminal runs (it short-circuits on task_run.is_terminal), so users were seeing the "done" hedgehog appear repeatedly while the agent was still working.

Root cause: RelaySlackMessageInput.reaction_emoji defaulted to "hedgehog", and the relay_message endpoint never passed an explicit value, so the default fired on every interim relay.

Changes

  • RelaySlackMessageInput.reaction_emoji and execute_posthog_code_agent_relay_workflow's reaction_emoji arg now default to None.
  • relay_slack_message only calls handler.update_reaction(...) when the caller explicitly passes a reaction.
  • forward_pending_message (the delivery-failure case that legitimately needs to mark :x:) is unchanged — it already passed reaction_emoji=\"x\" explicitly.
  • Net effect: interim agent relays no longer touch the reaction. post_slack_update remains the sole authority on lifecycle reactions.

How did you test this code?

  • Updated the existing test_relay_posts_message_and_marks_sent test to assert that an interim relay does not call update_reaction.
  • Added test_relay_with_explicit_reaction_updates_reaction to confirm an explicit reaction_emoji=\"x\" (the forward_pending_message path) still flows through.
  • I'm an agent — no manual testing in a live Slack workspace was performed.

Publish to changelog?

no

Docs update

No docs changes needed.

🤖 Agent context

Authored by Claude Code (Opus 4.7) via the PostHog Code sandbox. The user reported that the hedgehog emoji was appearing on their Slack mention before the task was actually done. Traced the reaction lifecycle through products/slack_app/backend/slack_thread.py:update_reaction, products/tasks/backend/temporal/process_task/activities/post_slack_update.py (intended terminal-state authority), and products/tasks/backend/temporal/slack_relay/activities.py (the bug site).

Considered two fixes:

  1. Change the relay default to \"eyes\" so interim messages reaffirm the working indicator.
  2. Make reaction_emoji optional (None) and only update the reaction when explicitly set.

Picked (2): the existing forward_pending_message caller already passes \"x\" explicitly, so the parameter is preserved for legitimate state-bearing relays; absence of an arg now means "don't touch it" rather than the previous footgun of "mark as done". This also avoids redundant Slack API calls (the update_reaction flow does a reactions_remove per stale emoji plus a reactions_add) on every interim message.

The reaction lifecycle on a Slack-launched task's mention message is:
seedling (received) → eyes (working) → hedgehog (done) / x (failed). Only
post_slack_update is supposed to flip to hedgehog, gated on COMPLETED /
CANCELLED / PR-opened.

RelaySlackMessageInput.reaction_emoji defaulted to "hedgehog", and the
relay_message API endpoint never passes one. Because that endpoint only
fires for non-terminal runs (by design — interim agent output), every
mid-task relay was clearing seedling/eyes and stamping hedgehog,
falsely signalling completion while the agent was still working.

Make reaction_emoji optional (None) and only call update_reaction when a
caller explicitly sets it. forward_pending_message still passes "x" for
the delivery-failure case; the relay_message endpoint stops touching
reactions and lets post_slack_update remain the sole authority on
terminal-state reactions.

Generated-By: PostHog Code
Task-Id: 8ff1e5da-c326-4cc3-b4bf-514021d34a87
@assign-reviewers-posthog assign-reviewers-posthog Bot requested a review from a team May 25, 2026 03:54
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 25, 2026

Comments Outside Diff (1)

  1. products/tasks/backend/temporal/slack_relay/tests/test_activities.py, line 68-103 (link)

    P2 These two tests cover mutually complementary cases for the reaction_emoji field (absent → no update_reaction call; explicit value → call fires) and are a good fit for @parameterized.expand. The parameterized import is already present in the file.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: products/tasks/backend/temporal/slack_relay/tests/test_activities.py
    Line: 68-103
    
    Comment:
    These two tests cover mutually complementary cases for the `reaction_emoji` field (absent → no `update_reaction` call; explicit value → call fires) and are a good fit for `@parameterized.expand`. The `parameterized` import is already present in the file.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
products/tasks/backend/temporal/slack_relay/tests/test_activities.py:68-103
These two tests cover mutually complementary cases for the `reaction_emoji` field (absent → no `update_reaction` call; explicit value → call fires) and are a good fit for `@parameterized.expand`. The `parameterized` import is already present in the file.

```suggestion
    @parameterized.expand(
        [
            ("no_reaction_emoji", "relay-1", "Which license should I use?", None, False, None),
            ("explicit_reaction_emoji", "relay-2", "Could not deliver follow-up", "x", True, "x"),
        ]
    )
    @patch("products.slack_app.backend.slack_thread.SlackThreadHandler.update_reaction")
    @patch("products.slack_app.backend.slack_thread.SlackThreadHandler.post_thread_message")
    @patch("products.slack_app.backend.slack_thread.SlackThreadHandler.delete_progress")
    def test_relay_reaction_behaviour(
        self, _name, relay_id, text, reaction_emoji, expects_update, expected_emoji, mock_delete_progress, mock_post, mock_update
    ):
        relay_slack_message(
            RelaySlackMessageInput(
                run_id=str(self.task_run.id),
                relay_id=relay_id,
                text=text,
                user_message_ts="1234.5",
                reaction_emoji=reaction_emoji,
            )
        )

        mock_delete_progress.assert_called_once()
        mock_post.assert_called_once()
        if expects_update:
            mock_update.assert_called_once_with(expected_emoji)
        else:
            mock_update.assert_not_called()
```

Reviews (1): Last reviewed commit: "fix(tasks): stop stamping hedgehog react..." | Re-trigger Greptile

@richardsolomou richardsolomou requested a review from a team May 25, 2026 03:59
Collapse the two relay reaction-behavior tests into a single
parameterized test covering both the no-reaction-emoji and explicit
reaction_emoji cases. Matches the repo convention of preferring
parameterized tests for variations of the same logic.

Generated-By: PostHog Code
Task-Id: 8ff1e5da-c326-4cc3-b4bf-514021d34a87
@github-actions
Copy link
Copy Markdown
Contributor

ClickHouse migration SQL per cloud environment

No ClickHouse migrations changed in this PR.

@richardsolomou richardsolomou merged commit a2aa087 into master May 28, 2026
218 checks passed
@richardsolomou richardsolomou deleted the posthog-code/fix-slack-relay-hedgehog-reaction branch May 28, 2026 16:01
richardsolomou added a commit that referenced this pull request May 28, 2026
The slack relay path was fixed in #59858 to stop stamping `:hedgehog:`
on interim agent replies, but `post_slack_update` still swapped the
reaction to `:hedgehog:` the first time it saw a `pr_url` while the run
was non-terminal. That fires when the agent is still alive (e.g.
handling follow-ups), making the hog land before the task is actually
done. Only terminal statuses (or post-cleanup branches) should set the
hedgehog now.

Generated-By: PostHog Code
Task-Id: 1fd5c724-c4da-426c-b742-03aeb7eef615
@deployment-status-posthog
Copy link
Copy Markdown

deployment-status-posthog Bot commented May 28, 2026

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-05-28 16:34 UTC Run
prod-us ✅ Deployed 2026-05-28 17:00 UTC Run
prod-eu ✅ Deployed 2026-05-28 17:02 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants