Add Ambxst theme integration - #2556
Conversation
- Update app, server, web, desktop, and docs copy to use k3code - Adjust branded titles, prompts, and tests to match the new name
- Remove the T3 wordmark from the sidebar header - Replace the label with the new k3code brand text
- Switch window title bar config based on `hideWindowControls` - Remove separate macOS button visibility sync
- Extract platform titlebar options into a shared helper - Relaunch Electron when `hideWindowControls` changes - Clarify in settings UI that the change applies after restart
github-actions
Bot
added
vouch:unvouched
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eff64d4. Configure here.
| const LINUX_WM_CLASS = isDevelopment ? "t3code-dev" : "t3code"; | ||
| const USER_DATA_DIR_NAME = isDevelopment ? "t3code-dev" : "t3code"; | ||
| const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code (Dev)" : "T3 Code (Alpha)"; | ||
| const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "k3code (Dev)" : "k3code (Alpha)"; |
There was a problem hiding this comment.
Legacy profile path changed
High Severity
The change to LEGACY_USER_DATA_DIR_NAME from T3 Code (Alpha) to k3code (Alpha) means existing desktop installs will no longer find and reuse their Chromium profile data, like localStorage and cookies, after an update.
Reviewed by Cursor Bugbot for commit eff64d4. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function unsupportedTextGeneration(operation: string) { | ||
| return new TextGenerationError({ | ||
| operation, | ||
| detail: "Pi does not support git text generation in this build.", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟢 Low Drivers/PiDriver.ts:69
The unsupportedTextGeneration function returns an error message stating "Pi does not support git text generation" but is also used for generateThreadTitle, which is not a git-related operation. When generateThreadTitle fails, users will see a misleading error message incorrectly blaming git.
| function unsupportedTextGeneration(operation: string) { | |
| return new TextGenerationError({ | |
| operation, | |
| detail: "Pi does not support git text generation in this build.", | |
| }); | |
| } | |
| function unsupportedTextGeneration(operation: string) { | |
| return new TextGenerationError({ | |
| operation, | |
| detail: "Pi does not support this text generation operation in this build.", | |
| }); | |
| } |
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Drivers/PiDriver.ts around lines 69-74:
The `unsupportedTextGeneration` function returns an error message stating "Pi does not support git text generation" but is also used for `generateThreadTitle`, which is not a git-related operation. When `generateThreadTitle` fails, users will see a misleading error message incorrectly blaming git.
Evidence trail:
apps/server/src/provider/Drivers/PiDriver.ts lines 69-73 (unsupportedTextGeneration function with detail 'Pi does not support git text generation in this build.'), line 352 (generateThreadTitle uses unsupportedTextGeneration), packages/contracts/src/git.ts lines 332-344 (TextGenerationError class with message format 'Text generation failed in ${this.operation}: ${this.detail}')
| } | ||
| } | ||
|
|
||
| async sendTurn(input: { |
There was a problem hiding this comment.
🟡 Medium src/piSdkManager.ts:1089
Two concurrent sendTurn calls for the same thread can both pass the turn-in-progress check at line 1098 before either sets context.currentTurn, allowing both to start session.prompt() calls. The second call then overwrites context.currentTurn at line 1151, orphaning the first turn — its completion callbacks return early at line 907 without emitting a turn.completed event, leaving the turn permanently incomplete. Consider acquiring a lock before the check or using an atomic compare-and-swap to set context.currentTurn.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/piSdkManager.ts around line 1089:
Two concurrent `sendTurn` calls for the same thread can both pass the turn-in-progress check at line 1098 before either sets `context.currentTurn`, allowing both to start `session.prompt()` calls. The second call then overwrites `context.currentTurn` at line 1151, orphaning the first turn — its completion callbacks return early at line 907 without emitting a `turn.completed` event, leaving the turn permanently incomplete. Consider acquiring a lock before the check or using an atomic compare-and-swap to set `context.currentTurn`.
Evidence trail:
apps/server/src/piSdkManager.ts line 1098: TOCTOU check `if (context.currentTurn && !context.currentTurn.completed)`. Lines 1122 (`await context.session.setModel`) and 1137 (`await this.materializePiImages`) are await points between check and assignment at line 1151 (`context.currentTurn = pendingTurn`). Lines 906-907 (`finalizePromptSuccess`): `if (context.currentTurn !== turn || turn.completed) { return; }` — orphaned turn silently returns. Line 939 (`finalizePromptFailure`): `if (context.currentTurn !== turn) { return; }` — same early return. Commit: REVIEWED_COMMIT.
| getAmbxstTheme: () => ipcRenderer.invoke(GET_AMBXST_THEME_CHANNEL), | ||
| onAmbxstTheme: (listener) => { | ||
| const wrappedListener = (_event: Electron.IpcRendererEvent, snapshot: unknown) => { | ||
| if (typeof snapshot !== "object" && snapshot !== null) return; |
There was a problem hiding this comment.
🟠 High src/preload.ts:117
In onAmbxstTheme, the guard typeof snapshot !== "object" && snapshot !== null uses && instead of ||. Because typeof null === "object", a null value passes through to the listener instead of being filtered out. Change to || to match the pattern used in onSshPasswordPrompt and onUpdateState.
| if (typeof snapshot !== "object" && snapshot !== null) return; | |
| if (typeof snapshot !== "object" || snapshot === null) return; |
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/desktop/src/preload.ts around line 117:
In `onAmbxstTheme`, the guard `typeof snapshot !== "object" && snapshot !== null` uses `&&` instead of `||`. Because `typeof null === "object"`, a `null` value passes through to the listener instead of being filtered out. Change to `||` to match the pattern used in `onSshPasswordPrompt` and `onUpdateState`.
Evidence trail:
apps/desktop/src/preload.ts line 117: `if (typeof snapshot !== "object" && snapshot !== null) return;` — uses `&&`
apps/desktop/src/preload.ts line 101: `if (typeof request !== "object" || request === null) return;` — uses `||` (correct pattern)
apps/desktop/src/preload.ts line 152: `if (typeof state !== "object" || state === null) return;` — uses `||` (correct pattern)
Commit: REVIEWED_COMMIT
| const LINUX_WM_CLASS = isDevelopment ? "t3code-dev" : "t3code"; | ||
| const USER_DATA_DIR_NAME = isDevelopment ? "t3code-dev" : "t3code"; | ||
| const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code (Dev)" : "T3 Code (Alpha)"; | ||
| const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "k3code (Dev)" : "k3code (Alpha)"; |
There was a problem hiding this comment.
🔴 Critical src/main.ts:144
Renaming LEGACY_USER_DATA_DIR_NAME from "T3 Code (...)" to "k3code (...)" orphans existing user data. The resolveUserDataPath() function checks for the legacy directory to preserve existing Chromium profiles, but this change makes it look for a non-existent path. Users with old "T3 Code (...)" directories will have their data abandoned and the app will create new empty profiles instead.
-const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "k3code (Dev)" : "k3code (Alpha)";
+const LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "T3 Code (Dev)" : "T3 Code (Alpha)";🤖 Copy this AI Prompt to have your agent fix this:
In file apps/desktop/src/main.ts around line 144:
Renaming `LEGACY_USER_DATA_DIR_NAME` from `"T3 Code (...)"` to `"k3code (...)"` orphans existing user data. The `resolveUserDataPath()` function checks for the legacy directory to preserve existing Chromium profiles, but this change makes it look for a non-existent path. Users with old `"T3 Code (...)`" directories will have their data abandoned and the app will create new empty profiles instead.
Evidence trail:
1. Current code at apps/desktop/src/main.ts:144 — `LEGACY_USER_DATA_DIR_NAME = isDevelopment ? "k3code (Dev)" : "k3code (Alpha)"`
2. resolveUserDataPath() at apps/desktop/src/main.ts:1115-1130 — checks `FS.existsSync(legacyPath)`, returns legacyPath if found, else returns new path
3. Commit 4088b7ac3cddc1fc92ccf3f99e7a844c629ebad0 (March 9, 2026) — original introduction of LEGACY_USER_DATA_DIR_NAME with values `"T3 Code (Dev)"` / `"T3 Code (Alpha)"`
4. Commit e646b301909e99163b1d1306a48c134963eb8da6 (May 1, 2026) — renamed LEGACY_USER_DATA_DIR_NAME to `"k3code (Dev)"` / `"k3code (Alpha)"` as part of branding rename
5. apps/desktop/package.json diff in e646b301 — productName changed from `"T3 Code (Alpha)"` to `"k3code (Alpha)"` simultaneously, but `resolveUserDataPath()` was already overriding the default since 4088b7ac, so no user ever had a `"k3code (Alpha)"` directory
| function makePiAdapter( | ||
| manager: PiSdkManager, | ||
| events: PubSub.PubSub<ProviderRuntimeEvent>, | ||
| ): ProviderAdapterShape<ProviderAdapterError> { | ||
| const providerKind = DRIVER_KIND; | ||
| const providerSlug = "pi" as const; | ||
| const toRequestError = (method: string, cause: unknown) => | ||
| new ProviderAdapterRequestError({ | ||
| provider: providerKind, | ||
| method, | ||
| detail: formatCause(cause), | ||
| cause, | ||
| }); | ||
|
|
||
| const listener = (event: unknown) => { | ||
| void Effect.runFork(PubSub.publish(events, event as never)); | ||
| }; | ||
| manager.on("event", listener); | ||
|
|
||
| return { | ||
| provider: providerKind, | ||
| capabilities: { | ||
| sessionModelSwitch: "unsupported", | ||
| }, | ||
| startSession: (input) => | ||
| Effect.tryPromise({ | ||
| try: () => { | ||
| const modelOptions = toPiModelOptions(input.modelSelection?.options); | ||
| return manager.startSession({ | ||
| threadId: input.threadId, | ||
| provider: providerSlug, | ||
| ...(input.cwd ? { cwd: input.cwd } : {}), | ||
| ...(input.modelSelection?.model ? { model: input.modelSelection.model } : {}), | ||
| ...(modelOptions ? { modelOptions } : {}), | ||
| ...(input.resumeCursor ? { resumeCursor: input.resumeCursor } : {}), | ||
| runtimeMode: input.runtimeMode, | ||
| }); | ||
| }, | ||
| catch: (cause) => toRequestError("session/start", cause), | ||
| }), | ||
| sendTurn: (input) => | ||
| Effect.tryPromise({ | ||
| try: () => { | ||
| const modelOptions = toPiModelOptions(input.modelSelection?.options); | ||
| return manager.sendTurn({ | ||
| threadId: input.threadId, | ||
| ...(input.input ? { input: input.input } : {}), | ||
| ...(input.attachments ? { attachments: input.attachments } : {}), | ||
| ...(input.modelSelection?.model ? { model: input.modelSelection.model } : {}), | ||
| ...(modelOptions ? { modelOptions } : {}), | ||
| ...(input.interactionMode ? { interactionMode: input.interactionMode } : {}), | ||
| }); | ||
| }, | ||
| catch: (cause) => toRequestError("turn/start", cause), | ||
| }), | ||
| interruptTurn: (threadId, turnId) => | ||
| Effect.tryPromise({ | ||
| try: () => manager.interruptTurn(threadId, turnId), | ||
| catch: (cause) => toRequestError("turn/interrupt", cause), | ||
| }), | ||
| respondToRequest: (threadId, requestId, decision) => | ||
| Effect.tryPromise({ | ||
| try: () => manager.respondToRequest(threadId, requestId, decision), | ||
| catch: (cause) => toRequestError("request/respond", cause), | ||
| }), | ||
| respondToUserInput: (threadId, requestId, answers) => | ||
| Effect.tryPromise({ | ||
| try: () => manager.respondToUserInput(threadId, requestId, answers), | ||
| catch: (cause) => toRequestError("user-input/respond", cause), | ||
| }), | ||
| stopSession: (threadId) => | ||
| Effect.tryPromise({ | ||
| try: () => manager.stopSession(threadId), | ||
| catch: (cause) => toRequestError("session/stop", cause), | ||
| }), | ||
| listSessions: () => Effect.promise(() => manager.listSessions()), | ||
| hasSession: (threadId) => Effect.promise(() => manager.hasSession(threadId)), | ||
| readThread: (threadId) => | ||
| Effect.tryPromise({ | ||
| try: () => manager.readThread(threadId), | ||
| catch: (cause) => toRequestError("thread/read", cause), | ||
| }), | ||
| rollbackThread: (threadId, numTurns) => | ||
| Effect.tryPromise({ | ||
| try: () => manager.rollbackThread(threadId, numTurns), | ||
| catch: (cause) => toRequestError("thread/rollback", cause), | ||
| }), | ||
| stopAll: () => | ||
| Effect.tryPromise({ | ||
| try: () => manager.stopAll(), | ||
| catch: (cause) => toRequestError("provider/stopAll", cause), | ||
| }), | ||
| streamEvents: Stream.fromPubSub(events), |
There was a problem hiding this comment.
🟡 Medium Drivers/PiDriver.ts:219
The event listener registered at manager.on("event", listener) is never removed, so events emitted during stopAll() (like session.exited from disposeContext) are still processed by the listener. This forks fibers via Effect.runFork(PubSub.publish(...)) that may attempt to publish to the PubSub after PubSub.shutdown(events) has been called, causing a race condition where forked fibers fail attempting to publish to a shut down PubSub.
const listener = (event: unknown) => {
void Effect.runFork(PubSub.publish(events, event as never));
};
manager.on("event", listener);
return {
provider: providerKind,
capabilities: {
sessionModelSwitch: "unsupported",
},
startSession: (input) =>🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Drivers/PiDriver.ts around lines 219-311:
The event listener registered at `manager.on("event", listener)` is never removed, so events emitted during `stopAll()` (like `session.exited` from `disposeContext`) are still processed by the listener. This forks fibers via `Effect.runFork(PubSub.publish(...))` that may attempt to publish to the PubSub after `PubSub.shutdown(events)` has been called, causing a race condition where forked fibers fail attempting to publish to a shut down PubSub.
Evidence trail:
apps/server/src/provider/Drivers/PiDriver.ts lines 232-235 (listener registered, never removed); apps/server/src/provider/Drivers/PiDriver.ts lines 340-344 (finalizer: stopAll then PubSub.shutdown, no listener removal); apps/server/src/piSdkManager.ts stopAll() method (calls stopSession for each session); apps/server/src/piSdkManager.ts disposeContext() method (emits session.exited via emitRuntimeEvent when emitExit=true); apps/server/src/piSdkManager.ts emitRuntimeEvent() (calls this.emit('event', event)); git_grep for 'manager.off|manager.removeListener|removeAllListeners' in PiDriver.ts returned no results.
| return context.sessionRecord; | ||
| } catch (error) { | ||
| this.startingSessions.delete(input.threadId); | ||
| if (context && this.sessions.get(input.threadId) === context) { | ||
| this.sessions.delete(input.threadId); | ||
| } | ||
| if (previousContext && this.sessions.get(input.threadId) === undefined) { | ||
| this.sessions.set(input.threadId, previousContext); | ||
| } | ||
| context?.session.dispose(); | ||
| throw error; | ||
| } finally { | ||
| this.startingThreadIds.delete(input.threadId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟢 Low src/piSdkManager.ts:1073
If disposeContext(previousContext, ...) at lines 1067-1071 throws (for example, an event listener throws during emitRuntimeEvent), the catch block restores previousContext to the sessions map. However, previousContext is already in a partially-disposed state with stopping = true, pending approvals resolved, and maps modified. Subsequent operations on that thread then reference a corrupted context, leading to inconsistent state and potential runtime errors.
return context.sessionRecord;
} catch (error) {
this.startingSessions.delete(input.threadId);
- if (context && this.sessions.get(input.threadId) === context) {
- this.sessions.delete(input.threadId);
- }
if (previousContext && this.sessions.get(input.threadId) === undefined) {
- this.sessions.set(input.threadId, previousContext);
+ // previousContext is partially disposed; don't restore it
+ // Sessions map should remain empty for this threadId on failure
}
context?.session.dispose();
throw error;🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/piSdkManager.ts around lines 1073-1087:
If `disposeContext(previousContext, ...)` at lines 1067-1071 throws (for example, an event listener throws during `emitRuntimeEvent`), the catch block restores `previousContext` to the `sessions` map. However, `previousContext` is already in a partially-disposed state with `stopping = true`, pending approvals resolved, and maps modified. Subsequent operations on that thread then reference a corrupted context, leading to inconsistent state and potential runtime errors.
Evidence trail:
apps/server/src/piSdkManager.ts lines 1064-1087 (try/catch block with disposeContext call and catch-block restoration), lines 1232-1292 (disposeContext implementation showing mutations before throw points), lines 403-405 (emitRuntimeEvent calling this.emit which propagates listener errors), lines 418-427 (updateSession modifying sessionRecord).
ApprovabilityVerdict: Needs human review 4 blocking correctness issues found. This PR bundles multiple significant changes: a complete branding rename, a new Pi AI provider with ~2000 lines of implementation, Ambxst theme integration, and window control settings. Critical unresolved review comments identify a data loss bug where existing user profile directories will be orphaned, plus a logic bug in the IPC preload layer. The scope and identified issues warrant human review. You can customize Macroscope's approvability policy. Learn more. |
1 participant
What Changed
Why
UI Changes
Checklist
Note
High Risk
High risk due to a large new provider integration (
PiSdkManager+PiDriver) and new desktop IPC/theme plumbing that affects session runtime, approvals, and window/titlebar behavior.Overview
Renames product-facing strings from T3 Code to k3code across docs, marketing pages, desktop branding, server CLI/log output, and web UI.
Adds Ambxst theme integration: the desktop app reads/watches Ambxst palette/config files and exposes snapshots over new IPC channels (
desktop:get-ambxst-theme,desktop:ambxst-theme), while the web app adds anambxsttheme option that applies CSS variables, caches background/mode to avoid startup flash, and updates dynamictheme-color.Introduces a new client setting
hideWindowControlswith titlebar behavior extracted towindowTitleBar.ts; desktop syncs window appearance on settings change and relaunches when this flag toggles.Adds a new Pi provider backed by
@mariozechner/pi-coding-agent, including a locked-down harness (no extensions/skills/themes), approval-required tool gating, plan-mode handling with<proposed_plan>extraction, and registersPiDriverin the server plus corresponding web settings/model-picker metadata.Reviewed by Cursor Bugbot for commit eff64d4. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Ambxst theme integration and Pi provider support with k3code rebrand
AmbxstThemeMonitorin the desktop main process (ambxstTheme.ts) that watches~/.cache/ambxst/colors.jsonand~/.config/ambxst/config/theme.json, debounces changes, and broadcasts snapshots to all windows via IPCuseTheme(useTheme.ts) to support an'ambxst'theme option that applies CSS variables from the desktop snapshot, toggles dark mode, and caches mode/background to localStorage for pre-boot themingPiDriver,piHarness.ts,piSdkManager.ts) with model slug helpers, athinkingLeveloption, and registration in the built-in driver registryhideWindowControlsclient setting with platform-aware title bar logic inwindowTitleBar.ts, exposed as a toggle in General Settings; changing it triggers an app relaunch📊 Macroscope summarized eff64d4. 48 files reviewed, 11 issues evaluated, 4 issues filtered, 6 comments posted
🗂️ Filtered Issues
apps/desktop/src/main.ts — 1 comment posted, 2 evaluated, 1 filtered
{ titleBarOverlay }fromgetWindowTitleBarOptions()will always yieldundefinedbecause thegetWindowTitleBarOptionsfunction (shown in references) never returns atitleBarOverlayproperty - it only returnstitleBarStyleand optionallytrafficLightPosition. As a result, the conditiontypeof titleBarOverlay === "object"is always false andwindow.setTitleBarOverlay()is never called, making the title bar overlay sync logic dead code. [ Out of scope ]apps/marketing/src/layouts/Layout.astro — 0 comments posted, 1 evaluated, 1 filtered
titleanddescriptionon lines 8-9 were changed from"T3 Code"to"k3code", which appears to be a typo (tandkare adjacent on a QWERTY keyboard). The repository is namedt3code, the nav icon alt text on line 34 still says"T3", and the footer on line 52 still says"T3 Tools Inc". This causes the page<title>and meta description to display the incorrect brand name"k3code"instead of"T3 Code", affecting SEO and user-facing branding on the marketing site. [ Out of scope (triage) ]apps/marketing/src/pages/index.astro — 0 comments posted, 1 evaluated, 1 filtered
T3 Codetok3codein both the tagline (line 6) and the screenshot alt text (line 22). The repository is namedt3code(owned bypingdotgg/t3code), sok3codeappears to be a typo — likelytwas accidentally replaced withk. This causes the marketing page to display an incorrect product name to all visitors. [ Cross-file consolidated ]apps/server/src/piSdkManager.ts — 2 comments posted, 3 evaluated, 1 filtered
listSessions()only returns sessions fromthis.sessions, excluding sessions inthis.startingSessions. However,hasSession()(line 1294) checks both maps. This inconsistency means a caller can observehasSession(threadId) === truewhile that threadId is absent fromlistSessions()results, which could cause confusion or bugs in consumer code that expects these methods to be consistent. [ Failed validation ]