Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions apps/desktop/docs/EXPO_BUTTON.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Expo Button

The Expo button is an icon-only button in the TopBar that lets users run `npx expo run:ios --device` in a dedicated terminal tab. It appears only when an Expo project is detected in the workspace.

## Detection

The button queries `workspaces.detectExpo` with the current `worktreePath`. If no Expo project is found, the button is hidden entirely.

## States

The button uses the Expo chevron logo (`logo-type-a`) with color to communicate state:

| State | Color | Behavior |
|-------|-------|----------|
| Idle | `text-muted-foreground` | Click → starts build |
| Starting | `text-muted-foreground` + `opacity-50` | Disabled, waiting for session |
| Running | `text-green-500` | Build is active |
| Running + hover | `text-red-500` | Click → sends Ctrl+C to stop |

Tooltips provide textual context for each state.

## Terminal Session Management

On first click, the button creates a new terminal tab (named "Expo iOS") and runs the command via `createOrAttach` with `initialCommands`. Subsequent clicks reuse the same tab — if the tab still exists, it focuses it and re-runs the command (sending Ctrl+C first to kill any prior process).

Session tracking uses a ref (`sessionRef`) for imperative tab/pane access and a state (`activePaneId`) to reactively enable the stream subscription.

## Exit Detection

The button subscribes to `terminal.stream` for the active pane. When the PTY process emits an `exit` event (crash, shell exit, tab close), the button resets to idle.

The button also watches the tabs store — if the user closes the Expo tab, state resets to idle.

## Known Limitation: Child Process Exit

The terminal stream `exit` event fires when the **shell process** (bash/zsh) exits, not when a child command (`npx expo run:ios`) finishes or is interrupted. This means:

- **Covered**: Shell crash, PTY death, tab close → button resets to idle
- **Not covered**: User types Ctrl+C in the terminal, Expo command fails/finishes on its own → button stays green

The root cause is that the PTY layer only tracks the shell PID, not foreground child processes. The codebase does not implement OSC 133 (FinalTerm shell integration protocol), which could detect command completion via `\x1b]133;D` sequences. Adding OSC 133 support would require:

1. Shell init scripts that emit OSC 133 sequences (zsh/bash/fish)
2. Parsing OSC 133 in the terminal data pipeline (similar to existing OSC-7 CWD tracking in `headless-emulator.ts`)
3. Exposing a "command finished" signal to the renderer

This is a broader terminal infrastructure change not scoped to the Expo button. See also `plans/20260107-1107-terminal-persistence-dx-hardening.md` which identifies this same limitation for general command completion detection.

## Files

| File | Purpose |
|------|---------|
| `TopBar/ExpoButton.tsx` | Button component with state machine and stream subscription |
| `TopBar/index.tsx` | Mounts ExpoButton in the top bar |
| `lib/trpc/routers/workspaces/` | `detectExpo` procedure |
| `lib/trpc/routers/terminal/terminal.ts` | `createOrAttach`, `write`, `stream` procedures |
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { z } from "zod";
import { publicProcedure, router } from "../../..";
import { secureFs } from "../../changes/security/secure-fs";

