From 7ac5631f55eb3890d0ed32ea5c429e1631f6e7c6 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Wed, 4 Nov 2026 17:18:10 -0800 Subject: [PATCH 1/2] fix(portal): lock profile-managed resources Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/components/ResourcePicker.stories.tsx | 115 ++++++++++++++++++ .../src/components/ResourcePicker.test.tsx | 5 + apps/portal/src/components/ResourcePicker.tsx | 26 +++- apps/portal/src/pages/SubmitRun.tsx | 80 ++++++------ .../src/pages/SubmitRun.variations.test.tsx | 74 ++++++++++- docs/architecture/resources.md | 11 +- 6 files changed, 255 insertions(+), 56 deletions(-) create mode 100644 apps/portal/src/components/ResourcePicker.stories.tsx diff --git a/apps/portal/src/components/ResourcePicker.stories.tsx b/apps/portal/src/components/ResourcePicker.stories.tsx new file mode 100644 index 000000000..764cff293 --- /dev/null +++ b/apps/portal/src/components/ResourcePicker.stories.tsx @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { useEffect, useState } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { http, HttpResponse } from "msw"; +import type { ResourceBindingSpec, ResourceDocument, ResourceRevisionDocument } from "@/types"; +import { getSelectedProjectId, setSelectedProjectIdHolder } from "@/lib/project-scope"; +import { ResourcePicker } from "./ResourcePicker"; + +const resource: ResourceDocument = { + _id: "resource-1", + projectId: "demo-project", + slug: "github-simulator", + name: "GitHub simulator", + revisionCounter: 1, + latestRevisionId: "resource-revision-1", + latestRevisionNumber: 1, + createdAt: "2026-01-01T00:00:00.000Z", +}; + +const revision: ResourceRevisionDocument = { + _id: "resource-revision-1", + resourceId: resource._id, + projectId: resource.projectId, + slug: resource.slug, + revisionNumber: 1, + ref: "github-simulator@r1", + setup: { sh: "echo ready" }, + exports: ["SIMULATOR_URL"], + parameters: [ + { name: "REPO", required: true, description: "Repository to simulate" }, + { name: "SCENARIO", required: false, default: "default-scenario" }, + ], + contentSha256: "sha", + createdAt: "2026-01-01T00:00:00.000Z", +}; + +function ResourcePickerHarness({ + initial, + profileBindings = [], + bindingsLocked = false, +}: { + initial: ResourceBindingSpec[]; + profileBindings?: ResourceBindingSpec[]; + bindingsLocked?: boolean; +}) { + const [selected, setSelected] = useState(initial); + const [scopeReady, setScopeReady] = useState(false); + + useEffect(() => { + const previousProjectId = getSelectedProjectId(); + setSelectedProjectIdHolder("demo-project"); + setScopeReady(true); + return () => setSelectedProjectIdHolder(previousProjectId); + }, []); + + if (!scopeReady) return null; + + return ( +
+ +
+ ); +} + +const profileBindings: ResourceBindingSpec[] = [{ + ref: revision.ref, + params: { REPO: "microsoft/scope" }, +}]; + +const meta = { + title: "Components/ResourcePicker", + component: ResourcePicker, + parameters: { + msw: { + handlers: [ + http.get("*/api/v1/resources", () => HttpResponse.json([resource])), + http.get("*/api/v1/resources/:resourceId/revisions", () => HttpResponse.json([revision])), + ], + }, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const RunOwned: Story = { + args: { + selected: [], + onChange: () => {}, + }, + render: () => , +}; + +export const ProfileManaged: Story = { + args: { + selected: profileBindings, + profileBindings, + bindingsLocked: true, + onChange: () => {}, + }, + render: () => ( + + ), +}; diff --git a/apps/portal/src/components/ResourcePicker.test.tsx b/apps/portal/src/components/ResourcePicker.test.tsx index 37c4f9c86..5a6a1c81a 100644 --- a/apps/portal/src/components/ResourcePicker.test.tsx +++ b/apps/portal/src/components/ResourcePicker.test.tsx @@ -65,6 +65,7 @@ afterEach(() => { function renderPicker(props: { selected: ResourceBindingSpec[]; profileBindings?: ResourceBindingSpec[]; + bindingsLocked?: boolean; onChange?: (bindings: ResourceBindingSpec[]) => void; }) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); @@ -73,6 +74,7 @@ function renderPicker(props: { , @@ -87,6 +89,7 @@ describe("ResourcePicker parameter bindings", () => { renderPicker({ selected, profileBindings: selected, + bindingsLocked: true, onChange, }); @@ -98,6 +101,8 @@ describe("ResourcePicker parameter bindings", () => { const scenarioInput = screen.getByLabelText("SCENARIO") as HTMLInputElement; expect(scenarioInput.disabled).toBe(false); expect(scenarioInput.value).toBe("default-scenario"); + expect(screen.queryByPlaceholderText("Search resources…")).toBeNull(); + expect(screen.queryByText("Create a resource")).toBeNull(); fireEvent.change(scenarioInput, { target: { value: "custom-scenario" } }); diff --git a/apps/portal/src/components/ResourcePicker.tsx b/apps/portal/src/components/ResourcePicker.tsx index 957408f3a..7119ed410 100644 --- a/apps/portal/src/components/ResourcePicker.tsx +++ b/apps/portal/src/components/ResourcePicker.tsx @@ -17,6 +17,8 @@ interface ResourcePickerProps { selected: ResourceBindingSpec[]; onChange: (specs: ResourceBindingSpec[]) => void; profileBindings?: ResourceBindingSpec[]; + /** Prevent adding, removing, or replacing bindings while leaving run-owned parameter gaps editable. */ + bindingsLocked?: boolean; disabled?: boolean; } @@ -26,7 +28,13 @@ export function parseResourceRef(ref: string): { slug: string; revisionRef?: str return { slug: ref }; } -export function ResourcePicker({ selected, onChange, profileBindings = [], disabled = false }: ResourcePickerProps) { +export function ResourcePicker({ + selected, + onChange, + profileBindings = [], + bindingsLocked = false, + disabled = false, +}: ResourcePickerProps) { const queryClient = useQueryClient(); const containerRef = useRef(null); const inputRef = useRef(null); @@ -72,7 +80,8 @@ export function ResourcePicker({ selected, onChange, profileBindings = [], disab onChange(selected.filter((_, i) => i !== index)); }; - const showDropdown = open && (matches.length > 0 || query.trim().length > 0); + const canChangeBindings = !disabled && !bindingsLocked; + const showDropdown = canChangeBindings && open && (matches.length > 0 || query.trim().length > 0); if (isLoading) return ; @@ -90,6 +99,7 @@ export function ResourcePicker({ selected, onChange, profileBindings = [], disab binding={binding} profileBinding={findProfileBinding(binding, profileBindings)} resources={activeResources} + bindingLocked={bindingsLocked} disabled={disabled} onReplace={(next) => replaceBinding(index, { ref: next })} onChange={(next) => replaceBinding(index, next)} @@ -99,7 +109,7 @@ export function ResourcePicker({ selected, onChange, profileBindings = [], disab )} - {!disabled && ( + {canChangeBindings && (
)} - {!disabled && ( + {canChangeBindings && (
); } diff --git a/apps/portal/src/pages/SubmitRun.tsx b/apps/portal/src/pages/SubmitRun.tsx index 3fc762efa..dd47e800f 100644 --- a/apps/portal/src/pages/SubmitRun.tsx +++ b/apps/portal/src/pages/SubmitRun.tsx @@ -307,6 +307,7 @@ export function SubmitRun() { const selectedBaseProfileVersion = selectedProfileId ? profileVersions.find((version) => version.version === selectedProfileVersion) ?? selectedBaseProfile?.version : undefined; + const profileControlsResources = (selectedBaseProfileVersion?.resources?.length ?? 0) > 0; const topProfiles = selectableProfileList.slice(0, 3); // ─── Effects ──────────────────────────────────────────────────────────── @@ -1026,50 +1027,6 @@ export function SubmitRun() {
)} - {/* ─── Resources (optional, discreet) ───────────────────────────── */} - {!(resourcesOpen || selectedResourceSpecs.length > 0) ? ( - - ) : ( -
-
-
- - -
- -
- -
- )} -
); @@ -113,3 +116,21 @@ export const ProfileManaged: Story = { /> ), }; + +export const VariationModeLocked: Story = { + args: { + selected: profileBindings, + profileBindings, + bindingsLocked: true, + disabled: true, + onChange: () => {}, + }, + render: () => ( + + ), +}; diff --git a/apps/portal/src/pages/SubmitRun.tsx b/apps/portal/src/pages/SubmitRun.tsx index dd47e800f..a03eefbc9 100644 --- a/apps/portal/src/pages/SubmitRun.tsx +++ b/apps/portal/src/pages/SubmitRun.tsx @@ -105,6 +105,28 @@ function randomVariationColor(): string { return VARIATION_COLORS[Math.floor(Math.random() * VARIATION_COLORS.length)]; } +/** + * Compares ordered resource binding specs, including their parameter values. + * + * @param left Current run-level resource configuration. + * @param right Profile-owned resource configuration. + */ +function resourceBindingSpecsEqual( + left: ResourceBindingSpec[], + right: ResourceBindingSpec[], +): boolean { + if (left.length !== right.length) return false; + + return left.every((binding, index) => { + const other = right[index]; + if (!other || binding.ref !== other.ref) return false; + + const params = Object.entries(binding.params ?? {}).sort(([a], [b]) => a.localeCompare(b)); + const otherParams = Object.entries(other.params ?? {}).sort(([a], [b]) => a.localeCompare(b)); + return JSON.stringify(params) === JSON.stringify(otherParams); + }); +} + interface GalleryCardProps { icon: React.ComponentType<{ className?: string }>; title: string; @@ -137,11 +159,22 @@ interface CollapsibleCardProps { open: boolean; onOpenChange: (open: boolean) => void; disabled?: boolean; + disabledReason?: string; help?: React.ReactNode; children: React.ReactNode; } -function CollapsibleCard({ icon: Icon, title, summary, open, onOpenChange, disabled, help, children }: CollapsibleCardProps) { +function CollapsibleCard({ + icon: Icon, + title, + summary, + open, + onOpenChange, + disabled, + disabledReason, + help, + children, +}: CollapsibleCardProps) { return (
@@ -308,6 +341,9 @@ export function SubmitRun() { ? profileVersions.find((version) => version.version === selectedProfileVersion) ?? selectedBaseProfile?.version : undefined; const profileControlsResources = (selectedBaseProfileVersion?.resources?.length ?? 0) > 0; + const variationModeActive = Boolean( + selectedProfileId && variationDrafts.some((variation) => variation.profileId.trim().length > 0), + ); const topProfiles = selectableProfileList.slice(0, 3); // ─── Effects ──────────────────────────────────────────────────────────── @@ -457,19 +493,30 @@ export function SubmitRun() { }; const addVariationDraft = () => { - setVariationDrafts((prev) => { - const used = new Set(prev.map((v) => v.profileId).filter(Boolean)); - const nextProfile = selectableProfileList.find((p) => p._id !== selectedProfileId && !used.has(p._id)); - if (!nextProfile) { - toast.info("All available profiles are already used in variations."); - return prev; - } - return [...prev, { - profileId: nextProfile._id, - profileVersion: nextProfile.latestVersion, - color: randomVariationColor(), - }]; - }); + const used = new Set(variationDrafts.map((variation) => variation.profileId).filter(Boolean)); + const nextProfile = selectableProfileList.find( + (profile) => profile._id !== selectedProfileId && !used.has(profile._id), + ); + if (!nextProfile) { + toast.info("All available profiles are already used in variations."); + return; + } + + // Variation requests resolve each profile independently and omit request-level + // resources, so remove edits that the API would otherwise silently ignore. + const profileResourceSpecs = selectedBaseProfileVersion?.resources ?? []; + if (!resourceBindingSpecsEqual(selectedResourceSpecs, profileResourceSpecs)) { + toast.warning( + "Run-owned resource edits were removed. Profile variations resolve resources independently, so every selected profile must provide complete resource bindings and required parameters.", + ); + setSelectedResourceSpecs(profileResourceSpecs); + } + + setVariationDrafts((previous) => [...previous, { + profileId: nextProfile._id, + profileVersion: nextProfile.latestVersion, + color: randomVariationColor(), + }]); }; const updateVariationDraft = (index: number, patch: Partial) => { @@ -1833,10 +1880,19 @@ export function SubmitRun() { } open={resourcesOpen} onOpenChange={setResourcesOpen} - disabled={profileControlsResources} + disabled={variationModeActive || profileControlsResources} + disabledReason={variationModeActive ? "managed independently by each profile" : undefined} >
- {profileControlsResources && ( + {variationModeActive ? ( +
+ + + Profile variations resolve resources independently. Every selected profile must provide complete + resource bindings and required parameters; run-owned resource edits are unavailable in variation mode. + +
+ ) : profileControlsResources ? (
@@ -1844,12 +1900,13 @@ export function SubmitRun() { Parameters the profile leaves unset remain editable for this run.
- )} + ) : null}
diff --git a/apps/portal/src/pages/SubmitRun.variations.test.tsx b/apps/portal/src/pages/SubmitRun.variations.test.tsx index a2d2c7704..91dadb1c5 100644 --- a/apps/portal/src/pages/SubmitRun.variations.test.tsx +++ b/apps/portal/src/pages/SubmitRun.variations.test.tsx @@ -18,6 +18,15 @@ vi.mock("@/hooks/useVisibleGates", () => ({ useVisibleGates: () => ["select"], })); +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + info: vi.fn(), + success: vi.fn(), + warning: vi.fn(), + }, +})); + vi.mock("@/lib/api", () => ({ api: { getVersion: vi.fn(async () => ({ strictAgentCapabilities: true })), @@ -36,6 +45,7 @@ vi.mock("@/lib/api", () => ({ })); import { api } from "@/lib/api"; +import { toast } from "sonner"; import { SubmitRun } from "./SubmitRun"; const createdAt = "2026-01-01T00:00:00.000Z"; @@ -215,6 +225,35 @@ describe("SubmitRun profile variation payload", () => { })); }); + it("discards and disables run-owned resource edits when a variation is added", async () => { + const user = userEvent.setup(); + render( + + + + + , + ); + + // Fill a parameter gap that is valid for a single-profile run, then switch + // into comparison mode where request-level resources are not submitted. + const baseProfileInput = await screen.findByPlaceholderText("Search existing profiles…"); + fireEvent.change(baseProfileInput, { target: { value: "Baseline profile" } }); + await user.click(screen.getByRole("listitem")); + const scenarioParameter = await screen.findByLabelText("SCENARIO") as HTMLInputElement; + fireEvent.change(scenarioParameter, { target: { value: "custom-scenario" } }); + await user.click(screen.getByRole("button", { name: "Add" })); + + await waitFor(() => expect(toast.warning).toHaveBeenCalledWith( + "Run-owned resource edits were removed. Profile variations resolve resources independently, so every selected profile must provide complete resource bindings and required parameters.", + )); + expect(scenarioParameter.disabled).toBe(true); + expect(scenarioParameter.value).toBe("default-scenario"); + expect(screen.getByText( + "Profile variations resolve resources independently. Every selected profile must provide complete resource bindings and required parameters; run-owned resource edits are unavailable in variation mode.", + )).toBeTruthy(); + }); + it("does not copy baseline profile-managed configuration into the shared request", async () => { const user = userEvent.setup(); render( diff --git a/docs/architecture/resources.md b/docs/architecture/resources.md index 6e16bf137..e64effe1b 100644 --- a/docs/architecture/resources.md +++ b/docs/architecture/resources.md @@ -82,7 +82,11 @@ values: those controls render visibly locked and cannot be added, removed, repla overridden. Parameter gaps the profile leaves unset remain editable because the single-profile API contract permits the run to fill them. A profile that declares no resources leaves resource configuration run-owned and editable. Showing profile-owned values as editable would be a lie in -the UI because the API rejects the apparent override. +the UI because the API rejects the apparent override. Once a comparison variation is added, +request-level resources are no longer submitted: every selected profile must independently provide +complete resource bindings and required parameters. The wizard therefore removes any existing +run-owned resource edits with a visible warning and disables all resource edits while variation +mode remains active. ## Run observability