From 6efa7812e3f6a0c2e5e8a169b7408f9776db8909 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 2 Nov 2026 22:21:32 -0800 Subject: [PATCH] fix(release): report short workbench timeouts, gate Sim links in Chat settings, scope the Chat plan modal - A workbench run with a short budget kept no time for its own timeout to answer, so the call deadline won and the output it printed was lost; hold back a small report reserve and bound the output listing by the time left - Chat listed the Sim links settings section to every organization; a settings adapter can now declare whether its section exists, and Sim links does so from its rollout flag - The Chat plan modal stays with the workspace whose send opened it - Organization conversations ask to upgrade the organization's plan --- .../chat-plan-modal/chat-plan-modal.tsx | 4 +- .../home/components/user-input/user-input.tsx | 4 +- .../[workspaceId]/home/hooks/use-chat.ts | 4 +- .../home/hooks/use-plan-upgrade-link.ts | 4 +- .../sim/lib/execution/remote-sandbox/index.ts | 30 +++++++++++---- .../remote-sandbox/session-sandbox.test.ts | 37 +++++++++++++++++++ .../mothership/tools/server/settings.test.ts | 23 ++++++++++++ .../lib/mothership/tools/server/settings.ts | 9 ++++- apps/sim/stores/modals/chat-plan/store.ts | 17 +++++---- 9 files changed, 110 insertions(+), 22 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/chat-plan-modal/chat-plan-modal.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/chat-plan-modal/chat-plan-modal.tsx index d39202f73f5..03caacc30e4 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/chat-plan-modal/chat-plan-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/chat-plan-modal/chat-plan-modal.tsx @@ -11,6 +11,7 @@ import { useRouter } from 'next/navigation' import { CHAT_PLAN_REQUIRED_UPGRADE } from '@/lib/billing/chat-plan' import { rememberSettingsReturnUrl } from '@/lib/navigation/settings-return' import { usePlanUpgradeLink } from '@/app/workspace/[workspaceId]/home/hooks/use-plan-upgrade-link' +import { useOptionalWorkspaceHostContext } from '@/app/workspace/[workspaceId]/providers/workspace-host-provider' import { useChatPlanModalStore } from '@/stores/modals/chat-plan/store' /** @@ -18,7 +19,8 @@ import { useChatPlanModalStore } from '@/stores/modals/chat-plan/store' * for a viewer who may, and who to ask for one who may not. Render it once per workspace shell. */ export function ChatPlanModal() { - const isOpen = useChatPlanModalStore((state) => state.isOpen) + const workspaceId = useOptionalWorkspaceHostContext()?.workspace.id + const isOpen = useChatPlanModalStore((state) => state.workspaceId === workspaceId) const close = useChatPlanModalStore((state) => state.close) return ( diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx index f8ef99060c6..9904c0df759 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx @@ -533,7 +533,7 @@ const UserInputImpl = forwardRef(function UserI const handleSubmit = useCallback(() => { if (chatPlanRequired) { - useChatPlanModalStore.getState().open() + useChatPlanModalStore.getState().open(workspaceId) return } const currentFiles = filesRef.current @@ -561,7 +561,7 @@ const UserInputImpl = forwardRef(function UserI ) if (accepted === false) return clearComposer() - }, [chatPlanRequired, onSubmit, clearComposer]) + }, [chatPlanRequired, workspaceId, onSubmit, clearComposer]) /** * Enter policy for the editor: mirror canSubmit's uploading guard (Enter diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index 62e55d0fc55..378fec8164f 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -4443,7 +4443,7 @@ export function useChat( ) => { if ((!message.trim() && !fileAttachments?.length) || !scopeKey) return if (chatPlanRequired) { - useChatPlanModalStore.getState().open() + useChatPlanModalStore.getState().open(workspaceId) return } @@ -5307,7 +5307,7 @@ export function useChat( if (!msg || queueState.editing[chatKey] === msg.id) return if (queuedMessageDispatchIds.has(msg.id)) return if (chatPlanRequired) { - useChatPlanModalStore.getState().open() + useChatPlanModalStore.getState().open(workspaceId) return } /* Sent by hand, it still must not run a second turn: if the history shows it diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-plan-upgrade-link.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-plan-upgrade-link.ts index 8bedaaa81a4..a5782551ce7 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-plan-upgrade-link.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-plan-upgrade-link.ts @@ -61,7 +61,9 @@ export function usePlanUpgradeLink(data: PlanUpgradeLinkInput): PlanUpgradeLink ? organizationContext.viewer.isAdmin : Boolean(hostContext && canManageWorkspaceBilling(hostContext, session?.user?.id))) const action = planRequired - ? 'upgrade this workspace’s plan' + ? organizationContext + ? 'upgrade the organization’s plan' + : 'upgrade this workspace’s plan' : 'manage this workspace’s usage limits' const unavailableMessage = organizationContext && !planRequired diff --git a/apps/sim/lib/execution/remote-sandbox/index.ts b/apps/sim/lib/execution/remote-sandbox/index.ts index 6de05b3acbd..3b9c35f5d3d 100644 --- a/apps/sim/lib/execution/remote-sandbox/index.ts +++ b/apps/sim/lib/execution/remote-sandbox/index.ts @@ -327,10 +327,15 @@ async function throwIfRunTimedOut( if (!result.timedOut) return if (!run.lease.session) throw new DOMException('timeout', 'AbortError') const { outputSandboxDir, sandbox } = run + // Half of what the call has left, so the report lands before the call's deadline. + const listingMs = Math.min( + TIMED_OUT_OUTPUT_LISTING_MS, + (getRemainingExecutionMs(run.signal) ?? Number.POSITIVE_INFINITY) / 2 + ) const listed = outputSandboxDir ? await withinDeadline( () => sandbox.listFiles(outputSandboxDir, { depth: SANDBOX_OUTPUT_DIR_MAX_DEPTH }), - Date.now() + TIMED_OUT_OUTPUT_LISTING_MS, + Date.now() + listingMs, run.signal ).catch(() => []) : [] @@ -908,21 +913,32 @@ function installBudgetMs(timeoutMs: number): number { */ const MIN_EXPORT_BUDGET_MS = 10_000 +/** + * Held back from a workbench run without an export reserve, capped at a quarter of its budget, + * so the run's own timeout answers and reports its progress before the call's deadline. + */ +const MAX_TIMEOUT_REPORT_BUDGET_MS = 4_000 + /** * The budget handed to user code, less an export reserve when this request will * read files back. Short budgets are left alone: taking the reserve out of one - * would starve the code to buy time for an export it never reaches. + * would starve the code to buy time for an export it never reaches. A workbench + * run, whose timeout reports progress, still keeps a smaller report reserve. */ function codeBudgetMs( req: { outputSandboxPath?: string; outputSandboxPaths?: string[]; outputSandboxDir?: string }, - signal: AbortSignal + signal: AbortSignal, + reportsTimeout: boolean ): number { const remainingMs = remainingSandboxBudgetMs(signal) const exportsFiles = Boolean( req.outputSandboxDir || req.outputSandboxPath || req.outputSandboxPaths?.length ) - if (!exportsFiles || remainingMs <= MIN_EXPORT_BUDGET_MS * 2) return remainingMs - return remainingMs - MIN_EXPORT_BUDGET_MS + if (exportsFiles && remainingMs > MIN_EXPORT_BUDGET_MS * 2) { + return remainingMs - MIN_EXPORT_BUDGET_MS + } + if (!reportsTimeout) return remainingMs + return remainingMs - Math.min(MAX_TIMEOUT_REPORT_BUDGET_MS, Math.floor(remainingMs / 4)) } /** @@ -1060,7 +1076,7 @@ async function executeInSandboxWithinBudget( signal, phases, (verifyCli) => { - runTimeoutMs = codeBudgetMs(req, signal) + runTimeoutMs = codeBudgetMs(req, signal, Boolean(lease.session)) return sandbox.runCode(code, { timeoutMs: runTimeoutMs, javascriptPreload: buildJavaScriptRuntimeBindingsSource(req.runtimeBindings ?? []), @@ -1283,7 +1299,7 @@ async function executeShellInSandboxWithinBudget( signal, phases, (verifyCli) => { - runTimeoutMs = codeBudgetMs(req, signal) + runTimeoutMs = codeBudgetMs(req, signal, Boolean(lease.session)) return sandbox.runCommand(code, { envs: commandEnvironment, timeoutMs: runTimeoutMs, diff --git a/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts b/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts index 93214c32c69..0cb4b375993 100644 --- a/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts +++ b/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts @@ -1130,6 +1130,43 @@ describe('session sandbox lease', () => { } }) + it('reports the output of a short run whose own timeout answers after a round trip', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) + try { + const { handle } = fakeSandbox('timeout-short') + mockFindSessionSandbox.mockResolvedValue(handle) + // The provider answers its own timeout one round trip late, and stops on the call's abort. + handle.runCommand = (_command, options) => + new Promise((resolve, reject) => { + const timer = setTimeout( + () => resolve({ stdout: 'step 1', stderr: '', exitCode: 124, timedOut: true }), + (options?.timeoutMs ?? 0) + 300 + ) + options?.signal?.addEventListener('abort', () => { + clearTimeout(timer) + reject(options.signal?.reason) + }) + }) + handle.listFiles = () => new Promise(() => {}) + const pending = executeShellInSandbox({ + ...CODE_REQUEST, + timeoutMs: 10_000, + envs: {}, + session: { key: 'timeout-short' }, + outputSandboxDir: OUTPUT_DIR, + }).then( + () => undefined, + (rejection: unknown) => rejection + ) + await vi.runAllTimersAsync() + const error = await pending + if (!(error instanceof SandboxExecutionTimeoutError)) throw error + expect(error.progress.output).toBe('step 1') + } finally { + vi.useRealTimers() + } + }) + it('keeps a cancellation that arrives while listing the outputs a cancellation', async () => { const { handle } = fakeSandbox('timeout-cancelled') mockFindSessionSandbox.mockResolvedValue(handle) diff --git a/apps/sim/lib/mothership/tools/server/settings.test.ts b/apps/sim/lib/mothership/tools/server/settings.test.ts index d1b7561df67..06297d12f15 100644 --- a/apps/sim/lib/mothership/tools/server/settings.test.ts +++ b/apps/sim/lib/mothership/tools/server/settings.test.ts @@ -1,4 +1,5 @@ import type { OrganizationDelegatedPrincipal } from '@sim/auth/principal' +import { featureFlagsMock, featureFlagsMockFns } from '@sim/testing/mocks/feature-flags.mock' import { knowledgeAvailabilityMock, knowledgeAvailabilityMockFns, @@ -11,6 +12,7 @@ vi.mock('@/lib/settings/application/organization-section-access', () => ({ authorizeOrganizationSettingsSection: sectionAccess, })) vi.mock('@/lib/knowledge/access/availability', () => knowledgeAvailabilityMock) +vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock) vi.mock('@/lib/mothership/application/settings-context', () => ({ resolveSettingsContext: resolve, })) @@ -98,6 +100,27 @@ describe('settings tool dispatch', () => { }) }) + it('offers Sim links only to an organization that has them enabled', async () => { + const simLinks = expect.objectContaining({ id: 'sim-links' }) + featureFlagsMockFns.mockIsFeatureEnabled.mockResolvedValue(false) + expect( + await settingsServerTool.execute({ scope: 'organization', action: 'list' }) + ).not.toMatchObject({ sections: expect.arrayContaining([simLinks]) }) + for (const action of ['open', 'get'] as const) { + await expect( + settingsServerTool.execute({ scope: 'organization', action, section: 'sim-links' }) + ).rejects.toThrow('unavailable') + } + + featureFlagsMockFns.mockIsFeatureEnabled.mockImplementation( + async (flag, context) => + flag === 'sim-links' && (context as { orgId?: string })?.orgId === 'org' + ) + expect( + await settingsServerTool.execute({ scope: 'organization', action: 'list' }) + ).toMatchObject({ sections: expect.arrayContaining([simLinks]) }) + }) + it('discovers names without eagerly reading every setting or dumping every schema', async () => { const read = vi.spyOn(readOrganizationSettings, 'execute') const result = await settingsServerTool.execute({ scope: 'organization', action: 'list' }) diff --git a/apps/sim/lib/mothership/tools/server/settings.ts b/apps/sim/lib/mothership/tools/server/settings.ts index 6ec644d6ebe..d6c415ee3d9 100644 --- a/apps/sim/lib/mothership/tools/server/settings.ts +++ b/apps/sim/lib/mothership/tools/server/settings.ts @@ -64,6 +64,7 @@ import { } from '@/lib/organizations/application/settings' import { authorizeOrganizationSettingsSection } from '@/lib/settings/application/organization-section-access' import { type SettingsSection, settingsSections } from '@/lib/settings/sections' +import { isSimLinksEnabled } from '@/lib/sim-links/feature-flag' import { delegatedAccountPreferencesSchema, updateCurrentUserPreferences, @@ -86,6 +87,8 @@ interface SettingAdapter { get(context: SettingsContext): Promise schema?: z.ZodType update?(context: SettingsContext, changes: Record): Promise + /** Whether the section exists for this target at all; a section that does not is never listed. */ + available?(context: SettingsContext): Promise } function organizationInput(context: SettingsContext) { @@ -254,7 +257,10 @@ const adapters: Record = { }, 'organization/members': { get: readSettingsRoster }, 'organization/requests': { get: (context) => readSettingsAccessRequests(context) }, - 'organization/sim-links': { get: (context) => readSettingsSimLinks(context) }, + 'organization/sim-links': { + available: (context) => isSimLinksEnabled(organizationInput(context).organizationId), + get: (context) => readSettingsSimLinks(context), + }, 'organization/whitelabeling': { schema: updateOrganizationWhitelabelBodySchema.strict(), get: async (context) => @@ -315,6 +321,7 @@ const adapters: Record = { async function describeSection(context: SettingsContext, section: SettingsSection) { const adapter = adapters[`${context.scope}/${section.id}`] + if (adapter?.available && !(await adapter.available(context))) return null const uiSection = context.scope === 'organization' && section.id === 'connected-accounts' && diff --git a/apps/sim/stores/modals/chat-plan/store.ts b/apps/sim/stores/modals/chat-plan/store.ts index 1008b2aa985..06c92a1d994 100644 --- a/apps/sim/stores/modals/chat-plan/store.ts +++ b/apps/sim/stores/modals/chat-plan/store.ts @@ -2,22 +2,23 @@ import { create } from 'zustand' import { devtools } from 'zustand/middleware' interface ChatPlanModalState { - isOpen: boolean - open: () => void + /** The workspace whose refused send opened the modal; another workspace never shows it. */ + workspaceId: string | null + open: (workspaceId: string | undefined) => void close: () => void } /** - * Whether the Chat plan modal is showing. A send the payer's plan refuses opens it from wherever - * the send started (the composer, a queued message, a handoff from another surface), and the - * workspace shell renders the one modal. + * Which workspace's Chat plan modal is showing. A send the payer's plan refuses opens it from + * wherever the send started (the composer, a queued message, a handoff from another surface), and + * the workspace shell renders the one modal, only for the workspace that opened it. */ export const useChatPlanModalStore = create()( devtools( (set) => ({ - isOpen: false, - open: () => set({ isOpen: true }), - close: () => set({ isOpen: false }), + workspaceId: null, + open: (workspaceId) => set({ workspaceId: workspaceId ?? null }), + close: () => set({ workspaceId: null }), }), { name: 'chat-plan-modal-store' } )