Commit Graph
8 Commits
Author SHA1 Message Date
Ahmed Ibrahim cc96a03f10 Fix stale model test fixtures (#18719)
Fixes stale test fixtures left after the active bundled model catalog
updates in #18586 and #18388. Those changes made `gpt-5.4` the current
default and removed several older hardcoded slugs, which left Windows
Bazel shards failing TUI and config tests.

What changed:
- Refresh TUI model migration, availability NUX, plan-mode, status, and
snapshot fixtures to use active bundled model slugs.
- Update the config edit test expectation for the TOML-quoted
`"gpt-5.2"` migration key.
- Move the model catalog tests into
`codex-rs/tui/src/app/tests/model_catalog.rs` so touching them does not
trip the blob-size policy for `app.rs`.

Verification:
- CI Bazel/lint checks are expected to cover the affected test shards.
2026-04-20 21:52:30 +00:00
Ahmed Ibrahim 0f0ef094b6 Show default reasoning in /status (#18373)
- Shows the model catalog default reasoning effort when no reasoning
override is configured.
- Adds /status coverage for the empty-config fallback.
2026-04-17 12:21:09 -07:00
pakrym-oai dd1321d11b Spread AbsolutePathBuf (#17792)
Mechanical change to promote absolute paths through code.
2026-04-14 14:26:10 -07:00
Eric Traut 46ab9974dc Expose instruction sources (AGENTS.md) via app server (#17506)
Addresses #17498

Problem: The TUI derived /status instruction source paths from the local
client environment, which could show stale <none> output or incorrect
paths when connected to a remote app server.

Solution: Add an app-server v2 instructionSources snapshot to thread
start/resume/fork responses, default it to an empty list when older
servers omit it, and render TUI /status from that server-provided
session data.

Additional context: The app-server field is intentionally named
instructionSources rather than AGENTS.md-specific terminology because
the loaded instruction sources can include global instructions, project
AGENTS.md files, AGENTS.override.md, user-defined instruction files, and
future dynamic sources.
2026-04-12 15:50:12 -07:00
Shijie Rao 930e5adb7e Revert "Option to Notify Workspace Owner When Usage Limit is Reached" (#17391)
Reverts openai/codex#16969

#sev3-2026-04-10-accountscheckversion-500s-for-openai-workspace-7300
2026-04-10 23:33:13 +00:00
richardopenai 9f2a585153 Option to Notify Workspace Owner When Usage Limit is Reached (#16969)
## Summary
- Replace the manual `/notify-owner` flow with an inline confirmation
prompt when a usage-based workspace member hits a credits-depleted
limit.
- Fetch the current workspace role from the live ChatGPT
`accounts/check/v4-2023-04-27` endpoint so owner/member behavior matches
the desktop and web clients.
- Keep owner, member, and spend-cap messaging distinct so we only offer
the owner nudge when the workspace is actually out of credits.

## What Changed
- `backend-client`
- Added a typed fetch for the current account role from
`accounts/check`.
  - Mapped backend role values into a Rust workspace-role enum.
- `app-server` and protocol
  - Added `workspaceRole` to `account/read` and `account/updated`.
- Derived `isWorkspaceOwner` from the live role, with a fallback to the
cached token claim when the role fetch is unavailable.
- `tui`
  - Removed the explicit `/notify-owner` slash command.
- When a member is blocked because the workspace is out of credits, the
error now prompts:
- `Your workspace is out of credits. Request more from your workspace
owner? [y/N]`
  - Choosing `y` sends the existing owner-notification request.
- Choosing `n`, pressing `Esc`, or accepting the default selection
dismisses the prompt without sending anything.
- Selection popups now honor explicit item shortcuts, which is how the
`y` / `n` interaction is wired.

## Reviewer Notes
- The main behavior change is scoped to usage-based workspace members
whose workspace credits are depleted.
- Spend-cap reached should not show the owner-notification prompt.
- Owners and admins should continue to see `/usage` guidance instead of
the member prompt.
- The live role fetch is best-effort; if it fails, we fall back to the
existing token-derived ownership signal.

## Testing
- Manual verification
  - Workspace owner does not see the member prompt.
- Workspace member with depleted credits sees the confirmation prompt
and can send the nudge with `y`.
- Workspace member with spend cap reached does not see the
owner-notification prompt.

### Workspace member out of usage

https://github.com/user-attachments/assets/341ac396-eff4-4a7f-bf0c-60660becbea1

### Workspace owner
<img width="1728" height="1086" alt="Screenshot 2026-04-09 at 11 48
22 AM"
src="https://github.com/user-attachments/assets/06262a45-e3fc-4cc4-8326-1cbedad46ed6"
/>
2026-04-09 21:15:17 -07:00
Felipe CouryandClaude Opus 4.6 359e17a852 fix(tui): reduce startup and new-session latency (#17039)
## TL;DR

- Fetches account/rateLimits/read asynchronously so the TUI can continue
starting without waiting for the rate-limit response.
- Fixes the /status card so it no longer leaves a stale “refreshing
cached limits...” notice in terminal history.

## Problem

The TUI bootstrap path fetched account rate limits synchronously
(`account/rateLimits/read`) before the event loop started for
ChatGPT/OpenAI-authenticated startups. This added ~670 ms of blocking
latency in the measured hot-start case, even though rate-limit data is
not needed to render the initial UI or accept user input. The delay was
especially noticeable on hot starts where every other RPC
(`account/read`, `model/list`, `thread/start`) completed in under 70 ms
total.

Moving that fetch to the background also exposed a `/status` UI bug: the
status card is flattened into terminal scrollback when it is inserted. A
transient "refreshing limits in background..." line could not be cleared
later, because the async completion updated the retained `HistoryCell`,
not the already-written terminal history.

## Mental model

Before this change, `AppServerSession::bootstrap()` performed three
sequential RPCs: `account/read` → `model/list` →
`account/rateLimits/read`. The result of the third call was baked into
`AppServerBootstrap` and applied to the chat widget before the event
loop began.

After this change, `bootstrap()` only performs two RPCs (`account/read`
+ `model/list`), and rate-limit fetching is kicked off as an async
background task immediately after the first frame is scheduled. A new
enum, `RateLimitRefreshOrigin`, tags each fetch so the event handler
knows whether the result came from the startup prefetch or from a
user-initiated `/status` command; they have different completion
side-effects.

The `get_login_status()` helper (used outside the main app flow) was
also decoupled: it previously called the full `bootstrap()` just to
check auth mode, wasting model-list and rate-limit work. It now calls
the narrower `read_account()` directly.

For `/status`, this PR keeps the background refresh request but stops
printing transient refresh notices into status history when cached
limits are already available. If a refresh updates the cache, the next
`/status` command will render the new values.

## Non-goals

- This change does not alter the rate-limit data itself.
- This change does not introduce caching, retries, or staleness
management for rate limits.
- This change does not affect the `model/list` or `thread/start` RPCs;
they remain on the critical startup path.

## Tradeoffs

- **Stale-on-first-render**: The status bar will briefly show no
rate-limit info until the background fetch completes; observed
background fetches landed roughly in the 400-900 ms range after the UI
appeared. This is acceptable because the user cannot meaningfully act on
rate-limit data in the first fraction of a second.
- **Error silence on startup prefetch**: If the startup prefetch fails,
the error is logged but the UI is not notified (unlike `/status` refresh
failures, which go through the status-command completion path). This
avoids surfacing transient network errors as a startup blocker.
- **Static `/status` history**: `/status` output is terminal history,
not a live widget. The card now avoids progress-style language that
would appear stuck in scrollback; users can run `/status` again to see
newly cached values.
- **`account_auth_mode` field removed from `AppServerBootstrap`**: The
only consumer was `get_login_status()`, which no longer goes through
`bootstrap()`. The field was dead weight.

## Architecture

### New types

- `RateLimitRefreshOrigin` (in `app_event.rs`): A `Copy` enum
distinguishing `StartupPrefetch` from `StatusCommand { request_id }`.
Carried through `RefreshRateLimits` and `RateLimitsLoaded` events so the
handler applies the right completion behavior.

### Modified types

- `AppServerBootstrap`: Lost `account_auth_mode` and
`rate_limit_snapshots`; gained `requires_openai_auth: bool` (passed
through from the account response so the caller can decide whether to
fire the prefetch).

### Control flow

1. `bootstrap()` returns with `requires_openai_auth` and
`has_chatgpt_account`.
2. After scheduling the first frame, `App::run_inner` fires
`refresh_rate_limits(StartupPrefetch)` if both flags are true.
3. When `RateLimitsLoaded { StartupPrefetch, Ok(..) }` arrives,
snapshots are applied and a frame is scheduled to repaint the status
bar.
4. When `RateLimitsLoaded { StartupPrefetch, Err(..) }` arrives, the
error is logged and no UI update occurs.
5. `/status`-initiated refreshes continue to use `StatusCommand {
request_id }` and call `finish_status_rate_limit_refresh` on completion
(success or failure).
6. `/status` history cells with cached rate-limit rows no longer render
an additional "refreshing limits" notice; the async refresh updates the
cache for future status output.

### Extracted method

- `AppServerSession::read_account()`: Factored out of `bootstrap()` so
that `get_login_status()` can call it independently without triggering
model-list or rate-limit work.

## Observability

- The existing `tracing::warn!` for rate-limit fetch failures is
preserved for the startup path.
- No new metrics or spans are introduced. The startup-time improvement
is observable via the existing `ready` timestamp in TUI startup logs.

## Tests

- Existing tests in `status_command_tests.rs` are updated to match on
`RateLimitRefreshOrigin::StatusCommand { request_id }` instead of a bare
`request_id`.
- Focused `/status` tests now assert that status history avoids
transient refresh text, continues to request an async refresh, and uses
refreshed cached limits in future status output.
- No new tests are added for the startup prefetch path because it is a
fire-and-forget spawn with no observable side-effect other than the
widget state update, which is already covered by the
snapshot-application tests.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 22:16:09 -03:00
Eric Traut ae057e0bb9 Fix stale /status rate limits in active TUI sessions (#16201)
Fix stale weekly limit in `/status` (#16194): /status reused the
session’s cached rate-limit snapshot, so the weekly remaining limit
could stay frozen within an active session.

With this change, we now dynamically update the rate limits after status
is displayed.

I needed to delete a few low-value test cases from the chatWidget tests
because the test.rs file is really large, and the new tests in this PR
pushed us over the 512K mandated limit. I'm working on a separate PR to
refactor that test file.
2026-03-31 17:03:05 -06:00