mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(site/src/pages/AgentsPage/components/ChatElements/tools): delete dead execute auth_required flow (#27687)
Stacked on #27684. Addresses review note CRF-2 from that PR: the kept `auth_required` execute path is dead by the same premise that PR proved for `wait_for_external_auth`. The chatd execute tool's `ExecuteResult` struct (`coderd/x/chatd/chattool/execute.go:79-88`) has no `auth_required`, `authenticate_url`, or `provider_*` fields, so the execute tool cannot emit the payload this path parsed. The `authenticate_url` matches elsewhere in Go are the unrelated workspace-creation external-auth flow (`codersdk.TemplateVersionExternalAuth`). Per `bb3a363ed4`, the `auth_required` execute payload was written and removed on an unmerged branch before #22290 squash merged, so no server version ever emitted it. Removes: - `ExecuteAuthRequiredTool` and its `ExecuteRenderer` branch - the `authenticateURL`/`providerLabel` chain in `getExecuteRenderData`, and the `Boolean(data.authenticateURL)` disjunct in `shouldRenderExecuteTool` - the `ExecuteAuthRequired` Storybook story - the now-dead `toProviderLabel` helper and its test block - the `auth_required` visibility test case The `providerLabel` identifiers elsewhere under `site/src` (ModelSelector, ModelRow, AISettings) belong to the unrelated AI model/provider selector and are untouched. 🤖 This pull request was created with Coder Agents.
This commit is contained in:
@@ -1,12 +1,6 @@
|
||||
import {
|
||||
CircleAlertIcon,
|
||||
ExternalLinkIcon,
|
||||
LayersIcon,
|
||||
OctagonXIcon,
|
||||
} from "lucide-react";
|
||||
import { LayersIcon, OctagonXIcon } from "lucide-react";
|
||||
import type React from "react";
|
||||
import type * as TypesGen from "#/api/typesGenerated";
|
||||
import { Button } from "#/components/Button/Button";
|
||||
import { CopyButton } from "#/components/CopyButton/CopyButton";
|
||||
import { ScrollArea } from "#/components/ScrollArea/ScrollArea";
|
||||
import {
|
||||
@@ -220,64 +214,3 @@ const ShellTranscriptBody: React.FC<{
|
||||
</ScrollArea>
|
||||
);
|
||||
};
|
||||
|
||||
export const ExecuteAuthRequiredTool: React.FC<{
|
||||
command: string;
|
||||
output: string;
|
||||
authenticateURL: string;
|
||||
providerLabel: string;
|
||||
}> = ({ command, output, authenticateURL, providerLabel }) => {
|
||||
const hasCommand = command.trim().length > 0;
|
||||
const hasOutput = output.trim().length > 0;
|
||||
|
||||
return (
|
||||
<div className="w-full overflow-hidden rounded-md border border-solid border-border-default bg-surface-primary">
|
||||
<div className="flex flex-wrap items-center gap-2 px-3 py-2">
|
||||
<CircleAlertIcon className="size-4 shrink-0 text-content-warning" />
|
||||
<span className="text-[13px] text-content-primary">
|
||||
Authenticate with {providerLabel} to continue this command.
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex flex-wrap items-center gap-2 px-3 pb-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() =>
|
||||
window.open(authenticateURL, "_blank", "width=900,height=600")
|
||||
}
|
||||
className="inline-flex cursor-pointer items-center gap-1 text-xs"
|
||||
>
|
||||
<ExternalLinkIcon className="size-3.5 shrink-0" />
|
||||
Authenticate with {providerLabel}
|
||||
</Button>
|
||||
<a
|
||||
href={authenticateURL}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
className="inline-flex items-center gap-1 text-xs text-content-link no-underline hover:underline"
|
||||
>
|
||||
<ExternalLinkIcon className="size-3.5 shrink-0" />
|
||||
Open authentication link
|
||||
</a>
|
||||
</div>
|
||||
{hasCommand && (
|
||||
<div className="px-3 pb-1">
|
||||
<code className="font-mono text-xs text-content-secondary">
|
||||
$ {command}
|
||||
</code>
|
||||
</div>
|
||||
)}
|
||||
{hasOutput && (
|
||||
<ScrollArea
|
||||
className="rounded-b-md border-t border-solid border-border-default text-2xs"
|
||||
viewportClassName="max-h-48"
|
||||
scrollBarClassName="w-1.5"
|
||||
>
|
||||
<pre className="m-0 whitespace-pre-wrap break-all border-0 bg-transparent px-3 py-2 font-mono text-xs text-content-secondary">
|
||||
{output}
|
||||
</pre>
|
||||
</ScrollArea>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -1,13 +1,5 @@
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import {
|
||||
expect,
|
||||
fn,
|
||||
screen,
|
||||
spyOn,
|
||||
userEvent,
|
||||
waitFor,
|
||||
within,
|
||||
} from "storybook/test";
|
||||
import { expect, fn, screen, userEvent, waitFor, within } from "storybook/test";
|
||||
import { reactRouterParameters } from "storybook-addon-remix-react-router";
|
||||
import { chatModelConfigsKey } from "#/api/queries/chats";
|
||||
import { MockChatModelConfig } from "#/testHelpers/chatModels";
|
||||
@@ -565,37 +557,6 @@ export const ProcessOutputStringError: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
export const ExecuteAuthRequired: Story = {
|
||||
args: {
|
||||
result: {
|
||||
auth_required: true,
|
||||
provider_display_name: "GitHub",
|
||||
authenticate_url: "https://coder.example.com/external-auth/github",
|
||||
output:
|
||||
"fatal: could not read Username for 'https://github.com': terminal prompts disabled",
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const button = canvas.getByRole("button", {
|
||||
name: "Authenticate with GitHub",
|
||||
});
|
||||
expect(button).toBeInTheDocument();
|
||||
expect(
|
||||
canvas.getByRole("link", { name: "Open authentication link" }),
|
||||
).toHaveAttribute("href", "https://coder.example.com/external-auth/github");
|
||||
|
||||
const openSpy = spyOn(window, "open").mockImplementation(() => null);
|
||||
await userEvent.click(button);
|
||||
expect(openSpy).toHaveBeenCalledWith(
|
||||
"https://coder.example.com/external-auth/github",
|
||||
"_blank",
|
||||
"width=900,height=600",
|
||||
);
|
||||
openSpy.mockRestore();
|
||||
},
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Subagent stories
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -14,10 +14,7 @@ import { ComputerTool } from "./ComputerTool";
|
||||
import { CreateWorkspaceTool } from "./CreateWorkspaceTool";
|
||||
import { DiffFileHeader } from "./DiffFileHeader";
|
||||
import { EditFilesTool } from "./EditFilesTool";
|
||||
import {
|
||||
ExecuteAuthRequiredTool,
|
||||
ExecuteTool as ExecuteToolComponent,
|
||||
} from "./ExecuteTool";
|
||||
import { ExecuteTool as ExecuteToolComponent } from "./ExecuteTool";
|
||||
import { ListAgentsTool } from "./ListAgentsTool";
|
||||
import { ListTemplatesTool } from "./ListTemplatesTool";
|
||||
import { ProcessOutputTool } from "./ProcessOutputTool";
|
||||
@@ -225,20 +222,6 @@ const ExecuteRenderer: FC<ToolRendererProps> = ({
|
||||
shellToolDisplayMode,
|
||||
}) => {
|
||||
const data = getExecuteRenderData(args, result);
|
||||
const outputBlock = data.transcriptBlocks.find(
|
||||
(block) => block.kind === "output",
|
||||
);
|
||||
|
||||
if (data.authenticateURL) {
|
||||
return (
|
||||
<ExecuteAuthRequiredTool
|
||||
command={data.command}
|
||||
output={outputBlock?.text ?? ""}
|
||||
authenticateURL={data.authenticateURL}
|
||||
providerLabel={data.providerLabel}
|
||||
/>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<ExecuteToolComponent
|
||||
command={data.command}
|
||||
|
||||
@@ -6,7 +6,7 @@ const stoppedWorkspaceError =
|
||||
|
||||
describe("toolVisibility", () => {
|
||||
describe("getExecuteRenderData", () => {
|
||||
it("parses execute output and auth metadata from result payloads", () => {
|
||||
it("parses execute output from result payloads", () => {
|
||||
expect(
|
||||
getExecuteRenderData(
|
||||
{ command: "git fetch origin" },
|
||||
@@ -14,9 +14,6 @@ describe("toolVisibility", () => {
|
||||
output: " fetched ",
|
||||
wall_duration_ms: "47200",
|
||||
background_process_id: "process-1",
|
||||
auth_required: true,
|
||||
authenticate_url: "https://example.com/auth",
|
||||
provider_display_name: "GitHub",
|
||||
},
|
||||
),
|
||||
).toEqual({
|
||||
@@ -25,8 +22,6 @@ describe("toolVisibility", () => {
|
||||
errorText: "",
|
||||
durationMs: 47200,
|
||||
isBackgrounded: true,
|
||||
authenticateURL: "https://example.com/auth",
|
||||
providerLabel: "GitHub",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -68,7 +63,7 @@ describe("toolVisibility", () => {
|
||||
});
|
||||
|
||||
describe("shouldRenderTool", () => {
|
||||
it("hides execute rows with neither a command nor an auth prompt", () => {
|
||||
it("hides execute rows without a command", () => {
|
||||
expect(
|
||||
shouldRenderTool({
|
||||
name: "execute",
|
||||
@@ -79,20 +74,6 @@ describe("toolVisibility", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps execute rows when auth is required even without a command", () => {
|
||||
expect(
|
||||
shouldRenderTool({
|
||||
name: "execute",
|
||||
status: "completed",
|
||||
args: {},
|
||||
result: {
|
||||
auth_required: true,
|
||||
authenticate_url: "https://example.com/auth",
|
||||
},
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("hides running wait_agent rows until chat_id is available", () => {
|
||||
expect(
|
||||
shouldRenderTool({
|
||||
|
||||
@@ -5,7 +5,6 @@ import {
|
||||
asString,
|
||||
parseArgs,
|
||||
type ToolStatus,
|
||||
toProviderLabel,
|
||||
} from "./utils";
|
||||
|
||||
export type ExecuteTranscriptBlock = {
|
||||
@@ -19,8 +18,6 @@ type ExecuteRenderData = {
|
||||
errorText: string;
|
||||
durationMs?: number;
|
||||
isBackgrounded: boolean;
|
||||
authenticateURL: string;
|
||||
providerLabel: string;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -52,14 +49,6 @@ export const getExecuteRenderData = (
|
||||
const isBackgrounded = Boolean(
|
||||
rec && asString(rec.background_process_id).trim(),
|
||||
);
|
||||
const authenticateURL = rec?.auth_required
|
||||
? asString(rec.authenticate_url).trim()
|
||||
: "";
|
||||
const providerLabel = toProviderLabel(
|
||||
rec ? asString(rec.provider_display_name).trim() : "",
|
||||
rec ? asString(rec.provider_id).trim() : "",
|
||||
rec ? asString(rec.provider_type).trim() : "",
|
||||
);
|
||||
|
||||
return {
|
||||
command,
|
||||
@@ -67,15 +56,9 @@ export const getExecuteRenderData = (
|
||||
errorText,
|
||||
durationMs,
|
||||
isBackgrounded,
|
||||
authenticateURL,
|
||||
providerLabel,
|
||||
};
|
||||
};
|
||||
|
||||
const shouldRenderExecuteTool = (data: ExecuteRenderData): boolean => {
|
||||
return data.command.trim().length > 0 || Boolean(data.authenticateURL);
|
||||
};
|
||||
|
||||
const shouldRenderSubagentLifecycleTool = ({
|
||||
name,
|
||||
status,
|
||||
@@ -122,7 +105,7 @@ export const shouldRenderTool = ({
|
||||
result?: unknown;
|
||||
}): boolean => {
|
||||
if (name === "execute") {
|
||||
return shouldRenderExecuteTool(getExecuteRenderData(args, result));
|
||||
return getExecuteRenderData(args, result).command.trim().length > 0;
|
||||
}
|
||||
|
||||
return shouldRenderSubagentLifecycleTool({ name, status, args, result });
|
||||
|
||||
@@ -29,7 +29,6 @@ import {
|
||||
sanitizeExecuteModelIntent,
|
||||
stripSvnIndexHeaders,
|
||||
summarizeParsedCommands,
|
||||
toProviderLabel,
|
||||
} from "./utils";
|
||||
|
||||
describe("formatModelIntentLabel", () => {
|
||||
@@ -84,24 +83,6 @@ describe("sanitizeExecuteModelIntent", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("toProviderLabel", () => {
|
||||
it("returns displayName when provided", () => {
|
||||
expect(toProviderLabel("GitHub", "gh-id", "oauth")).toBe("GitHub");
|
||||
});
|
||||
|
||||
it("falls back to providerID when displayName is empty", () => {
|
||||
expect(toProviderLabel("", "gh-id", "oauth")).toBe("gh-id");
|
||||
});
|
||||
|
||||
it("falls back to providerType when displayName and ID are empty", () => {
|
||||
expect(toProviderLabel("", "", "oauth")).toBe("oauth");
|
||||
});
|
||||
|
||||
it("returns default label when all are empty", () => {
|
||||
expect(toProviderLabel("", "", "")).toBe("Git provider");
|
||||
});
|
||||
});
|
||||
|
||||
describe("formatShellDurationMs", () => {
|
||||
it("returns empty string for invalid values", () => {
|
||||
expect(formatShellDurationMs(undefined)).toBe("");
|
||||
|
||||
@@ -101,23 +101,6 @@ const isCommandReference = (value: string, command: string): boolean => {
|
||||
const normalizeCommandReference = (value: string): string =>
|
||||
value.trim().toLowerCase().replace(/\s+/g, " ");
|
||||
|
||||
export const toProviderLabel = (
|
||||
providerDisplayName: string,
|
||||
providerID: string,
|
||||
providerType: string,
|
||||
): string => {
|
||||
if (providerDisplayName) {
|
||||
return providerDisplayName;
|
||||
}
|
||||
if (providerID) {
|
||||
return providerID;
|
||||
}
|
||||
if (providerType) {
|
||||
return providerType;
|
||||
}
|
||||
return "Git provider";
|
||||
};
|
||||
|
||||
const roundToTenths = (value: number): number => Number(value.toFixed(1));
|
||||
|
||||
export const formatShellDurationMs = (
|
||||
|
||||
Reference in New Issue
Block a user