Skip to content

Link Slack notifications to comment threads - #285

Merged
HamptonMakes merged 2 commits into
block:mainfrom
mrohan-sq:mrohan/slack-comment-deep-links
Oct 28, 2026
Merged

HamptonMakes merged 2 commits into
block:mainfrom
mrohan-sq:mrohan/slack-comment-deep-links

Conversation

@mrohan-sq

@mrohan-sq mrohan-sq commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What changed and why

CoPlan's Slack comment notifications open the document but leave the triggering discussion closed. Add ?thread=THREAD_ID to links from both Slack senders so clicking a notification opens the right discussion.

Use the document's readable address, preserve configured engine mount paths, and keep the discussion target in Slack previews. Keep the discussion target when a renamed or moved document redirects to its new address. This reuses existing discussion-opening behavior without changing recipients or permissions.

Evidence

The shared demo's “request-local memo” discussion opens directly from its link in both themes.

Screenshots were captured locally, but GitHub CLI requires upstream write access to attach them. This account has read access to block/coplan.

How to try it

  1. Start the development app and sign in as hampton@example.com.
  2. Open /hampton/demo-library, choose the search-latency example, and copy a discussion link.
  3. Open the link in a new tab. The matching discussion should open automatically.
  4. With Slack configured, follow a newly sent comment notification. Expect the same result, including from its preview title and after renaming the document.

The local demo is running at http://localhost:3107/hampton/engineering/active-projects/search-latency-cutting-p95-with-a-two-tier-cache?thread=01a17e85-409c-7aac-978f-e7d9bcdd15b3.

Testing

  • Manual browser check: opened the shared demo's direct discussion link in light and dark themes; the matching discussion opened.
  • No live Slack messages were sent. Job specs capture the generated message, and a JavaScript-enabled browser spec follows its actual URL.
  • Added a browser regression in spec/system/comment_ux_spec.rb and job coverage for single comments, batches, mount paths, and preview links.
  • No model, authorization, or API behavior changes; notification job specs cover the changed behavior.
  • Full bundle exec rspec not run locally; focused coverage passed, with the full suite left to CI:
    • bundle exec rspec spec/jobs/slack_notification_job_spec.rb spec/jobs/slack_notification_delivery_job_spec.rb spec/jobs/slack_unfurl_job_spec.rb spec/requests/slack_events_spec.rb spec/requests/agent_instructions_spec.rb spec/requests/browse_spec.rb — 129 examples passed.
    • bundle exec rspec spec/system/comment_ux_spec.rb:444 spec/system/comment_ux_spec.rb:429 spec/system/comment_ux_spec.rb:468 — 3 examples passed.
  • Tested affected behavior in light and dark themes.
  • Screenshot upload blocked by upstream write permissions; both themes were verified locally.

Generated with Codex

@mrohan-sq
mrohan-sq marked this pull request as ready for review October 27, 2026 20:26
@HamptonMakes
HamptonMakes merged commit 9f84958 into block:main Oct 28, 2026
15 checks passed
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