export const createDetectExpoProcedures = () => {
return router({
detectExpo: publicProcedure
.input(z.object({ worktreePath: z.string() }))
.query(async ({ input }) => {
try {
const content = await secureFs.readFile(
input.worktreePath,
"package.json",
);
const packageJson = JSON.parse(content);
const hasExpo = !!(
packageJson.dependencies?.expo ||
packageJson.devDependencies?.expo
);
return { hasExpo };
} catch {
return { hasExpo: false };
}
Comment on lines +10 to +23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add contextual error logging instead of silent fallback.
Line 10-23 swallows file/JSON errors and returns false, which makes failures invisible and hard to diagnose. Please log with a prefixed message while keeping the fallback behavior.

🐛 Proposed fix
-			} catch {
-				return { hasExpo: false };
-			}
+			} catch (error) {
+				console.error(
+					"[workspaces/detectExpo] Failed to read or parse package.json",
+					error,
+				);
+				return { hasExpo: false };
+			}

As per coding guidelines: Never swallow errors silently; at minimum log them with context; Use prefixed console logging with pattern [domain/operation] message for all logging.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
const content = await secureFs.readFile(
input.worktreePath,
"package.json",
);
const packageJson = JSON.parse(content);
const hasExpo = !!(
packageJson.dependencies?.expo ||
packageJson.devDependencies?.expo
);
return { hasExpo };
} catch {
return { hasExpo: false };
}
try {
const content = await secureFs.readFile(
input.worktreePath,
"package.json",
);
const packageJson = JSON.parse(content);
const hasExpo = !!(
packageJson.dependencies?.expo ||
packageJson.devDependencies?.expo
);
return { hasExpo };
} catch (error) {
console.error(
"[workspaces/detectExpo] Failed to read or parse package.json",
error,
);
return { hasExpo: false };
}
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/detect-expo.ts`
around lines 10 - 23, The try/catch in the detect-expo logic currently swallows
all errors; update the catch to log the error with context before returning the
fallback. Specifically, in the procedure that calls
secureFs.readFile(input.worktreePath, "package.json") and parses it to compute
hasExpo, change the catch to accept the error (e) and call console.error with a
prefixed message like "[workspaces/detect-expo] failed to read/parse
package.json at <input.worktreePath>:" followed by the error, then continue to
return { hasExpo: false } as the fallback.

}),
});
};
3 changes: 3 additions & 0 deletions apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { mergeRouters } from "../..";
import { createBranchProcedures } from "./procedures/branch";
import { createCreateProcedures } from "./procedures/create";
import { createDeleteProcedures } from "./procedures/delete";
import { createDetectExpoProcedures } from "./procedures/detect-expo";
import { createGitStatusProcedures } from "./procedures/git-status";
import { createInitProcedures } from "./procedures/init";
import { createQueryProcedures } from "./procedures/query";
Expand All @@ -18,6 +19,7 @@ import { createStatusProcedures } from "./procedures/status";
* - git-status: refreshGitStatus, getGitHubStatus, getWorktreeInfo, getWorktreesByProject
* - status: reorder, update, setUnread
* - init: onInitProgress, retryInit, getInitProgress, getSetupCommands
* - detect-expo: detectExpo
*/
export const createWorkspacesRouter = () => {
return mergeRouters(
Expand All @@ -28,6 +30,7 @@ export const createWorkspacesRouter = () => {
createGitStatusProcedures(),
createStatusProcedures(),
createInitProcedures(),
createDetectExpoProcedures(),
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
import { toast } from "@superset/ui/sonner";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { useCallback, useEffect, useRef, useState } from "react";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { useTabsStore } from "renderer/stores/tabs/store";

function ExpoIcon({ className }: { className?: string }) {
return (
<svg
viewBox="0 0 24 22"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
<path
d="M11.39 8.269c.19-.277.397-.312.565-.312.168 0 .447.035.637.312 1.49 2.03 3.95 6.075 5.765 9.06 1.184 1.945 2.093 3.44 2.28 3.63.7.714 1.66.269 2.218-.541.549-.797.701-1.357.701-1.954 0-.407-7.958-15.087-8.759-16.309C14.027.98 13.775.683 12.457.683h-.988c-1.315 0-1.505.297-2.276 1.472C8.392 3.377.433 18.057.433 18.463c0 .598.153 1.158.703 1.955.558.81 1.518 1.255 2.218.54.186-.19 1.095-1.684 2.279-3.63 1.815-2.984 4.267-7.029 5.758-9.06z"
fill="currentColor"
/>
</svg>
);
}

interface ExpoButtonProps {
workspaceId: string;
worktreePath: string;
}

type ExpoState = "idle" | "starting" | "running";

const EXPO_COMMAND = "npx expo run:ios --device";

export function ExpoButton({ workspaceId, worktreePath }: ExpoButtonProps) {
const addTab = useTabsStore((state) => state.addTab);
const renameTab = useTabsStore((state) => state.renameTab);
const setActiveTab = useTabsStore((state) => state.setActiveTab);
const tabs = useTabsStore((state) => state.tabs);

const [expoState, setExpoState] = useState<ExpoState>("idle");
const [isHovered, setIsHovered] = useState(false);
const [activePaneId, setActivePaneId] = useState<string | null>(null);
const sessionRef = useRef<{ tabId: string; paneId: string } | null>(null);

const createOrAttach = electronTrpc.terminal.createOrAttach.useMutation({
onSuccess: () => setExpoState("running"),
onError: (error) => {
toast.error(`Failed to start Expo build: ${error.message}`);
setExpoState("idle");
sessionRef.current = null;
setActivePaneId(null);
},
});

const writeMutation = electronTrpc.terminal.write.useMutation({
onError: (error) => {
toast.error(`Terminal write failed: ${error.message}`);
},
});

const { data, isLoading } = electronTrpc.workspaces.detectExpo.useQuery({
worktreePath,
});

// Listen for terminal process exit to reset button state
electronTrpc.terminal.stream.useSubscription(activePaneId ?? "", {
enabled: !!activePaneId,
onData: (event) => {
if (event.type === "exit") {
setExpoState("idle");
}
},
});

// Reset state if the tracked tab is closed by the user
useEffect(() => {
if (!sessionRef.current) return;
const tabStillExists = tabs.some((t) => t.id === sessionRef.current?.tabId);
if (!tabStillExists) {
setExpoState("idle");
sessionRef.current = null;
setActivePaneId(null);
}
}, [tabs]);
Comment on lines +74 to +83
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's verify the file exists and check its content around lines 74-83
cat -n apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx | head -100

Repository: superset-sh/superset

Length of output: 4235


🏁 Script executed:

# Continue reading the file to see the rest of the handleStart function and any other relevant code
cat -n apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx | tail -n +100 | head -50

Repository: superset-sh/superset

Length of output: 1887


🏁 Script executed:

# Search for other places where workspace changes might be handled or state is cleared
rg "workspaceId|worktreePath" apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx -B2 -A2

Repository: superset-sh/superset

Length of output: 1080


🏁 Script executed:

# Check if there's any zustand store managing workspace state or similar cleanup patterns
rg "useEffect.*workspaceId|useEffect.*workspace" apps/desktop/src/renderer -t tsx -A 3 | head -50

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Check the tabs store to understand how tabs relate to workspaces
cat apps/desktop/src/renderer/stores/tabs/store.ts

Repository: superset-sh/superset

Length of output: 31456


🏁 Script executed:

# Check how handleStart uses workspaceId and if the cross-workspace issue is real
rg "setActiveTab" apps/desktop/src/renderer --type ts -B2 -A2 | head -60

Repository: superset-sh/superset

Length of output: 5773


🏁 Script executed:

# Verify the current state of the component to see if there are any other workspace-related resets
grep -n "workspaceId\|worktreePath" apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx

Repository: superset-sh/superset

Length of output: 424


🏁 Script executed:

# Check if there's any workspace change detection or cleanup elsewhere in the component
cat -n apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx | tail -60

Repository: superset-sh/superset

Length of output: 2325


🏁 Script executed:

# Verify the full list of dependencies in handleStart callback to see if workspaceId changes already trigger recreation
rg "const handleStart" apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx -A 30

Repository: superset-sh/superset

Length of output: 1057


Reset the session when the workspace changes to prevent cross‑workspace state reuse.

When a user switches workspaces, sessionRef and activePaneId retain references to the previous workspace's tab and pane. If the user clicks "Start" in the new workspace, handleStart will attempt to write to a stale paneId that may not exist or belong to a different workspace, causing terminal operations to fail unnecessarily.

Add a useEffect that clears the session state whenever workspaceId or worktreePath changes:

Proposed fix
 	useEffect(() => {
 		if (!sessionRef.current) return;
 		const tabStillExists = tabs.some((t) => t.id === sessionRef.current?.tabId);
 		if (!tabStillExists) {
 			setExpoState("idle");
 			sessionRef.current = null;
 			setActivePaneId(null);
 		}
 	}, [tabs]);

+	useEffect(() => {
+		sessionRef.current = null;
+		setActivePaneId(null);
+		setExpoState("idle");
+	}, [workspaceId, worktreePath]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Reset state if the tracked tab is closed by the user
useEffect(() => {
if (!sessionRef.current) return;
const tabStillExists = tabs.some((t) => t.id === sessionRef.current?.tabId);
if (!tabStillExists) {
setExpoState("idle");
sessionRef.current = null;
setActivePaneId(null);
}
}, [tabs]);
// Reset state if the tracked tab is closed by the user
useEffect(() => {
if (!sessionRef.current) return;
const tabStillExists = tabs.some((t) => t.id === sessionRef.current?.tabId);
if (!tabStillExists) {
setExpoState("idle");
sessionRef.current = null;
setActivePaneId(null);
}
}, [tabs]);
useEffect(() => {
sessionRef.current = null;
setActivePaneId(null);
setExpoState("idle");
}, [workspaceId, worktreePath]);
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx`
around lines 74 - 83, Add a new useEffect inside the ExpoButton component that
resets the session when the workspace context changes: if workspaceId or
worktreePath changes, call setExpoState("idle"), setActivePaneId(null), and set
sessionRef.current = null to avoid reusing stale pane/tab references from the
previous workspace (this prevents handleStart from writing to a stale paneId).
Place the effect alongside the existing effects in ExpoButton.tsx and depend on
[workspaceId, worktreePath] so it runs whenever the workspace changes.


const handleStart = useCallback(() => {
if (createOrAttach.isPending || writeMutation.isPending) return;

const session = sessionRef.current;

if (session) {
setActiveTab(workspaceId, session.tabId);
setExpoState("starting");
// \x03 = Ctrl+C (kill any running process), \x15 = Ctrl+U (clear partial input)
writeMutation.mutate(
{ paneId: session.paneId, data: `\x03\x15${EXPO_COMMAND}\n` },
{ onSuccess: () => setExpoState("running") },
);
Comment on lines +90 to +97
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and read the specified lines
if [ -f "apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx" ]; then
  echo "=== File content at lines 85-105 ===" 
  sed -n '85,105p' "apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx"
else
  echo "File not found, searching for ExpoButton.tsx"
  find . -name "ExpoButton.tsx" -type f
fi

Repository: superset-sh/superset

Length of output: 782


🏁 Script executed:

# Check the full context of the component to understand state management
wc -l apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx

Repository: superset-sh/superset

Length of output: 139


🏁 Script executed:

# Search for writeMutation definition to understand how it's used
rg -A 10 -B 5 "writeMutation" apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx

Repository: superset-sh/superset

Length of output: 2422


Reset state on write failure to avoid a stuck "Starting" state.

The current code (line 90-97) sets expoState to "starting" but only updates it to "running" on success. While writeMutation has a global onError handler that shows a toast, this does NOT reset the state. On write failure, expoState remains "starting" and the button stays disabled, requiring a reload to recover. Add an onError callback to reset the state:

Proposed fix
 			writeMutation.mutate(
 				{ paneId: session.paneId, data: `\x03\x15${EXPO_COMMAND}\n` },
-				{ onSuccess: () => setExpoState("running") },
+				{
+					onSuccess: () => setExpoState("running"),
+					onError: () => setExpoState("idle"),
+				},
 			);

Note: handleStop() has the same issue and should also include error handling.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (session) {
setActiveTab(workspaceId, session.tabId);
setExpoState("starting");
// \x03 = Ctrl+C (kill any running process), \x15 = Ctrl+U (clear partial input)
writeMutation.mutate(
{ paneId: session.paneId, data: `\x03\x15${EXPO_COMMAND}\n` },
{ onSuccess: () => setExpoState("running") },
);
if (session) {
setActiveTab(workspaceId, session.tabId);
setExpoState("starting");
// \x03 = Ctrl+C (kill any running process), \x15 = Ctrl+U (clear partial input)
writeMutation.mutate(
{ paneId: session.paneId, data: `\x03\x15${EXPO_COMMAND}\n` },
{
onSuccess: () => setExpoState("running"),
onError: () => setExpoState("idle"),
},
);
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/screens/main/components/TopBar/ExpoButton.tsx`
around lines 90 - 97, The Expo start flow sets setExpoState("starting") but only
flips to "running" in writeMutation.mutate's onSuccess, so on write failure the
UI is stuck; add an onError callback to the writeMutation.mutate call used when
starting Expo (the block that calls setActiveTab(workspaceId, session.tabId),
setExpoState("starting"), writeMutation.mutate({... data:
`\x03\x15${EXPO_COMMAND}\n` ...})) that resets the state (e.g.,
setExpoState("idle")) on error; likewise update the stop flow in handleStop() to
include an onError that resets expo state so the button isn't permanently
disabled after a failed write.

} else {
setExpoState("starting");
const { tabId, paneId } = addTab(workspaceId);
sessionRef.current = { tabId, paneId };
setActivePaneId(paneId);
createOrAttach.mutate({
paneId,
tabId,
workspaceId,
initialCommands: [EXPO_COMMAND],
});
renameTab(tabId, "Expo iOS");
}
}, [workspaceId, addTab, renameTab, setActiveTab, createOrAttach, writeMutation]);

const handleStop = useCallback(() => {
const session = sessionRef.current;
if (!session || writeMutation.isPending) return;

// \x03 = Ctrl+C — terminal driver sends SIGINT to the foreground process group
writeMutation.mutate(
{ paneId: session.paneId, data: "\x03" },
{ onSuccess: () => setExpoState("idle") },
);
}, [writeMutation]);

const handleClick = useCallback(() => {
if (expoState === "running" && isHovered) {
handleStop();
} else if (expoState === "idle") {
handleStart();
}
}, [expoState, isHovered, handleStart, handleStop]);

// Hide button if loading or no Expo detected
if (isLoading || !data?.hasExpo) {
return null;
}

const isDisabled = expoState === "starting" || writeMutation.isPending;
const showStop = expoState === "running" && isHovered;

let tooltipText: string;
if (isDisabled) {
tooltipText =
expoState === "starting" ? "Starting Expo..." : "Stopping Expo...";
} else if (showStop) {
tooltipText = "Stop Expo build";
} else if (expoState === "running") {
tooltipText = "Expo build running";
} else {
tooltipText = "Run on iOS Device";
}

return (
<div className="no-drag">
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
onClick={handleClick}
onMouseEnter={() => setIsHovered(true)}
onMouseLeave={() => setIsHovered(false)}
disabled={isDisabled}
className={cn(
"flex items-center justify-center size-6 rounded border",
"transition-colors duration-150 ease-out",
"focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring",
isDisabled && "opacity-50 pointer-events-none",
showStop
? "border-red-500/60 bg-red-500/10 text-red-500 hover:bg-red-500/20 hover:border-red-500"
: expoState === "running"
? "border-green-500/60 bg-green-500/10 text-green-500"
: "border-border/60 bg-secondary/50 text-muted-foreground hover:bg-secondary hover:border-border hover:text-foreground",
)}
>
<ExpoIcon className="size-2.5" />
</button>
</TooltipTrigger>
<TooltipContent>{tooltipText}</TooltipContent>
</Tooltip>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useParams } from "@tanstack/react-router";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { ExpoButton } from "./ExpoButton";
import { OpenInMenuButton } from "./OpenInMenuButton";
import { OrganizationDropdown } from "./OrganizationDropdown";
import { WindowControls } from "./WindowControls";
Expand All @@ -26,6 +27,12 @@ export function TopBar() {
<div className="flex-1" />

<div className="flex items-center gap-3 h-full pr-4 shrink-0">
{workspace?.worktreePath && workspace?.id && (
<ExpoButton
workspaceId={workspace.id}
worktreePath={workspace.worktreePath}
/>
)}
{workspace?.worktreePath && (
<OpenInMenuButton
worktreePath={workspace.worktreePath}
Expand Down
Loading