feat: add thread undone, conversation undone and thread mark-unread

[lmjabreu]

Overview

tdc thread done, tdc conversation done and tdc thread mark-read had no inverses, so a caller that archives or marks read on someone's behalf (Bearing's comms "done" action is the one I have) cannot record an undo step. This adds the three:

• tdc thread undone <thread-ref>: unarchive, back to the inbox. Same shape as done (--yes, --dry-run, --json).
• tdc conversation undone <conversation-ref>: same, for conversations.
• tdc thread mark-unread [thread-refs...] [--from <comment-ref>]: same shape as mark-read, bulk refs on stdin included. Without --from the whole thread goes unread. With it, that comment and everything after it does; --from is single-thread only since a comment belongs to one thread.

The one non-obvious bit is what --from sends. The SDK's markUnread takes the objIndex of the last comment that stays read (-1 for none), so a comment at index N is sent as N-1. Probed live on a throwaway thread: after mark-unread --from <comment at 1>, thread view --unread shows comments 1 and 2 of 0..2.

The bulk-ref and unread-lookup helpers mark-read kept private moved to thread/helpers.ts so both verbs share them; mark-read behaviour is unchanged and its tests pass as they were.

Reference

n/a. Follow-up in Bearing once this is released: record tdc thread undone <id> --yes as the undo step for its comms archive action.

Changelog

Add thread undone, conversation undone and thread mark-unread (with --from <comment-ref>), the inverses of done and mark-read.

Test plan

Use a thread in a channel where you are the only member (a private one; add --include-private-channels) and your self-DM, so nobody else sees the round trip.

1. tdc thread done <ref> --yes, then tdc thread undone <ref> --dry-run
   • Prints Would unarchive thread with the title, no status line
2. tdc thread undone <ref> --yes --json, then tdc thread undone <ref> --dry-run
   • JSON is { id, isArchived: false }; the thread is back in tdc inbox; the second dry run says Status: already in inbox
3. Reply to the thread three times, tdc thread mark-read <ref>, then tdc thread mark-unread <ref> --from <second comment id>
   • tdc thread view <ref> --unread shows the second and third comments only
   • Running the same mark-unread --from again says is already unread from comment ... and makes no call
4. tdc thread mark-unread <ref>
   • view --unread now shows all three; mark-unread <ref> <ref> --from x errors with CONFLICTING_OPTIONS
5. tdc conversation done id:<self-dm> --yes, then tdc conversation undone id:<self-dm> --yes --json
   • JSON is { id, archived: false } and tdc conversation list --state archived no longer lists it

🤖 Generated with Claude Code

[lmjabreu]

@doistbot /review

[doistbot]

⚠️ PR size is large: Review quality may be affected

👋 @lmjabreu This PR is large enough that Doistbot's review may miss details.

Current diff: 991 review-load lines across 14 files (+991 / -197). I will still run the review, but this would be easier for your colleagues to review as smaller PRs or a PR stack 😅

ℹ️ To make it easier to review, the recommended diff size is < 750 review-load lines and < 25 files changed

To be mindful of their time I would suggest you split this PR

🪄 Suggested slicing plan 👇

Split the PR into a two-part stack: first deliver the archive inverse actions ('thread undone' and 'conversation undone') along with archive helper refactoring, then deliver the read-state inverse ('thread mark-unread') alongside the shared bulk read-state mutation runner and 'mark-read' refactoring. This keeps both slices under 700 review-load lines and groups changes by domain capability.

PR order

1. slice-1-feat-archive-add-thread-undone-and-conve → base main
2. slice-2-feat-thread-add-mark-unread-command-and- → base slice-1-feat-archive-add-thread-undone-and-conve

PR 1 feat(archive): add thread undone and conversation undone commands

Implements the unarchive inverse commands ('tdc thread undone' and 'tdc conversation undone') to move threads back to the inbox and restore conversations. Unifies archive and unarchive state handling for threads and conversations into shared functions and exports a thread label formatter.

Files (11):

• README.md
• skills/comms-cli/SKILL.md
• src/commands/conversation/archive.ts
• src/commands/conversation/conversation.test.ts
• src/commands/conversation/done.ts
• src/commands/conversation/index.ts
• src/commands/thread/helpers.ts
• src/commands/thread/index.ts
• src/commands/thread/mutate.ts
• src/commands/thread/thread.test.ts
• src/lib/skills/content.ts

PR 2 feat(thread): add mark-unread command and share read-state mutation runner

Implements 'tdc thread mark-unread' with support for whole-thread unread and partial unread from a specific comment via '--from <comment-ref&gt;'. Unifies the bulk ref handling, caching, dry-run JSON output, and summary reporting between 'mark-read' and 'mark-unread' in a shared helper.

Files (9):

• README.md
• skills/comms-cli/SKILL.md
• src/commands/thread/helpers.ts
• src/commands/thread/index.ts
• src/commands/thread/read.ts
• src/commands/thread/thread.test.ts
• src/commands/thread/unread.ts
• src/lib/api.ts
• src/lib/skills/content.ts

