mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-31 03:47:12 +08:00
aba681133aec6fe2387e3aea7eca33fc2e2a48e2
6475
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Waleed
|
aba681133a |
refactor: remove five more dead prop chains (#7046)
* refactor(auth): drop the isProduction prop nothing on the signin path reads Same shape as the `isWorkflowRunning` removal: declared, required, threaded through every layer, and never read at the end of the chain. `SocialLoginButtons` declares `isProduction: boolean` as a REQUIRED prop and never reads it, so every caller had to produce and forward a value that was discarded. Neither `login-form` nor `signup-form` reads it either — each only declares it, destructures it, and passes it down. `signup-form` forwards it twice, through its own inner `SignupFormContent` hop. With the chain gone, `getOAuthProviderStatus` has no consumer for the `isProduction: isProd` it returned: the pages destructured it only to forward it, and `/api/auth/providers` already takes just the three availability flags. So the return value and its `isProd` import go too. `isProduction` stays alive where it is genuinely used — `verify-content.tsx` branches on it and hands it to `useVerification`, and imports `isProd` directly rather than through this helper. That path is untouched. Found by the rule enabled in #7037: it was the only `.tsx` unused-parameter warning in `apps/sim`. (cherry picked from commit |
||
|
Bill LeoutsakosandBill Leoutsakos
|
445ef62880 |
fix(oauth): bind update access to selected credential (#6999)
* fix(oauth): bind update access to selected credential * fix(oauth): guard unresolved connector credentials * fix(oauth): clear stale connector return context * fix(oauth): fail closed when reconnect target disappears * fix(oauth): wait for reconnect credential lookup * fix(oauth): refresh resolved connector credential --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
Justin Blumencranz
|
472532e1a3 | feat(tables): confirm view deletion (#7041) | ||
|
Waleed
|
6d313a4a94 |
refactor(renderer): drop the isWorkflowRunning prop the views never read (#7040)
Removing only the unused binding and keeping the prop was half a fix. The views declared it, the app passed it, and nothing read it — so the prop was dead, and dead code does not become live by being documented. Its TSDoc claimed it "holds every block's action swell open". That behavior does not exist in either view. Keeping the prop on the chance someone wants it later is the speculative-generality smell: if the toolbar should pin open during a run, that gets implemented deliberately and the prop comes back with logic behind it. Removed from both view interfaces and from both call sites. The store subscription stays — `workflow-block.tsx` and `subflow-node.tsx` each passed the same value twice, once to the dead view prop and once to `ActionBar`, which has 28 real reads and is what the surrounding TSDoc is actually describing when it says the flag "only swaps Run for Stop and disables mutations". `workflow-edge-view` uses it too and is untouched. The renderer test that passed it loses the argument. Worth noting it set the flag to stage a workflow run, and since the view ignored it those two cases were never exercising the run state they name. |
||
|
Vikhyath Mondreti
|
cc611180be |
improvement(provenance): attribute stored-envelope display reads to their execution (#7039)
* improvement(provenance): attribute stored-envelope display reads to their execution A display materialization of an execution log imports the row's stored provenance envelopes into throwaway registries, and each import of an incomplete envelope re-emitted the registry's own summary — per envelope, per view, carrying counts and a workspace but never the execution id. A reader repeatedly materializing the same stored rows produced hundreds of identical lines that could not say which executions to go look at, and the volume scaled with views of a state that was fully recorded when the run wrote it. Verified against production before changing anything: essentially no new incomplete envelopes are being stored since the writer fix shipped, and no data drains exist — the stream is bounded re-reads of old rows through the display paths, not a live producer. The display registries are now staged — the existing concept for a registry that filters one value for a caller that reports against the real boundary — and each display function reports once per materialization with the execution id, workflow, workspace, and the parts that could not be vouched for. Severity is preserved: an incomplete stored envelope stays at warn, a malformed one stays at error. Projection behavior is unchanged everywhere — incomplete and malformed envelopes still fail their values closed exactly as before; only the reporting moves to the boundary that knows the execution. * improvement(provenance): fold the incomplete-envelope predicate and pin dual-site reporting Review pass over the previous commit: one helper instead of three copies of the incomplete-envelope check, the staged TSDoc generalized to cover both of its uses, and the block-outputs entry point's two-site reporting of one run envelope documented and pinned rather than left implicit. * improvement(provenance): classify every unusable stored envelope at the display boundary Review findings from the first round, both accepted: a present-but- malformed block or run envelope was withheld with no attributed line, and a complete envelope whose entries fail decryption latched the staged registry with only the unattributed entry-level error. Fault classification moves into the one import helper the display paths share, which now returns the registry and the fault together: absent is not a fault, unparseable is malformed, unable-to-vouch is incomplete, and a complete envelope whose registry latched during import — entry decryption is the only latch on that trusted path — is undecryptable. Every consumer reports through the same table, severity per kind, so the exact-value loop stops being the only site that could name a malformed envelope. Withholding behavior is unchanged at every site. |
||
|
Vikhyath Mondreti
|
04380b79a5 |
fix(webhooks): requeue deliveries dropped by retryable setup infrastructure failures (#7038)
* fix(webhooks): requeue deliveries dropped by retryable setup infrastructure failures * fix(webhooks): restore terminal log on failed requeue and make retry backoff abort-aware |
||
|
Waleed
|
cae80c5f20 |
chore(lint): turn on the rules that would have caught the dead code (#7037)
Three rules were off, so nothing enforced them. Measured, fixed the sites, and enabled them where the cost is bounded. `noAccumulatingSpread` — 2 violations, both real O(n²) reducers, both now `Object.fromEntries`. One duplicates a block's subBlocks on every block duplication; the other rebuilds a Record from every workspace env var. Enabled repo-wide. `noUnusedVariables` / `noUnusedFunctionParameters` — 633 repo-wide, but only 6 under `packages/`. Fixed those 6 and enabled both at error for `packages/**` via an override, which permanently covers 979 files. `apps/sim`'s remaining 627 are left deliberately: that is a sweep of its own, and a rule enabled with 627 outstanding warnings teaches people to ignore it. This is the class of rule whose absence let the dead code in #7019 accumulate — eleven unread loggers, a whole unimported file, write-only locals — none of which any gate could see. Two of the six were in `workflow-renderer`, where the fix is narrower than it looks. `isWorkflowRunning` is destructured-but-unread in both the block and subflow views, and the app passes it from `workflow-block.tsx` and `subflow-node.tsx`. Its TSDoc claimed it "holds every block's action swell open"; nothing reads it, so that behavior does not exist. Removing the prop breaks the callers and implementing it is a UX decision — there is adjacent logic deliberately not pinning the toolbar during a handoff. So only the unused binding goes, and the TSDoc now says what is true. Not enabled: `noDocumentCookie` (3 sites, and its fix is the CookieStore API, which is a browser-support call) and `useExhaustiveDependencies` (384 errors). |
||
|
Waleed
|
edf07ec3cd |
fix(vllm): support LM Studio endpoints (#7036)
* fix(vllm): support LM Studio endpoints * fix(vllm): validate compatible base URLs * fix(vllm): guard discovery URL validation |
||
|
Waleed
|
fbeea53bec | fix(files): normalize encoded embedded ids (#7035) | ||
|
Waleed
|
efe8a14bcc |
fix(ci): give push builds a base their audits can actually read (#7033)
Push builds fail the migration audit:
✗ Migration safety check could not run.
Cannot diff against 'HEAD~1'.
`actions/checkout` sets no `fetch-depth`, so it defaults to 1 — a single-commit
clone in which `HEAD~1` does not resolve. Both diff-based audits named `HEAD~1`
as their push base, so neither has ever had a base to read. The migration audit
answered that with `✓ No new migrations to check` and exit 0, so it had never
run on a push build at all; #7022 made it say it could not run instead, which is
what surfaced this. The block-registry check reports `⚠ … skipping` on the same
input — visible, and equally never run.
`HEAD~1` was the wrong base regardless. It names the last commit, so a push
carrying several commits audits the tip and lets every earlier commit through:
3-commit push, HEAD~1 base: mig3.sql
3-commit push, before base: mig1.sql mig2.sql mig3.sql
The base is now `github.event.before` — the tip the branch had before the push,
which is what GitHub provides for exactly this. It is fetched by SHA at depth 1;
the audits diff two tips and need no common ancestry between them. Resolved once
in a step both audits read, so the two cannot drift apart.
`HEAD~1` survives only as the fallback for an all-zero `before` (a new branch,
with no predecessor to diff), which is what `fetch-depth: 2` now covers.
Verified: both audits accept a raw SHA base and pass; the multi-commit case above
is a real reproduction, not a description.
|
||
|
|
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 |