-
Notifications
You must be signed in to change notification settings - Fork 14
Inbox + sessions: replay by export ID, stacked report logs, cloud workspace bootstrap #2011
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,13 +15,20 @@ import { WorkspaceSetupPrompt } from "@features/task-detail/components/Workspace | |
| import { useBranchMismatchDialog } from "@features/workspace/hooks/useBranchMismatchDialog"; | ||
| import { | ||
| useCreateWorkspace, | ||
| useWorkspace, | ||
| useWorkspaceLoaded, | ||
| workspaceApi, | ||
| } from "@features/workspace/hooks/useWorkspace"; | ||
| import { Box, Flex } from "@radix-ui/themes"; | ||
| import { useTRPC } from "@renderer/trpc/client"; | ||
| import type { Task } from "@shared/types"; | ||
| import { useQueryClient } from "@tanstack/react-query"; | ||
| import { logger } from "@utils/logger"; | ||
| import { getTaskRepository } from "@utils/repository"; | ||
| import { useCallback, useEffect } from "react"; | ||
|
|
||
| const bootstrapCloudLog = logger.scope("task-logs-panel-bootstrap"); | ||
|
|
||
| interface TaskLogsPanelProps { | ||
| taskId: string; | ||
| task: Task; | ||
|
|
@@ -30,7 +37,10 @@ interface TaskLogsPanelProps { | |
| } | ||
|
|
||
| export function TaskLogsPanel({ taskId, task, hideInput }: TaskLogsPanelProps) { | ||
| const trpcReact = useTRPC(); | ||
| const queryClient = useQueryClient(); | ||
| const isWorkspaceLoaded = useWorkspaceLoaded(); | ||
| const persistedWorkspace = useWorkspace(taskId); | ||
| const { isPending: isCreatingWorkspace } = useCreateWorkspace(); | ||
| const repoKey = getTaskRepository(task); | ||
| const { folders } = useFolders(); | ||
|
|
@@ -94,6 +104,50 @@ export function TaskLogsPanel({ taskId, task, hideInput }: TaskLogsPanelProps) { | |
| requestFocus(taskId); | ||
| }, [taskId, requestFocus]); | ||
|
|
||
| useEffect(() => { | ||
| if (!isWorkspaceLoaded) return; | ||
| if (persistedWorkspace) return; | ||
| if (task.latest_run?.environment !== "cloud") return; | ||
| if (isSuspended) return; | ||
| if (isProvisioning) return; | ||
|
|
||
| let cancelled = false; | ||
| void (async () => { | ||
| try { | ||
| await workspaceApi.create({ | ||
| taskId, | ||
| mainRepoPath: "", | ||
| folderId: "", | ||
| folderPath: "", | ||
| mode: "cloud", | ||
| }); | ||
| } catch (error) { | ||
| bootstrapCloudLog.warn("Cloud workspace bootstrap failed", { | ||
| taskId, | ||
| error, | ||
| }); | ||
| } | ||
| if (!cancelled) { | ||
| void queryClient.invalidateQueries( | ||
| trpcReact.workspace.getAll.pathFilter(), | ||
| ); | ||
| } | ||
|
Comment on lines
+130
to
+134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the component unmounts while
} catch (error) {
bootstrapCloudLog.warn("Cloud workspace bootstrap failed", { taskId, error });
}
// Always invalidate — cache updates are safe after unmount and prevent
// a stale null workspace from causing a duplicate-create on next mount.
void queryClient.invalidateQueries(
trpcReact.workspace.getAll.pathFilter(),
);
})();Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/code/src/renderer/features/task-detail/components/TaskLogsPanel.tsx
Line: 130-134
Comment:
**Cache invalidation skipped on early unmount**
If the component unmounts while `workspaceApi.create` is still in flight, the cleanup sets `cancelled = true`, the creation still succeeds on the server, but `queryClient.invalidateQueries` is never called. On the next mount, `persistedWorkspace` is still `null` (the query's `staleTime` is 60 s so it won't re-fetch), the effect fires again, and a second create is attempted.
`invalidateQueries` updates the shared cache — it is safe to call even after the component that triggered it has unmounted. The guard should be removed:
```ts
} catch (error) {
bootstrapCloudLog.warn("Cloud workspace bootstrap failed", { taskId, error });
}
// Always invalidate — cache updates are safe after unmount and prevent
// a stale null workspace from causing a duplicate-create on next mount.
void queryClient.invalidateQueries(
trpcReact.workspace.getAll.pathFilter(),
);
})();
```
How can I resolve this? If you propose a fix, please make it concise. |
||
| })(); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [ | ||
| isProvisioning, | ||
| isSuspended, | ||
| isWorkspaceLoaded, | ||
| persistedWorkspace, | ||
| queryClient, | ||
| task.latest_run?.environment, | ||
| taskId, | ||
| trpcReact, | ||
| ]); | ||
|
|
||
| const handleRestoreWorktree = useCallback(async () => { | ||
| await restoreTask(taskId); | ||
| }, [taskId, restoreTask]); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The effect fires whenever any of its dependencies change (e.g.,
isSuspendedtoggling). If a dependency change triggers the cleanup and immediately re-fires the effect while the previousworkspaceApi.createcall is still awaiting a response, two creates are in flight simultaneously —persistedWorkspaceis stillnullfor both, so neither guard catches the other.Bypassing the
useMutationpath also meansisCreatingWorkspacedoesn't reflect the bootstrap's pending state. Consider auseRefin-flight flag or using the existing mutation hook to serialise concurrent bootstrap attempts.Prompt To Fix With AI