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}
| readonly onShellResubscribe?: (environmentId: EnvironmentId) => void; | ||
| } | ||
|
|
||
| function createBootstrapGate() { |
There was a problem hiding this comment.
🟡 Medium src/environmentConnection.ts:45
When bootstrapGate.reset() is called in onResubscribe (line 134), callers already awaiting the previous promise from ensureBootstrapped() will wait forever. The reset() method overwrites resolve and reject with new callbacks for a new promise, orphaning the original promise's resolvers. External callers blocked on the old promise will never resolve, even when the new snapshot arrives and resolve() is called.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/client-runtime/src/environmentConnection.ts around line 45:
When `bootstrapGate.reset()` is called in `onResubscribe` (line 134), callers already awaiting the previous promise from `ensureBootstrapped()` will wait forever. The `reset()` method overwrites `resolve` and `reject` with new callbacks for a new promise, orphaning the original promise's resolvers. External callers blocked on the old promise will never resolve, even when the new snapshot arrives and `resolve()` is called.
Evidence trail:
packages/client-runtime/src/environmentConnection.ts lines 44-65: `createBootstrapGate()` implementation showing `reset()` creates new promise and overwrites resolve/reject callbacks (lines 59-64). Line 113: `bootstrapGate.resolve()` called on snapshot. Line 131: `bootstrapGate.reset()` called in `onResubscribe`. Line 147: `ensureBootstrapped: () => bootstrapGate.wait()` returns the promise.
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. |
| backoff.maxRetries === null ? Schedule.forever : Schedule.recurs(backoff.maxRetries); |
There was a problem hiding this comment.
🟢 Low src/wsRpcProtocol.ts:120
Schedule.recurs(backoff.maxRetries) produces maxRetries+1 iterations (indices 0 through maxRetries inclusive), but getReconnectDelayMs returns null when retryIndex >= maxRetries. With maxRetries = 7, the schedule runs 8 iterations, yet getReconnectDelayMs(7, ...) returns null, causing the 8th retry to use 0ms delay via the ?? 0 fallback instead of the expected maxDelayMs. Pass backoff.maxRetries + 1 to Schedule.recurs so the iteration count matches the number of delays available.
- backoff.maxRetries === null ? Schedule.forever : Schedule.recurs(backoff.maxRetries);
+ backoff.maxRetries === null ? Schedule.forever : Schedule.recurs((backoff.maxRetries ?? 0) + 1);Also found in 1 other location(s)
apps/web/src/uiStateStore.ts:518
The
insertIndexcalculation is incorrect. The formulaoriginalTargetIndex - Math.max(0, draggedBeforeTarget - 1)does not properly account for index shifts after removing dragged items. When dragging item C (index 2) onto target D (index 3),draggedBeforeTarget=1givesinsertIndex = 3 - Max(0, 0) = 3, but after removal D is at index 2, so this inserts C AFTER D instead of before it. The correct formula should beoriginalTargetIndex - draggedBeforeTargetto properly compensate for all items removed before the target position.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/client-runtime/src/wsRpcProtocol.ts around line 120:
`Schedule.recurs(backoff.maxRetries)` produces `maxRetries+1` iterations (indices 0 through `maxRetries` inclusive), but `getReconnectDelayMs` returns `null` when `retryIndex >= maxRetries`. With `maxRetries = 7`, the schedule runs 8 iterations, yet `getReconnectDelayMs(7, ...)` returns `null`, causing the 8th retry to use `0ms` delay via the `?? 0` fallback instead of the expected `maxDelayMs`. Pass `backoff.maxRetries + 1` to `Schedule.recurs` so the iteration count matches the number of delays available.
Evidence trail:
packages/client-runtime/src/wsRpcProtocol.ts lines 120-122 (Schedule.recurs and ?? 0 fallback); packages/client-runtime/src/reconnectBackoff.ts lines 19-24 (maxRetries=7), lines 37-39 (returns null when retryIndex >= maxRetries); packages/client-runtime/src/reconnectBackoff.test.ts lines 17-22 (confirms getReconnectDelayMs(6)=64000, getReconnectDelayMs(7)=null); Effect-TS Stream.ts docs ('Schedule.recurs(1) results in 2 total repetitions')
Also found in 1 other location(s):
- apps/web/src/uiStateStore.ts:518 -- The `insertIndex` calculation is incorrect. The formula `originalTargetIndex - Math.max(0, draggedBeforeTarget - 1)` does not properly account for index shifts after removing dragged items. When dragging item C (index 2) onto target D (index 3), `draggedBeforeTarget=1` gives `insertIndex = 3 - Max(0, 0) = 3`, but after removal D is at index 2, so this inserts C AFTER D instead of before it. The correct formula should be `originalTargetIndex - draggedBeforeTarget` to properly compensate for all items removed before the target position.
| 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.
| if (gitStatus.pr?.state === "open") { | ||
| parts.push(`PR #${gitStatus.pr.number} open`); | ||
| } |
There was a problem hiding this comment.
🟢 Low git/gitSheetComponents.tsx:156
When gitStatus.pr.state is "open" but gitStatus.pr.number is undefined, statusSummary returns "PR #undefined open" because the condition on line 156 checks state without verifying that number exists. Consider adding a check for number before interpolating it.
- if (gitStatus.pr?.state === "open") {
+ if (gitStatus.pr?.state === "open" && gitStatus.pr.number != null) {
parts.push(`PR #${gitStatus.pr.number} open`);
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/git/gitSheetComponents.tsx around lines 156-158:
When `gitStatus.pr.state` is `"open"` but `gitStatus.pr.number` is `undefined`, `statusSummary` returns `"PR #undefined open"` because the condition on line 156 checks `state` without verifying that `number` exists. Consider adding a check for `number` before interpolating it.
Evidence trail:
apps/mobile/src/features/threads/git/gitSheetComponents.tsx lines 125-162 (REVIEWED_COMMIT) - shows the `statusSummary` function with type definition at line 132 declaring `readonly number?: number` (optional), and lines 156-158 showing the condition checks `gitStatus.pr?.state === "open"` but interpolates `gitStatus.pr.number` without checking if `number` is defined.
|
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.
| const threads = Arr.sortWith( | ||
| threadsByProjectKey.get(projectKey) ?? [], | ||
| (s) => new Date(s.updatedAt ?? s.createdAt), | ||
| Order.Date, | ||
| ); |
There was a problem hiding this comment.
🟠 High lib/repositoryGroups.ts:83
At line 85-86, Arr.sortWith(..., Order.Date) sorts threads in ascending order (oldest first), but deriveProjectLatestActivity at line 89 assumes threads[0] is the most recent. This causes latestActivityAt to capture the oldest thread's timestamp instead of the latest, breaking both UI activity dates and downstream sorting of groups.
- const threads = Arr.sortWith(
+ const threads = Arr.sortWith(
threadsByProjectKey.get(projectKey) ?? [],
(s) => new Date(s.updatedAt ?? s.createdAt),
- Order.Date,
+ Order.reverse(Order.Date),
);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/lib/repositoryGroups.ts around lines 83-87:
At line 85-86, `Arr.sortWith(..., Order.Date)` sorts threads in ascending order (oldest first), but `deriveProjectLatestActivity` at line 89 assumes `threads[0]` is the most recent. This causes `latestActivityAt` to capture the oldest thread's timestamp instead of the latest, breaking both UI activity dates and downstream sorting of groups.
Evidence trail:
- apps/mobile/src/lib/repositoryGroups.ts:84-87 (REVIEWED_COMMIT): `Arr.sortWith(..., Order.Date)` sorting threads
- apps/mobile/src/lib/repositoryGroups.ts:54-58 (REVIEWED_COMMIT): `deriveProjectLatestActivity` uses `threads[0]` as `latestThread`
- https://github.com/Effect-TS/effect packages/effect/src/Order.ts:57: `export const number: Order<number> = make((self, that) => self < that ? -1 : 1)` (ascending order)
- https://github.com/Effect-TS/effect packages/effect/src/Order.ts:141: `export const Date: Order<Date> = mapInput(number, (date) => date.getTime())` (uses number order, so ascending)
- https://github.com/Effect-TS/effect packages/effect/src/Array.ts:1475-1477: documentation states "Order.number specifies that the lengths should be sorted in ascending order"
| <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.
|
|
||
| const knownGitStatusKeys = new Set<string>(); | ||
|
|
||
| export const gitStatusStateAtom = Atom.family((key: string) => { | ||
| knownGitStatusKeys.add(key); | ||
| return Atom.make(INITIAL_GIT_STATUS_STATE).pipe( | ||
| Atom.keepAlive, | ||
| Atom.withLabel(`git-status:${key}`), | ||
| ); | ||
| }); | ||
|
|
||
| export const EMPTY_GIT_STATUS_ATOM = Atom.make(EMPTY_GIT_STATUS_STATE).pipe( | ||
| Atom.keepAlive, | ||
| Atom.withLabel("git-status:null"), | ||
| ); | ||
|
|
There was a problem hiding this comment.
🟢 Low src/gitStatusState.ts:46
knownGitStatusKeys is a module-level Set shared across all manager instances, but reset() clears it unconditionally. If multiple managers exist, calling reset() on one manager clears the Set globally, orphaning atoms from other managers so they won't be reset in future reset() calls. Additionally, because gitStatusStateAtom uses Atom.family memoization, cleared keys that are later reused won't be re-added to the Set, so subsequent reset() calls will skip those atoms. Consider moving knownGitStatusKeys inside createGitStatusManager so each manager tracks and resets only its own atoms.
-/* ─── Atoms ─────────────────────────────────────────────────────────── */
-
-const knownGitStatusKeys = new Set<string>();
-
export const gitStatusStateAtom = Atom.family((key: string) => {
- knownGitStatusKeys.add(key);
return Atom.make(INITIAL_GIT_STATUS_STATE).pipe(
Atom.keepAlive,
Atom.withLabel(`git-status:${key}`),
);
});
export const EMPTY_GIT_STATUS_ATOM = Atom.make(EMPTY_GIT_STATUS_STATE).pipe(
Atom.keepAlive,
Atom.withLabel("git-status:null"),
);
/* ─── Helpers ───────────────────────────────────────────────────────── */🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/client-runtime/src/gitStatusState.ts around lines 46-61:
`knownGitStatusKeys` is a module-level Set shared across all manager instances, but `reset()` clears it unconditionally. If multiple managers exist, calling `reset()` on one manager clears the Set globally, orphaning atoms from other managers so they won't be reset in future `reset()` calls. Additionally, because `gitStatusStateAtom` uses `Atom.family` memoization, cleared keys that are later reused won't be re-added to the Set, so subsequent `reset()` calls will skip those atoms. Consider moving `knownGitStatusKeys` inside `createGitStatusManager` so each manager tracks and resets only its own atoms.
Evidence trail:
packages/client-runtime/src/gitStatusState.ts lines 47, 49-54 (module-level Set and Atom.family memoization), lines 259-267 (reset() clears knownGitStatusKeys globally), lines 96-102 (createGitStatusManager creates instance-local Maps for watched/refreshInFlight/lastRefreshAt but shares module-level knownGitStatusKeys)
| dispose: async () => { | ||
| cleanup(); | ||
| await input.client.dispose(); | ||
| }, |
There was a problem hiding this comment.
🟡 Medium src/environmentConnection.ts:169
When dispose() is called, any caller awaiting ensureBootstrapped() will hang indefinitely because cleanup() does not resolve or reject the bootstrapGate promise.
| dispose: async () => { | |
| cleanup(); | |
| await input.client.dispose(); | |
| }, | |
| dispose: async () => { | |
| bootstrapGate.reject(new Error("Connection disposed")); | |
| cleanup(); | |
| await input.client.dispose(); | |
| }, |
Also found in 1 other location(s)
apps/mobile/src/app/_layout.tsx:86
The
useFontshook returns[loaded, error]but onlyfontsLoadedis destructured. If font loading fails, the error is silently ignored andfontsLoadedremainsfalse, causing the app to be stuck on<LoadingScreen message="Loading remote workspace…" />indefinitely with no error handling or user feedback.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/client-runtime/src/environmentConnection.ts around lines 169-172:
When `dispose()` is called, any caller awaiting `ensureBootstrapped()` will hang indefinitely because `cleanup()` does not resolve or reject the `bootstrapGate` promise.
Evidence trail:
packages/client-runtime/src/environmentConnection.ts lines 42-61 (createBootstrapGate definition), line 74 (bootstrapGate creation), lines 127-132 (cleanup function - does not touch bootstrapGate), line 148 (ensureBootstrapped returns bootstrapGate.wait()), lines 154-157 (dispose calls cleanup then client.dispose)
Also found in 1 other location(s):
- apps/mobile/src/app/_layout.tsx:86 -- The `useFonts` hook returns `[loaded, error]` but only `fontsLoaded` is destructured. If font loading fails, the error is silently ignored and `fontsLoaded` remains `false`, causing the app to be stuck on `<LoadingScreen message="Loading remote workspace…" />` indefinitely with no error handling or user feedback.
| [refreshRemoteData], | ||
| ); | ||
|
|
||
| const onCreateThread = useCallback( |
There was a problem hiding this comment.
🟡 Medium threads/use-project-actions.ts:156
onCreateThread always returns null because it passes initialMessageText: "" to onCreateThreadWithOptions, which returns null when the message text is empty. Since onCreateThread provides no way for the caller to supply the message text, the function can never successfully create a thread.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/mobile/src/features/threads/use-project-actions.ts around line 156:
`onCreateThread` always returns `null` because it passes `initialMessageText: ""` to `onCreateThreadWithOptions`, which returns `null` when the message text is empty. Since `onCreateThread` provides no way for the caller to supply the message text, the function can never successfully create a thread.
Evidence trail:
apps/mobile/src/features/threads/use-project-actions.ts lines 95-98 (empty message check returning null), line 156-182 (`onCreateThread` definition), line 178 (`initialMessageText: ""`). Commit: REVIEWED_COMMIT
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
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 8 total unresolved issues (including 7 from previous reviews).
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit f86d466. Configure here.
| (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.
| const highlighted = { additionLines, deletionLines }; | ||
| resolvedHighlightCache.set(cacheKey, highlighted); | ||
| return highlighted; | ||
| })(); |
There was a problem hiding this comment.
🟡 Medium review/shikiReviewHighlighter.ts:203
When highlightReviewFile rejects (e.g., language loading fails), the rejected promise is cached in highlightCache and returned on all subsequent calls with the same key. This permanently blocks re-highlighting of that file/theme combination even if the transient error resolves. Consider removing the rejected promise from the cache in a .catch() handler to allow retry.
- return highlighted;
+ return highlighted;
+ })().catch((err) => {
+ highlightCache.delete(cacheKey);
+ throw err;
})();🚀 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 203:
When `highlightReviewFile` rejects (e.g., language loading fails), the rejected promise is cached in `highlightCache` and returned on all subsequent calls with the same key. This permanently blocks re-highlighting of that file/theme combination even if the transient error resolves. Consider removing the rejected promise from the cache in a `.catch()` handler to allow retry.
Evidence trail:
apps/mobile/src/features/threads/review/shikiReviewHighlighter.ts lines 32-33 (cache declarations), lines 178-208 (highlightReviewFile function). Key observations: Line 207 `highlightCache.set(cacheKey, promise)` stores the promise before resolution. Lines 188-190 return cached promise on subsequent calls. No `.catch()` handler exists to remove rejected promises from the cache.
| }; | ||
| 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.
1 participant
Summary
packages/client-runtimeandpackages/sharedso web and mobile can share the same behavior.Testing
Note
Medium Risk
Large addition of a new Expo/RN app plus connection + composer flows; also enables Android cleartext traffic, which can affect network/security expectations if misconfigured.
Overview
Adds a new
apps/mobileExpo/React Native client, including navigation/layout scaffolding, global theming (Tailwind/Uniwind), and EAS build variants (development/preview/production).Implements core mobile flows: remote backend connection management (manual host/code + QR scan with pairing URL parsing), home thread browsing/search grouped by project with live git status summaries, new-task creation UI with model/runtime options and image attachments, and thread detail with a glass-style composer supporting slash-command/skill/path triggers.
Includes supporting build/runtime config (Metro/Babel, mobile
.gitignore, icons, Android manifest plugin to allow cleartext traffic) and updates repo lint/format ignores to exclude generated Uniwind types plus disablesunicorn/no-array-sort.Reviewed by Cursor Bugbot for commit f86d466. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add T3 Code mobile app with remote environment connections, thread viewing, and git operations
apps/mobileExpo/React Native app with Expo Router navigation, themed UI, and three-variant (dev/preview/prod) build configuration viaapp.config.ts.ConnectionStatusDotpulsing indicator.packages/client-runtime(WebSocket transport with reconnect backoff, environment connection, shell/thread/git state managers with Effect atoms) andpackages/shared(composer triggers, remote pairing utilities, orchestration timing).gitGetReviewDiffsRPC method end-to-end: contract schema inpackages/contracts, server implementation inGitCore, WebSocket handler, and client-side API exposure.Macroscope summarized f86d466.