Repository navigation
Conversation
Refactored all three strategies to eliminate duplication. Each strategy now uses private helper methods that contain the shared logic:
The public Commit: 52ae9fc |
nzakas
left a comment
There was a problem hiding this comment.
Review of the thread support, requested by @nzakas. The reply mechanics for the three native strategies look right: Bluesky sets record.reply with root and parent {uri, cid} from the createRecord responses, Mastodon sends in_reply_to_id, and X sends reply.in_reply_to_tweet_id. Images attach per entry and the abort signal reaches every request. The problems are mostly in how Client.postThread() handles results and failures. Roughly by severity:
Client.postThread()rejects for every real strategy that hasgetUrlFromResponse()(inline,src/client.js). The strategy result is now an array, but it's passed straight togetUrlFromResponse(), which expects a single post. Twitter, Bluesky, and Mastodon all throw on an array, and the throw happens outsidePromise.allSettled(). So the thread gets posted, then the whole call rejects and every other strategy's result is lost too. I reproduced this with the PR'sclient.jsand Twitter'sgetUrlFromResponse(): it rejects withTweet ID not found in response. The client test for this uses a mockpostThread()that returns a plain object, which hides the bug.- Partial failure loses the posts that already went out. If post N fails, the posts before it are live, but the
FailureResponseholds only the error. The caller can't find, link, or clean up the half-posted thread. This applies to the nativepostThread()methods and the client fallback, and aborting mid-thread has the same effect. - Entries are validated inside the posting loop instead of before it. A bad entry at position 3 is found only after entries 1-2 are published.
validatePostOptions()/ Mastodon's image checks are also skipped for thread entries, so a bad image fails partway through the upload. - The fallback of posting each entry separately does damage on some platforms. On Dev.to each entry becomes its own published article, titled with its first line. On LinkedIn it makes N separate feed posts. Also, several platforms that get the fallback do support replies: Nostr (NIP-10
etags), Slack (thread_ts), Telegram (reply_parameters), and Discord (message_reference, already in the webhook typedefs). The original request was to addpostThread()wherever threads are native, so these could get real implementations, or at least a fallback that doesn't publish N unrelated posts. - Test coverage: no strategy-level tests for
postThread()intests/strategies/{twitter,bluesky,mastodon}.test.js. Nothing checks that the second request includesin_reply_to_id,reply.in_reply_to_tweet_id, orrecord.reply.root/parent, which is where the platform-specific bugs would be.
Not in the diff:
- No CLI, MCP, or README changes. The PR is titled "across social media platforms", but there's no way to post a thread from
crosspostor the MCP server, and the newClient#postThread()/Strategy#postThreadAPI isn't documented. The CLI will need to decide how to split input into posts (e.g. a delimiter line such as---) and whether to check each entry against each strategy'sMAX_MESSAGE_LENGTH/calculateMessageLength()before posting anything. Nothing checks length now, so one entry over the limit fails the thread partway through. - Branch state: 6 commits behind
main. That includes #159 (Bluesky aspect ratio, which touchespostMessage()), the LinkedIn API update, and the MCP SDK upgrade. It merges cleanly, but no CI checks have run on this branch (mergeStateStatus: UNSTABLE, "no checks reported"), so a rebase or merge frommainand a CI run would help. - Unrelated churn: the
package-lock.json"peer": trueremovals, Prettier reformatting ofsrc/bin.js,tests/bin.test.js, andtests/strategies/bluesky.test.js, and removal of thedata.edit_history_tweet_idsproperty fromTwitterPostResponse(X still returns that field). These would be better dropped or split out.
On backward compatibility, post() and postTo() are unchanged and postThread is optional on Strategy, so existing callers are fine. The one contract change for third-party strategies is item 1: getUrlFromResponse() now gets whatever postThread() returns.
| return new SuccessResponse( | ||
| this.#strategies[i].name, | ||
| result.value, | ||
| this.#strategies[i].getUrlFromResponse?.(result.value), |
There was a problem hiding this comment.
Bug: result.value here is an array, from native postThread() or from the fallback loop, but every built-in getUrlFromResponse() expects a single post response:
- Twitter:
response?.data?.idis undefined, so it throwsTweet ID not found in response - Bluesky and Mastodon:
response?.uriis undefined, so it throwsPost URI not found in response
This runs inside the .map() after Promise.allSettled(), so the throw rejects the whole postThread() call after the thread is already live, and the other strategies' results are thrown away too. I reproduced it with this file and Twitter's getUrlFromResponse().
Suggestion: build the URLs per post, e.g. url from value[0] (the thread root) plus a urls array with one entry per post. Also wrap the call so a URL failure can't turn a successful post into a rejection. Then SuccessResponse.response is always an array for threads, which is worth documenting.
| }); | ||
| responses.push(response); | ||
| } | ||
| return responses; |
There was a problem hiding this comment.
Partial failure: if strategy.post() throws on entry N, the posts for entries 0..N-1 are already published, but responses is dropped and the caller gets a FailureResponse with only the error. I confirmed this: with a strategy that fails on the 2nd of 3 entries, the result is [{ ok: false, reason: "boom" }] and there's no record of the first post. Native postThread() implementations lose data the same way, and so does an abort mid-thread.
A thread-level failure needs to carry what was posted. For example, a ThreadError with .responses (and .urls) that both the fallback and native implementations throw, surfaced on the FailureResponse. That way users can link to or delete the partial thread.
| this.#strategies.map(async strategy => { | ||
| // If strategy has native postThread support, use it | ||
| if (strategy.postThread) { | ||
| return strategy.postThread(entries, postOptions); |
There was a problem hiding this comment.
About the fallback on the next few lines: calling post() once per entry is harmful for some strategies. DevtoStrategy.post() creates a published article titled with the message's first line, so a 5-entry thread becomes 5 Dev.to articles. LinkedinStrategy makes 5 unrelated feed posts.
Also, several fallback strategies do support replies natively and could implement postThread(): Nostr (NIP-10 ["e", rootId, relay, "root"] / "reply" tags), Slack (thread_ts), Telegram (reply_parameters.message_id), and Discord (message_reference). For the rest, consider making the fallback opt-in, or posting only the first entry (or the entries joined together) rather than N standalone posts.
| let previousTweetId; | ||
|
|
||
| for (const entry of entries) { | ||
| if (!entry.message) { |
There was a problem hiding this comment.
Entries are validated inside the posting loop, so if entry 3 has no message, tweets 1-2 are already posted when the TypeError is thrown. Also, post() calls validatePostOptions() but this path never does, so bad images (not an array, or non-Uint8Array data) are found only when the upload fails partway through the thread.
Suggest validating every entry (message plus validatePostOptions({ images })) in a loop before posting anything. The same applies to the Bluesky and Mastodon postThread() implementations, and ideally to Client.postThread() as well.
| let previousPost; | ||
|
|
||
| for (const entry of entries) { | ||
| if (!entry.message) { |
There was a problem hiding this comment.
Same issue as in twitter.js: validation happens mid-loop, after earlier entries are already posted, and validatePostOptions() (which post() calls) is skipped for thread entries. Suggest validating all entries up front. The reply refs themselves look right: root = first response, parent = previous response, both {uri, cid}.
| let previousStatusId; | ||
|
|
||
| for (const entry of entries) { | ||
| if (!entry.message) { |
There was a problem hiding this comment.
Same issue as in twitter.js. Also, the refactor moved the image validation into post() only, so postThread() passes images to #postStatus() without any checks. Moving that validation into a shared helper, or into #postStatus(), and running it for every entry before the first status is posted would fix both.
| name: "Strategy", | ||
| id: "strategy1", | ||
| postThread() { | ||
| return Promise.resolve({ id: "123" }); |
There was a problem hiding this comment.
This mock returns a single object, but real postThread() implementations (and the fallback) return an array. That's why this test passes even though Client.postThread() rejects with the real Twitter, Bluesky, and Mastodon getUrlFromResponse() (see the comment on src/client.js). Please make the mock return an array.
There are also no postThread() tests in tests/strategies/{twitter,bluesky,mastodon}.test.js. They should check that request 2+ carries reply.in_reply_to_tweet_id, in_reply_to_id, and record.reply.root/parent with the right ids, and cover a failure partway through a thread.
…stodon) Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
- Add src/util/threads.js with ThreadError plus helpers to validate, post, join, and split thread entries - Validate every entry before posting anything - Throw a ThreadError with the published responses when a thread stops partway through, and expose their URLs on FailureResponse - Post threads natively on Nostr (NIP-10), Slack (thread_ts), Telegram (reply_parameters), and Discord bot (message_reference), in addition to Twitter, Bluesky, and Mastodon - Post the thread as a single message on services that can't reply (LinkedIn, Dev.to, Discord webhook) instead of separate posts - Return the URL of every post from Client.postThread(), and don't let a getUrlFromResponse() error reject the call - Add --thread to the CLI (one post per argument, or --- separators) - Add a post-thread-to-social-media tool to the MCP server - Document threads in the README Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NkESLBzNRvDYxNgSisFeZ
52ae9fc to
4f851d2
Compare
Adds thread posting, where each message replies to the previous one, to the library, the CLI, and the MCP server.
Behavior
Client.postThread(entries, { signal })posts to every configured service:reply.in_reply_to_tweet_idrecord.replywithrootandparentrefsin_reply_to_idetags (root/reply)reply_parameters(an entry's images reply to its text)message_referencethread_tsof the first messageTypeErrorand nothing is posted.ThreadErrorwhoseresponseshold the posts already published.Client.postThread()returns that as aFailureResponsewithurlsfor those posts, so the partial thread can be linked to or cleaned up.SuccessResponse.responseis an array with one response per post,urlis the first post's URL, andurlshas every URL. An error fromgetUrlFromResponse()no longer rejects the whole call.ThreadErroris exported from the package.CLI
An
--imageor--image-urlimage is attached to the first post. If a thread stops partway through, the CLI prints the URLs of the posts that were published.MCP
A new
post-thread-to-social-mediatool takesmessagesand an optionalstrategyIds. Its description lists which services post real replies.Changes since the original draft
main. The unrelatedpackage-lock.jsonchurn and the removal ofedit_history_tweet_idsare gone. That field is now optional because the X client library doesn't type it.Client.postThread()rejecting for every real strategy. It had passed the array of responses togetUrlFromResponse().src/util/threads.js.Testing
npm testpasses: 402 passing, 2 pending (the same 2 asmain).ThreadErrorwith the published posts.Client.postThread(), the MCP tool, and the CLI--threaderrors.Threads have not been posted to the real services, only against mocked APIs.
🤖 Generated with Claude Code
https://claude.ai/code/session_013NkESLBzNRvDYxNgSisFeZ