T3 Code Mobile [WIP]#2013
Merged
Merged
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}
Contributor
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}
Contributor
ApprovabilityVerdict: Needs human review 7 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. |
Comment on lines
+25
to
+27
| const [status, setStatus] = useState<"loading" | "loaded" | "error">(() => | ||
| faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading", | ||
| ); |
Contributor
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.
Contributor
|
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. |
Contributor
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 }; |
Contributor
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 ?? ""); |
Contributor
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
Contributor
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.
Contributor
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.
juliusmarminge
added a commit
that referenced
this pull request
Apr 14, 2026
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
Contributor
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> | ||
| ); | ||
| } |
Contributor
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.
Comment on lines
+72
to
+78
| <SymbolView | ||
| name="xmark" | ||
| size={9} | ||
| tintColor="#ffffff" | ||
| type="monochrome" | ||
| weight="bold" | ||
| /> |
Contributor
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.
juliusmarminge
added a commit
that referenced
this pull request
Apr 15, 2026
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
Contributor
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.
Contributor
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.
Comment on lines
+48
to
+56
| if (input.status === "exited") { | ||
| return "Exited"; | ||
| } | ||
| if (input.status === "error") { | ||
| return "Error"; | ||
| } | ||
|
|
||
| return "Not started"; | ||
| } |
Contributor
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)
Comment on lines
+798
to
+808
| } | ||
| } finally { | ||
| highlightQueueActiveRef.current = false; | ||
| if ( | ||
| generation === highlightQueueGenerationRef.current && | ||
| highlightQueueRef.current.length > 0 | ||
| ) { | ||
| startHighlightQueueRunner(); | ||
| } | ||
| } | ||
| })(); |
Contributor
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.
juliusmarminge
added a commit
that referenced
this pull request
Apr 17, 2026
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
Comment on lines
+14
to
+16
| url.hash = new URLSearchParams([["token", c]]).toString(); | ||
| return url.toString(); | ||
| } catch { | ||
| return `${h}#token=${c}`; |
Contributor
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.
Suggested change
| 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
Comment on lines
+18
to
+36
| 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: "" }; | ||
| } | ||
| } |
Contributor
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: { |
Contributor
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"
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>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Comment on lines
+758
to
+761
| useEffect(() => { | ||
| if (!environment) return; | ||
| setPathInput(getAddProjectInitialQuery(environment.baseDirectory)); | ||
| }, [environment]); |
Contributor
There was a problem hiding this comment.
🟡 Medium projects/AddProjectScreen.tsx:758
The useEffect at line 758 depends on the entire environment object, but environment gets a new object identity whenever useEnvironmentOptions recomputes due to background data changes. This causes the effect to fire and reset pathInput to the initial query via getAddProjectInitialQuery, overwriting the user's typed input while they're still editing. The dependency should be environment?.baseDirectory instead of environment so the effect only runs when the actual base directory value changes.
- }, [environment]);
+ }, [environment?.baseDirectory]);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/projects/AddProjectScreen.tsx around lines 758-761:
The `useEffect` at line 758 depends on the entire `environment` object, but `environment` gets a new object identity whenever `useEnvironmentOptions` recomputes due to background data changes. This causes the effect to fire and reset `pathInput` to the initial query via `getAddProjectInitialQuery`, overwriting the user's typed input while they're still editing. The dependency should be `environment?.baseDirectory` instead of `environment` so the effect only runs when the actual base directory value changes.
Evidence trail:
apps/mobile/src/features/projects/AddProjectScreen.tsx lines 245-256: `useEnvironmentOptions` creates new objects on each memo recomputation via `.map()`.
apps/mobile/src/features/projects/AddProjectScreen.tsx lines 501-510: `useEnvironmentFromParam` does `.find()` on the result with no further memoization, returning a new object reference.
apps/mobile/src/features/projects/AddProjectScreen.tsx lines 758-761: `useEffect` depends on `[environment]` (object reference), calls `setPathInput(getAddProjectInitialQuery(environment.baseDirectory))` which resets user input.
apps/mobile/src/features/projects/AddProjectScreen.tsx lines 752-754: `pathInput` state initialized once, but the effect overwrites it on every `environment` reference change.
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>
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>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Comment on lines
+182
to
+195
| function splitTruncationMarker(diff: string): { | ||
| readonly text: string; | ||
| readonly truncated: boolean; | ||
| } { | ||
| const trimmed = diff.trimEnd(); | ||
| if (!trimmed.endsWith("[truncated]")) { | ||
| return { text: trimmed, truncated: false }; | ||
| } | ||
|
|
||
| return { | ||
| text: trimmed.replace(/\n*\[truncated\]\s*$/, "").trimEnd(), | ||
| truncated: true, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 Low review/reviewModel.ts:182
splitTruncationMarker treats any diff ending with [truncated] as server-truncated, even when that text is legitimate content like a code comment // [truncated]. The regex then corrupts the final line by stripping the fake marker from actual diff content. Consider requiring the marker to be on its own line to distinguish real server truncation from coincidental content.
function splitTruncationMarker(diff: string): {
readonly text: string;
readonly truncated: boolean;
} {
const trimmed = diff.trimEnd();
- if (!trimmed.endsWith("[truncated]")) {
+ if (!trimmed.endsWith("\n[truncated]") && trimmed !== "[truncated]") {
return { text: trimmed, truncated: false };
}
return {
- text: trimmed.replace(/\n*\[truncated\]\s*$/, "").trimEnd(),
+ text: trimmed.replace(/\n+\[truncated\]\s*$/, "").trimEnd(),
truncated: true,
};
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/review/reviewModel.ts around lines 182-195:
`splitTruncationMarker` treats any diff ending with `[truncated]` as server-truncated, even when that text is legitimate content like a code comment `// [truncated]`. The regex then corrupts the final line by stripping the fake marker from actual diff content. Consider requiring the marker to be on its own line to distinguish real server truncation from coincidental content.
Evidence trail:
apps/mobile/src/features/review/reviewModel.ts lines 182-195 (splitTruncationMarker function, REVIEWED_COMMIT) — endsWith check at line 187, regex strip at line 192. Caller at line 591 in buildReviewParsedDiff, with truncation notice shown at lines 596-598.
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>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/client-runtimeandpackages/sharedso web and mobile can share the same behavior.Testing
Note
High Risk
Large greenfield surface (native modules, vendored Ghostty, new RPC/review paths) plus cross-platform refactors to shared runtime and terminal ID defaults that can break persisted client state.
Overview
Adds a new Expo/React Native mobile client under
apps/mobile(dev/preview/production variants, EAS, Uniwind theming, Expo Router) that talks to the same backend via@t3tools/client-runtimeand@t3tools/shared.Native surfaces: an iOS Ghostty-backed terminal module (
t3-terminal, vendoredGhosttyKit.xcframework) and an iOS native review diff renderer (t3-review-diff) with JSON-driven rows, token patches, and gestures; review highlighting defaults to JavaScript/Shiki via Metro workspace resolution.Tooling & CI: new macOS job Mobile Native Static Analysis (
swiftlint/ktlint/detektviascripts/mobile-native-static-check.ts),bun lint:mobiledocumented inAGENTS.md, and formatter/linter ignore patterns for generatedios/androidtrees.The broader PR (per description) also refactors web/desktop connection, terminal IDs, VCS/git state, and server review/terminal streaming to align with the shared runtime—treat persisted terminal IDs and RPC contracts as migration-sensitive.
Reviewed by Cursor Bugbot for commit 9a20e0b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add mobile app with terminal, review diff, and Git action support
apps/mobileReact Native/Expo app with routing, navigation, and theming; includes home, thread, review, terminal, and connection management screensT3TerminalSurfacenative module with input/resize events and buffer replay logicT3ReviewDiffSurfacewith Shiki-based syntax highlighting, word-level diff ranges, and a review comment composer sheet with selection controllerpackages/contractswithterminal.attach,subscribeTerminalMetadata, andreview.getDiffPreviewRPC endpoints; updatesWsRpcClientand server WS layer accordinglyReviewServiceon the server with workspace-bound cwd validation andGitVcsDriver.getReviewDiffPreviewproducing dirty and branch-range diff sources with SHA-256 hashesterminalStateStoreto a newterminalUiStateStore(Zustand, persisted) with stable incremental terminal IDs (term-1,term-2, …) and server-side terminal open callsclient-runtimemodules:WsTransport,terminalSessionState,vcsStatusState,vcsRefState,vcsActionState,threadDetailState,checkpointDiffState,composerPathSearchState,archivedThreadsState,environmentConnection, and othersterminalIdis now required (no default) inTerminalSessionInput,TerminalWriteInput,TerminalResizeInput, andTerminalClearInput; callers not providing an explicit id will fail schema decodingMacroscope summarized 9a20e0b.