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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 20 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
| }; | ||
| return pipe( | ||
| parsed.connections ?? [], | ||
| Arr.filter((c) => !!c.environmentId && !!c.bearerToken?.trim()), |
There was a problem hiding this comment.
🟢 Low lib/storage.ts:29
When stored JSON contains malformed entries (e.g., null values in the connections array), the filter callback crashes with a TypeError on line 29 because it accesses c.environmentId before checking if c is nullish. Consider adding a null check: Arr.filter((c) => !!c && !!c.environmentId && !!c.bearerToken?.trim()).
| Arr.filter((c) => !!c.environmentId && !!c.bearerToken?.trim()), | |
| Arr.filter((c) => !!c && !!c.environmentId && !!c.bearerToken?.trim()), |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/lib/storage.ts around line 29:
When stored JSON contains malformed entries (e.g., `null` values in the `connections` array), the filter callback crashes with a `TypeError` on line 29 because it accesses `c.environmentId` before checking if `c` is nullish. Consider adding a null check: `Arr.filter((c) => !!c && !!c.environmentId && !!c.bearerToken?.trim())`.
Evidence trail:
apps/mobile/src/lib/storage.ts lines 23-31 at REVIEWED_COMMIT - the filter callback `(c) => !!c.environmentId && !!c.bearerToken?.trim()` accesses c.environmentId without first checking if c is nullish. The data comes from JSON.parse with only a type assertion (line 23-25), not runtime validation.
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.
| while ((ranges[rangeIndex]?.end ?? 0) <= tokenStart) { | ||
| rangeIndex += 1; | ||
| } |
There was a problem hiding this comment.
🔴 Critical review/reviewWordDiffs.ts:211
In applyDiffRangesToTokens, when rangeIndex exceeds ranges.length, the while loop at line 211 becomes infinite because (ranges[rangeIndex]?.end ?? 0) evaluates to 0 when rangeIndex is out of bounds, and 0 <= tokenStart is always true for non-negative offsets. This causes an infinite loop when processing tokens that appear after the last range. Consider changing the condition to check rangeIndex < ranges.length before accessing the range.
- while ((ranges[rangeIndex]?.end ?? 0) <= tokenStart) {
- rangeIndex += 1;
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/review/reviewWordDiffs.ts around lines 211-213:
In `applyDiffRangesToTokens`, when `rangeIndex` exceeds `ranges.length`, the while loop at line 211 becomes infinite because `(ranges[rangeIndex]?.end ?? 0)` evaluates to `0` when `rangeIndex` is out of bounds, and `0 <= tokenStart` is always true for non-negative offsets. This causes an infinite loop when processing tokens that appear after the last range. Consider changing the condition to check `rangeIndex < ranges.length` before accessing the range.
Evidence trail:
apps/mobile/src/features/threads/review/reviewWordDiffs.ts lines 188-220 (REVIEWED_COMMIT): Function `applyDiffRangesToTokens` shows line 211 with the while loop `while ((ranges[rangeIndex]?.end ?? 0) <= tokenStart) { rangeIndex += 1; }`. The early return at line 193 checks `ranges.length === 0` but doesn't prevent the infinite loop when ranges exist but have all been exhausted during processing.
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"
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 15 total unresolved issues (including 14 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: CI lint step silently skips execution with
node- Changed
nodetobunin the CI workflow so thatimport.meta.mainevaluates totrueand the mobile native static checks actually execute.
- Changed
Or push these changes by commenting:
@cursor push d0b1e7d285
Preview (d0b1e7d285)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -99,7 +99,7 @@
run: brew bundle install --file apps/mobile/Brewfile
- name: Lint mobile native sources
- run: node scripts/mobile-native-static-check.ts
+ run: bun scripts/mobile-native-static-check.ts
release_smoke:
name: Release SmokeYou can send follow-ups to the cloud agent here.
| run: brew bundle install --file apps/mobile/Brewfile | ||
|
|
||
| - name: Lint mobile native sources | ||
| run: node scripts/mobile-native-static-check.ts |
There was a problem hiding this comment.
CI lint step silently skips execution with node
Medium Severity
The CI step runs node scripts/mobile-native-static-check.ts, but the script's entry-point guard uses import.meta.main — a Bun/Deno API that doesn't exist in Node.js (it evaluates to undefined/falsy). This means the lint step will load the module but never call Command.run, silently passing without executing any checks. The rest of the project uses bun to run scripts; this step likely needs bun instead of node.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ef1ed88. Configure here.
- 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>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
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 16 total unresolved issues (including 15 from previous reviews).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Style default value is unreachable due to cap
- Lowered the fileHeaderCountGap fallback default from 6 to 4 so it matches the min(..., 4) cap applied at both usage sites.
Or push these changes by commenting:
@cursor push 4682d2d692
Preview (4682d2d692)
diff --git a/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift b/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift
--- a/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift
+++ b/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift
@@ -216,7 +216,7 @@
fileHeaderHorizontalPadding: metric(payload?.fileHeaderHorizontalPadding, fallback: 12),
fileHeaderPathRightPadding: metric(payload?.fileHeaderPathRightPadding, fallback: 128),
fileHeaderCountColumnWidth: metric(payload?.fileHeaderCountColumnWidth, fallback: 42),
- fileHeaderCountGap: metric(payload?.fileHeaderCountGap, fallback: 6),
+ fileHeaderCountGap: metric(payload?.fileHeaderCountGap, fallback: 4),
codeFontSize: metric(payload?.codeFontSize, fallback: 12),
codeFontWeight: fontWeight(payload?.codeFontWeight, fallback: .bold),
lineNumberFontSize: metric(payload?.lineNumberFontSize, fallback: 11),You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit fd93d7f. Configure here.
| let addText = "+\(additions)" | ||
| let deleteWidth = textWidth(deleteText, font: fileHeaderMetaFont) | ||
| let addWidth = textWidth(addText, font: fileHeaderMetaFont) | ||
| let countsGap = min(style.fileHeaderCountGap, 4) |
There was a problem hiding this comment.
Style default value is unreachable due to cap
Low Severity
The fileHeaderCountGap style property has a fallback default of 6, but every usage site caps it with min(style.fileHeaderCountGap, 4). The declared default of 6 can never be realized — it always resolves to 4. Either the fallback should be lowered to 4 to match the cap, or the cap should be raised to at least 6 so the configured default is respected.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit fd93d7f. Configure here.
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>
Co-authored-by: codex <codex@users.noreply.github.com>
| return value.endsWith("\n") ? value.slice(0, -1) : value; | ||
| } | ||
|
|
||
| function pushOrJoinSpan(input: { |
There was a problem hiding this comment.
🟡 Medium review/reviewWordDiffs.ts:22
When pushOrJoinSpan processes a single-character neutral token that follows a highlighted span, the condition on line 40 (isNeutral && operation.value.length === 1 && !lastSpanIsNeutral) incorrectly merges it into the highlighted span while keeping kind=1. This causes spansToRanges to include the neutral character in the highlight range. For example, diffing "a." vs ".b" produces deletion operations [{removed: "a"}, {common: "."}, {added: "b"}]. The . gets merged into the deletion span as [1, "a."] instead of staying neutral as [0, "."], so the output incorrectly highlights both "a" and "." when only "a" was removed.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/review/reviewWordDiffs.ts around line 22:
When `pushOrJoinSpan` processes a single-character neutral token that follows a highlighted span, the condition on line 40 `(isNeutral && operation.value.length === 1 && !lastSpanIsNeutral)` incorrectly merges it into the highlighted span while keeping `kind=1`. This causes `spansToRanges` to include the neutral character in the highlight range. For example, diffing `"a."` vs `".b"` produces deletion operations `[{removed: "a"}, {common: "."}, {added: "b"}]`. The `.` gets merged into the deletion span as `[1, "a."]` instead of staying neutral as `[0, "."]`, so the output incorrectly highlights both "a" and "." when only "a" was removed.
Evidence trail:
apps/mobile/src/features/review/reviewWordDiffs.ts lines 38-44 (condition and merge logic), lines 49-62 (spansToRanges), lines 64-81 (mergeNearbyRanges — similar philosophy of absorbing small gaps). apps/mobile/src/features/review/reviewWordDiffs.test.ts lines 7-16 (test named 'joins adjacent word replacements across a single shared separator' confirming intentional bridging behavior). Commit d686613e34 — original introduction of the file.
| const branches = | ||
| gitState.selectedThreadBranches.length > 0 | ||
| ? gitState.selectedThreadBranches | ||
| : await gitActions.refreshSelectedThreadBranches(); | ||
| const newBranchName = resolveAutoFeatureBranchName( | ||
| branches.filter((branch) => !branch.isRemote).map((branch) => branch.name), | ||
| ); | ||
| await gitActions.onCreateSelectedThreadBranch(newBranchName); | ||
| await gitActions.onRunSelectedThreadGitAction({ action: confirmAction }); | ||
| }, [confirmAction, gitActions, gitState.selectedThreadBranches, includesCommit, params, router]); |
There was a problem hiding this comment.
🟢 Low git/GitConfirmSheet.tsx:72
movePendingActionToFeatureBranch calls onCreateSelectedThreadBranch and then immediately runs the git action without checking whether branch creation succeeded. Since onCreateSelectedThreadBranch swallows errors internally and returns void, a failed branch creation silently proceeds to run the action on the current (wrong) branch instead of the intended feature branch. Consider checking the branch creation result or restructuring the flow to ensure the action only runs after successful creation.
- const branches =
- gitState.selectedThreadBranches.length > 0
- ? gitState.selectedThreadBranches
- : await gitActions.refreshSelectedThreadBranches();
- const newBranchName = resolveAutoFeatureBranchName(
- branches.filter((branch) => !branch.isRemote).map((branch) => branch.name),
- );
- await gitActions.onCreateSelectedThreadBranch(newBranchName);
- await gitActions.onRunSelectedThreadGitAction({ action: confirmAction });
+ const branches =
+ gitState.selectedThreadBranches.length > 0
+ ? gitState.selectedThreadBranches
+ : await gitActions.refreshSelectedThreadBranches();
+ const newBranchName = resolveAutoFeatureBranchName(
+ branches.filter((branch) => !branch.isRemote).map((branch) => branch.name),
+ );
+ const created = await gitActions.onCreateSelectedThreadBranch(newBranchName);
+ if (created !== null) {
+ await gitActions.onRunSelectedThreadGitAction({ action: confirmAction });
+ }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/git/GitConfirmSheet.tsx around lines 72-81:
`movePendingActionToFeatureBranch` calls `onCreateSelectedThreadBranch` and then immediately runs the git action without checking whether branch creation succeeded. Since `onCreateSelectedThreadBranch` swallows errors internally and returns `void`, a failed branch creation silently proceeds to run the action on the current (wrong) branch instead of the intended feature branch. Consider checking the branch creation result or restructuring the flow to ensure the action only runs after successful creation.
Evidence trail:
apps/mobile/src/features/threads/git/GitConfirmSheet.tsx lines 64-81 (movePendingActionToFeatureBranch function, REVIEWED_COMMIT); apps/mobile/src/state/use-selected-thread-git-actions.ts lines 95-128 (runSelectedThreadGitMutation with try-catch returning null on error); apps/mobile/src/state/use-selected-thread-git-actions.ts lines 210-230 (onCreateSelectedThreadBranch awaiting runSelectedThreadGitMutation without returning result)
| } | ||
| } | ||
|
|
||
| export async function connectSavedEnvironment( |
There was a problem hiding this comment.
🟢 Low state/use-remote-environment-registry.ts:160
connectSavedEnvironment is not reentrant: if called twice for the same environmentId before the first call reaches setEnvironmentSession, the second call's disconnectEnvironment finds no session and the first call's transport, client, and connection are orphaned. The orphaned transport's callbacks continue firing and update state via environmentRuntimeManager.patch, causing conflicting state updates when the second call eventually overwrites the session.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/state/use-remote-environment-registry.ts around line 160:
`connectSavedEnvironment` is not reentrant: if called twice for the same `environmentId` before the first call reaches `setEnvironmentSession`, the second call's `disconnectEnvironment` finds no session and the first call's `transport`, `client`, and `connection` are orphaned. The orphaned transport's callbacks continue firing and update state via `environmentRuntimeManager.patch`, causing conflicting state updates when the second call eventually overwrites the session.
Evidence trail:
apps/mobile/src/state/use-remote-environment-registry.ts lines 140-270 (connectSavedEnvironment and disconnectEnvironment), apps/mobile/src/state/environment-session-registry.ts lines 16-28 (setEnvironmentSession/removeEnvironmentSession — simple Map get/set/delete with no guards), apps/mobile/src/state/use-remote-environment-registry.ts line 461 (fire-and-forget call site via `void connectSavedEnvironment(...)`)
- Scope cleartext mobile config to development - Fix stale loads, ordering, and terminal reuse - Clean up terminal metadata subscriptions on failure
| const loadBranches = useCallback(async () => { |
There was a problem hiding this comment.
🟡 Medium threads/new-task-flow-provider.tsx:359
When selectedProjectKey is null and selectedProject falls back to the first project in projectsForEnvironment, the early return check selectedProjectKey !== projectKey compares the stale closure-captured values null and "env:project". Since null !== "env:project" is always true, loadBranches discards the fetched branches and returns early instead of updating state. The check fails to detect mid-flight selection changes because both values are captured at callback creation time, making it ineffective for staleness detection and actively harmful when no explicit project is selected.
- const loadBranches = useCallback(async () => {
+ const loadBranches = useCallback(async () => {
+ const currentSelectedProjectKey = selectedProjectKeyRef.current;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/new-task-flow-provider.tsx around line 359:
When `selectedProjectKey` is `null` and `selectedProject` falls back to the first project in `projectsForEnvironment`, the early return check `selectedProjectKey !== projectKey` compares the stale closure-captured values `null` and `"env:project"`. Since `null !== "env:project"` is always `true`, `loadBranches` discards the fetched branches and returns early instead of updating state. The check fails to detect mid-flight selection changes because both values are captured at callback creation time, making it ineffective for staleness detection and actively harmful when no explicit project is selected.
Evidence trail:
apps/mobile/src/features/threads/new-task-flow-provider.tsx lines 268-273 (selectedProject fallback logic), lines 343-348 (selectEnvironment sets selectedProjectKey to null), lines 359-394 (loadBranches callback with the staleness check at line 375), line 394 (dependency array confirming closure capture of selectedProject and selectedProjectKey).
2 participants
Summary
packages/client-runtimeandpackages/sharedso web and mobile can share the same behavior.Testing
Note
Medium Risk
Medium risk due to introducing substantial new mobile/native code and vendored iOS framework plus new CI checks; failures are likely to surface in build/lint/tooling rather than existing web/desktop behavior.
Overview
Adds a new
apps/mobileExpo app scaffold with variant-basedapp.config.ts, Metro/Uniwind/Tailwind setup, EAS build profiles, and mobile-specific docs/config/ignore files.Introduces two local native Expo modules:
t3-review-diff(large Swift-based diff renderer surface with theming, token patching, scrolling/gesture handling, and debug events) andt3-terminal(podspec plus vendoredGhosttyKit.xcframeworkand licensing docs).Tightens tooling/CI around mobile native code by adding a macOS
mobile_native_static_analysisjob that installsswiftlint/ktlint/detektfromapps/mobile/Brewfileand runsscripts/mobile-native-static-check.ts, plus updatesAGENTS.mdandoxfmt/oxlintignore patterns (and disablesunicorn/no-array-sort).Reviewed by Cursor Bugbot for commit 168ec8a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add mobile app (React Native/Expo) with terminal, review diff, git controls, and remote connection support
apps/mobileas a new React Native/Expo application with navigation, theming, and a full feature set including thread management, git controls, review diffs, and terminal access via a native Ghostty-backed surface.packages/client-runtimemodules for WebSocket transport (WsTransport), environment connection lifecycle, and atom-backed state managers for VCS status/refs, terminal sessions, checkpoint diffs, archived threads, shell snapshots, and thread detail.attachStreamandsubscribeMetadataAPIs, dynamic child process label resolution, event sequence numbers, and a newReviewServiceexposinggetDiffPreviewvia the WebSocket RPC layer.packages/sharedandpackages/client-runtimeand migrates web app imports accordingly.terminalUiStateStore/vcsStatusStateabstractions backed by the new client-runtime managers;TerminalEventsubscriptions are replaced withterminal.attachandterminal.onMetadata.DEFAULT_TERMINAL_IDchanges from"default"to"term-1"andTerminalSessionInputnow requiresterminalIdto be explicitly provided — existing persisted state and integrations relying on the old default will need migration.Macroscope summarized 168ec8a.