Skip to content

fix(thread): show the thread when the channel lookup is rate limited - #94

Draft
roryist wants to merge 2 commits into
mainfrom
roryist/thread-view-rate-limit
Draft

roryist wants to merge 2 commits into
mainfrom
roryist/thread-view-rate-limit

Conversation

@roryist

@roryist roryist commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR

When Comms rate limits the channel lookup, tdc thread view now 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:

Output Before After
Text Fails with RATE_LIMITED Shows the thread, with [ch:CH100] in place of [General] new
--json / --ndjson Fails with RATE_LIMITED Shows the thread, with "channelName": null new
Any other error (404, 403, …) Fails Fails

And the hint on every rate-limit error:

Before After
Wait a moment, then retry Wait a minute or two before retrying. Retrying sooner keeps the limit in place, because rejected calls count against it too. new

Those are the only changes. Everything else is how it already worked.

I chose null for scripts because it says "unknown" without pretending the id is a name. The key is always there, and channelId already sits next to it for anyone who wants a fallback. Text output shows ch:<id>, the same fallback tdc inbox already 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

  • Thread view no longer acts as a brake. Before, a rate limit made thread views fail, which at least told the caller something was wrong. Now the views succeed, so a sweep has no reason to slow down, and its channel lookups keep getting 429s. Those are cheap for the backend (no database work), but the user's apps still can't load channels until the sweep ends. Stopping that is backend work, like giving each route its own budget, not this PR.
  • channelName can now be null in 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.
  • The hint applies to every 429, not just channel lookups. It lives in the shared error handling. In the logs, every CLI 429 came from the channel lookup. I haven't checked how other Comms limits count, but waiting a minute or two is still safe advice there, if longer than needed.
  • No automatic wait or retry. I left that out on purpose. The hint tells the caller what to do. Once the backend sends a Retry-After header, 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 inbox fetch channel names in one call instead of one call per channel. comms-backend#1155 sends Retry-After on 429s. The SDK doesn't expose response headers yet, so the CLI can't read that header until it does.

References

🤖 Generated with Claude Code

(Written by Rory's agent)

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>
@roryist roryist self-assigned this Oct 23, 2026
@roryist

roryist commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

@doistbot /review

@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.

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 viewThread path calls assertChannelIsPublic before getChannelName, so a 429 on that separate channels.getChannels call still aborts the view before the new guard kicks in — the tests mock assertChannelIsPublic, so this path isn't exercised. Worth handling its rate limit the same way so both default and --json views 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)
  • P3 src/commands/thread/view.ts:54: This introduces a second channel-fallback prefix: channel:CH100 here, while src/commands/inbox.ts:152 already falls back with ch:${thread.channelId}. Consumers parsing either command's output would have to handle both spellings. Pick one prefix (probably ch:, since it shipped first) or reuse a shared helper so the fallback format stays consistent.
  • P3 src/commands/thread/thread.test.ts:882: This covers --json, but the PR also changes --ndjson thread output so the first line can carry channelName: null when 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 }.

Share Feedback • Review Logs

const channel = await client.channels.getChannel(channelId)
return channel.name
} catch (error) {
if (isCliErrorCode(error, 'RATE_LIMITED')) return null

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.

P2 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants