mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-13 07:38:33 +08:00
fb7fd5b62137e4abd21af6ebe3e8940fc202fbe5
5847
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Waleed
|
fb7fd5b621 | chore(copilot): remove the training controls feature (#6257) | ||
|
Waleed
|
63faeb555f | chore(deps): bump undici to 7.29.0 to close 5 security advisories (#6255) | ||
|
Vikhyath MondretiandSim Pi Agent
|
f079c72923 |
feat(library): Best AI Agents for Executive Assistant Tasks (#6254)
Co-authored-by: Sim Pi Agent <pi@sim.ai> |
||
|
WaleedandBohdan Vilishchuk
|
47f5fee8cb |
fix(setup): launch the docker app the CLI is actually pointed at (#6253)
* fix(setup): detect OrbStack vs Docker Desktop before relaunching the daemon ensureDocker() always ran `open -a Docker` to relaunch a stopped daemon on macOS, which silently no-ops for OrbStack users (no Docker.app bundle exists), leading to a misleading "GUI license acceptance" timeout error. Now it checks the docker CLI's active context first (accurate regardless of install location) and falls back to checking for OrbStack.app, so the wizard launches and messages the app that's actually installed. * fix(setup): don't let an installed OrbStack override an explicit Docker Desktop context macDockerApp() fell through to the OrbStack.app existence check whenever docker context show returned anything other than "orbstack" — including a known, explicit context like "desktop-linux". With both apps installed but Docker Desktop active and stopped, this launched OrbStack while daemonUp() kept polling Docker Desktop's socket, timing out with OrbStack-flavored guidance for a Docker Desktop problem. The path fallback now only runs when the context command gives no answer at all (null); any resolved context is trusted outright. Flagged identically by Greptile and Cursor Bugbot on PR #6250. * fix(setup): fall back to the installed app when the context isn't OrbStack Context detection only fell back to the app bundle when `docker context show` failed outright, so an OrbStack-only Mac sitting on the `default` context still resolved to Docker Desktop — the same 90s hang this fix exists to remove. Treat an explicit OrbStack selection as the only positive context signal and otherwise pick whichever app is installed. Read `DOCKER_HOST` first: it overrides the active context, so the context name is not authoritative while it is set. * fix(setup): require OrbStack to be installed before selecting it A context or DOCKER_HOST left behind by an OrbStack uninstall selected an app that can never launch, turning a working Docker Desktop start into a guaranteed 90s timeout. Gate the OrbStack signal on the bundle being present and fall through to whichever app is. Look in ~/Applications as well as /Applications while here — Homebrew casks honour --appdir, so a user-local install is not unusual and a hardcoded /Applications check would misread it as "not installed". * fix(setup): resolve the docker app through LaunchServices, not fixed paths A Homebrew `--appdir` can put OrbStack anywhere, so enumerating install directories will always have a tail that reads a present app as missing and sends setup to the wrong one. Fall back to LaunchServices when the well-known directories miss: that is the same lookup `open -a` performs, so availability now agrees with what the launch will actually do. * fix(setup): settle the docker app with open(1) instead of probing for it `path to application` can raise a modal "Where is …?" picker when the name does not resolve, which in a terminal wizard reads as a hang. Drop it: the launch itself already answers the question, since `open` exits non-zero when macOS knows no such app, instantly and without UI. That inverts the design. Rather than predict which app is installed and then launch it, pick a provider, try to start it, and let the exit code correct a guess — so the directory probe no longer has to enumerate every possible install location to be right. An explicit OrbStack selection is now never redirected to Docker Desktop. The CLI is addressing OrbStack's socket, so `docker info` keeps failing no matter how well Docker Desktop starts; the earlier fallback only replaced a 90s timeout with a differently worded one. Say the context is stale and how to fix it instead. * fix(setup): honour `required` when the docker app fails to launch db.ts and redis.ts call ensureDocker(false) and branch on the boolean to offer an external Postgres or Redis instead. Throwing past that aborts the whole wizard when a working non-Docker path was on the table, so every post-confirm failure now warns and returns false unless Docker is required. That covers the 90s-timeout throw too, which ignored `required` before this branch existed — leaving it as the one path that still aborts would make the flag mean two different things in one function. Also name DOCKER_CONTEXT in the stale-selection hint. It overrides the config context, so `docker context use` alone leaves the CLI pointed at OrbStack and the next run fails identically. * improvement(setup): don't tell CLI-runtime users to install Docker Desktop Having the docker CLI but neither GUI app is exactly what a colima or Rancher Desktop user looks like, and the failure told them to install Docker Desktop — advice for a problem they don't have. Name the situation accurately and add starting an existing runtime as an option. --------- Co-authored-by: Bohdan Vilishchuk <iamtheflex@gmail.com> |
||
|
Waleed
|
3d8b2ed050 |
improvement(docs): inherit the platform border and font-weight scales (#6252)
* improvement(docs): inherit the platform border and font-weight scales #6241 consolidated the app's neutral border tokens and flattened its font-weight scale. `apps/docs` was carrying an untouched copy of the pre-migration values, so the two have visibly drifted — the docs `@theme` block already declares it "mirrors apps/sim/tailwind.config.ts", so the drift is against stated intent rather than a deliberate divergence. Borders — same consolidation as the app: - `--border` #dedede -> #d8d8d8 (light), #333333 -> #444444 (dark) - `--border-1` and `--border-muted` become aliases of `--border`, so the 19 existing `var(--border-1)` consumers pick up the unified colour without being touched - `--divider` is retired; its single consumer moves to `--border` Font weights — the arbitrary values the app dropped: - `font-[480]`/`font-[470]`/`font-[500]` -> `font-medium`, `font-[430]` -> `font-normal`, `font-[600]` -> `font-semibold` The navbar's active tab and its invisible width-reserving ghost both used `font-[480]`; they move to `font-medium` together, so the anti-layout-shift trick still holds. Deliberately NOT ported: the app's `--border-width` hairline (0.5px on hi-dpi). The app wires it through `borderWidth.DEFAULT` in a Tailwind v3 JS config; docs is Tailwind v4 CSS-first, which hardcodes `border: 1px` in the utility with no theme key, so matching it means overriding a Tailwind utility. That is a site-wide visual change and wants its own PR with visual review. Also unchanged: the inline SVGs. `components/icons.tsx` (328) is the brand and integration set, `sim-logo` is a brand mark, and the handful of remaining shapes are bespoke and positioned by hand. Docs already consumes `@sim/emcn/icons` in the 15 places where a shared icon is the right call, and imports zero lucide. * improvement(docs): finish the platform token sweep Follow-on within the same PR. A full comparison of every custom token docs rolls against the platform found three more classes of drift. Text scale — #6241 retuned these and docs kept the old values: - `--text-body` #3b3b3b -> #434343 (light), #cdcdcd -> #c1c1c1 (dark) - `--text-muted` #707070 -> #7a7a7a (light), #787878 -> #6e6e6e (dark) - `--text-icon` #525252 -> #5a5a5a (light), #a0a0a0 -> #969696 (dark) Docs and the app now agree on all 62 shared token names, with zero divergent values. Missing shared-component tokens. Docs renders `@sim/emcn` (Badge, Chip, ChipLink) and `@sim/workflow-renderer` (block, subflow and note views), but never defined 14 of the tokens those components reference — an undefined `var()` silently falls back to `currentColor`, so the failure is invisible until the branch that uses it renders. Several are live: `--warning` on an edited subflow, `--caution` on inline code inside a note, `--text-placeholder` on an empty note, `--border-success` on a successful run edge. Added with the app's values, along with `--text-icon-muted` (new in #6241) and the four Badge palettes docs lacked (teal, cyan, pink, blue-secondary), so any variant renders correctly rather than being one prop away from black. Type scale — docs declares micro/xs/caption/small/base/md in `@theme` but 20 call sites bypassed it with identical raw values (`text-[13px]`, `text-[0.8125rem]`, `text-[12px]`, `text-[15px]`, `text-[10px]`). Each now uses the token; every value is byte-identical, so this is a rename, not a restyle. The class reordering in the same files is biome's `useSortedClasses` reacting to the rename — verified as a pure permutation, with the class multiset unchanged in every file. Deliberately left alone: - `#33C482` / `#2FB3FF` — brand-mark SVG fills and default props in demo data, not styling. - The Ask AI button's inverted `#383838`/`#575757`/`#e0e0e0`/`#cfcfcf`. The platform's `--surface-inverted`/`--surface-inverted-hover` hold *different* values, so adopting them would restyle the control rather than tokenize it. Worth doing, but as a visual change with review. |
||
|
Emir KarabegandWaleed Latif
|
9b9da81a27 |
improvement(platform): drop lucide-react for the in-house icon set, flatten the type and border scales, and retire scheduled tasks and workflow references (#6241)
* border styling * improvement(platform): migrate off lucide-react, flatten the font-weight scale, and retire scheduled tasks and workflow references * chore(platform): drop the dead schedule client layer and repair stale rule and skill docs Follow-up cleanup for the platform commit, which removed the workspace scheduled-tasks surface and migrated off lucide-react. Both left dead tails that type-check clean, so nothing flagged them. Six mutation hooks in hooks/queries/schedules.ts lost their only consumer when the scheduled-tasks page was deleted: useDisableSchedule, useResumeSchedule, useDeleteSchedule, useExcludeOccurrence, useUpdateSchedule, useCreateSchedule. They are removed along with the three contract objects that served only them — disableScheduleContract, excludeOccurrenceContract, deleteScheduleContract. disableScheduleBodySchema and excludeOccurrenceBodySchema are deliberately kept: both are members of scheduleUpdateSchema, the discriminated union the live PUT /api/schedules/[id] route parses. Dropping them would collapse the union and 400 the disable and exclude_occurrence actions. The schedule-calendar tree and its utils stay unmounted for later reuse. Its TSDoc now says so, since it has no importer and would otherwise read as dead code on the next sweep. The add-enrichment skill templated an import from lucide-react, a dependency the platform commit deleted, so running it produced an unresolvable import. It now points at @sim/emcn/icons, matching all five shipped enrichments. The emcn-design-review skill and several rule files still pointed at apps/sim/components/emcn/**, which moved to packages/emcn/**. Also corrects the documented Chip variant list — it advertised a ghost variant that never existed and omitted border — repoints the sim-url-state date-parser example at an inline snippet now that its source file is gone, and normalizes the one strokeWidth the icon migration left at 1.5 in bubble-chat-delay. * fix(platform): mark the resource chrome as client components `skills/page.tsx` is a Server Component, and this branch moved its `IntegrationTabsHeader` import onto the `@/app/workspace/[workspaceId]/components` barrel. That barrel re-exports `SortDropdown` from `resource-options`, which calls `useState`, so the server graph now reaches a client-only module and `next build` fails. `resource-header` has the same latent problem (`useState`, `useEffect`, `useRef`). Both files are genuinely client components, so they get the directive rather than the page dropping the barrel import — local feature barrels are the convention here. Also drops a stale `lucide-react` mention now that the dependency is gone. * chore(scheduled-tasks): remove the scheduled-task logic Scheduled tasks are retired. This removes the `sourceType = 'job'` half of `workflow_schedule` from the application, leaving the workflow Schedule trigger (`sourceType = 'workflow'`) untouched. Gone: - the job orchestration layer (`lib/workflows/schedules/orchestration.ts`) and the agent-job runner in `background/schedule-execution.ts` - the job claim/dispatch half of the schedules execute tick - POST /api/schedules (job creation) and the job branches of GET /api/schedules and PUT/DELETE /api/schedules/[id] - the copilot job tools and handlers, the `scheduledtask` resource type and chat-context kind, and the VFS `jobs/` materialization - the scheduled-task analytics events and the job variant of the schedule-disabled email Kept on purpose: `scheduled-tasks/components/schedule-calendar/**` and `scheduled-tasks/utils/**`, which the agents module will reuse. `packages/db/schema.ts` is deliberately untouched — the columns stay for now and come out in a follow-up with a proper expand/contract migration. The generated copilot catalog and VFS snapshot types are regenerated from the matching copilot PR, which removes the tools and the `jobs` snapshot field at the source. Verified: 23/23 type-check, biome, api-validation, production build, and the full vitest suite (18361 passing; the one failure in executor/handlers/pi/cloud-review-tools.test.ts predates this branch). * fix(sidebar): derive the settings and switcher widths from SIDEBAR_WIDTH This branch moved `SIDEBAR_WIDTH.DEFAULT` from 248 to 238 but left two hardcoded `248px` chrome widths behind, so both sat 10px wider than the live sidebar: - the workspace-switcher menu, which is meant to line up with the sidebar column it drops out of - the standalone settings sidebar, whose own comment says to keep it in step with the in-workspace chrome Both now read `SIDEBAR_WIDTH.DEFAULT` directly rather than repeating the number, so the next change to the constant cannot leave them stale again. * fix(schedules): stop the API accepting actions it no longer handles Adversarial pass on the scheduled-task removal found a real regression in PUT /api/schedules/[id]. Removing the job-only `update` and `exclude_occurrence` handlers left them in `scheduleUpdateSchema`, so those bodies still parsed. The handler chain is `disable` first and then an unguarded fall-through to reactivate, so an `action: 'update'` request would have silently REACTIVATED the schedule instead of being rejected. Both actions are dropped from the discriminated union, so `parseRequest` now rejects them with a 400. Their bodies, response types and the orphaned `createScheduleContract` (its POST route is gone, and nothing imported it) go with them. * chore(landing): retire the scheduled-tasks marketing surface The feature is gone from the product, so the marketing pages stop selling it. - deletes the `/scheduled-tasks` landing page and its calendar-loop hero, and the `LandingPreviewScheduledTasks` panel - drops the view from the landing preview: the `SidebarView` member, the nav entry and its now-unused Calendar icon, the callout label, both render branches, and the staged chat copy in `workflow-data` - removes the navbar and footer links and the sitemap entry - removes the route from `LANDING_ROUTES`, the COEP exemption list that must list every `app/(landing)` route `/scheduled-tasks` is indexed, so it 301s to `/workflows` rather than starting to 404 — that is the surface that still carries scheduled execution via the workflow Schedule trigger. Left alone deliberately: `demo-scheduler` is the Cal.com booking embed for the demo page, unrelated to this feature, and the scheduling library article is a generic SEO piece that never pitched it. * perf(chat): stop the resource picker fetching schedules it no longer shows Dropping the `scheduledtask` group from the add-resource dropdown left `useWorkspaceSchedules` behind, so the picker still issued a workspace schedules request whose result never reached a group. Worse than a wasted request: `schedulesPending` was still in the hydration gate, so the whole picker waited on that response before it could settle, and `schedules` was still a `useMemo` dependency, re-running the group build when it resolved. The hook and its route stay — `/api/schedules?workspaceId=` still correctly lists workflow schedules, unlike `createScheduleContract`, whose route this branch removed. * chore(scheduled-tasks): drop the leftovers the removal stranded An independent audit of the branch turned up dead code and stale docs that the compiler cannot see — nothing behavioural, but all of it rots silently. - README still sold the feature: the "Scheduled tasks" tile, the prose listing it as a workspace surface, and the now-unreferenced screenshot. The landing surface went in c61770a8c; this tile was missed. - `resource-content.tsx`: `SCHEDULE_STATUS_LABEL`, `formatScheduleInstant` and `ScheduledTaskField` were orphaned when the schedule render branch went. - `computeNextRunAt`: zero callers, including tests — its only consumer was the removed agent-job runner. - `applyScheduleUpdate`'s `allowCompleted` option: no call site passes it, and its comment described self-completion, which no longer exists. The guard stays (legacy `sourceType='job'` rows still carry `status='completed'` until the DB follow-up); it is simply unconditional now. - Three TSDoc blocks still described a create-job route and "opening a scheduled-task artifact". Type-check re-run with --force, since a cached turbo replay is not a check. --------- Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Waleed
|
2977db5133 |
fix(deps): revert next to 16.2.12, its 16.3.0 optimizer deletes live code (#6242)
Next 16.3.0's Turbopack optimizer models a bare `return <asyncCall>()` tail call inside an async function as returning the promise object, then propagates that always-truthy fact through the caller's `await`. Where the result feeds an `if (x)` whose every branch returns, it concludes the branch is always taken and deletes everything after it from the emitted bundle. Two sites shipped to production that way: - `POST /api/credentials` lost its entire create path — the transaction, the org locks, the insert, the audit, the 201. A first-time create fell into the existing-credential branch and threw on `existingCredential.id`, so every new credential 500'd. - `upsertAsyncToolCall` collapsed to `async () => await getAsyncToolCall(id)`. The insert is simply gone; it returns null for every new async copilot tool call. Silent — no error, no failed request. A differential scan of 71,266 source string literals across `.next/server` and `.next/static`, comparing images built from the same commit on 16.2.12 and 16.3.0, found exactly these two and nothing else. That scan cannot see dropped branches with no distinctive string literal, which is why the version goes back rather than the two sites being patched alone. Both are also hardened with `return await`, verified to defeat the miscompile in a minimal reproduction. The TypeScript toolchain cleanup from the original bump (dropping @typescript/native-preview, `useTypeScriptCli`) is kept. |
||
|
Theodore Li
|
40cbee4934 | improvement(skills): ask for feature flag granularity (#6238) | ||
|
Waleed
|
856fe0ffb6 |
fix(docker): upgrade bun to 1.3.14 (#6236)
* fix(docker): upgrade bun to 1.3.14 to unbreak the Next 16.3.0 server
Bun 1.3.13 cannot load Next 16.3.0's compiled server runtime. The app container
runs the Next server under Bun (`oven/bun:1.3.13-slim`, `bun apps/sim/bootstrap.js`),
so every app-page render threw and `/api/health` returned 500:
⨯ Error: Failed to load external module
next/dist/compiled/next-server/app-page-turbo.runtime.prod.js:
TypeError: Expected CommonJS module to have a function wrapper.
If you weren't messing around with Bun's internals, this is a bug in Bun
Isolated to Bun, not Next, by loading that exact module in the real images:
Next 16.2.12 + Bun 1.3.13 -> loads (why staging was fine before)
Next 16.3.0 + Bun 1.3.13 -> CJS wrapper error
Next 16.3.0 + Bun 1.3.14 -> loads
Bun 1.3.14 is the current stable and already fixes it, so this bumps every pin
rather than reverting the framework upgrade, which would only defer the same
latent Bun bug to the next attempt.
Why no gate caught it: local dev machines and this bump's own verification run
Bun 1.3.14, while the container and CI pinned 1.3.13 — and CI only *builds* the
image, it never boots one and probes `/api/health`. A container smoke test in CI
would have caught this before merge; that is worth adding separately.
* fix(docker): align the remaining bun pins with 1.3.14
Two pins were missed in the first pass because the search was scoped to
docker/, package.json and .github/workflows/:
- .devcontainer/Dockerfile still built on oven/bun:1.3.13-alpine
- PI_BUN_VERSION in apps/sim/scripts/pi-sandbox-packages.ts was still 1.3.13,
despite being documented as mirroring the root packageManager field, so Pi
sandbox images would have kept installing the Bun release that cannot load
the Next 16.3.0 server runtime.
Fixed surgically rather than with a repo-wide replace: "1.3.13" also appears
inside SVG path data in apps/sim/components/icons.tsx and
apps/docs/components/icons.tsx, which a blind sed would have corrupted.
|
||
|
Waleed
|
894083390e |
improvement(agent): allow variable references in reasoning effort, verbosity, and thinking level (#6233)
* improvement(agent): allow variable references in reasoning effort and verbosity
Reasoning Effort and Verbosity were select-only dropdowns, so a workflow could
not sweep them from a variable or an upstream block the way it already can with
the model. Both become editable comboboxes, matching the model field directly
above them and the managed-agent selectors.
- switch both subblocks to `combobox`, keeping their fetched per-model option
lists intact
- keep them visible when `model` itself holds a reference, since the concrete
model id is only known at execution time and cannot be matched against the
static capability list
- normalize the resolved level in the provider chokepoint so a reference that
resolves to `"High"` or to nothing behaves sanely instead of hitting a
provider 400
* improvement(agent): allow variable references in thinking level
Extends the same treatment to Thinking Level so all three model-tuning fields
behave consistently, and logs a level a model does not declare.
- switch `thinkingLevel` to `combobox` with the reference-aware condition
- normalize it alongside the other two; an empty resolve now takes the
deliberate "send nothing" path rather than the incoherent half-state it hit
before, and stays distinct from an explicit `none`
- warn when a level is not one the model declares, still forwarding it: Sim's
per-model lists drive the pickers and can lag a provider, and a sweep needs
the provider's own error rather than a silent fallback to the default
* improvement(agent): report a model level the sanitizer discards
A model bound to a variable or block reference only resolves at execution time,
so a run whose reference landed on a model outside Sim's catalogue had its
requested level cleared with no signal and quietly fell back to that model's
default. Dropping stays the safe default — a provider with no such parameter
rejects the whole request — but it is now reported.
- log the field, model, and value whenever an unsupported-field level is cleared
- cover both diagnostics, including that they stay quiet for a declared level
and for the `auto` / `none` sentinels
* fix(providers): redact resolved level content from sanitizer diagnostics
The model-level fields accept environment and block references, so an
unrecognized level is not necessarily a mistyped level — it is whatever the
reference resolved to, which can be secret content. The diagnostics added for
dropped and undeclared levels echoed it straight into server logs.
- log a level only when the catalogue declares it somewhere, or it is an
`auto` / `none` sentinel; anything else is reported by length alone
- stop discarding levels for a model the catalogue has never seen. Absent is
unknown, not known-incapable, and a reference is exactly how a newly released
model arrives before Sim catalogues it — the provider decides instead. Models
the catalogue knows, and every dynamic-provider id, keep the protective drop
* fix(providers): redact the level in Anthropic's unsupported-thinking warning
Forwarding an undeclared level is deliberate, but it means the Anthropic adapter
receives it and interpolates it straight into its "not supported, ignoring"
warning. Since the field is reference-bound, that value can be whatever a
mistyped `{{ENV_VAR}}` or block reference resolved to — so the redaction added
for the sanitizer's own diagnostics was leaking one layer downstream.
- promote the level renderer to `providers/utils` as `describeModelLevel`, the
single gate every site echoing a caller-supplied level goes through
- use it in Anthropic's warning and in both sanitizer diagnostics
* refactor(providers): drop the sanitizer's level diagnostics
The two warnings logged server-side, where the workflow author who set the level
never sees them, and the surprising case they described — a level discarded for
a model newer than the catalogue — is now fixed at the source rather than
narrated. They also carried the redaction that leaked resolved content before it
was caught, so removing them removes that surface entirely.
Levels still normalize, and still drop for a catalogued model that does not take
the field. `describeModelLevel` stays for Anthropic's unsupported-thinking
warning, which is a pre-existing log this feature newly exposes to resolved
reference content.
|
||
|
Waleed
|
d05289cfa2 |
fix(providers): route 6-10MB attachments to the provider large-file path (#6232)
* fix(providers): route 6-10MB attachments to the provider large-file path
The inline base64 cap was 10 MB of raw bytes, but the execution payload store
refuses a single value above 8 MiB and base64 inflates by 4/3. Every raw file
over 6 MiB therefore produced a base64 string the store rejected — and the
rejection came from the base64 *cache* write, which threw and failed the run
with "Execution memory limit exceeded" even though the bytes had already been
read successfully. Because shouldUseLargeFilePath only fires above the inline
cap, 6-10 MB attachments had no path at all on any provider: they never reached
the OpenAI or Gemini Files API upload they were supposed to take.
Derive the cap from the payload-store ceiling instead of hardcoding it, and
degrade a refused cache write to "not cached" rather than failing a request
whose bytes are in hand. Every other size guard in the chain compares raw bytes
against maxBytes; only the Redis write sees the encoded size, which is why this
went unnoticed — and why it failed only where Redis is configured.
Also correct the provider ceilings against the vendors' current documentation:
- openai: 50 MiB -> 50,000,000. The gate is `size > maxBytes`, so 50 MiB
admitted 52,428,800 bytes; the docs say each file must be *under* 50 MB.
- bedrock: had no entry and inherited the inline cap, which is above what
Converse accepts (3.75 MB per image, 4.5 MB per document).
- groq: 20 MiB -> 20,000,000, and modelled as the request cap the docs
actually describe rather than a per-file MiB ceiling.
- fireworks: had no entry; its 10 MB budget is on the base64 total, so the
raw-byte equivalent is 7.5 MB.
Add perRequestMaxBytes for the combined ceilings, enforced before any upload
spend, and cover the OpenAI upload path end to end — it had no test at all.
* chore(deps): upgrade @google/genai to 2.13.0 and @anthropic-ai/sdk to 0.115.0
@google/genai 2.x reworks the Interactions API, which the Gemini deep-research
provider is built on. Migrate it:
- `Interaction.outputs` (a flat content array) is now `steps`, a discriminated
timeline; the report text lives in the `model_output` steps' text content,
alongside thought and tool steps we skip.
- `Usage.total_reasoning_tokens` is now `total_thought_tokens`. The old code
already fell back to that name through a cast, so this just makes the field
the SDK actually returns the typed one.
- SSE events renamed: `content.delta` -> `step.delta`, `interaction.start` ->
`interaction.created`, `interaction.complete` -> `interaction.completed`.
The new event types are discriminated, so the payload casts are gone.
Both `interactions.create` calls also stop annotating their params with
`Interactions.CreateAgentInteractionParams{,Non}Streaming`. In 2.13.0 those
namespace aliases resolve to `CreateAgentInteraction`, whose `stream` is a
plain `boolean` rather than a literal — annotating with them erases the
discriminant and the call resolves to the union-returning overload, so the
result is typed as `Interaction | Stream` at every use. An inline
`stream: true as const` keeps the correct overload.
Neither upgrade required a `minimum-release-age` waiver: 2.15.0 and 0.115.0
were checked and 2.13.0 is the newest genai release clearing the 7-day window.
* chore(deps): upgrade openai to 7.0.0
v5 is the only major with real breaking changes for us; v6 widened a Responses
output type and v7 only raised the Node floor to 22, which apps/sim already
requires. Three things needed fixing:
`ChatCompletionMessageToolCall` became a union of function and custom tool
calls, and the custom variant has no `function` field — 43 unguarded `.function`
accesses across the OpenAI-compatible providers. Narrow once at each
`message.tool_calls` read through a shared `isFunctionToolCall` guard rather
than casting at every use.
That guard deliberately tests for the `function` payload instead of
`type === 'function'`. Many OpenAI-compatible vendors omit `type` on tool calls
entirely — our own fixtures do — so discriminating on it type-checks perfectly
and then silently drops every tool call those providers return.
`ChatCompletionCreateParams.verbosity` narrowed from `string` to a literal
union, and the Responses API's output and input item unions now diverge on
members Sim never emits (computer-use call outputs, whose `status` admits
`failed`, and the `AdditionalTools` escape hatch). Echoing output back as input
is what a tool loop is supposed to do, so that conversion is asserted once in
convertResponseOutputToInputItems and the streaming loop now routes through it
instead of pushing raw output items.
The hand-rolled multipart upload in file-attachments.server.ts can now be
replaced with the SDK's typed `expires_after` — left for a follow-up so this
commit stays a pure upgrade.
* fix(providers): correct defects found auditing the attachment and SDK changes
The mechanical rewrite that added `isFunctionToolCall` to every `tool_calls`
read also rewrote three truthiness guards, where the filtered array was
computed, discarded, and the unfiltered value used in the body. Filter once and
use that value. The helper also landed between `trackForcedToolUsage`'s TSDoc
block and its declaration, leaving that block documenting the wrong function.
Raise the Bedrock ceiling from 3.75 MB to 4.5 MB. Converse caps an image at
3.75 MB and a document at 4.5 MB, and a single `maxBytes` cannot express both.
Taking the lower bound looked conservative but regressed 3.75-4.5 MB documents,
which Converse accepts and which work today. At the document bound every size
that works now still works, and only genuinely-too-large files are rejected
early; oversized images in that band keep surfacing as a Bedrock API error,
exactly as they do without the entry.
Both limits re-verified verbatim against the primary docs: Converse's Message
reference ("Each image's size ... no more than 3.75 MB", "Each document's size
must be no more than 4.5 MB") and Fireworks' vision guide ("Total base64-encoded
images must be less than 10MB").
* fix(providers): keep every attachment that works today working
Auditing the routing change for backwards compatibility turned up two bands it
silently broke.
Lowering the single inline cap made the upload path mandatory above ~6 MB, but
every large-file path reads its bytes back out of cloud object storage. A
deployment without it — local dev, any disk-backed self-host — inlines those
files as base64 today and would have started failing outright with "requires
cloud file storage". Split the one number in two: the inline ceiling stays at
10 MiB, and a separate threshold marks where an upload becomes *preferable*
because the base64 copy no longer fits the payload store. Where no upload path
is reachable, base64 hydration now runs to the inline ceiling as before, and a
missing cloud-storage backend leaves the file for the inline path instead of
throwing.
The two strategies also cross over at different sizes now. `files-api` carries
every type the provider already accepts, so it takes over at the lower
threshold. `remote-url` only fetches images and PDFs, so switching early would
have started rejecting 6-10 MB text documents that inline fine today; it takes
over only once inlining is genuinely impossible.
Revert the Groq ceilings. Its published "20MB" governs a request carrying an
image URL, and on this path the body holds only the URL, so it cannot bind on
the files maxBytes guards. Groq documents no limit on the image it fetches, so
tightening the per-file cap to 20,000,000 and summing raw bytes against the
request cap would both reject uploads that work today on no documented basis.
* fix(providers): drop the provider ceiling changes and close the audit findings
A six-agent line-by-line audit against the vendors' live docs found the
`models.ts` ceiling work was not the strict improvement it was written as, so
all of it is reverted:
- bedrock's 4.5 MB cap broke video. Converse takes image, document AND video
blocks, and video is allowed 25 MB base64 — a single `maxBytes` cannot
express three content classes, and every 4.5-10 MiB `.mp4` that works today
would have started failing.
- openai's combined 50 MB cap is the FILE-input limit. Image inputs are
governed separately at 512 MB / 1500 images, so summing every attachment
rejected eight 8 MB PNGs that OpenAI documents as legal.
- fireworks' per-file ceiling was unreachable behind the request budget, while
the upload picker went on advertising it — a size the UI accepts and
execution always rejects.
- The whole `perRequestMaxBytes` feature goes with them: it summed raw bytes
against caps that are variously on encoded bytes, on one content class, or on
a body that carries only URLs, and it double-counted a file referenced from
several messages even though the uploader dedupes by key.
Only openai's per-file `maxBytes` stays corrected, to decimal 50,000,000 — the
one number a vendor states unambiguously and writes no MiB against.
Also fixed, all found by the same audit:
The hydration cap stopped short of where `remote-url` actually switches over,
so 6-10 MiB attachments on anthropic/openrouter/xai/groq/together/baseten/vllm
had neither base64 nor a handle and failed outright — the very band this branch
exists to fix. Both decisions now come from one function so they cannot drift.
Eight more sites where the mechanical rewrite computed a filtered array and
then read the unfiltered one (deepseek, sakana, nvidia, kimi), leaving those
providers without the narrowing they appear to have.
`isFunctionToolCall` threw on a null or primitive `tool_calls` entry, because
`in` requires an object — reachable exactly on the self-hosted gateways this
filter was added for. It is now total, and all 32 test mocks match it rather
than being quietly more permissive.
`checkForForcedToolUsage` in utils/litellm/mistral evaluated the response before
the `tool_choice` test, turning a tolerated malformed body into a TypeError on a
path that never used to touch it.
Gemini: `satisfies` restores the excess-property checking the dropped
annotations removed, the poll loop recognises the terminal statuses v2 added
instead of spinning for an hour and reporting a timeout, and the streaming doc
block no longer names five events that were renamed six lines below it.
* fix(providers): report attachment limits in the unit vendors publish
The size ceilings are decimal MB — that is how OpenAI, AWS and Fireworks all
write them — but the error messages divided by 1024², so OpenAI's 50 MB cap
was reported to the user as "48MB". Someone shrinking a 49 MB file to get
under it was chasing a limit that does not exist.
One formatter, used by all three messages, so the file size and the ceiling in
the same sentence are always in the same unit.
* fix(providers): derive the limit unit from the ceiling it belongs to
The previous commit fixed OpenAI's "48MB" by dividing every ceiling by 10⁶ —
which broke the other seven. Only OpenAI's constant is decimal; anthropic,
google, together and openrouter are 50 MiB, baseten and vllm 25 MiB, groq and
xai 20 MiB. Rendering those as decimal MB overstated each by ~5%, so a 21 MB
file on groq was rejected with "(21MB) exceeds the 21MB limit" — a sentence
that contradicts itself and sends the user to shrink a file to a size that is
still over. Same class of bug as the one being fixed, sign flipped.
Both figures now render through one unit taken from the ceiling, so the number
a user is told is the number the vendor publishes and the two sizes in a
sentence are always comparable. Tested against every ceiling in the registry
rather than only the values that happened to round cleanly.
Two more from the same audit:
A file with a missing or zero declared size was stranded on a files-api
provider: hydration bailed on the real byte length while `shouldUseLargeFilePath`
saw `0 > threshold` as false, so it got neither base64 nor a handle and failed
as "may no longer be accessible" — a size failure wearing an access failure's
message. Uploads read the real bytes and enforce the ceiling themselves, so an
unknown size now routes to one.
The oversized-attachment error blamed the provider for a deployment problem:
a files-api provider on a host without cloud storage reported that the provider
"has no large-file upload path", which is not true of the provider.
`isFunctionToolCall` only proves `function` is present, never that it is well
formed, so the trace enricher is defensive again about a hollow payload without
giving up the compile-time gate. The 32 test mocks now match production exactly.
* fix(providers): stop an over-limit size rendering as the limit itself
Deriving the unit from the ceiling fixed the 5% error but left the precision
fixed at two decimals, so a file one byte over a 20 MiB cap still printed
"(20.00MB) exceeds the 20MB agent attachment limit" — the same self-contradicting
sentence, now in a ~5 KB band above every ceiling in the registry. The size
rounds up and the ceiling rounds down, so the two can no longer collide.
The test that was supposed to guard this asserted a file 0.03MB over and an
OpenAI file that was under the limit — neither anywhere near the band — so it
passed while the bug was live. It now walks `limit + 1` for every ceiling, and
goes red against the old rounding.
The reason clause added last commit also claimed a deployment had no cloud file
storage whenever the strategy was not inline. A generated document on a
remote-url provider reaches that same error with storage fully configured,
because a signed URL points at the generation source rather than the rendered
artifact — so it was told something false about its own deployment. That case
now names itself.
* fix(providers): order the attachment failure reason by how general the cause is
The generated-document arm was checked first, so it won over both other causes
and told users two things that were not true.
On an inline-strategy provider — bedrock, mistral, ollama, fireworks, litellm,
vertex, kimi — there is no upload path for any file, generated or not, but the
message blamed the document format and implied a plain PDF would go through.
On openai or google with cloud storage unconfigured it was simply false: a
generated document does take the Files API path there, and that exact file
uploads fine once storage exists. The one actionable fix was hidden from the
operator.
A provider with no upload path cannot be helped by changing the file, and a
deployment with no object storage cannot reach any upload path whatever the
file is, so both now outrank the format-specific case — which is left saying
only what is true of it: a signed URL points at the generation source rather
than the rendered file.
The formatter is unchanged. It was brute-forced over every real ceiling and
three million random pairs with no collision or inversion, but the test's six
ceilings all divide to exact integers, so floor, round and ceil are
indistinguishable on them and the limit-side rounding was unpinned. A ceiling
with a fractional remainder now covers it.
|
||
|
Waleed
|
ed17bb2bac |
chore(deps): upgrade next to 16.3.0 and clean up the TypeScript toolchain (#6235)
Bumps next, @next/env and the @next/swc-* optional deps to 16.3.0 across the root overrides, apps/sim, apps/docs and packages/emcn. Two config notes worth keeping: - `experimental.turbopackFileSystemCacheForBuild`'s default flipped false -> true for stable in 16.3.0, so our explicit `false` is now load-bearing rather than defensive. Without it this bump would have silently re-enabled a build cache measured 3.2x slower on this codebase (#6078). Comment updated to say so. - `experimental.useTypeScriptCli: true` is now pinned. TypeScript 7 ships no JavaScript compiler API until 7.1, so Next's default checker cannot run and needs the project-local `tsc` CLI instead. 16.2.12 was silently skipping build-time type checking entirely because it detected @typescript/native-preview and short-circuited the stage ("Finished TypeScript in 138ms"); pinning the flag keeps that from drifting back. TypeScript toolchain cleanup that the upgrade makes possible: - Drop @typescript/native-preview from apps/sim and apps/docs. It was the pre-release channel for TS 7 and is superseded by typescript@7 (nightlies now ship as typescript@next), and its presence is what suppressed build type checks. - Align packages/browser-protocol and packages/terminal-protocol from typescript ^5.7.3 to ^7.0.2 so every workspace is on one compiler version. apps/sim keeps @typescript/typescript6 as a production dependency: the function sandbox at app/api/function/execute dynamically imports the TS 6 compiler API to transpile user code, and TS 7 has no API to replace it yet. Build and dev were benchmarked 3x per version on a byte-identical tree; the upgrade is performance-neutral (build median 100s -> 99s, dev:full warm 10s -> 9s). |
||
|
Waleed
|
f210a6edc0 |
fix(execution): offload buffered event values under budget pressure (#6231)
* fix(execution): offload buffered event values under budget pressure An execution buffers EVENT_LIMIT events inside a per-execution byte budget, so a full ring only fits if events average under budget/EVENT_LIMIT. Values were only offloaded to object storage at the shared 8 MiB cap, far above that, so a run emitting large block outputs exhausted its budget within a few dozen events and stayed pinned at its ceiling for the rest of its life. Applying that ceiling to every run would be worse than the problem: the SSE stream carries the compacted event and the terminal renders a ref only as a preview, so ordinary block outputs would stop being readable live, and every value would cost an object-storage write on the hot path. Engage the tight ceiling only once a run has actually buffered past half its budget. A short run keeps full-fidelity output and pays nothing; a runaway one stops accumulating. Both bounds derive from the existing budget rather than being asserted, and preserved UserFile base64 is exempt — it is an explicit request for inline delivery, already bounded by its own cap and the strip-and-recompact fallback. Also stop a failed resume-path buffer write from failing the run: it was awaited bare, so the rejection propagated into the executor callback and failed work that had already completed. The buffer only backs reconnect replay, so degrade to live-only delivery the way the execute route does. * fix(execution): measure pressure at write time and keep terminal status last Pressure was read from bytes counted once a flush succeeded, but a burst is compacted long before the scheduled flush runs — so the very batch that exhausts the budget went through at the loose ceiling and was dropped instead of offloaded. Count bytes as each event is compacted. Separately, the terminal-alone retry stamped terminal status while entries queued ahead of it were still unwritten. Terminal status is the reader's end-of-run signal: a reconnecting client drains what is in Redis and closes, so those entries were stranded behind a stream it had already finished with. Drain the backlog first, then publish the terminal event. * fix(execution): do not lose the backlog or publish terminal status early Draining the backlog ahead of the terminal event left the terminal status armed, so whichever chunk emptied the queue stamped the run complete before its terminal event was written — the inverse of the ordering the drain was added to guarantee. Disarm the status for the drain and restore it afterwards. The drain's result was also discarded: a transient Redis failure requeues its batch, and the unconditional reassignment that followed dropped those events even though the budget never rejected them. Keep whatever could not be persisted, and publish the terminal event alone only once nothing earlier is still queued — failing otherwise lets the caller degrade, which records the status without claiming the missing events arrived. Leave eventId unset on a failed resume-path write. Assigning 0 was persisted by clients as a reconnect cursor and rewound them to the start of the run. * fix(execution): keep an event in the buffer when a pressure offload fails Durable compaction runs before an event is queued, so a storage or metadata failure dropped it from replay entirely — a reconnecting client would never see it, even though the live path carried on. Offloading under pressure is only an optimization that keeps a heavy run from exhausting its budget, so when the value cannot be persisted, fall back to buffering it inline: exactly what the run would have done before pressure engaged. |
||
|
Waleed
|
41592dfbf1 |
fix(execution): stop the event buffer retaining a run-length backlog (#6229)
The Redis byte-budget branch in doFlush requeued the rejected batch and rethrew, skipping the MAX_PENDING_EVENTS trim every other failure path applies. The backlog then grew for the rest of the run and each retry re-serialized it, so a wide parallel fan-out could drive unbounded heap growth and stall the event loop. Drop rejected chunks instead of requeueing, pace retries through the existing backoff, and split batches that exceed the single-write cap so an oversized batch can make progress instead of stalling forever. Terminal status is now writer-scoped, since a concurrent scheduled flush can be the loop that drains the final chunk, and a terminal event whose batch was dropped is retried on its own rather than lost with it. Record terminal stream meta when the terminal event cannot be buffered, so reconnecting readers stop polling an active stream until their deadline. Drop the unused reserve/release budget helpers. |
||
|
Siddharth Ganesan
|
5ab5f2c7ed |
feat(browser, terminal): implement browser driver, password manager, terminal features (#6196)
* icon styling * feat(desktop): isolate chat browser and terminal sessions * feat(desktop): uncap browser and terminal tabs * feat(desktop): polish browser and terminal resources * improvement desktop * fixes * updates * fixes * fix * update tests |
||
|
Waleed
|
3de63c94e3 |
feat(self-host): align Docker Compose with Helm and overhaul self-hosting docs (#6225)
* feat(self-host): align Docker Compose with Helm and overhaul self-hosting docs Docker Compose shipped no scheduler, so scheduled workflows, every polling trigger, connector syncs, the outbox, and data drains silently never ran. Adds a cron service running the same 18 jobs the Helm chart schedules as CronJobs, and closes the remaining behavioral gaps between the two paths: bundled Redis in the chart, no hosted plan caps in chart defaults, pinned image tags, and fail-fast secrets. A CI check keeps the schedulers in sync. Also rewrites the self-hosting docs: 14 new pages, 8 updated, reorganized into Install / Configure / Operate. * fix(self-host): drop bun install from chart CI, remove air-gapped and backup docs The scheduler-parity check pulled a full dependency install into the chart-validation job, which fails building isolated-vm on that runner. Rewritten to use only node builtins so the job installs nothing. Also removes the air-gapped and backup/restore pages, and stops pinning a concrete release in the docs so the examples do not go stale each release. * fix(helm): bundle Redis in secret-manager modes unless the URL is supplied Suppressing Redis whenever a secret mode was active left those deployments with no Redis at all — REDIS_URL is optional there and both shipped examples omit it. The chart now steps aside only on a detectable signal: an explicit app.env.REDIS_URL, an ESO remoteRefs.app.REDIS_URL mapping, or the new redis.provideUrl=false opt-out for a pre-created Secret it cannot read. * fix(compose): derive realtime BETTER_AUTH_URL from NEXT_PUBLIC_APP_URL realtime read BETTER_AUTH_URL directly and fell back to localhost while simstudio derived it from NEXT_PUBLIC_APP_URL, so setting only the public origin left realtime authenticating against http://localhost:3000. * fix(helm): deliver bundled REDIS_URL via ConfigMap so an operator value always wins Injecting REDIS_URL as an inline container env made it beat every envFrom source, so a REDIS_URL held in a pre-created Secret or synced by External Secrets was silently shadowed and traffic moved to a fresh in-cluster Redis. Kubernetes resolves duplicate envFrom keys by letting the last source win, so the bundled URL now ships as a ConfigMap listed before the app Secret. Any operator-supplied value overrides it without the chart needing to read it, which also removes the redis.provideUrl flag the previous attempt required. * docs(helm): spell out the egress rule external datastores need The default NetworkPolicy allows 443 plus the bundled Postgres and Redis by pod selector. Anything you run outside the chart on another port needs its own rule, which is easiest to miss when REDIS_URL arrives via a Secret the chart cannot inspect. Adds a copyable example to the production checklist and the security guide. * feat(helm): add networkPolicy.allowExternalEgress for managed datastores The default policy allows 443 plus the bundled Postgres and Redis by pod selector, so a managed datastore on another port needs a hand-written CIDR rule — awkward when REDIS_URL arrives via a Secret the chart cannot inspect. Adds an opt-in switch that drops the port restriction while still blocking the cloud metadata endpoints. Defaults to false, keeping this chart stricter than the common chart default of unrestricted egress. |
||
|
Waleed
|
1242301bd3 |
improvement(emcn): share one emails/domains chip input across share and deploy modals (#6226)
* improvement(emcn): share one emails/domains chip input across share and deploy modals
Extracts the emails chip lifecycle out of ChipModalField type='emails' into a
standalone ChipEmailsInput, and points both the file share modal and the deploy
modal's chat tab at it instead of their hand-rolled TagInput wiring.
- add ChipEmailsInput (dedupe, normalize, format gate, paste, per-chip errors)
with an allowDomains opt-in for bare @domain.tld entries
- share modal and deploy modal chat tab now use it; drop both hand-rolled
add/remove/validate implementations and the dead emailError state
- move the shared allowlist policy into validateAllowlistEntry
- drop the "Add specific emails or whole domains" hint text
- give OutputSelect a size prop; the deploy modal chat tab uses the 30px chip
trigger so it lines up with the Title field above it
- drop overflow-y-auto from the chat deploy form, which was promoting overflow-x
to auto and rendering a stray horizontal scrollbar
* improvement(utils): one email syntax gate, drop the backtracking placeholder regex
Audit follow-ups on the emails chip input.
- move EMAIL_SYNTAX_REGEX and the new @domain pattern into @sim/utils/string as
isValidEmailSyntax, so emcn and lib/messaging/email/validation.ts stop keeping
byte-identical copies of the RFC 5322 regex
- allow single-label domains (@intranet) again — requiring a dot rejected
entries the old startsWith('@') check accepted, which self-hosted
deployments use. A lone @ and malformed labels stay rejected
- replace derivePlaceholderWithTags' /^Enter\s+(.+?)s?$/i with string ops;
CodeQL flagged it as polynomial backtracking (js/redos). Verified identical
output across the placeholder shapes in use
- forward the emails control's props explicitly instead of underscore-discard
destructuring, matching ChipModalFileControl in the same file
* test(email): pin the allowlist entry rules
Covers isValidEmailSyntax's allowDomains branch (single-label domains stay
valid, malformed bare domains that the old startsWith('@') check accepted do
not), the 254-character cap, the DNS label limit, and validateAllowlistEntry
waiving address-level policy for bare domains. Verified both new rules fail
when the behavior is reverted.
|
||
|
Waleed
|
67e355b1ec | chore(landing): remove the Russell Investments logo (#6228) | ||
|
Waleed
|
c759a889d8 |
refactor(voice): load STT availability through React Query (#6224)
* refactor(voice): load STT availability through React Query useSpeechToText fetched `/api/settings/voice` inside an effect and stored the result in useState behind a hand-rolled mountedRef guard: no cache, no dedupe across mounts, and no AbortSignal, so the response was fetched and parsed even after unmount. Two simultaneously mounted consumers issued two requests. It also bypassed hooks/queries/**, which is where every other server read in the app lives — and it escaped `check:react-query`, whose audit only covers useQuery/useMutation call sites. The value is server env read at request time, so it cannot change within a session; the new hook uses an infinite staleTime and a caller-controlled `enabled` so clients without the audio APIs never issue the request. Hydration is unchanged: SSR renders unavailable, and the first client render still resolves unavailable because `data` is undefined until the fetch settles. No initialData, deliberately — adding it would break that. mountedRef stays; it is still load-bearing for the streaming lifecycle. * test(queries): unmount rendered roots between tests renderHookWithClient created a React root per test but never tore it down, so trees stayed mounted with live QueryClient observers until worker teardown and async notifications could cross test boundaries. Audited every test in the repo using createRoot: 51 of 53 already unmount. The two that did not were both mine — voice.test.tsx here and chats.test.tsx from #6223 — so both are fixed and the pattern is now uniform. * fix(voice): let a failed STT probe recover on a later mount The app QueryClient sets retryOnMount: false and retry: 1, and refetchOnWindowFocus only refetches stale queries — which an infinite staleTime never becomes. So one transient failure cached the error for the life of the client and hid the mic until a full page reload. The effect this replaced refetched on every run, so retryOnMount: true restores parity: no refetch after success, a retry per mount after failure. Test asserts recovery under the app's real query defaults and fails without the override. |
||
|
Theodore Li
|
3f70096841 |
improvement(self-host): gate email verification on a mail provider, add self-host settings, land setup on signup (#6216)
* fix(auth): skip email verification when no mail provider is configured Signup pushed /verify unconditionally, stranding self-hosted deployments with no mail provider on a screen no email could ever satisfy. Derive one server-side effective value (verification enabled AND deliverable) and read it from Better Auth enforcement, signup routing, and the verify page. * feat(settings): add a self-host section with the managed Chat keys link Self-hosters had no in-app pointer to the managed service that issues their Chat keys. New Settings > System > Self-host section, gated on `requiresSelfHosted` so it is absent on hosted Sim, containing only that link. * improvement(setup): land the wizard handoff on signup A freshly provisioned deployment has no accounts and / renders the marketing landing page, so the bare origin left operators hunting for the CTA. Single-source the URLs and point every open-Sim handoff at /signup across all three modes. * improvement(settings): mark the self-host section with a sprout Server was already doing double duty for MCP servers and Mothership, and the icon set ships no botanical glyph, so the mark is a text emoji. * improvement(settings): draw the sprout as an emcn line icon, move to Platform The emoji rendered in the platform's own colors, so it was the one glyph in the nav that ignored --text-icon. Replaced with a hand-drawn emcn Sprout (24 grid, 1.55 stroke, currentColor) matching the house style, renamed the tab to Self hosting, and regrouped it under Platform — self-hosting is deployment-wide, not per-workspace. Still self-hosted-only. * improvement(settings): drop the section header from self hosting One row does not need a section label, and removing it takes the divider with it. The body is now the Chat keys row and its managed-keys link, nothing else. |
||
|
Waleed
|
1708173970 |
refactor(chat): clean up the deployed chat surface (#6220)
* refactor(chat): clean up the deployed chat surface Eight-angle cleanup pass over the full contents of the chat surface and the speech code that survived the voice-mode removal. Dead code - enforceChatRateLimit: added for the TTS relay in #6212, orphaned when #6215 deleted that route. Zero consumers. - ChatToolCallStatus, ChatErrorType, and six unused CHAT_ERROR_MESSAGES keys (only GENERIC_ERROR and CHAT_UNAVAILABLE are read). - scrollToMessage was declared and destructured by ChatMessageContainer but never used in its body; removing the prop also made the scrollToShowOnlyMessage branch unreachable, since the sole caller passed true. - permissionState and the language prop on useSpeechToText: both write-only across the repo. - The image branch in ChatFileDownload's renderIcon returned the same DefaultFileIcon at the same size as the fallback. - chatKeys.status/detail: aliases of deploymentKeys nothing imported, and misleading since they root under a different key namespace. Redundant state - password-auth and email-auth each kept a boolean in lockstep with `errors.length > 0`; email-auth also validated on every keystroke and then immediately hid the result. - file-download tracked hover in state to drive one opacity class; now group-hover. Verified emcn Button sets no `group` class of its own. Memoization - ChatMessageContainer's memo() could never bail: chat.tsx passes an inline arrow for scrollToBottom and displayMessages is a fresh array. Four of the five things that re-render ChatClient are its props anyway, so the memo is dropped rather than propped up. - ClientChatMessage keeps its memo — it blocks markdown re-parsing — but loses the custom comparator, which compared proxies (a key:status fingerprint, files by length) and ignored attachments and type entirely. Default shallow compare on its single prop is both simpler and stricter. - Six useCallbacks whose consumers are native DOM handlers or inline arrows, so nothing observed their identity. Effects - The scroll listener attached in an effect keyed on [chatConfig, authRequired] — values it never reads, standing in for "the container has mounted". It now attaches via a ref callback, so it no longer re-attaches on every config refetch. Design system and a11y - z-[100] -> z-[var(--z-dropdown)] (same value), shadow-lg -> shadow-medium, list styles from inline style to Tailwind classes, hover: -> hover-hover: on touch-reachable targets, Check sourced from emcn alongside its Duplicate pair. - Accessible names on the remove-attachment, stop, and send buttons, which announced only as "button". - Dropped a keyboard handler on a role='group' div with no tabIndex, where target === currentTarget was unreachable, and the Tooltip Provider wrappers and delayDuration, which emcn documents as no-op passthroughs. * fix(chat): restore markdown list markers The design-system pass swapped inline `listStyleType` for Tailwind classes, but the edit that added `list-disc`/`list-decimal` silently did not apply while the one removing the inline style did. With Preflight setting `list-style: none`, every bullet and number in an assistant response disappeared. `list-item` on the `li` sets display only, not the marker type. |
||
|
Waleed
|
b0491f1161 |
fix(chat): invalidate deployment queries after a chat mutation (#6223)
PATCH /api/chat/manage/[id] calls performFullDeploy when the workflow has drifted from its active deployment, so editing a chat can mint a new deployment version. useUpdateChat invalidated only chatStatus and chatDetail, leaving the deployment panel showing the previous version and a stale "needs redeployment" indicator until the staleTime expired. Both mutations now route through invalidateDeploymentQueries, the shared helper the rest of the deployment surface uses. That also picks up deployedState, which useCreateChat's hand-rolled list had omitted even though performChatDeploy replaces the deployed workflow state. Tests cover both mutations and were verified to fail against the previous invalidation. |
||
|
Waleed
|
36aace765e |
fix(auth): correct callback URL resolution across SSR and hydration (#6217)
* fix(sso): derive the SSO callback URL during render
`callbackUrl` was seeded into `useState('/workspace')` and overwritten from
a `useEffect` that read `searchParams`, so the first painted frame always
carried the default. On any deep link with `?callbackUrl=`, the "Sign in
with email" and "Sign up" links briefly pointed at `/workspace` instead of
the requested destination, and a click landing in that window navigated to
the wrong place.
- derive `callbackUrl` from `searchParams` during render; the validation
gate is unchanged, so an off-origin or malformed value still falls back
to `/workspace`
- keep the warning for a rejected value in an effect, now keyed on the
param itself rather than the `searchParams` object, so it fires once per
actual change instead of once per identity change
- add first-frame tests via `renderToString`, which runs no effects and so
pins exactly the window the old code got wrong
* fix(auth): resolve callback URLs against the app origin server-side
`validateCallbackUrl` compared against a sentinel base
(`https://callback-url-validator.invalid`) when `window` was undefined, so
the server rejected every absolute URL — including the same-origin ones the
function documents as valid. A component deriving a callback URL during
render therefore produced one destination in the SSR markup and a different
one after hydration.
The exposure was not new to the SSO form: `login-form.tsx` and
`signup-form.tsx` already derive their callback URL during render on
`force-dynamic` pages, so both carried the same divergence.
- resolve against the deployment's own origin server-side, so the server
reaches the same verdict the browser will after hydration
- fall back to the sentinel when the app URL is unset or unparseable, which
keeps the server fail-closed: absolute URLs are rejected, as before
- cover the absolute same-origin case and the unset-app-URL fallback in the
existing suite; all 15 open-redirect rejection cases are unchanged
* fix(env): drop the getBaseUrl browser-origin fallback
The fallback was added in #6214 as a safety net while the real cause — the
hosted env script losing its `beforeInteractive` strategy — was fixed in the
same PR. With the injection ordering restored, `window.__ENV` is populated
before hydration, so the fallback is unreachable in any correctly configured
deployment.
Guessing the origin was also unsafe in the one case it could still fire. An
opaque origin — a sandboxed iframe, and `/chat/*` is deliberately embeddable
— serializes to the string `'null'`, which is truthy, so `getBaseUrl()` would
have returned `'null'` and every call site would have silently built
`null/api/...`. A throw surfaces the misconfiguration instead of encoding it
into request URLs.
- restore the unconditional throw when NEXT_PUBLIC_APP_URL is unset or blank
- mirror it back in the shared testing mock
- flip the two fallback tests to assert the throw, keeping whitespace-only
coverage
Server-side behavior is unchanged: there was never a `window` to fall back
to, so callers that already guard `getBaseUrl()` (`getBaseDomain`,
`validateCallbackUrl`) keep their existing fail-closed paths.
|
||
|
Waleed
|
09eba8a0f9 |
improvement(ci): move CodeQL off default setup onto Blacksmith (#6219)
* improvement(ci): move CodeQL off default setup onto Blacksmith Default setup pinned every scan to a 4-vCPU GitHub-hosted runner with no cancel-in-progress: PR scans ran 30-125 min and re-ran on every push (#6183 burned six overlapping runs). None of that is reachable from the settings UI, so the config moves into the repo. - Blacksmith 8-vCPU via the same CI_PROVIDER escape hatch as ci.yml - cancel-in-progress scoped to pull_request so push/schedule scans finish - push to main + PR to main/staging + nightly safety net - paths filter so doc-only PRs skip the run entirely - paths-ignore config drops tests/mocks/fixtures: 12,716 -> 11,128 files - languages: javascript-typescript + actions; python dropped (7 files in tree) Default setup has been disabled; the two cannot both be active. * fix(ci): restore CodeQL coverage of the data-drain test route Review round 1. - Drop the '**/test/**' and '**/tests/**' globs. A `test` directory is a routable Next.js path segment, and those globs excluded apps/sim/app/api/organizations/[id]/data-drains/[drainId]/test/route.ts — a POST handler that authorizes, decrypts destination credentials and makes an outbound request. CodeQL paths-ignore has no `!` negation to carve it back out, and the globs only covered 76 of 12,716 files. - Add `ready_for_review` to the pull_request activity types. It is not a default type, so a PR opened as a draft and later marked ready was skipped by the draft guard and never rescanned until the next push. |
||
|
Waleed
|
83988c1ebb |
refactor(chat): drop code the voice removal left unreachable (#6218)
Full-file cleanup pass over the files voice mode was removed from. - /api/speech/token: with the chatId branch gone, billingAttribution is always resolved, so four `billingAttribution ? ... : ...` guards and the checkActorUsageLimits fallback were unreachable. Removed, along with the now-unused imports and test mock. - chat.tsx: the inputValue state had one remaining writer (`''`) and no reader — ChatInput has owned its own input value since it went uncontrolled, and removing the voice-transcript caller left handleSendMessage always receiving an explicit message. - use-chat-streaming: messageIdMap was written in three frame handlers and never read, and setIsStreamingResponse was returned but never destructured by the only consumer. - Comments: dropped JSX section labels that restate the element beneath them and TSDoc that restates the identifier; narrowed the speech contract's workspaceId doc, which existed to contrast with the removed chatId field. |
||
|
Waleed
|
78740c05b6 |
chore(chat): remove deployed-chat voice mode (#6215)
Removes the voice-first interface and TTS playback from the deployed chat, keeping workspace dictation, which is a separate feature. - Deletes the VoiceInterface UI and its particles canvas, the chat mic input, the TTS audio-streaming hook, the /api/proxy/tts/stream relay and its contract, and the voice-settings query hook. - Unpicks the voice wiring in chat.tsx and use-chat-streaming: the audio stream handler, sentence-splitting for speech, voice-first mode state, and the isVoiceInput plumbing through ChatInput. - Drops the now-dead chatId branch from /api/speech/token. It was the anonymous public-chat path; with no caller left it would have stayed an unauthenticated relay spending the platform key. That leaves resolveDeployedChatCaller unused, so it goes too. - Removes code the above orphaned: MAX_CHAT_SESSION_MS, the noop util, the audio/position refs in use-chat-streaming that were only ever written, and the chatId field on the speech contract. Keeps /api/settings/voice, lib/speech and use-speech-to-text: the workspace home input still uses them. Keeps the voice-output usage source, enum and label — Postgres cannot drop an enum value, and historical usage_log rows still need a label to render. |
||
|
Waleed
|
dd7951563e |
fix(env): restore beforeInteractive on the hosted public env script (#6214)
The hosted `<PublicEnvScript>` rendered a plain `<script>`, which lands at the end of `<head>` — after the ~40 `<script async>` chunk tags Next emits at the top of the document. An async script runs as soon as its fetch resolves, so on a warm cache a Next chunk could execute (and hydration begin) before the parser reached the env tag, leaving `window.__ENV` undefined for the first render. That surfaced as "Something went wrong" on the workflow page, since `getBaseUrl()` throws during the deploy modal's render, and as the socket falling back to the page origin instead of NEXT_PUBLIC_SOCKET_URL. Regressed in #5522, which replaced next-runtime-env's PublicEnvScript (to avoid its unstable_noStore forcing dynamic rendering) with a static equivalent that dropped the beforeInteractive strategy. - render the library's own `<EnvScript>`, which defaults to beforeInteractive and does not call unstable_noStore — hosted and self-hosted now share one implementation and one loading strategy - drop the hand-rolled serialization and `<` escaping; Next's beforeInteractive path already runs the payload through htmlEscapeJsonString, which escapes `& > < U+2028 U+2029` - fall back to the browser origin in getBaseUrl() rather than throwing, so a missing injected env can never tear down a page through the error boundary; server-side callers still fail loudly - read NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED via getEnv() in the SSO form, matching login/signup/auth-modal — `env.X` returns the build-time placeholder, not the runtime value |
||
|
Waleed
|
0bc4fb4656 |
fix(security): meter and throttle the deployed-chat TTS relay (#6212)
* fix(security): meter and throttle the deployed-chat TTS relay POST /api/proxy/tts/stream treated "a live public chat exists" as authorization to spend the platform ElevenLabs key. A public chat id is handed to every visitor, so any anonymous caller could synthesize speech with no length cap, no rate limit and no usage accounting. Bring the relay in line with its STT sibling (/api/speech/token): - Resolve the chat's workspace and bill synthesized characters to that payer via a new `voice-output` usage source, so spend is attributable and counts against the plan's usage limit (402 once exceeded). - Throttle per IP before any database work, and per chat afterwards, to bound both one caller hammering many chats and many callers hammering one chat. - Cap `text` at 2000 characters and allowlist `voiceId`/`modelId`, so the caller can no longer choose an unbounded charge, a premium or cloned voice, or the billing model. - Drop `Access-Control-Allow-Origin: *`, which let any third-party page read the audio; deployed chat and the Office embed are same-origin. * fix(security): correct TTS metering, pricing and body cap Follow-up review of the previous commit found five defects in it: - Usage rows collided. `usage_log.event_key` is unique and inserts are conflict-do-nothing, and the key is derived from the entry's stable fields. With no explicit sourceReference, two synthesis calls of equal character count in the same workspace produced the same key, so every repeat length went unbilled — defeating the metering this change is for. Each call now carries a unique sourceReference. - Priced at $0.10 per 1k characters, twice the published ElevenLabs Flash/Turbo rate of $0.05, which would have overcharged customers 2x. - No body cap, so an anonymous caller could make the route buffer up to the shared 50 MB default before validation. Now 16 KB, as the STT sibling does. - Threshold settlement ran per sentence: several queries and a possible Stripe call on a realtime path. The workflow execution that produced the text already settles the payer. - The per-IP bucket was described as preventing database amplification. getClientIp trusts the leftmost X-Forwarded-For, so an attacker rotates past it; the comment now says the per-chat bucket is load-bearing. * refactor(chat): share the deployed-chat auth gate across voice routes Review of the previous commits surfaced duplication and one more gap: - The TTS and STT routes had grown near-identical copies of the chat auth + payer lookup. Extracted to resolveDeployedChatCaller, so the gate and the payer resolve together and cannot drift per route — that duplication is how the unmetered TTS path shipped in the first place. - Neither copy filtered chat.archivedAt, so an archived chat could still authorize spend against its former owner's workspace. The shared lookup now filters it, fixing both routes at once. Note: not covered by a test — the db chain mock does not evaluate WHERE clauses, so an assertion here could not fail. - Replaced the route's hand-rolled 429 builder with the existing enforceIpRateLimit helper, and added enforceChatRateLimit alongside the per-user/IP/workspace helpers. Gains the standard Retry-After and X-RateLimit-Reset headers plus throttle logging. - Dropped a test that asserted a module the route no longer imports was never called: it could not fail. - Narrowed the contract: unexported the single-use allowlists and dropped .passthrough() now that the body is a closed shape. * fix(security): fail closed when voice-output usage cannot be recorded Review round 1 findings: - A ledger write failure previously logged and streamed the audio anyway, leaving the spend unrecorded and the payer's usage understated. The caller is anonymous, so serving audio we could not charge for is the unmetered spend this route exists to prevent — it now returns 500. - Use generateId() from @sim/utils/id rather than crypto.randomUUID, per the AGENTS.md ID rule. generateId returns a full UUID v4, so the per-call uniqueness the usage_log event_key depends on is unchanged. * fix(chat): split long TTS text so the relay cap cannot drop audio The client sentence-splits on Western `.!?` only, so text that never matches — CJK punctuation, or a list with no terminal punctuation — accumulates and is flushed as one block at the end of the stream. Against the new 2000-character relay cap that block is rejected and the whole message plays no audio, a regression introduced by adding the cap. Split to cap-sized pieces at the single point that enqueues synthesis, so both the per-sentence path and the end-of-stream flush are covered. Prefers a whitespace or CJK punctuation boundary, falling back to a hard cut when a block has none. The server cap stays as the enforcement point. * fix(security): release the vendor stream when metering rejects the request The fail-closed branch returned 500 with the ElevenLabs response body still open, so synthesis and download kept consuming vendor and runtime resources for a caller that was already rejected. Cancel it before returning, and assert the cancellation in the test. |
||
|
Waleed
|
806ea0c5bb |
fix(tooltip): stop text blurring for ~150ms every time a tooltip appears (#6211)
* fix(tooltip): remove velocity skew/scale that blurred text on every appear The tooltip animated a fractional scale() + skew() over 150ms on the element containing its text. Chrome promotes the bubble to a compositor layer for the transition, rasterizes the text once at the pre-transition scale, then GPU-resamples that bitmap for the duration — so text rendered blurry until the transition settled and the layer re-rasterized at 1:1. It fired on every appear: a pointer entering a trigger is by definition moving, so the first pointermove after pointerenter always set a non-zero skew and a fractional scale. - drop the velocity-reactive skew/scale flourish and the pointer-velocity bookkeeping that existed only to feed it - round tooltip position to whole pixels; clientX/clientY are fractional on HiDPI/zoomed displays, leaving the bubble on a subpixel boundary - drop the dead `filter` from the transition list — nothing ever set a filter - skip the state update when the rounded position is unchanged, so pointer jitter no longer re-renders every Tooltip.Trigger/Content consumer The 150ms ease-out translate is kept, so the bubble still trails the cursor. * improvement(tooltip): keep the velocity flourish, drive it without a CSS transition Restores the velocity-reactive skew/scale removed in the previous commit. The flourish was never the problem on its own — handing it to a CSS transition was. An interpolated fractional scale makes the compositor rasterize the tooltip's text once and resample that bitmap for the duration, which is what read as blur. Applied as a static value per pointer event instead, so every frame is rasterized at its own scale: - split the transform across the individual `translate`, `scale`, and `transform: skew()` properties, and transition only `translate` — position still eases toward the cursor, the flourish no longer interpolates - smooth the pointer velocity in JS (low-pass filter) to replace the smoothing the CSS transition used to provide, so the squish still ramps rather than snapping between raw per-event velocities - quantize the flourish to 3 decimals so jitter below the visible threshold settles instead of re-rendering every consumer Whole-pixel position rounding and the redundant-update bail-out are unchanged. * fix(tooltip): don't seed pointer velocity from the trigger box on focus The previous commit routed `onFocus` through a shared reveal helper that seeds `lastPointerRef` from the coordinates it is given. For focus those are the trigger's box center, not the pointer — so if the pointer already happened to be over the trigger, the next `pointermove` measured the box-to-cursor delta as velocity and spiked the skew/scale flourish. Split the helper in two: reveal-from-pointer seeds velocity tracking, reveal-from-element leaves it cleared. Restores the pre-PR behavior, where focus explicitly nulled the pointer snapshot. Caught by Cursor Bugbot. * fix(tooltip): make the flourish smoothing frame-rate independent The velocity low-pass filter applied a fixed coefficient per pointer event, so how fast the squish settled depended on how fast the device emitted events — 233ms at 30Hz down to 29ms at 240Hz, an 8x spread for the same gesture. It was also far snappier than the 150ms CSS ease-out it replaced, so the flourish read as twitchier than before. Derive the coefficient from the real elapsed time instead (1 - exp(-dt / tau), tau = 50ms). Settling is now flat at ~150ms from 60Hz upward, matching the duration of the transition this stands in for. Also separates the smoothing delta from the velocity-normalization delta: the latter is still floored at one frame to keep a 1ms event from reporting an enormous velocity, but flooring the former was itself a source of frame-rate dependence below 16ms. Verified against Chrome's documented re-raster behavior: a layer is re-rastered at its new scale when the scale changes via script, but not when a declarative animation interpolates it, which is why the flourish must stay out of the transition list. https://developer.chrome.com/blog/re-rastering-composite |
||
|
Waleed
|
6806fc2f41 |
fix(resources): sort folders and resources as one list so pinned items reach the top (#6213)
* fix(resources): stop hoisting folders so pinned items reach the top Folders and their sibling resources were sorted as two lists and concatenated folders-first, so a pinned file, table, or knowledge base could never rank above an unpinned folder — pinning only reordered within each partition. Sort them as one list instead: pinned -> sort key -> name, in a shared `sortResources` comparator. Rows with no value for the active column (a folder has no row count or token count) sort last in both directions, and the name tiebreak is never inverted by `desc`. Also aligns the surfaces that browse the same resources: the mothership resource trees, the collapsed sidebar flyout (whose nested levels partitioned folders-first while its own root interleaved), the sidebar file tree, and the recently-deleted tiebreak. Files' sort params were nullable only to encode "folders name/asc, files updated/desc"; with one list that state is gone, so they default to updated/desc like Tables and Knowledge. * improvement(resources): prefetch pinned ids and members with the resource lists Files, Tables, and Knowledge already prefetched their items and folders, but not the two lists a complete row needs. Pinned ids are the list's primary sort key now, so a page that painted before they arrived rendered the whole list in the wrong order and then visibly re-sorted. Members back the Owner column, which painted empty and filled in after. Both now hydrate alongside the lists via `prefetchResourceListChrome`. Pinned keys move to `hooks/queries/utils/pinned-item-keys` so the server prefetch can address them without pulling the contracts barrel and the optimistic-mutation machinery into the route. * chore(knowledge): drop the dead pre-redesign knowledge base sort `sortKnowledgeBases` was a second, complete ordering for knowledge bases (name/createdAt/updatedAt/docCount) left over from the old sort dropdown, with no consumers since the shared Resource sort menu replaced it — the kind of duplicate that silently diverges from the page it shadows. Its `SortOption`/`SortOrder` types and the `SORT_OPTIONS` list that fed it were dead with it. `utils/sort.ts` now holds only a filter, so it is renamed to `utils/filter.ts`. * fix(resources): sort unknown owners last and align the knowledge sort menu An owner id that resolves to no workspace member renders an empty cell, but its sort key was `''`, so those rows floated to the TOP of an ascending Owner sort while every other valueless cell sorted last. They now key `null` and follow the same nulls-last rule on all three pages. The Knowledge sort menu also listed Owner after Last Updated while its column order — and Tables' menu — put Owner before it. * chore(knowledge): delete the dead base-card grid view `BaseCard`/`BaseCardSkeleton`/`BaseCardSkeletonGrid` were the pre-redesign grid-card knowledge base view, kept alive only by the barrel re-export — no call sites since the Resource list replaced it. Removing it orphans `components/constants.ts` entirely (its sort types went with the dead sort; its three class-name constants had no consumers left), so that goes too. |
||
|
Waleed
|
feaddc437a |
improvement(headers): one action cluster and one ordering app-wide (#6210)
Extends the settings-header ordering past detail pages. Five headers hand-rolled their own action wrapper. `resource-header` used `flex shrink-0 items-center` — no height, no gap, so chips on tables, files, knowledge, logs and scheduled tasks sat flush against each other; the integrations tab strip and the integration block detail each used `ml-auto flex items-center`. Only the settings shell and credential detail wore the intended `flex h-[30px] items-center gap-1`. That string is now HEADER_ACTION_CLUSTER, next to PAGE_HEADER_BAR, and all five compose it. `Resource.Header` now ranks its actions through orderHeaderActions too, so the resource pages inherit the same order as settings rather than rendering their array verbatim. `ResourceAction` gains `id`, which was the only field keeping it from being a subset of `SettingsAction`. Delete is now ranked by its `id` rather than by where the caller put it. That matters for a header with no primary action: the file detail listed `Download → Share → Delete`, leaving a destructive chip in the slot a primary would occupy. Tagging it `id:'delete'` fixes that without inventing a primary. |
||
|
Waleed
|
b741176d4b |
improvement(settings): one header action order across detail pages (#6206)
Detail headers disagreed on where Delete sits. The skills page reads
`Share → Delete → Discard → Save`, but every SettingsPanel page spread
saveDiscardActions() first and appended Delete, rendering it to the RIGHT of
the primary chip: sandboxes, custom tools, custom blocks, permission groups and
data retention all did this.
Fixed in the shell rather than at nine callsites. orderHeaderActions() ranks
actions — secondary, then `id:'discard'`, then `variant:'primary'` — stably
within each band, so writing the array the natural way now produces the right
header and a new detail page cannot get it wrong. This generalizes past Save:
workflow MCP servers' `Add workflows` primary is now right-most with Delete
before it, instead of the reverse.
The ranking has to survive one trap. The shell routes onSelect through
configRef.current.actions[index] to avoid stale closures, so reordering the
render without preserving the source index would bind every chip to the wrong
handler — clicking Delete would Save. orderHeaderActions carries {action,index}
pairs; settings-header-shell.test.tsx pins that at the render level, including
the conditional-Discard case where a missing action shifts every index.
Delete is also now a plain chip on the nine resource-detail headers, matching
skills, each with a stable `id:'delete'` (three lacked one, so the chip
remounted when its label flipped to Deleting...). `variant:'destructive'` is
kept for actions destructive at scale — Delete all passwords, Clear all
browsing data, Sign out all members — which the confirm modal does not cover
the way it covers removing the single resource you are looking at.
|
||
|
|
03333ce30d |
feat(library): AEO vs GEO: What Answer Engine and Generative Engine Optimization Actually Mean (#6204)
* feat(library): AEO vs GEO: What Answer Engine and Generative Engine Optimization Actually Mean * feat(library): add generated cover for AEO vs GEO post --------- Co-authored-by: Sim Pi Agent <pi@sim.ai> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Vikhyath Mondreti
|
f07160aec7 | fix(trigger): deps to include e2b and daytona (#6205) | ||
|
Waleed
|
030c4e2a5e |
refactor(auth): extract connector definitions out of auth.ts (#6203)
auth.ts had grown to 3,927 lines, of which ~2,340 were the genericOAuth
connector list — the OAuth apps a workspace connects tools to, as distinct from
the handful of providers used to sign in to Sim. Adding a connector meant
editing the same file that configures sessions, database hooks and Stripe.
Moves that list to lib/auth/connectors/providers.ts behind
buildConnectorProviders(), and relocates getMicrosoftUserInfoFromIdToken to
lib/oauth/microsoft.ts alongside the three Microsoft helpers it already depends
on. auth.ts drops to 1,489 lines and reads as auth configuration again.
Pure move, verified mechanically: the connector array is token-identical after
stripping whitespace, and all 179 template literals emit byte-identical strings
(the one apparent diff was reindentation inside a ${} expression, not text).
Behavior, evaluation order and log scopes are unchanged; the array is still
built once, when betterAuth() runs.
The explicit GenericOAuthConfig[] return type is required, not cosmetic —
inline, the entries were contextually typed by the config property. Without it
prompt: 'consent' widens to string and every getUserInfo parameter becomes
implicitly any.
|
||
|
Waleed
|
14d9542f2a |
fix(credentials): capture the correct provider identity on connect and rotate (#6201)
* fix(credentials): capture the correct provider identity on connect and rotate Attio OAuth recorded an arbitrary workspace member instead of the authorizing user, so two members connecting under one Sim user collapsed into a single account row via the stale-sibling dedupe. Notion read `profile.person.email`, which never exists on a bot token. Synthetic connector emails were minted on live third-party domains. Google service-account rotation left the credential labeled with the old key's client_email and skipped audit metadata entirely. Box and Salesforce identity lookups failed silently with no logger in either file. Service-account principals are now a single ServiceAccountPrincipal union (user / tenant / lookup_failed / null) mirrored centrally into both audit and stored metadata, so a principal can no longer be captured and forgotten, and "which account is this credential?" is answerable from SQL. * fix(credentials): keep the provider-reported name when identity lookup degrades Box and Salesforce returned early on a missing user id, discarding a `name` or `login` the response did carry and relabeling the credential to the enterprise or host fallback. Only the principal should degrade; the human label still beats an id-derived string. Also notes the Salesforce `openid` scope in the connect help text. The client credentials minter sends no scope parameter — effective scopes come from the customer's Connected App — so without `openid` the userinfo lookup can 403 and the run-as user silently never reaches the audit record. |
||
|
Waleed
|
786c854e04 |
improvement(settings): consolidate resource UI onto shared primitives (#6202)
* improvement(settings): consolidate resource UI onto shared primitives Sandboxes, MCP tools, and MCP servers each modeled their list rows and detail headers differently. Collapse them — and the surfaces they drifted from — onto one set of primitives. SettingsResourceRow now owns the row entirely: tile, title/subtitle tokens, padding and bleed, hover band, hit area, focus ring, and the one navigation chevron. Adds onClick/href (a stretched overlay, so interactive trailing controls keep their clicks), navigable, iconVariant='custom', and a badge slot for decoration that must not swallow row clicks. Rows that open a detail page get a chevron and a whole-row click; flat records keep the "..." menu. Delete moves to the detail header as a destructive chip behind a confirm modal — sandboxes previously deleted with no confirmation at all. Also folds in SettingsField (new), SettingsEmptyState tone='error', RESOURCE_LIST_STACK/GRID, RESOURCE_TILE_PLAIN, and a shared MemberAvatar; deletes DetailIconTile (byte-identical to ResourceTile); and standardizes on the emcn ArrowRight, which is a visibly different glyph from lucide's. * fix(settings): correct row bleed, delete-confirm binding, and avatar seeding Follow-up from review of the consolidation. The sandbox delete confirmation was boolean-only state. Browser Back unmounts the modal without closing it, so opening another sandbox re-opened it already confirmed — against the wrong sandbox. Reset it where the draft guard already handles the same history hazard. RESOURCE_LIST_GRID kept an 8px column gap after the bleed moved onto the row, so neighbouring cards overlapped by exactly the gutter and the right-hand card's stretched hit area won it: clicking between two cards opened the wrong one. Budget the gap for the bleed instead. Also: restore the `group` the template-icon hover outlines depend on; add a `flush` row for headings and overflow boxes; seed MemberAvatar identically on every surface; keep the MCP transport label visible in all row states; give the Delete chip a stable id so it doesn't remount mid-delete; and stop an empty subtitle rendering a phantom line. Docs: widen the rule's path globs to the surfaces it now governs, drop the `text-[14px]` example that contradicted the token rule, and add a Mode C for migrating rows onto the primitive. * revert(settings): keep the dense member roster avatar Consolidating the Teammates/Organization avatar onto the credential member row's was a redesign, not a deduplication. The two encode different things: the roster is a dense list keyed on email where the avatar is a 14px neutral marker, while a member management row carries a name, an email, and a role control and earns a 36px hashed avatar. Merging them made those rows ~70% taller, gave every workspace in a permission group a colour-hashed "avatar" seeded on its name, and cut the Add Members picker from ~7 visible rows to ~5 inside its fixed-height box. Restores both avatars and the containers that owned their bleed, and records in the rule why they stay separate. Keeps only the unrelated fix in that area: the picker row was the one settings row rounded at `sm`. * fix(settings): wire flush, drop the lone skeleton, close review gaps Final review round. Two fixes the previous commit claimed but did not land: `CredentialDetailHeading` never passed `flush`, so every credential detail heading wore list-row padding, and the empty-subtitle guard was never applied. Both were scripted replacements that silently no-op'd. RESOURCE_LIST_GRID also dropped to one column 32px earlier than the grid it replaced — `auto-fit` measures tracks, not margin boxes, so widening the gap for the rows' bleed moved the breakpoint. Track minimum now budgets for it. Removes the BYOK skeleton rather than maintaining a second copy of the row: it was the only skeleton in settings, and it had already desynced from the row it imitates. Its peers render nothing while loading. Also: unify the glyph-tile treatment across MCP/sandboxes/workflow-MCP with custom tools; move decoration out of `trailing` in verified-domains and recently-deleted; convert the last hand-rolled row and empty states in workflow-MCP, api-keys, copilot and group-detail; give copilot's delete the same `...` affordance as api-keys; announce the row description via aria-describedby, which the stretched overlay had silenced; and let SettingsField render its own value so callers stop restating type tokens. Docs: correct claims that predate this PR — `aside` does not exist, the navigation source of truth is under components/, beforeunload mounts in the layouts, `getSettingsSectionMeta` takes two args — and stop asserting a literal-pixel grep returns zero when display type legitimately uses it. |
||
|
Waleed
|
2ccda18f3e |
test(files): collaborative agent-streaming coverage (two-writer, multi-editor, undo, persist round-trip) (#6199)
* test(files): two-writer concurrent-editing regression coverage for agent streaming * test(files): multi-editor + undo + persist round-trip + late-joiner streaming integration coverage * test(files): make full-rewrite two-writer test actually exercise the concurrent peer edit * test(files): assert every peer edit lands (no false-green from a no-op peerInsertNear) |
||
|
Waleed
|
5460133e07 |
fix(files): collapse blank-line runs to markdown standard (no empty-paragraph explosion / reflow) (#6198)
parseMarkdownToDoc now strips ALL top-level empty paragraphs (leading, interior, trailing), not just trailing. A run of blank lines between blocks is insignificant in markdown (CommonMark collapses it), but @tiptap/markdown reconstructs one empty paragraph per blank line — which made the mounted editor render vertical gaps that exist nowhere else the file is viewed (GitHub, download, our own static preview), let a pathological blank run explode into thousands of empty nodes, and caused the visible reflow on open (static preview collapses empty <p>; the live editor gives each a trailing-break line). Collapsing on parse keeps normal one-blank-line spacing, matches every standard renderer, and stays idempotent so the round-trip-safety probe still reaches a fixed point (files stay editable; existing files normalize on next cold-open + save). Serializer is intentionally NOT changed — a global blank-run collapse there would corrupt blank lines inside fenced code blocks. |
||
|
Vikhyath Mondreti
|
746756d8a9 |
improvement(misc): settings organization, chat agent deploy request keys (#6195)
* improvement(misc): settings organization, chat agent deploy request keys * fix subtitle * adjust perms |
||
|
Vikhyath Mondreti
|
b8ec114382 |
improvement(chat): secrets mounting / exposure improvements and controls (#6191)
* fix(copilot): secrets injection into sandbox * improvement(chat): secrets mounting / exposure improvements and controls * fix(secrets): simplify copilot mounting flow * test(secrets): preserve standard tool permissions * fix(copilot): bind workflow tool completions * fix(secrets): preserve own environment keys * fix(copilot): release failed workflow claims * fix(copilot): trust compacted workflow completion |
||
|
Waleed
|
25e609167f |
fix(search): disambiguate tables and knowledge bases by folder (#6192)
* fix(search): disambiguate tables and knowledge bases by folder The Cmd-K search modal listed tables and knowledge bases without the folder breadcrumb workflows and files already showed, and the table, knowledge base, and search-and-replace pickers in the workflow editor rendered bare names -- so two resources sharing a name in different folders were indistinguishable. Extracts the disambiguation the workflow selector already did into shared collectDuplicateNames + disambiguateLabelByFolder, and shares the search row's folder breadcrumb and its memo comparator, which were duplicated between the workflow and file rows. Also routes folder text through filterAndCap's secondary-rank parameter rather than concatenating it into the name, so an exact name match can no longer be outranked by a folder that happens to fuzzy-match. * fix(zoho-desk): use the real Zoho Desk mark on a white tile The icon was a generic headset placeholder drawn in currentColor, so it never resembled Zoho at all. Replaces it with the mark from Zoho's official logo -- wordmark stripped, viewBox set to the mark's own bounding box so it centers -- and moves the tile to white, matching the other brand-mark integrations. |
||
|
Waleed
|
f113b0bc24 |
fix(chat): show table and knowledge base folders in the resource menus (#6190)
Tables and knowledge bases gained folder support, but the chat attachment menu and the embedded resource menu still listed them flat. Both now render the same hierarchy the Tables and Knowledge pages show. Unifies the two near-duplicate workflow/file tree builders into one shared buildResourceFolderTree plus a spec-driven useResourceTreeSections, so all four foldered families go through one code path. Items whose folder no longer resolves now surface at the root instead of vanishing. |
||
|
mzxchandraandWaleed Latif
|
87aeca6f0c |
feat(zoho-desk): add Zoho Desk integration (#6157)
* feat(zoho-desk): add Zoho Desk integration
Add a full Zoho Desk integration: tools, block, icon, and a webhook trigger.
Tools (tools/zoho_desk): list/get/update tickets, list/add comments,
list/get threads, get contact, list organizations, and download attachments
as UserFiles via an internal route. Registered in tools/registry.ts.
Block (blocks/blocks/zoho-desk.ts): operation dropdown, OAuth credential,
an organization selector backed by GET /organizations, per-operation fields,
and BlockMeta templates. Wires the Zoho Desk trigger.
OAuth (zoho-desk provider): authorize/token at accounts.zoho.com with
access_type=offline + prompt=consent; the Desk REST base is derived from the
token response api_domain and persisted so calls honor data residency instead
of assuming desk.zoho.com. Every call sends Authorization: Zoho-oauthtoken and
the orgId header.
Trigger + webhook handler (triggers/zoho_desk, lib/webhooks/providers/zoho-desk.ts):
Sim creates and tears down the Zoho Desk webhook subscription. Inbound events
are verified with JWT RS256 (X-ZDesk-JWT) against the data-center JWKS, ACKed
via the durable queue to meet Zoho's 5s deadline, and fail loudly on
Free/Standard editions that cannot create webhooks.
* fix(zoho-desk): OAuth PKCE, DC scope-marker parsing, SSRF, and e2e fixes
OAuth: forward code_verifier in the custom getToken (PKCE is enabled, so the
exchange must echo the verifier or Zoho rejects the request with invalid_request).
Surface Zoho's error/error_description, which it returns in the JSON body with
HTTP 200, instead of collapsing every failure into "no access token".
Data-center base parsing: better-auth persists Zoho's scopes comma-joined with no
spaces, so the greedy \S+ marker regex swallowed the whole scope list into the
host. Stop the capture at a comma or whitespace in both read sites (token route
and webhook handler), so apiDomain resolves to the real Desk host.
Attachment SSRF: replace the permissive host regex (which accepted attacker
domains like zoho.attacker.com) with a strict Zoho-apex suffix allowlist.
Block: guard Number() pagination so a non-numeric typo can't send NaN; add the
ignoreSourceId -> sourceId loop-guard header to update_ticket (matching add_comment).
Organizations route: surface fetch/Zoho failures with a real status instead of a
200 with an empty list, so the org selector no longer fails silently.
* fix(zoho-desk): webhook creation, attachment naming, and HTML content handling
Webhook trigger (verified end-to-end against a live Enterprise org):
- Omit ignoreSourceId; Zoho rejects a non-Zoho UUID with INVALID_DATA. Drop
the generateId() fallback and its providerConfig persistence.
- Answer Zoho's create-time notification-URL probe via the existing pending
webhook verification mechanism (GET/HEAD matchers) so subscription creation
no longer 405s.
- mapZohoWebhookError now surfaces Zoho's real errorCode / message / field
errors instead of a catch-all edition message, and attaches an HTTP status so
4xx flow through NonRetryableDeploymentError while 429/5xx stay retryable.
- Propagate the real status through deploy.ts so failed creates don't retry-loop.
get_attachment polish:
- Return the downloaded file's name under `name` (ToolFileData key) instead of
`filename`, and derive it (explicit -> Content-Disposition -> URL segment ->
fallback) so attachments are no longer stored as "untitled".
- Gate the add_comment-only `contentType` param so it isn't sent to get_attachment.
HTML content handling (Zoho content fields emit raw HTML):
- Add a Zoho-local html-to-text converter mirroring the Outlook dual-field
pattern: when contentType is 'html', derive a plain-text `contentText`
alongside the untouched raw `content` + `contentType`; plainText mirrors.
- Apply to comments (list/add), threads (list/get), the ticket description
(descriptionText), and the webhook trigger payload.
Trigger org selector: Organization is now a credential-scoped combobox that
lists the connected account's Zoho Desk organizations.
* fix(zoho-desk): review round - DC-base derivation, org-loader resilience, batched-event visibility
- deriveZohoDeskBaseFromApiDomain: preserve an already-regional desk.zoho.<tld>
api_domain instead of falling back to the US (.com) data center, and map the
DC TLD from any zoho(apis).<tld> host - keeps Desk calls in the right data
center for residency.
- fetchZohoDeskOrganizationOptions: wrap the token/org fetch in try/catch and
degrade to an empty list (the org field is a free-text combobox, so manual
entry still works) instead of hard-failing the selector on token/DC/network
errors.
- formatInput: warn (not silently drop) if Zoho ever delivers more than one
event in a single payload.
* fix(zoho-desk): harden attachment download against redirect-based SSRF/token leak
Replace the raw fetch in the attachment route with secureFetchWithValidation
(the same guarded fetch the copilot file-download tool uses). The download URL
is user/LLM-influenced and Zoho may redirect, so auto-following redirects could
send the OAuth token / orgId to an untrusted or internal host. The guarded fetch
pins the resolved IP, blocks private/reserved targets on every hop, drops the
Authorization header if a redirect leaves the origin (stripAuthOnRedirect), and
enforces the 50MB cap while streaming. The strict Zoho apex allowlist still
gates the initial origin as defense in depth.
* fix(zoho-desk): only add the edition hint when Zoho's error indicates it
mapZohoWebhookError appended the "requires Professional edition or higher"
guidance to every 403, but a 403 can also mean a wrong org, a missing scope, or
a bad token. Gate the hint on Zoho's own errorCode / message matching the
permission/edition pattern instead of the bare status, so unrelated 403s surface
Zoho's real reason without the misleading suffix. Adds a test for the
non-edition 403 path.
* fix(zoho-desk): stop duplicating /api/v1 when resolving a relative attachment href
A relative attachment href that already starts with `api/v1` (as Zoho's hrefs
often do) was concatenated onto getZohoDeskApiBase (which ends in /api/v1),
producing `/api/v1/api/v1/...` and a failing download. Extract a tested
resolveZohoAttachmentUrl helper that uses absolute hrefs as-is and strips a
leading slash + `api/v1/` prefix from relative ones before joining, so the path
is correct for absolute, root-relative, and api/v1-prefixed hrefs alike.
* fix(zoho-desk): reject an empty update_ticket PATCH with a clear error
update_ticket built its PATCH body from optional fields via filterUndefined, so
a call with no fields set sent `{}` and surfaced an opaque Zoho failure. Guard
the body builder to throw an actionable "provide at least one field" error
before the request. Adds a test for the empty and populated body paths.
* fix(zoho-desk): fall back to the credential Desk domain in webhook JWT verify
verifyAuth chose the JWKS host from providerConfig.apiDomain and otherwise
defaulted to the US host (desk.zoho.com), so a non-US webhook row missing
apiDomain would verify against the wrong JWKS and reject legitimate events. When
apiDomain is absent, resolve it from the OAuth credential's __zoho_domain__ scope
marker (mirroring deleteSubscription). The persisted-apiDomain fast path stays
DB-free to respect the 5s delivery deadline. Adds tests for both paths.
* fix(zoho-desk): apply the Zoho host allowlist to the organizations route
The organizations route built its URL from the client-supplied apiDomain and
attached the OAuth token without the https-Zoho-host allowlist the attachment
route already enforced, so a session-access caller could point the server at an
arbitrary origin and leak the token. Extract the shared isZohoHost allowlist and
an assertZohoUrl guard into tools/zoho_desk/utils (two consumers now), guard the
organizations URL before fetching, and refactor the attachment route to reuse
the shared helper. Adds tests for the allowlist and guard.
* fix(zoho-desk): propagate provider 4xx in the stable webhook prepare path
The v2 stable deploy preparation flattened every registration failure (except
path conflicts) to HTTP 500, so a provider-attached permanent 4xx - e.g. Zoho's
edition/validation failures from createSubscription - retried instead of failing
the deploy terminally. Propagate the attached status (`?? 500`), matching the
legacy save path's status-aware mapping so both deploy paths route 4xx through
NonRetryableDeploymentError.
* fix(zoho-desk): make createSubscription config failures non-retryable
createSubscription threw plain Errors (no status) for missing orgId, event type,
or credentials, and for a Zoho success with no webhook id - so the deploy outbox
mapped them to 500 and retried permanent configuration failures. Attach a 4xx
via statusError (400 for missing config/credentials; 422 for the no-id anomaly,
where a retry risks duplicate webhooks) so they fail the deploy terminally like
the mapped Zoho API 4xx responses. Tests assert the 400 status on the guard paths.
* fix(zoho-desk): enrich prevState with contentText symmetrically with payload
formatInput derived plain-text contentText only on payload, so an update event
for a comment/thread left prevState as raw HTML while payload carried
contentText - inconsistent shapes for before/after comparisons. Apply
withDerivedContentText to prevState too. Test asserts both are enriched.
* docs(zoho-desk): regenerate integration docs
Regenerate zoho_desk.mdx from the current tool definitions: removes the stale
add_comment `ignoreSourceId` input row (the field was dropped because Zoho
rejects arbitrary values) and adds the derived `contentText` / `descriptionText`
plain-text fields on comments, threads, and tickets.
* fix(zoho-desk): validate the persisted Desk base against the strict host allowlist
deriveZohoDeskBaseFromApiDomain trusted any host matching `desk.zoho.[a-z.]+`,
so a crafted api_domain like `desk.zoho.com.attacker.com` passed and was
persisted as the credential's `__zoho_domain__` REST base - later receiving the
OAuth token on every Desk tool/webhook call. Gate the derivation on the strict
isZohoHost apex allowlist (which rejects that lookalike), extracted with
assertZohoUrl into a dependency-free host-allowlist module so the auth
token-exchange path validates hosts without pulling in the tool utilities. The
attachment and organizations routes now import the shared guard from there.
Also: formatInput now emits the normalized null trigger shape for an empty/
malformed event array instead of leaking a raw `[]` to downstream steps. Tests
cover the empty-array shape and the lookalike-host rejection.
* fix(zoho-desk): correct API field names, scopes, and host validation
Validation pass against Zoho's published Desk API surfaced six defects that
typecheck, lint, and the existing suite all passed over, because each one fails
silently against the live API rather than erroring.
Wire-name mismatches (Zoho ignores unknown keys, so all three were silent):
- update_ticket sent `customFields`; the ticket PATCH body names it `cf`.
`customFields` exists only as a deprecated alias on other Desk resources and
on the separate validate-field-updates endpoint, so updates reported success
and applied nothing.
- ZOHO_DESK_TICKET_PROPERTIES and ZOHO_DESK_CONTACT_PROPERTIES advertised a
`customFields` output; both resources return `cf`. The declared field always
resolved undefined and the real one was undeclared.
- list_tickets sent `departmentId`; the query param is `departmentIds`, so the
department filter was dropped and every department's tickets came back.
Content handling:
- deriveZohoContentText matched `contentType === 'html'`, but Zoho spells the
discriminator per resource: comments use `html`, threads use the MIME form
`text/html`. Every thread's `contentText` was therefore raw markup - the exact
opposite of the field's purpose. Now normalized across both spellings,
parameterized values, and casing, with regression tests.
Scopes (least privilege):
- Desk.tickets.ALL -> Desk.tickets.READ + Desk.tickets.UPDATE. No tool creates
or deletes a ticket; ALL additionally granted ticket DELETE.
- Dropped Desk.search.READ (no search tool exists) and Desk.webhooks.READ /
.UPDATE (the provider only creates and deletes), plus their orphaned
SCOPE_DESCRIPTIONS entries.
Host validation - the webhook provider was the only token-carrying path not
anchored to the Zoho apex allowlist, including the JWKS fetch, where an
unrecognized host would have stood in as the JWT issuer:
- createSubscription, deleteSubscription, and verifyAuth now route their base
through a shared allowlist check.
- getZohoDeskApiBase validates rather than trusting injection precedence.
- The organizations route uses secureFetchWithValidation with
stripAuthOnRedirect, matching the attachment route it had diverged from.
Block and trigger:
- The trigger's department field is renamed `triggerDepartmentIds`; sharing the
`departmentIds` id let a value typed as a list_tickets filter become the
webhook subscription's filter when switching modes.
- `isPublic` no longer serializes onto all ten operations, matching the existing
gating for `contentType`.
- from/limit reject negatives and fractions instead of forwarding them.
- update_ticket gains description, resolution, and classification (all already
declared as outputs), and a departmentId input so a ticket can be moved.
Accuracy corrections to user-facing text, all against the published parameter
tables: `from` is 0-based (0-4999, default 0), not 1-based; per-endpoint limits
are tickets 1-100/10, comments 1-100/50, threads 1-200/100; sortBy lists Zoho's
actual allowed values; the two `include` sets genuinely differ per endpoint;
status and priority accept comma-separated lists.
Also: path IDs are trimmed via requireZohoDeskId so a pasted trailing space
fails with a clear message instead of a %20 404; comment `commenter` and thread
`status`/`isDescriptionThread`/`visibility`/`canReply` are now declared;
ZOHO_CLIENT_ID/SECRET added to the oauth test env; docs page gains a
MANUAL-CONTENT intro covering capabilities, the Professional-edition webhook
requirement, and the US-data-center limitation.
Not verified from documentation, needs a live account before merge:
- the OAuth scope for the attachment content sub-path (Zoho publishes none, and
there is an unanswered SCOPE_MISMATCH report against it)
- 12 of the 17 offered webhook event ids (5 are confirmed); Ticket_Delete is
documented but not offered
- the ticket `descriptionContentType` key, and the POST /api/v1/webhooks body
shape, neither of which appears in any reachable Zoho reference
* chore(zoho-desk): regenerate tool metadata
The param and description corrections in the previous commit changed the
generated tool surface, so tool-metadata:check failed in CI. Regenerated;
the diff is two Zoho-only lines.
* fix(zoho-desk): stop posting null for untouched update_ticket fields
`filterUndefined` strips only `undefined`, but an untouched subBlock never
arrives as `undefined`: the workflow serializer initializes every subBlock value
to `null` (stores/workflows/utils.ts) and extractBlockParams writes those nulls
straight into tool params, with nothing between the serializer and request.body
filtering them.
Reproduced against the real serializer and block with only `status` set:
basic {"subject":null,"status":"Closed"}
advanced {"subject":null,"status":"Closed","priority":null,...,"cf":null}
`subject` leaks even in basic mode because it declares no `mode`, so
shouldSerializeSubBlock never drops it. Zoho documents subject as a writable
field, so every status-only edit either failed the PATCH or blanked the ticket's
subject; in advanced mode the whole update surface nulled out, including `cf`.
Two things hid this. The empty-PATCH guard was unreachable from the block (the
body always carried at least `subject`), and the existing test called buildBody
with fields *absent* rather than null - the shape the block never produces - so
it could not fail on the real path.
Replaces filterUndefined with a local omitUnset that drops undefined, null, and
'' (a cleared input means "leave unchanged", not "set to empty"). Adds three
tests using the real serializer shape, all verified to fail before the fix.
Also fixes the same null-blindness in the block's param mapping, where
Number(null) === 0 injected from=0 on every operation, and corrects the shared
limit placeholder, which claimed max 100 while list_threads allows 200.
* feat(zoho-desk): add Self Client service-account credential
Adds a second way to connect Zoho Desk, alongside the interactive OAuth flow: a
Zoho Self Client, pasted as client id + client secret + organization id. Built
on the existing client-credential-accounts framework rather than a new credential
path, so it behaves like the Zoom Server-to-Server and Box CCG accounts already
in the repo - a short-lived token minted on demand, no refresh token.
Two Zoho behaviors the generic framework does not cover:
- `scope` must be COMMA-separated on Zoho's token endpoint; a space-separated
list is rejected as an invalid scope. The list comes from
getCanonicalScopesForProvider('zoho-desk'), so the Self Client and the OAuth
flow can never drift apart on scopes.
- Zoho reports OAuth failures in the JSON body, frequently with HTTP 200
(e.g. {"error":"invalid_client"}), so the success body is inspected for an
`error` field before the token is read - a status-only check would accept a
failed mint.
deriveZohoDeskBaseFromApiDomain moves out of auth.ts into the dependency-free
host-allowlist module so the minter and the OAuth path share one derivation
instead of duplicating it, and the mint response's api_domain now flows through
to tools as `apiDomain` (the SA branch of the token route previously returned
none, so SA calls would have assumed desk.zoho.com).
Docs: hand-authored zoho-desk-service-account.mdx following the existing
*-service-account.mdx pages, registered in meta.json and in the generator's
keep-list so stale-page cleanup does not delete it.
Known limitation, documented in the descriptor helpText and the docs page:
webhook triggers still require an OAuth connection. Webhook provisioning resolves
credentials through getCredentialOwner/refreshAccessTokenIfNeeded, which is
OAuth-account-only for every provider in the repo - not a Zoho-specific gap.
Unverified from documentation, needs a live Zoho org before merge:
- the `ZohoDesk.` soid prefix. Zoho documents only the syntax
{servicename}.{zsoid} with a single CRM example; no first-party doc states the
Desk prefix. normalizeZohoDeskSoid passes through any value already containing
a '.', so an operator can paste a corrected full soid without a code change.
- whether zsoid is the same identifier as the Desk orgId header value.
- whether the client-credentials endpoint accepts Desk.webhooks.CREATE/DELETE
for a Self Client.
- whether the mint response populates api_domain for Desk (documented for CRM);
if absent the derivation falls back to the US Desk host.
* fix(zoho-desk): derive descriptionText for ticket-shaped payloads
Cursor Bugbot: webhook ticket events reached workflows as raw HTML with no
plain-text sibling. `withDerivedContentText` only looked at `content` /
`contentType`, but ticket resources carry their body on `description` /
`descriptionContentType`, so trigger output disagreed with get_ticket.
The helper now derives both, which also removed two inconsistencies on the tool
side: get_ticket had its own inline copy of the derivation (now one shared
implementation that cannot drift), and update_ticket returned its PATCH response
raw despite the shared output map declaring descriptionText.
`descriptionContentType` remains the one field name unconfirmed in any Zoho
reference. It degrades safely - an absent key makes deriveZohoContentText return
the value unchanged, so descriptionText mirrors description rather than breaking,
exactly as get_ticket already behaved - and it is now one helper to correct if
Zoho names it differently.
* feat(zoho-desk): let the service account pick its data center
Zoho's accounts server is per region, and the integration pinned every call to
the US host. For the interactive OAuth flow that is currently unavoidable -
better-auth's authorize/token URLs are static per provider - but the service
account mints its own token, so the region can simply be chosen. This makes the
Self Client the only way a non-US Zoho org can connect.
Adds an optional `dataCenter` field to the client-credential framework. Optional
matters: ClientCredentialAccountFieldId and ClientCredentialAccountFields are
shared with Zoom, Box and Salesforce, whose descriptors and minters are
unchanged. Blank keeps the previous behavior (US), so existing credentials are
unaffected.
Only us/eu/in/au are offered - the four regions where both the accounts server
and the Desk REST host are confirmed. CA is deliberately absent: Zoho's accounts
docs say accounts.zohocloud.ca while Zoho's own Desk SDK says accounts.zoho.ca,
and the two cannot both be right. JP/SA/CN/UK lack a confirmed Desk host.
The Desk base is now derived from the selected region rather than inferred from
the mint response, which also removes a dependency on `api_domain` being
populated for Desk (Zoho documents it for CRM only). When `api_domain` IS present
and disagrees with the region, it wins - it is authoritative about where the
token actually works - and the mismatch is logged so a mis-selected region is
diagnosable. deriveZohoDeskBaseFromApiDomain gains a `try` variant returning
undefined so an untrusted api_domain can no longer masquerade as an authoritative
US answer and silently override a correct region.
A wrong region fails loudly rather than silently: the minter runs as verification
on both create and reconnect, so the credential is never persisted in a broken
state. Because Zoho reports it as `invalid_client` - a Self Client only exists on
its own region's accounts server - the operator hint for that code now names the
data center as a candidate cause.
Copy is scoped per path rather than blanket "US only": the OAuth service
description, trigger setup instructions, and the docs intro now say which path
each limitation applies to, and the service-account page documents the four
regions with a sign-in-domain to region-code table.
* fix(zoho-desk): strip ticket description HTML, classify body-reported refresh failures
Final validation pass findings.
descriptionText never stripped anything. It was gated on a
`descriptionContentType` discriminator that Zoho does not send: the Ticket_Add
webhook sample ships `"description": "<div>Description</div>"` with no such key,
and the ticket GET/PATCH response field lists have no content-type sibling
either. So get_ticket, update_ticket, and every webhook ticket payload emitted
descriptionText as a byte-identical copy of the raw HTML, while the declared
output promised stripped text.
The tests did not catch it because they fabricated the shape - both fixtures
constructed `descriptionContentType: 'html'`, a key Zoho never emits, proving the
branch works without proving it is ever taken. Ticket descriptions are HTML by
convention, so the strip is now unconditional (html-to-text is a near-identity on
genuinely plain text), an explicit descriptionContentType is still honored if
Zoho ever adds one, and the fixtures now use Zoho's real shape with no
content-type key anywhere.
A body-reported refresh failure was unclassified. Zoho answers a revoked refresh
token with HTTP 200 and `{"error":"invalid_client"}`; refreshOAuthToken only
checked `data.ok === false` (a Slack-ism), so the request fell through to the
"no access token" guard and returned no errorCode. isTerminalRefreshError could
therefore never recognize invalid_client as terminal, the credential was never
marked dead, and every later execution retried a refresh that cannot succeed -
with the user shown "No access token in refresh response" instead of a reconnect
prompt. The body is now classified before the status is trusted, matching what
the token exchange and the service-account mint already did. That guard also
stopped logging the whole response body, which carries live tokens on a partial
success.
Also: an unrecognized dataCenter now fails with a named error instead of quietly
resolving to US and surfacing as an opaque invalid_client (blank still means US);
the webhook JWKS cache is bounded, since its key derives from a providerConfig
field that SYSTEM_MANAGED_FIELDS protects from diffing but not from being
written; and the attachment `size` output no longer asserts bytes, a unit Zoho
documents as KB.
* feat(zoho-desk): canonical selectors and BlockMeta skills
The block picked its organization with an ad-hoc `combobox` + `fetchOptions`.
Only five blocks in the repo did that, and the other four are core blocks
(agent/credential/function/logs) - no other OAuth integration used it. Every
other resource a user has to identify was a bare short-input taking an opaque
numeric id.
Zoho Desk now uses the same machinery as the other 25 selector providers:
hooks/selectors/providers/zoho-desk/selectors.ts registered in the selector
registry, consumed from the block as basic selector + advanced manual input
sharing one canonicalParamId, for organization, update-ticket department, and
the list-tickets department filter. The trigger's org field moves to the same
selector. zoho-desk-org-options.ts is deleted rather than left beside the new
path, so blocks/ has zero fetchOptions usages outside the core blocks.
Wire params are unchanged (orgId, departmentId, departmentIds, assigneeId,
ticketId, contactId) - this is a UI change, not an API change.
The organizations route now resolves the credential server-side. It previously
had the browser fetch an access token and POST it back, which an earlier audit
flagged as the one place a Zoho token left the server; the new selector-credential
resolver keeps it server-side for both the OAuth and service-account credential
types and re-anchors every outbound host to the Zoho apex allowlist.
No agents selector: the endpoint is documented but its OAuth scope is not, and
the nearest evidence points at Desk.agents.READ, which we do not request. Adding
it would force every existing Zoho Desk user to reconnect for a convenience
field, so assigneeId stays a manual input until the scope can be confirmed
against a live org.
Adds the skills array BlockMeta was missing - 227 of 300 blocks declare one and
this did not. Seven skills, each grounded in a use case Zoho or the ecosystem
actually advertises (auto-triage, SLA escalation, digest, AI draft reply,
customer context, engineering handoff, knowledge-gap report) and each exercising
only tools in tools.access. CSAT surveys, ticket creation, dedup and keyword
search were deliberately left out: the integration has no tool for them, and a
skill implying an unsupported action is worse than a shorter list.
* feat(zoho-desk): agents selector and free-text trigger organization
Three improvements that were previously deferred only to avoid forcing existing
users to reconnect or orphaning saved workflows. This integration is unmerged and
has no users, so the constraint does not apply and the better option wins.
assigneeId was the last field still asking for an opaque numeric id. It is now a
canonical selector pair backed by a new zoho_desk.agents selector, which required
adding the Desk.agents.READ scope - the reason it was skipped before. Route
follows the departments one exactly: auth before parseRequest, host anchored to
the Zoho apex allowlist, secureFetchWithValidation with stripAuthOnRedirect, and
a page drain capped at 20 pages with 204 treated as end-of-list.
Scope caveat: Zoho publishes no explicit scope line for the list-all
GET /api/v1/agents. Every other endpoint in the Agents module documents
Desk.agents.READ (get by id, get by email, roles/{id}/agents), and it is the only
agents-module scope Zoho defines, so that is the basis. Inference across a module
rather than a direct quote - worth one live call before merge, same as the
existing attachment-scope note.
The trigger regained free-text organization entry, lost when the org field became
a selector. The earlier concern - that a manual value would land under its raw
subBlock id and never reach the provider - turned out not to hold: buildProviderConfig
already collapses canonical pairs and writes the active member under the canonical
key. The real gap is narrower and does exist: when canonicalModes pins the group
to basic while only the manual field has a value, the collapse deletes the
canonical key even though the required-field check passes, so the deploy succeeds
and then fails at subscription time. resolveConfigOrgId closes that, with a test.
The block/trigger `orgId` id overlap stays shared, now with a comment. Two earlier
audits disagreed; renaming turns out to be the wrong call. buildCanonicalIndex has
an explicit guard for trigger-mode reuse and blocks.test.ts codifies it as a valid
pattern, orgId means the same portal in both modes (unlike departmentIds, which is
correctly distinct), and a separate triggerManualOrgId would put two advanced
members in one canonical group - getCanonicalValues takes the first non-empty, so
a stale tool-mode value could silently supply the trigger's organization.
* fix(zoho-desk): make the attachment cap reachable, unbreak selector paging
Final audit round.
The 50 MB attachment ceiling could never be hit. This route returns the file as
base64 inside its JSON body, and the executor reads internal tool responses
through readToolResponseBody, capped at 10 MB. Base64 inflates 4/3, so ~7.5 MB
of raw bytes is the real ceiling - and the old limit meant a larger attachment
was downloaded, encoded and serialized in full (peaking near 250 MB of live
allocation, with nothing bounding concurrent downloads) purely to be rejected
afterwards. The cap is now the reachable size, so the limit enforces itself while
the bytes are still streaming, and an overflow returns 413 with the actual
ceiling instead of a generic 500. Raising it properly means uploading in the
route and returning a file reference, as the WhatsApp media route does - not a
bigger constant.
Selector paging assumed a 0-based `from`. Zoho's docs contradict themselves:
the pagination section says "range 0-4999, default 0" while the listing examples
read as 1-based ("from=5 and limit=50 retrieves records 5 to 54"). Under the
1-based reading, stepping by exactly the page size re-fetches the boundary record
and the dropdown shows a duplicate per page. Rather than pick a base that cannot
be confirmed without a live tenant, the department and agent drains dedupe by id,
which is correct under either reading.
The organization list was unpaginated, and Zoho's listing APIs default to ten per
page. An account with more accessible portals silently got a truncated dropdown,
and since every other selector and every tool call is gated on orgId, a missing
portal was unreachable except through the advanced manual field. Both the
selector route and list_organizations now request the documented maximum.
Docs: regenerated so the trigger table includes manualOrgId, and two
service-account claims are hedged to match what the code already says it cannot
verify - that zsoid equals the Desk orgId header value, and that every tool works
under the requested scopes (Zoho publishes no scope for the attachment content
sub-path).
Also: status and priority move out of advanced mode - they are the fields most
often changed on a ticket update; the custom-fields wand prompt now ends with the
required "Return ONLY" clause; and the shared-orgId rationale comment cites the
mechanism that actually applies (buildCanonicalIndex dedupe plus the first-non-
empty rule in getCanonicalValues) rather than a blocks.test.ts branch that never
evaluates this pair.
* fix(zoho-desk): five-audit round - serializer trigger-advanced leak, scopes, paging
Five independent audits (OAuth/scopes, tools-vs-docs, block/selectors,
blast-radius, /validate-trigger). Findings, most severe first.
A trigger-mode field was a live tool-mode required param. `shouldSerializeSubBlock`
excluded `mode: 'trigger'` but not `'trigger-advanced'`, so the trigger's required
`manualOrgId` validated on every tool operation. Reproduced against the real
serializer: with the Organization field pinned to advanced, running
List Organizations failed with "Missing required fields: Organization ID" - a
field that operation does not even render, and which the user could not clear
without switching operations. Fixed in the serializer rather than locally,
because the Google Sheets/Drive/Calendar pollers have the identical shape.
`limit=200` on /organizations was an undocumented parameter I added by
extrapolating from /departments and /agents. Zoho documents NO parameters for
that endpoint and its sample is a bare GET; the other siblings cap at 100 and
Zoho answers out-of-range with 422. Since orgId gates every tool and both other
selectors, a 422 there would have made the whole integration unreachable. Reverted
to Zoho's documented shape.
`descriptionText` was HTML-stripping plain text. The previous round made the strip
unconditional after finding Zoho sends no `descriptionContentType`, but Zoho's REST
samples show plain descriptions while only the webhook payload is HTML - and the
webhook path runs this over contact/account/department bodies too. html-to-text is
not identity on plain text: it decodes entities and deletes tag-shaped content
("a < b > c", XML snippets). Now sniffs for markup first.
`omitUnset` made every documented field-clear impossible. Zoho's own PATCH sample
uses `"classification": ""` and `"productId": ""` to clear. Dropping `''` meant no
scalar field could be cleared. Now drops only undefined/null - the serializer-null
case it was written for - and forwards `''`.
status/priority leaked between operations. One shared subBlock served both the
list_tickets filter and the update_ticket value, and subBlock values survive an
operation switch, so a filter of "Open,On Hold" could be PATCHed onto a ticket and
an update value could silently filter a later list. Split per operation.
Auth: `invalid_code` added to TERMINAL_ERRORS - it is Zoho's code for a revoked
refresh token, so without it the previous round's refresh fix never actually
dead-flagged the credential it was written for. The shared refresh body-error
branch now also requires `!data.access_token`, so no provider can have a
successful refresh misclassified. The token route now uses the validating
`extractZohoDeskBaseFromScope` instead of a private regex with no https/allowlist
check - that value is injected into every tool call. Scope list falls back to the
requested scopes when Zoho omits `scope`, which would otherwise flag every
credential as needing reconnect. The Self Client mint no longer sends
`aaaserver.profile.READ`, a scope that grant never uses.
Trigger: `includePrevState` now set for every *_Update event, not just tickets -
it defaults to false, so prevState was permanently null for contact/agent/task/
article updates while the trigger advertised it. `departmentIds` is only sent for
events Zoho documents as accepting it, and the field is conditioned accordingly.
Empty filters serialize as `null`, matching Zoho's examples, rather than `{}`.
JWKS fetch bounded to 1.5s - jose's default is 5000ms, exactly Zoho's whole
delivery deadline, and Zoho publishes no retry. The create-time validation POST
fallback is now matched by the pending-verification probe. Ticket_Delete added.
All 17 webhook event ids, the POST /api/v1/webhooks body contract, and the JWT
claim/JWKS specifics are now confirmed verbatim against Zoho's webhook
documentation - previously 12 of 17 events and the entire subscription contract
were unverified.
* revert(zoho-desk): back out both shared lib/oauth changes
Reverting two changes to shared OAuth code because their premise is inferred
rather than proven, and neither meets the bar for touching a path every provider
runs.
`refreshOAuthToken` body-error branch. The premise was that Zoho reports refresh
failures with HTTP 200 and an `error` body. That is documented and empirically
confirmed for the authorization-code EXCHANGE (see the comment on getToken in
auth.ts), but I never confirmed it for the REFRESH grant specifically - and if
Zoho returns a proper 4xx there, the existing `!response.ok` path already
classifies it via extractErrorCode, making the branch dead code that every one
of the ~34 providers still executes on each refresh. A shared branch whose only
justification is an unverified inference about one provider is not worth its
blast radius.
`invalid_code` in TERMINAL_ERRORS. Same problem, worse downside: the code is
sourced from a Zoho community post rather than official docs, TERMINAL_ERRORS is
consulted for every provider, and a false positive marks a credential dead for an
hour. Not adding it simply preserves today's behavior (retry rather than
dead-flag), so reverting costs nothing that was previously working.
Both are cheap to reinstate, correctly scoped, once a live Zoho account shows
what a revoked refresh token actually returns.
Kept: the token-redaction on the "no access token" warn, which is an unambiguous
improvement independent of Zoho.
Also kept, deliberately, is the serializer `trigger-advanced` exclusion - that one
rests on a reproduced bug rather than an inference, and it aligns the serializer
with the convention the rest of the codebase already follows (blocks.test.ts
treats `trigger` and `trigger-advanced` identically in six places, as does the
copilot block-metadata tool, and blocks/types.ts documents trigger-advanced as
"the advanced side of a trigger field").
* fix(zoho-desk): carry the stored data center through a credential reconnect
A reconnect rebuilds the service-account secret blob from the submitted fields
only, and the connect modal never prefills - correctly, since for every other
field in this family the stored value is a secret the admin must retype. The
data center is the first non-secret member of that set, so it was being silently
dropped: rotating a client secret on an EU/IN/AU credential moved it back to the
US accounts server, where the next mint fails with an opaque invalid_client.
performUpdateCredential now reads the stored dataCenter out of the existing blob
when the caller does not supply one. The read is failure-tolerant - an
undecryptable or unparseable blob yields undefined rather than throwing, so it
can never block a reconnect, and the provider default applies as before.
Raised independently by three reviewers; I twice argued it was acceptable because
the mint fails loudly rather than corrupting silently. That was true and beside
the point - the operator still had to guess why.
* fix(zoho-desk): delta-audit findings - prevState scope, status leak, HTML sniffer
An audit of the commits the earlier five audits never saw. All four findings are
in code written as fixes for those audits, which is where this branch has
repeatedly introduced new problems.
`includePrevState` was sent for Ticket_Comment_Update. The previous commit gated
it on an `_Update` suffix and claimed Zoho supports it on every update event.
Zoho's webhook doc lists the attribute on Ticket/Contact/Agent/Task/Article update
events but NOT on Ticket_Comment_Update, which documents only `departmentIds`.
That made it an undocumented filter key on a live subscription create - the same
class of risk the same commit reverted `limit=200` for, so it failed that commit's
own stated bar. Now an explicit set rather than a suffix rule.
The status/priority split did not stop the leak it was written for. The mapping
used `operation === 'list_tickets' ? filterValue : updateValue`, whose bare else
covers all eight other operations - so a stale Update Ticket status was forwarded
into get_ticket, list_comments and the rest. Harmless on the wire (those tools
ignore it) but exactly the stale-value pattern the neighbouring gates exist to
prevent. Both fields are now scoped to the two operations that declare them.
The HTML sniffer destroyed plain text. `/<[a-z!\/][^>]*>/` fires on any `<`
followed by a letter with a later `>`, so realistic ticket bodies lost content:
"if x<y then z>0" became "if x0", and "replace <username> with the real name"
lost the placeholder. It now requires a real element - a paired tag, a
self-closing tag, a comment/doctype - or an entity, and the entity arm covers hex
references it previously missed. Regression tests verified by reverting to the
loose pattern and watching them go red.
The reconnect data-center carry-forward is scoped to client-credential providers.
As written it added a DB read plus a decrypt to every service-account reconnect
for every provider - Slack, Atlassian, all token-paste providers - to carry a
field only Zoho has.
Also: the JWKS cache-bound TSDoc had been orphaned onto the wrong constant by an
earlier insertion, and `cooldownDuration` was dropped since it restated jose's
default while only `timeoutDuration` needed justifying.
* test(zoho-desk): cover the webhook subscription filter rules
The subscription filter logic had no test coverage at all, and it is where the
last two rounds both found bugs - includePrevState on an event Zoho does not
document it for, and departmentIds sent to events that accept no filters.
Adds six cases against the real createSubscription: includePrevState is set for
each of the five documented update events and NOT for Ticket_Comment_Update,
departmentIds is kept for a filterable event and dropped for one that is not, and
an event with no filters serializes as null rather than an empty object.
Verified the guard bites: reverting PREV_STATE_EVENTS to the `endsWith('_Update')`
rule turns the Ticket_Comment_Update case red.
The Ticket_Comment_Update assertion checks the with-departments case as well as
the bare one - asserting only `not.toHaveProperty` on the bare filter would pass
vacuously, since that filter is legitimately null.
---------
Co-authored-by: Waleed Latif <walif6@gmail.com>
|
||
|
Waleed
|
69289d26e7 |
fix(execution): stop run-buffer failures from stranding resumed executions (#6187)
* fix(resume): stop a failed run-buffer publish from stranding a resumed execution The run buffer is a replay convenience for stream readers; the durable execution record is authoritative. A failed terminal event publish was deciding the outcome of work that had already run: it threw, the resume was marked failed, and the paused execution was left paused forever. That path is also not retryable — the workflow had already executed — so the next resume attempt re-ran its side effects. Degrade instead. Record the terminal status on the stream meta so readers are not left polling an 'active' stream, log the failure, and let the resume settle on its real result. The same treatment applies when the terminal event is never published at all, which previously synthesized an error for the same stranding effect. Removes the now-unreachable TERMINAL_PUBLISH_ERROR constant and its branch. Pre-execution buffer failures stay fatal and retryable, since no work has happened yet. * fix(execution): give per-user Redis budget keys a fixed window The per-user byte budget refreshed its TTL on every accepted write, so for any user who never went a full TTL without writing, the key never expired. The per-execution data it accounted for kept expiring underneath it, so the counter accrued bytes Redis had already dropped and drifted toward the ceiling. On reaching it, every subsequent write for that user was rejected until they stopped writing for a full TTL — and since a rejected write does not refresh the TTL, it recovered on its own and then refilled. User keys now get a fixed window: the TTL is set when the key is created and never extended. Applied to all five Lua scripts that touch the key so the writers cannot drift apart. Execution-scoped keys keep sliding — they are refreshed on the same schedule as the data they account for, so their counter and bytes stay in step. * fix(execution): heal a user budget key that somehow has no expiry The no-op branch of the flush script dropped the user-key EXPIRE outright rather than guarding it like every other site. No current path can create the key without an expiry, but if one ever did, that branch was the one place that would never give it one — and a user counter with no expiry is the unbounded version of the bug this series fixes. Guard it instead, so the branch heals such a key rather than skipping it, and so all five scripts read the same way. * fix(stream): end a replay cleanly when terminal metadata has no terminal event Terminal metadata is the authoritative end-of-run signal — the happy path writes it atomically with the terminal event, and a run whose terminal event could not be buffered records the status on its own. The reader required both, so the degraded case threw and turned a replay that was merely incomplete into a broken one. Metadata without a matching event means the buffer degraded, not that the run is still going. Log it and close the stream: the reader has already received every event that was buffered, and the durable record is unaffected either way. |
||
|
|
13772565a3 |
fix(chat): show deployment passwords to admins (#6177)
* fix(chat): show deployment passwords to admins
* fix(chat): reject whitespace-only passwords
* fix(chat): preserve password visibility on regenerate
* fix(chat): harden password reveal and close deployment lockout paths
Follow-ups from a security review of the password reveal endpoint. The
permission model itself was correct — the reveal is gated on workspace
admin via the canonical resolver, so derived org-admin access is honored.
These address secret handling and validation around it.
- Cap set-path passwords at the same 1024 chars the chat login accepts.
Neither the input nor the schema bounded length, so a longer password
saved fine and then failed the login POST on length before auth ran,
locking every visitor out permanently.
- Discard the revealed password when the field is hidden. It previously
stayed in state and in the input's DOM value with Copy still armed, so
the field read as hidden while still handing out the plaintext.
- Evict the decrypted password from the mutation cache on unmount, and
correct the TSDoc claiming it was never retained — it sat in the
MutationCache for the default five minutes after the modal closed.
- Validate the password inside performChatDeploy, the writer both callers
must use. The copilot deploy_chat tool bypasses the route contract and
could still store a whitespace-only or over-long password, or create a
password-protected chat with no password at all.
- Stop echoing raw decryption errors from the reveal endpoint.
- Only persist a new password when the chat ends up password-protected;
PATCH { authType: 'email', password } used to re-arm the secret that the
auth-type branch had just cleared.
Also replaces the hand-rolled copy state with useCopyToClipboard, which
fixes an unawaited clipboard write that surfaced as an unhandled rejection
and a "Copied" confirmation shown even when the write failed.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(chat): correct password-change confirmation gate and stale reveal error
Addresses both open Bugbot findings.
- shouldConfirmPasswordChange keyed on "a chat exists" rather than "a password
exists", so switching a public chat to password protection for the first time
asked the admin to confirm changing a password that was never set. It now
takes the existing-password signal the component already computes.
- A failed reveal left "Failed to load the current password" on screen while the
admin typed or generated a replacement, because the mutation only drops its
error on the next attempt. Editing or regenerating now resets it.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Waleed
|
797f781855 |
chore(deps): bump @opentelemetry/core to 2.8.0 for CVE-2026-54285 (#6182)
W3CBaggagePropagator.extract() had no size limits on inbound baggage headers. Bumps the OTel core packages to 2.8.0 and the exporter/sdk-node packages to 0.219.0, which pin core 2.8.0 so no vulnerable copy is nested under them. |
||
|
Waleed
|
a76b928771 |
fix(security): bind copilot chat attachment keys to their owner (#6179)
* fix(security): bind copilot chat attachment keys to their owner `POST /api/copilot/chat` accepted a client-supplied `fileAttachments[].key` and passed it straight into `trackChatUpload`, which wrote it into the `workspace_files` ownership binding with no permission check and no key-ownership validation. That binding is what `verifyFileAccess` and the Files feature resolve authorization from, so any workspace member — including read-only — could hand in another member's key and have their file re-parented to a private chat: removed from the Files listing, from folders and from download-by-id, and destroyable through the chat-delete FK cascade. The sibling register route already enforced these invariants; the copilot path did not. - `trackChatUpload` now rejects keys that do not address the target workspace, only re-links a chat-upload row the caller already owns (matched by row id, not by raw key), and only mints a new binding when the key has no prior record at all — including soft-deleted ones, which the partial active-key unique index would otherwise let it insert over. - Minting a new binding verifies the object exists in storage, matching `registerUploadedWorkspaceFile`. - `buildCopilotRequestPayload` gates attachment tracking on write/admin, the same grant the upload routes that issue these keys require. * fix(security): make the chat-upload ownership check atomic with its write The ownership lookup and the binding UPDATE were separate statements, and the UPDATE matched on the captured row id alone. A concurrent `materialize_file` sets `context='workspace'` and clears `chatId` on that same row, so the tracking write still matched and dragged the saved file back into chat scope — hiding it from every workspace-file listing and re-exposing it to the chat-delete cascade, with materialize's storage-usage increment left stranded. Re-assert every ownership predicate in the UPDATE so the statement is itself the atomic check. The lookup now only decides UPDATE-vs-INSERT and is no longer load-bearing for authorization, which also makes the existing `updated.length === 0` fail-closed branch correct rather than dead. * fix(uploads): tolerate transient storage probes and reject unowned keys with 403 Follow-ups from a backward-compatibility audit of the attachment-key hardening. The existence probe is hygiene, not authorization — the key-format and no-prior-record guards already carry that, and a binding to a nonexistent object grants nothing readable. But `headObject` rethrows non-404 provider errors, so a transient 5xx or throttle would drop a legitimate attachment. Only a definitive not-found now rejects; a thrown error logs and proceeds on the ownership guards. This path is reached solely by >50MB multipart uploads, the one flow that persists no metadata row at upload time. The stage route mapped an ownership rejection to a 500. It is a client error; return 403 instead. * fix(security): compare-and-swap the chat binding on chat uploads Two overlapping chat requests could both observe the same claimable row with `chat_id IS NULL` and both satisfy the update predicate, so the later write silently moved the upload to its own chat — taking over the delete-cascade lifecycle of a file the first chat had already bound. Scope the update to `chat_id IS NULL OR chat_id = <target>` so the statement is a compare-and-swap: the loser matches zero rows and fails closed. The resolver applies the same rule so the update-vs-insert decision stays coherent. This also makes an upload bind to exactly one chat, matching the 409 the sibling `local-files/stage` route already returns for the same case; verified no client flow relinks a key across chats (drafts are per-chat, every retry path replays under a pinned chat id, forking copies blobs to fresh keys). * refactor(copilot): gate attachment tracking with the shared permission predicate `permissionSatisfies` is the documented single source of truth for permission comparisons and exists to replace hand-written `=== 'admin' || === 'write'` ladders. `userPermission` is typed `string` for legacy reasons, so narrow it with `isPermissionType` first — an unrecognized value fails the gate instead of ranking below every level. Behavior is unchanged for all three levels. Imported from the dependency-free `/predicates` subpath rather than `/workspace`, which would pull `@sim/db` onto the chat request path. |
||
|
Vikhyath MondretiandClaude
|
e90378a5c1 |
improvement(permissions): confine workspace role changes to existing members (#6180)
* improvement(permissions): confine workspace role changes to existing members * improvement(permissions): retry lock timeouts and audit admin upserts truthfully Bounding the row-lock wait with `lock_timeout` made a competing writer raise 55P03, which the retry set excluded — so the bounded wait failed the request where the unbounded one would have waited. 55P03 is now retried like the other contention aborts, the timeout is shortened since each attempt releases its pooled connection, and contention that outlives every attempt answers with a busy conflict instead of a generic server error. The admin member upsert recorded MEMBER_ADDED even when the conflict branch amended an existing membership, putting a join that never happened in the workspace audit trail. It now records a role change, matching the action the response already reported. Adds the missing test coverage for withTransactionRetry. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Waleed
|
9064039c19 |
improvement(logfire): scope block outputs per operation and refresh brand chrome (#6178)
* improvement(logfire): scope block outputs per operation and refresh brand chrome - gate each block output on the operations that actually return it - swap in the official Logfire mark, black tile with brand-magenta bare icon - move host to advanced mode and alphabetize the tool registry entries - add track-logfire-llm-cost and verify-logfire-token-target skills * fix(logfire): honor numeric-string limits and surface token validity fields - accept a numeric-string limit so agent-invoked calls stop silently falling back to Logfire's 100-row default - keep an hour-only UTC offset intact instead of producing +05Z - surface expiresAt and spendingCapReachedAt on Get Token Info - document pending_span as a fourth record kind * chore(logfire): regenerate tool metadata and document the step in the skill - regenerate apps/sim/tools/generated/tool-outputs.ts, which CI's tool-metadata:check requires after a tool output change - add the regeneration step and artifact-diff guidance to the validate-integration skill so the gate stops being missed * chore(skills): sync validate-integration projections |