mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-30 19:32:46 +08:00
528b34f564de2ea83e2457e54a5b216e8d2fe972
6465
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
528b34f564 |
fix(workflow): hide idle nested subflow end handles (#6976)
* fix(workflow): hide idle nested subflow end handles * perf(workflow): avoid repeated subflow edge scans * perf(workflow): stabilize subflow edge selector --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Waleed
|
67fc2aeb30 |
fix(ci): require a default export before treating a file as a route entry (#7028)
* fix(ci): require a default export before treating a file as a route entry Follow-up to #7026, which added `error.tsx` to the entry filenames and with it picked up `[workspaceId]/components/error/error.tsx` — named like a boundary, and not one. It exports `ErrorShell` and `ErrorState` for the thirteen real boundaries to use; Next would reject it as a boundary for having no default export. Counting it inflated the coverage number and would have recorded a shared component in the graph-weight baseline as though it were a route. The filename was never the right test. Every convention-composed entry must default-export the thing Next renders, so that is the discriminator now. Entry count goes 60 → 59, and all thirteen real `error.tsx` boundaries still walk. Also adds `template.tsx` and `default.tsx`. Neither exists under `app/workspace` today, so this changes nothing now — but the enumeration claims to cover what Next composes, and leaving two out makes that claim false the day someone adds one. Both raised in review on #7026 (Cursor and Greptile respectively); I merged before reading them, so this lands separately. * fix(ci): count every form that declares a default export `export { default } from './page'` is a valid Next entry and the regex required `as default`, so such an entry would have dropped out of the walk and skipped both the registry gate and the graph-weight ratchet — silently, which is the dangerous direction for a discriminator to fail in. Latent rather than live: the form appears once under `app/workspace`, in a barrel, not in an entry filename. Four forms now count — `export default …`, `export { default } from`, `export { default, … } from`, and `export { X as default }`. `export { default as X }` still does not: it re-exports another module's default under a name and leaves this one without one. Verified all ten variants, including that last distinction. Raised by both Cursor and Greptile on #7028. |
||
|
Waleed
|
82b02fb4d9 |
improvement(ui): standardize modal default actions (#7029)
* improvement(ui): standardize modal default actions * fix(ui): keep aggregate deletes on safe default |
||
|
Waleed
|
1aa714cf91 |
fix(ci): stop failing the API audit for adding a compliant route (#7027)
`totalRoutes` sits at 1162 and the repo has exactly 1162 routes, so the next
route fails CI whether or not it is contract-backed:
API validation audit failed:
- route count increased from 1161 to 1162
The invariant worth holding is that every route has a contract, and
`nonZodRoutes` states exactly that. It is 0, and it rises the moment a route
ships without one — `zodRoutes === totalRoutes` today, so the total adds no
information the other two counters do not already carry.
What it adds instead is a habit. The only way past it is editing the number, and
this file holds seven other baselines that work only while nobody bumps a
baseline casually.
The total is still printed; it is no longer a failure. Verified both directions:
a compliant new route passes where it previously failed, and a route without a
contract still fails through `nonZodRoutes`.
|
||
|
Waleed
|
f37c24e541 |
fix(ci): walk every route entry the workspace app composes, not just pages and layouts (#7026)
The tool-registry guard collected `page.tsx` and `layout.tsx`, and Next composes three more entries by convention: `error.tsx`, `loading.tsx`, `not-found.tsx`. Twenty-six exist under `app/workspace` and none was walked. `error.tsx` is always a Client Component — Next requires it — so a registry edge there reaches the browser bundle exactly as one from a page does. Coverage goes from 34 entry graphs to 60. Nothing new is reported: the hole was unexploited, and closing it costs nothing. The root deliberately stays at `app/workspace`. Widening it to `app` reports `(interfaces)/resume/[workflowId]/[executionId]/page.tsx`, a Server Component (`runtime = 'nodejs'`, `force-dynamic`) whose `PauseResumeManager` import resolves server-side and never reaches a client bundle. The guard cannot distinguish server from client entries, so it stays where its premise holds. |
||
|
Waleed
|
cc16d23d90 |
fix(react-query): close the lint's blind spots, and the drift they hid (#7020)
`check-react-query-patterns.ts` reported a clean strict zone while never
looking at part of it. Two gaps in one regex:
`\buseQuery\s*\(` does not match `useQuery<Row[]>({ ... })` — a type argument
sits between the name and the paren. Twenty query calls carry one, ten of them
inside the zero-tolerance zone, so that zone's "0 violations" was partly a
statement about what the scan could see.
`useQueries` was absent from both the call pattern and the file pre-filter,
where `\buse(Query|...)\b` rejects it on the trailing `s`. All sixteen call
sites were unscanned, and its options nest one level deeper — inside a
`queries` array — so it needs its own pass per entry rather than one that reads
the wrapper and takes a single `staleTime` anywhere inside as covering them all.
With both closed, three real violations surfaced:
- `knowledge-base-selector` served `knowledgeKeys.detail(id)` with an inline
`60 * 1000` while `useKnowledgeBaseQuery` serves the same cache key from
`KNOWLEDGE_BASE_DETAIL_STALE_TIME`. The two agree only by coincidence, and
TanStack resolves staleTime per observer, so tuning the constant would have
left this component on the old window for the same entry.
- The same call dropped the `AbortSignal`, which `fetchKnowledgeBase` accepts.
- `use-permission-config` gave `staleTime` as a literal with no named constant.
The new `stale-time-literal` category makes the second half of the CLAUDE.md
rule enforceable — it required a named constant, and only the presence of
`staleTime` was ever checked. `0` is exempt: it is the sentinel for "always
refetch", not a window anyone keeps in step with a prefetch.
Verified the new rules can fail by reverting each fix and watching the audit
report it, then restoring.
|
||
|
Waleed
|
7e0d8681d2 |
fix: surface an unbilled run, and clamp the google-docs page cap (#7025)
Two places where a failure is reported as something smaller than it is.
**A run that is never billed logs as a notification problem.** The usage
safety net re-records billing when an earlier step threw before the single
record call, and its own failure went into a bare `catch {}`. With a degraded
database the user lookup throws first, the re-record hits the same database and
is swallowed, and the only line emitted reads "Usage threshold notification
check failed (non-fatal)" — which is true of the outer failure and badly wrong
about the inner one. It now logs at error with the execution and workflow ids,
and says the run may be unbilled. The outer warn still covers the email path it
was written for.
**google-docs can ask Drive for a negative page.** `remaining` was
`maxDocs - previouslyFetched` unclamped, where its google-slides twin carries
`Math.max(0, …)` under the comment "Last-page precision". Both then run
`if (documents.length > remaining) documents = documents.slice(0, remaining)`,
and a negative `remaining` makes that guard true for any non-empty page while
`slice` counts from the end — keeping the leading documents and dropping the
trailing ones, where the cap says to keep none. Reachable when `maxDocs` is
lowered while a sync cursor persists. google-drive guards the same case with an
early return; google-docs had neither.
|
||
|
Waleed
|
0cbff0e01b |
fix(settings): report a failed settings write instead of reporting success (#7023)
The PATCH catch answered `{ success: true }` with 200, so a failed upsert was
indistinguishable from a saved one.
`useUpdateGeneralSetting` is optimistic: `onMutate` writes the new value into
the cache and calls `syncThemeToNextThemes`, and `onError` restores the previous
settings. `requestJson` only throws on a non-2xx, so `onError` could never run —
the rollback and its theme re-sync were unreachable code. A user toggling a
consent-shaped setting (telemetry, email opt-out) saw it applied and it was not
saved, until a later refetch quietly reverted it.
The catch now returns 500, which is what the mutation was already written to
handle.
Left alone deliberately: GET still falls back to `defaultUserSettings` on error.
Failing it would take the settings page down on a transient read, and the value
of changing it is a separate judgement from this one.
Covered by a route test that drives the failure through the real handler.
Verified it fails when the 200 is put back.
|
||
|
Waleed
|
297e970b09 |
refactor: delete code nothing reaches (#7019)
`biome.json:101-102` turns off `noUnusedVariables` and `noUnusedFunctionParameters`, so none of this was ever going to be flagged. Everything here was confirmed by grepping the symbol across `apps/` and `packages/` and finding only its own declaration; `tsc --noEmit` then proves each deleted binding was unread, since a read one fails to compile. - Eleven module-scope loggers that nothing logs through, with the now-orphaned `createLogger` import each left behind. - `execute-platform-context-use-case.ts` — the whole file. No importer, no barrel, and neither export is named anywhere. - `routeToolCall` and, once it goes, `ToolRoute` and `ToolRouteTarget` with it. The catalog accessors around them stay live. - `processPastChat`, superseded by `processPastChatFromDb`. It carried the last `boundary-raw-fetch` exemption in the file. - `withMessageId`, pasted into three server tools and called in none. - Write-only locals: `activeSubagent` (assigned twice, read never — the scoped maps replaced it), `resolvedReadPath`, `workflowPath`, and `workflow` in an execution-core destructure. - `ACCEPTED_AUDIO_TYPES` / `ACCEPTED_VIDEO_TYPES`, never wired to an accept attribute the way their live sibling is. - Unused `catch` bindings in `error-extractors.ts` and `defaults.ts`. `diff-engine.ts` drops a `proposedSubKeys.includes(key)` guard that the `!proposedSub` check three lines down already covers: a key absent from the proposed block reads back `undefined` there, and so does a key present with a nullish value. Same answer on every input, without the O(n) scan per iteration. |
||
|
Waleed
|
95d08d2a49 |
fix(ci): stop the migration safety audit from passing on a branch it never read (#7022)
* fix(ci): stop the migration safety audit from passing on a branch it never read
The zero-downtime audit reports the same empty file list for 'this branch adds
no migrations' and 'I could not diff against the base', and the second prints
as `✓ No new migrations to check` with exit 0. Reproduced on this checkout:
$ bun run scripts/check-migrations-safety.ts origin/does-not-exist-branch
✓ No new migrations to check. exit=0
`changedMigrationFiles` returned `[]` whenever `git diff` failed, with a comment
deferring the decision to the caller — but the caller only recognised a missing
git binary (`git rev-parse HEAD === null`), never an unusable ref.
CI supplied exactly that input. `git fetch --depth=1 … 2>/dev/null || true` hid a
failed fetch, leaving `origin/<base>` absent, so a PR adding a destructive
`DROP COLUMN` would clear the only guard on production DDL with a green check.
Two halves:
- The audit now distinguishes the cases. Absent git is still the one legitimate
skip and is checked before the diff; a diff that fails with git present raises
`BaseRefUnusableError` and exits 1.
- The fetch is its own step with no `|| true`, so a failure fails the job. Depth
stays 1: without a merge-base the audit diffs the two tips, which under
`--diff-filter=AM` is exactly the migrations new on the branch.
Covered by a test that runs the script end to end, since the defect was in the
exit code rather than in any function's return value. Verified it fails when the
throw is reverted to `return []`.
* fix(ci): fetch the base ref once, and stop swallowing the failure
The same `git fetch --depth=1 … 2>/dev/null || true` appeared in both base-ref
audits. Fixing only the migration one would have left the identical defect a few
steps above it.
Neither audit can tell an absent base ref apart from a branch that changed
nothing. The block-registry check at least degrades to a visible
`⚠ Could not diff against base ref — skipping`; the migration audit printed
`✓ No new migrations to check` and exited 0.
Both now share one fetch step that fails the job when it fails.
|
||
|
Waleed
|
cc087498ae |
refactor(utils): add slugify and adopt it at the eight sites that hand-rolled it (#7018)
The same three-step derivation — lowercase, collapse each non-alphanumeric run to a hyphen, strip the leading and trailing one — sat in eight files. Two of them carried a TSDoc line whose only job was to warn that they mirrored a third (`instance-org.ts`: "Derives a slug the same way the admin organization API does"; `consolidate-users-into-organization.ts`: "Mirrors the slug derivation used by POST /api/v1/admin/organizations"). A comment asserting two implementations agree is the shape duplication takes when it cannot be checked. All eight were semantically identical. Two anchored the strip with `-+` rather than `-`, and one followed it with a `--+` collapse, but `[^a-z0-9]+` has already collapsed every run by that point, so neither could ever match more than the single-hyphen form. Nothing changes. Truncation stays at the call sites. Four of them bound the result — at 24, 64 and 80 — and only `copy-chats.ts` strips again afterwards, because slicing can land mid-run and leave a trailing hyphen the earlier strip never saw. Folding a `maxLength` into the helper would have had to pick one of those behaviors and silently impose it on the others. `artifact-stylesheet.ts` keeps its copy: it lives inside the `SIM_ARTIFACT_SHELL` template literal and runs in the viewer's browser, where there is no import to resolve. |
||
|
Waleed
|
49593b3191 |
refactor: replace hand-rolled utilities and dead code with the shared forms (#7021)
* refactor: replace hand-rolled utilities and dead code with the shared forms Each of these has a mandated helper or an established accessor in the repo that the site predates or missed. All are behavior-preserving: - `omit()` for the three `Object.fromEntries(Object.entries(x).filter(...))` block-input filters, which also recovers the `Omit<T, K>` typing that `Object.fromEntries` erases to an index signature. - `getErrorMessage()` for the inline `instanceof Error` message ternary. - `getBlock()` for two `getAllBlocks().find((b) => b.type === x)` scans, one of them inside a loop over selected tools. The same file already resolves the same values through `getBlock`. - A memoised `Map` for three `.find()`-by-id scans over the workspace skill list, one of them inside a render `.map()`. - `SELECTOR_SEARCH_STALE` for three copy-pasted `15 * 1000` literals. They are deliberately shorter than `SELECTOR_STALE`, so this is a new named constant rather than a fold into the existing one. - Tailwind classes for the static half of two duplicated anchor styles, keeping only the genuinely dynamic `left`/`top` inline. - Dropped the unused `catch` bindings on three intentional JSON-parse swallows. `panel.tsx`'s run-button gate loses a `TODO`-stubbed `hasValidationErrors = false` and the `isWorkflowBlocked` term built on it. That term was dead twice over: it reduced to `isExecuting`, and the enclosing expression is already guarded by `!isExecuting`. * fix: guard the registry lookups, and scope the search-stale doc to its callers `getBlock` normalizes its argument with `type.replace(...)`, so it throws on `undefined` where the `getAllBlocks().find(...)` it replaced returned `undefined` harmlessly. Both call sites can be reached without a type: `tool-input` reads `state.blocks[blockId]?.type`, which is undefined once the block is deleted while the panel is mounted — and `Record` indexing hides that from the compiler, so it would have thrown during render. `agent-handler`'s `tool.type` is optional and the compiler did catch it. Also index the skill lookup in `resolveSkillsLabel`, which runs a `.find()` inside a `.map()` for every block on the canvas — the case the memoised map in `skill-input` addressed for one component while leaving the hot path. `providers/utils.ts` keeps its `getAllBlocks().find(...)`: it takes the registry as an injected dependency precisely so a client-reachable module never imports it, and reaching for `getBlock` there would cross that boundary. The new constant's doc claimed search-backed selectors take a shorter window. Several still sit on `SELECTOR_STALE`, so it now describes the value its three callers share rather than asserting a rule the tree does not follow. * fix: guard the second registry lookup in tool-input `selectedTools` validates only `value[0]?.type` and then casts the whole array, so a persisted workflow whose later rows lost their `type` yields `undefined` here — the cast is what makes the compiler believe otherwise. `getBlock` normalizes with `type.replace`, so that throws during render. |
||
|
Waleed
|
fc7aa66a39 |
fix(api): withhold internal failure messages from internal route responses (#7015)
An orchestration result carrying `errorCode: 'internal'` holds whatever text the fault happened to have — `workflow-lifecycle.ts` catch-alls return `toError(error).message`, which is the driver's failed SQL. Three application helpers projected that straight into an `OrchestrationError`, and the internal route policy rendered its message into a 500 body, so raw SQL reached clients. The v2 envelope already scrubbed the same failures; internal routes did not. `messageForOrchestrationError` already encoded the rule and two sites honored it. The three that hand-rolled it disagreed, and `workflow-vfs` disagreed with itself: it defaulted the code with `?? 'internal'` but compared the raw `errorCode` against `'internal'`, so an uncoded failure was classified internal and still rendered its own message. Pair the two in `throwOrchestrationFailure` so a code and its message cannot disagree, and scrub at the internal route boundary as well, matching v2 — no call site authors a curated `internal` message, so nothing legitimate is masked, and site N+1 cannot reopen this by forgetting the rule. |
||
|
Waleed
|
a20a5465ef |
perf(db): optimize recurring query paths (#7014)
* perf(db): optimize recurring query paths * perf(logs): batch keyset export reads * fix(workspaces): type nullable member count targets * fix(db): harden query performance changes * fix(logs): guard export stream cancellation |
||
|
Vikhyath Mondreti
|
9cecf0b837 |
improvement(provenance): aggregate and attribute unrecorded durable reads (#7017)
Fail-open on unrecorded durable provenance rests on one compensating control: the audit entry telling the people who own the secrets that a read proceeded unvouched. An audit of all four surfaces found the control incomplete in exactly the places this closes, and confirmed the policy itself sound — so nothing here changes what any read or write does, only what gets recorded about it. Knowledge was the one surface with no audit trail at all: the per-record import reports without a workspace, and the report skips the audit row when it cannot name one, so fail-open knowledge reads emitted one error log line per record and zero audit entries. Both importers now count unrecorded records while the surface is open and report once per read with the workspace, actor, and count — the shape memory and tables already use. The search read reports once across chunks and rendered metadata, and only when the registry did not latch, since a latched read never reaches a model. Fault returns stay silent; those reads fail closed. Memory had the one silent local degrade: a record whose canonical hash outgrows its bounds, or whose entries fail normalization, was stored unknown with nothing logged anywhere — the table writer logs its equivalent. The binding now logs the cause at error where it is decided. An incoming unknown stays silent; its producer already reported. The memory list contract gains the page ceiling every other list already has (max 1000, matching the table convention); no caller in the repo passes a limit at all, and the route is internal-auth only. Workspace-file audit rows now carry the acting user where the caller already holds one — copilot vfs, the agent and mothership handlers, and the provider attachment filter. Everywhere else, including principals with no user to name, the actor stays null, which the report type has always permitted. Two comments catch up with the code: the file sidecar stores three statuses since the absence/taint split, and the mounted-file scanner's scan-overflow-to-taint is deliberate where the registry scan over-approximates — that scan only narrows an already-sound candidate set, while this one decides whether egress redaction would suffice for bytes the same matcher just failed on. |
||
|
Waleed
|
1cb9c868ce |
improvement(chat): speed up conversation navigation (#7011)
* improvement(chat): speed up conversation navigation * fix(chat): prefetch direct navigation intent * fix(chat): preserve quick-click prefetch intent |
||
|
Waleed
|
b44d285607 |
fix(files): download a markdown file as a zip only when it really has assets (#7009)
* fix(files): download a markdown file as a zip only when it really has assets A document that merely mentions an embed URL in prose or an inline code span counted as having attachments, so any document about the files API downloaded as a zip whose assets/ folder was empty. - Detect embeds with the markdown lexer instead of scanning raw text, so only real image embeds count: prose, code spans, fenced samples, and links no longer do - Choose the export format after resolving assets rather than from the candidate count, so a missing, unreadable, or oversized embed falls back to the plain document instead of an empty zip - Move the document scan out of the copilot tool tree into lib/uploads/server, where both file routes already live, and drop two pass-through wrappers - Share one <img> src reader between the clipboard handlers and the scan - Walk tokens explicitly: marked's walkTokens concatenates per token and costs O(n^2), measuring 5.4s on a 254KB document against 14ms here, on a path anonymous public-share traffic reaches * fix(files): keep an embed id spelled as the document spells it Decoding the id let a percent-encoded embed resolve and bundle its asset while the rewrite, which searches the document for that id, found nothing — the zip kept an API URL that renders as a broken image offline. Keys stay decoded; they are matched against stored keys, not against document text. * fix(files): resolve an export asset by its stored id, rewrite by its spelling An embed carries two representations and they are not interchangeable: metadata resolves by the stored id, while the rewrite finds the embed by searching the document for the spelling it used. Using one for both either drops a percent-encoded asset or bundles it behind a link still pointing at the API. * fix(files): resolve an embed by its stored id wherever one is read from a document The export bundler decoded an embed's spelling before looking it up, but the file-agent's embeddability warning did not, so a percent-encoded embed the export resolves and bundles could still be reported as one that will not survive an export. Both now share one helper. Request-supplied ids are untouched: their route contracts already constrain them to the plain id charset, so there is no spelling to decode. |
||
|
Vikhyath Mondreti
|
ed335d4137 | improvement(pricing): list Sandboxes as a Max and Enterprise feature (#7013) | ||
|
Waleed
|
f3867694b8 |
fix(files): allowlist the schemes a markdown link may target (#7012)
`normalizeLinkHref` rejected only `file` for a `scheme://` target, so any other scheme was returned unchanged. `scheme://` is well-formed for every scheme, so the check let through spellings that are not navigable targets at all. - Keep a scheme only when it is http(s), ftp(s), mailto, or tel; drop the rest - Leave an existing link alone when a committed target normalizes away, rather than unsetting it — the editor seeds that field with the current href, so committing an untouched one previously removed the link Detection is unchanged for relative, anchor, protocol-relative, and bare-domain targets. A document's stored markdown is untouched: normalization runs on the render and edit paths, never on parse or serialize, so a target that is refused still round-trips verbatim. |
||
|
Waleed
|
465bdbd12b | fix(settings): keep billing header stable (#7010) | ||
|
Theodore Li
|
d831c0937c |
fix(security): isolate rejected OTP attempts (#7008)
* fix(security): isolate rejected OTP attempts * fix(security): make OTP requests non-enumerating * fix(security): defer OTP delivery work |
||
|
Vikhyath MondretiandClaude Opus 5
|
33feaa158a |
fix(inbox): stop an unattributed sender inheriting owner write authority (#7006)
* fix(inbox): stop an unattributed sender inheriting owner write authority resolveInboxExecutionActor refuses to name a raw-secret actor when the sender matches no workspace member, then hands the run ws.ownerId for everything else. That identity also supplies userPermission, which is what executeTool gates on, so the owner's admin satisfied every requiredPermission check. In headless mode the client-routed workflow tools fall back to their registered server handlers (see the comment in tool-executor/executor.ts), so create_workflow, edit_workflow and run_workflow — all requiredPermission 'write' — were reachable. runWorkflowFromCopilot then executes with enforceCredentialAccess and the owner as actor, which resolves the owner's workspace and personal secrets. An allowlisted external correspondent could therefore reach, through a workflow it had the agent build and run, exactly what the null secret actor refuses for a direct mount. Cap the run's tool permission at read when no member owns the message. An attributed message is unchanged and still uses the sender's own permission, so a read-only member emailing the inbox still cannot run or edit anything. Read rather than none because answering an external correspondent from workspace context is the point of the inbox; only mutation and execution are withheld. The owner identity itself stays: billing attribution and workspace reads need a real user. This separates that need from the authority that came with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): bar the headless client-tool fallback below write Client-routed tools carry no catalog requiredPermission because the browser runs them through the workflow APIs, which authorize the caller's own session. The headless fallback in executeTool has no session and runs under the request's principal instead, with nothing standing in for that check. So the read cap from the previous commit did not reach run_workflow, run_workflow_until_block, run_block or run_from_block: all four are route 'client' with no requiredPermission, unlike create_workflow and edit_workflow. An unattributed inbox sender could therefore still run an existing workflow, which executes with enforceCredentialAccess under the workspace owner and resolves the owner's workspace and personal secrets. Derive the requirement at the gate instead: a client-routed tool taking the headless fallback requires write. Interactive callers never reach this branch, so the browser path is unaffected. The catalog itself is generated from the copilot contracts repo and cannot carry this rule, which only applies to the fallback. Also corrects the inboxToolPermission doc, which claimed run_workflow gates on requiredPermission 'write'. It does not; it is gated here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Waleed
|
66a9a18d1f |
improvement(perf): eight verified cuts to workspace cold-load JavaScript (#6996)
* improvement(perf): eight verified cuts to workspace cold-load JavaScript Second round of load-time work, adversarially verified for strict behaviour preservation before implementation. Each item is an import-graph fix — none changes what renders, when it renders, or any data path: - knowledge/[id] imported one modal through the [documentId] components barrel, which also exports the chunk editor and therefore js-tiktoken (~2.5 MB gzip of BPE tables) on a route that never edits chunks. Deep import. - prepareBlockState moved out of stores/workflows/utils.ts into its own module. It is the only function there needing the block registry and the generated tool-outputs artifact (~476 KB gzip), and utils.ts is reached by the persistent shell — so every workspace route paid for a canvas-only helper, including a module-scope JSON.parse of a 5.4 MB string. - ExecutionSnapshot (the frozen-canvas modal) is now React.lazy behind its interaction gates, per the code-splitting procedure in sim-imports.md: deep import, dead barrel re-export deleted, sibling imports in log-details deepened to break the parent->child barrel cycle, local Suspense at both render sites. Takes ~7.6 MB of source off logs hydration. - The api contracts barrel no longer re-exports ./tools, ./selectors, ./v1, or ./demo-requests (~58 KB gzip of Zod schema construction on every route). Zero importers used the barrel path for any of them. - createCsvParser (streaming csv-parse, a Node Transform) moved to a server-only module so its stream polyfill leaves client bundles. Deliberately not re-exported from the lib/table barrel. - jszip is dynamically imported at both remaining static call sites (skill zip extraction, pptx parsing) — both already-async, user-triggered paths, mirroring the existing pattern in workflow import-export. - The desktop local-filesystem tool executor is dynamically imported in use-chat; a chunk-load failure now reports an error completion so the server-side tool call settles instead of hanging. Production build, JS downloaded before the load event, vs the previous release: /home 4.44 -> 3.87 MB /logs 4.44 -> 3.64 MB /knowledge 4.22 -> 3.68 MB /tables 4.17 -> 3.61 MB /files 4.68 -> 4.10 MB /w/[id] 4.80 -> 4.67 MB /home total after idle prefetch: 8.15 -> 5.52 MB The lazy snapshot was exercised end-to-end: its chunk loads when a log detail opens (off the route's cold path, warm before the View Snapshot click) and the modal renders without errors. Boundary baseline retightened. * improvement(logs): contain snapshot chunk-load failures and settle the local-fs tool on recovery failure Review round: wrap both lazy ExecutionSnapshot render sites in a small error boundary (Suspense handles the lazy import's pending state, not its rejection — a failed chunk load would have unwound to the route boundary and replaced the logs page over an optional modal; mirrors PreviewErrorBoundary), and contain rejections inside the local-filesystem executor's load-failure recovery so a failed completion report degrades to a log instead of an unhandled rejection. * fix(logs): recover cleanly from snapshot chunk failures * fix(logs): preserve snapshot modal while loading |
||
|
Waleed
|
5a1602e961 |
fix(copilot): enforce the chat's delegated workspace scope across model-steerable server tools (#6995)
* fix(copilot): enforce delegated workspace scope in query_logs and set_environment_variables
A model-supplied workspaceId (or a workflowId in another workspace) could
steer both tools to any workspace the acting principal can reach, bypassing
the asserted-vs-context workspace comparison the rest of the Copilot tool
surface enforces. Both now resolve through requireCopilotWorkspace — moved
to a shared module — so an asserted workspace may only re-state the chat's
execution workspace, and the default-workspace fallback is removed so a
missing scope fails closed.
* fix(copilot): apply the workspace-scope guard across every model-steerable copilot surface
Extends requireCopilotWorkspace to the remaining copilot tools that resolved
their target workspace from model-supplied arguments: get_credentials (a
workflowId could steer the credential listing to any workspace the user can
access) and publish_custom_block (a workflowId could deploy/undeploy custom
blocks from another workspace's workflow). The handlers already protected
downstream by the application adapter (create workflow, generate API key,
list/create workspace MCP servers) now use the same guard so a mismatch is
rejected uniformly at the surface, and the getDefaultWorkspaceId fallback is
deleted entirely — no copilot path picks a workspace for the model anymore.
* refactor(copilot): classify both workspace-scope guard branches and drop call-site boilerplate
requireCopilotWorkspace now accepts an undefined context and throws a
classified OrchestrationError for the missing-workspace branch too, so every
caller drops the 'context ?? {}' and '|| undefined' coercions and one
instanceof covers the guard. query_logs inlines its now-one-line wrapper,
get_credentials drops the workspace-less special case (a workflow with no
workspace asserts nothing), and publish_custom_block handles the guard
locally instead of widening its catch-all — keeping its deliberate
assume-not-published guidance for unrelated failures.
* chore(copilot): drop call-site comments that restate the workspace-scope guard's TSDoc
* test(copilot): type the new query-logs scope tests instead of casting to any
|
||
|
Vikhyath MondretiandClaude Opus 5
|
43aba98527 |
fix(desktop): stop the OAuth connect callback from failing on a bare-path callback URL (#7005)
* fix(desktop): stop the OAuth connect callback from failing on a bare-path callback URL The desktop connect launcher passed better-auth a same-origin path as its callbackURL. Better Auth stores that value verbatim in the OAuth state, and the callback's credential-draft reader parsed it with a bare `new URL()`, which rejects a path. That throw happened inside the `account.create.before` database hook, which better-auth's OAuth callback does not guard, so the provider redirect landed on a 500 after authorization had already succeeded. Send an absolute URL from the connect page, matching the workspace-scoped branch and every other connect surface, and accept a path-absolute callback URL in the draft reader so the shape can never fail the callback again. Protocol-relative and malformed values still throw, keeping an unreadable binding loud. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(desktop): compose the connect completion URL through the URL API Concatenating `getBaseUrl()` with the completion path leaves the result dependent on how the deployment spelled `NEXT_PUBLIC_APP_URL`: the helper only adds a missing protocol, so a trailing slash produced `//desktop/connect/complete`, a pathname that matches no route. The completion page is what bounces the OAuth result to the desktop app's loopback, so that typo would have stranded the flow just past the callback it was meant to fix. Both callback URLs in the page — the launcher's and the workspace-scoped authorize redirect's — now go through one helper that resolves the path against the base with `new URL`, matching how the same function already builds the authorize URL, with coverage for a trailing-slash base. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(urls): give base URLs the no-trailing-slash form their call sites assume `getBaseUrl()` returned `NEXT_PUBLIC_APP_URL` as the operator spelled it, while almost every consumer builds `${base}/path`. A base configured with a trailing slash therefore produced a `//path` pathname that matches no route, and broke the `startsWith(`${base}/`)` prefix checks that decide whether a redirect target is our own — the OAuth authorize route rejected its own completion callback and fell back to the workspace page, so the desktop handoff never ran on those deployments. The previous commit fixed one such URL; this fixes the reason it was wrong, for the ~30 concatenation sites that share the assumption. `normalizeBaseUrl` now strips trailing slashes alongside the protocol it already added, which is the invariant SITE_URL has always documented. A path-prefixed base keeps its path. `getInternalApiBaseUrl` gets the same treatment, since its callers concatenate identically. `@sim/testing`'s urls mock is a hand-written mirror of this module, so it moves in step. `internal-api-base-url.test.ts` now unmocks the module it names — otherwise it asserts against that mirror and any drift between the two passes unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(urls): stop claiming path-prefixed base URLs are supported The previous commit's doc and test said a path-prefixed base keeps its path. That reads as support for a deployment shape the app does not have: there is no Next `basePath`, so routes are served at the origin root and such a value could not address them however the base were normalized. Every documented example is origin-only. Says only what is true — trailing slashes are the one spelling absorbed — and reframes the test as pinning the trim's shape rather than asserting a path-prefixed deployment works. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Theodore Li
|
bbf408bf30 |
fix(security): harden public auth rate limits (#6997)
* fix(security): harden public auth rate limits * fix(security): fail closed without client IP * fix(security): backstop public OTP requests * fix(security): preserve independent rate-limit backstops |
||
|
Vikhyath MondretiandClaude Opus 5
|
81ff24a2fc |
improvement(secrets): gate Copilot code mounting at use level (#7004)
* improvement(secrets): gate Copilot code mounting at use level Mounting a saved secret into Copilot code required credential-admin on that key, while a workflow Function block resolves the same secret for the same person at use level through getPersonalAndWorkspaceEnv. Copilot reaches that path itself — edit_workflow plus run_workflow — so the admin bar contained nothing. It redirected a Credential Member through a detour that mutates a persisted workflow, while the direct path is ephemeral and files a usage row. The inconsistency was also internal to Copilot: the secret names advertised to the model come from getAccessibleEnvCredentials and getPersonalAndWorkspaceEnv, both role-agnostic, so Copilot listed every secret the caller could use and then refused to mount all but the admin ones. Widen the workspace and shared-personal predicates to any active grant, and drop the matching role filter from the query. Workspace write is still required, revoked and pending grants are still refused, and a caller with no grant still gets nothing. The view gate stays where Copilot cannot route around it: values remain masked under Settings, and See usage remains admin-only, so a member's use is recorded for whoever can rotate the key. Model-egress projection is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(secrets): stop implying Personal secrets are shareable The Copilot code-execution paragraph listed "any secret shared with you as a Credential Member or Credential Admin" among what mounts, which reads as though a Personal secret can be shared. It cannot through any product surface: CredentialMembersSection renders only for workspace secrets and OAuth credentials, and the personal-credential sync only ever grants the owner. Narrow the sentence to Workspace grants. The comparison table's "Only you can use" row for Personal was correct and is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath MondretiandSim Pi Agent
|
9ff1d77306 |
feat(library): Best No-Code and Low-Code AI Agent Builders in 2026 (#7003)
Co-authored-by: Sim Pi Agent <pi@sim.ai> |
||
|
Theodore Li
|
8104c33bba |
Fix knowledge connector sync follow-up (#6927)
* Fix knowledge connector sync follow-up * Fix connector sync pause race * fix(knowledge): surface connector sync dispatch failures * fix(knowledge): make connector sync recovery durable * fix(knowledge): deduplicate connector sync dispatches * fix(knowledge): preserve pending connector syncs * fix(knowledge): lock connector sync snapshot |
||
|
Vikhyath MondretiandClaude Opus 5
|
763f3aa1b5 |
fix(deployments): stop superseded activations from dead-lettering (#6522)
* fix(deployments): stop superseded activations from dead-lettering 29 workflow.deployment.prepare.v2 events dead-lettered with "Webhook registration operation is stale", every one at attempts = max_attempts. A full retry budget means the failure is deterministic, which rules out the preparation path: an attempt superseded while preparing is marked superseded, so its next attempt short-circuits at the top of the handler and completes. The branch a retry re-enters is the other one. isTerminalNonActiveOperation covers failed and superseded but not active, so an attempt that activated and was then superseded by the next deploy keeps its own active status, re-enters post-activation work on every retry, and re-fails the same generation fence until the event dies. The fence it fails is correct — it takes the same workflow row lock the generation bump takes, and compares generations exactly — so nothing about the detection is racy; only the reaction to it was wrong. Reaching it needs a handler timeout, which parks the row for the 10-minute reaper instead of the 2s/4s/8s backoff, opening a window wide enough for a redeploy to land. Gate the resume branch on the operation still owning the current generation, matching the sibling cleanup that already does this, and complete the event as a no-op when it does not. The newer generation adopts the leftover work anyway: it collects every retired registration below its own fence. Also reverse the post-activation order. The audit entry, analytics event, socket notification, and workspace event describe a cutover that is already durable, and each is separately checkpointed, but they ran behind retiring the previous generation's external subscriptions — one provider call per retired row, and by far the most failure-prone step there. A single flaky provider silently cost the deploy its audit trail and left clients on the old version until something else refreshed them. Both call sites now share one helper so the order cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(deployments): pin staging's active-resume tests to the current generation Two tests that staging added alongside durable PostHog delivery drive the `operation.status === 'active'` resume branch this PR now gates, and neither overrides `mockIsDeploymentOperationCurrent` — the suite's `beforeEach` defaults it to `false`. Under the new gate they read as superseded, so the handler short-circuits: the delivery test sees a resolve where it asserts a rejection, and the unconsumed workflow row leaks into the next test. Both are still-current resumes by intent, so they say so explicitly. The checkpoints, not the generation gate, remain what keeps analytics from being captured twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(analytics): stop PostHog delivery from failing a deploy `deliverOutboxServerEvent` awaited `client.flush()` before letting the deployment outbox checkpoint advance, so an unreachable PostHog failed the event, retried it, and eventually dead-lettered it — while holding the socket notification, the workspace event, and retired-subscription cleanup behind a third party. `flush()` also drains the whole shared client queue, so an unrelated event's network error surfaced here as a failed deploy. It bought no durability the process did not already have: the outbox handler runs in the long-lived app container, where the client flushes on its own 10s interval and again from the `SIGTERM`/`SIGINT` hook in `instrumentation-node.ts`. The helper had one caller and arrived inside an unrelated squashed PR (#5273) with no rationale, against 161 fire-and-forget `captureServerEvent` call sites. Deleted it and restored `captureServerEvent`, whose contract is already "never throws". `insertId` still collapses retried captures. That contract was untested, which is why this regressed unnoticed, so `server.test.ts` now pins it — spying on the real client, since the lazy `require` defeats `vi.mock` and a disabled client would pass every assertion vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(deployments): fence the cleanup, not the notifications The resume guard sat at the top of the `active` branch, so it skipped every post-activation step whenever `isDeploymentOperationCurrent` went false. That predicate goes false as soon as any newer generation row exists — including one still `preparing` or already `failed` — and in that window this activation is still the live cutover. Nothing newer would ever adopt its audit entry, analytics event, socket notification, or workspace event, so the guard permanently dropped them and completed the outbox event as if they were owed to someone else. Only the two cleanups are generation-fenced. `cleanupInactiveDeploymentsForOperation` already gated itself on that exact predicate and returned quietly; the retired webhook cleanup was the one that instead let the store's `assertCurrentOperation` throw. It now carries the same guard, on the same fence the store asserts — `deploymentVersionId` and `statuses: ['active']` included, so passing the gate actually implies passing the assert. The notifications run unconditionally, still idempotent through their checkpoints. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath Mondreti
|
d4195ff682 |
fix(media): stop the ffmpeg output format from choosing its own path (#7001)
`convert` and `extract_audio` built their output path as
`path.join(dir, `out.${format}`)`, and `format` arrives from the tool as an
unconstrained `{type: 'string'}` — no pattern, no enum, and the handler passes it
through untouched. A format of `../../../../../../../../tmp/x.mp4` resolves to
`/tmp/x.mp4`, so FFmpeg writes attacker-influenced media wherever the traversal
points. Verified against a real binary: the run produced a 44,078-byte MP4
outside the temp directory, which the `rm -rf` cleanup then never saw, because
the file was never inside the directory being removed.
Output extensions are now letters and digits only. The pattern admits no `.` and
no separator, so `out.${ext}` is always a single path segment and containment
follows from the validation itself rather than from a second check that could
drift away from it.
`trim` and `fade` build their paths from `extFromMime`, which returns
`mime.split('/')[1]` — that can never contain a separator, so those stay inside
the temp dir and keep accepting values like `x-msvideo` that this stricter
pattern would wrongly reject.
|
||
|
Justin Blumencranz
|
bb0792564a |
fix(chat): copy workspace resources as portable links (#6956)
* fix(chat): copy workspace resources as portable links * refactor(chat): simplify portable resource copying |
||
|
Justin Blumencranz
|
61c405afb6 | fix(chat): copy orchestrator responses as markdown (#6903) | ||
|
Vikhyath Mondreti
|
e406b8a277 |
fix(charts): confine untrusted chart options to canvas render paths (#6998)
A `.chart` document is untrusted input — any workspace member authors one, and a public share link renders it to anonymous visitors on the app origin — but `parseChartSpec` forwarded `option` to `setOption` unchanged. ECharts draws through canvas with two exceptions: a `tooltip` left in its default `renderMode: 'html'` assigns its content to `innerHTML`, and a string `formatter` is that content's template verbatim (only substituted values are escaped), while `toolbox` assigns `dataView.lang` to `innerHTML` and fills a `saveAsImage` popup with `document.write`. Force the render mode and drop the toolbox so the document has no DOM sink, rather than filtering the values that flow through one. The walk is deep: `baseOption`, `media[].option` and timeline `options[]` each carry their own tooltip, a tooltip declared only under `media` still instantiates the component in HTML mode, and a `media` entry can override a top-level `renderMode`. Also escape the series name in the pptx chart renderer's bubble tooltip — it comes from the uploaded document and is interpolated into a hand-built `innerHTML` string, where ECharts escapes only the markup it builds itself. |
||
|
Bill LeoutsakosandBill Leoutsakos
|
6f919ef192 |
fix(terminal): keep running-command tooltips concise (#6991)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
Vikhyath Mondreti
|
cbf3aad569 |
fix(workflows): scope the canonical sub-block index to the active surface (#6990)
* fix(workflows): scope the canonical sub-block index to the active surface A block that is both an action and a trigger holds one `subBlocks` array — its own fields plus its trigger's, spread in after them. The two sets routinely share a `canonicalParamId` under different ids, so indexing them together collapses a trigger field into an action pair whose `basicId` it can never be. Every group-relative question about that field then answers for the dormant surface. The serializer was never affected: `shouldSerializeSubBlock` drops the inactive surface before the canonical collapse reads it, so it resolves against a value map the dormant surface cannot appear in. Every other caller resolves against the block's full value map, so the scoping has to live in the index. - add `getCanonicalSubBlocksForSurface` / `buildCanonicalIndexForSurface`, and move the three sites that already had the filter inline onto them - `getCardSubBlocks` derives its own index instead of accepting one; it already took `triggerMode`, and accepting an index is what let all three callers pass one built for the other surface - scope the remaining consumers that resolve against a full value map: the canvas card, autolayout, both preview surfaces, the dependsOn gate, the canonical value hook, reactive conditions, and the copilot selector lint - keep a canonical group with no advanced member out of the legacy `advancedMode` path, which deleted its basic member and republished nothing - merge legacy type-scoped tool modes as a baseline under index-scoped ones, so the first re-toggle stops reverting the ids the user has not touched * fix(workspace-forking): scope the canonical gates to the block's active surface `createCanonicalModeGates` indexed a block's whole `subBlocks` array, so on a mixed action/trigger block a trigger field sharing a `canonicalParamId` with an action pair was read as a member of THAT pair. Being neither its `basicId` nor in its `advancedIds`, `isDormantMember` answered true the moment the shared mode resolved to advanced — and a fork acts on that by clearing the value, so a configured trigger field was silently wiped on fork/sync. Reachable without any explicit toggle: a block configured as an action with a manual id and then switched to trigger mode leaves the pair's value heuristic resolving to advanced on its own. - `createCanonicalModeGates` takes the surface and scopes its index - thread `triggerMode` through `RemapForkContext`, `SubBlockTransform`, `clearDependentsOnRemap`, `collectClearedDependents`, the reference scanners, and the promote cleared-ref collectors - nested tool params and the dependent scan are unchanged: a tool is always the action surface, and the dependent scan already narrows its configs * fix(workspace-forking): keep the dormant surface classified as it was before scoping Surface scoping decides canonical membership for the ACTIVE surface. Applying it to every key also re-classified the dormant surface's own values: they stopped being dormant members, which meant the remap no longer cleared them AND started detecting them as references — turning a stale action selector on a trigger-mode block into a mapping requirement that can block promote/sync. The gates now pick the index per key: the scoped one for anything the active surface defines (the fix — a trigger field gets its own group instead of being read as a stranded member of an action pair), the whole array for everything else, which is byte-for-byte the pre-scoping behavior. Also adds `check:canonical-index`, an audit that fails any call building a canonical index off a config's whole `subBlocks`, or calling the fork gates without a surface, unless annotated with why. This defect shipped three times in three subsystems; the 14 sites that legitimately mean one fixed surface now say so at the call. * fix(audits): stop the canonical-index guard flagging its own source The audit holds `buildCanonicalIndex(` and `createCanonicalModeGates(` as string literals to search for, and its own regex matched them — the arg-count rule then fired on the literal. It passed locally only because the file was still untracked when it ran, so `git ls-files` did not list it; committing it made the audit scan itself and fail CI on the first run. Exempts the audit's own source alongside the module that defines the primitives. Verified the guard still fails on both regression shapes after the exemption. |
||
|
Vikhyath MondretiandClaude Opus 5
|
91e927442b |
fix(media): bound the ffmpeg tool's child processes, inputs and scale targets (#6989)
* fix(media): bound the ffmpeg tool's child processes, inputs and scale targets The copilot ffmpeg tool shells out to FFmpeg in the Sim app process — not in a sandbox — and nothing bounded the run. runCommand had no timer, no kill and no signal, so a transcode ran until it finished and survived the request that asked for it: a stopped copilot turn left the encode pinning both cores of a shared instance. Every operation now shares one 10-minute wall-clock deadline, and both the deadline and the caller's cancellation SIGKILL the child. The budget is per-operation rather than per-command because concat runs a full re-encode per input, so a per-command timeout would let N inputs multiply into N timeouts. Cancellation is wired to context.abortSignal, whose every abort reason is an explicit user stop — the copilot lifecycle tracks a passive client disconnect separately and does not abort on it — so an encode dies when the user asks and not before. (userStopSignal, which assertServerToolNotAborted reads, has no producer on this path.) ffprobe had the same shape in miniature: fluent-ffmpeg's static ffprobe hands back no process handle, so a timeout could only race the callback and leave a wedged prober alive, and concat probes once per input. It now runs through execFile, which takes timeout, killSignal and signal natively. Scale targets reached the filter graph unvalidated, where libavfilter sizes its per-frame buffers from them — scale=30000:30000 is ~2.7 GB a frame, allocated in a child that shares the instance's memory. Dimensions are now rejected outside 16-4096 with a message the model can act on, plus an area cap. concat's targets come from the source container rather than a caller assertion, so those are clamped instead. Two further holes: readOut buffered the output with no ceiling, and CRF-18 re-encodes routinely exceed their input, so the input budget did not bound it; and the handler accepted unlimited input files. Both are capped, and the byte check now runs against the recorded size before the download rather than after. FFMPEG_LIMITS is the single source for the four numbers the Go tool catalog mirrors into its schema, pinned by ffmpeg-schema-parity.test.ts so the model is never told a ceiling the executor does not enforce. Companion: simstudioai/copilot#PENDING Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(media): fit concat inside the shared area budget and stop input prep on cancel Two review findings, both real. concat clamped each probed axis independently, so a 4096x4096 source produced a normalization target of exactly that — nearly double the area budget scale_pad enforces through the same scale=/pad= graph. The pair is now scaled down together, so aspect ratio survives and one ceiling governs both entry points. Dimensions come back even too, which yuv420p requires and per-axis rounding did not guarantee. Preparing the inputs is itself the expensive half of a many-file call — up to the whole byte budget in storage reads — and the abort signal was only consulted after the loop, so an explicit stop was observed only once every download had already finished. The loop now checks it per input. Also corrects the rationale comments on the mirrored limits. maxItems/minimum/ maximum do not reach the model: copilot's NormalizeToolParameters allowlists type/properties/items/description/enum/required, so the bounds travel only the contract path, where Sim's router validates against them. That is still worth having — it turns an out-of-range argument into a structured rejection before any storage read or child process — but the model learns the limits from the parameter descriptions, and the comments now say so instead of claiming the schema teaches it. * fix(media): floor the scaled concat axes so the area bound actually holds The scale factor lands both axes on a product of exactly the budget, so an axis allowed to round up can put the pair back over it — and when both round up and both land even, nothing pulls them back. A brute force over every dimension pair in 16..4096 finds 219,280 that violate the bound under round-to-even, worst 2694x3520 -> 2688x3512, 3072 pixels over. Under floor-to-even: none. Flooring keeps each axis at or below its exact target, so the product cannot exceed the budget. Probed dimensions are already integers, so this changes only the scaled path. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vikhyath Mondreti
|
2c61b2989c |
fix(analytics): stop dropping client events captured before PostHog init (#6988)
* fix(analytics): stop dropping client events captured before PostHog init posthog.capture is a no-op until init() has run — its body sits behind if (this.__loaded) with no buffer and no warning. PostHogProvider reaches init through two dynamic imports while a caller needs only one, so mount-time events and error-boundary crash reports during first paint landed in that dead window and were discarded silently. Also routes the workflow canvas error boundary through captureException so its crashes reach error tracking with a parsed stack. * fix(analytics): drop unactionable browser noise from error tracking ResizeObserver loop notices were 92% of everything client-side exception autocapture reported. They are specified behaviour, not faults — the observer defers the remaining notifications to the next frame — and they carry no stack that points anywhere. Alongside opaque cross-origin "Script error." reports and cancellation signals (fetch AbortError, Monaco's CancellationError), they bury the handful of real crashes. Filters them in before_send so they never leave the browser. Fails open on anything unrecognized: a wrong guess here would delete product analytics, not just over-report. * fix(analytics): match cancellations on error name, not coerced type Two holes in the exception filter, both found reading PostHog's coercers rather than trusting the shape. The DOMException coercer always reports type "DOMException" and folds the name into the value as "AbortError: signal is aborted without reason", so matching AbortError on type alone never fired for a real aborted fetch — the filter was dead code for exactly the events it was written for. Match the error name wherever the coercer put it. The filter also applied to deliberate captureException reports, which our error boundaries make. Only exceptions the browser raised itself (mechanism.handled === false) are now eligible; a report we chose to send is never dropped. * test(analytics): make posthog client tests order-independent Readiness is module-level and settles once, so the capture and exception cases silently depended on an earlier case having settled the gate and failed when run in isolation. Walk the lifecycle in one case instead. Moves the disabled-analytics case to its own file, where vitest's per-file isolation supplies an unsettled gate — dropping the vi.resetModules() and dynamic import the repo's testing conventions rule out. Replaces the hand-rolled setTimeout flush with vi.waitFor. |
||
|
Waleed
|
b654bc11cd |
improvement(navigation): eliminate hidden workspace requests (#6979)
# Conflicts: # apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx |
||
|
Waleed
|
ab35ff4c8c |
fix(resources): make context menus selection-aware (#6980)
* fix(resources): make context menus selection-aware * fix(resources): correct bulk action scopes * fix(knowledge): avoid overstating select-all scope * fix(knowledge): keep select-all actions consistent |
||
|
Waleed
|
e46fc1d1f5 |
improvement(settings): reduce navigation and section load latency (#6992)
* improvement(settings): reduce navigation and section load latency * improvement(settings): seed a section's first-paint data on the server A settings section paid three serial hops: the route payload, its lazily loaded chunk, and only then its own queries. The first two are addressed; this starts on the third, which is what fills in after the heading paints. Generalises the existing general-settings prefetch into a per-section registry, awaited before dehydration and started as soon as workspace access is established, so the body renders populated the moment its chunk lands instead of blanking again while it fetches. Adds credential-groups as the first section beyond general/billing/admin. It qualifies on both counts the registry requires: `listCredentialGroupSettings` is a `defineAuthorizedWorkspaceUseCase`, so prefetching through it applies the same authorization and entitlement checks a client request would, and it returns exactly what the hook caches under `credentialGroupKeys.list`. The registry is deliberately sparse. An audit of all 29 sections found that most authorize in their route handler rather than their data layer, so prefetching them means first lifting that check out of the route — and two must never be prefetched naively at all, because the route is also what redacts the response: SSO strips the OIDC client secret before it leaves the server, and MCP withholds upstream credential headers from read-only members. Reaching past either would ship secrets into the dehydrated payload. * fix(settings): keep optional prefetch failures nonfatal * fix(settings): surface organization load failures |
||
|
Waleed
|
b1759f3545 |
improvement(menus): band by what the action acts on, not by verb (#6994)
Replaces "at most one separator" with the test that actually earns a rule: does the next group stop acting on the thing the user clicked. Most row menus have only the destructive transition and keep their one rule. Three menus have a second, and now show it — the logs row menu between this log and the page's filters, and the table row and column menus where they stop acting on the clicked cell or column and start creating siblings. Count follows content rather than a fixed number. The workflow panel menu had six items across four scopes and no separator at all, so "Delete workflow" sat flush against "Duplicate workflow"; it now carries the destructive rule every other menu has. Two label fixes: the logs menu wrote 'Retrying...' with ASCII dots two lines above 'Stopping…' with the character, and "Import CSV…" was the only ellipsis anywhere in the workspace menu surface while the same action reads "Import CSV" in the list menu. |
||
|
Waleed
|
37590bdf73 | fix(csv): neutralize formula-leading exports (#6993) | ||
|
|
c26529a82e |
feat(bitbucket): add repository webhook triggers (#6934)
* feat(bitbucket): add repository webhook triggers * fix(bitbucket): harden webhook trigger delivery * fix(bitbucket): address final trigger review * chore(bitbucket): address review conventions * fix(bitbucket): harden triggers and connector sync --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Theodore Li
|
7173a3fb37 |
feat(status): surface major service incidents (#6987)
* feat(status): surface major service incidents * fix(status): isolate status endpoint failures |
||
|
Vikhyath Mondreti
|
7ea8692e7d |
fix(files): read a file nobody vouched for, as an untracked one already is (#6986)
* fix(files): read a file nobody vouched for, as an untracked one already is A workspace file whose secret provenance could not be recorded was refused by every model and runtime boundary at once — attachments, mounts, and the tool routes that parse, transcribe or describe a file. A file that was never tracked at all returned exact-empty and worked fine. Both say exactly as much about their contents: nothing. So the refusal was a permanent penalty for having tried to record provenance and failed, with no way back, since nothing rewrites a file's provenance but another content write. Workspace files now do what the other durable surfaces do — proceed and record an audit entry naming the surface and the count, so the people who own the secrets are told. The state is distinguished from the four that really are unknown (missing row, version mismatch, stale binding, malformed), which stay refused; only a current sidecar that says so reads as unrecorded. Absence survives a merge rather than dropping through to exact, so a derived file cannot launder an unrecorded input into a positive claim. Script migration 0007 clears the files already stranded in the old state, taking the parent lock first in id order to match the live writer, with the unknown status re-checked under it so a concurrent write is never undone. Nothing changes for a legacy file: no sidecar row still means untracked, still reads exact-empty. The enforcement flag stays in place. * fix(files): narrow an initialization to a status the column accepts The union carries three states; the sidecar's CHECK constraint accepts two. Only the replace path discriminated — initialize forwarded the status verbatim, so an 'unrecorded' would have reached the database as a value it rejects, aborting the enclosing transaction rather than writing a bad row. Nothing passes one today, which is why it belongs here and not in a caller. Also names the storage mismatch on the type itself: a stored 'unknown' reads back as 'unrecorded', not as the union's 'unknown'. Same word, two layers, different meanings. * fix(provenance): repair only what the reader calls an absence The repair selected on status alone, which is wider than the branch it was meant to undo. The reader answers 'unrecorded' only for a sidecar that is version 1, bound to the file's current bytes, and holding a well-formed entries array; anything else is a fault it refuses and no policy relaxes. Clearing a fault was not a smaller claim but a larger one. Deleting the sidecar sets secret_provenance_version to NULL, and a NULL version reads as exact-empty — so a file refused because its provenance described bytes it no longer holds would have come back positively vouched for, silently, with no audit entry. Four such rows exist in production today. Both the candidate query and the delete now carry every condition, so a concurrent write cannot lose the distinction between an absence and a fault. Stale, unversioned and malformed sidecars stay refused exactly as the surface refuses them, and recover as they always have, on the next content write. * fix(provenance): store an absence apart from a refusal Relaxing a stored `unknown` assumed it always meant "nobody recorded this". It did not. The sidecar collapsed two opposite claims into one value: bytes nobody recorded, and bytes a writer refused on purpose — a child of an archive whose parent provably held secrets, a generated asset whose safety decision came back false, a transcode whose scanner knew secrets were present and could not locate them. Relaxing the second would have walked known-secret-bearing content through every model boundary at once. The decision type already told them apart: a registry that never ran is `safe: true`, while every refusal is `safe: false`. Only storage lost it. Writers now persist `unrecorded` and `unknown` separately, readers relax the first and refuse the second, and the constraint is widened to accept it. Only workspace files store this distinction, deliberately. Every other durable surface produces a non-exact sidecar from one condition — an incomplete incoming bundle or registry — which is always an absence. Files are the only surface that derives one stored object from another, so they are the only one that can refuse on purpose. The shared policy is unchanged and uniform: read an absence and audit it, refuse a taint. Also aligns the metadata batch classifier, which answered `unknown` where the single-file reader answers `unrecorded`, leaving two classifiers describing one policy differently. * test(provenance): assert the enforcement switch on a row it can act on The test that pins "closing the surface refuses an unrecorded file again" used a stored unknown, which the split had just turned into a taint. A taint is refused whatever the flag says, so the assertion held with enforcement off too and proved nothing about the switch this posture rests on. It now uses a recorded absence, and fails if the flag is ignored. * fix(files): copy a recorded absence as an absence The copy path required an exact source, so a stored unrecorded one fell in with the refusals and the target was written as a taint the source never carried. A workspace fork or a chat file copy therefore turned a readable file into a permanently refused one — the pathology this surface exists to undo, reached by copying — and nothing rewrites a file's provenance but another content write. Safe past the scope checks it now precedes: those exist to stop one workspace's secret entries landing in another's file, and a recorded absence has no entries to carry. The legacy Function export marker keeps writing unknown. Its record had resolved secret names in scope and an unreadable bundle, so whether that is an absence is not something this change can establish, and guessing in the readable direction is what caused this round. |
||
|
Justin Blumencranz
|
fb8f0d66d1 | improvement(files): improve file sharing UI (#6983) | ||
|
Vikhyath Mondreti
|
8c7a2f1df0 |
fix(uploads): require an explicit byte ceiling on workspace-file downloads (#6985)
* fix(uploads): require an explicit byte ceiling on workspace-file downloads Workspace files are admitted at 5 GB because they stream straight to object storage, but a tool that pulls one back to hand it to a third party buffers the whole thing in the shared app process. maxBytes was optional on every download helper, so 51 call sites had silently inherited "unbounded". Make maxBytes required on all five entry points so a new call site cannot inherit it again, and give each existing site a ceiling: the destination's own documented limit where the route already declared one, otherwise the 100 MB this codebase already uses for buffered work. Multi-attachment routes were the worse case — Gmail, Outlook, SendGrid and SMTP downloaded every attachment via Promise.all and only summed the sizes once they were all resident, so their pre-check on declared sizes protected nothing. Add downloadServableFilesWithinBudget, which walks the list against a shrinking budget, and use the same running budget in Slack, Jira, Discord and Quiver. * fix(uploads): bound Sim-page asset inlining before the bytes are resident The ceiling on the rendered page checked the finished document, by which point renderSimPageDocumentWithAssets had already downloaded every referenced image concurrently with no per-download limit and base64-inlined them — so the allocation the check exists to prevent had already happened. Pick the inline set from recorded sizes before fetching anything, against a per-document budget as well as the existing per-image one, and give each download its own ceiling in case a row understates its object. An image that does not fit keeps its URL reference, exactly as an oversized one already did. * improvement(uploads): charge the page-render budget by delivered bytes Planning the inline set from recorded sizes left the aggregate ceiling resting on metadata being accurate, and needed a paragraph explaining why that was safe. Downloading one image at a time and subtracting what each download actually returned needs no such argument: the budget cannot be exceeded whatever a row says, and the peak is one image rather than the sum of them. Also drop two rough edges the first pass left behind — an SFTP total-size check that became unreachable once the download carried the remaining budget, and a Dataverse error helper whose optional size argument existed only to paper over one caller that had not passed it. |
||
|
Vikhyath MondretiandClaude Opus 5
|
28cfa4361a |
fix(deploy): resolve subblock values to their configuration on both sides of change detection (#6984)
* fix(deploy): resolve subblock values to their configuration on both sides of change detection Focusing a deployed trigger block flipped the deploy button from Live to Update for a change the user never made, and redeploying could not clear it. Deploy materializes every declared `defaultValue` into `webhook.providerConfig` (`getConfigValue`). Opening a trigger block's panel reads that derived artifact back into live state through a non-persisting `setValue`, so the block gains keys the DB draft — which deploy snapshots — does not have. The comparison then reported a difference for a field nobody set. Because deploy snapshots the draft, the next deployment lacked the keys too: a fixpoint. Measured on production: 89 of 89 recent `generic_webhook` deployments were missing `acceptOtherMethods`, and 1,046 of 2,139 active webhooks carry at least one `providerConfig` key their block has no entry for. This is the tenth instance of one failure mode — |
||
|
Vikhyath Mondreti
|
b861062370 |
fix(provenance): clear the rows that went unknown after the first repair (#6982)
* fix(provenance): clear the rows that went unknown after the first repair 0005 is finished and will not run again — the runner records a name in script_migrations and never offers it back, which is the contract a run-once repair wants. But it cleared the backlog that existed at the instant it ran, and the writer that produced that backlog kept running until the fix in this branch. Nothing heals such a row in place, so each one goes on reporting on every later read; a few dozen of them account for thousands of log lines a week. A second entry rather than deleting the first's tracking row: the registry is append-only, and a repair that ran twice should say so twice. It shares 0005's walk rather than restating it — the parent-first lock ordering and the status re-check under that lock are subtleties worth having once — and is idempotent, so it costs one empty query if there is nothing left to repair. * chore(provenance): record the deploy ordering the second pass depends on promote-images needs migrate, so a script migration runs while the previous image is still serving. A row an old instance creates between this walk and the end of the rollout sits behind the cursor, and the name is recorded on success, so it is never offered again. Widening the walk would not help — the exposure is the minutes after it returns, not the milliseconds during — so the requirement is to ship it in a release after the writer fix is already promoted, which the file now says. |