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}
| }): string | null { | ||
| if (!args.hasUpdateFeedConfig) { | ||
| return "Automatic updates are not available because no update feed is configured."; | ||
| } |
There was a problem hiding this comment.
Update feed check shadows more specific dev-build message
Medium Severity
The hasUpdateFeedConfig check is evaluated before isDevelopment || !isPackaged, so development builds without a dev-app-update.yml file (the common case) now receive "no update feed is configured" instead of the more informative "only available in packaged production builds" message. The more specific condition (dev/unpackaged) is masked by the more general one (no feed config). The tests adapted by always passing hasUpdateFeedConfig: true for dev scenarios, hiding this real-world behavior change.
Reviewed by Cursor Bugbot for commit fbcf04a. Configure here.
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.
| }; | ||
| void props.onUpdateModelSelection(updated); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Fast mode option applied to all providers indiscriminately
Medium Severity
The handleOptionsMenuAction for fast mode spreads options onto every provider's ModelSelection without checking the provider type, unlike the effort and context-window handlers which correctly guard with currentModelSelection.provider === "claudeAgent". This can attach unsupported fastMode options to providers that don't recognize them. The new-task draft screen correctly limits fast mode to claudeAgent and codex only.
Reviewed by Cursor Bugbot for commit a77ea8f. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 6 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
| const [status, setStatus] = useState<"loading" | "loaded" | "error">(() => | ||
| faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading", | ||
| ); |
There was a problem hiding this comment.
🟡 Medium components/ProjectFavicon.tsx:25
When faviconUrl changes due to prop updates (e.g., httpBaseUrl or workspaceRoot), status remains at its previous value because useState only initializes on mount. If the previous URL had finished loading (status === "loaded"), the new image renders immediately without showing the folder fallback during its load.
const [status, setStatus] = useState<"loading" | "loaded" | "error">(() =>
faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading",
);
+ useEffect(() => {
+ setStatus(
+ faviconUrl && loadedFaviconUrls.has(faviconUrl) ? "loaded" : "loading",
+ );
+ }, [faviconUrl]);Also found in 1 other location(s)
apps/desktop/src/main.ts:1675
The
titleBarOverlay.symbolColoris set once at window creation time based onnativeTheme.shouldUseDarkColors, but is never updated when the theme changes. WhenSET_THEME_CHANNELhandler setsnativeTheme.themeSource, the titlebar symbol color on Windows/Linux will remain stale, potentially making the minimize/maximize/close buttons hard to see or invisible against the new theme background.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/components/ProjectFavicon.tsx around lines 25-27:
When `faviconUrl` changes due to prop updates (e.g., `httpBaseUrl` or `workspaceRoot`), `status` remains at its previous value because `useState` only initializes on mount. If the previous URL had finished loading (`status === "loaded"`), the new image renders immediately without showing the folder fallback during its load.
Evidence trail:
apps/mobile/src/components/ProjectFavicon.tsx at REVIEWED_COMMIT:
- Lines 19-22: faviconUrl derived from props
- Lines 24-26: useState initializer only runs on mount
- No useEffect to reset status when faviconUrl changes
- Line 28: showImage = faviconUrl && status === "loaded"
- Lines 42-43: folder fallback only shows when !showImage
Also found in 1 other location(s):
- apps/desktop/src/main.ts:1675 -- The `titleBarOverlay.symbolColor` is set once at window creation time based on `nativeTheme.shouldUseDarkColors`, but is never updated when the theme changes. When `SET_THEME_CHANNEL` handler sets `nativeTheme.themeSource`, the titlebar symbol color on Windows/Linux will remain stale, potentially making the minimize/maximize/close buttons hard to see or invisible against the new theme background.
|
Bugbot Autofix prepared fixes for both issues found in the latest run.
Or push these changes by commenting: Preview (07aefa635b)diff --git a/apps/desktop/src/updateState.test.ts b/apps/desktop/src/updateState.test.ts
--- a/apps/desktop/src/updateState.test.ts
+++ b/apps/desktop/src/updateState.test.ts
@@ -71,7 +71,7 @@
platform: "darwin",
appImage: undefined,
disabledByEnv: false,
- hasUpdateFeedConfig: true,
+ hasUpdateFeedConfig: false,
}),
).toContain("packaged production builds");
});
diff --git a/apps/desktop/src/updateState.ts b/apps/desktop/src/updateState.ts
--- a/apps/desktop/src/updateState.ts
+++ b/apps/desktop/src/updateState.ts
@@ -36,12 +36,12 @@
disabledByEnv: boolean;
hasUpdateFeedConfig: boolean;
}): string | null {
+ if (args.isDevelopment || !args.isPackaged) {
+ return "Automatic updates are only available in packaged production builds.";
+ }
if (!args.hasUpdateFeedConfig) {
return "Automatic updates are not available because no update feed is configured.";
}
- if (args.isDevelopment || !args.isPackaged) {
- return "Automatic updates are only available in packaged production builds.";
- }
if (args.disabledByEnv) {
return "Automatic updates are disabled by the T3CODE_DISABLE_AUTO_UPDATE setting.";
}
diff --git a/apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx b/apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx
deleted file mode 100644
--- a/apps/mobile/src/features/connection/NewConnectionRouteScreen.tsx
+++ /dev/null
@@ -1,253 +1,0 @@
-import { CameraView, useCameraPermissions } from "expo-camera";
-import { Stack, useLocalSearchParams, useRouter } from "expo-router";
-import { SymbolView } from "expo-symbols";
-import { useCallback, useEffect, useState } from "react";
-import { Alert, Pressable, ScrollView, View } from "react-native";
-import { useSafeAreaInsets } from "react-native-safe-area-context";
-import { useThemeColor } from "../../lib/useThemeColor";
-
-import { AppText as Text, AppTextInput as TextInput } from "../../components/AppText";
-import { ErrorBanner } from "../../components/ErrorBanner";
-import { dismissRoute } from "../../lib/routes";
-import { ConnectionSheetButton } from "./ConnectionSheetButton";
-import { extractPairingUrlFromQrPayload } from "./pairing";
-import { useRemoteConnections } from "../../state/use-remote-environment-registry";
-import { buildPairingUrl, parsePairingUrl } from "./pairing";
-
-export function NewConnectionRouteScreen() {
- const {
- connectionError,
- connectionPairingUrl,
- connectionState,
- onChangeConnectionPairingUrl,
- onConnectPress,
- } = useRemoteConnections();
- const router = useRouter();
- const params = useLocalSearchParams<{ mode?: string }>();
- const insets = useSafeAreaInsets();
- const [hostInput, setHostInput] = useState("");
- const [codeInput, setCodeInput] = useState("");
- const [isSubmitting, setIsSubmitting] = useState(false);
- const [showScanner, setShowScanner] = useState(params.mode === "scan_qr");
- const [cameraPermission, requestCameraPermission] = useCameraPermissions();
- const [scannerLocked, setScannerLocked] = useState(false);
-
- const textColor = useThemeColor("--color-icon");
- const placeholderColor = useThemeColor("--color-placeholder");
-
- const connectDisabled =
- isSubmitting || connectionState === "connecting" || hostInput.trim().length === 0;
-
- useEffect(() => {
- const { host, code } = parsePairingUrl(connectionPairingUrl);
- setHostInput(host);
- setCodeInput(code);
- }, [connectionPairingUrl]);
-
- useEffect(() => {
- if (connectionError) {
- setIsSubmitting(false);
- }
- }, [connectionError]);
-
- const handleHostChange = useCallback((value: string) => {
- setHostInput(value);
- }, []);
-
- const handleCodeChange = useCallback((value: string) => {
- setCodeInput(value);
- }, []);
-
- const openScanner = useCallback(async () => {
- if (cameraPermission?.granted) {
- setScannerLocked(false);
- setShowScanner(true);
- return;
- }
-
- const permission = await requestCameraPermission();
- if (permission.granted) {
- setScannerLocked(false);
- setShowScanner(true);
- return;
- }
-
- Alert.alert("Camera access needed", "Allow camera access to scan a backend pairing QR code.");
- }, [cameraPermission?.granted, requestCameraPermission]);
-
- const closeScanner = useCallback(() => {
- setShowScanner(false);
- setScannerLocked(false);
- }, []);
-
- const handleQrScan = useCallback(
- ({ data }: { readonly data: string }) => {
- if (scannerLocked) {
- return;
- }
-
- setScannerLocked(true);
-
- try {
- const pairingUrl = extractPairingUrlFromQrPayload(data);
- const { host, code } = parsePairingUrl(pairingUrl);
- setHostInput(host);
- setCodeInput(code);
- onChangeConnectionPairingUrl(pairingUrl);
- setShowScanner(false);
- } catch (error) {
- Alert.alert(
- "Invalid QR code",
- error instanceof Error ? error.message : "Scanned QR code was not recognized.",
- );
- } finally {
- setTimeout(() => {
- setScannerLocked(false);
- }, 600);
- }
- },
- [onChangeConnectionPairingUrl, scannerLocked],
- );
-
- const handleSubmit = useCallback(async () => {
- setIsSubmitting(true);
-
- try {
- const pairingUrl = buildPairingUrl(hostInput, codeInput);
- onChangeConnectionPairingUrl(pairingUrl);
- await onConnectPress(pairingUrl);
- dismissRoute(router);
- } catch {
- setIsSubmitting(false);
- }
- }, [codeInput, hostInput, onChangeConnectionPairingUrl, onConnectPress, router]);
-
- return (
- <View collapsable={false} className="flex-1 bg-sheet">
- <Stack.Screen
- options={{
- title: showScanner ? "Scan QR Code" : "Add Backend",
- headerRight: () => (
- <Pressable
- className="h-10 w-10 items-center justify-center rounded-full border border-border bg-secondary"
- onPress={() => {
- if (showScanner) {
- closeScanner();
- } else {
- void openScanner();
- }
- }}
- >
- <SymbolView
- name={showScanner ? "xmark" : "qrcode.viewfinder"}
- size={showScanner ? 14 : 18}
- tintColor={textColor}
- type="monochrome"
- weight="semibold"
- />
- </Pressable>
- ),
- }}
- />
-
- <ScrollView
- contentInsetAdjustmentBehavior="automatic"
- showsVerticalScrollIndicator={false}
- style={{ flex: 1 }}
- contentContainerStyle={{
- paddingHorizontal: 20,
- paddingTop: 16,
- paddingBottom: Math.max(insets.bottom, 18) + 18,
- }}
- >
- <View collapsable={false} className="gap-5">
- {showScanner ? (
- cameraPermission?.granted ? (
- <View
- className="overflow-hidden rounded-[24px]"
- style={{ borderCurve: "continuous" }}
- >
- <CameraView
- barcodeScannerSettings={{ barcodeTypes: ["qr"] }}
- onBarcodeScanned={handleQrScan}
- style={{ aspectRatio: 1, width: "100%" }}
- />
- </View>
- ) : (
- <View
- className="items-center gap-3 rounded-[24px] bg-card px-5 py-8"
- style={{ borderCurve: "continuous" }}
- >
- <Text className="text-center text-[14px] leading-[20px] text-foreground-muted">
- Camera permission is required to scan a QR code.
- </Text>
- <ConnectionSheetButton
- compact
- icon="camera"
- label="Allow camera"
- tone="secondary"
- onPress={() => {
- void openScanner();
- }}
- />
- </View>
- )
- ) : (
- <View collapsable={false} className="gap-4 rounded-[24px] bg-card p-4">
- <View collapsable={false} className="gap-1.5">
- <Text
- className="text-[11px] font-t3-bold uppercase text-foreground-muted"
- style={{ letterSpacing: 0.8 }}
- >
- Host
- </Text>
- <TextInput
- autoCapitalize="none"
- autoCorrect={false}
- keyboardType="url"
- placeholder="192.168.1.100:8080"
- placeholderTextColor={placeholderColor}
- value={hostInput}
- onChangeText={handleHostChange}
- className="rounded-[14px] border border-input-border bg-input px-4 py-3.5 text-[15px] text-foreground"
- />
- </View>
-
- <View collapsable={false} className="gap-1.5">
- <Text
- className="text-[11px] font-t3-bold uppercase text-foreground-muted"
- style={{ letterSpacing: 0.8 }}
- >
- Pairing code
- </Text>
- <TextInput
- autoCapitalize="none"
- autoCorrect={false}
- placeholder="abc-123-xyz"
- placeholderTextColor={placeholderColor}
- value={codeInput}
- onChangeText={handleCodeChange}
- className="rounded-[14px] border border-input-border bg-input px-4 py-3.5 text-[15px] text-foreground"
- />
- </View>
-
- {connectionError ? <ErrorBanner message={connectionError} /> : null}
-
- <ConnectionSheetButton
- icon="plus"
- label={
- isSubmitting || connectionState === "connecting" ? "Pairing..." : "Add backend"
- }
- disabled={connectDisabled}
- tone="primary"
- onPress={() => {
- void handleSubmit();
- }}
- />
- </View>
- )}
- </View>
- </ScrollView>
- </View>
- );
-}
\ No newline at end of fileYou can send follow-ups to the cloud agent here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: CARD_SHADOW exports are defined but never imported
- Removed the unused CARD_SHADOW and CARD_SHADOW_DARK constants and their export statement from ConnectionSheetButton.tsx, as they were never imported anywhere in the codebase.
- ✅ Fixed: Debug console.log statements left in production code
- Removed all three console.log statements with the [new task flow] prefix from new-task-flow-provider.tsx (in reset callback, environment init effect, and the dedicated state-logging effect).
Or push these changes by commenting:
@cursor push 12dd1bcb97
Preview (12dd1bcb97)
diff --git a/apps/mobile/src/features/connection/ConnectionSheetButton.tsx b/apps/mobile/src/features/connection/ConnectionSheetButton.tsx
--- a/apps/mobile/src/features/connection/ConnectionSheetButton.tsx
+++ b/apps/mobile/src/features/connection/ConnectionSheetButton.tsx
@@ -5,28 +5,6 @@
import { AppText as Text } from "../../components/AppText";
import { cn } from "../../lib/cn";
-const CARD_SHADOW = Platform.select({
- ios: {
- shadowColor: "rgba(23,23,23,0.08)",
- shadowOffset: { width: 0, height: 4 },
- shadowOpacity: 1,
- shadowRadius: 16,
- },
- android: { elevation: 3 },
-});
-
-const CARD_SHADOW_DARK = Platform.select({
- ios: {
- shadowColor: "#000",
- shadowOffset: { width: 0, height: 2 },
- shadowOpacity: 0.18,
- shadowRadius: 8,
- },
- android: { elevation: 4 },
-});
-
-export { CARD_SHADOW, CARD_SHADOW_DARK };
-
export function ConnectionSheetButton(props: {
readonly icon: React.ComponentProps<typeof SymbolView>["name"];
readonly label: string;
diff --git a/apps/mobile/src/features/threads/new-task-flow-provider.tsx b/apps/mobile/src/features/threads/new-task-flow-provider.tsx
--- a/apps/mobile/src/features/threads/new-task-flow-provider.tsx
+++ b/apps/mobile/src/features/threads/new-task-flow-provider.tsx
@@ -189,10 +189,6 @@
}, []);
const reset = useCallback(() => {
- console.log("[new task flow] reset", {
- defaultEnvironmentId: projects[0]?.environmentId ?? null,
- projectCount: projects.length,
- });
setSelectedEnvironmentId(projects[0]?.environmentId ?? "");
setSelectedProjectKey(null);
setSelectedModelKey(null);
@@ -216,9 +212,6 @@
return;
}
- console.log("[new task flow] initializing environment", {
- environmentId: projects[0]!.environmentId,
- });
setSelectedEnvironmentId(projects[0]!.environmentId);
}, [projects, selectedEnvironmentId]);
@@ -470,24 +463,6 @@
],
);
- useEffect(() => {
- console.log("[new task flow] state", {
- availableBranchCount: availableBranches.length,
- environmentCount: environments.length,
- logicalProjectCount: logicalProjects.length,
- selectedEnvironmentId,
- selectedProjectKey,
- selectedProjectTitle: selectedProject?.title ?? null,
- });
- }, [
- availableBranches.length,
- environments.length,
- logicalProjects.length,
- selectedEnvironmentId,
- selectedProject?.title,
- selectedProjectKey,
- ]);
-
return <NewTaskFlowContext.Provider value={value}>{props.children}</NewTaskFlowContext.Provider>;
}You can send follow-ups to the cloud agent here.
| android: { elevation: 4 }, | ||
| }); | ||
|
|
||
| export { CARD_SHADOW, CARD_SHADOW_DARK }; |
There was a problem hiding this comment.
CARD_SHADOW exports are defined but never imported
Low Severity
CARD_SHADOW and CARD_SHADOW_DARK are defined and explicitly exported but never imported anywhere in the codebase. These are unused dead exports that add clutter without providing value.
Reviewed by Cursor Bugbot for commit 3d3e847. Configure here.
| defaultEnvironmentId: projects[0]?.environmentId ?? null, | ||
| projectCount: projects.length, | ||
| }); | ||
| setSelectedEnvironmentId(projects[0]?.environmentId ?? ""); |
There was a problem hiding this comment.
Debug console.log statements left in production code
Medium Severity
Multiple console.log calls with the [new task flow] prefix are left in new-task-flow-provider.tsx. These log internal state like environmentId, selectedProjectKey, branch counts, and project titles on every state change — verbose debug output that shouldn't ship in a production mobile app.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 3d3e847. Configure here.
juliusmarminge
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: QR scanner lock uses state instead of ref
- Replaced useState with useRef for the scanner lock so the guard value is read synchronously, preventing duplicate scan processing from rapid native barcode callbacks before React re-renders.
Or push these changes by commenting:
@cursor push 9d24bfb15f
Preview (9d24bfb15f)
diff --git a/apps/mobile/src/app/connections/new.tsx b/apps/mobile/src/app/connections/new.tsx
--- a/apps/mobile/src/app/connections/new.tsx
+++ b/apps/mobile/src/app/connections/new.tsx
@@ -1,7 +1,7 @@
import { CameraView, useCameraPermissions } from "expo-camera";
import { Stack, useLocalSearchParams, useRouter } from "expo-router";
import { SymbolView } from "expo-symbols";
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
import { Alert, Pressable, ScrollView, View } from "react-native";
import { useSafeAreaInsets } from "react-native-safe-area-context";
import { useThemeColor } from "../../lib/useThemeColor";
@@ -30,7 +30,7 @@
const [isSubmitting, setIsSubmitting] = useState(false);
const [showScanner, setShowScanner] = useState(params.mode === "scan_qr");
const [cameraPermission, requestCameraPermission] = useCameraPermissions();
- const [scannerLocked, setScannerLocked] = useState(false);
+ const scannerLockedRef = useRef(false);
const textColor = useThemeColor("--color-icon");
const placeholderColor = useThemeColor("--color-placeholder");
@@ -60,14 +60,14 @@
const openScanner = useCallback(async () => {
if (cameraPermission?.granted) {
- setScannerLocked(false);
+ scannerLockedRef.current = false;
setShowScanner(true);
return;
}
const permission = await requestCameraPermission();
if (permission.granted) {
- setScannerLocked(false);
+ scannerLockedRef.current = false;
setShowScanner(true);
return;
}
@@ -77,16 +77,16 @@
const closeScanner = useCallback(() => {
setShowScanner(false);
- setScannerLocked(false);
+ scannerLockedRef.current = false;
}, []);
const handleQrScan = useCallback(
({ data }: { readonly data: string }) => {
- if (scannerLocked) {
+ if (scannerLockedRef.current) {
return;
}
- setScannerLocked(true);
+ scannerLockedRef.current = true;
try {
const pairingUrl = extractPairingUrlFromQrPayload(data);
@@ -102,11 +102,11 @@
);
} finally {
setTimeout(() => {
- setScannerLocked(false);
+ scannerLockedRef.current = false;
}, 600);
}
},
- [onChangeConnectionPairingUrl, scannerLocked],
+ [onChangeConnectionPairingUrl],
);
const handleSubmit = useCallback(async () => {You can send follow-ups to the cloud agent here.
| } | ||
| }, | ||
| [onChangeConnectionPairingUrl, scannerLocked], | ||
| ); |
There was a problem hiding this comment.
QR scanner lock uses state instead of ref
Low Severity
handleQrScan reads scannerLocked from a stale closure since it's a useState value in a useCallback dependency array. Native barcode scanner callbacks can fire multiple times before React re-renders with the updated callback. A useRef would reliably prevent duplicate scan processing, since the ref value is always current across all closure instances.
Reviewed by Cursor Bugbot for commit efc36b4. Configure here.
| truncateOutputAtMaxBytes: true, | ||
| }, | ||
| ); | ||
| const untrackedPaths = splitNullSeparatedPaths(untrackedOutput, false); |
There was a problem hiding this comment.
🟠 High Layers/GitCore.ts:1643
readUntrackedReviewDiffs hardcodes false for the truncated parameter when calling splitNullSeparatedPaths, even though runGitStdoutWithOptions may truncate the output. When truncation occurs, the OUTPUT_TRUNCATED_MARKER text corrupts the final path, which then gets passed to git diff --no-index as an invalid filename. Consider detecting the marker or using executeGit directly to pass the actual truncation state to splitNullSeparatedPaths.
- const untrackedPaths = splitNullSeparatedPaths(untrackedOutput, false);
+ const untrackedPaths = untrackedOutput.endsWith(OUTPUT_TRUNCATED_MARKER)
+ ? splitNullSeparatedPaths(untrackedOutput.slice(0, -OUTPUT_TRUNCATED_MARKER.length), true)
+ : splitNullSeparatedPaths(untrackedOutput, false);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/git/Layers/GitCore.ts around line 1643:
`readUntrackedReviewDiffs` hardcodes `false` for the `truncated` parameter when calling `splitNullSeparatedPaths`, even though `runGitStdoutWithOptions` may truncate the output. When truncation occurs, the `OUTPUT_TRUNCATED_MARKER` text corrupts the final path, which then gets passed to `git diff --no-index` as an invalid filename. Consider detecting the marker or using `executeGit` directly to pass the actual truncation state to `splitNullSeparatedPaths`.
Evidence trail:
- apps/server/src/git/Layers/GitCore.ts lines 1634-1643 (REVIEWED_COMMIT): `runGitStdoutWithOptions` called with `truncateOutputAtMaxBytes: true`, then `splitNullSeparatedPaths(untrackedOutput, false)` hardcodes `false`
- apps/server/src/git/Layers/GitCore.ts lines 852-862 (REVIEWED_COMMIT): `runGitStdoutWithOptions` appends `OUTPUT_TRUNCATED_MARKER` when `stdoutTruncated` is true but only returns the string, not the truncation state
- apps/server/src/git/Layers/GitCore.ts line 44 (REVIEWED_COMMIT): `OUTPUT_TRUNCATED_MARKER = "\n\n[truncated]"`
- apps/server/src/git/Layers/GitCore.ts lines 131-140 (REVIEWED_COMMIT): `splitNullSeparatedPaths` removes last path when `truncated` is true, but won't with hardcoded `false`
- apps/server/src/git/Layers/GitCore.ts lines 1778, 1831 (REVIEWED_COMMIT): Correct usage pattern with `result.stdoutTruncated`
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.
| modelSelection: modelWithOptions, | ||
| envMode: flow.workspaceMode, | ||
| branch: flow.selectedBranchName, | ||
| worktreePath: flow.workspaceMode === "worktree" ? null : flow.selectedWorktreePath, |
There was a problem hiding this comment.
Worktree path condition is inverted
High Severity
The worktreePath ternary is backwards: it passes null when workspaceMode is "worktree" and flow.selectedWorktreePath when it's "local". The worktree path is only meaningful when actually using worktree mode, so the condition needs to be flipped.
Reviewed by Cursor Bugbot for commit 1705361. Configure 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>
- Add syntax-highlighted diff rendering and cached review state - Support pasted/attached images in review comments - Improve mobile review sheet handling for large or partial diffs Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/mobile-remote-connect
branch
from
c799180 to
f86d466
Compare
| (s) => new Date(s.latestActivityAt), | ||
| Order.Date, | ||
| ), | ||
| }); |
There was a problem hiding this comment.
Repository group sort uses ascending instead of descending
Medium Severity
When merging repository groups, the latestActivityAt comparison is inverted. compareIsoDateDescending returns a positive number when right (the new activity) is more recent, so the condition > 0 picks the older timestamp as the group's latest activity rather than the newer one.
Reviewed by Cursor Bugbot for commit f86d466. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The logic correctly selects the more recent timestamp: when the new candidate is newer, compareIsoDateDescending returns positive and the ternary picks it; when existing is newer, the result is negative and the ternary keeps the existing value.
You can send follow-ups to the cloud agent here.
| }; | ||
| 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.
Rebuild PR pingdotgg#2013 on top of current origin/main without the duplicated commit lineage. Co-authored-by: codex <codex@users.noreply.github.com>
- Switch review screen to a virtualized list layout - Queue diff highlighting and default to the JS engine - Add word-diff helpers and tests
| scss: () => import("@shikijs/langs/scss"), | ||
| less: () => import("@shikijs/langs/less"), | ||
| xml: () => import("@shikijs/langs/xml"), | ||
| svg: () => import("@shikijs/langs/xml"), |
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.
Co-authored-by: Julius Marminge <julius@macmini.local>
- Use variant-specific Expo schemes for development, preview, and production - Preserve the production `t3code` scheme while fixing remote-connect routing
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).
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.
Reviewed by Cursor Bugbot for commit acdfb6a. Configure here.
| NSAllowsArbitraryLoads: true, | ||
| }, | ||
| ITSAppUsesNonExemptEncryption: false, | ||
| }, |
There was a problem hiding this comment.
Cleartext HTTP allowed unconditionally in production builds
Medium Severity
NSAllowsArbitraryLoads: true in the iOS config and the withAndroidCleartextTraffic plugin apply to all build variants, including production. The app already has a variant system (APP_VARIANT) but these security-sensitive settings aren't gated behind it. This exposes production users to man-in-the-middle attacks by permitting HTTP connections and may require additional justification during Apple App Store review. These settings could be conditionally applied only for development and preview variants.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit acdfb6a. Configure here.
| const knownTerminalSessionKeys = new Set<string>(); | ||
| const textEncoder = new TextEncoder(); |
There was a problem hiding this comment.
🟡 Medium src/terminalSessionState.ts:47
knownTerminalSessionKeys is module-level mutable state shared across all createTerminalSessionManager instances. When invalidate() is called without a target on one manager, it iterates keys added by other managers and writes EMPTY_TERMINAL_SESSION_STATE to the wrong registry, creating orphaned atom state. It also clears the set, causing other managers to lose track of their sessions.
-const knownTerminalSessionKeys = new Set<string>();
const textEncoder = new TextEncoder();🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/client-runtime/src/terminalSessionState.ts around lines 47-48:
`knownTerminalSessionKeys` is module-level mutable state shared across all `createTerminalSessionManager` instances. When `invalidate()` is called without a target on one manager, it iterates keys added by other managers and writes `EMPTY_TERMINAL_SESSION_STATE` to the wrong registry, creating orphaned atom state. It also clears the set, causing other managers to lose track of their sessions.
Evidence trail:
packages/client-runtime/src/terminalSessionState.ts lines 47, 49-54 (module-level `knownTerminalSessionKeys` Set and its population), lines 131-133 (`createTerminalSessionManager` taking `config.getRegistry()`), lines 278-291 (`invalidate()` function iterating shared keys, writing to own registry, and clearing the shared set).
| 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.
1 participant
Summary
packages/client-runtimeandpackages/sharedso web and mobile can share the same behavior.Testing
Note
High Risk
Introduces a new mobile app plus a vendored iOS XCFramework/native module, which significantly increases repo surface area and adds platform-specific build/runtime complexity (including permissive iOS ATS settings).
Overview
Adds a new
apps/mobileExpo/React Native client scaffold with variant-aware Expo config (development/preview/production), EAS build profiles, Metro/Babel setup, and a Tailwind/Uniwind-driven theming stylesheet.Introduces a local native terminal module (
apps/mobile/modules/t3-terminal) that vendorsGhosttyKit.xcframeworkfor iOS via a new podspec and documents third-party notices, plus updates repo tooling configs (.oxlintrc.json,.oxfmtrc.json) to ignore generated Uniwind types and adjust lint rules.Reviewed by Cursor Bugbot for commit acdfb6a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add T3 Code mobile app with native terminal, thread management, and git workflows
apps/mobileReact Native/Expo app with navigation, theming, and connection management for remote T3 Code environments via QR code or manual pairing.apps/mobile/modules/t3-terminal) backed by vendored GhosttyKit on iOS and a customExpoViewon Android, with input/resize event bridging and theme support.packages/client-runtime, each backed by atom families and shared across web and mobile.createEnvironmentConnection,WsRpcClient, transport error utilities, reconnect backoff, and WebSocket protocol/transport layers intopackages/client-runtimeandpackages/shared, replacing local web-app implementations with re-exports.git.getReviewDiffsend-to-end: new contract types, server-side diff assembly (dirty worktree + base branch), WebSocket RPC handler,EnvironmentApimethod, and a mobile review sheet with syntax highlighting via Shiki and inline word-diff emphasis.Macroscope summarized acdfb6a.