feat(files): preview HEIC photos in the file viewer - #6350
Merged
Conversation
The agent can read HEIC since #6346, but the Files page still showed 'Preview not available' — an <img> pointed at the serve route got the stored HEIF under nosniff, which no browser outside Safari renders. The serve route now resolves a JPEG derivative for HEIF bytes, cached in the artifact store and keyed by the source's storage key. Workspace keys are regenerated on every content replacement, so the key is already a content version and using it avoids streaming the original just to hash it. Caching matters here in a way it did not for the vision path: a preview is re-fetched on every view and the WASM decode costs roughly a second for a phone photo. The original stays the stored object — downloads and raw=1 serve it untouched, so this never changes what a user gets back. compileDocumentIfNeeded becomes resolveServableBytes, since it now resolves images as well as generated documents. .tif/.tiff stay download-only: nothing decodes those on either side.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The workspace
The UI treats Reviewed by Cursor Bugbot for commit 9165266. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThe PR adds cached JPEG derivatives for HEVC-based HEIF previews while preserving original bytes for downloads and raw requests.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/uploads/server/image-derivative.ts | Adds storage-keyed loading, generation, and best-effort caching of JPEG derivatives for HEVC-based HEIF images. |
| apps/sim/app/api/files/serve/[...path]/route.ts | Resolves image derivatives only for preview requests while retaining raw and download behavior. |
| apps/sim/app/api/files/public/[token]/content/route.ts | Adds preview-only derivative resolution to public shared-file content responses. |
| apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/image-preview.tsx | Requests preview derivatives and replaces images that fail to load with the unsupported-preview state. |
| apps/sim/lib/uploads/server/heic.ts | Separates HEVC-specific detection from broad HEIF detection and bounds compatible-brand scanning. |
| apps/sim/hooks/use-file-content-source.tsx | Propagates the preview marker through authenticated and public file-content URLs. |
Sequence Diagram
sequenceDiagram
participant UI as ImagePreview
participant Route as File serve route
participant Cache as Derivative cache
participant Decoder as HEIF decoder
UI->>Route: "GET content?preview=1"
Route->>Cache: Load derivative by storage key
alt Cached JPEG exists
Cache-->>Route: JPEG bytes
else Cache miss
Route->>Decoder: Transcode HEIF to JPEG
alt Transcode succeeds
Decoder-->>Route: JPEG bytes
Route->>Cache: Store derivative
else Transcode fails
Decoder-->>Route: No derivative
Route-->>UI: Original bytes
UI->>UI: onError renders unsupported fallback
end
end
Route-->>UI: Renderable response
Reviews (7): Last reviewed commit: "improvement(copilot): only ask for a pre..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…n image Five issues from review, all interlocking around one decision. The derivative is now requested with preview=1 rather than suppressed with raw=1. raw=1 would have corrupted generated-document downloads: every non-markdown workspace download routes through the serve route and relies on resolveServableDocBytes compiling stored source into the real binary. Opt-in separates the three consumers cleanly — previews get the JPEG, downloads get untouched stored bytes, and doc compilation stays unconditional. - Public shares resolve the derivative too, with the same preview/download split; the viewer requests it, the download button does not. - Split the brand predicate. isHeifContainer stays broad for the vision path, where it only runs after sharp has already failed. The serve path runs first, so it uses isHevcHeifContainer — an AVIF was costing a storage round-trip, a WASM load and a misleading warn per request. - A derivative that cannot be produced (past the 20MB ceiling, or a decode failure) now falls back to 'Preview not available' instead of a broken image. UnsupportedPreview moved to preview-shared to avoid a module cycle. - The chat composer chip requests the derivative, so HEIC attachments stop rendering as broken thumbnails.
Collaborator
Author
|
Pushed a83cfce addressing all four threads plus a fifth issue none of them flagged. The fifth one was the most severe and it changed the design. So the derivative is now opt-in via
Also fixed the chat composer chip ( 1302 tests pass, typecheck and Still not verified in a browser — worth loading an actual |
Collaborator
Author
Collaborator
Author
|
@cursor review |
An overwrite preserves the storage key, which is what the parent keys this component on, so only the URL version changes and it never remounts. The previous bytes' outcome therefore stuck, leaving a replaced image parked on 'Preview not available' until something else forced a remount. Reset on URL change during render rather than in an effect — this is derived state, and an effect would render the stale outcome first.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 76e496d. Configure here.
…d scan
- Content writes mint a new storage key, so the parent's key={file.key}
already remounts ImagePreview; the render-phase reset was unreachable and
made renames flash a loading overlay.
- Clamp the ftyp compatible-brand scan to a real box size. The declared size
is attacker-controlled and this now runs on every preview request.
- UnsupportedPreview takes a primitive name so memo is load-bearing.
- Fix the hardcoded ? in the public preview URL builder.
Collaborator
Author
Collaborator
Author
|
@cursor review |
…bnails A video has no derivative path, so preview=1 there only spent a brand sniff per request. Adds the missing test coverage for the helper.
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9165266. Configure here.
Collaborator
Author
|
@greptile The 4/5 rests on a premise that is factually incorrect for this codebase: that a content replacement is written under the same storage key. It is not — every byte-changing write mints a brand-new key, so the parent's Evidence:
I audited every This is also why An earlier reply on this PR did claim the fix landed in Separately, this round did fix two real issues: the |
Collaborator
Author
|
@cursor review |
Contributor
|
You're right, and the evidence is conclusive. The premise behind that concern was wrong. The three pointers in The earlier reply on this PR that claimed otherwise was incorrect on the mechanism, and the revert of The two real fixes you call out — the |
Collaborator
Author
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9165266. Configure here.
TheodoreSpeaks
added a commit
that referenced
this pull request
Aug 10, 2026
* fix(logger): stop a server-side jsdom window from silencing all logging in production (#6339)
* fix(logger): stop a server-side jsdom window from silencing all logging in production
* fix(logger): widen the stubbed process cast so type-check passes
* fix(logger): never let structured serialization throw into the caller (#6331)
* fix(logger): never let structured serialization throw into the caller
In production the JSON branch merged caller-supplied arguments into the log
entry and stringified it with no error handling. A cyclic reference, a BigInt,
or a throwing getter in that metadata raised a TypeError out of `logger.info`
and friends: the line was lost and the caller's code path aborted.
Dev was unaffected — the colorized branch already routes objects through
`formatObject`, which catches — so this class of bug is invisible locally and
only surfaces in production, where it reads as structured logs disappearing
while raw stack traces keep shipping.
Build and serialize through `serializeEntry`, which falls back to a
cycle/BigInt-tolerant replacer and then to a minimal entry flagged with
`serializationError`.
* fix(logger): keep hostile child metadata from throwing into the caller
* fix(logger): keep a throwing toJSON from escaping the final fallback
* fix(logger): keep repeated references out of the circular-reference fallback
* fix(scripts): make the sql Date-binding audit precise and crash-proof (#6340)
* fix(scripts): make the sql Date-binding audit precise and crash-proof
Resolve the drizzle `sql` tag from its import binding, scope Date bindings
lexically, tolerate unparseable files, accept the allow annotation above a
multi-line template, and scan the root scripts directory.
* fix(scripts): honor shadowed bindings and defaulted destructured Dates
* fix(scripts): audit drizzle sql tags bound through a dynamic import
* chore(scripts): drop the sql Date-binding unit tests and the exports that served them
* chore(scripts): drop the script unit tests and the exports that served them
* fix(files): render audio and video stored as application/octet-stream (#6341)
* fix(files): render audio and video stored as application/octet-stream
The file viewer built the blob backing <audio>/<video> from the record's stored
content type with a truthiness fallback, so a stored application/octet-stream
was passed straight through and the element could not determine the format.
Downloading the same file worked because the download path derives its content
type from the filename.
- Add resolveEffectiveMimeType, which resolves a generic stored type against the
filename, and use it for the media blob, the type column, and the type filter
(an octet-stream video was also invisible to the Audio/Video/Image filters)
- Map .webm to video/webm rather than audio/webm: a <video> element plays an
audio-only stream, an <audio> element drops the picture
- Preview .bmp, .avif and .ico, which upload accepts but the viewer sent to the
download-only path; serve them with their real content type so nosniff does
not block them. .tiff and .heic stay unsupported - no browser renders them
- Open .jsonl in the text editor, and fill the extension-to-mime gaps for
.mmd, .diff, .patch and .fish
* fix(files): settle the audio/video container ambiguity at the call site
Follow-up to the review pass on this branch.
- Revert the global .webm -> video/webm remap. EXTENSION_TO_MIME is shared with
non-viewer callers, and a .webm with an empty stored type would have started
taking the STT route's video branch (stt/route.ts:211 -> extractAudioFromVideo),
which 500s where no ffmpeg binary is on PATH. The ambiguity is now settled in
resolveMediaMimeType, which knows which element the caller is rendering
- Resolve the public share route's Content-Type from the filename via
getContentType, matching the workspace serve route, instead of echoing the
client-declared stored type into a public unauthenticated response. Add the
audio/video entries contentTypeMap was missing so a shared media file keeps a
real Content-Type (disposition is unchanged - none are inline-safe)
- Make resolveEffectiveMimeType total (string, not string | null); the null
contract only bought one label edge case and cost a ?? at every call site,
one of which was dead
- Drop .jsonl from the text-editable set. The editor loads the whole file and
only CSV has a byte cap, so a large .jsonl would trade a download-only
fallback for a crashed tab. Needs the size guard generalized first
- Trim two comments that restated their code
* fix(files): resolve dual audio/video containers to the kind the app presents
The viewer routes .webm to the video player, but the Type column and the
audio/video filters resolved it through EXTENSION_TO_MIME and read audio/webm,
so one file showed as Audio and opened in a <video>.
resolveEffectiveMimeType now consults a DUAL_CONTAINER_MIME map first. It stays
out of EXTENSION_TO_MIME because the speech-to-text and ElevenLabs routes read
that table directly, where a video/* label pushes a .webm into ffmpeg audio
extraction it does not need.
* fix(files): keep the dual-container video default out of the persisted type
resolveFileType writes user_file.content_type, and it delegated to
resolveEffectiveMimeType, so DUAL_CONTAINER_MIME could persist video/webm. The
speech-to-text route reads that back as file.type, which sends the upload into
the ffmpeg extraction path the previous commit set out to avoid.
resolveFileType now resolves through EXTENSION_TO_MIME alone; the video default
stays on the presentation path. Both share an identifiesFormat predicate.
* fix(deployment): prevent trigger registry initialization crash (#6342)
* fix(deployment): initialize block registry before triggers
* fix(triggers): break the triggers <-> blocks initialization cycle
Replaces the import-order guard from the previous commit with the structural fix.
Block configs spread `getTrigger('...').subBlocks` while their module body runs, so
`blocks/*` depends on `triggers/*` by design. Thirteen edges closed the loop back the
other way, which made module evaluation order load-bearing: enter the graph through
`@/triggers` and a block config calls `getTrigger()` before `TRIGGER_REGISTRY` is
initialized, throwing
ReferenceError: Cannot access 'TRIGGER_REGISTRY' before initialization
Eleven deployment routes crashed on import: `POST /api/workflows/[id]/deploy`, the v1
public and admin deploy/rollback/activate routes, both deployment-version routes, and
the three custom-tool deployment routes. All of them funnel through
`lib/webhooks/deploy.ts`, which stayed safe only because it imported a value from
`@/blocks` — biome sorts that above `@/triggers`, so the safe barrel always evaluated
first. #6272 deleted that import as unused cleanup and took the whole surface with it.
The reverse edges came from two places, both layering violations rather than anything
inherent to triggers:
- `triggers/index.ts` imported the mock-payload generator from `trigger-utils`, which
imports `@/blocks` for unrelated helpers. The generator is pure, so it moves to
`lib/workflows/triggers/mock-payload.ts` and both callers import it there.
- Eleven trigger modules statically imported the editor's Zustand stores to read
sub-block values inside `fetchOptions`/`fetchOptionById`. Those reads now go through
`triggers/editor-state.ts`, which loads the stores with a dynamic `import()` —
resolved when the resolver is called, not during module evaluation, so it carries no
initialization-order obligation.
Side effect: `@/triggers` drops from 744 statically reachable modules to 526. The block
registry, the workflow Zustand stores and their React Query graph are no longer pulled
into every server module that imports a trigger.
`scripts/check-trigger-block-cycle.ts` fails the build if a static edge returns, and
reports the shortest offending chain. The existing suite could not have caught this —
`deploy.test.ts` mocks both `@/blocks/registry` and `@/triggers`, and `vitest.setup.ts`
mocks `@/blocks/registry` globally, so it passed 18/18 against the broken code.
---------
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
Co-authored-by: Waleed Latif <walif6@gmail.com>
* fix(chat): stop chats storing a resource they can never send with (#6344)
* fix(chat): stop chats storing a resource they can never send with
A chat resource persisted with a blank id made every later message fail:
the write contract accepted `id: ''` while the send schema required
`min(1)`, so the request 400d before a stream existed and the client's
reconnect 404d. The tab could not be removed either, since the delete
route requires a non-empty id. Twelve production chats were in this state.
The id came from an agent-written file chip that carried only a filename:
the client filled the missing id with `''` when the file was absent from
its list, which it always is for a file the agent just created.
- model the unresolved state (`WorkspaceResourceRef`) instead of faking an
id, and resolve chip refs at one choke point that may refuse
- close the stale-cache race by fetching the file list before giving up,
so clicking a just-created file opens it instead of doing nothing
- reject blank ids at the stream, write and send boundaries, and drop them
wherever stored resources are read, which self-heals affected chats
- collapse the 5-6 duplicate POSTs every resource add was firing
- log rejected chat bodies, which previously left no trace at all
* fix(chat): require a file chip's reference to resolve before opening it
A rendered link collapses a resource's id and path into one href, so the
click handler cannot tell them apart. Classifying on a separator got a
bare filename in `path` wrong, and the resolver then trusted it as an id
— opening and persisting a tab pointing at nothing.
Drop the classifier and let the resolver try each candidate as an id, a
VFS path and a unique name. A file ref must now match a record the
workspace actually has; the stale-list case is covered by the refetch,
so an id that never resolves was never an id.
* fix(chat): tell the user when a resource chip resolves to nothing
The chip renders as a button with a hover state, so refusing to open it
silently reads as a broken control. Say what happened instead.
* fix(chat): do not report an unreachable workspace as a missing file
A failed refetch and a successful one that found nothing were both
collapsed to an empty list, so a network blip told the user the file does
not exist. Keep the two apart and say which happened.
* feat(embeddings): multi-provider Embeddings block on a shared core (#6317)
* feat(embeddings): multi-provider Embeddings block on a shared core
The Embeddings block was OpenAI-only with a bare fetch: no batching, no
retry, no metering, and no hosted-key support. Meanwhile the knowledge-base
indexing path already had a real multi-provider engine. Nothing bridged the
two, so the block could not reach Gemini and the KB engine could not be
reached from a workflow.
Extract the shared core into lib/embeddings/ first, then build breadth on
top of it, so both the KB path and the block resolve models and providers
from one catalog and one set of adapters instead of a third parallel
implementation.
- lib/embeddings/: catalog, client, key resolution, batching, L2
normalization, and adapters for OpenAI, Azure OpenAI, Gemini, Cohere,
and Mistral
- lib/knowledge/embeddings.ts becomes a thin KB wrapper with its exported
signatures unchanged; the 1536-dimension vector invariant does not move
- one tool per provider from a shared factory, behind a single
/api/tools/embeddings route and contract
- new `embeddings` block type; the `openai` block is left functionally
untouched and only leaves the discovery surfaces via hideFromToolbar
plus sunset.replacedBy, so placed instances keep working unmigrated
- openai_embeddings is now an alias of embeddings_openai, so legacy
instances pick up batching, retry, and metering with no visible change
* fix(embeddings): report an unsupported dimension as a client error
The route validated the model and the provider match up front but left
`dimensions` to be checked inside embed(), where resolveDimensions throws
and the generic catch maps it to 502. A typo in the block's dimension
field, or a reference expression resolving to an out-of-range value, was
reported as an upstream gateway failure rather than bad input.
Resolve dimensions in the route alongside the other boundary checks and
return 400. The throw stays the single source of the message, so the two
call sites cannot drift.
Adds route tests covering auth, the response shape, each boundary
rejection, input normalization, and the 502 path for genuine provider
failures.
* fix(embeddings): only send a dimension when the caller asked to reduce
resolveDimensions() returns the model's native size when no reduction is
requested, and that resolved value was handed straight to the adapter. The
adapters guard on `dimensions !== undefined`, so the field was always
populated and always sent.
Models that support Matryoshka reduction accept their own native size, so
this was invisible for text-embedding-3-*, gemini-embedding-001,
embed-v4.0, and codestral-embed. Models that do not support the parameter
at all reject it outright: every unreduced request to text-embedding-ada-002
and mistral-embed failed with a 400, which is both of the models whose
catalog entry has no supportedDimensions.
Track the caller's explicit reduction separately from the resolved
dimensionality. The resolved value still drives reporting and billing; only
the requested one reaches the wire.
Found by driving the live provider matrix against all four providers.
* test(knowledge): de-flake the sync-engine suite
Every test dynamically imported the module under test, so the first one to
run paid the whole cold-load cost inside its own 10s timeout and failed
intermittently under load.
The dynamic imports were working around a hoisting problem: mockMapTags is
a top-level const read by a vi.mock factory, and vi.mock is hoisted above
it, so a static import of the module under test crashes with a
use-before-initialization error. Declaring the mock through vi.hoisted()
removes that constraint, which is the pattern the testing guidelines
already call for.
One static import replaces 42 dynamic ones. The file drops from ~15s to
~2s and passed 5 consecutive runs.
* fix(embeddings): drop a capability the selected model no longer offers
The per-model Dimensions and Task Type dropdowns each share one subblock
id, and nothing clears a stored subblock value when its dependsOn fields
change — dependsOn only feeds rendering. A choice made for one model
therefore outlives a switch to another.
Picking 3072 on text-embedding-3-large and switching to -3-small left 3072
stored while the dropdown offered at most 1536, and the block forwarded it.
Same for a task type: 'similarity' chosen on Gemini survived a switch to
Cohere, which has no equivalent input type.
The guards only checked that the model declared the capability at all, not
that the value was one it lists. Check membership so a stale value falls
back to the model's native size, or is omitted, instead of being sent and
rejected. The user cannot have deliberately chosen an option the dropdown
stopped presenting.
* feat(embeddings): use the latent-constellation mark for the block icon
Replaces the scatter-plot-on-axes placeholder with a centre node, four
neighbours, and the rays between them — a point and its nearest neighbours
in embedding space, which is what the block actually produces. The axes
mark read as a generic chart and said nothing specific to embeddings.
Nodes are filled so they hold their shape at small sizes. The rays carry
less weight than the nodes to keep the hierarchy, but at 1.6/0.9 rather
than the 1.4/0.75 they were drawn at, so they do not thin out to loose
dots in the 14px block-search row.
Kept byte-identical between the app and docs icon sets.
* fix(embeddings): declare the outputs the legacy openai block returns
openai_embeddings became an alias of embeddings_openai, so the legacy
block's runtime payload gained `provider` and `dimensions`. Its declared
outputs still listed only embeddings/model/usage, so the tag picker never
offered two fields every run demonstrably returns, and downstream blocks
could not reference them.
Declaring them is additive and does not touch execution. Asserts the
legacy block's output keys match the replacement's, since both run the
same tool and neither should expose fields the other lacks.
* fix(copilot): resolve same-id subblock variants before validating
A block may declare one field id several times, each variant conditioned
on another field — the embeddings block declares model, dimensions, and
taskType once per provider, and the image and video generators do the
same. Validation keyed a map by id alone, so whichever variant was
declared last silently became the validator for every write to that
field.
Programmatic edits to an embeddings block were therefore checked against
Mistral's option lists whatever the saved provider: `text-embedding-3-small`
was rejected as not one of mistral-embed/codestral-embed, and dimensions
valid only elsewhere (3072, 768) could not be set at all. Values that
happened to overlap the last variant passed, so automation saw partial
success rather than a clean failure.
Keep every candidate per id and pick the one whose condition holds,
evaluating against the mutation's inputs merged over the block's saved
values so a partial write still resolves. When no condition matches, fall
back to the union of all variants' options rather than guessing.
Conditions still never gate whether a field may be written — that was a
deliberate choice and a hidden field stays writable. They only select
which definition describes the field, and an unresolved condition widens
the accepted set instead of narrowing it.
* fix(copilot): prefer a conditioned variant over an unconditioned catch-all
An unconditioned same-id variant matches every set of values, so it would
shadow a genuinely selected variant purely by being declared first. Prefer
a variant that actually asserted something about the current values.
No block in the registry currently declares a catch-all ahead of a
conditioned variant on a field where it would change validation, so this
is a guard against the pattern rather than a fix for a live case.
* chore(embeddings): scope this branch to the multi-provider block
Two changes made while building the Embeddings block are not part of it and
ship separately, so their files are restored to staging here:
- copilot edit-workflow validation resolving same-id conditional subblock
variants. The embeddings block surfaced it, but it is a platform fix
affecting ~20 blocks that declare a field id more than once, and it
narrows what programmatic edits accept — that deserves its own review.
- the sync-engine test de-flake, which is unrelated test hygiene.
Both are preserved in full on feat/embeddings-full-snapshot.
Note this restores the reported bug where a programmatic edit to an
embeddings block validates model/dimensions against the last-declared
provider variant. The block is unaffected in the editor and at runtime.
* fix(embeddings): honor per-model token limits and bound the JSON input path
Review round 1.
Batching used one 8,000-token constant for every model, inherited from the
knowledge-base engine this branch extracted. `batchByTokenLimit` truncates
any single text above the limit it is given, so that constant both sent
oversized input to models with a lower ceiling and silently dropped content
models with a higher one accept:
- Gemini declares 2,048, so a 3,000-token text passed through whole and the
provider rejected it, surfacing as a 502. This also affected knowledge-base
indexing on staging, which uses the same constant.
- Cohere declares 128,000, so anything past 8,000 was truncated for no reason.
Batch against the selected model's own `maxInputTokens` instead. Using the
per-input ceiling as the per-batch budget also keeps every individual text
within it.
The contract bounds the array arm of `input`, but a JSON-encoded array
arrives as a plain string and `normalizeInput` only expands it after
validation — so neither the 1,000-input cap nor the non-empty checks applied
to the reference-expression path the route was written to accept. `"[]"`
also reported success with no vectors. Re-check the normalized list so the
bounds hold for both shapes.
* chore(embeddings): regenerate tool metadata for the new embedding tools
CI's tool-metadata:check gate failed: registering embeddings_openai,
embeddings_gemini, embeddings_cohere, and embeddings_mistral left the
generated tool-ids/metadata/outputs artifacts stale.
* fix(embeddings): project before batching, and keep the sunset block's docs icon
Review round 2.
Projection ran inside callEmbeddingAPI, after batchByTokenLimit had already
measured and truncated the original text. The projector rewrites resolved
secrets to placeholders, which changes length, so batching sized against a
string that was never sent: a lengthening projection then pushed input past
the model's ceiling and the provider rejected it, and a shortening one
discarded document content that would have fit.
Project once up front, then batch the projected text, so truncation measures
what actually goes to the provider. This also keeps projection to exactly one
call per embed(), so no retry can re-project.
Separately, marking the legacy openai block hideFromToolbar dropped it from
the generated docs icon map, which only retains hidden blocks when they are
versioned. integrations/openai.mdx is deliberately kept — docsLink is baked
into every placed instance — so BlockInfoCard lost its icon and fell back to
a text tile. A sunset block keeps its docs page for the same reason a hidden
versioned block does, so the generator now treats it the same way.
The sim-side integrations map still omits it, which is intended: that feeds
the discovery page a sunset block should not appear on, and placed blocks
render from the registry's own icon reference.
* fix(embeddings): override stale block params instead of omitting them
Review round 3.
The generic handler merges the params() result over the original inputs
(`{ ...inputs, ...transformedParams }`), so omitting a key leaves the stale
value in place. The previous round dropped an unsupported taskType or
dimensions by omission, which was therefore a no-op through the executor
path: a reduction or task type chosen for one model still reached the tool
after a model switch.
Rewrite each stale field to an explicit `undefined`, which does override in a
spread.
Same class of bug for `model` itself, which was forwarded whenever present
without checking it belongs to the selected provider. Every provider's model
dropdown shares the `model` id, so switching provider kept the previous
provider's model and failed at the route as a mismatch. It now falls back to
the provider's default unless the saved model actually belongs to it.
Tests assert the merged result rather than the returned object, since the
return shape alone cannot distinguish an omitted key from an overridden one —
which is exactly why the previous fix looked correct and was not.
* fix(embeddings): discount the batch ceiling when the tokenizer is foreign
Review round 4.
Batching measures with tiktoken, which only has encodings for OpenAI models —
every other id falls back to cl100k_base. Gemini's 2048, Cohere's 128k, and
Mistral's 8192 were therefore enforced in OpenAI token units, so an input near
one of those ceilings could still be rejected upstream or trimmed more than
needed.
A true fix needs per-provider tokenizers, which the repo does not have:
estimateTokenCount is a chars-per-token heuristic, and truncation needs a real
encode/decode pair to slice on a token boundary. So the ceiling is discounted
for foreign tokenizers rather than trusted exactly.
The discount is one-sided on purpose. Overshooting means the provider rejects
the whole request; undershooting only trims a text that was already at the
limit, so the margin errs toward the second.
resolveBatchTokenCeiling is a pure function tested directly, rather than
inferred from truncation behavior, so the guarantee holds per model as the
catalog grows.
* fix(embeddings): keep the batch ceiling exact and warn before truncating
Review round 5. Reverts the safety margin from round 4.
The two review findings were in direct tension: round 4 flagged that a
foreign model's ceiling is measured in tiktoken units, and the margin added
to absorb that error reintroduced the round 3 harm — valid content truncated
below the provider's declared limit.
The margin was the wrong trade. It swapped a loud failure for a silent one:
an undercount surfaces as a provider rejection the caller can see and act on,
while shortening an embedding's input produces a degraded vector that is
indistinguishable from a good one at every layer above it. Silent quality
loss in a retrieval index is the worse outcome, and it is also the harder one
to ever notice.
So the declared ceiling is applied exactly, and truncation is no longer
silent: an input above the limit now logs a warning naming the model, the
limit, and whether the count was approximate. hasApproximateTokenCount
records which models are counted with a foreign tokenizer without being used
to shrink anything.
The tokenizer imprecision itself remains, and cannot be fixed without
per-provider BPE the repo does not have — estimateTokenCount is a
chars-per-token heuristic, and truncation needs a real encode/decode pair to
slice on a token boundary.
* refactor(embeddings): drop dead surface and enforce OpenAI's item cap
Audit follow-ups on the multi-provider embeddings work:
- Enforce OpenAI's documented 2048-entry `input` array cap in the OpenAI and
Azure adapters. Nothing bounded item count on the OpenAI path — batching
bounds tokens per request, so a batch of many short inputs could exceed it.
- Make the provider item cap single-source. It was declared both on the catalog
entry and on the adapter, read through a `??`; the adapter is the wire-protocol
owner, so the catalog copy is gone.
- Have the knowledge-base view call `getKbEligibleModels()` instead of
re-deriving the same `kbEligible` filter inline.
- Remove dead surface: the unused `EMBEDDING_TASK_TYPES` constant,
`EmbeddingToolDefinition`, `HOSTED_KEY_PROVIDERS`, and the five request-body
fields (`workspaceId`, `workflowId`, `executionId`, `userId`,
`useHostedCostTracking`) the route never reads.
- Trim `@/lib/embeddings` to what callers outside the module use.
- Drop the route's manual request-id plumbing; `withRouteHandler` supplies it.
- Fix two comments that had drifted onto the wrong declaration.
* fix(embeddings): normalize reduced Cohere output; correct OpenAI token ceiling
Second validation pass against provider documentation.
- Cohere: normalize locally when `output_dimension` reduces below native.
Cohere documents the parameter as Matryoshka truncation but never states that
it renormalizes, and an unnormalized vector silently skews cosine similarity.
`l2Normalize` is idempotent, so this is a no-op if Cohere already returns unit
vectors and a correctness fix if it does not. Covered by a test that fails
without it.
- OpenAI: raise the per-input ceiling from 8191 to the 8192 the API reference
documents, so a maximal input is no longer truncated by one token.
- Share the OpenAI response type with the Azure adapter instead of declaring an
identical copy, mirroring how the mail providers share `_nodemailer`.
- Rewrite the Gemini item-cap comment to say the 100-item limit is observed
rather than documented, which is what Google's reference actually supports.
Docs: add a manual intro to the Embeddings page covering providers, models,
inputs, outputs, and comparability rules. The generated Input tables are empty
because `createEmbeddingTool` builds params programmatically and the docs
generator only reads literals, so the manual section carries that reference.
* fix(embeddings): split per-input and per-request token limits; close provider gaps
Four gaps found in the validation pass.
Gemini token counts were estimated, not measured. `BatchEmbedContentsResponse`
carries `usageMetadata.promptTokenCount`; without reading it the client fell back
to tiktoken, which has no Gemini encoding and silently used `cl100k_base` — the
wrong tokenizer on a count knowledge-base runs bill against.
`maxInputTokens` was doing two jobs: the per-input ceiling that decides
truncation, and the per-request budget that decides how many inputs share a
batch. These are different provider limits, and conflating them meant Cohere
packed batches against its 128k per-document ceiling while OpenAI's documented
300,000-token request cap went unenforced. They are now separate fields.
Truncation moves out of `batchByTokenLimit` and into `embed`, so it happens once,
against the per-input ceiling, and always logs. The request budget is floored at
that ceiling — a budget below it would truncate inputs the provider accepts.
Batch sizes are unchanged everywhere except Gemini, which rises from 2048 to the
8192 the other providers already used.
codestral-embed now offers its documented 3072 maximum. Its API default is 1536,
so the offered sizes straddle the default; the catalog invariant relaxes from
"native size first" to "native size present", which is what the block relies on.
The Mistral API-key field no longer differs from the other three. Sim stocks
`MISTRAL_API_KEY` — `mistral_parse` already hides its key field on hosted — so
one field with `hideWhenHosted` replaces the conditional pair.
Docs: correct the API-key row, which described the old Mistral-only behavior.
* refactor(embeddings): derive block options from the catalog; use shared helpers
Findings from a four-angle quality review.
Reuse: `splitByItemLimit` and `processWithConcurrency` were reimplementations of
`chunkArray` (`@sim/utils`) and `mapWithConcurrency`
(`@/lib/core/utils/concurrency`), so `lib/embeddings/batching.ts` is gone. That
helper's doc forbade a throwing mapper; embedding legitimately wants a failed
batch to fail the call, since a partial vector set is not a usable result, so the
contract is reworded to cover both intents rather than forked.
The block no longer hand-copies the catalog. Its model, task-type, and dimension
dropdowns are derived from `EMBEDDING_MODELS`, which deletes roughly 150 lines of
literals that had to be kept in step by a drift test. The comment claiming this
was impossible was wrong: `generate-docs.ts` only reads `subBlocks` looking for
an `id: 'operation'` entry, which this block does not have. Verified by
regenerating — `embeddings.mdx` and `integrations.json` come out byte-identical.
Single-sourced two maps that were stated twice: BYOK provider ids (which encode
the non-obvious gemini -> google mapping) and the per-provider default model.
The route previously took its default from `getModelsForProvider(provider)[0]`,
which silently depended on catalog key order.
Azure's `endpoint` and `apiVersion` are required on their own context type
instead of optional on the shared one, so the adapter can no longer be built
without them and emit an `undefined/...` URL.
Also: contract enums now `satisfies` the catalog unions so they cannot drift,
the barrel exports only what callers outside the module use, the redundant
`requestedDimensions` field is a parameter, the bare `getEmbeddingModelInfo()`
call is a named `assertKbEmbeddingModel`, and the route checks payload size
before scanning entries rather than copying the body first.
* docs(embeddings): correct comments that drifted from the code
A comment pass over the feature found four that no longer matched what they sat
on, all introduced by earlier rounds of this work.
The contract's `satisfies` note promised that adding a catalog provider could
not leave the wire enum stale. It cannot deliver that: `satisfies` proves every
listed member is valid, not that the list is exhaustive, so an addition stays
silently absent. Reworded to say what it does and does not catch.
The client cited Gemini as a provider that omits usage, which the Gemini adapter
now contradicts — it reads `usageMetadata.promptTokenCount`. Every adapter
defines `parseTokens`, so the fallback is about a response lacking a usage block,
not about a particular provider.
`l2Normalize` documented only Gemini, though Cohere now calls it for a different
and stronger reason, and "normalizes in place" read as mutation when the function
returns a copy.
The route's new size-guard comment claimed it avoids copying the payload; nothing
there copies. The real reason is that summing lengths gates before the per-entry
character scan.
Also: split the derived-sub-block TSDoc so both constants carry hover text, gave
the payload cap its own doc, dropped one comment that restated a signature, and
tightened two long blocks without losing a fact.
* fix(docs): generate tool inputs for factory-built tools
The four embeddings tools rendered header-only Input tables. `extractToolInfo`
finds a tool's `params` by regex over the tool's own file, and these files hold
nothing but a `createEmbeddingTool({...})` call — the params live in the
factory's module. There was already a fallback for a same-file `...spread` base,
so this adds the cross-module equivalent: follow the factory's import and read
`params` from there.
Two things surfaced once the tables populated.
`hosting` was not in the set of keys that terminate the `params` capture, so the
non-greedy match ran past it to `request:` and swallowed the whole hosting block.
Every tool with a `hosting:` section between `params:` and `request:` was
publishing `pricing` and `rateLimit` as if they were user-facing inputs — this
drops those rows from eight unrelated integration pages as well.
The shared apiKey description was a template literal, which the regex emitted
verbatim as `${name} API key`. It is now a static string, matching how every
other tool in the repo declares one.
Docs: the Embeddings page keeps a prose intro in its MANUAL-CONTENT block like
other integrations, with the hand-written input/output tables removed now that
the generated ones are correct. The sunset `openai` page loses its
`encodingFormat` row — page generation skips hidden blocks, so that page is
frozen and would otherwise keep advertising a parameter the aliased tool no
longer accepts.
---------
Co-authored-by: Waleed Latif <walif6@gmail.com>
* fix(tables): resolve active selector before schema enrichment (#6345)
* fix(search): restore cmd+k autofocus on the search input (#6347)
* feat(files): let the agent read HEIC photos (#6346)
* feat(files): let the agent read HEIC photos
iPhone photos reach the model as HEIC, which no vision model accepts - the
Claude Messages API takes JPEG, PNG, GIF and WebP only - so the agent saw
nothing. 75 HEIC files are already in production, 64 of them in one workspace
uploaded over the last two days.
sharp cannot cover this: its prebuilt libvips ships libheif with AV1 but not
HEVC (sharp.format.heif.input.fileSuffix is ['.avif']), so a real iPhone photo
fails with 'Security limit exceeded'. Verified against both a HEVC-coded
sample (sharp fails, heic-convert decodes 2.99MB to a 3992x2992 JPEG in
~950ms) and an AV1-coded mif1 sample (sharp decodes it natively).
Decoder selection is capability-based, not brand-based: sharp is always tried
first and the WebAssembly decoder runs only on bytes it could not read. The
container brand cannot identify the codec anyway - mif1 carries either - so
choosing from it would push AV1 files down the slow path. This mirrors how
PhotoPrism layers libvips over libheif.
Also route the image path on the effective MIME type, since a phone upload
commonly stores as application/octet-stream and would otherwise be read as
a binary the model never sees, and stop reporting an undecodable image as
'too large'.
* refactor(files): gate every vision passthrough on model-supported media types
Review found two passthroughs that still handed the model bytes it cannot
decode. The sharp-load-failure branch returned raw HEIF, and the
already-small-enough branch returned raw AVIF, TIFF, BMP or ICO — all of
which isImageFileType accepts and no vision model does.
Gating all three on the existing MODEL_SUPPORTED_IMAGE_MIME_TYPES subsumes
the ad-hoc isHeifContainer re-sniff, and re-encoding an unsupported format
falls out of the resize ladder that was already there.
Also drop two constants that were pure indirection (a one-use alias for
'image/jpeg', and a quality value identical to heic-convert's default), trim
the oversized comments, log successful transcodes so the ratio is visible in
prod, and replace a detection test that could not fail.
* fix(files): read HEIF compatible brands, not just the major brand
A standards-valid HEIF may carry a generic major brand such as isom and
declare heic, heix or mif1 only among the compatible brands that follow the
minor_version at offset 12. Reading bytes 8-11 alone classified those as
non-HEIF, skipping the fallback decode and leaving a small undecodable file
to reach the model as raw bytes.
* fix(files): bound the HEIF fallback decode input (#6348)
Uploads allow 100MB and prepareImageForVision runs sharp with
limitInputPixels: false, so nothing upstream capped what could reach the
single-threaded WebAssembly decoder. A tenant-controlled file could therefore
spend unbounded CPU and memory on one read.
Cap the transcode input at 20MB — generous headroom over any phone photo,
which runs 1-4MB. Pixel-dimension bombs stay bounded by libheif's own
security limits during parse.
* fix(utils): drop the .js specifiers Turbopack cannot resolve (#6351)
* fix(utils): drop the .js specifiers Turbopack cannot resolve
Every dev server on staging is currently returning 500 from any route whose module
graph reaches the `@sim/utils` barrel:
Module not found: Can't resolve './errors.js'
> 1 | export { getErrorMessage, getPostgresErrorCode, toError } from './errors.js'
Import trace:
./packages/utils/src/index.ts
./apps/sim/lib/embeddings/client.ts
./apps/sim/lib/knowledge/embeddings.ts
./apps/sim/app/api/knowledge/route.ts
`packages/utils/src/index.ts` addresses its siblings as `./errors.js` while the files
are `./errors.ts`. webpack rewrites that through `resolve.extensionAlias`; Turbopack has
no equivalent (vercel/next.js#82945). `next build` is webpack and `next dev` is
Turbopack, so this passes CI and breaks every local dev server — #6317 went green.
Nothing required the extensions: the repo is on `moduleResolution: "bundler"`, and no
other package barrel uses them.
Two changes, either of which fixes the symptom; both are here because they fail
differently:
- `packages/utils/src/index.ts` drops all 12 `.js` specifiers. Fixes the barrel for
every current and future consumer.
- `apps/sim/lib/embeddings/client.ts` imports `chunkArray` from `@sim/utils/helpers`
rather than the barrel. #6317 added the only bare-barrel `@sim/utils` import in the
monorepo; the subpath form is the documented convention (CLAUDE.md, "Common
Utilities") and resolves to one module instead of pulling twelve.
`scripts/check-import-specifiers.ts` fails the build on either shape and runs in CI.
Verified it goes red by restoring both halves of the bug. It scans only bundler-compiled
source — vitest and standalone `bun run` scripts resolve `.js` -> `.ts` themselves, so
flagging their specifiers would be noise.
Verified against a real dev server with production env: `/api/knowledge`,
`/api/tools/embeddings` and `/api/workflows/[id]/deploy` all go 500 -> 401, `/workspace`
renders, and the Turbopack log is free of resolution errors. `tsc --noEmit` clean,
`packages/utils` 147/147.
* refactor(scripts): resolve specifiers instead of pattern-matching one mistake
The first version banned `.js` specifiers by regex, which catches the bug that happened
and nothing adjacent to it. This runs the actual resolution algorithm with Turbopack's
rules — extensionAlias deliberately absent — and fails on anything that does not land on
a real file.
That covers the whole "Module not found" class rather than one shape of it: `.js`
specifiers, typo'd paths, files moved or deleted with a stale importer left behind, `@/`
aliases pointing nowhere, and `@sim/*` subpaths a package does not export. Verified
against three synthetic breakages the regex version passed clean:
'@/lib/webhooks/providerz' — '@/' alias matches a tsconfig path but nothing is there
'./does-not-exist' — no file at that path
'@sim/utils/chunking' — @sim/utils does not export './chunking'
Getting to zero false positives on 37,307 specifiers needed three things the naive
version got wrong:
- tsconfig `paths` are per-workspace. `@/*` is `apps/sim/*` inside apps/sim but
`apps/realtime/src/*` inside apps/realtime, and apps/sim maps `@sim/db/*` straight at
the package directory, legitimately bypassing that package's exports map. One
hardcoded alias produced ~30 false positives in apps/realtime alone.
- `exports` maps have wildcards. `@sim/emcn` publishes `"./*": "./src/*"`, so
`@sim/emcn/components/code/code.css` is valid despite no literal entry.
- TSDoc contains example imports. `packages/db/triggers.ts` documents
`import { ensureRowCountTriggers } from '@sim/db/triggers'` — a subpath the package
deliberately does not export. Comments are now blanked in place, preserving byte
offsets so reported line numbers stay exact.
* fix(scripts): close three coverage gaps in the specifier audit
Review round 1 on #6351. All three findings were real and all three let the exact
regression this guard exists for slip through.
- Reported line numbers were one early. `SPECIFIER_RE` opens with `(?:^|\n)`, so
`m.index` is the newline ENDING the previous line, not the start of the statement.
`./helpers.js` on line 13 was reported as line 12. Anchoring to the specifier's own
offset is exact, and for a multi-line import it points at the `from '...'` line —
where the reader needs to look anyway.
- `require()` was not scanned. This repo uses lazy requires deliberately to break import
cycles: `tools/params.ts` reaches `@/blocks` that way and `blocks/blocks/agent.ts`
reaches `@/blocks/registry`, 22 first-party call sites in total. Those edges resolve
exactly like static ones, so a bad specifier in one fails identically. Verified by
pointing `tools/params.ts` at a non-existent module and watching the audit catch it.
- `apps/docs` was not scanned, despite being a second Next.js app with its own
`next.config.ts` — so it carries identical Turbopack exposure. Now covered, and clean.
Side-effect imports and dynamic `import()` were called out in the same round but are
already covered: the optional `from` group in `SPECIFIER_RE` matches bare `import '...'`,
and `DYNAMIC_RE` handles `import('...')`. That review ran against 1c6073e0, before the
resolver rewrite.
Coverage goes from 37,307 specifiers across 11,182 files to 37,438 across 11,243, still
with zero violations.
* chore(tools): regenerate the stale tool metadata
`bun run tool-metadata:check` has been failing on staging since #6317, so every PR
branched off it inherits a red CI regardless of its own contents. Reproduced against a
clean `origin/staging` to confirm it is not this branch's doing.
#6317 rewrote the embeddings tools' `apiKey` descriptions from provider-specific strings
to one generic string in `tools/embeddings/factory.ts`, but did not regenerate
`tools/generated/tool-metadata.ts`. The whole delta is 89 bytes of description text — the
tool set is unchanged at 4380 ids, none added, none removed:
- "description":"Cohere Embeddings API key"
+ "description":"API key for the selected embedding provider"
The old strings no longer exist anywhere in source, so the generated file was the stale
side. `tool-metadata:check` passes after regenerating, and the generator's own resolver
cross-check agrees.
`mship:check` and `mship-tools:check` also fail locally, but neither is a CI gate and both
fail only because they read contracts from the sibling copilot repo, which is not checked
out here. Left alone.
* fix(scripts): substitute every wildcard in a resolved target
CodeQL js/incomplete-sanitization, two instances, both correct.
`String.replace('*', x)` fills only the first occurrence. Node's `exports`
resolver uses a global regex, so a target carrying more than one `*` — e.g.
`"./src/*/index-*.ts"` — gets every occurrence substituted. Replacing only the
first leaves a literal `*` in the path, so `probe()` finds nothing and the audit
reports a perfectly valid subpath as missing.
TypeScript `paths` allows at most one `*`, so the tsconfig branch was already
correct in practice; it changes for consistency and because nothing enforces that
assumption.
Not a suppression — the resolver now matches Node's behaviour. 37,438 specifiers
still resolve clean.
* fix(scripts): do not assert on generated output in the specifier audit
CI red on a fresh checkout, green locally — the tell that the audit was
depending on build state rather than on source.
apps/docs/lib/source.ts imports '@/.source/server'. apps/docs maps '@/.source/*'
at './.source/*', which fumadocs-mdx generates and apps/docs/.gitignore excludes.
It exists on any machine that has built the docs and is absent from CI's
checkout, so the audit reported a valid import as unresolvable.
A path landing in output the scanner itself refuses to read as source —
node_modules, a build directory, any dot-directory — is now treated as
unverifiable rather than missing. That is the consistent rule: if we do not scan
it as source, we cannot assert on its presence, and asserting anyway makes the
verdict depend on build order. Applied to all three resolution paths (relative,
tsconfig paths, exports map), with a GENERATED sentinel keeping 'matched but
generated' distinct from 'matched and genuinely missing'.
Only the repo-relative portion is inspected. Checking the absolute path would
match the '.claude/worktrees/...' a git worktree lives under and silently skip
every specifier in the repo.
Verified both directions: passes with apps/docs/.source moved away (CI's state),
and still catches a require('@/blocks/still-not-real') planted in tools/params.ts.
* refactor(scripts): trim the specifier audit's comments
The audit shipped at 24% comment lines — the header alone retold the whole
incident. Cut to 15% (452 -> 401 lines) by collapsing the narrative and keeping
only what the code cannot say: the webpack/Turbopack extensionAlias divergence,
why '.js' is a probed extension but not a fallback, why paths resolve
per-workspace, why targets substitute with replaceAll, why generated output is
unverifiable, and the '.claude/' worktree trap in the relative-path check.
No behaviour change: 37,437 specifiers still resolve clean.
* fix(chat): stop classifying secret-free binary sandbox exports as unknown (#6349)
* fix(execution): stop classifying secret-free binary sandbox exports as unknown
* fix(execution): fail closed when files are mounted without a provenance envelope
The binary classifier read an absent mounted-file scanner as "no mounted
secrets". That is absence of evidence, not evidence of absence: the request
contract permits _sandboxFiles without the provenance envelope, so a caller
that mounts secret-bearing bytes and omits the envelope would have a derived
binary persisted as provably secret-free.
Not reachable today — the route is internal-JWT-only and its one file-mounting
caller always emits the envelope — but the classification rested on an
invariant nothing enforced.
- the copilot handler emits the envelope on the same condition that produces
the mount, so tables ship one too and the two cannot drift apart
- a mount with no verified scanner now counts as secret material in scope, so
the classification is never stronger than what the caller attested to
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(execution): treat partial and unscannable mount attestations as unknown
Two ways the envelope could read as stronger evidence than it was.
The copilot handler preserved `_sandboxFiles` that arrived on the params and
then exported provenance from `mountedRegistry`, which knows only about the
files it resolved itself. The route would have read that partial envelope as a
complete attestation over every mounted byte. The envelope now covers the whole
mounted set or is not emitted at all, and a mount with no envelope already
fails closed.
`hasSecrets` was derived from whether entries produced scannable literals, so
an envelope listing entries that all failed to decrypt reported false and let a
derived binary be marked exact-empty. It now reflects what the envelope
attested to: entries that yield no plaintext make the mount less classifiable,
not more.
Neither was reachable — `_sandboxFiles` is absent from the copilot tool schema,
so nothing can populate the preserved-mount branch — but both had the
classification resting on a property nothing enforced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(tools): regenerate stale tool metadata
`bun run tool-metadata:check` fails on origin/staging as well as here, so this
is not from this branch — #6317 landed the artifact generated from a factory
that still built a per-provider apiKey description, and the source was later
genericized without regenerating.
Regenerating changes exactly the five embeddings entries' apiKey description to
the text `tools/embeddings/factory.ts:74` actually produces. The per-provider
strings appear nowhere in source. Included here only because the gate is red on
every branch cut from staging until someone lands it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(execution): count the runtime payload as secret material in scope
An execution with no mounted files and no env secret still carries `params` and
`contextVariables` into the sandbox — the runtime payload is serialized into a
private-input file, so resolved block outputs and workflow variables land as
plaintext regardless of `_sandboxFiles`. The scope predicate only looked at
mounts and env secrets, so a binary derived from them was classified
exact-empty.
The route has no catalog for those values and cannot tell a secret-bearing one
from an ordinary one, so they count as in scope. Only an execution with nothing
at all in scope earns an exact-empty binary.
This narrows where the relaxation applies rather than regressing anything: every
binary export was unknown before this branch, so a workflow Function block
carrying block references keeps exactly the behavior it has today. The
mothership path is unaffected — its tool sets no contextVariables, blockData, or
workflowVariables, which is the case this branch exists to fix.
Values, not keys, for the params check: `executionParams._context` is set to
undefined before the context is built, so a key count reads every execution as
carrying params.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Revert "fix(execution): count the runtime payload as secret material in scope"
This reverts commit 754e37cedb.
The classifier's secret catalog is the Secrets feature and nothing else:
`outputSecretNamesByScanLiteral` and `outputSecretPlaintextsByName` are built
only from `envVars`, and mounted-file entries trace back to the same place.
`contextVariables`, `blockData`, and `workflowVariables` are ordinary workflow
data — resolved block outputs the user already sees in logs — and the text
export path does not scan them either.
Treating their mere presence as secret material was a heuristic, not a security
property, and it created exactly the asymmetry rejected two rounds earlier: a
binary derived from a context variable would be `unknown` while a text export of
the same bytes stays exact-empty. Stricter than the text path for the same
content is not a boundary.
It was also nearly inert. `scopeEnvironmentVariables` returns every workspace
secret when scope is `all` (the default), so any workflow Function block with
secrets configured already trips the env branch. The only slice it changed was
executions with no env vars at all, where the workspace has no secret for a
context variable to carry.
A Secret resolved into an upstream block's output and arriving here through
blockData is a real gap, but it is pre-existing, identical for text exports, and
belongs at the executor -> route boundary as a provenance envelope for params —
not as a presence check in this classifier.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scripts): make the specifier audit path-separator agnostic (#6355)
Review round: isGeneratedPath split the repo-relative path on '/', but
path.relative returns backslashes on Windows, so '.source' and 'node_modules'
never matched a segment and generated output was treated as source. The repo
does support Windows dev — scripts/setup branches on win32.
The finding named one site; there were three. isCompiledSource compared against
'apps/sim/scripts/' with the same assumption, and workspaceFor matched
`${w.dir}/`, which on Windows never matches an absolute path and would have
dropped every file out of its own workspace — silently disabling tsconfig paths
resolution rather than erroring.
Normalized behind a repoPath() helper, with workspaceFor using path.sep against
absolute paths. Reported paths now go through it too, so output is identical on
either platform. spec.split('/') is left alone: import specifiers are always
'/'-separated regardless of host.
Verified by simulating win32 separators through the same predicates, and posix
behaviour is unchanged at 37,437 specifiers.
* improvement(sandbox): exempt caller-consumed streams from the output retention budget (#6353)
* fix(sandbox): exempt caller-consumed streams from the output retention budget
A Pi agent turn emits one JSONL event per step and passes the 10 MB process
output budget on an ordinary session, killing the run. The bytes were never a
result: `handleChunk` parses every chunk as it arrives and keeps none of it,
and the accumulated copy is only ever read back to build an error message.
The budget bounds what Sim RETAINS, so a stream the caller consumes itself is
exempt and only a 64 KB diagnostic tail is kept. The limit is unchanged for
everything else.
Gated per stream, not per command: a caller that streams stdout but not stderr
still has stderr fully bounded. Both adapters gate on the handler's presence, so
the calls that parse markers out of stdout (Pi's clone/prepare/push, which do
not stream) keep full retention and full budgeting — the case daytona.ts already
warns about.
E2B's SDK still accumulates internally, so this bounds what Sim retains rather
than the provider's peak; Daytona accumulates locally and is bounded outright.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(sandbox): drop explicit any from the new conformance stream mocks
The two new E2B mocks annotated their arguments as `any`, which both violates
the repo's no-`any` rule and defeats the point of a mock: an invalid SDK shape
would type-check.
Matches the sibling mock a few lines above (`async (_code, options) =>`) and
infers from the `vi.fn()` signature instead of naming a type, so the mock stays
bound to whatever the adapter actually calls.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(sandbox): cut Daytona's retained tail to the same bound as E2B
`appendStreamedSandboxOutput` deliberately lets the accumulator grow to twice
the tail before collapsing, so a single re-cut is amortized across chunks rather
than paid on every one. That leaves it anywhere inside that band when the stream
ends. E2B tails the value it returns, Daytona returned the accumulator as-is, so
a stream finishing between one and two tails came back roughly 96 KB on Daytona
and 64 KB on E2B.
The two adapters must agree — a divergence here surfaces as changed behavior
during a failover, which is the one moment nobody wants surprises. Daytona now
takes the same final cut on every return path.
The conformance test that should have caught this asserted the bound as
`tail * 2`, which is satisfied by both the correct and the incorrect value. It
now asserts the tail plus the truncation note, and a second case exercises the
band between one and two tails where the two providers could disagree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* perf(typecheck): run the native TypeScript 7 compiler (#6356)
A bare `tsc` was silently resolving to the JavaScript TypeScript 6 compiler.
`apps/sim` depends on `@typescript/typescript6` for its runtime TypeScript AST
API, which pulls in `@typescript/old` (an alias of `typescript@6`) declaring its
own `tsc` bin. Package managers pick bin winners by lexical sort rather than
dependency depth, so `@typescript/old` beat `typescript` and won
`node_modules/.bin/tsc`.
Identical diagnostics, ~10x slower, and it fails silently: the check still
passes, it just burns minutes. Both compilers check an identical 11,066-source-
file program with byte-identical diagnostics; the only `--listFiles` delta is
lib relocation plus TS7 deduping nested .d.ts copies.
The `@typescript/native` alias sorts ahead of `@typescript/old` and reclaims the
bin. This is the TypeScript team's own recommendation on typescript-go#4567 --
the original blog example was wrong. Every `type-check` script is unchanged;
`bunx tsc` and ad-hoc invocations are fixed too.
apps/sim cold 83s -> 8.5s; all 23 workspaces 96s -> 9.4s.
The alias is invisible-load-bearing: nothing imports it, so removing it looks
like dead-dependency cleanup and costs 10x with no visible failure.
check:native-typecheck asserts a bare `tsc` reports 7.x and fails CI otherwise.
Also drops NODE_OPTIONS=--max-old-space-size=8192 from apps/sim's type-check --
it only ever mattered for the JS compiler's V8 heap.
* feat(files): preview HEIC photos in the file viewer (#6350)
* feat(files): preview HEIC photos in the file viewer
The agent can read HEIC since #6346, but the Files page still showed 'Preview
not available' — an <img> pointed at the serve route got the stored HEIF under
nosniff, which no browser outside Safari renders.
The serve route now resolves a JPEG derivative for HEIF bytes, cached in the
artifact store and keyed by the source's storage key. Workspace keys are
regenerated on every content replacement, so the key is already a content
version and using it avoids streaming the original just to hash it. Caching
matters here in a way it did not for the vision path: a preview is re-fetched
on every view and the WASM decode costs roughly a second for a phone photo.
The original stays the stored object — downloads and raw=1 serve it untouched,
so this never changes what a user gets back.
compileDocumentIfNeeded becomes resolveServableBytes, since it now resolves
images as well as generated documents. .tif/.tiff stay download-only: nothing
decodes those on either side.
* fix(files): make the preview derivative opt-in and never show a broken image
Five issues from review, all interlocking around one decision.
The derivative is now requested with preview=1 rather than suppressed with
raw=1. raw=1 would have corrupted generated-document downloads: every
non-markdown workspace download routes through the serve route and relies on
resolveServableDocBytes compiling stored source into the real binary. Opt-in
separates the three consumers cleanly — previews get the JPEG, downloads get
untouched stored bytes, and doc compilation stays unconditional.
- Public shares resolve the derivative too, with the same preview/download
split; the viewer requests it, the download button does not.
- Split the brand predicate. isHeifContainer stays broad for the vision path,
where it only runs after sharp has already failed. The serve path runs
first, so it uses isHevcHeifContainer — an AVIF was costing a storage
round-trip, a WASM load and a misleading warn per request.
- A derivative that cannot be produced (past the 20MB ceiling, or a decode
failure) now falls back to 'Preview not available' instead of a broken
image. UnsupportedPreview moved to preview-shared to avoid a module cycle.
- The chat composer chip requests the derivative, so HEIC attachments stop
rendering as broken thumbnails.
* fix(files): reset the image preview when the file is overwritten
An overwrite preserves the storage key, which is what the parent keys this
component on, so only the URL version changes and it never remounts. The
previous bytes' outcome therefore stuck, leaving a replaced image parked on
'Preview not available' until something else forced a remount.
Reset on URL change during render rather than in an effect — this is derived
state, and an effect would render the stale outcome first.
* improvement(files): drop the dead preview reset and cap the ftyp brand scan
- Content writes mint a new storage key, so the parent's key={file.key}
already remounts ImagePreview; the render-phase reset was unreachable and
made renames flash a loading overlay.
- Clamp the ftyp compatible-brand scan to a real box size. The declared size
is attacker-controlled and this now runs on every preview request.
- UnsupportedPreview takes a primitive name so memo is load-bearing.
- Fix the hardcoded ? in the public preview URL builder.
* improvement(copilot): only ask for a preview derivative on image thumbnails
A video has no derivative path, so preview=1 there only spent a brand sniff
per request. Adds the missing test coverage for the helper.
* fix(tooltip): dismiss floating tooltip when its trigger is hidden without pointer events (#6354)
* fix(tooltip): dismiss floating tooltip when its trigger is hidden without pointer events
* fix(tooltip): catch display: none triggers in the legacy visibility fallback
* feat(smartlead): add Smartlead integration (#6352)
* feat(smartlead): add Smartlead integration
Adds a Smartlead block with 22 tools covering campaigns, sequences, leads,
analytics, and webhooks.
Every request path, parameter, enum, and response mapping was verified against
the live Smartlead API rather than its documentation, which proved unreliable:
- `POST /campaigns/new` (documented) 404s; the real path is `/campaigns/create`
- `GET /campaigns/{id}` and `/sequences` return bare payloads, not the
documented `{success, data}` envelopes
- `/statistics` returns paginated per-email rows, not the documented aggregate
- `POST /campaigns/{id}/leads` returns import counters under entirely
different field names than documented
- documented `/leads/{id}`, `/top-level-analytics`, `/all-leads-activities`,
`/lead-lists/`, and `/lead-tags/` all 404
Enum values (campaign status, track settings, stop-lead settings, webhook event
types, engagement status) were probed value-by-value against the API.
Notes on the API's shape, encoded in the mappers:
- string-encoded numbers (`total_leads: "1"`, `sent_count: "0"`) are normalized
to numbers so a field never changes type between operations
- `seq_delay_details` is read as `delayInDays` but written as `delay_in_days`
- webhook writes echo `event_type_map`/`category_id_map` objects while the list
endpoint returns `event_types`/`categories` arrays; both map to arrays
- `track_settings` reads back in a vocabulary it will not accept on write
Statistics rows and lead message-history entries pass through unmapped: no
account could produce a non-empty sample, so no field names were invented.
Email-account tools and a webhook trigger are omitted for the same reason.
Adds a `smartlead-errors` extractor since the API's 400s put the useful text in
`message` while `error` is only "Bad Request".
* feat(smartlead): expand to the core workflow surface and fix review findings
Grows the block from 22 to 47 tools and fixes every defect found in review.
New tools (all executed against the live API end to end):
campaign email accounts (list/add/remove), duplicate, delete, CSV lead export,
webhook delete + delivery summary, lead + mailbox statistics, top-level
analytics by date, lead activities, get lead by id, unsubscribe from campaign,
unsubscribe globally, mark complete, delete from campaign, master-inbox
replies, lead lists (list/get/create/update/delete), email accounts, clients.
The endpoint inventory was rebuilt by extracting method+path from all 212
reference pages, which corrected several earlier conclusions: get-lead-by-id is
`/leads/{id}` (not under `/campaigns/`), lead lists are `/lead-list/`
(singular), and lead activities are …
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The agent can read HEIC since #6346, but the Files page still showed "Preview not available for .heic files". An
<img>pointed at the serve route received the stored HEIF underX-Content-Type-Options: nosniff, which no browser outside Safari renders.The serve route now resolves a JPEG derivative for HEIF bytes and caches it, so
.heicuploads preview like any other image.Design
?raw=1serve it untouched, so this never changes what a user gets back.compileDocumentIfNeededbecomesresolveServableBytes, since it now resolves images as well as generated documents.Scope
.tif/.tiffstay download-only — nothing decodes those on either side, so previewing them would show a broken image rather than a picture. That exclusion is now the only one, and the comment says why.Type of Change
Testing
Six tests on the resolver covering passthrough for non-HEIF, transcode-and-cache on a miss, cache hit without re-decoding, storage-key derivation (replaced content misses the old entry), image still served when caching fails, and null when the decode fails. Viewer categorisation tests updated. 681 tests, typecheck, lint, and
check:api-validationpass.Not verified in a browser — worth a look at an actual
.heicin the Files page before merge.Checklist