This plan is based on the current PR head. Keep each slice buildable and move tests with the behavior they cover.

You can use your agent of choice (Codex/Claude etc) to help you split this PR 😊 Just copy the link to this comment and ask them Can you please create a PR stack based on the suggestions in this comment

[doistbot]

Nice follow-through on adding the missing inverses — thread undone, conversation undone, and thread mark-unread slot cleanly into the existing done/mark-read patterns, with the helper extraction into thread/helpers.ts making good reuse of the bulk-ref plumbing. A few things worth tightening:

• markThreadUnread duplicates the bulk-ref loop from markThreadRead (ref collection, --yes guard, preview/changed/unchanged branches, summary footer); consider extracting a shared driver parameterized by per-thread callbacks so the control flow can't drift.
• mark-unread --json --dry-run produces no output for either output mode — emit the collected jsonStatuses in dry-run (like react) or reject the flag combo, and cover it with a test.
• conversation undone --yes always fetches the conversation even though only the ID is used; branch on dry-run/--yes first so scripted calls skip the extra round trip and stale pre-check.
• With --from, the workspace-wide unread list is loaded before the comment ref is resolved, so an invalid comment triggers a potentially large request before INVALID_REF; resolve/validate --from first.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (4)

• src/commands/thread/mutate.ts:53: Use the new threadLabel(thread) helper from ./helpers.js instead of inlining ${thread.title} (${threadId}). That is the same label format the shared read/unread paths now use; markThreadDone above duplicates it too.
• src/commands/thread/helpers.ts:136: getLatestObjIndex is now exported from thread/helpers.ts, but only read.ts imports it; unread.ts never uses it. The project keeps helpers.ts for utilities shared by multiple subcommands, so this function can stay private in read.ts instead of widening the shared surface.
• src/commands/thread/thread.test.ts:2077: The bulk mark-unread tests assert counts but not which thread each ref maps to. marks bulk refs unread with --yes only checks markUnread was called twice, not { id: '500', objIndex: -1 } and { id: '501', objIndex: -1 }; the stdin preview test uses the default getThread mock (which returns Test Thread (500) for every id) and asserts only one Would mark unread... line. A ref-resolution regression that collapsed both refs to 500 would still pass. Mirror the mark-read bulk tests: mock getThread per id and assert the per-ref calls/labels.
• src/commands/thread/thread.test.ts:2061: previews bulk refs from stdin and asks for --yes never controls process.stdin.isTTY. The command action returns early and prints help when refs.length === 0 && process.stdin.isTTY, so this test fails in a TTY test environment before exercising markThreadUnread. The three mark-read stdin tests explicitly save and restore process.stdin.isTTY = false; do the same here so the test's outcome doesn't depend on the runner's TTY state.

_{Share Feedback • Review Logs}

[doistbot]

conversation undone <id> --yes always fetches the conversation before mutating, but the execution paths only use the ID and print a fixed status object/message. Branch on dryRun/yes first and fetch only for previews or confirmation, so scripted undo calls avoid an extra network round trip and a stale existence pre-check.

[lmjabreu]

Keeping as-is. It mirrors conversation done and thread done/undone, which all fetch first, and I'd rather the four stay one shape than have one skip the lookup. Happy to drop the fetch across all four in a follow-up if you think the round trip matters.

[lmjabreu]

I also included a few optional follow-up notes in the details below.

The four optional notes: all done in 9959a58. done/undone use threadLabel, getLatestObjIndex is private to read.ts again, the bulk mark-unread tests mock getThread per id and assert both refs' calls and labels, and the stdin test pins isTTY like its mark-read siblings.

[lmjabreu]

@doistbot /review

[doistbot]

Adds three inverse verbs — thread undone, conversation undone, and thread mark-unread (with --from <comment-ref>) — mirroring the existing done/mark-read commands, with the bulk-ref and unread-lookup helpers shared via thread/helpers.ts. Few things worth tightening:

• The --from single-thread guard only sees positional refs since it runs before collectThreadRefs, so stdin-supplied refs bypass the CONFLICTING_OPTIONS check — the first thread gets marked unread before an INVALID_REF failure on the second, leaving a partial mutation. Move the check to after ref collection (e.g. inside runThreadReadStateMutation).
• Add a test covering --from with multiple refs piped via stdin, asserting CONFLICTING_OPTIONS and no markUnread calls, so the guard gap above is locked in once fixed.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (8)

