diff --git a/apps/web/src/components/settings/ProviderInstanceCard.environment.browser.test.tsx b/apps/web/src/components/settings/ProviderInstanceCard.environment.browser.test.tsx index 1c9820b5addb5..dceb8c23c4722 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.environment.browser.test.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.environment.browser.test.tsx @@ -53,3 +53,124 @@ describe("ProviderEnvironmentSection saved secrets", () => { expect(onChange).toHaveBeenLastCalledWith([]); }); }); + +describe("ProviderEnvironmentSection external settings writes", () => { + it("keeps a typed unpublished row when an external write redelivers the environment", async () => { + const onChange = vi.fn(); + await using screen = await render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Add variable" })); + await userEvent.type( + screen.getByRole("textbox", { name: "Environment variable name 1" }), + "KEEP_ME_TYPED", + ); + expect(onChange).not.toHaveBeenCalled(); + + // A settings write from elsewhere on the instance redelivers a redacted, stored variable. + await screen.rerender( + , + ); + + await expect + .element(screen.getByRole("textbox", { name: "Environment variable name 2" })) + .toHaveValue("KEEP_ME_TYPED"); + }); + + it("does not duplicate a redacted published row on the sibling reseed", async () => { + const onChange = vi.fn(); + await using screen = await render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Add variable" })); + await userEvent.fill( + screen.getByRole("textbox", { name: "Environment variable name 1" }), + "STORE_ME", + ); + await userEvent.fill(screen.getByLabelText("Environment variable value 1"), "fake-secret"); + await userEvent.tab(); + expect(onChange).toHaveBeenLastCalledWith([ + { name: "STORE_ME", value: "fake-secret", sensitive: true, valueRedacted: false }, + ]); + + await userEvent.click(screen.getByRole("button", { name: "Add variable" })); + await userEvent.type( + screen.getByRole("textbox", { name: "Environment variable name 2" }), + "UNPUBLISHED_ONE", + ); + + // The stored secret comes back redacted, so the rows reseed without matching the echo. + await screen.rerender( + , + ); + + await expect + .element(screen.getByRole("textbox", { name: "Environment variable name 1" })) + .toHaveValue("STORE_ME"); + await expect + .element(screen.getByRole("textbox", { name: "Environment variable name 2" })) + .toHaveValue("UNPUBLISHED_ONE"); + }); + + it("keeps a committed but unpublishable row across an external write", async () => { + const onChange = vi.fn(); + await using screen = await render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Add variable" })); + await userEvent.fill( + screen.getByRole("textbox", { name: "Environment variable name 1" }), + "KEEP-ME", + ); + await userEvent.tab(); + expect(onChange).not.toHaveBeenCalled(); + + await screen.rerender( + , + ); + await expect + .element(screen.getByRole("textbox", { name: "Environment variable name 2" })) + .toHaveValue("KEEP-ME"); + }); + + it("drops an acknowledged row when an external write removes it", async () => { + const onChange = vi.fn(); + await using screen = await render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Add variable" })); + await userEvent.fill( + screen.getByRole("textbox", { name: "Environment variable name 1" }), + "PUBLISH_ME", + ); + await userEvent.tab(); + expect(onChange).toHaveBeenLastCalledWith([{ name: "PUBLISH_ME", value: "", sensitive: true }]); + + // The server echo acknowledges the publish and keeps the rows; nothing is republished. + await screen.rerender( + , + ); + expect(onChange).toHaveBeenCalledTimes(1); + const acknowledged = screen.getByRole("textbox", { name: "Environment variable name 1" }); + + // Another client deletes the variable. The row must not come back. + await screen.rerender(); + await expect.element(acknowledged).not.toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index b676e64399d97..f0040802a2e5e 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -97,7 +97,9 @@ function ProviderStatusDiagnostic({ } let environmentVariableDraftId = 0; -const nextEnvironmentVariableDraftId = () => `provider-env-${environmentVariableDraftId++}`; +const DRAFT_ROW_ID_PREFIX = "provider-env-"; +const nextEnvironmentVariableDraftId = () => + `${DRAFT_ROW_ID_PREFIX}${environmentVariableDraftId++}`; type EnvironmentDraftRow = { readonly id: string; @@ -346,12 +348,23 @@ export function ProviderEnvironmentSection(props: { const [storedDraft, setDraft] = useState(() => seedRowsDraft(props.environment, toEnvironmentDraftRows), ); - const draft = reconcileRowsDraft( + let draft = reconcileRowsDraft( storedDraft, props.environment, providerEnvironmentsEqual, toEnvironmentDraftRows, ); + if (draft.rows !== storedDraft.rows) { + // Keep rows the server never acknowledged, even across an external reseed. + const acknowledged = new Set(storedDraft.seen.map((variable) => variable.name)); + const unpublished = storedDraft.rows.filter( + (row) => + row.id.startsWith(DRAFT_ROW_ID_PREFIX) && + !acknowledged.has(row.name) && + !draft.rows.some((incoming) => incoming.name === row.name), + ); + if (unpublished.length > 0) draft = { ...draft, rows: [...draft.rows, ...unpublished] }; + } if (draft !== storedDraft) setDraft(draft); const rows = draft.rows; const setRows = (nextRows: ReadonlyArray) =>