diff --git a/apps/mobile/src/features/settings/SettingsRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsRouteScreen.tsx index 77036c212517..d57904b7e7a2 100644 --- a/apps/mobile/src/features/settings/SettingsRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsRouteScreen.tsx @@ -41,14 +41,8 @@ import { DEFAULT_SERVER_SETTINGS, MAX_SIDEBAR_AUTO_SETTLE_AFTER_DAYS, MIN_SIDEBAR_AUTO_SETTLE_AFTER_DAYS, - type ServerSettingsPatch, } from "@t3tools/contracts"; -import { - filterSharedServerPatch, - findSharedSettingsMismatches, - pickSharedServerSettings, - supportsSharedSettingsSync, -} from "@t3tools/client-runtime/state/shared-settings"; +import { supportsSharedSettingsSync } from "@t3tools/client-runtime/state/shared-settings"; import { useThreadListV2Enabled } from "../threads/use-thread-list-v2-enabled"; import { type AppUpdateCheckState, @@ -61,6 +55,7 @@ import { SettingsRow } from "./components/SettingsRow"; import { SettingsSection } from "./components/SettingsSection"; import { SettingsSwitchRow } from "./components/SettingsSwitchRow"; import { resolveAgentAwarenessPlatformPresentation } from "./SettingsRouteScreen.logic"; +import { planAutoSettleSettingsSync, type AutoSettleSettings } from "./autoSettleSettingsSync"; type NotificationStatus = "checking" | "enabled" | "disabled" | "unsupported"; type LiveActivityStatus = "checking" | "enabled" | "disabled" | "signed-out" | "linking"; @@ -588,10 +583,9 @@ function GeneralSettingsSection() { const AUTO_SETTLE_DEFAULT_DAYS = DEFAULT_SERVER_SETTINGS.sidebarAutoSettleAfterDays ?? 3; /** - * Auto-settlement is a user preference that every server has to hold. Mobile - * has no primary environment, so the first eligible sync target provides the - * reference value. Edits fan out to every eligible target, and a mismatch row - * lets the user push the reference out. + * Mobile edits auto-settle defaults across connected, capable environments. + * The first target supplies the displayed values. Applying them leaves each + * environment's other defaults and overrides intact. */ function AutoSettleSettingsRows() { const { environments } = useEnvironments(); @@ -610,24 +604,20 @@ function AutoSettleSettingsRows() { return null; } - const writeToAll = (patch: ServerSettingsPatch) => { + const writeToAll = (patch: Partial) => { for (const environment of syncTargets) { void updateSettings({ environmentId: environment.environmentId, input: { patch } }); } }; - const mismatches = findSharedSettingsMismatches({ - primaryEnvironmentId: reference.environmentId, - primarySettings: referenceSettings, - primaryCapabilities: reference.serverConfig?.environment.capabilities, - environments: environments.map((environment) => ({ + const { patch: autoSettlePatch, mismatches } = planAutoSettleSettingsSync( + { environmentId: reference.environmentId, settings: referenceSettings }, + syncTargets.map((environment) => ({ environmentId: environment.environmentId, label: environment.label, - syncEligible: supportsSharedSettingsSync(environment), settings: environment.serverConfig?.settings ?? null, - capabilities: environment.serverConfig?.environment.capabilities, })), - }); + ); const afterDays = referenceSettings.sidebarAutoSettleAfterDays; const commitDays = () => { @@ -681,7 +671,7 @@ function AutoSettleSettingsRows() { {mismatches.length > 0 ? ( - Settings differ + Auto-settle defaults differ {mismatches.map((mismatch) => mismatch.label).join(", ")} @@ -689,30 +679,18 @@ function AutoSettleSettingsRows() { { - const patch = pickSharedServerSettings( - referenceSettings, - reference.serverConfig?.environment.capabilities, - ); for (const mismatch of mismatches) { - const target = environments.find( - (candidate) => candidate.environmentId === mismatch.environmentId, - ); void updateSettings({ environmentId: mismatch.environmentId, - input: { - patch: filterSharedServerPatch( - patch, - target?.serverConfig?.environment.capabilities, - target?.serverConfig?.settings, - referenceSettings, - ), - }, + input: { patch: autoSettlePatch }, }); } }} className="rounded-full bg-subtle px-4 py-2 active:opacity-70" > - Apply to all + + Apply auto-settle defaults + ) : null} diff --git a/apps/mobile/src/features/settings/autoSettleSettingsSync.test.ts b/apps/mobile/src/features/settings/autoSettleSettingsSync.test.ts new file mode 100644 index 000000000000..ec550725adcf --- /dev/null +++ b/apps/mobile/src/features/settings/autoSettleSettingsSync.test.ts @@ -0,0 +1,78 @@ +import { DEFAULT_SERVER_SETTINGS, EnvironmentId } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { planAutoSettleSettingsSync } from "./autoSettleSettingsSync"; + +const reference = { + environmentId: EnvironmentId.make("reference"), + settings: { + ...DEFAULT_SERVER_SETTINGS, + sidebarAutoSettleAfterDays: 7, + sidebarAutoSettleOnMerge: true, + newWorktreesStartFromOrigin: false, + continueThreadsAfterServerUpdate: false, + }, +}; + +describe("auto-settle settings sync", () => { + it("ignores differences in independently configured environment settings", () => { + const target = { + environmentId: EnvironmentId.make("remote"), + label: "Remote", + settings: { + ...reference.settings, + newWorktreesStartFromOrigin: true, + continueThreadsAfterServerUpdate: true, + sourceControlWritingStyle: { + ...reference.settings.sourceControlWritingStyle, + customInstructions: "Keep this environment's writing instructions.", + }, + }, + }; + + const plan = planAutoSettleSettingsSync(reference, [target]); + + expect(plan.mismatches).toEqual([]); + expect(plan.patch).toEqual({ + sidebarAutoSettleAfterDays: 7, + sidebarAutoSettleOnMerge: true, + }); + }); + + it("applies only auto-settle defaults when another environment differs", () => { + const target = { + environmentId: EnvironmentId.make("remote"), + label: "Remote", + settings: { + ...reference.settings, + sidebarAutoSettleAfterDays: null, + sidebarAutoSettleOnMerge: false, + newWorktreesStartFromOrigin: true, + continueThreadsAfterServerUpdate: true, + sourceControlWritingStyle: { + ...reference.settings.sourceControlWritingStyle, + customInstructions: "Preserve these instructions.", + }, + }, + }; + + const plan = planAutoSettleSettingsSync(reference, [target]); + const updated = { ...target.settings, ...plan.patch }; + + expect(plan.mismatches).toEqual([target]); + expect(updated.sidebarAutoSettleAfterDays).toBe(7); + expect(updated.sidebarAutoSettleOnMerge).toBe(true); + expect(updated.newWorktreesStartFromOrigin).toBe(true); + expect(updated.continueThreadsAfterServerUpdate).toBe(true); + expect(updated.sourceControlWritingStyle).toEqual(target.settings.sourceControlWritingStyle); + }); + + it("does not compare the reference or a target without loaded settings", () => { + const plan = planAutoSettleSettingsSync(reference, [ + { ...reference, label: "Reference" }, + { environmentId: EnvironmentId.make("loading"), label: "Loading", settings: null }, + ]); + + expect(plan.mismatches).toEqual([]); + }); +}); diff --git a/apps/mobile/src/features/settings/autoSettleSettingsSync.ts b/apps/mobile/src/features/settings/autoSettleSettingsSync.ts new file mode 100644 index 000000000000..6addfa381fde --- /dev/null +++ b/apps/mobile/src/features/settings/autoSettleSettingsSync.ts @@ -0,0 +1,31 @@ +import type { EnvironmentId, ServerSettings } from "@t3tools/contracts"; + +export type AutoSettleSettings = Pick< + ServerSettings, + "sidebarAutoSettleAfterDays" | "sidebarAutoSettleOnMerge" +>; + +interface AutoSettleSyncTarget { + readonly environmentId: EnvironmentId; + readonly label: string; + readonly settings: AutoSettleSettings | null; +} + +/** Receives connected, capable targets. Applying these defaults must preserve other settings. */ +export function planAutoSettleSettingsSync( + reference: { readonly environmentId: EnvironmentId; readonly settings: AutoSettleSettings }, + targets: readonly AutoSettleSyncTarget[], +) { + const patch: AutoSettleSettings = { + sidebarAutoSettleAfterDays: reference.settings.sidebarAutoSettleAfterDays, + sidebarAutoSettleOnMerge: reference.settings.sidebarAutoSettleOnMerge, + }; + const mismatches = targets.filter( + (target) => + target.environmentId !== reference.environmentId && + target.settings !== null && + (target.settings.sidebarAutoSettleAfterDays !== patch.sidebarAutoSettleAfterDays || + target.settings.sidebarAutoSettleOnMerge !== patch.sidebarAutoSettleOnMerge), + ); + return { patch, mismatches }; +} diff --git a/apps/mobile/src/features/threads/new-task-flow-provider.tsx b/apps/mobile/src/features/threads/new-task-flow-provider.tsx index 535581b58e75..d0687e7e7b66 100644 --- a/apps/mobile/src/features/threads/new-task-flow-provider.tsx +++ b/apps/mobile/src/features/threads/new-task-flow-provider.tsx @@ -13,10 +13,12 @@ import { CommandId, DEFAULT_PROVIDER_INTERACTION_MODE, DEFAULT_RUNTIME_MODE, + DEFAULT_SERVER_SETTINGS, MessageId, T3_PROJECT_FILE_NAME, ThreadId, } from "@t3tools/contracts"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import { parseT3ProjectFile } from "@t3tools/shared/t3ProjectFile"; import { isDefaultThreadEnvModeSettled, @@ -428,17 +430,32 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { if (t3ProjectFileData === null || t3ProjectFileData.truncated) return null; return parseT3ProjectFile(t3ProjectFileData.contents)?.defaultThreadEnvMode ?? null; }, [t3ProjectFileData]); + // Environment settings with the project's overrides applied; the + // aggregate's own legacy fields still count until the server folds them. + const projectSettings = useMemo( + () => + resolveProjectSettings( + selectedEnvironmentServerConfig?.settings ?? DEFAULT_SERVER_SETTINGS, + selectedProject?.id ?? null, + selectedProject, + ), + [selectedEnvironmentServerConfig?.settings, selectedProject], + ); + const projectThreadEnvMode = + projectSettings.sources.defaultThreadEnvMode === "project" + ? projectSettings.settings.defaultThreadEnvMode + : undefined; const defaultWorkspaceMode: WorkspaceMode = resolveDefaultThreadEnvMode({ - projectSetting: selectedProject?.defaultThreadEnvMode, + projectSetting: projectThreadEnvMode, projectFile: t3ProjectFileDefaultMode, - globalDefault: selectedEnvironmentServerConfig?.settings.defaultThreadEnvMode ?? "local", + globalDefault: projectSettings.settings.defaultThreadEnvMode, }); // While unsettled the resolved default is provisional. Nothing may write // it into the draft during that window (the auto-branch effect does), or // the frozen interim value beats the t3.json default once it loads. const defaultWorkspaceModeSettled = isDefaultThreadEnvModeSettled({ explicitMode: selectedProjectDraft.workspaceSelection?.mode, - projectSetting: selectedProject?.defaultThreadEnvMode, + projectSetting: projectThreadEnvMode, projectFilePending: t3ProjectFileQuery.isPending, }); const workspaceMode = selectedProjectDraft.workspaceSelection?.mode ?? defaultWorkspaceMode; @@ -449,9 +466,7 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { // value keeps tracking the server setting when the config loads late. const draftStartFromOrigin = selectedProjectDraft.workspaceSelection?.startFromOrigin; const startFromOrigin = - draftStartFromOrigin ?? - selectedEnvironmentServerConfig?.settings.newWorktreesStartFromOrigin ?? - true; + draftStartFromOrigin ?? projectSettings.settings.newWorktreesStartFromOrigin; const runtimeMode = selectedProjectDraft.runtimeMode ?? DEFAULT_RUNTIME_MODE; // Antigravity keeps unavailable selections so sign-out or a catalog change @@ -463,9 +478,7 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { ); const projectDefaultModelSelection = resolveDefaultableModelSelection( selectedEnvironmentServerConfig, - selectedProject?.defaultModelSelection ?? - selectedEnvironmentServerConfig?.settings.defaultModelSelection ?? - null, + projectSettings.settings.defaultModelSelection, ); const storedStickyModelSelection = useStickyComposerModelSelection(); const stickyModelSelection = resolveDefaultableModelSelection( diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 51b9c5eabc48..fc529b02a73b 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -68,6 +68,7 @@ import { projectScriptRuntimeEnv, resolveProjectScripts, } from "@t3tools/shared/projectScripts"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import { truncate } from "@t3tools/shared/String"; import { resolveThreadReferenceCopyTarget } from "@t3tools/shared/threadReference"; import { @@ -304,7 +305,6 @@ import { environmentServerConfigsAtom, primaryServerAvailableEditorsAtom, primaryServerKeybindingsAtom, - primaryServerSettingsAtom, serverEnvironment, } from "../state/server"; import { terminalEnvironment } from "../state/terminal"; @@ -1516,7 +1516,6 @@ export default function ChatView(props: ChatViewProps) { }, [routeKind, routeThreadRef, routeThreadState]); const markThreadVisited = useUiStateStore((store) => store.markThreadVisited); const settings = useEnvironmentSettings(environmentId); - const primaryServerSettings = useAtomValue(primaryServerSettingsAtom); const setStickyComposerModelSelection = useComposerDraftStore( (store) => store.setStickyModelSelection, ); @@ -1802,17 +1801,14 @@ export default function ChatView(props: ChatViewProps) { ? buildLocalDraftThread( threadId, draftThread, - fallbackDraftProject?.defaultModelSelection ?? - settings.defaultModelSelection ?? - NO_PROVIDER_MODEL_SELECTION, + resolveProjectSettings( + settings, + fallbackDraftProject?.id ?? null, + fallbackDraftProject ?? undefined, + ).settings.defaultModelSelection ?? NO_PROVIDER_MODEL_SELECTION, ) : undefined, - [ - draftThread, - fallbackDraftProject?.defaultModelSelection, - settings.defaultModelSelection, - threadId, - ], + [draftThread, fallbackDraftProject, settings, threadId], ); // Promotion is data-driven: the draft route keeps rendering while the // server thread (same pre-allocated ref) starts, so live state must not @@ -2033,12 +2029,16 @@ export default function ChatView(props: ChatViewProps) { [activeThread?.environmentId, activeThread?.projectId], ); const activeProject = useProject(activeProjectRef); + // Environment settings with the active project's overrides applied. + const activeProjectSettings = useMemo( + () => resolveProjectSettings(settings, activeProject?.id ?? null, activeProject ?? undefined), + [activeProject, settings], + ); const activeProjectScripts = useMemo( () => (activeProject ? resolveProjectScripts(settings, activeProject) : []), [activeProject, settings], ); - const activeProjectDefaultModelSelection = - activeProject?.defaultModelSelection ?? settings.defaultModelSelection; + const activeProjectDefaultModelSelection = activeProjectSettings.settings.defaultModelSelection; const handleNewThreadInActiveProject = useCallback(() => { startNewThreadForProject(activeProjectRef, handleNewThread); }, [activeProjectRef, handleNewThread]); @@ -3945,6 +3945,9 @@ export default function ChatView(props: ChatViewProps) { ], ); + const supportsProjectSettingsOverrides = + environmentById.get(environmentId)?.serverConfig?.environment.capabilities + .projectSettingsOverrides === true; const persistProjectScripts = useCallback( async (input: { projectId: ProjectId; @@ -3958,11 +3961,22 @@ export default function ChatView(props: ChatViewProps) { await updateProjectScriptSettings({ environmentId, input: { - patch: { - projectScriptOverrides: { - [input.projectId]: input.nextScripts, - }, - }, + // The canonical key on servers that understand it; the legacy + // per-project map is still translated on older ones. + patch: supportsProjectSettingsOverrides + ? { + projectSettingsOverrides: { + [input.projectId]: { + ...settings.projectSettingsOverrides[input.projectId], + defaultProjectScripts: input.nextScripts, + }, + }, + } + : { + projectScriptOverrides: { + [input.projectId]: input.nextScripts, + }, + }, }, }), () => undefined, @@ -3987,7 +4001,13 @@ export default function ChatView(props: ChatViewProps) { } return updateResult; }, - [environmentId, updateProjectScriptSettings, upsertKeybinding], + [ + environmentId, + settings.projectSettingsOverrides, + supportsProjectSettingsOverrides, + updateProjectScriptSettings, + upsertKeybinding, + ], ); const saveProjectScript = useCallback( async (input: NewProjectScriptInput): Promise> => { @@ -5396,7 +5416,7 @@ export default function ChatView(props: ChatViewProps) { ? (draftThread?.startFromOrigin ?? false) : canOverrideServerThreadEnvMode ? (pendingServerThreadStartFromOriginByThreadId[activeThread?.id ?? ""] ?? - primaryServerSettings.newWorktreesStartFromOrigin) + activeProjectSettings.settings.newWorktreesStartFromOrigin) : false; const sendEnvMode = resolveSendEnvMode({ requestedEnvMode: envMode, @@ -8045,7 +8065,7 @@ export default function ChatView(props: ChatViewProps) { envMode: mode, startFromOrigin: resolveNewDraftStartFromOrigin({ envMode: mode, - newWorktreesStartFromOrigin: primaryServerSettings.newWorktreesStartFromOrigin, + newWorktreesStartFromOrigin: activeProjectSettings.settings.newWorktreesStartFromOrigin, }), ...(mode === "worktree" && draftThread?.worktreePath ? { worktreePath: null } : {}), }); @@ -8057,7 +8077,7 @@ export default function ChatView(props: ChatViewProps) { composerDraftTarget, draftThread?.worktreePath, isLocalDraftThread, - primaryServerSettings.newWorktreesStartFromOrigin, + activeProjectSettings.settings.newWorktreesStartFromOrigin, setPendingServerThreadEnvMode, scheduleComposerFocus, setDraftThreadContext, diff --git a/apps/web/src/components/CommandPalette.tsx b/apps/web/src/components/CommandPalette.tsx index 4a9877f87a02..3533678ecb2d 100644 --- a/apps/web/src/components/CommandPalette.tsx +++ b/apps/web/src/components/CommandPalette.tsx @@ -1814,8 +1814,7 @@ function OpenCommandPaletteDialog(props: { }, }); - // There is no projects listing page; the action targets the contextual - // project (active thread/draft, falling back to the first sidebar group). + // Target the active thread or draft's project, falling back to the first sidebar group. const contextualProjectGroup = (contextualProjectRef ? projectGroupByTargetKey.get( @@ -1867,8 +1866,6 @@ function OpenCommandPaletteDialog(props: { run: async () => { await navigate({ to: item.to, - search: (previous) => - item.to === "/settings/projects" ? { ...previous, project: undefined } : previous, hash: item.targetId ?? item.id, replace: pathname === item.to, hashScrollIntoView: false, diff --git a/apps/web/src/components/chat/DraftHeroHeadline.tsx b/apps/web/src/components/chat/DraftHeroHeadline.tsx index 4a9421f2011f..1e0d295c098f 100644 --- a/apps/web/src/components/chat/DraftHeroHeadline.tsx +++ b/apps/web/src/components/chat/DraftHeroHeadline.tsx @@ -27,6 +27,7 @@ import { MenuTrigger, } from "../ui/menu"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; interface DraftHeroHeadlineProps { readonly draftId: DraftId | null; @@ -150,11 +151,13 @@ export function DraftHeroHeadline({ ); if (!hasExplicitComposerModelSelection(currentDraft)) { applyStickyState(draftId); - const defaultModelSelection = - project.defaultModelSelection ?? - environments.find( - (environment) => environment.environmentId === project.environmentId, - )?.serverConfig?.settings.defaultModelSelection; + const environmentSettings = environments.find( + (environment) => environment.environmentId === project.environmentId, + )?.serverConfig?.settings; + const defaultModelSelection = environmentSettings + ? resolveProjectSettings(environmentSettings, project.id, project).settings + .defaultModelSelection + : project.defaultModelSelection; if (defaultModelSelection) { setModelSelection(draftId, defaultModelSelection, { replaceOptions: true, diff --git a/apps/web/src/components/chat/ProviderModelPicker.test.tsx b/apps/web/src/components/chat/ProviderModelPicker.test.tsx index b1bb8ba9c74a..41e86f841ee4 100644 --- a/apps/web/src/components/chat/ProviderModelPicker.test.tsx +++ b/apps/web/src/components/chat/ProviderModelPicker.test.tsx @@ -34,6 +34,7 @@ function renderPicker(input: { model: string; options: ReadonlyArray; includeEntry?: boolean; + triggerLabel?: string; }) { const instanceId = ProviderInstanceId.make(input.instanceId); const entry = providerEntry(input.instanceId, input.driver); @@ -45,11 +46,25 @@ function renderPicker(input: { instanceEntries={input.includeEntry === false ? [] : [entry]} modelOptionsByInstance={new Map([[instanceId, input.options]])} onInstanceModelChange={() => {}} + {...(input.triggerLabel ? { triggerLabel: input.triggerLabel } : {})} />, ); } describe("ProviderModelPicker", () => { + it("shows a neutral aggregate value without a representative model or availability badge", () => { + const markup = renderPicker({ + instanceId: "codex_personal", + driver: "codex", + model: "gpt-5", + options: [{ slug: "gpt-5", name: "GPT 5", isUnavailable: true }], + triggerLabel: "Mixed values", + }); + expect(markup).toContain("Mixed values"); + expect(markup).not.toContain("GPT 5"); + expect(markup).not.toContain("Unavailable"); + }); + it.each(["", ANTIGRAVITY_DEFAULT_MODEL])( "shows a choice prompt before Antigravity has an account catalog for %s", (model) => { diff --git a/apps/web/src/components/chat/ProviderModelPicker.tsx b/apps/web/src/components/chat/ProviderModelPicker.tsx index fb2cdb2f1280..6f777399451d 100644 --- a/apps/web/src/components/chat/ProviderModelPicker.tsx +++ b/apps/web/src/components/chat/ProviderModelPicker.tsx @@ -48,6 +48,8 @@ export const ProviderModelPicker = memo(function ProviderModelPicker(props: { open?: boolean; triggerVariant?: VariantProps["variant"]; triggerClassName?: string; + /** Aggregate settings can show a neutral value without claiming one provider is selected. */ + triggerLabel?: string; triggerAriaLabel?: string; onOpenChange?: (open: boolean) => void; onOpenProviderSetup?: (instanceId: ProviderInstanceId) => void; @@ -181,7 +183,7 @@ export const ProviderModelPicker = memo(function ProviderModelPicker(props: { - {activeEntry ? ( + {activeEntry && props.triggerLabel === undefined ? ( } > - {triggerTitle} + {props.triggerLabel ?? triggerTitle} - {triggerLabel} + {props.triggerLabel ?? triggerLabel} - {selectedModel?.isUnavailable ? ( + {selectedModel?.isUnavailable && props.triggerLabel === undefined ? ( Unavailable diff --git a/apps/web/src/components/projectScriptEditor.test.tsx b/apps/web/src/components/projectScriptEditor.test.tsx new file mode 100644 index 000000000000..becb3a369f62 --- /dev/null +++ b/apps/web/src/components/projectScriptEditor.test.tsx @@ -0,0 +1,245 @@ +import * as Cause from "effect/Cause"; +import { AsyncResult } from "effect/unstable/reactivity"; +import { act, StrictMode, type ReactNode } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +vi.mock("./ui/dialog", () => ({ + Dialog: ({ open, children }: { open: boolean; children: ReactNode }) => (open ? children : null), + DialogDescription: "p", + DialogFooter: "footer", + DialogHeader: "header", + DialogPanel: "section", + DialogPopup: "section", + DialogTitle: "h2", +})); +vi.mock("./ui/alert-dialog", () => ({ + AlertDialog: ({ open, children }: { open: boolean; children: ReactNode }) => + open ? children : null, + AlertDialogClose: "button", + AlertDialogDescription: "p", + AlertDialogFooter: "footer", + AlertDialogHeader: "header", + AlertDialogPopup: "section", + AlertDialogTitle: "h2", +})); +vi.mock("./ui/button", () => ({ Button: "button" })); +vi.mock("./ui/input", () => ({ Input: "input" })); +vi.mock("./ui/label", () => ({ Label: "label" })); +vi.mock("./ui/popover", () => ({ + Popover: ({ children }: { children: ReactNode }) => children, + PopoverPopup: () => null, + PopoverTrigger: "button", +})); +vi.mock("./ui/switch", () => ({ Switch: "input" })); +vi.mock("./ui/textarea", () => ({ Textarea: "textarea" })); + +import { + EMPTY_PROJECT_SCRIPT_INPUT, + ProjectScriptEditorDialog, + type ProjectScriptActionResult, + type ProjectScriptEditorRequest, +} from "./projectScriptEditor"; + +const onSubmit = vi.fn[0]["onSubmit"]>(); +const onClose = vi.fn(); +const onDelete = vi.fn(); +let renderer: ReactTestRenderer | null; + +function request(name: string, error?: string): ProjectScriptEditorRequest { + return { + scriptId: name, + initial: { ...EMPTY_PROJECT_SCRIPT_INPUT, name, command: `run-${name}` }, + ...(error === undefined ? {} : { error }), + }; +} + +function editor(nextRequest: ProjectScriptEditorRequest) { + return ( + + + + ); +} + +function open(nextRequest: ProjectScriptEditorRequest) { + act(() => { + if (renderer) renderer.update(editor(nextRequest)); + else renderer = create(editor(nextRequest)); + }); +} + +function submit(): Promise { + return renderer!.root.findByType("form").props.onSubmit({ preventDefault() {} }); +} + +function saveButton() { + return renderer!.root.findAllByType("button").find((button) => button.props.type === "submit")!; +} + +function deferredSave() { + let resolve!: (result: ProjectScriptActionResult) => void; + let reject!: (error: Error) => void; + const promise = new Promise((resolveResult, rejectResult) => { + resolve = resolveResult; + reject = rejectResult; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => { + renderer = null; + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + onSubmit.mockReset(); + onClose.mockReset(); + onDelete.mockReset(); +}); + +afterEach(async () => { + await act(async () => renderer?.unmount()); + vi.unstubAllGlobals(); +}); + +describe("project action editor save lifecycle", () => { + it("blocks repeated submits and edits until the current save completes", async () => { + const save = deferredSave(); + onSubmit.mockReturnValue(save.promise); + open(request("build")); + + let completion!: Promise; + act(() => { + completion = submit(); + void submit(); + }); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(saveButton().props.disabled).toBe(true); + expect(renderer!.root.findByType("fieldset").props.disabled).toBe(true); + const cancel = renderer!.root + .findAllByType("button") + .find((button) => button.children.includes("Cancel"))!; + expect(cancel.props.disabled).not.toBe(true); + + await act(async () => { + save.resolve(AsyncResult.success(undefined)); + await completion; + }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("does not close a replacement request or release its in-flight save", async () => { + const first = deferredSave(); + const second = deferredSave(); + onSubmit.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + open(request("build")); + let firstCompletion!: Promise; + act(() => { + firstCompletion = submit(); + }); + + open(request("test")); + expect(saveButton().props.disabled).toBe(false); + expect(renderer!.root.findByProps({ id: "script-name" }).props.value).toBe("test"); + let secondCompletion!: Promise; + act(() => { + secondCompletion = submit(); + }); + + await act(async () => { + first.resolve(AsyncResult.success(undefined)); + await firstCompletion; + }); + expect(onClose).not.toHaveBeenCalled(); + expect(saveButton().props.disabled).toBe(true); + + await act(async () => { + second.resolve(AsyncResult.success(undefined)); + await secondCompletion; + }); + expect(onClose).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls.map(([scriptId]) => scriptId)).toEqual(["build", "test"]); + }); + + it.each(["failure", "rejection"] as const)( + "ignores a stale %s after the request changes", + async (outcome) => { + const save = deferredSave(); + onSubmit.mockReturnValue(save.promise); + open(request("build")); + let completion!: Promise; + act(() => { + completion = submit(); + }); + + open(request("test", "New request error")); + await act(async () => { + if (outcome === "failure") + save.resolve(AsyncResult.failure(Cause.fail(new Error("Old save error")))); + else save.reject(new Error("Old save error")); + await completion; + }); + + const messages = renderer!.root.findAllByType("p").flatMap((paragraph) => paragraph.children); + expect(messages).toContain("New request error"); + expect(messages).not.toContain("Old save error"); + expect(saveButton().props.disabled).toBe(false); + expect(onClose).not.toHaveBeenCalled(); + }, + ); + + it("shows a current save error and allows retry", async () => { + onSubmit.mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("Save failed")))); + onSubmit.mockResolvedValueOnce(AsyncResult.success(undefined)); + open(request("build")); + + await act(async () => { + await submit(); + }); + expect(renderer!.root.findAllByType("p").flatMap((paragraph) => paragraph.children)).toContain( + "Save failed", + ); + expect(saveButton().props.disabled).toBe(false); + expect(renderer!.root.findByType("fieldset").props.disabled).toBe(false); + expect(onClose).not.toHaveBeenCalled(); + + await act(async () => { + await submit(); + }); + expect(onSubmit).toHaveBeenCalledTimes(2); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it.each(["cancel", "unmount"] as const)("ignores save completion after %s", async (exit) => { + const save = deferredSave(); + onSubmit.mockReturnValue(save.promise); + open(request("build")); + let completion!: Promise; + act(() => { + completion = submit(); + }); + + act(() => { + if (exit === "cancel") { + renderer!.root + .findAllByType("button") + .find((button) => button.children.includes("Cancel"))! + .props.onClick(); + } else { + renderer!.unmount(); + renderer = null; + } + }); + onClose.mockClear(); + await act(async () => { + save.resolve(AsyncResult.success(undefined)); + await completion; + }); + expect(onClose).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/components/projectScriptEditor.tsx b/apps/web/src/components/projectScriptEditor.tsx index 4ffd453955e9..74b189a02fc5 100644 --- a/apps/web/src/components/projectScriptEditor.tsx +++ b/apps/web/src/components/projectScriptEditor.tsx @@ -16,7 +16,14 @@ import { PlayIcon, WrenchIcon, } from "lucide-react"; -import React, { type FormEvent, type KeyboardEvent, useEffect, useState } from "react"; +import React, { + type FormEvent, + type KeyboardEvent, + useEffect, + useLayoutEffect, + useRef, + useState, +} from "react"; import { keybindingValueForCommand, @@ -156,9 +163,22 @@ export function ProjectScriptEditorDialog({ const [autoOpenPreview, setAutoOpenPreview] = useState(false); const [validationError, setValidationError] = useState(null); const [deleteConfirmOpen, setDeleteConfirmOpen] = useState(false); + const [savingRequest, setSavingRequest] = useState(null); + const pendingSubmissionRef = useRef<{ request: ProjectScriptEditorRequest } | null>(null); const isOpen = request !== null; const isEditing = request?.scriptId != null; + const isSaving = request !== null && savingRequest === request; + + // A save completion must not affect a replacement request or an unmounted editor. + useLayoutEffect( + () => () => { + if (pendingSubmissionRef.current?.request === request) { + pendingSubmissionRef.current = null; + } + }, + [request], + ); // Hydrate the form whenever a new request opens the dialog. useEffect(() => { @@ -172,8 +192,16 @@ export function ProjectScriptEditorDialog({ setPreviewUrl(request.initial.previewUrl ?? ""); setAutoOpenPreview(request.initial.autoOpenPreview); setValidationError(request.error ?? null); + setSavingRequest(null); }, [request]); + const close = () => { + pendingSubmissionRef.current = null; + setSavingRequest(null); + setIconPickerOpen(false); + onClose(); + }; + const captureKeybinding = (event: KeyboardEvent) => { if (event.key === "Tab") return; event.preventDefault(); @@ -188,7 +216,7 @@ export function ProjectScriptEditorDialog({ const submit = async (event: FormEvent) => { event.preventDefault(); - if (!request) return; + if (!request || pendingSubmissionRef.current !== null) return; const trimmedName = name.trim(); const trimmedCommand = command.trim(); if (trimmedName.length === 0) { @@ -228,16 +256,31 @@ export function ProjectScriptEditorDialog({ return; } - const result = await onSubmit(request.scriptId, payload); - if (result._tag === "Failure") { - if (!isAtomCommandInterrupted(result)) { - const error = squashAtomCommandFailure(result); + const submission = { request }; + pendingSubmissionRef.current = submission; + setSavingRequest(request); + setIconPickerOpen(false); + try { + const result = await onSubmit(request.scriptId, payload); + if (pendingSubmissionRef.current === submission) { + if (result._tag === "Failure") { + if (!isAtomCommandInterrupted(result)) { + const error = squashAtomCommandFailure(result); + setValidationError(error instanceof Error ? error.message : "Failed to save action."); + } + } else { + close(); + } + } + } catch (error) { + if (pendingSubmissionRef.current === submission) { setValidationError(error instanceof Error ? error.message : "Failed to save action."); } - return; } - setIconPickerOpen(false); - onClose(); + if (pendingSubmissionRef.current === submission) { + pendingSubmissionRef.current = null; + setSavingRequest(null); + } }; return ( @@ -246,8 +289,7 @@ export function ProjectScriptEditorDialog({ open={isOpen} onOpenChange={(open) => { if (!open) { - setIconPickerOpen(false); - onClose(); + close(); } }} > @@ -259,112 +301,115 @@ export function ProjectScriptEditorDialog({ -
-
- -
- - - } - > - - - -
- {SCRIPT_ICONS.map((entry) => { - const isSelected = entry.id === icon; - return ( - - ); - })} -
- - + +
+
+ +
+ + + } + > + + + +
+ {SCRIPT_ICONS.map((entry) => { + const isSelected = entry.id === icon; + return ( + + ); + })} +
+ + + setName(event.target.value)} + /> +
+
+
+ setName(event.target.value)} + id="script-keybinding" + placeholder="Press shortcut" + value={keybinding} + readOnly + onKeyDown={captureKeybinding} /> +

+ Press a shortcut. Use Backspace to clear. Shortcuts are + environment-wide. Projects using the same action share its shortcut. +

-
-
- - -

- Press a shortcut. Use Backspace to clear. -

-
-
- -