Skip to content

feat: add --notify NONE for thread replies - #62

Merged
rmartins90 merged 1 commit into
mainfrom
rmartins90/thread-reply-notify-none
Sep 15, 2026
Merged

rmartins90 merged 1 commit into
mainfrom
rmartins90/thread-reply-notify-none

Conversation

@rmartins90

Copy link
Copy Markdown
Contributor

Overview

Add --notify NONE to post a thread reply without notifying anyone, including replies that close or reopen the thread. The default remains EVERYONE_IN_THREAD, and existing user/group recipients keep their current behavior.

Changes

  • Send empty recipient, group, and direct-mention lists without adding a notification audience.
  • Show Notify: NONE in dry-run previews and document the option in help, shell completion, and the generated agent skill.

Test plan

  1. From this branch, build the CLI and run node dist/index.js thread reply <thread-ref> "Quiet update" --notify NONE --dry-run. Check that the preview shows Notify: NONE and no comment is posted.
  2. Repeat with --close and --reopen. Check that the preview shows the intended thread action and still shows Notify: NONE.
  3. Omit --notify and check that the preview shows EVERYONE_IN_THREAD. Try explicit user/group IDs and check that the preview names the intended recipients.

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds a NONE sentinel for --notify on thread replies, wired consistently through the reply, close, and reopen paths with empty recipient/group/mention lists, plus dry-run preview output (Notify: NONE), help text, completion choices, README, and SKILL_CONTENT/skill file updates kept in sync. No inline issues were flagged — the implementation reuses the existing notify helpers, keeps authorization checks unconditional, and is well covered by new tests.

I also left one optional follow-up note in the details below.

Optional follow-up note (1)
  • P3 src/commands/thread/thread.test.ts:449: preserves existing recipients for $notify re-runs the same six recipient modes against all three actions (18 cases), even though the ID-resolution and notificationFields construction are shared across reply/close/reopen and the action-specific spread is already exercised by the --notify NONE tests. Consolidate: keep the six-mode matrix on reply only, and add one representative non-empty case (e.g. 123,id:group1) for close and reopen to confirm non-empty recipients/groups still reach those methods.

Share FeedbackReview Logs

@rmartins90
rmartins90 marked this pull request as ready for review September 15, 2026 10:51
@rmartins90 rmartins90 added the 🙋 Ask PR PR must be reviewed before merging label Sep 15, 2026
@doistbot
doistbot requested a review from pawelgrimm September 15, 2026 10:51
@rmartins90
rmartins90 merged commit 9e95c8d into main Sep 15, 2026
8 checks passed
@rmartins90
rmartins90 deleted the rmartins90/thread-reply-notify-none branch September 15, 2026 11:39
doist-release-bot Bot added a commit that referenced this pull request Sep 15, 2026
## [3.2.0](v3.1.0...v3.2.0) (2026-09-15)

### Features

* add --notify NONE for thread replies ([#62](#62)) ([9e95c8d](9e95c8d))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

🙋 Ask PR PR must be reviewed before merging released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants