diff --git a/react-compiler.config.js b/react-compiler.config.js index 3863459ff..14c257061 100644 --- a/react-compiler.config.js +++ b/react-compiler.config.js @@ -99,6 +99,7 @@ export const REACT_COMPILER_ENABLED_DIRS = [ "src/components/shared/Dialogs/RenameDialog.tsx", "src/components/shared/SecretsManagement/components/SecretsBackendUnavailable.tsx", "src/components/shared/SecretsManagement/components/ExpiryDatePicker.tsx", + "src/components/shared/SecretsManagement/useRunIdentity.ts", "src/components/shared/HighlightText.tsx", "src/components/shared/Notices", "src/components/shared/Markdown", diff --git a/src/agent/toolBridgeApi.ts b/src/agent/toolBridgeApi.ts index f873ee7c1..2515ac2a3 100644 --- a/src/agent/toolBridgeApi.ts +++ b/src/agent/toolBridgeApi.ts @@ -69,10 +69,17 @@ export interface StickyNoteUpdates extends StickyNoteContent { locked?: boolean; } +export interface RunSubmissionOptions { + asTeam?: boolean; +} + export interface RunSubmissionResult { success: boolean; runId?: string; rootExecutionId?: string; + actingAsTeam?: string; + actingAsWorkspace?: string; + warning?: string; error?: string; } @@ -234,7 +241,9 @@ export interface ToolBridgeApi { searchComponents(args: SearchComponentsArgs): Promise; - submitPipelineRun(): Promise; + submitPipelineRun( + options?: RunSubmissionOptions, + ): Promise; getRunDetails(runId: string): Promise; getExecutionDetails(executionId: string): Promise; getExecutionState(executionId: string): Promise; diff --git a/src/agent/tools/runTools.test.ts b/src/agent/tools/runTools.test.ts index a3e2b846f..7efdb5b3f 100644 --- a/src/agent/tools/runTools.test.ts +++ b/src/agent/tools/runTools.test.ts @@ -35,6 +35,25 @@ describe("createRunTools", () => { ]); }); + it.each([ + [{ asTeam: true }, { asTeam: true }], + [{ asTeam: null }, { asTeam: undefined }], + [{}, { asTeam: undefined }], + ])( + "submit_pipeline_run passes %o to the bridge as %o", + async (payload, options) => { + const submitPipelineRun = vi + .fn() + .mockResolvedValue({ success: true, runId: "7" }); + const tools = createRunTools(makeBridge({ submitPipelineRun })); + + const result = await invoke(tools.submitPipelineRun, payload); + + expect(submitPipelineRun).toHaveBeenCalledWith(options); + expect(result).toEqual({ success: true, runId: "7" }); + }, + ); + it("get_run_status derives overall status from execution_status_stats", async () => { const getRunDetails = vi.fn().mockResolvedValue({ id: "1", diff --git a/src/agent/tools/runTools.ts b/src/agent/tools/runTools.ts index a637af2a4..1082cc771 100644 --- a/src/agent/tools/runTools.ts +++ b/src/agent/tools/runTools.ts @@ -31,9 +31,18 @@ export function createRunTools(bridge: ToolBridgeApi) { const submitPipelineRun = tool({ name: "submit_pipeline_run", description: - "Submit the currently-open pipeline to the backend for execution. Only call this when the user has explicitly asked to run the pipeline. Does not take parameters — always submits the live pipeline.", - parameters: z.object({}), - execute: async () => asJson(await bridge.submitPipelineRun()), + "Submit the currently-open pipeline to the backend for execution. Only call this when the user has explicitly asked to run the pipeline. Always submits the live pipeline. A run uses the secrets of whoever submits it: it goes as the user unless asTeam is set, or unless it needs a secret only the project's workspace has, in which case it goes as the workspace. The result's actingAsWorkspace names the workspace it went as, and actingAsTeam is that workspace's team id.", + parameters: z.object({ + asTeam: z + .boolean() + .nullable() + .optional() + .describe( + "Submit as the project's workspace, so the run uses the workspace's secrets instead of the user's. Set it when the user asks for the workspace, or when the run needs a secret only the workspace has. Refused when the user can't act as the workspace.", + ), + }), + execute: async ({ asTeam }) => + asJson(await bridge.submitPipelineRun({ asTeam: asTeam ?? undefined })), }); const getRunStatus = tool({ diff --git a/src/components/Project/RunPipelineButton.test.tsx b/src/components/Project/RunPipelineButton.test.tsx index d692ab9ef..93598f14e 100644 --- a/src/components/Project/RunPipelineButton.test.tsx +++ b/src/components/Project/RunPipelineButton.test.tsx @@ -54,7 +54,11 @@ vi.mock( showCopyFromRun, team, }: { - onConfirm: (args: Record, notes: string) => void; + onConfirm: ( + args: Record, + notes: string, + teamId: string | undefined, + ) => void; showCopyFromRun?: boolean; team: TeamResolution; }) => ( @@ -65,10 +69,22 @@ vi.mock( > + {team.status === "active" && ( + + )} ), }), @@ -178,6 +194,32 @@ describe("RunPipelineButton", () => { "42", "https://backend.test", expect.objectContaining({ notes: "why this run" }), + undefined, + ); + }); + + it("starts the run as the identity chosen in the dialog", async () => { + vi.mocked(useTeamForSubmission).mockReturnValue({ + status: "active", + team: { id: "ml-team@example.com", label: "ML Research", canWrite: true }, + }); + renderButton({}); + const user = userEvent.setup(); + + await user.click(runButton()); + await user.click( + screen.getByRole("button", { name: "Submit as the workspace" }), + ); + submit.mock.calls[0][0].onSuccess({ id: 42 }); + + expect(submit).toHaveBeenCalledWith( + expect.objectContaining({ teamId: "ml-team@example.com" }), + ); + expect(saveAnnotations).toHaveBeenCalledWith( + "42", + "https://backend.test", + expect.objectContaining({ notes: "why this run" }), + "ml-team@example.com", ); }); diff --git a/src/components/Project/RunPipelineButton.tsx b/src/components/Project/RunPipelineButton.tsx index e3efea525..40f0d8e20 100644 --- a/src/components/Project/RunPipelineButton.tsx +++ b/src/components/Project/RunPipelineButton.tsx @@ -35,20 +35,38 @@ export function RunPipelineButton({ const runTeam = useTeamForSubmission([projectId]); const { mutate: saveAnnotations } = useMutation({ - mutationFn: ({ runId, notes }: { runId: string; notes: string }) => - saveRunAnnotations(runId, backendUrl, { notes, componentSpec: spec }), + mutationFn: ({ + runId, + notes, + teamId, + }: { + runId: string; + notes: string; + teamId?: string; + }) => + saveRunAnnotations( + runId, + backendUrl, + { notes, componentSpec: spec }, + teamId, + ), }); const runnable = pipelineValidity(spec) !== "invalid"; - const run = (taskArguments: Record, notes: string) => { + const run = ( + taskArguments: Record, + notes: string, + teamId: string | undefined, + ) => { setDialogOpen(false); submit({ componentSpec: spec, taskArguments, projectIds: [projectId], + teamId, onSuccess: (run: PipelineRun) => { - saveAnnotations({ runId: run.id.toString(), notes }); + saveAnnotations({ runId: run.id.toString(), notes, teamId }); track("projects.run_pipeline_completed", {}); notify("Run started in this project", "success"); }, diff --git a/src/components/shared/SecretsManagement/SelectSecretDialog.test.tsx b/src/components/shared/SecretsManagement/SelectSecretDialog.test.tsx index 1c93e50cb..6715061d4 100644 --- a/src/components/shared/SecretsManagement/SelectSecretDialog.test.tsx +++ b/src/components/shared/SecretsManagement/SelectSecretDialog.test.tsx @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { TeamResolution } from "@/services/teams/types"; import { useActingTeam } from "@/services/teams/useActingTeam"; +import type { RunAs } from "./runIdentity"; import { addSecret } from "./secretsStorage"; import { SelectSecretDialog } from "./SelectSecretDialog"; import type { Secret } from "./types"; @@ -46,7 +47,7 @@ const secret = (name: string): Secret => ({ updatedAt: new Date("2026-10-02T09:00:00Z"), }); -function renderDialog(team?: TeamResolution) { +function renderDialog(team?: TeamResolution, defaultSaveTo?: RunAs) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }); @@ -57,6 +58,7 @@ function renderDialog(team?: TeamResolution) { onOpenChange={vi.fn()} onSelect={onSelect} team={team} + defaultSaveTo={defaultSaveTo} />, { wrapper: ({ children }: { children: ReactNode }) => ( @@ -185,11 +187,23 @@ describe("SelectSecretDialog", () => { ); }); + it("starts on the team's secrets when the run goes as the team", async () => { + const { user } = renderDialog(ACTIVE, "team"); + + await user.click(await screen.findByTestId("select-secret-add-button")); + + expect( + screen.getByRole("radio", { + name: "The ML Research workspace's secrets", + }), + ).toBeChecked(); + }); + it("saves only to your own secrets in a team you can't change", async () => { - const { user } = renderDialog({ - status: "active", - team: { ...TEAM, canWrite: false }, - }); + const { user } = renderDialog( + { status: "active", team: { ...TEAM, canWrite: false } }, + "team", + ); await user.click(await screen.findByTestId("select-secret-add-button")); expect(screen.queryByRole("radiogroup")).toBeNull(); diff --git a/src/components/shared/SecretsManagement/SelectSecretDialog.tsx b/src/components/shared/SecretsManagement/SelectSecretDialog.tsx index ee3635289..d83f04e35 100644 --- a/src/components/shared/SecretsManagement/SelectSecretDialog.tsx +++ b/src/components/shared/SecretsManagement/SelectSecretDialog.tsx @@ -40,6 +40,7 @@ interface SelectSecretDialogProps { onOpenChange: (open: boolean) => void; onSelect: (secretName: string) => void; team?: TeamResolution; + defaultSaveTo?: SecretOwner; } function SelectSecretDialogContentSkeleton() { @@ -194,11 +195,13 @@ function SaveToControl({ interface SelectSecretDialogContentProps { resolution: Exclude; + defaultSaveTo: SecretOwner; onSelect: (secretName: string) => void; } function SelectSecretDialogContentInternal({ resolution, + defaultSaveTo, onSelect, }: SelectSecretDialogContentProps) { const teamId = actingTeamId(resolution); @@ -210,7 +213,7 @@ function SelectSecretDialogContentInternal({ }); const [mode, setMode] = useState("select"); - const [saveTo, setSaveTo] = useState("you"); + const [saveTo, setSaveTo] = useState(defaultSaveTo); const handleSecretSelect = (secret: Secret) => { onSelect(secret.name); @@ -310,10 +313,15 @@ const SelectSecretDialogContent = withSuspenseWrapper( interface TeamSecretsPickerProps { resolution: TeamResolution; + defaultSaveTo: SecretOwner; onSelect: (secretName: string) => void; } -function TeamSecretsPicker({ resolution, onSelect }: TeamSecretsPickerProps) { +function TeamSecretsPicker({ + resolution, + defaultSaveTo, + onSelect, +}: TeamSecretsPickerProps) { if (resolution.status === "resolving") { return ; } @@ -333,15 +341,23 @@ function TeamSecretsPicker({ resolution, onSelect }: TeamSecretsPickerProps) { ); } function PageTeamSecretsPicker({ + defaultSaveTo, onSelect, -}: Pick) { - return ; +}: Pick) { + return ( + + ); } export function SelectSecretDialog({ @@ -349,6 +365,7 @@ export function SelectSecretDialog({ onOpenChange, onSelect, team, + defaultSaveTo = "you", }: SelectSecretDialogProps) { const { available } = useBackend(); const mockBackend = useTourMockBackend(); @@ -364,9 +381,16 @@ export function SelectSecretDialog({ {!available && !mockBackend && } {(available || mockBackend) && (team ? ( - + ) : ( - + ))} )} diff --git a/src/components/shared/SecretsManagement/runIdentity.test.ts b/src/components/shared/SecretsManagement/runIdentity.test.ts new file mode 100644 index 000000000..c37e7bd26 --- /dev/null +++ b/src/components/shared/SecretsManagement/runIdentity.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from "vitest"; + +import { needsReview, runAsFor, runIdentity, teamIdFor } from "./runIdentity"; + +const TEAM = { id: "search@example.com", label: "Search" }; + +const named = (...names: string[]) => names.map((name) => ({ name })); + +function identityFor( + referenced: string[], + yours: string[], + teams: string[] | undefined, +) { + return runIdentity({ + team: teams ? TEAM : undefined, + referenced, + yourSecrets: named(...yours), + teamSecrets: teams && named(...teams), + }); +} + +describe("runIdentity", () => { + it("runs as you when the run uses no secrets", () => { + const identity = identityFor([], [], []); + + expect(identity.rule).toEqual({ kind: "open", defaultAs: "you" }); + expect(runAsFor(identity)).toBe("you"); + expect(needsReview(identity)).toBe(false); + }); + + it("runs as you when you have every secret, and lets you choose the team", () => { + const identity = identityFor(["TEST_PERSONAL"], ["TEST_PERSONAL"], []); + + expect(identity.rule).toEqual({ kind: "open", defaultAs: "you" }); + expect(runAsFor(identity, "team")).toBe("team"); + expect(identity.missing).toEqual({ you: [], team: ["TEST_PERSONAL"] }); + }); + + it("forces the team when only the team has secrets you lack", () => { + const identity = identityFor( + ["TEST_PERSONAL", "Test"], + ["TEST_PERSONAL"], + ["TEST_PERSONAL", "Test"], + ); + + expect(identity.rule).toEqual({ + kind: "team-required", + teamOnly: ["Test"], + }); + expect(runAsFor(identity)).toBe("team"); + expect(runAsFor(identity, "you")).toBe("team"); + expect(teamIdFor(identity, runAsFor(identity))).toBe(TEAM.id); + expect(needsReview(identity)).toBe(false); + }); + + it("says which side has which when no identity has every secret", () => { + const identity = identityFor( + ["TEST_PERSONAL", "Test"], + ["TEST_PERSONAL"], + ["Test"], + ); + + expect(identity.rule).toEqual({ + kind: "split", + yoursOnly: ["TEST_PERSONAL"], + teamOnly: ["Test"], + }); + expect(runAsFor(identity)).toBe("you"); + expect(needsReview(identity)).toBe(true); + }); + + it("runs as you when a secret is missing from both", () => { + const identity = identityFor(["GONE"], [], []); + + expect(identity.rule).toEqual({ kind: "open", defaultAs: "you" }); + expect(identity.missing).toEqual({ you: ["GONE"], team: ["GONE"] }); + expect(needsReview(identity)).toBe(true); + }); + + it("defaults to the team when it has more of the secrets but not all", () => { + const identity = identityFor(["Test", "GONE"], [], ["Test"]); + + expect(identity.rule).toEqual({ kind: "open", defaultAs: "team" }); + expect(runAsFor(identity)).toBe("team"); + expect(identity.missing.team).toEqual(["GONE"]); + }); + + it("only runs as you without a team to act as", () => { + const identity = identityFor(["Test"], [], undefined); + + expect(identity.rule).toEqual({ kind: "open", defaultAs: "you" }); + expect(runAsFor(identity, "team")).toBe("you"); + expect(teamIdFor(identity, "team")).toBeUndefined(); + expect(identity.missing).toEqual({ you: ["Test"], team: [] }); + }); + + it("treats a list it couldn't read as having every secret", () => { + const identity = runIdentity({ + team: TEAM, + referenced: ["Test"], + teamSecrets: named("Test"), + }); + + expect(identity.rule).toEqual({ kind: "open", defaultAs: "you" }); + expect(identity.missing).toEqual({ you: [], team: [] }); + }); +}); diff --git a/src/components/shared/SecretsManagement/runIdentity.ts b/src/components/shared/SecretsManagement/runIdentity.ts new file mode 100644 index 000000000..c4c571a56 --- /dev/null +++ b/src/components/shared/SecretsManagement/runIdentity.ts @@ -0,0 +1,88 @@ +import type { TeamSummary } from "@/services/teams/types"; + +import { missingSecretNames } from "./secretReferences"; +import type { Secret } from "./types"; + +export type RunAs = "you" | "team"; + +export type RunAsRule = + | { kind: "open"; defaultAs: RunAs } + | { kind: "team-required"; teamOnly: string[] } + | { kind: "split"; yoursOnly: string[]; teamOnly: string[] }; + +export interface RunIdentity { + team: TeamSummary | undefined; + rule: RunAsRule; + missing: Record; +} + +interface RunIdentityInputs { + team: TeamSummary | undefined; + referenced: readonly string[]; + yourSecrets?: readonly Pick[]; + teamSecrets?: readonly Pick[]; +} + +function ruleFor( + missingFromYours: string[], + missingFromTeam: string[] | undefined, +): RunAsRule { + if (!missingFromTeam) return { kind: "open", defaultAs: "you" }; + + const teamOnly = missingFromYours.filter( + (name) => !missingFromTeam.includes(name), + ); + const yoursOnly = missingFromTeam.filter( + (name) => !missingFromYours.includes(name), + ); + + if (teamOnly.length === 0) return { kind: "open", defaultAs: "you" }; + if (yoursOnly.length > 0) return { kind: "split", yoursOnly, teamOnly }; + if (missingFromTeam.length === 0) { + return { kind: "team-required", teamOnly }; + } + return { kind: "open", defaultAs: "team" }; +} + +export function runIdentity({ + team, + referenced, + yourSecrets, + teamSecrets, +}: RunIdentityInputs): RunIdentity { + const missingFromYours = yourSecrets + ? missingSecretNames(referenced, yourSecrets) + : []; + const missingFromTeam = + team && teamSecrets + ? missingSecretNames(referenced, teamSecrets) + : undefined; + + return { + team, + rule: ruleFor(missingFromYours, missingFromTeam), + missing: { you: missingFromYours, team: missingFromTeam ?? [] }, + }; +} + +export function runAsFor(identity: RunIdentity, choice?: RunAs): RunAs { + if (!identity.team) return "you"; + const { rule } = identity; + if (rule.kind === "team-required") return "team"; + if (choice) return choice; + return rule.kind === "open" ? rule.defaultAs : "you"; +} + +export function teamIdFor( + identity: RunIdentity, + runAs: RunAs, +): string | undefined { + return runAs === "team" ? identity.team?.id : undefined; +} + +export function needsReview(identity: RunIdentity): boolean { + return ( + identity.rule.kind === "split" || + identity.missing[runAsFor(identity)].length > 0 + ); +} diff --git a/src/components/shared/SecretsManagement/useRunIdentity.test.tsx b/src/components/shared/SecretsManagement/useRunIdentity.test.tsx new file mode 100644 index 000000000..c435c10dc --- /dev/null +++ b/src/components/shared/SecretsManagement/useRunIdentity.test.tsx @@ -0,0 +1,210 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { TeamResolution } from "@/services/teams/types"; + +import { runIdentity } from "./runIdentity"; +import { createSecretArgument, type Secret, SecretsQueryKeys } from "./types"; +import { useRunAsChoice, useRunIdentity } from "./useRunIdentity"; + +const { fetchSecrets } = vi.hoisted(() => ({ fetchSecrets: vi.fn() })); + +vi.mock("./secretsStorage", async () => { + const { SecretsQueryKeys } = await import("./types"); + return { + secretsListQuery: (teamId?: string) => ({ + queryKey: SecretsQueryKeys.All(teamId), + queryFn: () => fetchSecrets(teamId), + }), + }; +}); + +const TEAM = { id: "ml-team@example.com", label: "ML Research" }; +const ACTIVE: TeamResolution = { + status: "active", + team: { ...TEAM, canWrite: true }, +}; + +const secret = (name: string): Secret => ({ + id: name, + name, + createdAt: new Date("2026-10-01T09:00:00Z"), + updatedAt: new Date("2026-10-02T09:00:00Z"), +}); + +const usingSecrets = (...names: string[]) => + Object.fromEntries(names.map((name) => [name, createSecretArgument(name)])); + +const testQueryClient = () => + new QueryClient({ defaultOptions: { queries: { retry: false } } }); + +function check( + resolution: TeamResolution, + sources: unknown[], + queryClient = testQueryClient(), +) { + return renderHook(() => useRunIdentity(resolution, sources), { + wrapper: ({ children }: { children: ReactNode }) => ( + {children} + ), + }); +} + +describe("useRunIdentity", () => { + beforeEach(() => { + fetchSecrets.mockImplementation(async (teamId?: string) => + teamId ? [secret("TEAM_KEY")] : [secret("MY_KEY")], + ); + }); + + afterEach(() => vi.clearAllMocks()); + + it("checks your secrets and the team's", async () => { + const { result } = check(ACTIVE, [usingSecrets("MY_KEY", "TEAM_KEY")]); + + await waitFor(() => expect(result.current.isChecking).toBe(false)); + expect(result.current.rule).toEqual({ + kind: "split", + yoursOnly: ["MY_KEY"], + teamOnly: ["TEAM_KEY"], + }); + expect(result.current.missing).toEqual({ + you: ["TEAM_KEY"], + team: ["MY_KEY"], + }); + expect(fetchSecrets).toHaveBeenCalledWith(undefined); + expect(fetchSecrets).toHaveBeenCalledWith(TEAM.id); + }); + + it("goes as the team when only the team has a secret", async () => { + const { result } = check(ACTIVE, [usingSecrets("TEAM_KEY")]); + + await waitFor(() => expect(result.current.isChecking).toBe(false)); + expect(result.current.rule).toEqual({ + kind: "team-required", + teamOnly: ["TEAM_KEY"], + }); + expect(result.current.team).toEqual(ACTIVE.team); + }); + + it("keeps checking while the team's secrets load, though yours have", async () => { + let listTeamSecrets: (secrets: Secret[]) => void = () => {}; + fetchSecrets.mockImplementation((teamId?: string) => + teamId + ? new Promise((resolve) => { + listTeamSecrets = resolve; + }) + : Promise.resolve([secret("MY_KEY")]), + ); + const queryClient = testQueryClient(); + queryClient.setQueryData(SecretsQueryKeys.All(), [secret("MY_KEY")]); + + const { result } = check(ACTIVE, [usingSecrets("TEAM_KEY")], queryClient); + + expect(result.current.isChecking).toBe(true); + + act(() => listTeamSecrets([secret("TEAM_KEY")])); + + await waitFor(() => expect(result.current.isChecking).toBe(false)); + expect(result.current.rule).toEqual({ + kind: "team-required", + teamOnly: ["TEAM_KEY"], + }); + }); + + it("checks only your secrets when the run falls back to you", async () => { + const { result } = check({ status: "not-member", team: TEAM }, [ + usingSecrets("TEAM_KEY"), + ]); + + await waitFor(() => expect(result.current.isChecking).toBe(false)); + expect(result.current.team).toBeUndefined(); + expect(result.current.missing).toEqual({ you: ["TEAM_KEY"], team: [] }); + expect(fetchSecrets).not.toHaveBeenCalledWith(TEAM.id); + }); + + it("checks nothing for a run that uses no secrets", () => { + const { result } = check(ACTIVE, [{ greeting: "hello" }]); + + expect(result.current).toMatchObject({ + rule: { kind: "open", defaultAs: "you" }, + isChecking: false, + }); + expect(fetchSecrets).not.toHaveBeenCalled(); + }); + + it("waits for the team before checking", () => { + const { result } = check({ status: "resolving" }, [usingSecrets("MY_KEY")]); + + expect(result.current.isChecking).toBe(true); + expect(fetchSecrets).not.toHaveBeenCalled(); + }); + + it("checks nothing for projects on different teams", () => { + const { result } = check( + { + status: "conflict", + teams: [TEAM, { id: "data@example.com", label: "Data" }], + }, + [usingSecrets("MY_KEY")], + ); + + expect(result.current.isChecking).toBe(false); + expect(fetchSecrets).not.toHaveBeenCalled(); + }); + + it("names nothing missing when the secrets can't be listed", async () => { + fetchSecrets.mockRejectedValue(new Error("backend down")); + + const { result } = check(ACTIVE, [usingSecrets("MY_KEY")]); + + await waitFor(() => expect(result.current.isChecking).toBe(false)); + expect(result.current.missing).toEqual({ you: [], team: [] }); + }); +}); + +describe("useRunAsChoice", () => { + const openIdentity = runIdentity({ team: TEAM, referenced: [] }); + + it("starts from the rule's default and follows a choice", () => { + const { result } = renderHook(() => useRunAsChoice(openIdentity)); + + expect(result.current.runAs).toBe("you"); + act(() => result.current.choose("team")); + expect(result.current.runAs).toBe("team"); + act(() => result.current.choose(undefined)); + expect(result.current.runAs).toBe("you"); + }); + + it("forgets a choice made for another team", () => { + const { result, rerender } = renderHook( + ({ identity }) => useRunAsChoice(identity), + { initialProps: { identity: openIdentity } }, + ); + + act(() => result.current.choose("team")); + rerender({ + identity: runIdentity({ + team: { id: "data@example.com", label: "Data" }, + referenced: [], + }), + }); + + expect(result.current.runAs).toBe("you"); + }); + + it("ignores a choice of you while only the team works", () => { + const forced = runIdentity({ + team: TEAM, + referenced: ["TEAM_KEY"], + yourSecrets: [], + teamSecrets: [secret("TEAM_KEY")], + }); + const { result } = renderHook(() => useRunAsChoice(forced)); + + act(() => result.current.choose("you")); + expect(result.current.runAs).toBe("team"); + }); +}); diff --git a/src/components/shared/SecretsManagement/useRunIdentity.ts b/src/components/shared/SecretsManagement/useRunIdentity.ts new file mode 100644 index 000000000..9a8d774cb --- /dev/null +++ b/src/components/shared/SecretsManagement/useRunIdentity.ts @@ -0,0 +1,71 @@ +import { useQuery } from "@tanstack/react-query"; +import { useState } from "react"; + +import type { TeamResolution } from "@/services/teams/types"; + +import { + type RunAs, + runAsFor, + type RunIdentity, + runIdentity, +} from "./runIdentity"; +import { referencedSecretNames } from "./secretReferences"; +import { secretsListQuery } from "./secretsStorage"; + +interface CheckedRunIdentity extends RunIdentity { + isChecking: boolean; +} + +export function useRunIdentity( + resolution: TeamResolution, + sources: readonly unknown[], +): CheckedRunIdentity { + const referenced = referencedSecretNames(...sources); + const team = resolution.status === "active" ? resolution.team : undefined; + const settled = + resolution.status !== "resolving" && resolution.status !== "conflict"; + const shouldCheck = settled && referenced.length > 0; + + const yourSecrets = useQuery({ ...secretsListQuery(), enabled: shouldCheck }); + const teamSecrets = useQuery({ + ...secretsListQuery(team?.id), + enabled: shouldCheck && team !== undefined, + }); + + const unchecked = runIdentity({ team, referenced: [] }); + if (referenced.length === 0) return { ...unchecked, isChecking: false }; + if (resolution.status === "resolving") { + return { ...unchecked, isChecking: true }; + } + if (!shouldCheck) return { ...unchecked, isChecking: false }; + if (yourSecrets.isPending || (team && teamSecrets.isPending)) { + return { ...unchecked, isChecking: true }; + } + + return { + ...runIdentity({ + team, + referenced, + yourSecrets: yourSecrets.data, + teamSecrets: team ? teamSecrets.data : undefined, + }), + isChecking: false, + }; +} + +interface RunAsChoice { + teamId: string | undefined; + runAs: RunAs | undefined; +} + +export function useRunAsChoice(identity: RunIdentity) { + const [choice, setChoice] = useState(); + const teamId = identity.team?.id; + const runAsChosen = + choice && choice.teamId === teamId ? choice.runAs : undefined; + + return { + runAs: runAsFor(identity, runAsChosen), + choose: (runAs: RunAs | undefined) => setChoice({ teamId, runAs }), + }; +} diff --git a/src/components/shared/Submitters/Tangle/TangleSubmitter.test.tsx b/src/components/shared/Submitters/Tangle/TangleSubmitter.test.tsx new file mode 100644 index 000000000..055f190bb --- /dev/null +++ b/src/components/shared/Submitters/Tangle/TangleSubmitter.test.tsx @@ -0,0 +1,372 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + type RunIdentity, + runIdentity, +} from "@/components/shared/SecretsManagement/runIdentity"; +import { useRunIdentity } from "@/components/shared/SecretsManagement/useRunIdentity"; +import type { TeamResolution } from "@/services/teams/types"; +import { useTeamForSubmission } from "@/services/teams/useTeamForSubmission"; +import type { ArgumentType, ComponentSpec } from "@/utils/componentSpec"; + +import TangleSubmitter from "./TangleSubmitter"; + +const submit = vi.fn(); +const saveAnnotations = vi.fn(); + +vi.mock("@tanstack/react-router", () => ({ useNavigate: () => vi.fn() })); + +vi.mock("@tanstack/react-query", () => ({ + useMutation: ({ + mutationFn, + }: { + mutationFn: (variables: unknown) => unknown; + }) => ({ mutate: mutationFn }), +})); + +vi.mock("@/components/shared/Authentication/useAwaitAuthorization", () => ({ + useAwaitAuthorization: () => ({ isAuthorized: true }), +})); + +vi.mock("@/components/shared/Settings/useFlags", () => ({ + useFlagValue: () => false, +})); + +vi.mock("@/hooks/useToastNotification", () => ({ default: () => vi.fn() })); + +vi.mock("@/providers/BackendProvider", () => ({ + useBackend: () => ({ + backendUrl: "https://backend.test", + configured: true, + available: true, + }), +})); + +vi.mock("@/providers/TourProvider/tourMockBackend", () => ({ + useTourMockBackend: () => false, +})); + +vi.mock("@/components/shared/SecretsManagement/useRunIdentity", () => ({ + useRunIdentity: vi.fn(), +})); + +vi.mock("@/services/teams/useTeamForSubmission", () => ({ + useTeamForSubmission: vi.fn(), +})); + +vi.mock("@/components/Project/RunProjectField", () => ({ + RunProjectField: () => null, +})); + +vi.mock("./useRunAttribution", () => ({ + useRunAttribution: () => ({ + projectId: "project-chosen", + setProjectId: vi.fn(), + resetToContext: vi.fn(), + defaultProjectIds: ["project-page"], + chosenProjectIds: ["project-chosen"], + }), +})); + +vi.mock("./useSubmitPipeline", () => ({ + useSubmitPipeline: () => ({ mutate: submit, isPending: false }), +})); + +vi.mock("./saveRunAnnotations", () => ({ + saveRunAnnotations: (...args: unknown[]) => saveAnnotations(...args), +})); + +vi.mock("./components/SubmitTaskArgumentsDialog", () => ({ + SubmitTaskArgumentsDialog: ({ + open, + onConfirm, + team, + }: { + open: boolean; + onConfirm: ( + args: Record, + notes: string, + teamId: string | undefined, + ) => void; + team: TeamResolution; + }) => + open ? ( +
+ + {team.status === "active" && ( + + )} +
+ ) : null, +})); + +const spec: ComponentSpec = { + name: "churn-training", + implementation: { + graph: { + tasks: { + Greet: { + componentRef: { + spec: { + name: "Greet", + implementation: { + container: { image: "python:3.11", command: ["echo"] }, + }, + }, + }, + }, + }, + }, + }, +}; + +const ML_TEAM = { id: "ml-team@example.com", label: "ML Research" }; +const DATA_TEAM = { id: "data@example.com", label: "Data" }; +const ML_ACTIVE: TeamResolution = { + status: "active", + team: { ...ML_TEAM, canWrite: true }, +}; + +const named = (...names: string[]) => names.map((name) => ({ name })); + +function withIdentity(identity: RunIdentity, isChecking = false) { + vi.mocked(useRunIdentity).mockReturnValue({ ...identity, isChecking }); +} + +function mlIdentity(referenced: string[], yours: string[], teams: string[]) { + return runIdentity({ + team: ML_TEAM, + referenced, + yourSecrets: named(...yours), + teamSecrets: named(...teams), + }); +} + +function withTeams(page: TeamResolution, chosen: TeamResolution = page) { + vi.mocked(useTeamForSubmission).mockImplementation((projectIds) => + projectIds.includes("project-page") ? page : chosen, + ); +} + +const submitButton = () => screen.getByRole("button", { name: /Submit Run/ }); + +describe("TangleSubmitter", () => { + beforeEach(() => { + withTeams({ status: "none" }); + withIdentity(runIdentity({ team: undefined, referenced: [] })); + }); + + afterEach(() => vi.clearAllMocks()); + + it.each([ + ["no secrets", mlIdentity([], [], [])], + [ + "only your own secrets", + mlIdentity(["TEST_PERSONAL"], ["TEST_PERSONAL"], []), + ], + ])( + "submits as you in a team's project when the run uses %s", + async (_, identity) => { + withTeams(ML_ACTIVE); + withIdentity(identity); + render(); + const user = userEvent.setup(); + + expect(screen.queryByTestId("submit-as-team")).toBeNull(); + await user.click(submitButton()); + + expect(submit).toHaveBeenCalledWith( + expect.objectContaining({ teamId: undefined }), + ); + submit.mock.calls[0][0].onSuccess({ id: 42 }); + expect(saveAnnotations).toHaveBeenCalledWith( + "42", + "https://backend.test", + expect.anything(), + undefined, + ); + }, + ); + + it("submits as the team, and says so, when only the team has a secret", async () => { + withTeams(ML_ACTIVE); + withIdentity(mlIdentity(["Test"], [], ["Test"])); + render(); + const user = userEvent.setup(); + + expect(screen.getByTestId("submit-as-team")).toHaveTextContent( + "as ML Research", + ); + expect(screen.getByTitle(ML_TEAM.id)).toHaveTextContent("ML Research"); + await user.click(submitButton()); + + expect(submit).toHaveBeenCalledWith( + expect.objectContaining({ + projectIds: ["project-page"], + teamId: ML_TEAM.id, + }), + ); + submit.mock.calls[0][0].onSuccess({ id: 42 }); + expect(saveAnnotations).toHaveBeenCalledWith( + "42", + "https://backend.test", + expect.anything(), + ML_TEAM.id, + ); + }); + + it("asks you to review a run whose secrets are split between you and the team", async () => { + withTeams(ML_ACTIVE); + withIdentity( + mlIdentity(["TEST_PERSONAL", "Test"], ["TEST_PERSONAL"], ["Test"]), + ); + render(); + const user = userEvent.setup(); + + await user.click(submitButton()); + + expect(screen.getByRole("dialog")).toBeInTheDocument(); + expect(submit).not.toHaveBeenCalled(); + }); + + it("asks you to review a run that would run as you instead of the team", async () => { + withTeams({ status: "not-member", team: ML_TEAM }); + render(); + const user = userEvent.setup(); + + await user.click(submitButton()); + + expect(screen.getByRole("dialog")).toBeInTheDocument(); + expect(submit).not.toHaveBeenCalled(); + }); + + it("asks you to review a run whose secrets are missing", async () => { + withIdentity( + runIdentity({ + team: undefined, + referenced: ["API_KEY"], + yourSecrets: [], + }), + ); + render(); + const user = userEvent.setup(); + + await user.click(submitButton()); + + expect(screen.getByRole("dialog")).toBeInTheDocument(); + expect(submit).not.toHaveBeenCalled(); + }); + + it("submits straight away outside a team", async () => { + render(); + const user = userEvent.setup(); + + await user.click(submitButton()); + + expect(submit).toHaveBeenCalledWith( + expect.objectContaining({ teamId: undefined }), + ); + }); + + it("waits for the page's team before submitting", () => { + withTeams({ status: "resolving" }); + render(); + + expect(submitButton()).toBeDisabled(); + }); + + it("waits for the secrets check before submitting", () => { + withIdentity(runIdentity({ team: undefined, referenced: [] }), true); + render(); + + expect(submitButton()).toBeDisabled(); + }); + + it("won't submit for projects on different teams", () => { + withTeams({ status: "conflict", teams: [ML_TEAM, DATA_TEAM] }); + render(); + + expect(submitButton()).toBeDisabled(); + expect( + screen.getByText("(projects on different workspaces)"), + ).toBeVisible(); + }); + + it("submits a reviewed run as the identity chosen in it", async () => { + withTeams( + { status: "not-member", team: ML_TEAM }, + { status: "active", team: { ...DATA_TEAM, canWrite: true } }, + ); + render(); + const user = userEvent.setup(); + + await user.click(submitButton()); + expect(screen.getByRole("dialog")).toHaveAttribute( + "data-team-status", + "active", + ); + await user.click( + screen.getByRole("button", { name: "Confirm as the workspace" }), + ); + + expect(submit).toHaveBeenCalledWith( + expect.objectContaining({ + projectIds: ["project-chosen"], + teamId: DATA_TEAM.id, + }), + ); + submit.mock.calls[0][0].onSuccess({ id: 42 }); + expect(saveAnnotations).toHaveBeenCalledWith( + "42", + "https://backend.test", + expect.anything(), + DATA_TEAM.id, + ); + }); + + it("submits a reviewed run as you when you choose yourself on a team's page", async () => { + withTeams(ML_ACTIVE); + withIdentity(mlIdentity(["Test"], [], ["Test"])); + render( + , + ); + const user = userEvent.setup(); + + expect(screen.getByTestId("submit-as-team")).toHaveTextContent( + "as ML Research", + ); + await user.click(screen.getByTestId("run-with-arguments-button")); + await user.click(screen.getByRole("button", { name: "Confirm as you" })); + + expect(submit).toHaveBeenCalledWith( + expect.objectContaining({ teamId: undefined }), + ); + submit.mock.calls[0][0].onSuccess({ id: 42 }); + expect(saveAnnotations).toHaveBeenCalledWith( + "42", + "https://backend.test", + expect.anything(), + undefined, + ); + }); + + it("checks the run's secrets against the page's team", () => { + const pageTeam: TeamResolution = { status: "not-member", team: ML_TEAM }; + withTeams(pageTeam, { + status: "active", + team: { ...DATA_TEAM, canWrite: true }, + }); + + render(); + + expect(useRunIdentity).toHaveBeenCalledWith(pageTeam, [spec]); + }); +}); diff --git a/src/components/shared/Submitters/Tangle/TangleSubmitter.tsx b/src/components/shared/Submitters/Tangle/TangleSubmitter.tsx index 3767edff6..8ba08461b 100644 --- a/src/components/shared/Submitters/Tangle/TangleSubmitter.tsx +++ b/src/components/shared/Submitters/Tangle/TangleSubmitter.tsx @@ -5,7 +5,14 @@ import { type MouseEvent, useRef, useState } from "react"; import { RunProjectField } from "@/components/Project/RunProjectField"; import { useAwaitAuthorization } from "@/components/shared/Authentication/useAwaitAuthorization"; +import { + needsReview, + runAsFor, + teamIdFor, +} from "@/components/shared/SecretsManagement/runIdentity"; +import { useRunIdentity } from "@/components/shared/SecretsManagement/useRunIdentity"; import { useFlagValue } from "@/components/shared/Settings/useFlags"; +import { TeamName } from "@/components/shared/Teams/TeamName"; import { Button } from "@/components/ui/button"; import { Icon } from "@/components/ui/icon"; import { InlineStack } from "@/components/ui/layout"; @@ -15,6 +22,7 @@ import { cn } from "@/lib/utils"; import { useBackend } from "@/providers/BackendProvider"; import { useTourMockBackend } from "@/providers/TourProvider/tourMockBackend"; import { getDefaultRunPath } from "@/routes/runRoutes"; +import { fallsBackToYou } from "@/services/teams/teamResolution"; import { useTeamForSubmission } from "@/services/teams/useTeamForSubmission"; import type { PipelineRun } from "@/types/pipelineRun"; import { @@ -50,7 +58,10 @@ const TangleSubmitter = ({ const { mutate: submit, isPending: isSubmitting } = useSubmitPipeline(); const isAutoRedirect = useFlagValue("redirect-on-new-pipeline-run"); const attribution = useRunAttribution(); + const pageTeam = useTeamForSubmission(attribution.defaultProjectIds); const chosenProjectsTeam = useTeamForSubmission(attribution.chosenProjectIds); + const pageIdentity = useRunIdentity(pageTeam, [componentSpec]); + const pageTeamId = teamIdFor(pageIdentity, runAsFor(pageIdentity)); const [submitSuccess, setSubmitSuccess] = useState(null); const [isArgumentsDialogOpen, setIsArgumentsDialogOpen] = useState(false); @@ -59,13 +70,19 @@ const TangleSubmitter = ({ const navigate = useNavigate(); const runNotes = useRef(""); + const runTeamId = useRef(undefined); const { mutate: saveAnnotations } = useMutation({ mutationFn: (runId: string) => - saveRunAnnotations(runId, backendUrl, { - notes: runNotes.current, - componentSpec, - }), + saveRunAnnotations( + runId, + backendUrl, + { + notes: runNotes.current, + componentSpec, + }, + runTeamId.current, + ), }); const handleError = (message: string) => { @@ -128,8 +145,9 @@ const TangleSubmitter = ({ }; const handleSubmit = async ( - taskArguments?: Record, - runProjectIds: string[] = attribution.defaultProjectIds, + taskArguments: Record | undefined, + runProjectIds: string[], + teamId: string | undefined, ) => { if (!componentSpec) { handleError("No pipeline to submit"); @@ -152,10 +170,12 @@ const TangleSubmitter = ({ } setSubmitSuccess(null); + runTeamId.current = teamId; submit({ componentSpec, taskArguments, projectIds: runProjectIds, + teamId, onSuccess, onError, }); @@ -164,10 +184,22 @@ const TangleSubmitter = ({ const handleSubmitWithArguments = ( args: Record, notes: string, + teamId: string | undefined, ) => { runNotes.current = notes; setIsArgumentsDialogOpen(false); - handleSubmit(args, attribution.chosenProjectIds); + handleSubmit(args, attribution.chosenProjectIds, teamId); + }; + + const pageTeamNeedsReview = + fallsBackToYou(pageTeam) || needsReview(pageIdentity); + + const handleQuickSubmit = () => { + if (pageTeamNeedsReview) { + openArgumentsDialog(); + return; + } + handleSubmit(undefined, attribution.defaultProjectIds, pageTeamId); }; const hasConfigurableInputs = (componentSpec?.inputs?.length ?? 0) > 0; @@ -191,6 +223,11 @@ const TangleSubmitter = ({ isGraphImplementation(componentSpec.implementation) && Object.keys(componentSpec.implementation.graph.tasks).length > 0; + const isPageTeamUnsettled = + pageTeam.status === "resolving" || + pageTeam.status === "conflict" || + pageIdentity.isChecking; + const isButtonDisabled = isSubmitting || !isSubmittable; const isArgumentsButtonVisible = @@ -216,13 +253,27 @@ const TangleSubmitter = ({ <>