fix(terminal): opencode not getting right theme#1951
Conversation
|
@greptile |
Greptile SummaryThis PR fixes opencode and other termenv-based TUIs launching with the wrong light/dark colorscheme by propagating the app's effective theme as
Confidence Score: 5/5Safe to merge; the change is additive env-var injection at spawn time with no effect on existing env var semantics. Every code path that spawns a PTY now resolves the theme once and injects COLORFGBG before the process starts. The logic is covered by unit tests that verify both the correct values and the priority ordering over providerVars. The CSS hardcoding is a straightforward alignment of visual colors with the reported values. No files require special attention.
|
| Filename | Overview |
|---|---|
| src/main/core/pty/pty-env.ts | Adds EffectiveTheme type, colorFgBgFor, and withThemeColorFgBg helpers; extends buildAgentEnv and buildTerminalEnv to accept an optional theme that sets COLORFGBG. Logic is correct: theme is applied after providerVars so it is authoritative over provider-supplied overrides. |
| src/main/core/settings/effective-theme.ts | New helper resolves the app theme at spawn time: returns the explicit 'emlight'/'emdark' setting or falls back to Electron's nativeTheme.shouldUseDarkColors for system/unset themes. Clean and correct. |
| src/main/core/conversations/impl/local-conversation.ts | Resolves theme before spawning the agent PTY and passes it to both buildAgentEnv (sets COLORFGBG internally) and the outer withThemeColorFgBg wrapper, resulting in COLORFGBG being written twice for the same value. |
| src/main/core/terminals/impl/local-terminal-provider.ts | buildTerminalEnv() is correctly called without a theme option; withThemeColorFgBg applies the theme after merging taskEnvVars, ensuring theme beats any provider-supplied COLORFGBG. |
| src/main/core/pty/pty-env.test.ts | Adds two well-structured tests: one verifying light/dark COLORFGBG values and the absence of COLORFGBG when theme is omitted; one confirming theme is authoritative over providerVars. |
| src/renderer/index.css | Replaces CSS variable references for xterm-bg/xterm-fg with hardcoded hex values that match the COLORFGBG indices set in the PTY env, aligning the visual terminal background with the value reported to TUIs. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Spawn PTY / SSH session] --> B[resolveEffectiveTheme]
B --> C{theme setting}
C -->|'emlight' or 'emdark'| D[Return explicit theme]
C -->|'system' / unset| E[nativeTheme.shouldUseDarkColors]
E -->|true| F[emdark]
E -->|false| G[emlight]
D --> H[colorFgBgFor theme]
F --> H
G --> H
H -->|emdark → 15;0| I[COLORFGBG]
H -->|emlight → 0;15| I
I --> J[buildAgentEnv / buildTerminalEnv]
J --> K[withThemeColorFgBg wraps merged env]
K --> L[PTY spawned with correct COLORFGBG]
L --> M[termenv TUI detects light/dark correctly]
Reviews (2): Last reviewed commit: "fix(terminal): rely on color env for the..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| args: resolved.args, | ||
| cwd: resolved.cwd, | ||
| env: { ...buildTerminalEnv(), ...this.taskEnvVars }, | ||
| env: withThemeColorFgBg({ ...buildTerminalEnv({ theme }), ...this.taskEnvVars }, theme), |
There was a problem hiding this comment.
COLORFGBG ends up being set twice here. buildTerminalEnv({ theme }) already calls colorFgBgFor(theme) internally (pty-env.ts line 179), and then withThemeColorFgBg sets it again from the same value. The outer wrap only adds real value when this.taskEnvVars might carry its own COLORFGBG that should be overridden by the theme — if that's the intent, a comment explaining this would help; otherwise the inner theme option to buildTerminalEnv could be dropped.
| env: withThemeColorFgBg({ ...buildTerminalEnv({ theme }), ...this.taskEnvVars }, theme), | |
| env: withThemeColorFgBg({ ...buildTerminalEnv(), ...this.taskEnvVars }, theme), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/main/core/terminals/impl/local-terminal-provider.ts
Line: 146
Comment:
`COLORFGBG` ends up being set twice here. `buildTerminalEnv({ theme })` already calls `colorFgBgFor(theme)` internally (pty-env.ts line 179), and then `withThemeColorFgBg` sets it again from the same value. The outer wrap only adds real value when `this.taskEnvVars` might carry its own `COLORFGBG` that should be overridden by the theme — if that's the intent, a comment explaining this would help; otherwise the inner `theme` option to `buildTerminalEnv` could be dropped.
```suggestion
env: withThemeColorFgBg({ ...buildTerminalEnv(), ...this.taskEnvVars }, theme),
```
How can I resolve this? If you propose a fix, please make it concise.| }); | ||
| } | ||
|
|
||
| private primeTerminalColorDetection( | ||
| pty: Pty, | ||
| theme: Awaited<ReturnType<typeof resolveEffectiveTheme>> | ||
| ) { | ||
| const response = terminalColorQueryResponseFor(theme); | ||
| for (const delayMs of [0, 50, 150]) { | ||
| setTimeout(() => { | ||
| try { | ||
| pty.write(response); | ||
| } catch {} |
There was a problem hiding this comment.
primeTerminalColorDetection is duplicated character-for-character in both LocalConversationProvider and SshConversationProvider. Consider extracting it to a shared utility in pty-env.ts or a sibling helper module so the retry counts and delay values can't drift between the two implementations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/main/core/conversations/impl/local-conversation.ts
Line: 222-234
Comment:
`primeTerminalColorDetection` is duplicated character-for-character in both `LocalConversationProvider` and `SshConversationProvider`. Consider extracting it to a shared utility in `pty-env.ts` or a sibling helper module so the retry counts and delay values can't drift between the two implementations.
How can I resolve this? If you propose a fix, please make it concise.
|
@greptile |
1 participant
No description provided.