fix(dry-run): run validation before short-circuit and print structured preview#177
Merged
scottlovegrove merged 2 commits intomainfrom Apr 16, 2026
Merged
fix(dry-run): run validation before short-circuit and print structured preview#177scottlovegrove merged 2 commits intomainfrom
scottlovegrove merged 2 commits intomainfrom
Conversation
…d preview Closes #135 - Move getThread + assertChannelIsPublic above the dry-run check in thread done/mute/unmute/rename so dry-run catches missing or inaccessible threads the same way the real command would. - Add printDryRun(action, details) helper in src/lib/output.ts modelled on todoist-cli. Output is now a structured preview with a yellow header, resolved resource details, and a dim footer pointing to the real execution. - Adopt the helper across all 18 commands that support --dry-run. Dry-run messages now show the resolved target (e.g. thread title + id, channel name) instead of just the raw id. - Remove dead printNotifyLines helper after folding notify output into the details dict. - Update SKILL_CONTENT with a brief Dry Run section; regenerate SKILL.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
reviewed
Apr 16, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR introduces a helpful structured preview format for --dry-run commands and ensures validation runs before the short-circuit for several thread actions. These updates greatly improve the actionable feedback users receive while keeping the codebase aligned with expected CLI behavior. There are just a couple of areas to refine, specifically regarding the indentation of multiline values in the new output helper and ensuring the agent documentation accurately reflects which commands currently perform pre-fetch validation.
Address PR #177 review feedback: - printDryRun now splits detail values on newlines and indents each continuation line so multi-paragraph previews stay structured instead of rendering flush-left and blending into the footer. - Narrow the SKILL_CONTENT Dry Run section to say pre-flight validation runs "where a command performs it" rather than implying every command validates first — several commands (conversation done/mute/unmute, comment/msg delete) still parse the ref and print the preview without hitting the API. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doist-release-bot bot
added a commit
that referenced
this pull request
Apr 16, 2026
## [2.31.1](v2.31.0...v2.31.1) (2026-04-16) ### Bug Fixes * **dry-run:** run validation before short-circuit and print structured preview ([#177](#177)) ([d552e45](d552e45))
Contributor
|
🎉 This PR is included in version 2.31.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
scottlovegrove
added a commit
that referenced
this pull request
Apr 16, 2026
Follow-up to #177. The earlier PR fixed validation ordering for the four thread commands that already had pre-flight validation. Five more commands short-circuited on --dry-run without touching the API at all, so previews against nonexistent, inaccessible, or non-owned resources falsely reported success. This adds a fetch (and, where appropriate, a creator/channel check) before the dry-run short-circuit: - conversation done/mute/unmute: getConversation, enrich preview with resolved title + current mute state. No creator check — archive and mute are per-user inbox/notification state, not shared-data mutations. - msg delete: client.batch(getMessage, getSessionUser), throw NOT_CREATOR if message.creator != sessionUser.id. Mirrors the existing thread delete pattern. - comment delete: client.batch(getComment, getSessionUser), assertChannelIsPublic(channelId, workspaceId), NOT_CREATOR check. Comments carry channelId on the entity so no extra thread fetch is needed. Also adds a `conversationLabel` helper in conversation/helpers.ts for consistent "title (id)" rendering in dry-run previews. Tests: expanded msg.test.ts from a 42-line stub to a full fixture covering delete, dry-run, creator-rejection and --json paths. Updated comment and conversation tests to cover the new fetch + gate, and added a new conversation done describe block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #135.
Summary
thread done,thread mute,thread unmute,thread renamenow rungetThread+assertChannelIsPublicbefore the--dry-runshort-circuit. Previously, a dry-run against a missing or inaccessible thread falsely reported success — it now fails the same way the real command would.printDryRun(action, details)helper insrc/lib/output.ts(modelled on the todoist-cli pattern) replaces the oldDry run: would ...one-liners with a structured preview showing resolved state.--dry-runcall sites (thread, conversation, msg, comment, away, react/unreact) now use the helper. Resource references show resolved data (e.g. thread title + id) instead of raw IDs.SKILL_CONTENTpicks up a brief Dry Run section;SKILL.mdregenerated.New output format
Breaking change (output only)
The dry-run message format changes from
Dry run: would mute thread 500 for 60 minutesto the structured form above. Any external scraper of dry-run text needs to update its parser. CLI syntax is unchanged.Out of scope
These will be done in later PRs
conversation done/mute/unmute,msg delete,comment delete). The issue asks that existing validation not be skipped; introducing new validation is a separate concern.react/unreact, which already emitted JSON dry-run and keeps its existing shape.Test plan
npm run lint:check— cleannpm run type-check— cleannpm run check:skill-sync— in syncnpm test— 458 tests pass (new tests prove validation runs in dry-run for the four fixed commands; new unit tests forprintDryRun)tw away clear --dry-run,tw away set vacation 2026-05-01 --dry-run,tw react thread 123 +1 --dry-run(text + JSON) against the built binarytw thread mute <valid-id> --minutes 30 --dry-runrenders resolved title;tw thread mute 999999 --dry-runfails with "thread not found"🤖 Generated with Claude Code