- Notifications
You must be signed in to change notification settings - Fork1.1k
feat: support devcontainer agents in ui and unify backend#18332
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
765c2cf4019358452dbc96a23998ee3ed36fc1a90a88ce78fb1431a32c2bf2818e1593df28ff99f69f69ecfe48371c61b62e1c31f7e41c1579e18448ce0aecd3bda05d4f208b7e5ede07a3674ac0607b1e1ea4bff150bad2b22ee24b9d218e021c8d0c44de8File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,6 +4,7 @@ import type { | ||
| Workspace, | ||
| WorkspaceAgent, | ||
| WorkspaceAgentDevcontainer, | ||
| WorkspaceAgentListContainersResponse, | ||
| } from "api/typesGenerated"; | ||
| import { Button } from "components/Button/Button"; | ||
| import { displayError } from "components/GlobalSnackbar/utils"; | ||
| @@ -20,7 +21,8 @@ import { Container, ExternalLinkIcon } from "lucide-react"; | ||
| import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; | ||
| import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; | ||
| import type { FC } from "react"; | ||
| import { useEffect } from "react"; | ||
| import { useMutation, useQueryClient } from "react-query"; | ||
| import { portForwardURL } from "utils/portForward"; | ||
| import { AgentApps, organizeAgentApps } from "./AgentApps/AgentApps"; | ||
| import { AgentButton } from "./AgentButton"; | ||
| @@ -51,12 +53,7 @@ export const AgentDevcontainerCard: FC<AgentDevcontainerCardProps> = ({ | ||
| }) => { | ||
| const { browser_only } = useFeatureVisibility(); | ||
| const { proxy } = useProxy(); | ||
| const queryClient = useQueryClient(); | ||
| // The sub agent comes from the workspace response whereas the devcontainer | ||
| // comes from the agent containers endpoint. We need alignment between the | ||
| @@ -80,64 +77,105 @@ export const AgentDevcontainerCard: FC<AgentDevcontainerCardProps> = ({ | ||
| showVSCode || | ||
| appSections.some((it) => it.apps.length > 0); | ||
| const rebuildDevcontainerMutation = useMutation({ | ||
| mutationFn: async () => { | ||
| const response = await fetch( | ||
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. In the FE, we handle requests in two ways: queries or mutations. In this case, you should wrap this request into a mutation.Here is the docs. 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. When using a mutation, you can invalidate other queries or update their data to reflect the most recent status, instead of doing it manually, so you wouldn't need the isRebuilding or subAgentRemoved statuses.Here is how you can do it. If you need more examples, you can search in the codebase for "invalidateQueries". MemberAuthor 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. Thanks for the tip! This was superb, actually solved a couple of issues I wanted to address but didn't know how! | ||
| `/api/v2/workspaceagents/${parentAgent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, | ||
| { method: "POST" }, | ||
| ); | ||
| if (!response.ok) { | ||
| const errorData = await response.json().catch(() => ({})); | ||
| throw new Error( | ||
| errorData.message || `Failed torebuild: ${response.statusText}`, | ||
| ); | ||
| } | ||
| return response; | ||
| }, | ||
| onMutate: async () => { | ||
| await queryClient.cancelQueries({ | ||
| queryKey: ["agents", parentAgent.id, "containers"], | ||
| }); | ||
| // Snapshot the previous data for rollback in case of error. | ||
| const previousData = queryClient.getQueryData([ | ||
| "agents", | ||
| parentAgent.id, | ||
| "containers", | ||
| ]); | ||
| // Optimistically update the devcontainer status to | ||
| // "starting" and zero the agent and container to mimic what | ||
| // the API does. | ||
| queryClient.setQueryData( | ||
| ["agents", parentAgent.id, "containers"], | ||
| (oldData?: WorkspaceAgentListContainersResponse) => { | ||
| if (!oldData?.devcontainers) return oldData; | ||
| return { | ||
| ...oldData, | ||
| devcontainers: oldData.devcontainers.map((dc) => { | ||
| if (dc.id === devcontainer.id) { | ||
| return { | ||
| ...dc, | ||
| agent: null, | ||
| container: null, | ||
| status: "starting", | ||
| }; | ||
| } | ||
| return dc; | ||
| }), | ||
| }; | ||
| }, | ||
| ); | ||
| return { previousData }; | ||
| }, | ||
Comment on lines +94 to +131 MemberAuthor 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. @BrunoQuaresma should I keep or remove this? It essentially just leads to a slightly faster UI response. 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. It is up to you! If you think this faster UI experience, worth the amount of code/complexity, go for it. MemberAuthor 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. Ok, I think it's a bit better and without it's dependent on ping between user <-> coderd <-> agent, so let's keep for now. Pretty easy to eliminate if it becomes a burden. 👍🏻 | ||
| onSuccess: async () => { | ||
| // Invalidate the containers query to refetch updated data. | ||
| await queryClient.invalidateQueries({ | ||
| queryKey: ["agents", parentAgent.id, "containers"], | ||
| }); | ||
| }, | ||
| onError: (error, _, context) => { | ||
| // If the mutation fails, use the context returned from | ||
| // onMutate to roll back. | ||
| if (context?.previousData) { | ||
| queryClient.setQueryData( | ||
| ["agents", parentAgent.id, "containers"], | ||
| context.previousData, | ||
| ); | ||
| } | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : "An unknown error occurred."; | ||
| displayError(`Failed to rebuild devcontainer: ${errorMessage}`); | ||
| console.error("Failed to rebuild devcontainer:", error); | ||
| }, | ||
| }); | ||
| // Re-fetch containers when the subAgent changes to ensure data is | ||
| // in sync. | ||
| const latestSubAgentByName = subAgents.find( | ||
| (agent) => agent.name === devcontainer.name, | ||
| ); | ||
| useEffect(() => { | ||
| if (!latestSubAgentByName) { | ||
| return; | ||
| } | ||
| queryClient.invalidateQueries({ | ||
| queryKey: ["agents", parentAgent.id, "containers"], | ||
| }); | ||
| }, [latestSubAgentByName, queryClient, parentAgent.id]); | ||
| const showDevcontainerControls = subAgent && devcontainer.container; | ||
| const showSubAgentApps = | ||
| devcontainer.status !== "starting" && | ||
| subAgent?.status === "connected" && | ||
| hasAppsToDisplay; | ||
| const showSubAgentAppsPlaceholders = | ||
| devcontainer.status === "starting" || subAgent?.status === "connecting"; | ||
| const handleRebuildDevcontainer = () => { | ||
| rebuildDevcontainerMutation.mutate(); | ||
| }; | ||
| const appsClasses = "flex flex-wrap gap-4 empty:hidden md:justify-start"; | ||
| @@ -172,15 +210,15 @@ export const AgentDevcontainerCard: FC<AgentDevcontainerCardProps> = ({ | ||
| md:overflow-visible" | ||
| > | ||
| {subAgent?.name ?? devcontainer.name} | ||
| {devcontainer.container && ( | ||
| <span className="text-content-tertiary"> | ||
| {" "} | ||
| ({devcontainer.container.name}) | ||
| </span> | ||
| )} | ||
| </span> | ||
| </div> | ||
| {subAgent?.status === "connected" && ( | ||
| <> | ||
| <SubAgentOutdatedTooltip | ||
| devcontainer={devcontainer} | ||
| @@ -190,7 +228,7 @@ export const AgentDevcontainerCard: FC<AgentDevcontainerCardProps> = ({ | ||
| <AgentLatency agent={subAgent} /> | ||
| </> | ||
| )} | ||
| {subAgent?.status === "connecting" && ( | ||
| <> | ||
| <Skeleton width={160} variant="text" /> | ||
| <Skeleton width={36} variant="text" /> | ||
| @@ -203,9 +241,9 @@ export const AgentDevcontainerCard: FC<AgentDevcontainerCardProps> = ({ | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={handleRebuildDevcontainer} | ||
| disabled={devcontainer.status === "starting"} | ||
| > | ||
| <Spinner loading={devcontainer.status === "starting"} /> | ||
| Rebuild | ||
| </Button> | ||
Uh oh!
There was an error while loading.Please reload this page.