From 4f3f7c9d0b18d624e7644fa8d479307a693a815b Mon Sep 17 00:00:00 2001 From: Justin <159104807+Justin24506@users.noreply.github.com> Date: Tue, 3 Nov 2026 13:28:33 +0530 Subject: [PATCH] fix(web): keep typed unpublished variable rows across settings re-deliveries A row typed in an Add-variable card disappeared whenever an external settings write re-delivered the environment. reconcileRowsDraft reseeds rows on any non-echo change, and an unpublished row can never echo. ProviderEnvironmentSection now keeps rows the server never acknowledged (provider-env-* ids) across reseeds. Rows the server has acknowledged stay governed by the reseed, so a foreign delete removes them again. Model and harness: opencode/pulse-alpha-free (OpenCode harness, via T3 Code). Panel review: gemini-3.8-flash (high), glm-5.3-flash-free (max), deepseek-v4.1-flash-free (max). --- ...rInstanceCard.environment.browser.test.tsx | 121 ++++++++++++++++++ .../settings/ProviderInstanceCard.tsx | 17 ++- 2 files changed, 136 insertions(+), 2 deletions(-) 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) =>