Skip to content

fix(usage): keep cached usage-limits commands local - #10812

Open
Lucenx9 wants to merge 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/local-usage-limits-command
Open

fix(usage): keep cached usage-limits commands local#10812
Lucenx9 wants to merge 1 commit into
pingdotgg:mainfrom
Lucenx9:fix/local-usage-limits-command

Conversation

@Lucenx9

@Lucenx9 Lucenx9 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What changed

T3's /usage-limits command can become an ordinary provider prompt when its catalog survives a reconnect but its external quota data does not.

Mark T3's command with optional source: "t3" metadata and use it in the web, desktop, and mobile composers. Existing threads keep the command local even when limits are unavailable. Mobile New Task keeps it out of the send queue.

Why

The cache intentionally omits external quota snapshots while retaining the provider catalog. Command ownership must survive that gap without inventing quota data or claiming a provider's own command with the same name.

The existing data-based check remains as a fallback for older servers. Old caches without the marker need one connection to an updated server; this change cannot distinguish them from native commands while offline.

flowchart LR
    catalog["Cached command: source=t3"] --> web["Web and desktop"]
    catalog --> thread["Mobile thread"]
    catalog --> draft["Mobile New Task"]
    web --> local["Handle locally"]
    thread --> local
    draft --> guard["Do not queue a turn"]
Loading

UI changes

Real web client in Chromium, with an isolated project and controlled quota responses. PNGs are 2560 × 1600. The before capture uses the original ownership check with the same fixture. Turn-start requests are recorded and stopped at the browser boundary, so no agent runs.

Before After
Before: the command becomes a user message and starts the sending UI After: the command stays in the composer and shows the local unavailable notice
Before video After video

Verification

  • 75 focused tests pass across contracts, shared usage helpers, the mobile command menu, and server config subscriptions.
  • The new catalog-decoding regression failed before the schema fix.
  • Browser checks cover typed and menu-selected commands without quota data, available external limits, and an unmarked native command with the identical label. Local commands dispatch zero turns; the native command still reaches the send boundary.
  • Typechecks pass for contracts, shared, web, and mobile. Targeted lint has no errors or warnings on changed lines. Formatting and git diff --check pass.
  • Fallow's single-thread audit passes with no newly introduced findings. No repository-wide checks ran.

Desktop shares the web composer. Mobile is covered by focused tests and typechecking, not a device run. No product defaults or diagnostic suppressions change.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for UI changes
  • I included videos for the interaction change

Model: GPT-5. Harness: Codex in T3 Code.

Summary by CodeRabbit

  • Bug Fixes

    • The /usage-limits command now remains available while usage-limit information reconnects.
    • Improved handling distinguishes locally available usage-limit commands from provider-provided commands.
    • Cached commands are preserved across provider status updates and older server data formats.
  • Reliability

    • Usage-limit command availability and provider command data are now handled more consistently across mobile and web experiences.

Copilot AI lite review requested due to automatic review settings September 8, 2026 17:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T17:27:32.254111Z 3e675e6 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 8, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 3e675e6

Macroscope's review found this PR approvable — This is a focused compatibility bug fix that preserves local /usage-limits ownership across cached provider catalogs while keeping native provider commands unchanged. The additive metadata and targeted tests keep the runtime impact limited to composer command routing.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 7713e8d3-3004-4085-bf7c-a5d922820183

📥 Commits

Reviewing files that changed from the base of the PR and between 12391bd and 3e675e6.

📒 Files selected for processing (9)
  • apps/mobile/src/features/threads/NewTaskDraftScreen.tsx
  • apps/mobile/src/features/threads/ThreadComposer.tsx
  • apps/mobile/src/features/threads/use-composer-command-menu.test.ts
  • apps/server/src/server.test.ts
  • apps/web/src/components/ChatView.tsx
  • packages/contracts/src/server.test.ts
  • packages/contracts/src/server.ts
  • packages/shared/src/usageLimits.test.ts
  • packages/shared/src/usageLimits.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Adds local ownership metadata for the usage-limits command, preserves it through provider decoding, supports legacy providers, and updates mobile and web availability checks.

Changes

Usage-limits command ownership

Layer / File(s) Summary
Ownership contract and detection
packages/contracts/src/server.ts, packages/contracts/src/server.test.ts, packages/shared/src/usageLimits.ts, packages/shared/src/usageLimits.test.ts
ServerProviderSlashCommand accepts source: "t3". hasLocalUsageLimitsCommand detects locally owned commands and falls back to legacy data-based detection. Tests cover decoding and ownership cases.
Server provider payloads
apps/server/src/server.test.ts
Provider status expectations include source: "t3" on injected usage-limits commands.
Client command availability
apps/mobile/src/features/threads/*, apps/mobile/src/features/threads/use-composer-command-menu.test.ts, apps/web/src/components/ChatView.tsx
Mobile and web use hasLocalUsageLimitsCommand with the selected provider status. Command-menu tests cover local and non-local commands.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3e675

This change keeps locally handled usage-limits commands available across reconnects while retaining compatibility with older server catalogs. The ownership contract, fallback behavior, and affected client flows are covered, with no concrete merge-blocking risk identified.

Suggested reviewers: juliusmarminge

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: keeping cached usage-limits commands local.
Description check ✅ Passed The description explains what changed, why it changed, the UI impact, verification results, and checklist status. It closely matches the required template and provides relevant testing limitations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants