Skip to content

Fix Slack thread ownership transfer context#757

Open
gugu91 wants to merge 2 commits into
mainfrom
fix/slack-thread-ownership-transfer
Open

Fix Slack thread ownership transfer context#757
gugu91 wants to merge 2 commits into
mainfrom
fix/slack-thread-ownership-transfer

Conversation

@gugu91
Copy link
Copy Markdown
Owner

@gugu91 gugu91 commented May 18, 2026

Summary

  • append explicit Slack thread transfer context (thread_ts/channel/recovery hint) to broker A2A transfer messages
  • store transferred Slack channel/source metadata on threadOwnershipTransfer
  • surface transferred Slack threads in Pinet inbox pointers with slack_send guidance
  • hydrate follower local thread state from transfer metadata so transferred workers have channel context available for reply resolution

Fixes #756.

Investigation notes

The DB ownership transfer path was updating the broker thread owner, but the recipient-visible A2A message stayed opaque: follower polling treats agent messages separately from regular Slack inbox sync, so the worker could receive only a Pinet pointer and generic reply via pinet action=send guidance. If the original Slack request had already been read, no unread Slack inbox rows were reassigned, leaving the worker without an obvious Slack reply context/recovery path.

This PR keeps the existing first-owner-wins state machine, but makes explicit broker transfers carry and surface the Slack thread context needed by the new owner.

Validation

  • pnpm --filter @gugu910/pi-slack-bridge lint
  • pnpm --filter @gugu910/pi-slack-bridge typecheck
  • pnpm --filter @gugu910/pi-slack-bridge test
  • pre-push: pnpm lint && pnpm typecheck && pnpm test

@gugu91
Copy link
Copy Markdown
Owner Author

gugu91 commented May 18, 2026

Will's reviewer agent Patch Platypus

Verdict

  • Block
  • The new transfer context is useful, but the follower-side hydration still preserves a stale local owner for already-cached threads.

Top findings:

  • slack-bridge/helpers.ts:2212syncTransferredSlackThreadContexts() keeps existing?.owner instead of overwriting to the recipient's owner token. For an explicit transfer, that leaves the local cache thinking someone else still owns the thread, so reconnect/reclaim (slack-bridge/follower-runtime.ts:232) and /pinet status (slack-bridge/pinet-commands.ts:492) stay wrong.
  • Please add a regression test for the existing-cached-thread/other-owner case.

@gugu91
Copy link
Copy Markdown
Owner Author

gugu91 commented May 18, 2026

Will's reviewer agent Thread Newt

Re-review after 4a81820 on PR #757: Approve.

  • Resolved: slack-bridge/helpers.ts:2212 now overwrites stale cached owner with agentOwner, so transferred-thread cache, reclaim, and /pinet status converge on the new owner.
  • Covered: slack-bridge/helpers.test.ts:3482 adds the cached-thread/other-owner regression case.

Verification:

  • pnpm --filter @gugu910/pi-slack-bridge lint
  • pnpm --filter @gugu910/pi-slack-bridge typecheck
  • pnpm --filter @gugu910/pi-slack-bridge test

No remaining blocking findings from the earlier stale-owner cache review.

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.

Bug: Slack thread ownership transfer can leave broker and workers unable to post

1 participant