• src/commands/thread/mutate.ts:45: markThreadUndone repeats markThreadDone almost verbatim (resolve, fetch, public-channel check, dry-run, --yes guard, JSON/status output), differing only in verb, API call, and status field. The repo already models this exact pair with setArchiveState(ref, options, archive) in src/commands/channel/archive.ts. Extract an equivalent setThreadArchiveState and have both commands delegate to it with archive: true/false so the control flow can't drift.
• src/commands/conversation/undone.ts:7: markConversationUndone duplicates markConversationDone (resolve, fetch, dry-run, --yes/MISSING_YES_FLAG guard, JSON/status output) with only the archive direction differing. Follow the existing setArchiveState pattern from src/commands/channel/archive.ts by extracting a shared setConversationArchiveState and delegating both done and undone to it. This keeps the current fetch-first order while removing the duplicated control flow.
• src/commands/thread/mutate.ts:67: This new line inlines ${thread.title} while the dry-run branch just above uses the shared threadLabel(thread) helper, and conversation undone uses conversationLabel(conversation) at its equivalent line. Use threadLabel(thread) here too so the confirmation preview carries the same Title (id) format as the other unarchive outputs.
• src/commands/thread/helpers.ts:144: Bulk mark-read/mark-unread runs each thread serially: every ref awaits getThread (plus the first getUnread/channel lookups) and then its mutation before the next ref starts. The per-thread calls are independent, so a large stdin batch costs ~2N sequential round trips. The codebase already parallelizes independent calls elsewhere (e.g. resolveChannelMemberRefs, channel/set); consider Promise.all over the refs once the shared per-workspace caches are warmed.
• src/commands/thread/helpers.ts:146: mark-unread --from resolves the comment here before loadThreadReadState starts its getThread call on the next line. Those lookups use already-known IDs and only need to be compared afterward, so each successful --from invocation pays two sequential API round trips (then the unread lookup). Split thread loading so the comment lookup and initial thread fetch run in parallel, and defer getUnread until comment validation succeeds to preserve the existing invalid-comment optimization.
• src/commands/thread/thread.test.ts:2163: This test duplicates rejects a --from comment that belongs to another thread (line 1988): same fixture, same command, same INVALID_REF assertion. Fold the getUnread not-called assertion into that test and delete this one.
• src/commands/thread/thread.test.ts:2023: The name promises "at or before", but this only exercises lastReadObjIndex === target (objIndex: 0 vs target 0). The < target half of isUnchanged (e.g. a fully unread thread with objIndex: -1 plus --from a later comment) is untested, so changing <= to === would still pass. Extend the case so an already-earlier unread position is a no-op.
• src/commands/thread/unread.ts:47: The unchanged JSON row's lastReadObjIndex: state.lastReadObjIndex ?? target (the thread's actual read position, not the requested target) is never asserted: both --json tests use fully read threads, which take the planned path. A test like mark-unread 500 --from 11 --json against a thread already unread at objIndex 0 with a --from target of 2 would lock the contract that scripts see the current state — and fail if the ternary is simplified to always return target.

_{Share Feedback • Review Logs}

[lmjabreu]

Optional follow-up notes (8)

The eight optional notes, in 92c38d3: done/undone now share setThreadArchiveState and setConversationArchiveState (the done.ts/undone.ts pair became conversation/archive.ts, same shape as the channel one); the confirmation line uses threadLabel too; the duplicate INVALID_REF test is folded into its sibling; the "at or before" case is an it.each over 0 and -1; and the unchanged JSON row has its own test.

Two kept as-is: bulk stays serial because a batch of mutations is easier to reason about when it stops at the first failure with everything before it applied in order, and mark-read was already serial; and the --from comment fetch stays sequential since it is one extra round trip on the single-thread path only.

[lmjabreu]

@doistbot /review

[doistbot]

This PR adds the three inverse verbs — thread undone, conversation undone, and thread mark-unread (with --from <comment-ref>) — and consolidates the previously duplicated mark-read/mark-unread control flow into a shared driver with setArchiveState helpers mirroring the existing channel pattern. No inline issues were flagged; prior review rounds' items (bulk-ref loop duplication, JSON dry-run output, --from validation ordering, the stdin single-thread guard, helper reuse, and test coverage) are all addressed in the current head, and the security and reuse passes found nothing new.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (4)

• src/commands/thread/mutate.ts:44: In execute mode undone --yes always calls unarchiveThread, even when thread.isArchived is already false — the same state the dry-run branch reports as already in inbox. The channel archive helper skips the write via channel.archived !== archive; mirror that here (and in setConversationArchiveState) so a repeated undo is a no-op instead of a redundant API call plus a misleading unarchived success line.
• src/commands/thread/helpers.ts:144: jsonStatuses and textStatuses are both populated for every ref, but the output modes are exclusive: JSON mode returns before reading textStatuses, while text mode never reads jsonStatuses. For large stdin batches this retains unnecessary per-ref state and constructs unused JSON status objects. Collect only the statuses for the selected output mode (or use a single outcome collection and materialize the selected representation).
• src/commands/thread/unread.ts:79: The INVALID_REF guard for comments without a numeric objIndex has no test. A regression here (e.g. dropping the guard) would send NaN/undefined to markUnread instead of failing fast. Add a thread mark-unread 500 --from 11 case where getComment returns a comment with no objIndex and assert it rejects with INVALID_REF and never calls markUnread.
• src/commands/thread/thread.test.ts:1923: This dry-run test only covers an already-in-inbox thread (Status: already in inbox). The normal preview path from the PR test plan — thread undone <archived-ref> --dry-run — should print Would unarchive thread with the title and no status line, but that branch (thread.isArchived === true) is untested. Add an archived-thread dry-run case asserting the title and that no Status: line is emitted.

_{Share Feedback • Review Logs}