Repository navigation
cli: render the first frame before the terminal probe and keychain answer - #1585
Merged
Merged
Conversation
…swer Startup awaited two IPC-bound operations ahead of the renderer: 1. detectTerminalTheme() — a GLOBAL_OSC_TIMEOUT_MS (2s) bounded pair of OSC 10/11 queries over stdin. Terminals that swallow OSC queries (misconfigured tmux, some SSH muxes) burned the whole 2s before a single frame painted; even responsive terminals paid tens-to-hundreds of ms for the round trips. 2. loadStoredAuthToken() — an OS keychain round trip, ahead of parseArgs/registry/renderer work that never reads the token. The OSC probe now starts in the background, still BEFORE createCliRenderer() so its stdin data listener is attached first and the reply is consumed as the probe answer rather than arriving as stray keystrokes at the focused input. The first frame resolves the theme from the synchronous evidence instead — the same signals the awaited path already fell back to (TERM_BACKGROUND, COLORFGBG, and the per-terminal Apple-Terminal/xterm hints, now gated on "no query will be sent" so a query answer can never be pre-empted by a hint it supersedes). When the answer lands it is recorded into the OSC slot and applied through the existing live re-theme path (applyOscDetectedTheme → recomputeSystemTheme, the same update path the theme file watchers and SIGUSR2 use), which the theme detector priority already reads ahead of OS detection. Settled state is identical to the old ordering whenever the terminal answered the query, because first-consumed ordering is preserved rather than timing: the OSC slot is only ever consumed through the resolver, whose detector order is unchanged (env override → IDE → OSC → OS), and the slot wins over the env evidence the moment it is set. A settle that races initializeApp just stores the value and the store's initial detectSystemTheme() picks it up through that same order. Probe semantics are untouched: same two queries, same per-query and global timeouts, same swallow→null outcome (a fallback await before renderer creation restores the old pre-paint wait for the cases with no work to overlap, still bounded by the same deadline). The keychain read is overlapped with parseArgs, the agent/skill registries and renderer-library staging, and awaited only at the points that actually publish a decision: the login/publish command branches, the launch/crash analytics rows (which ship without a retry, so they must not record a sign-in state the keychain read would overturn a tick later), and a final fallback before renderer creation. Every remaining token consumer (AppWithAsyncAuth's effect, useAuthQuery, useAuthState, the agent run itself) reads synchronously post-paint or waits on the user, so the token is in the credentials cache before anyone dereferences it and the first paint never waits on the keychain. Also on the resume cold path: getMostRecentChatDir statted every chats/ entry twice (mtime in the map, then isDirectory() again inside the filter's type guard). It now reads dirents once and stats each directory exactly once for the mtime — same skip-on-failure and missing-dir outcomes, half the syscalls on the --continue scan that runs inside the first effect after the resume paint. Coverage: terminal-color-detection gains env/fallback/gating tests for the new sync evidence helper and the async settle path, and a new use-theme-osc-order suite pins the user-visible ordering contract — env value resolvable on the first frame, pre-init OSC answers picked up by the initial resolve, post-paint answers applied through the live re-theme path, and same-theme settles being identity-preserving no-ops.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Startup awaited two IPC-bound operations before the renderer existed:
detectTerminalTheme()— a pair of OSC 10/11 queries bounded by a 2 s global timeout. Terminals that swallow OSC queries (misconfigured tmux, some SSH muxes) burned the whole 2 s before a single frame painted; even responsive terminals paid tens-to-hundreds of ms.loadStoredAuthToken()— an OS keychain round trip, ahead ofparseArgs/registry/renderer work that never reads the token.Change
Theme: the OSC probe now starts in the background — still before
createCliRenderer(), so its stdin data listener attaches first and the reply is consumed as the probe answer rather than stray keystrokes. The first frame resolves from the synchronous evidence (COLORFGBG,TERM_BACKGROUND, per-terminal hints, gated on "no query was sent" so a hint can never pre-empt a real answer). When the answer lands it is applied through the existing live re-theme path the theme file watchers and SIGUSR2 use. Settled state is identical to the old ordering whenever the terminal answers: the OSC slot wins over the env evidence by the same detector priority, and probe semantics (two queries, per-query + global timeouts, swallow→null) are untouched. A fallback await before renderer creation restores the old wait only when there was nothing to overlap, still deadline-bounded.Keychain: overlapped with
parseArgs, the agent/skill registries, and renderer staging; awaited only where a decision is actually published (login/publish branches, launch/crash analytics rows) plus a bounded fallback pre-renderer. Every other consumer reads post-paint.Resume path bonus:
getMostRecentChatDirstatted every chat dir twice (mtime, thenisDirectory()in a filter type-guard) — now onewithFileTypespass, one stat per dir, half the syscalls on the--continuescan.Tests
New coverage pins the ordering contract: env value resolvable on the first frame, pre-init OSC answers picked up by the initial resolve, post-paint answers applied via live re-theme, same-theme settles are no-ops. Full cli suite: 4343 pass / 41 skip / 0 fail, typecheck clean.