Repository navigation
Conversation
Channel lookups are the only call in `thread view` with a per-user rate limit, so a bulk read of threads hits it first. The thread and comments have already loaded by then, and the channel name is only a label. On a 429, thread view now shows the channel id instead of failing, and --json / --ndjson output sets channelName to null. Any other error still fails as before. The 429 hint now says to wait a minute or two, and that retrying sooner keeps the limit in place. The old "Wait a moment" read as "retry now". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@doistbot /review |
doistbot
left a comment
There was a problem hiding this comment.
Nice fix — when the channel lookup hits a rate limit, tdc thread view now still renders the thread (with channel:<id> in text, null in JSON) instead of failing outright, and the 429 hint now steers agents away from immediate retries.
Few things worth tightening:
- The default
viewThreadpath callsassertChannelIsPublicbeforegetChannelName, so a 429 on that separatechannels.getChannelscall still aborts the view before the new guard kicks in — the tests mockassertChannelIsPublic, so this path isn't exercised. Worth handling its rate limit the same way so both default and--jsonviews actually survive the limit. - Minor: the updated hint is a good step, though agents may still retry instantly if nothing discourages it — the degradation itself helps a lot here.
I also included a few optional follow-up notes in the details below.
Optional follow-up notes (2)
src/commands/thread/view.ts:54: This introduces a second channel-fallback prefix:
channel:CH100here, whilesrc/commands/inbox.ts:152already falls back withch:${thread.channelId}. Consumers parsing either command's output would have to handle both spellings. Pick one prefix (probablych:, since it shipped first) or reuse a shared helper so the fallback format stays consistent.src/commands/thread/thread.test.ts:882: This covers
--json, but the PR also changes--ndjsonthread output so the first line can carrychannelName: nullwhen the channel lookup is rate limited. That serialization contract is currently untested, so a regression in the ndjson branch would pass the suite. Add a test that parses the first ndjson line and asserts{ type: 'thread', channelName: null }.
| const channel = await client.channels.getChannel(channelId) | ||
| return channel.name | ||
| } catch (error) { | ||
| if (isCliErrorCode(error, 'RATE_LIMITED')) return null |
There was a problem hiding this comment.
This catch only tolerates
RATE_LIMITED from getChannel, but the default viewThread path calls assertChannelIsPublic(thread.channelId, thread.workspaceId) before reaching getChannelName. That helper issues a separate channels.getChannels call through the same wrapped client, so a 429 there still throws RATE_LIMITED and aborts the view before this guard runs. The new tests mock assertChannelIsPublic, so that path isn't exercised. Handle the public-channel lookup's rate limit the same way (or otherwise keep it from making an unguarded channel call) so the default and --json views actually survive the rate limit.
The inbox already shows `ch:<id>` for a channel it can't name, so thread view now uses the same prefix instead of adding `channel:<id>`. Also covers the --ndjson shape when the channel lookup is rate limited. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TL;DR
When Comms rate limits the channel lookup,
tdc thread viewnow shows the thread and its comments, with the channel id where the name would be. Any other error still fails as before. The rate-limit error also now says to wait a minute or two, not "a moment".Why
Showing a thread takes four calls to Comms: the thread, its comments, the workspace's people, and the channel. Only the channel call has a per-user rate limit. In practice it's about 100 calls a minute.
On 23 October an agent read about 1,000 threads in two minutes. The channel lookups hit the limit, and every view after that failed. The thread and its comments had loaded fine. All that was missing was the channel's name.
The error then said "Wait a moment, then retry". An agent reads that as "retry now". Comms counts rejected calls against the limit too, so retrying straight away keeps the user locked out. The limit is per user, so while it lasts, that person's web app can't load channels either.
What changes
When the channel lookup gets a 429:
RATE_LIMITED[ch:CH100]in place of[General]new--json/--ndjsonRATE_LIMITED"channelName": nullnewAnd the hint on every rate-limit error:
Those are the only changes. Everything else is how it already worked.
I chose
nullfor scripts because it says "unknown" without pretending the id is a name. The key is always there, andchannelIdalready sits next to it for anyone who wants a fallback. Text output showsch:<id>, the same fallbacktdc inboxalready uses for a channel it can't name.Only a rate limit is skipped over. A 404 or 403 means something is actually wrong, and hiding that would be worse than failing.
The private-channel check that runs before the name lookup still fails on any error, a 429 included. If it can't run, the CLI can't tell whether the thread is private, so carrying on could show a private thread. It also uses a different call that has no per-user limit, so it isn't the one these 429s come from.
The hint says "a minute or two" rather than "a minute". Comms adds up the current minute and the whole previous minute, so after a burst it can take up to two minutes before calls get through again.
No new metrics: the CLI has no metrics pipeline.
Four things to look at
channelNamecan now benullin JSON. Before, it was always a string or the command failed. A script that treats it as a string without checking could break. I think that's rare, and better than losing the whole thread.Retry-Afterheader, the SDK could pass it on and the hint could give the exact wait.How it relates to other PRs
It stands alone. It's one of three fixes from the same investigation. comms-cli#93 makes
tdc inboxfetch channel names in one call instead of one call per channel. comms-backend#1155 sendsRetry-Afteron 429s. The SDK doesn't expose response headers yet, so the CLI can't read that header until it does.References
api.tsatdba87ddview.tsatdba87ddrate_limiter.py🤖 Generated with Claude Code
(Written by Rory's agent)