Skip to content

update check_run.completed skill to avoid duplicate posts - #202

Open
dembrane-sam-bot wants to merge 1 commit into
mainfrom
sam/update-check-run-dedupe
Open

update check_run.completed skill to avoid duplicate posts#202
dembrane-sam-bot wants to merge 1 commit into
mainfrom
sam/update-check-run-dedupe

Conversation

@dembrane-sam-bot

Copy link
Copy Markdown
Contributor

What is this change?

Updates src/recipes/software-delivery/skills/github-webhook-events/check-run-completed.md to instruct Sam to check the Slack thread history (conversations.replies) before posting a passing CI notification. If a success notification for the latest commit has already been posted, Sam should avoid duplicate posts and exit cleanly. It also clarifies that Sam must use respond() instead of ack() to post the success message so that the daemon's silent-exit gate is satisfied.

What did Sam notice that led to this?

Sameer reported that Sam posted the duplicate "CI passed successfully" notification twice in thread 1785328582.306859.
A deep dive into daemon and tool logs revealed:

  1. GitHub fired multiple check_run.completed webhook events sequentially (once for each check run in the suite).
  2. The daemon serializes runs on the same thread by deferring later messages.
  3. Session 1 (aba2a133e024) posted the passing notification, but did so using ack() instead of respond(), leaving the silent-exit gate open.
  4. When Session 1 finished, the daemon popped the first deferred webhook and spawned Session 2 (dd1c20c94a8f).
  5. Session 2 queried conversations.replies on its own initiative, saw the post by Session 1, wrote a journal entry about avoiding duplicate posts, and exited cleanly without posting.
  6. When Session 2 finished, the daemon popped the second deferred webhook and spawned Session 3 (7b680d372306).
  7. Session 3 did not check the Slack history, followed the previous (undocumented) instructions in check-run-completed.md, and blindly posted the duplicate success notification again.

Codifying the conversations.replies check and the respond() requirement prevents this entire class of duplicate notifications.

Tier?

Tier 1 (freely propose prose edits to recipe skills).

Confidence?

100% confident. The logs show a clear, reproducible race/state collision across deferred sequential webhook deliveries, which is cleanly resolved by making the Slack history double-check a required step in the skill.

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.

1 participant