Add preferred Pi model settings and pickers

[jlmcmich]

Summary

• add Pi settings for favorite models and a configurable default model
• use those preferences in the splash screen and chat composer model pickers
• fix provider switching so Pi falls back to Pi defaults instead of reusing another provider model slug

Validation

• bun fmt
• bun typecheck
• bun run test src/modelSelection.test.ts src/composerDraftStore.model.test.ts src/composerDraftStore.test.ts (from apps/web)

Notes

• bun run test --filter=@workbench/web still hits an unrelated existing failure in apps/web/src/components/chat/MessagesTimeline.test.tsx

---

Note

Medium Risk
Moderate risk because it changes CLI/package identifiers, filesystem base directories, environment variable names, and Electron protocol/userData handling, which can affect upgrades and runtime startup across platforms.

Overview
Rebrands the product from T3 Code to Workbench across docs, marketing, release workflow metadata, package names, and user-facing strings.

Renames the server CLI/package from t3 to workbench, updates build/release filters, and adds backward-compatible config/env handling (e.g., WORKBENCH_* with fallback to legacy T3CODE_*, default base dir migration from ~/.t3 to ~/.workbench).

Updates the desktop app identity (bundle id, protocol scheme workbench://, Linux desktop entry/userData dir naming) while keeping legacy support (t3:// scheme registration and legacy userData dir detection), and improves dev DX by starting a apps/server bundle watcher alongside Electron dev. Also adds a new /.well-known/workbench/environment endpoint alongside the legacy /.well-known/t3/environment, and extends git text-generation provider typing to include "pi".

^{Reviewed by Cursor Bugbot for commit f0912ed. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Add 'pi' provider settings, model pickers, and Console rail to Workbench

• Introduces a new pi provider throughout the stack: contracts (PiSettings, PiModelOptions, PiModelSelection), server adapter (PiAdapter.ts) that spawns the pi CLI in JSON mode to stream runtime events, and a PiProvider.ts that checks install/auth/version status and lists models.
• Adds settings UI for pi in SettingsPanels.tsx: a PiLoginPanel that fetches backends from /api/provider/pi/backends, triggers terminal login via /api/provider/pi/login, and a preferred-models picker with favorite/default model controls.
• Replaces PlanSidebar in ChatView.tsx with a resizable ConsoleRail that renders Files, Recent Changes, and Tasks panes, supports per-pane collapse/visibility, workspace file preview/edit/diff, and selection-to-chat insertion.
• Renames the monorepo from @t3tools/* to @workbench/* across all packages, renames the server CLI binary from t3 to workbench, updates localStorage keys (with legacy fallback reads), and updates all branding strings, icons, and well-known paths (e.g. /.well-known/workbench/environment).
• Risk: localStorage key migration is one-shot and transparent, but NetService context identifier changed (@workbench/shared/Net/NetService), which could break any external code resolving the service by string key.

📊 Macroscope summarized f0912ed. 73 files reviewed, 7 issues evaluated, 0 issues filtered, 5 comments posted

🗂️ Filtered Issues

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c31ce3e0-d5ae-4074-8155-af0cd921ca7c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cursor]

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 f0912ed. Configure here.}

[cursor]

Legacy environment route layer likely never mounted

Medium Severity

The original serverEnvironmentRouteLayer export was split into two: a new serverEnvironmentRouteLayer (serving /.well-known/workbench/environment) and a new legacyServerEnvironmentRouteLayer (serving /.well-known/t3/environment). If the server composition code only references serverEnvironmentRouteLayer, the legacy route layer is exported but never mounted, silently breaking backward compatibility for any existing clients hitting the old /.well-known/t3/environment endpoint.

Additional Locations (1)

• apps/server/src/http.ts#L61-L70

 

^{Reviewed by Cursor Bugbot for commit f0912ed. Configure here.}

[jlmcmich]

Closing this because the branch should be reviewed in jlmcmich/workbench rather than upstream.

[macroscopeapp]

🟡 Medium lib/workspaceFileTree.ts:114

At line 114, changedPaths.has(entry.path) uses the original entry.path which may have a trailing slash (e.g., "workbench/"), but changedPaths contains artifact paths without trailing slashes. Entries for submodules or nested git repos will never be marked as changed even when their normalized path exists in changedPaths. The check should use normalizedPath instead.

-    const changed = changedPaths.has(entry.path);
+    const changed = changedPaths.has(normalizedPath);

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/web/src/lib/workspaceFileTree.ts around line 114:

At line 114, `changedPaths.has(entry.path)` uses the original `entry.path` which may have a trailing slash (e.g., `"workbench/"`), but `changedPaths` contains artifact paths without trailing slashes. Entries for submodules or nested git repos will never be marked as changed even when their normalized path exists in `changedPaths`. The check should use `normalizedPath` instead.

Evidence trail:
apps/web/src/lib/workspaceFileTree.ts lines 82-87 (comment explaining trailing slash case), lines 85-86 (hasTrailingSlash and normalizedPath logic), line 114 (changedPaths.has(entry.path) using entry.path instead of normalizedPath), apps/web/src/components/console/ConsoleRail.tsx lines 233-236 (changedPaths populated from artifacts without trailing slashes), apps/web/src/lib/workspaceFileTree.test.ts lines 32-45 (test confirming trailing-slash entries exist but no test combining trailing-slash entries with changedPaths lookup)

[macroscopeapp]

🟢 Low src/composerDraftStore.ts:75

removeItem only deletes the new key, so stale legacy data remains in localStorage. When a user clears drafts and then reopens the composer, getItem falls back to the legacy key and resurrects the old draft. Consider also removing LEGACY_COMPOSER_DRAFT_STORAGE_KEY in removeItem to prevent zombie data.

Suggested change

	removeItem: (name: string) => {
	localStorage.removeItem(name);
	},
	};
	removeItem: (name: string) => {
	localStorage.removeItem(name);
	+ if (name === COMPOSER_DRAFT_STORAGE_KEY) {
	+ localStorage.removeItem(LEGACY_COMPOSER_DRAFT_STORAGE_KEY);
	+ }
	},

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/web/src/composerDraftStore.ts around lines 75-78:

`removeItem` only deletes the new key, so stale legacy data remains in `localStorage`. When a user clears drafts and then reopens the composer, `getItem` falls back to the legacy key and resurrects the old draft. Consider also removing `LEGACY_COMPOSER_DRAFT_STORAGE_KEY` in `removeItem` to prevent zombie data.

Evidence trail:
apps/web/src/composerDraftStore.ts lines 60-78 (REVIEWED_COMMIT): `composerBaseStorage` IIFE defines:
- `getItem` (lines 65-71): fallback to `LEGACY_COMPOSER_DRAFT_STORAGE_KEY` when new key returns null
- `removeItem` (lines 75-77): only removes the passed `name`, not the legacy key

Constants at lines 44-46:
- `COMPOSER_DRAFT_STORAGE_KEY = "workbench:composer-drafts:v1"`
- `LEGACY_COMPOSER_DRAFT_STORAGE_KEY = "t3code:composer-drafts:v1"`

[macroscopeapp]

🟡 Medium provider/piRuntime.ts:43

Using child.once("exit", ...) returns the exit code before stdout and stderr streams finish draining, so the final chunks of output are lost in the returned PiCommandResult. The close event fires only after all stdio streams are fully closed. Consider changing "exit" to "close" to capture complete output.

+  const code = await new Promise<number>((resolve, reject) => {
+    child.once("error", reject);
+    child.once("close", (exitCode) => resolve(exitCode ?? 0));
+  });

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/server/src/provider/piRuntime.ts around lines 43-46:

Using `child.once("exit", ...)` returns the exit code before `stdout` and `stderr` streams finish draining, so the final chunks of output are lost in the returned `PiCommandResult`. The `close` event fires only after all stdio streams are fully closed. Consider changing `"exit"` to `"close"` to capture complete output.

Evidence trail:
apps/server/src/provider/piRuntime.ts lines 43-53 at REVIEWED_COMMIT - shows `child.once("exit", ...)` pattern followed by immediately returning the collected chunks. Node.js child_process documentation confirms that `exit` fires when the process terminates while `close` fires after all stdio streams are fully closed: https://nodejs.org/api/child_process.html#event-close

[macroscopeapp]

🟢 Low console/ConsoleRail.browser.tsx:435

Line 435 uses expect(page.getByLabelText("Edit file contents")).toBeDefined(), which always passes because page.getByLabelText() returns a Locator object that is always truthy regardless of whether the element exists in the DOM. This test cannot fail even when the textarea is missing. Consider using await expect(...).toBeVisible() or a direct DOM query like line 517 to verify actual element presence.

-    await vi.waitFor(() => {
-      expect(page.getByLabelText("Edit file contents")).toBeDefined();
-      expect(document.body.textContent ?? "").toContain("Save");
-      expect(document.body.textContent ?? "").toContain("Cancel");
-    });
+    await vi.waitFor(() => {
+      expect(document.querySelector('textarea[aria-label="Edit file contents"]')).not.toBeNull();
+      expect(document.body.textContent ?? "").toContain("Save");
+      expect(document.body.textContent ?? "").toContain("Cancel");
+    });

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/web/src/components/console/ConsoleRail.browser.tsx around lines 435-438:

Line 435 uses `expect(page.getByLabelText("Edit file contents")).toBeDefined()`, which always passes because `page.getByLabelText()` returns a Locator object that is always truthy regardless of whether the element exists in the DOM. This test cannot fail even when the textarea is missing. Consider using `await expect(...).toBeVisible()` or a direct DOM query like line 517 to verify actual element presence.

Evidence trail:
apps/web/src/components/console/ConsoleRail.browser.tsx lines 430-438 (showing the defective assertion at line 435: `expect(page.getByLabelText("Edit file contents")).toBeDefined();`), apps/web/src/components/console/ConsoleRail.browser.tsx lines 510-525 (showing correct approach at line 517: `expect(document.querySelector('textarea[aria-label="Edit file contents"]')).not.toBeNull();`), Playwright documentation confirms getByLabelText() returns a Locator object which is always truthy.

[macroscopeapp]

🟠 High components/NoActiveThreadState.tsx:162

LandingPillButton spreads {...props} after the computed className, so when callers pass className="justify-start" the default pill styles (h-10 rounded-full border...) are completely replaced instead of merged. The spread should come before the explicit className prop, or className should be omitted from the spread and merged explicitly with cn().

+function LandingPillButton(props: React.ComponentProps<typeof Button>) {
+  const { className, ...rest } = props;
   return (
     <Button
       size="sm"
       variant="ghost"
       className={cn(
         "h-10 rounded-full border border-border/60 bg-background/84 px-4 text-[15px] text-foreground shadow-[0_10px_30px_-24px_rgba(0,0,0,0.55)] backdrop-blur hover:bg-background/96",
-        props.className,
+        className,
       )}
-      {...props}
+      {...rest}
     />
   );
 }

🤖 Copy this AI Prompt to have your agent fix this:

In file apps/web/src/components/NoActiveThreadState.tsx around lines 162-174:

`LandingPillButton` spreads `{...props}` after the computed `className`, so when callers pass `className="justify-start"` the default pill styles (`h-10 rounded-full border...`) are completely replaced instead of merged. The spread should come before the explicit `className` prop, or `className` should be omitted from the spread and merged explicitly with `cn()`.

Evidence trail:
apps/web/src/components/NoActiveThreadState.tsx lines 162-176 at REVIEWED_COMMIT: Shows `LandingPillButton` component with `className={cn(..., props.className)}` followed by `{...props}` spread. In JSX, props spread after an explicit attribute will overwrite matching properties, so caller-provided className will replace the computed className.

[macroscopeapp]

Approvability

Verdict: Needs human review

3 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR.

^{You can customize Macroscope's approvability policy. Learn more.}