T3 Code Mobile [WIP]#2013
Conversation
|
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)
Comment |
github-actions
Bot
added
size:XXL
![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.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Fast mode option applied to all providers indiscriminately
- Added provider guards to the fast-mode handler in ThreadComposer so fastMode is only applied to claudeAgent and codex providers, matching the existing logic in NewTaskDraftScreen.
Or push these changes by commenting:
@cursor push fb5f02f846
Preview (fb5f02f846)
diff --git a/apps/mobile/src/features/threads/ThreadComposer.tsx b/apps/mobile/src/features/threads/ThreadComposer.tsx
--- a/apps/mobile/src/features/threads/ThreadComposer.tsx
+++ b/apps/mobile/src/features/threads/ThreadComposer.tsx
@@ -596,10 +596,15 @@
}
if (event.startsWith("options:fast-mode:")) {
const fastMode = event.endsWith(":on");
- const updated: ModelSelection = {
- ...currentModelSelection,
- options: { ...currentModelSelection.options, fastMode: fastMode || undefined },
- };
+ const updated: ModelSelection =
+ currentModelSelection.provider === "claudeAgent"
+ ? {
+ ...currentModelSelection,
+ options: { ...currentModelSelection.options, fastMode: fastMode || undefined },
+ }
+ : currentModelSelection.provider === "codex"
+ ? { ...currentModelSelection, options: { fastMode: fastMode || undefined } }
+ : currentModelSelection;
void props.onUpdateModelSelection(updated);
return;
}You can send follow-ups to the cloud agent here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 6 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. |
| const [status, setStatus] = useState<"loading" | "loaded" | "error">(() => | ||
| faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading", | ||
| ); |
There was a problem hiding this comment.
🟡 Medium components/ProjectFavicon.tsx:25
When faviconUrl changes due to prop updates (e.g., httpBaseUrl or workspaceRoot), status remains at its previous value because useState only initializes on mount. If the previous URL had finished loading (status === "loaded"), the new image renders immediately without showing the folder fallback during its load.
const [status, setStatus] = useState<"loading" | "loaded" | "error">(() =>
faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading",
);
+ useEffect(() => {
+ setStatus(
+ faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading",
+ );
+ }, [faviconUrl]);Also found in 1 other location(s)
apps/desktop/src/main.ts:1675
The
titleBarOverlay.symbolColoris set once at window creation time based onnativeTheme.shouldUseDarkColors, but is never updated when the theme changes. WhenSET_THEME_CHANNELhandler setsnativeTheme.themeSource, the titlebar symbol color on Windows/Linux will remain stale, potentially making the minimize/maximize/close buttons hard to see or invisible against the new theme background.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/components/ProjectFavicon.tsx around lines 25-27:
When `faviconUrl` changes due to prop updates (e.g., `httpBaseUrl` or `workspaceRoot`), `status` remains at its previous value because `useState` only initializes on mount. If the previous URL had finished loading (`status === "loaded"`), the new image renders immediately without showing the folder fallback during its load.
Evidence trail:
apps/mobile/src/components/ProjectFavicon.tsx at REVIEWED_COMMIT:
- Lines 19-22: faviconUrl derived from props
- Lines 24-26: useState initializer only runs on mount
- No useEffect to reset status when faviconUrl changes
- Line 28: showImage = faviconUrl && status === "loaded"
- Lines 42-43: folder fallback only shows when !showImage
Also found in 1 other location(s):
- apps/desktop/src/main.ts:1675 -- The `titleBarOverlay.symbolColor` is set once at window creation time based on `nativeTheme.shouldUseDarkColors`, but is never updated when the theme changes. When `SET_THEME_CHANNEL` handler sets `nativeTheme.themeSource`, the titlebar symbol color on Windows/Linux will remain stale, potentially making the minimize/maximize/close buttons hard to see or invisible against the new theme background.
|
Bugbot Autofix prepared fixes for both issues found in the latest run.
Or push these changes by commenting: Preview (07aefa635b)diff --git a/apps/desktop/src/updateState.test.ts b/apps/desktop/src/updateState.test.ts
--- a/apps/desktop/src/updateState.test.ts
+++ b/apps/desktop/src/updateState.test.ts
@@ -71,7 +71,7 @@
platform: "darwin",
appImage: undefined,
disabledByEnv: false,
- hasUpdateFeedConfig: true,
+ hasUpdateFeedConfig: false,
}),
).toContain("packaged production builds");
});
diff --git a/apps/desktop/src/updateState.ts b/apps/desktop/src/updateState.ts
--- a/apps/desktop/src/updateState.ts
+++ b/apps/desktop/src/updateState.ts
@@ -36,12 +36,12 @@
disabledByEnv: boolean;
hasUpdateFeedConfig: boolean;
}): string | null {
+ if (args.isDevelopment || !args.isPackaged) {
+ return "Automatic updates are only available in packaged production builds.";
+ }
if (!args.hasUpdateFeedConfig) {
return "Automatic updates are not available because no update feed is configured.";
}
- if (args.isDevelopment || !args.isPackaged) {
- return "Automatic updates are only available in packaged production builds.";
- }
if (args.disabledByEnv) {
return "Automatic updates are disabled by the T3CODE_DISABLE_AUTO_UPDATE setting.";
}
diff --git a/apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx b/apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx
deleted file mode 100644
--- a/apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx
+++ /dev/null
@@ -1,253 +1,0 @@
-import { CameraView, useCameraPermissions } from "expo-camera";
-import { Stack, useLocalSearchParams, useRouter } from "expo-router";
-import { SymbolView } from "expo-symbols";
-import { useCallback, useEffect, useState } from "react";
-import { Alert, Pressable, ScrollView, View } from "react-native";
-import { useSafeAreaInsets } from "react-native-safe-area-context";
-import { useThemeColor } from "../../lib/useThemeColor";
-
-import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText";
-import { ErrorBanner } from "../../components/ErrorBanner";
-import { dismissRoute } from "../../lib/routes";
-import { ConnectionSheetButton } from "./ConnectionSheetButton";
-import { extractPairingUrlFromQrPayload } from "./pairing";
-import { useRemoteConnections } from "../../state/use-remote-environment-registry";
-import { buildPairingUrl, parsePairingUrl } from "./pairing";
-
-export function NewConnectionRouteScreen() {
- const {
- connectionError,
- connectionPairingUrl,
- connectionState,
- onChangeConnectionPairingUrl,
- onConnectPress,
- } = useRemoteConnections();
- const router = useRouter();
- const params = useLocalSearchParams<{ mode?: string }>();
- const insets = useSafeAreaInsets();
- const [hostInput, setHostInput] = useState("");
- const [codeInput, setCodeInput] = useState("");
- const [isSubmitting, setIsSubmitting] = useState(false);
- const [showScanner, setShowScanner] = useState(params.mode === "scan_qr");
- const [cameraPermission, requestCameraPermission] = useCameraPermissions();
- const [scannerLocked, setScannerLocked] = useState(false);
-
- const textColor = useThemeColor("--color-icon");
- const placeholderColor = useThemeColor("--color-placeholder");
-
- const connectDisabled =
- isSubmitting || connectionState === "connecting" || hostInput.trim().length === 0;
-
- useEffect(() => {
- const { host, code } = parsePairingUrl(connectionPairingUrl);
- setHostInput(host);
- setCodeInput(code);
- }, [connectionPairingUrl]);
-
- useEffect(() => {
- if (connectionError) {
- setIsSubmitting(false);
- }
- }, [connectionError]);
-
- const handleHostChange = useCallback((value: string) => {
- setHostInput(value);
- }, []);
-
- const handleCodeChange = useCallback((value: string) => {
- setCodeInput(value);
- }, []);
-
- const openScanner = useCallback(async () => {
- if (cameraPermission?.granted) {
- setScannerLocked(false);
- setShowScanner(true);
- return;
- }
-
- const permission = await requestCameraPermission();
- if (permission.granted) {
- setScannerLocked(false);
- setShowScanner(true);
- return;
- }
-
- Alert.alert("Camera access needed", "Allow camera access to scan a backend pairing QR code.");
- }, [cameraPermission?.granted, requestCameraPermission]);
-
- const closeScanner = useCallback(() => {
- setShowScanner(false);
- setScannerLocked(false);
- }, []);
-
- const handleQrScan = useCallback(
- ({ data }: { readonly data: string }) => {
- if (scannerLocked) {
- return;
- }
-
- setScannerLocked(true);
-
- try {
- const pairingUrl = extractPairingUrlFromQrPayload(data);
- const { host, code } = parsePairingUrl(pairingUrl);
- setHostInput(host);
- setCodeInput(code);
- onChangeConnectionPairingUrl(pairingUrl);
- setShowScanner(false);
- } catch (error) {
- Alert.alert(
- "Invalid QR code",
- error instanceof Error ? error.message : "Scanned QR code was not recognized.",
- );
- } finally {
- setTimeout(() => {
- setScannerLocked(false);
- }, 600);
- }
- },
- [onChangeConnectionPairingUrl, scannerLocked],
- );
-
- const handleSubmit = useCallback(async () => {
- setIsSubmitting(true);
-
- try {
- const pairingUrl = buildPairingUrl(hostInput, codeInput);
- onChangeConnectionPairingUrl(pairingUrl);
- await onConnectPress(pairingUrl);
- dismissRoute(router);
- } catch {
- setIsSubmitting(false);
- }
- }, [codeInput, hostInput, onChangeConnectionPairingUrl, onConnectPress, router]);
-
- return (
- <View collapsable={false} className="flex-1 bg-sheet">
- <Stack.Screen
- options={{
- title: showScanner ? "Scan QR Code" : "Add Backend",
- headerRight: () => (
- <Pressable
- className="h-10 w-10 items-center justify-center rounded-full border border-border bg-secondary"
- onPress={() => {
- if (showScanner) {
- closeScanner();
- } else {
- void openScanner();
- }
- }}
- >
- <SymbolView
- name={showScanner ? "xmark" : "qrcode.viewfinder"}
- size={showScanner ? 14 : 18}
- tintColor={textColor}
- type="monochrome"
- weight="semibold"
- />
- </Pressable>
- ),
- }}
- />
-
- <ScrollView
- contentInsetAdjustmentBehavior="automatic"
- showsVerticalScrollIndicator={false}
- style={{ flex: 1 }}
- contentContainerStyle={{
- paddingHorizontal: 20,
- paddingTop: 16,
- paddingBottom: Math.max(insets.bottom, 18) + 18,
- }}
- >
- <View collapsable={false} className="gap-5">
- {showScanner ? (
- cameraPermission?.granted ? (
- <View
- className="overflow-hidden rounded-[24px]"
- style={{ borderCurve: "continuous" }}
- >
- <CameraView
- barcodeScannerSettings={{ barcodeTypes: ["qr"] }}
- onBarcodeScanned={handleQrScan}
- style={{ aspectRatio: 1, width: "100%" }}
- />
- </View>
- ) : (
- <View
- className="items-center gap-3 rounded-[24px] bg-card px-5 py-8"
- style={{ borderCurve: "continuous" }}
- >
- <Text className="text-center text-[14px] leading-[20px] text-foreground-muted">
- Camera permission is required to scan a QR code.
- </Text>
- <ConnectionSheetButton
- compact
- icon="camera"
- label="Allow camera"
- tone="secondary"
- onPress={() => {
- void openScanner();
- }}
- />
- </View>
- )
- ) : (
- <View collapsable={false} className="gap-4 rounded-[24px] bg-card p-4">
- <View collapsable={false} className="gap-1.5">
- <Text
- className="text-[11px] font-t3-bold uppercase text-foreground-muted"
- style={{ letterSpacing: 0.8 }}
- >
- Host
- </Text>
- <TextInput
- autoCapitalize="none"
- autoCorrect={false}
- keyboardType="url"
- placeholder="192.168.1.100:8080"
- placeholderTextColor={placeholderColor}
- value={hostInput}
- onChangeText={handleHostChange}
- className="rounded-[14px] border border-input-border bg-input px-4 py-3.5 text-[15px] text-foreground"
- />
- </View>
-
- <View collapsable={false} className="gap-1.5">
- <Text
- className="text-[11px] font-t3-bold uppercase text-foreground-muted"
- style={{ letterSpacing: 0.8 }}
- >
- Pairing code
- </Text>
- <TextInput
- autoCapitalize="none"
- autoCorrect={false}
- placeholder="abc-123-xyz"
- placeholderTextColor={placeholderColor}
- value={codeInput}
- onChangeText={handleCodeChange}
- className="rounded-[14px] border border-input-border bg-input px-4 py-3.5 text-[15px] text-foreground"
- />
- </View>
-
- {connectionError ? <ErrorBanner message={connectionError} /> : null}
-
- <ConnectionSheetButton
- icon="plus"
- label={
- isSubmitting || connectionState === "connecting" ? "Pairing..." : "Add backend"
- }
- disabled={connectDisabled}
- tone="primary"
- onPress={() => {
- void handleSubmit();
- }}
- />
- </View>
- )}
- </View>
- </ScrollView>
- </View>
- );
-}
\ No newline at end of fileYou can send follow-ups to the cloud agent here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: CARD_SHADOW exports are defined but never imported
- Removed the unused CARD_SHADOW and CARD_SHADOW_DARK constants and their export statement from ConnectionSheetButton.tsx, as they were never imported anywhere in the codebase.
- ✅ Fixed: Debug console.log statements left in production code
- Removed all three console.log statements with the [new task flow] prefix from new-task-flow-provider.tsx (in reset callback, environment init effect, and the dedicated state-logging effect).
Or push these changes by commenting:
@cursor push 12dd1bcb97
Preview (12dd1bcb97)
diff --git a/apps/mobile/src/features/connection/ConnectionSheetButton.tsx b/apps/mobile/src/features/connection/ConnectionSheetButton.tsx
--- a/apps/mobile/src/features/connection/ConnectionSheetButton.tsx
+++ b/apps/mobile/src/features/connection/ConnectionSheetButton.tsx
@@ -5,28 +5,6 @@
import { AppText as Text } from "../../components/AppText";
import { cn } from "../../lib/cn";
-const CARD_SHADOW = Platform.select({
- ios: {
- shadowColor: "rgba(23,23,23,0.08)",
- shadowOffset: { width: 0, height: 4 },
- shadowOpacity: 1,
- shadowRadius: 16,
- },
- android: { elevation: 3 },
-});
-
-const CARD_SHADOW_DARK = Platform.select({
- ios: {
- shadowColor: "#000",
- shadowOffset: { width: 0, height: 2 },
- shadowOpacity: 0.18,
- shadowRadius: 8,
- },
- android: { elevation: 4 },
-});
-
-export { CARD_SHADOW, CARD_SHADOW_DARK };
-
export function ConnectionSheetButton(props: {
readonly icon: React.ComponentProps<typeof SymbolView>["name"];
readonly label: string;
diff --git a/apps/mobile/src/features/threads/new-task-flow-provider.tsx b/apps/mobile/src/features/threads/new-task-flow-provider.tsx
--- a/apps/mobile/src/features/threads/new-task-flow-provider.tsx
+++ b/apps/mobile/src/features/threads/new-task-flow-provider.tsx
@@ -189,10 +189,6 @@
}, []);
const reset = useCallback(() => {
- console.log("[new task flow] reset", {
- defaultEnvironmentId: projects[0]?.environmentId ?? null,
- projectCount: projects.length,
- });
setSelectedEnvironmentId(projects[0]?.environmentId ?? "");
setSelectedProjectKey(null);
setSelectedModelKey(null);
@@ -216,9 +212,6 @@
return;
}
- console.log("[new task flow] initializing environment", {
- environmentId: projects[0]!.environmentId,
- });
setSelectedEnvironmentId(projects[0]!.environmentId);
}, [projects, selectedEnvironmentId]);
@@ -470,24 +463,6 @@
],
);
- useEffect(() => {
- console.log("[new task flow] state", {
- availableBranchCount: availableBranches.length,
- environmentCount: environments.length,
- logicalProjectCount: logicalProjects.length,
- selectedEnvironmentId,
- selectedProjectKey,
- selectedProjectTitle: selectedProject?.title ?? null,
- });
- }, [
- availableBranches.length,
- environments.length,
- logicalProjects.length,
- selectedEnvironmentId,
- selectedProject?.title,
- selectedProjectKey,
- ]);
-
return <NewTaskFlowContext.Provider value={value}>{props.children}</NewTaskFlowContext.Provider>;
}You can send follow-ups to the cloud agent here.
| android: { elevation: 4 }, | ||
| }); | ||
|
|
||
| export { CARD_SHADOW, CARD_SHADOW_DARK }; |
There was a problem hiding this comment.
CARD_SHADOW exports are defined but never imported
Low Severity
CARD_SHADOW and CARD_SHADOW_DARK are defined and explicitly exported but never imported anywhere in the codebase. These are unused dead exports that add clutter without providing value.
Reviewed by Cursor Bugbot for commit 3d3e847. Configure here.
| defaultEnvironmentId: projects[0]?.environmentId ?? null, | ||
| projectCount: projects.length, | ||
| }); | ||
| setSelectedEnvironmentId(projects[0]?.environmentId ?? ""); |
There was a problem hiding this comment.
Debug console.log statements left in production code
Medium Severity
Multiple console.log calls with the [new task flow] prefix are left in new-task-flow-provider.tsx. These log internal state like environmentId, selectedProjectKey, branch counts, and project titles on every state change — verbose debug output that shouldn't ship in a production mobile app.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 3d3e847. Configure here.
juliusmarminge
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: QR scanner lock uses state instead of ref
- Replaced useState with useRef for the scanner lock so the guard value is read synchronously, preventing duplicate scan processing from rapid native barcode callbacks before React re-renders.
Or push these changes by commenting:
@cursor push 9d24bfb15f
Preview (9d24bfb15f)
diff --git a/apps/mobile/src/app/connections/new.tsx b/apps/mobile/src/app/connections/new.tsx
--- a/apps/mobile/src/app/connections/new.tsx
+++ b/apps/mobile/src/app/connections/new.tsx
@@ -1,7 +1,7 @@
import { CameraView, useCameraPermissions } from "expo-camera";
import { Stack, useLocalSearchParams, useRouter } from "expo-router";
import { SymbolView } from "expo-symbols";
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
import { Alert, Pressable, ScrollView, View } from "react-native";
import { useSafeAreaInsets } from "react-native-safe-area-context";
import { useThemeColor } from "../../lib/useThemeColor";
@@ -30,7 +30,7 @@
const [isSubmitting, setIsSubmitting] = useState(false);
const [showScanner, setShowScanner] = useState(params.mode === "scan_qr");
const [cameraPermission, requestCameraPermission] = useCameraPermissions();
- const [scannerLocked, setScannerLocked] = useState(false);
+ const scannerLockedRef = useRef(false);
const textColor = useThemeColor("--color-icon");
const placeholderColor = useThemeColor("--color-placeholder");
@@ -60,14 +60,14 @@
const openScanner = useCallback(async () => {
if (cameraPermission?.granted) {
- setScannerLocked(false);
+ scannerLockedRef.current = false;
setShowScanner(true);
return;
}
const permission = await requestCameraPermission();
if (permission.granted) {
- setScannerLocked(false);
+ scannerLockedRef.current = false;
setShowScanner(true);
return;
}
@@ -77,16 +77,16 @@
const closeScanner = useCallback(() => {
setShowScanner(false);
- setScannerLocked(false);
+ scannerLockedRef.current = false;
}, []);
const handleQrScan = useCallback(
({ data }: { readonly data: string }) => {
- if (scannerLocked) {
+ if (scannerLockedRef.current) {
return;
}
- setScannerLocked(true);
+ scannerLockedRef.current = true;
try {
const pairingUrl = extractPairingUrlFromQrPayload(data);
@@ -102,11 +102,11 @@
);
} finally {
setTimeout(() => {
- setScannerLocked(false);
+ scannerLockedRef.current = false;
}, 600);
}
},
- [onChangeConnectionPairingUrl, scannerLocked],
+ [onChangeConnectionPairingUrl],
);
const handleSubmit = useCallback(async () => {You can send follow-ups to the cloud agent here.
| } | ||
| }, | ||
| [onChangeConnectionPairingUrl, scannerLocked], | ||
| ); |
There was a problem hiding this comment.
QR scanner lock uses state instead of ref
Low Severity
handleQrScan reads scannerLocked from a stale closure since it's a useState value in a useCallback dependency array. Native barcode scanner callbacks can fire multiple times before React re-renders with the updated callback. A useRef would reliably prevent duplicate scan processing, since the ref value is always current across all closure instances.
Reviewed by Cursor Bugbot for commit efc36b4. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 7 total unresolved issues (including 6 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Worktree path condition is inverted
- The ternary condition was indeed inverted, passing null for worktree mode and the path for local mode; flipped the condition so worktree mode correctly receives the selected worktree path.
Or push these changes by commenting:
@cursor push 677fe73be8
Preview (677fe73be8)
diff --git a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx
--- a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx
+++ b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx
@@ -369,7 +369,7 @@
modelSelection: modelWithOptions,
envMode: flow.workspaceMode,
branch: flow.selectedBranchName,
- worktreePath: flow.workspaceMode === "worktree" ? null : flow.selectedWorktreePath,
+ worktreePath: flow.workspaceMode === "worktree" ? flow.selectedWorktreePath : null,
runtimeMode: flow.runtimeMode,
interactionMode: flow.interactionMode,
initialMessageText: flow.prompt.trim(),You can send follow-ups to the cloud agent here.
Rebuild PR #2013 on top of current origin/main without the duplicated commit lineage. Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/mobile-remote-connect
branch
from
1705361 to
c799180
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 7 total unresolved issues (including 6 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Route file duplicates entire feature component inline
- Replaced the duplicated ~250-line inline implementation in apps/mobile/src/app/connections/new.tsx with a simple import and render of NewConnectionRouteScreen from the features/connection/ module, matching the delegation pattern used by other route files.
Or push these changes by commenting:
@cursor push 59b1d755d9
Preview (59b1d755d9)
diff --git a/apps/mobile/src/app/connections/new.tsx b/apps/mobile/src/app/connections/new.tsx
--- a/apps/mobile/src/app/connections/new.tsx
+++ b/apps/mobile/src/app/connections/new.tsx
@@ -1,253 +1,5 @@
-import { CameraView, useCameraPermissions } from "expo-camera";
-import { Stack, useLocalSearchParams, useRouter } from "expo-router";
-import { SymbolView } from "expo-symbols";
-import { useCallback, useEffect, useState } from "react";
-import { Alert, Pressable, ScrollView, View } from "react-native";
-import { useSafeAreaInsets } from "react-native-safe-area-context";
-import { useThemeColor } from "../../lib/useThemeColor";
+import { NewConnectionRouteScreen } from "../../features/connection/NewConnectionRouteScreen";
-import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText";
-import { ErrorBanner } from "../../components/ErrorBanner";
-import { dismissRoute } from "../../lib/routes";
-import { ConnectionSheetButton } from "../../features/connection/ConnectionSheetButton";
-import { extractPairingUrlFromQrPayload } from "../../features/connection/pairing";
-import { useRemoteConnections } from "../../state/use-remote-environment-registry";
-import { buildPairingUrl, parsePairingUrl } from "../../features/connection/pairing";
-
-export default function ConnectionsNewRouteScreen() {
- const {
- connectionError,
- connectionPairingUrl,
- connectionState,
- onChangeConnectionPairingUrl,
- onConnectPress,
- } = useRemoteConnections();
- const router = useRouter();
- const params = useLocalSearchParams<{ mode?: string }>();
- const insets = useSafeAreaInsets();
- const [hostInput, setHostInput] = useState("");
- const [codeInput, setCodeInput] = useState("");
- const [isSubmitting, setIsSubmitting] = useState(false);
- const [showScanner, setShowScanner] = useState(params.mode === "scan_qr");
- const [cameraPermission, requestCameraPermission] = useCameraPermissions();
- const [scannerLocked, setScannerLocked] = useState(false);
-
- const textColor = useThemeColor("--color-icon");
- const placeholderColor = useThemeColor("--color-placeholder");
-
- const connectDisabled =
- isSubmitting || connectionState === "connecting" || hostInput.trim().length === 0;
-
- useEffect(() => {
- const { host, code } = parsePairingUrl(connectionPairingUrl);
- setHostInput(host);
- setCodeInput(code);
- }, [connectionPairingUrl]);
-
- useEffect(() => {
- if (connectionError) {
- setIsSubmitting(false);
- }
- }, [connectionError]);
-
- const handleHostChange = useCallback((value: string) => {
- setHostInput(value);
- }, []);
-
- const handleCodeChange = useCallback((value: string) => {
- setCodeInput(value);
- }, []);
-
- const openScanner = useCallback(async () => {
- if (cameraPermission?.granted) {
- setScannerLocked(false);
- setShowScanner(true);
- return;
- }
-
- const permission = await requestCameraPermission();
- if (permission.granted) {
- setScannerLocked(false);
- setShowScanner(true);
- return;
- }
-
- Alert.alert("Camera access needed", "Allow camera access to scan a backend pairing QR code.");
- }, [cameraPermission?.granted, requestCameraPermission]);
-
- const closeScanner = useCallback(() => {
- setShowScanner(false);
- setScannerLocked(false);
- }, []);
-
- const handleQrScan = useCallback(
- ({ data }: { readonly data: string }) => {
- if (scannerLocked) {
- return;
- }
-
- setScannerLocked(true);
-
- try {
- const pairingUrl = extractPairingUrlFromQrPayload(data);
- const { host, code } = parsePairingUrl(pairingUrl);
- setHostInput(host);
- setCodeInput(code);
- onChangeConnectionPairingUrl(pairingUrl);
- setShowScanner(false);
- } catch (error) {
- Alert.alert(
- "Invalid QR code",
- error instanceof Error ? error.message : "Scanned QR code was not recognized.",
- );
- } finally {
- setTimeout(() => {
- setScannerLocked(false);
- }, 600);
- }
- },
- [onChangeConnectionPairingUrl, scannerLocked],
- );
-
- const handleSubmit = useCallback(async () => {
- setIsSubmitting(true);
-
- try {
- const pairingUrl = buildPairingUrl(hostInput, codeInput);
- onChangeConnectionPairingUrl(pairingUrl);
- await onConnectPress(pairingUrl);
- dismissRoute(router);
- } catch {
- setIsSubmitting(false);
- }
- }, [codeInput, hostInput, onChangeConnectionPairingUrl, onConnectPress, router]);
-
- return (
- <View collapsable={false} className="flex-1 bg-sheet">
- <Stack.Screen
- options={{
- title: showScanner ? "Scan QR Code" : "Add Backend",
- headerRight: () => (
- <Pressable
- className="h-10 w-10 items-center justify-center rounded-full border border-border bg-secondary"
- onPress={() => {
- if (showScanner) {
- closeScanner();
- } else {
- void openScanner();
- }
- }}
- >
- <SymbolView
- name={showScanner ? "xmark" : "qrcode.viewfinder"}
- size={showScanner ? 14 : 18}
- tintColor={textColor}
- type="monochrome"
- weight="semibold"
- />
- </Pressable>
- ),
- }}
- />
-
- <ScrollView
- contentInsetAdjustmentBehavior="automatic"
- showsVerticalScrollIndicator={false}
- style={{ flex: 1 }}
- contentContainerStyle={{
- paddingHorizontal: 20,
- paddingTop: 16,
- paddingBottom: Math.max(insets.bottom, 18) + 18,
- }}
- >
- <View collapsable={false} className="gap-5">
- {showScanner ? (
- cameraPermission?.granted ? (
- <View
- className="overflow-hidden rounded-[24px]"
- style={{ borderCurve: "continuous" }}
- >
- <CameraView
- barcodeScannerSettings={{ barcodeTypes: ["qr"] }}
- onBarcodeScanned={handleQrScan}
- style={{ aspectRatio: 1, width: "100%" }}
- />
- </View>
- ) : (
- <View
- className="items-center gap-3 rounded-[24px] bg-card px-5 py-8"
- style={{ borderCurve: "continuous" }}
- >
- <Text className="text-center text-[14px] leading-[20px] text-foreground-muted">
- Camera permission is required to scan a QR code.
- </Text>
- <ConnectionSheetButton
- compact
- icon="camera"
- label="Allow camera"
- tone="secondary"
- onPress={() => {
- void openScanner();
- }}
- />
- </View>
- )
- ) : (
- <View collapsable={false} className="gap-4 rounded-[24px] bg-card p-4">
- <View collapsable={false} className="gap-1.5">
- <Text
- className="text-[11px] font-t3-bold uppercase text-foreground-muted"
- style={{ letterSpacing: 0.8 }}
- >
- Host
- </Text>
- <TextInput
- autoCapitalize="none"
- autoCorrect={false}
- keyboardType="url"
- placeholder="192.168.1.100:8080"
- placeholderTextColor={placeholderColor}
- value={hostInput}
- onChangeText={handleHostChange}
- className="rounded-[14px] border border-input-border bg-input px-4 py-3.5 text-[15px] text-foreground"
- />
- </View>
-
- <View collapsable={false} className="gap-1.5">
- <Text
- className="text-[11px] font-t3-bold uppercase text-foreground-muted"
- style={{ letterSpacing: 0.8 }}
- >
- Pairing code
- </Text>
- <TextInput
- autoCapitalize="none"
- autoCorrect={false}
- placeholder="abc-123-xyz"
- placeholderTextColor={placeholderColor}
- value={codeInput}
- onChangeText={handleCodeChange}
- className="rounded-[14px] border border-input-border bg-input px-4 py-3.5 text-[15px] text-foreground"
- />
- </View>
-
- {connectionError ? <ErrorBanner message={connectionError} /> : null}
-
- <ConnectionSheetButton
- icon="plus"
- label={
- isSubmitting || connectionState === "connecting" ? "Pairing..." : "Add backend"
- }
- disabled={connectDisabled}
- tone="primary"
- onPress={() => {
- void handleSubmit();
- }}
- />
- </View>
- )}
- </View>
- </ScrollView>
- </View>
- );
+export default function ConnectionsNewRoute() {
+ return <NewConnectionRouteScreen />;
}You can send follow-ups to the cloud agent here.
| </ScrollView> | ||
| </View> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Route file duplicates entire feature component inline
Medium Severity
apps/mobile/src/app/connections/new.tsx is a near-identical copy of NewConnectionRouteScreen.tsx in features/connection/. The route file reimplements all QR scanning, form input, and submission logic instead of importing the existing feature component. Any future bug fix applied to one copy will be missed in the other, leading to divergent behavior.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c799180. Configure here.
| <SymbolView | ||
| name="xmark" | ||
| size={9} | ||
| tintColor="#ffffff" | ||
| type="monochrome" | ||
| weight="bold" | ||
| /> |
There was a problem hiding this comment.
🟢 Low components/ComposerAttachmentStrip.tsx:72
On Android, SymbolView with name="xmark" renders nothing because expo-symbols expects an object with platform-specific keys ({ android: string, web: string }), not a plain string. This causes the remove button to appear as an empty circle on Android. Pass an object with android and web keys to ensure the icon renders on both platforms.
- <SymbolView
- name="xmark"
- size={9}
- tintColor="#ffffff"
- type="monochrome"
- weight="bold"
- />Also found in 1 other location(s)
apps/mobile/src/features/threads/review/ReviewCommentComposerSheet.tsx:21
On iOS,
ui-monospaceis a CSS keyword that does not work as afontFamilyvalue in React Native. React Native requires actual iOS font family names likeMenloorCourier. Usingui-monospacewill cause the text to fall back to the default proportional font, defeating the intent of displaying monospace code.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/components/ComposerAttachmentStrip.tsx around lines 72-78:
On Android, `SymbolView` with `name="xmark"` renders nothing because `expo-symbols` expects an object with platform-specific keys (`{ android: string, web: string }`), not a plain string. This causes the remove button to appear as an empty circle on Android. Pass an object with `android` and `web` keys to ensure the icon renders on both platforms.
Evidence trail:
1. apps/mobile/src/components/ComposerAttachmentStrip.tsx lines 72-78 - SymbolView uses `name="xmark"` (plain string)
2. https://docs.expo.dev/versions/latest/sdk/symbols/ - Official expo-symbols documentation states: "If you only pass a string, it is treated as an SF Symbol name and renders only on iOS. On Android and web, nothing will be rendered unless you provide a `fallback`" and shows the `name` prop type as `SFSymbol | { android: AndroidSymbol, ios: SFSymbol, web: AndroidSymbol }`
Also found in 1 other location(s):
- apps/mobile/src/features/threads/review/ReviewCommentComposerSheet.tsx:21 -- On iOS, `ui-monospace` is a CSS keyword that does not work as a `fontFamily` value in React Native. React Native requires actual iOS font family names like `Menlo` or `Courier`. Using `ui-monospace` will cause the text to fall back to the default proportional font, defeating the intent of displaying monospace code.
Rebuild PR #2013 on top of current origin/main without the duplicated commit lineage. Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/mobile-remote-connect
branch
from
c799180 to
f86d466
Compare
There was a problem hiding this comment.
🟡 Medium review/shikiReviewHighlighter.ts:83
When a file with an .svg extension is highlighted, resolveLanguageFromPath returns "svg" as the language identifier. However, the svg entry in languageImports loads the @shikijs/langs/xml module, which Shiki registers under the ID "xml". This causes highlighter.codeToTokensBase to request "svg", a language that was never registered, resulting in failed or fallback highlighting for SVG files.
- svg: () => import("@shikijs/langs/xml"),🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/review/shikiReviewHighlighter.ts around line 83:
When a file with an `.svg` extension is highlighted, `resolveLanguageFromPath` returns `"svg"` as the language identifier. However, the `svg` entry in `languageImports` loads the `@shikijs/langs/xml` module, which Shiki registers under the ID `"xml"`. This causes `highlighter.codeToTokensBase` to request `"svg"`, a language that was never registered, resulting in failed or fallback highlighting for SVG files.
Evidence trail:
apps/mobile/src/features/threads/review/shikiReviewHighlighter.ts lines 84 (`svg: () => import("@shikijs/langs/xml")`), 108-136 (languageAliases without 'svg'), 327-357 (loadSingleLanguage adds 'language' to loadedLanguages but Shiki registers the module's internal ID), 359-383 (resolveLanguageFromPath returns 'candidate' not the registered ID). @shikijs/langs package.json (https://github.com/shikijs/shiki/blob/main/packages/langs/package.json) shows no './svg' export. tm-grammars README (https://github.com/shikijs/textmate-grammars-themes/tree/main/packages/tm-grammars) confirms xml has no 'svg' alias.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 9 total unresolved issues (including 8 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cleartext HTTP allowed unconditionally in production builds
- Gated NSAllowsArbitraryLoads and withAndroidCleartextTraffic plugin behind APP_VARIANT !== 'production' so cleartext HTTP is only allowed in development and preview builds.
Or push these changes by commenting:
@cursor push f252a0a26f
Preview (f252a0a26f)
diff --git a/apps/mobile/app.config.ts b/apps/mobile/app.config.ts
--- a/apps/mobile/app.config.ts
+++ b/apps/mobile/app.config.ts
@@ -77,9 +77,11 @@
supportsTablet: true,
bundleIdentifier: variant.iosBundleIdentifier,
infoPlist: {
- NSAppTransportSecurity: {
- NSAllowsArbitraryLoads: true,
- },
+ ...(APP_VARIANT !== "production" && {
+ NSAppTransportSecurity: {
+ NSAllowsArbitraryLoads: true,
+ },
+ }),
ITSAppUsesNonExemptEncryption: false,
},
},
@@ -128,7 +130,7 @@
],
"expo-secure-store",
"expo-router",
- "./plugins/withAndroidCleartextTraffic.cjs",
+ ...(APP_VARIANT !== "production" ? ["./plugins/withAndroidCleartextTraffic.cjs"] : []),
],
extra: {
appVariant: APP_VARIANT,You can send follow-ups to the cloud agent here.
| if (input.status === "exited") { | ||
| return "Exited"; | ||
| } | ||
| if (input.status === "error") { | ||
| return "Error"; | ||
| } | ||
|
|
||
| return "Not started"; | ||
| } |
There was a problem hiding this comment.
🟢 Low threads/terminalMenu.ts:48
The status union includes "closed" but getTerminalStatusLabel has no explicit case for it, causing closed terminals to return "Not started". A terminal that was previously running and then closed will misleadingly show "Not started" to users.
if (input.status === "error") {
return "Error";
}
+ if (input.status === "closed") {
+ return "Closed";
+ }
return "Not started";🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/terminalMenu.ts around lines 48-56:
The `status` union includes `"closed"` but `getTerminalStatusLabel` has no explicit case for it, causing closed terminals to return `"Not started"`. A terminal that was previously running and then closed will misleadingly show "Not started" to users.
Evidence trail:
apps/mobile/src/features/threads/terminalMenu.ts lines 7 (status union definition includes "closed"), lines 40-55 (getTerminalStatusLabel function with no case for "closed"), lines 75-79 (buildTerminalMenuSessions sets status: "closed" showing active use of this status)
| } | ||
| } finally { | ||
| highlightQueueActiveRef.current = false; | ||
| if ( | ||
| generation === highlightQueueGenerationRef.current && | ||
| highlightQueueRef.current.length > 0 | ||
| ) { | ||
| startHighlightQueueRunner(); | ||
| } | ||
| } | ||
| })(); |
There was a problem hiding this comment.
🟡 Medium review/ReviewSheet.tsx:798
The highlight queue runner's finally block unconditionally sets highlightQueueActiveRef.current = false even when its generation is stale. This allows a third runner to start while Runner 2 is still processing: Runner 1 completes and clears the flag, then Runner 3 sees highlightQueueActiveRef === false and starts despite Runner 2 being active. Consider only clearing the flag when generation === highlightQueueGenerationRef.current so the active runner retains ownership.
} finally {
- highlightQueueActiveRef.current = false;
- if (
+ if (generation === highlightQueueGenerationRef.current) {
+ highlightQueueActiveRef.current = false;
+ }
+ if (
generation === highlightQueueGenerationRef.current &&
highlightQueueRef.current.length > 0
) {
startHighlightQueueRunner();
}
}Also found in 1 other location(s)
apps/mobile/src/state/use-remote-environment-registry.ts:318
Race condition in cleanup: The
void Promise.all(connections.map(...))at line 318 starts asyncconnectSavedEnvironmentcalls without awaiting them, but the cleanup function only disposes sessions present inenvironmentSessionsat cleanup time. If unmount occurs while these connections are being established (e.g., during theawait disconnectEnvironment()orawait ensureBootstrapped()within eachconnectSavedEnvironment), the cleanup will run and clear the map, but the in-flightconnectSavedEnvironmentcalls will continue and subsequently add new sessions to the already-clearedenvironmentSessions. These late-added sessions will never be disposed, leaking WebSocket connections.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/review/ReviewSheet.tsx around lines 798-808:
The highlight queue runner's `finally` block unconditionally sets `highlightQueueActiveRef.current = false` even when its generation is stale. This allows a third runner to start while Runner 2 is still processing: Runner 1 completes and clears the flag, then Runner 3 sees `highlightQueueActiveRef === false` and starts despite Runner 2 being active. Consider only clearing the flag when `generation === highlightQueueGenerationRef.current` so the active runner retains ownership.
Evidence trail:
apps/mobile/src/features/threads/review/ReviewSheet.tsx lines 725-730 (guard and flag setting), line 735 (while loop generation check), lines 800-805 (unconditional flag clearing in finally), lines 891-894 (generation increment and flag reset on section/theme change), line 950 (external call to startHighlightQueueRunner)
Also found in 1 other location(s):
- apps/mobile/src/state/use-remote-environment-registry.ts:318 -- Race condition in cleanup: The `void Promise.all(connections.map(...))` at line 318 starts async `connectSavedEnvironment` calls without awaiting them, but the cleanup function only disposes sessions present in `environmentSessions` at cleanup time. If unmount occurs while these connections are being established (e.g., during the `await disconnectEnvironment()` or `await ensureBootstrapped()` within each `connectSavedEnvironment`), the cleanup will run and clear the map, but the in-flight `connectSavedEnvironment` calls will continue and subsequently add new sessions to the already-cleared `environmentSessions`. These late-added sessions will never be disposed, leaking WebSocket connections.
Rebuild PR #2013 on top of current origin/main without the duplicated commit lineage. Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/mobile-remote-connect
branch
from
b7da92c to
4d137a1
Compare
| url.hash = new URLSearchParams([["token", c]]).toString(); | ||
| return url.toString(); | ||
| } catch { | ||
| return `${h}#token=${c}`; |
There was a problem hiding this comment.
🟢 Low connection/pairing.ts:14
In buildPairingUrl, the fallback path on line 14 returns ${h}#token=${c} without URL-encoding c. When the code contains special characters like = or & (e.g., abc=123&xyz), and the URL constructor throws (e.g., due to invalid host formatting), the resulting fragment is malformed. Later, parsePairingUrl uses URLSearchParams to extract the token, which only reads the portion before the first = or &, corrupting the pairing code. Consider using encodeURIComponent(c) in the fallback to ensure special characters are preserved.
| return `${h}#token=${c}`; | |
| return `${h}#token=${encodeURIComponent(c)}`; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/connection/pairing.ts around line 14:
In `buildPairingUrl`, the fallback path on line 14 returns `${h}#token=${c}` without URL-encoding `c`. When the code contains special characters like `=` or `&` (e.g., `abc=123&xyz`), and the URL constructor throws (e.g., due to invalid host formatting), the resulting fragment is malformed. Later, `parsePairingUrl` uses `URLSearchParams` to extract the token, which only reads the portion before the first `=` or `&`, corrupting the pairing code. Consider using `encodeURIComponent(c)` in the fallback to ensure special characters are preserved.
Evidence trail:
apps/mobile/src/features/connection/pairing.ts (full file viewed at REVIEWED_COMMIT, commit baedbdbc6f):
- Line 11: try path uses `url.hash = new URLSearchParams([['token', c]]).toString()` which URL-encodes `c`
- Line 14: catch/fallback path uses `return `${h}#token=${c}`;` with no encoding
- Lines 22-23: `parsePairingUrl` uses `new URLSearchParams(parsed.hash.slice(1))` and `hashParams.get('token')` which will misparse unencoded special characters (`=`, `&`) in the token value
| export function parsePairingUrl(url: string): { host: string; code: string } { | ||
| const trimmed = url.trim(); | ||
| if (!trimmed) return { host: "", code: "" }; | ||
|
|
||
| try { | ||
| const parsed = new URL(trimmed); | ||
| const hashParams = new URLSearchParams(parsed.hash.slice(1)); | ||
| const hashToken = hashParams.get("token"); | ||
| const queryToken = parsed.searchParams.get("token"); | ||
| const code = hashToken || queryToken || ""; | ||
|
|
||
| parsed.hash = ""; | ||
| parsed.search = ""; | ||
| parsed.pathname = "/"; | ||
| return { host: parsed.toString().replace(/\/$/, ""), code }; | ||
| } catch { | ||
| return { host: trimmed, code: "" }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟢 Low connection/pairing.ts:18
parsePairingUrl unconditionally sets parsed.pathname = "/", which discards any path from the original URL. A pairing URL like https://192.168.1.100:8080/api/connect#token=abc returns host https://192.168.1.100:8080 without the /api/connect path, so subsequent calls to buildPairingUrl reconstruct the wrong endpoint. Consider preserving the original pathname instead of overwriting it.
+ parsed.hash = "";
+ parsed.search = "";
+ const pathname = parsed.pathname;
+ const hostUrl = pathname && pathname !== "/" ? parsed.toString() : parsed.toString().replace(/\/$/, "");
+ return { host: hostUrl, code };🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/connection/pairing.ts around lines 18-36:
`parsePairingUrl` unconditionally sets `parsed.pathname = "/"`, which discards any path from the original URL. A pairing URL like `https://192.168.1.100:8080/api/connect#token=abc` returns host `https://192.168.1.100:8080` without the `/api/connect` path, so subsequent calls to `buildPairingUrl` reconstruct the wrong endpoint. Consider preserving the original pathname instead of overwriting it.
Evidence trail:
apps/mobile/src/features/connection/pairing.ts lines 18-36: `parsePairingUrl` function, specifically line 30 `parsed.pathname = "/";` unconditionally overwrites pathname. apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx lines 42-44: `parsePairingUrl` output populates `hostInput`. Line 116: `buildPairingUrl(hostInput, codeInput)` reconstructs URL from the stripped host.
|
|
||
| export { CARD_SHADOW, CARD_SHADOW_DARK }; | ||
|
|
||
| export function ConnectionSheetButton(props: { |
There was a problem hiding this comment.
🟢 Low connection/ConnectionSheetButton.tsx:30
String icon names like "camera" and "plus" are passed to SymbolView, but SymbolView returns the fallback when name is not a platform object, and no fallback is provided. On Android and web, this renders nothing instead of the intended icon. Consider passing platform-specific icon objects like { ios: 'camera', android: 'camera_alt', web: 'camera' } instead of plain strings.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/connection/ConnectionSheetButton.tsx around line 30:
String icon names like `"camera"` and `"plus"` are passed to `SymbolView`, but `SymbolView` returns the `fallback` when `name` is not a platform object, and no `fallback` is provided. On Android and web, this renders nothing instead of the intended icon. Consider passing platform-specific icon objects like `{ ios: 'camera', android: 'camera_alt', web: 'camera' }` instead of plain strings.
Evidence trail:
1. apps/mobile/src/features/connection/ConnectionSheetButton.tsx lines 89-94 - SymbolView usage with no fallback prop
2. apps/mobile/src/app/connections/new.tsx lines 185-190 - ConnectionSheetButton called with icon="camera"
3. apps/mobile/src/app/connections/new.tsx lines 236-244 - ConnectionSheetButton called with icon="plus"
4. https://docs.expo.dev/versions/latest/sdk/symbols/ - "If you only pass a string, it is treated as an SF Symbol name and renders only on iOS. On Android and web, nothing will be rendered unless you provide a fallback"
- Reject diff preview requests outside the configured workspace - Add tests covering allowed and denied cwd paths
- Split file visibility, diff data, and comment selection logic out of `ReviewSheet` - Add tests for review file visibility and native diff bridge behavior - Wire review selection and mobile remote connect state through shared hooks
Co-authored-by: codex <codex@users.noreply.github.com>
- Move composer path search and source-control discovery into shared managed state - Persist review async/loading state in atoms and add coverage - Update mobile/web composers and review highlighter initialization
- Drop unterminated paths from truncated NUL-separated git stdout - Preserve truncation state when building untracked review diffs
- Switch mobile and client-runtime state to the generic VcsRef API - Preserve background refreshes with client change handling and errors
- Migrate branch selector and git actions to useVcsRefs and new source control action hooks - Remove gitReactQuery and projectReactQuery modules in favor of sourceControlActions and vcsRefState - Add staleTimeMs to mobile vcs ref manager configuration
- Move archived threads state into shared client-runtime - Add stale-aware refresh/invalidation for path search, discovery, and VCS refs - Wire provider invalidations through the web runtime
- Replace git-scoped client/runtime state with VCS-scoped APIs - Add checkpoint diff preview state and reuse cached diffs - Update mobile and web review flows to use the new shared state
Co-authored-by: codex <codex@users.noreply.github.com>
- Scope cleartext mobile config to development - Fix stale loads, ordering, and terminal reuse - Clean up terminal metadata subscriptions on failure
- Deduplicate terminal IDs and favorite model slugs with trim-aware iteration - Use effect-based filtering for local branch and built-in model lists - Switch stable array handling to non-mutating `toSorted`
- Replace chained array transforms with explicit loops across mobile and server paths - Tighten provider, git, and orchestration parsing while preserving behavior - Improve remote-connect UI data shaping and thread activity filtering
- Replace ad hoc sorts with shared Effect order helpers - Rework remote catalog and review/terminal lists for clearer data flow - Keep threaded/review selection and diff adapters aligned with the new ordering logic
- Rename backends to environments across mobile UI - Add richer loading and empty states for connections, home, and new task flows - Improve review section loading handling and terminal keyboard layout
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/mobile-remote-connect
branch
from
399b13d to
6b48288
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 13 total unresolved issues (including 11 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Native diff ignores highlight engine
- Replaced hardcoded
engine: "native"with a module-levelCONFIGURED_ENGINEconstant that resolvesEXPO_PUBLIC_REVIEW_HIGHLIGHTER_ENGINEvia the sharedresolveReviewHighlighterEnginePreferencehelper, matching the pattern used inshikiReviewHighlighter.ts.
- Replaced hardcoded
- ✅ Fixed: Unset variant resolves production config
- Changed
resolveAppVariant's default return from"production"to"development"so local dev scripts without an explicitAPP_VARIANTget development config (cleartext HTTP, dev bundle IDs), while EAS builds are unaffected since all profiles setAPP_VARIANTexplicitly.
- Changed
Or push these changes by commenting:
@cursor push 7294763bfe
Preview (7294763bfe)
diff --git a/apps/mobile/app.config.ts b/apps/mobile/app.config.ts
--- a/apps/mobile/app.config.ts
+++ b/apps/mobile/app.config.ts
@@ -44,7 +44,7 @@
case "production":
return value;
default:
- return "production";
+ return "development";
}
}
diff --git a/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts b/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts
--- a/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts
+++ b/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts
@@ -7,7 +7,13 @@
} from "../diffs/nativeReviewDiffHighlighter";
import type { NativeReviewDiffRow } from "../diffs/nativeReviewDiffSurface";
import type { NativeReviewDiffFile } from "../diffs/nativeReviewDiffTypes";
+import { resolveReviewHighlighterEnginePreference } from "./reviewHighlighterEngine";
+const CONFIGURED_ENGINE: NativeReviewDiffHighlightEngine = resolveReviewHighlighterEnginePreference(
+ process.env.EXPO_PUBLIC_REVIEW_HIGHLIGHTER_ENGINE ??
+ (process.env.NODE_ENV === "test" ? "javascript" : "native"),
+);
+
interface NativeReviewVisibleRange {
readonly firstRowIndex: number;
readonly lastRowIndex: number;
@@ -68,7 +74,7 @@
const abortController = new AbortController();
const requestRange = visibleRangeRef.current;
- const engine: NativeReviewDiffHighlightEngine = "native";
+ const engine: NativeReviewDiffHighlightEngine = CONFIGURED_ENGINE;
void (async () => {
try {You can send follow-ups to the cloud agent here.
| firstRowIndex: requestRange.firstRowIndex, | ||
| lastRowIndex: requestRange.lastRowIndex, | ||
| alreadyHighlightedRowIds: highlightedRowIdsRef.current, | ||
| signal: abortController.signal, |
There was a problem hiding this comment.
Native diff ignores highlight engine
Medium Severity
The native review diff visible-row highlighter always uses the native Shiki path because engine is fixed to "native", so EXPO_PUBLIC_REVIEW_HIGHLIGHTER_ENGINE and the shared getActiveReviewHighlighterEngine preference are never applied on this code path.
Reviewed by Cursor Bugbot for commit 6b48288. Configure here.
| return value; | ||
| default: | ||
| return "production"; | ||
| } |
There was a problem hiding this comment.
Unset variant resolves production config
Low Severity
When APP_VARIANT is missing, resolveAppVariant returns production, so local runs without an explicit variant pick production bundle IDs and disable development-only settings such as cleartext HTTP, even though several npm scripts start Expo without setting APP_VARIANT.
Reviewed by Cursor Bugbot for commit 6b48288. Configure here.
| if (token.content.length === 0) { | ||
| nextTokens.push(token); | ||
| return; | ||
| } | ||
|
|
||
| while (rangeIndex < ranges.length && ranges[rangeIndex]!.end <= tokenStart) { | ||
| rangeIndex += 1; | ||
| } | ||
|
|
||
| if ((ranges[rangeIndex]?.start ?? Number.POSITIVE_INFINITY) >= tokenEnd) { | ||
| nextTokens.push(token.diffHighlight ? { ...token, diffHighlight: false } : token); | ||
| return; |
There was a problem hiding this comment.
🟡 Medium review/reviewWordDiffs.ts:206
When a token falls outside all highlight ranges, the original token reference is pushed to nextTokens unchanged (lines 207, 216). If the next token's unhighlighted prefix has matching color/fontStyle/diffHighlight, appendTokenSegment mutates it via previous.content += content — this corrupts the input tokens array that may be referenced elsewhere. Consider cloning tokens before pushing (e.g., { ...token }) to prevent mutation of the input array.
- if (token.content.length === 0) {
- nextTokens.push(token);
+ if (token.content.length === 0) {
+ nextTokens.push({ ...token });
return;
}
while (rangeIndex < ranges.length && ranges[rangeIndex]!.end <= tokenStart) {
rangeIndex += 1;
}
if ((ranges[rangeIndex]?.start ?? Number.POSITIVE_INFINITY) >= tokenEnd) {
- nextTokens.push(token.diffHighlight ? { ...token, diffHighlight: false } : token);
+ nextTokens.push(token.diffHighlight ? { ...token, diffHighlight: false } : { ...token });🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/review/reviewWordDiffs.ts around lines 206-217:
When a token falls outside all highlight ranges, the original token reference is pushed to `nextTokens` unchanged (lines 207, 216). If the next token's unhighlighted prefix has matching `color`/`fontStyle`/`diffHighlight`, `appendTokenSegment` mutates it via `previous.content += content` — this corrupts the input `tokens` array that may be referenced elsewhere. Consider cloning tokens before pushing (e.g., `{ ...token }`) to prevent mutation of the input array.
Evidence trail:
apps/mobile/src/features/review/reviewWordDiffs.ts lines 87-114 (appendTokenSegment with mutation at line 104), lines 189-266 (applyDiffRangesToTokens), specifically lines 206-208 and 215-217 pushing original references, apps/mobile/src/features/review/shikiReviewHighlighter.ts lines 493-548 (caller doing shallow array copy at lines 497-498 then passing shared token objects)
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 14 total unresolved issues (including 13 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Heartbeat check always false
- Added activity tracking (lastActivityAt) to WsTransport updated on successful requests and stream values, exposed isHeartbeatFresh() with a 30-second threshold, and wired createWsRpcClient to delegate to it instead of returning a hardcoded false.
Or push these changes by commenting:
@cursor push 75d703ba56
Preview (75d703ba56)
diff --git a/packages/client-runtime/src/wsRpcClient.ts b/packages/client-runtime/src/wsRpcClient.ts
--- a/packages/client-runtime/src/wsRpcClient.ts
+++ b/packages/client-runtime/src/wsRpcClient.ts
@@ -171,7 +171,7 @@
): WsRpcClient {
return {
dispose: () => transport.dispose(),
- isHeartbeatFresh: () => false,
+ isHeartbeatFresh: () => transport.isHeartbeatFresh(),
reconnect: async () => {
options?.beforeReconnect?.();
await transport.reconnect();
diff --git a/packages/client-runtime/src/wsTransport.ts b/packages/client-runtime/src/wsTransport.ts
--- a/packages/client-runtime/src/wsTransport.ts
+++ b/packages/client-runtime/src/wsTransport.ts
@@ -44,6 +44,7 @@
}
const DEFAULT_SUBSCRIPTION_RETRY_DELAY = Duration.millis(250);
+const HEARTBEAT_FRESH_THRESHOLD_MS = 30_000;
const NOOP: () => void = () => undefined;
interface TransportSession {
@@ -68,6 +69,7 @@
private hasReportedTransportDisconnect = false;
private reconnectChain: Promise<void> = Promise.resolve();
private session: TransportSession;
+ private lastActivityAt = performance.now();
constructor(
url: WsRpcProtocolSocketUrlProvider,
@@ -89,7 +91,9 @@
const session = this.session;
const client = await session.clientPromise;
- return await session.runtime.runPromise(Effect.suspend(() => execute(client)));
+ const result = await session.runtime.runPromise(Effect.suspend(() => execute(client)));
+ this.lastActivityAt = performance.now();
+ return result;
}
async requestStream<TValue>(
@@ -105,6 +109,7 @@
await session.runtime.runPromise(
Stream.runForEach(connect(client), (value) =>
Effect.sync(() => {
+ this.lastActivityAt = performance.now();
try {
listener(value);
} catch {
@@ -228,6 +233,10 @@
await this.closeSession(this.session);
}
+ isHeartbeatFresh(): boolean {
+ return performance.now() - this.lastActivityAt < HEARTBEAT_FRESH_THRESHOLD_MS;
+ }
+
private closeSession(session: TransportSession) {
return session.runtime.runPromise(Scope.close(session.clientScope, Exit.void)).finally(() => {
session.runtime.dispose();
@@ -283,6 +292,7 @@
return;
}
+ this.lastActivityAt = performance.now();
markValueReceived();
try {
listener(value);You can send follow-ups to the cloud agent here.
| return { | ||
| dispose: () => transport.dispose(), | ||
| isHeartbeatFresh: () => false, |
There was a problem hiding this comment.
Heartbeat check always false
Medium Severity
createWsRpcClient hardcodes isHeartbeatFresh to always return false, so browser-resume reconnect logic never treats any environment connection as healthy and always triggers reconnect() for every connected environment after tab focus returns.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3ee6fbb. Configure here.
- Ignore intentional close events during transport disposal - Track heartbeat freshness from pong traffic and reset on reconnect - Propagate request lifecycle hooks through the RPC client
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 16 total unresolved issues (including 14 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Missing persisted terminal id migration
- Added migrateTerminalId and migrateThreadTerminalUiState helpers to the migration function that remap the legacy 'default' terminal id to 'term-1' in terminalIds, activeTerminalId, and group terminalIds during store hydration.
- ✅ Fixed: Browser tests use stale terminal id
- Updated the terminalOpen mock fallback and the seeded terminal UI state in the browser test to use DEFAULT_TERMINAL_ID instead of the hardcoded legacy 'default' string.
Or push these changes by commenting:
@cursor push 7c3f8ee714
Preview (7c3f8ee714)
diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx
--- a/apps/web/src/components/ChatView.browser.tsx
+++ b/apps/web/src/components/ChatView.browser.tsx
@@ -1087,7 +1087,7 @@
if (tag === WS_METHODS.terminalOpen) {
return {
threadId: typeof body.threadId === "string" ? body.threadId : THREAD_ID,
- terminalId: typeof body.terminalId === "string" ? body.terminalId : "default",
+ terminalId: typeof body.terminalId === "string" ? body.terminalId : DEFAULT_TERMINAL_ID,
cwd: typeof body.cwd === "string" ? body.cwd : "/repo/project",
worktreePath:
typeof body.worktreePath === "string"
@@ -1944,10 +1944,12 @@
[THREAD_KEY]: {
terminalOpen: true,
terminalHeight: 280,
- terminalIds: ["default"],
- activeTerminalId: "default",
- terminalGroups: [{ id: "group-default", terminalIds: ["default"] }],
- activeTerminalGroupId: "group-default",
+ terminalIds: [DEFAULT_TERMINAL_ID],
+ activeTerminalId: DEFAULT_TERMINAL_ID,
+ terminalGroups: [
+ { id: `group-${DEFAULT_TERMINAL_ID}`, terminalIds: [DEFAULT_TERMINAL_ID] },
+ ],
+ activeTerminalGroupId: `group-${DEFAULT_TERMINAL_ID}`,
},
},
});
diff --git a/apps/web/src/terminalUiStateStore.ts b/apps/web/src/terminalUiStateStore.ts
--- a/apps/web/src/terminalUiStateStore.ts
+++ b/apps/web/src/terminalUiStateStore.ts
@@ -12,6 +12,7 @@
import { resolveStorage } from "./lib/storage";
import {
DEFAULT_THREAD_TERMINAL_HEIGHT,
+ DEFAULT_THREAD_TERMINAL_ID,
MAX_TERMINALS_PER_GROUP,
type ThreadTerminalGroup,
} from "./types";
@@ -33,6 +34,20 @@
terminalStateByThreadKey?: Record<string, ThreadTerminalUiState>;
}
+function migrateTerminalId(id: string): string {
+ return id === "default" ? DEFAULT_THREAD_TERMINAL_ID : id;
+}
+
+function migrateThreadTerminalUiState(state: ThreadTerminalUiState): ThreadTerminalUiState {
+ const terminalIds = state.terminalIds.map(migrateTerminalId);
+ const activeTerminalId = migrateTerminalId(state.activeTerminalId);
+ const terminalGroups: ThreadTerminalGroup[] = state.terminalGroups.map((group) => ({
+ id: group.id,
+ terminalIds: group.terminalIds.map(migrateTerminalId),
+ }));
+ return { ...state, terminalIds, activeTerminalId, terminalGroups };
+}
+
export function migratePersistedTerminalUiStateStoreState(
persistedState: unknown,
_version: number,
@@ -45,9 +60,9 @@
const persistedUiStateByThreadKey =
candidate.terminalUiStateByThreadKey ?? candidate.terminalStateByThreadKey ?? {};
const terminalUiStateByThreadKey = Object.fromEntries(
- Object.entries(persistedUiStateByThreadKey).filter(([threadKey]) =>
- parseScopedThreadKey(threadKey),
- ),
+ Object.entries(persistedUiStateByThreadKey)
+ .filter(([threadKey]) => parseScopedThreadKey(threadKey))
+ .map(([threadKey, state]) => [threadKey, migrateThreadTerminalUiState(state)]),
);
return { terminalUiStateByThreadKey };You can send follow-ups to the cloud agent here.
| ); | ||
|
|
||
| return { terminalUiStateByThreadKey }; | ||
| } |
There was a problem hiding this comment.
Missing persisted terminal id migration
Medium Severity
Persisted terminal UI state can still use the old primary id default after the default becomes term-1. The store migration only filters thread keys and never rewrites terminalIds, activeTerminalId, or group membership. Until server metadata reconciliation runs, attach, write, and resize can target default while the server’s primary session is term-1, so the drawer can show the wrong or empty shell after upgrade.
Reviewed by Cursor Bugbot for commit 1e929ad. Configure here.
| @@ -1068,6 +1100,7 @@ function resolveWsRpc(body: NormalizedWsRpcRequestBody): unknown { | |||
| history: "", | |||
| exitCode: null, | |||
| exitSignal: null, | |||
| label: "Terminal 1", | |||
There was a problem hiding this comment.
Browser tests use stale terminal id
Low Severity
Browser harnesses still seed and mock the legacy id default while terminal metadata fixtures use DEFAULT_TERMINAL_ID (term-1). The WS terminalOpen mock also defaults missing terminalId to default. Tests can pass even when UI and RPC layers disagree on the primary shell id, which no longer matches the contract enforced elsewhere in this PR.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1e929ad. Configure here.
- Add new project screens for local folders and remote repositories - Support source-control provider lookup, clone destination selection, and folder browsing - Extract shared add-project path helpers and add coverage for the new runtime logic
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 19 total unresolved issues (including 16 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Metadata overrides attach exit status
- Changed combineSessionState to prefer buffer terminal statuses (exited/closed/error) over stale metadata summary status when the buffer has been updated by the attach stream (version > 0).
- ✅ Fixed: Attach locks before worktree loads
- Added selectedThreadDetail?.worktreePath to the bootstrap useEffect dependency array so the terminal re-attaches with the correct worktree path when thread detail hydrates after the initial attach.
- ✅ Fixed: Unused ThreadTerminalPanel export
- Deleted the unused ThreadTerminalPanel.tsx file which contained duplicate attach logic not imported anywhere in the codebase.
Or push these changes by commenting:
@cursor push 34938f8253
Preview (34938f8253)
diff --git a/apps/mobile/src/features/terminal/ThreadTerminalPanel.tsx b/apps/mobile/src/features/terminal/ThreadTerminalPanel.tsx
deleted file mode 100644
--- a/apps/mobile/src/features/terminal/ThreadTerminalPanel.tsx
+++ /dev/null
@@ -1,175 +1,0 @@
-import { DEFAULT_TERMINAL_ID, type EnvironmentId, type ThreadId } from "@t3tools/contracts";
-import { SymbolView } from "expo-symbols";
-import { memo, useCallback, useEffect, useRef, useState } from "react";
-import { Pressable, View } from "react-native";
-
-import { AppText as Text } from "../../components/AppText";
-import { getEnvironmentClient } from "../../state/environment-session-registry";
-import {
- attachTerminalSession,
- useTerminalSession,
- useTerminalSessionTarget,
-} from "../../state/use-terminal-session";
-import { TerminalSurface } from "./NativeTerminalSurface";
-import { hasNativeTerminalSurface } from "./nativeTerminalModule";
-import { terminalDebugLog } from "./terminalDebugLog";
-
-interface ThreadTerminalPanelProps {
- readonly environmentId: EnvironmentId;
- readonly threadId: ThreadId;
- readonly cwd: string;
- readonly worktreePath: string | null;
- readonly visible: boolean;
- readonly onClose: () => void;
-}
-
-const DEFAULT_TERMINAL_COLS = 80;
-const DEFAULT_TERMINAL_ROWS = 24;
-
-export const ThreadTerminalPanel = memo(function ThreadTerminalPanel(
- props: ThreadTerminalPanelProps,
-) {
- const nativeTerminalAvailable = hasNativeTerminalSurface();
- const terminalId = DEFAULT_TERMINAL_ID;
- const target = useTerminalSessionTarget({
- environmentId: props.environmentId,
- threadId: props.threadId,
- terminalId,
- });
- const terminal = useTerminalSession(target);
- const [lastGridSize, setLastGridSize] = useState({
- cols: DEFAULT_TERMINAL_COLS,
- rows: DEFAULT_TERMINAL_ROWS,
- });
- const lastGridSizeRef = useRef(lastGridSize);
- lastGridSizeRef.current = lastGridSize;
-
- const terminalKey = `${props.environmentId}:${props.threadId}:${terminalId}`;
- const isRunning = terminal.status === "running" || terminal.status === "starting";
-
- useEffect(() => {
- if (!props.visible) {
- return;
- }
-
- const client = getEnvironmentClient(props.environmentId);
- if (!client) {
- terminalDebugLog("panel:attach-skip", {
- reason: "no-environment-client",
- environmentId: props.environmentId,
- });
- return;
- }
-
- terminalDebugLog("panel:attach", {
- environmentId: props.environmentId,
- threadId: props.threadId,
- terminalId,
- });
-
- return attachTerminalSession({
- environmentId: props.environmentId,
- client,
- terminal: {
- threadId: props.threadId,
- terminalId,
- cwd: props.cwd,
- worktreePath: props.worktreePath,
- cols: lastGridSizeRef.current.cols,
- rows: lastGridSizeRef.current.rows,
- },
- });
- }, [
- props.cwd,
- props.environmentId,
- props.threadId,
- props.worktreePath,
- props.visible,
- terminalId,
- ]);
-
- const handleInput = useCallback(
- (data: string) => {
- const client = getEnvironmentClient(props.environmentId);
- if (!client || !isRunning) {
- return;
- }
-
- void client.terminal.write({
- threadId: props.threadId,
- terminalId,
- data,
- });
- },
- [isRunning, props.environmentId, props.threadId, terminalId],
- );
-
- const handleResize = useCallback(
- (size: { readonly cols: number; readonly rows: number }) => {
- if (size.cols === lastGridSize.cols && size.rows === lastGridSize.rows) {
- return;
- }
-
- setLastGridSize(size);
- const client = getEnvironmentClient(props.environmentId);
- if (!client || !isRunning) {
- return;
- }
-
- void client.terminal.resize({
- threadId: props.threadId,
- terminalId,
- cols: size.cols,
- rows: size.rows,
- });
- },
- [
- isRunning,
- lastGridSize.cols,
- lastGridSize.rows,
- props.environmentId,
- props.threadId,
- terminalId,
- ],
- );
-
- if (!props.visible) {
- return null;
- }
-
- return (
- <View className="absolute inset-x-3 bottom-28 top-28 overflow-hidden rounded-[8px] border border-white/10 bg-neutral-950 shadow-2xl">
- <View className="flex-row items-center justify-between border-b border-white/10 px-3 py-2">
- <View className="min-w-0 flex-1">
- <Text className="font-t3-bold text-[13px] text-neutral-100" numberOfLines={1}>
- Terminal
- </Text>
- <Text className="text-[11px] text-neutral-500" numberOfLines={1}>
- {nativeTerminalAvailable ? "Native Ghostty surface" : "Text fallback active"}
- </Text>
- </View>
- <View className="flex-row items-center gap-2">
- {terminal.error ? (
- <Text className="max-w-44 text-right text-[11px] text-red-300" numberOfLines={1}>
- {terminal.error}
- </Text>
- ) : null}
- <Pressable
- className="h-8 w-8 items-center justify-center rounded-[8px] bg-white/10"
- onPress={props.onClose}
- >
- <SymbolView name="xmark" size={13} tintColor="#e5e5e5" type="monochrome" />
- </Pressable>
- </View>
- </View>
- <TerminalSurface
- terminalKey={terminalKey}
- buffer={terminal.buffer}
- isRunning={isRunning}
- onInput={handleInput}
- onResize={handleResize}
- style={{ flex: 1 }}
- />
- </View>
- );
-});
\ No newline at end of file
diff --git a/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx b/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx
--- a/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx
+++ b/apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx
@@ -725,6 +725,7 @@
router,
selectedThread?.environmentId,
selectedThread?.id,
+ selectedThreadDetail?.worktreePath,
selectedThreadProject?.workspaceRoot,
terminalId,
]);
diff --git a/packages/client-runtime/src/terminalSessionState.ts b/packages/client-runtime/src/terminalSessionState.ts
--- a/packages/client-runtime/src/terminalSessionState.ts
+++ b/packages/client-runtime/src/terminalSessionState.ts
@@ -242,16 +242,24 @@
return Date.parse(left) >= Date.parse(right) ? left : right;
}
+function isBufferTerminalStatus(buffer: TerminalBufferState): boolean {
+ return (
+ buffer.version > 0 &&
+ (buffer.status === "exited" || buffer.status === "closed" || buffer.status === "error")
+ );
+}
+
function combineSessionState(
summary: TerminalSummary | null,
buffer: TerminalBufferState,
): TerminalSessionState {
+ const bufferTerminal = isBufferTerminalStatus(buffer);
return {
summary,
buffer: buffer.buffer,
- status: summary?.status ?? buffer.status,
+ status: bufferTerminal ? buffer.status : (summary?.status ?? buffer.status),
error: buffer.error,
- hasRunningSubprocess: summary?.hasRunningSubprocess ?? false,
+ hasRunningSubprocess: bufferTerminal ? false : (summary?.hasRunningSubprocess ?? false),
updatedAt: latestTimestamp(summary?.updatedAt ?? null, buffer.updatedAt),
version: buffer.version,
};You can send follow-ups to the cloud agent here.
| hasRunningSubprocess: summary?.hasRunningSubprocess ?? false, | ||
| updatedAt: latestTimestamp(summary?.updatedAt ?? null, buffer.updatedAt), | ||
| version: buffer.version, | ||
| }; |
There was a problem hiding this comment.
Metadata overrides attach exit status
Medium Severity
In combineSessionState, when metadata summary is present, status always comes from summary.status and ignores the attach buffer’s status. After an attach-stream exited or error event, stale metadata can still say running, so clients show a live terminal and accept input while the attach buffer already recorded exit.
Reviewed by Cursor Bugbot for commit 907f4d7. Configure here.
| selectedThread?.id, | ||
| selectedThreadProject?.workspaceRoot, | ||
| terminalId, | ||
| ]); |
There was a problem hiding this comment.
Attach locks before worktree loads
Medium Severity
The bootstrap useEffect sets hasOpenedRef and attaches once, but its dependency list omits selectedThreadDetail?.worktreePath and thread worktree fields that attachTerminal uses in resolveTerminalOpenLocation. If thread detail arrives after the first attach, the shell can stay bound to the workspace root instead of the thread worktree.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 907f4d7. Configure here.
| /> | ||
| </View> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Unused ThreadTerminalPanel export
Low Severity
ThreadTerminalPanel is exported and implements its own attachTerminalSession subscription, but nothing in the mobile app imports it. That leaves duplicate attach logic alongside ThreadTerminalRouteScreen and risks future drift if one path is updated and the other is not.
Reviewed by Cursor Bugbot for commit 907f4d7. Configure here.
- Extract shared new-task sheet header - Refresh add-project form spacing and controls - Align project creation screens with the new sheet layout
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
There are 23 total unresolved issues (including 19 from previous reviews).
Bugbot Autofix prepared fixes for 2 of the 4 issues found in the latest run.
- ✅ Fixed: Idle global terminal event stream
- Made the terminal.onEvent subscription conditional on applyTerminalEvent being provided, so connections without a handler no longer open a redundant WebSocket stream.
- ✅ Fixed: Terminal buffers leak after remove
- Added buffer reset to EMPTY_TERMINAL_BUFFER_STATE in the metadata snapshot pruning path so terminals no longer reported by the server have their retained string data (up to 512KB each) released; the per-terminal remove path intentionally preserves the buffer as local closed state per existing design.
Or push these changes by commenting:
@cursor push 1cc9019479
Preview (1cc9019479)
diff --git a/apps/web/src/environments/runtime/connection.ts b/apps/web/src/environments/runtime/connection.ts
--- a/apps/web/src/environments/runtime/connection.ts
+++ b/apps/web/src/environments/runtime/connection.ts
@@ -143,11 +143,13 @@
},
);
- const unsubTerminalEvent = input.client.terminal.onEvent(
- (event: Parameters<Parameters<WsRpcClient["terminal"]["onEvent"]>[0]>[0]) => {
- input.applyTerminalEvent?.(event, environmentId);
- },
- );
+ const unsubTerminalEvent = input.applyTerminalEvent
+ ? input.client.terminal.onEvent(
+ (event: Parameters<Parameters<WsRpcClient["terminal"]["onEvent"]>[0]>[0]) => {
+ input.applyTerminalEvent!(event, environmentId);
+ },
+ )
+ : () => undefined;
const cleanup = () => {
disposed = true;
diff --git a/packages/client-runtime/src/terminalSessionState.ts b/packages/client-runtime/src/terminalSessionState.ts
--- a/packages/client-runtime/src/terminalSessionState.ts
+++ b/packages/client-runtime/src/terminalSessionState.ts
@@ -402,6 +402,10 @@
for (const key of Object.keys(next)) {
if (!retainedKeys.has(key)) {
+ const removedEntry = next[key];
+ if (removedEntry) {
+ setBuffer(removedEntry.target, EMPTY_TERMINAL_BUFFER_STATE);
+ }
delete next[key];
}
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 4db082a. Configure here.
| | { readonly kind: "open" } { | ||
| if (!input.hasThread || !input.hasWorkspaceRoot || input.hasOpened) { | ||
| return { kind: "idle" }; | ||
| } |
There was a problem hiding this comment.
Opened flag blocks re-attach
High Severity
resolveTerminalRouteBootstrap returns idle whenever hasOpened is true, before it checks whether the attach stream still needs to run. After a session ends or the stream stops while the route stays mounted, hasOpenedRef can remain true with an empty buffer, so terminal.attach never runs again and the terminal stays blank.
Reviewed by Cursor Bugbot for commit 4db082a. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The attach subscription is a persistent WebSocket stream that stays open even after PTY exit (receiving restarted/activity events); hasOpenedRef is properly reset on effect cleanup, terminalKey change, and error, and the transport layer handles reconnection with automatic resubscription.
You can send follow-ups to the cloud agent here.
| @@ -145,7 +145,7 @@ export function createEnvironmentConnection( | |||
|
|
|||
| const unsubTerminalEvent = input.client.terminal.onEvent( | |||
| (event: Parameters<Parameters<WsRpcClient["terminal"]["onEvent"]>[0]>[0]) => { | |||
| input.applyTerminalEvent(event, environmentId); | |||
| input.applyTerminalEvent?.(event, environmentId); | |||
There was a problem hiding this comment.
Idle global terminal event stream
Medium Severity
Every web environment connection still subscribes to client.terminal.onEvent, but createEnvironmentConnectionHandlers no longer supplies applyTerminalEvent, so those events are discarded. Terminal output and presence now use subscribeTerminalMetadata plus per-session terminal.attach, leaving a redundant global stream on each connection.
Reviewed by Cursor Bugbot for commit 4db082a. Configure here.
| * `term-N`; there's no "default" intrinsic. Kept as a named constant so callers | ||
| * that want "the primary shell" don't hardcode `"term-1"`. | ||
| */ | ||
| export const DEFAULT_TERMINAL_ID = "term-1"; |
There was a problem hiding this comment.
Terminal history orphaned after ID
High Severity
The default client terminal id changed from default to term-1, but persisted server terminal history files are still named from the encoded terminalId. Existing installs keep history under the old id, so attach/open with term-1 starts with an empty buffer instead of prior scrollback.
Reviewed by Cursor Bugbot for commit 4db082a. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The server's historyPath function maps DEFAULT_TERMINAL_ID to the unsuffixed path ${threadPart}.log regardless of the constant's string value, so both "default" and "term-1" resolve to the same file path when they are the default terminal.
You can send follow-ups to the cloud agent here.
|
|
||
| const next = { ...getMetadata(environmentId) }; | ||
| delete next[keyFromKnownTarget(knownTarget)]; | ||
| setMetadata(environmentId, next); |
There was a problem hiding this comment.
Terminal buffers leak after remove
Medium Severity
When terminal metadata is removed (per-terminal remove events or metadata snapshot pruning), only the metadata map entry is deleted. The corresponding terminalSessionBufferAtom keeps its trimmed history (up to 512KB per session), so deleted or archived threads can leave retained buffers in memory.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4db082a. Configure here.
- Move remote HTTP calls into `@t3tools/client-runtime` - Let mobile parse hosted pairing links and use shared runtime - Keep web and mobile remote connection flows aligned
| const url = new URL(normalizedInput); |
There was a problem hiding this comment.
🟢 Low src/remote.ts:18
normalizeRemoteBaseUrl throws TypeError: Invalid URL when given a protocol-relative URL like //example.com. The condition on line 15 allows these through unchanged, but new URL("//example.com") without a base URL is invalid. Previously, window.location.origin was passed as the base. Consider restoring the base URL parameter to resolve protocol-relative URLs correctly.
- const url = new URL(normalizedInput);
+ const url = new URL(normalizedInput, window.location.origin);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/shared/src/remote.ts around line 18:
`normalizeRemoteBaseUrl` throws `TypeError: Invalid URL` when given a protocol-relative URL like `//example.com`. The condition on line 15 allows these through unchanged, but `new URL("//example.com")` without a base URL is invalid. Previously, `window.location.origin` was passed as the base. Consider restoring the base URL parameter to resolve protocol-relative URLs correctly.
Evidence trail:
packages/shared/src/remote.ts lines 8-23 (REVIEWED_COMMIT) — normalizeRemoteBaseUrl function. Line 15: `trimmed.startsWith('//')` passes protocol-relative URLs through unchanged. Line 18: `new URL(normalizedInput)` called without a base URL argument. WHATWG URL spec: `new URL('//example.com')` without a base throws TypeError. Node.js docs confirm WHATWG URL parsing: https://nodejs.org/api/url.html#new-urlinput-base
2 participants
Summary
packages/client-runtimeandpackages/sharedso web and mobile can share the same behavior.Testing
Note
High Risk
High risk due to large new iOS native surface implementation (
T3ReviewDiffView.swift) plus vendored native terminal framework, and new CI enforcement for native linting which can break builds/tooling on macOS runners.Overview
Adds a new
apps/mobileExpo app scaffold with variant-awareapp.config.ts, Metro/Babel setup (including Uniwind + Shiki resolution), styling tokens inglobal.css, and supporting repo hygiene files (.gitignore,.editorconfig,README,eas.json).Introduces native iOS modules for mobile: a native review diff surface (
T3ReviewDiffSurfaceviaT3ReviewDiffModule.swift+ largeT3ReviewDiffView.swift) and a terminal module podspec that vendorsGhosttyKit.xcframework(with third-party notices).Updates tooling/CI to support mobile native code: adds a macOS
mobile_native_static_analysisGitHub Actions job that installsswiftlint/ktlint/detektfromapps/mobile/Brewfileand runsscripts/mobile-native-static-check.ts, expandsoxfmt/oxlintignore patterns for generatedapps/mobile/android/iosfolders, and documents the newbun lint:mobilerequirement inAGENTS.md.Reviewed by Cursor Bugbot for commit 554a1d2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add mobile app foundation with terminal, review, and git screens to T3 Code
apps/mobile) with Expo Router navigation, global atom registry, and environment connection management including QR-code pairing and secure storage of credentials.T3TerminalSurface), with a fullThreadTerminalRouteScreenmanaging session attach, input, resize, and font-size controls.T3ReviewDiffSurfaceiOS module, Shiki-based syntax highlighting, word-level diff ranges, and review comment composition with inline diff previews.vcsActionManageranduseSelectedThreadGitActionshook that orchestrates operations via the server.terminalStateStoretoterminalUiStateStore, introducesterminalSessionStatebacked byattachStream/subscribeMetadataRPC, and replaces React Query-based git/VCS hooks with newuseVcsStatus,useVcsRefs, anduseCheckpointDiffstate managers.ReviewServiceandgetReviewDiffPreviewtoGitVcsDriverCore, exposing working-tree and branch-range diff sources over a newreview.getDiffPreviewWS RPC endpoint.terminal.attachandsubscribeTerminalMetadatastreaming endpoints; terminal session snapshots now carry monotonically increasing sequence numbers and dynamic labels (including child process names).WsTransport,wsRpcProtocol(configurable backoff and telemetry),threadDetailState,vcsStatusState,vcsRefState,vcsActionState,terminalSessionState,checkpointDiffState,composerPathSearchState, andarchivedThreadsState.DEFAULT_TERMINAL_IDchanges from"default"to"term-1"andterminalIdis now required on allTerminalSessionInput; any clients using the old default or omitting the field will break.Macroscope summarized 554a1d2.