mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-01 14:59:19 +08:00
60f6d6d8fe9dfe98af839760e01f4a297e16b216
5923 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Justin Blumencranz
|
60f6d6d8fe |
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 |
||
|
Waleed
|
ff3b422efd |
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. |
||
|
Waleed
|
a512a263c8 |
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. |
||
|
Vikhyath Mondreti
|
f340ad965d |
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> |
||
|
Waleed
|
de02bc6ad5 |
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.
|
||
|
Vikhyath Mondreti
|
5ad820daf6 |
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
|
||
|
Waleed
|
10878fbde5 |
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
|
||
|
Waleed
|
2b35a3c9c7 |
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. |
||
|
Waleed
|
5596640b3b |
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. |
||
|
Waleed
|
85a4cb0c1c | fix(search): restore cmd+k autofocus on the search input (#6347) | ||
|
Theodore Li
|
f76d46bc53 | fix(tables): resolve active selector before schema enrichment (#6345) | ||
|
mzxchandra
|
dc5bab6e54 |
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>
|
||
|
Waleed
|
9ba51f9aef |
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. |
||
|
Bill Leoutsakos
|
b04fee8aa7 |
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>
|
||
|
Waleed
|
a4973ec576 |
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. |
||
|
Waleed
|
8c49d35a9c |
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 |
||
|
Waleed
|
0ae09c1518 |
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 |
||
|
Waleed
|
2ab6be6421 |
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 |
||
|
Siddharth Ganesan
|
93b68f049a | feat(mship): async runs- #6329 | ||
|
Waleed
|
3e3d8605fc |
fix(uploads): drop the stray 'use server' directive that enables Server Actions app-wide (#6335)
* fix(uploads): drop the stray 'use server' directive that enables Server Actions app-wide `file-utils.server.ts` was the repo's only `'use server'` module, and the sole reason Next's `hasServerActions()` returned true. With actions registered, Next loses its early-404 escape hatch for Server Action requests — and it classifies a request as an action from headers alone, with no body inspection and no auth. Any unauthenticated `POST` with `Content-Type: multipart/form-data` to any App Router path therefore took the non-fetch action path, which bare-throws and surfaces as an HTTP 500. Nothing invokes these functions as Server Actions: every one of the ~77 importers is server-side, with zero `'use client'` importers. The directive was a misuse of `'use server'` where "server-only module" was meant — the `.server.ts` suffix already carries that convention. Extends check-client-boundary-imports.ts to fail on any `'use server'` directive so this cannot regress. * fix(scripts): match boundary directives that carry a trailing comment A directive keeps its meaning when a note follows it on the same line, so strip a trailing '//' or block comment before matching. Shared by the 'use client' and 'use server' detectors. |
||
|
Waleed
|
9b4793083a |
fix(observability): attach the real cause at three error-swallowing sites (#6336)
Three log sites discarded the underlying error, which blocked root-cause
analysis in production.
- Trace secret projection swallowed TraceSecretProjectionError in four
catch blocks (per-field omission, whole-tree fallback, post-transform
invariant, structural traversal) across ~30 distinct throw sites, so no
warning said which invariant fired. Each now reports the failure. Only
TraceSecretProjectionError messages are logged — they are fixed literals
describing an invariant. A failure raised outside the module may quote
trace content (a JSON parse error embeds the text it choked on), so those
are reported by name only.
- ExecutionLogger's unbilled-charge error logged `"error":{}` because a
plain Error has non-enumerable message/stack. It now logs describeError.
- WorkspaceFileStorage / FetchExternalUrl logged `saveError:{}` for the
same reason, and the upload wrapper rethrew without a cause, so Drizzle's
`Failed query:` wrapper dropped the Postgres SQLSTATE. The wrapper now
chains the cause and both sites log describeError, which reports the
deepest link's code.
describeError additionally strips the `params:` tail Drizzle appends to its
message, so bound parameter values never reach logs.
|
||
|
Waleed
|
2ba455647b |
fix(db): bind every raw-sql Date through its column encoder (#6337)
* fix(db): bind every raw-sql Date through its column encoder `drizzle()` overwrites postgres-js's temporal serializers (OIDs 1082/1083/ 1114/1184/1182/1185/1115/1231) with an identity function because drizzle maps timestamps itself through the column's `mapToDriverValue`. A raw `sql` template carries no column context, so an interpolated `Date` skips that mapping, reaches the identity serializer unchanged, and the wire encoder throws `ERR_INVALID_ARG_TYPE`. The pools' `prepare` / `fetch_types` options are irrelevant: the serializer swap happens for all four combinations. Five live sites still interpolated a bare `Date`, the stale schedule-job filter among them — it has no try/catch, so a database async backend would surface a 500 from the schedule tick. Bind each cutoff with `sql.param(date, column)`. The testing `sql` mock's guard cannot see untested code or the tests that override the drizzle-orm mock, so add `check:sql-date-binding`: a Babel-AST audit over apps/** and packages/** that resolves Date-valued bindings per file and rejects any that reach a raw template unbound. Correct the mock's comment, which attributed the failure to postgres-js under `fetch_types: false`. * fix(scripts): require the documented sql-date-bound annotation form and a reason |
||
|
Waleed
|
8e3e608c53 |
fix(knowledge): align document tag provenance selections with the serialized request (#6332)
* fix(knowledge): align document tag provenance selections with the serialized request
The create/upsert document tools counted one provenance selection pair per
parseDocumentTags entry, while the write route built targets from the
serialized documentTagsData and dropped entries whose value is the empty
string. A tag value that is truthy before coercion but stringifies to empty
(`[]`, `[null]`, `{ toString: () => '' }`) was therefore counted by the tool
and not by the route, and the bundle length check rejected the write with 400.
Both sides now read one shared parser over the exact bytes that go on the
wire, so their counts cannot diverge.
* test(knowledge): use as const for the empty-stringifying tag fixture
|
||
|
Waleed
|
eb245adb1a |
fix(billing): stop the false unbilled-charge error on zero-cost runs (#6333)
recordExecutionUsage required a billing context before it knew whether there was anything to bill, so a usage-gated run (skipCost, no billingContext) threw and logged 'charge may be unbilled' for a run that never executed and had no cost. Move the no-billable-target early return above the attribution requirement: a genuine ledger write failure still logs at ERROR. |
||
|
Waleed
|
c3da54470b |
docs(sso): correct callback host and issuer guidance, document Entra SAML and IdP-initiated behavior (#6334)
* docs(sso): use the deployed host in callback and entity ID examples * docs(sso): correct host, issuer, and provider-id guidance; document Entra SAML and IdP-initiated behavior * docs(sso): send Entra federation metadata to the field that reads it |
||
|
Waleed
|
64b3472e9b |
feat(admin): provision a user with an emailed password reset instead of a set password (#6328)
* feat(admin): provision a user with an emailed password reset instead of a set password * fix(emcn): keep focus on the input so the password Hide toggle actually masks * fix(admin): surface a created user when only its reset email failed * test(emcn): cover keyboard activation of the password reveal toggle |
||
|
Waleed
|
82e654d73c |
fix(cron): bind stale async-job cutoffs through the column encoder (#6327)
The stale-processing predicate interpolated `Date` values straight into a raw `sql` template. A raw template carries no column context, so drizzle skips `PgTimestamp.mapToDriverValue` (which stringifies via `toISOString`) and postgres-js receives a `Date` it cannot serialize under the pools' `prepare: false` / `fetch_types: false` options. Every run of the job has failed its async-job sweep since the change shipped, leaving stuck jobs unreaped while the surrounding typed `lt(column, date)` sweeps succeeded. Bind both cutoffs with `sql.param(date, column)`, matching the workflow sweep in the same handler. The testing `sql` mock already rejected `sql.param(date)` for this reason but not the interpolated form that shipped, so extend it to cover both. That guard alone fails three existing tests when the fix is reverted, and the full suite shows no other route binding a bare Date this way. |
||
|
Vikhyath Mondreti
|
71d7d8da56 |
fix(tools): align private provenance with wire payloads (#6325)
* fix(tools): align private provenance with wire payloads * fix(execution): separate provenance source from actor |
||
|
Waleed
|
c4ccee05fa |
fix(sso): show the saved client secret as a masked fact with an explicit Replace action (#6321)
* fix(sso): stop showing the redaction sentinel in the client secret field * fix(sso): hide the reveal toggle when there is nothing to reveal * feat(sso): show the saved client secret as a masked fact with an explicit Replace action * refactor(sso): extract the client secret field and give it its own reveal state * test(sso): cover client secret preservation, and disambiguate the back-out label * fix(sso): reject a blank replacement instead of overwriting the stored secret * fix(sso): clear the required-error when a secret replacement is backed out |
||
|
Waleed
|
4340e2ef38 |
fix(tables): stop row writes 500ing on a column outside the table schema (#6323)
createTableWriteProvenanceTargets (added in #6247) required every submitted column to translate to exactly one storage id and threw otherwise. The wire translator has always dropped keys naming no column in the schema, so any internal-JWT write carrying such a key threw an uncaught error and surfaced as a 500 — where the same write previously succeeded, since the write path drops the column identically. Give a dropped column a null column id instead of throwing. It still gets a target, so the bundle completeness check that pairs one selection per submitted column is unchanged, but no provenance is recorded for a value that is never stored. |
||
|
Vikhyath Mondreti
|
5157a59879 | fix(memory): provenance checks (#6322) | ||
|
Waleed
|
01f4b430cf |
fix(sso): re-grant provider trust when an already-verified domain is re-submitted (#6320)
* fix(sso): re-grant provider trust when an already-verified domain is re-submitted * fix(sso): distinguish a failed DNS lookup from a missing record, and label the domain fields * chore(sso): tighten the re-grant rationale comment * fix(sso): state what a failed DNS lookup tells us instead of assigning blame |
||
|
Waleed
|
ab257555d8 |
fix(sso): link Entra sign-ins to existing accounts and enforce unique provider IDs (#6311)
* fix(sso): link Entra sign-ins to existing accounts and enforce unique provider IDs
Better Auth 1.6.23 calls the account-linking handler with trustProviderByName:
false, which disables the trustedProviders allowlist for SSO entirely. Trust now
comes only from the provider's domainVerified flag, which Sim never set — so any
user who already had a Sim account was stranded on "account not linked". Entra
never sends email_verified, so this hit every Microsoft tenant.
Sim already proves domain ownership via sso_domain before a provider can be
registered, so the register route mirrors that decision onto domainVerified.
The column defaults to true so existing providers keep signing in across the
deploy, since enabling the option turns sign-in into a hard gate.
Also enforces the providerId uniqueness Better Auth already assumes: it rejects
any id that exists in any tenant and resolves providers by that column alone, so
a second customer picking "azure-ad" could not register at all and got an opaque
422. Sim now returns a 409 naming a free id, and a unique index makes the
duplicate-row state unreachable.
* fix(sso): revoke domain trust when verification is removed mid-update
The create path re-checks domain ownership after Better Auth persists the
provider and rolls the row back if the verified sso_domain row disappeared in
that window. The update path had no equivalent, so deleting the verified domain
while updateSSOProvider was in flight still set domainVerified, restoring
same-email account-linking trust for a domain the org no longer proves it owns.
The update path has no newly-created row to roll back, so it clears the flag
instead: that denies linking and blocks sign-in on the provider until the domain
is verified again.
* fix(sso): make domain-trust grants atomic and propagate revocation
Greptile flagged that the ownership check and the domainVerified write were
separate statements, so a domain deleted between them still ended with trust
granted. Two changes close it from both sides.
The grant now folds the ownership test into the UPDATE's WHERE clause, so
Postgres evaluates both in one statement and the write matches nothing once the
proof is gone.
Removing a verified domain now clears domainVerified for providers on that
domain, in the same transaction as the delete. This was a standing gap, not just
a race: deleting a domain previously left linking trust set indefinitely.
Together the provider cannot end up trusted without current ownership in either
commit order — if the grant lands first the delete clears it, and if the delete
lands first the grant no-ops.
* fix(sso): report a refused domain-trust grant instead of returning success
The conditional grant could match zero rows if the verified domain was deleted
between the pre-write check and the write. The route ignored that and returned
200, leaving a provider that cannot sign anyone in while telling the admin it
saved.
The grant now reports whether it matched, and that result is the single decision
point on both paths: the create path rolls the provider back, the update path
clears the flag, and both return SSO_DOMAIN_NOT_VERIFIED. This also drops the
separate post-write ownership read, since the UPDATE re-tests ownership itself.
* feat(sso): let admins map IdP claims, and trim setup comments
Identity providers disagree on which claim carries each value — Entra can send
the address as `upn` rather than `email` — and the mapping was hardcoded, so a
mismatch had no fix in the UI at all. Adds an Attribute mapping section for both
protocols, defaulting to each protocol's standard claim names shown as
placeholders, so the common case still needs no input.
Editing an existing provider now loads its stored mapping and only treats a
value as an override when it differs from the default, so a saved custom mapping
is never silently rewritten.
* feat(sso): expose the standard enterprise IdP options in the setup form
Rounds out the form with the options Better Auth already accepts but the UI hid,
so a non-standard IdP no longer dead-ends at a field that cannot be set.
SAML gains signature algorithm, digest algorithm and NameID format. Only SHA-256
and stronger are offered: Better Auth warns on SHA-1 as deprecated and rejects
anything outside its secure set, so weaker choices would only produce failed
saves.
SAML also surfaces the SP Entity ID beside the ACS URL. IdP admins are usually
handed a vendor metadata document; Sim does not publish one, and these are the
two values it would carry.
OIDC gains authorization, token and JWKS endpoint overrides for providers whose
discovery document is incomplete or unreachable. Discovery still fills them in
when they are left blank.
All of these load from the stored config when editing, so re-saving a provider
cannot quietly drop them.
* fix(sso): withhold domain trust from personal providers on the hosted deployment
A personal (org-less) provider has no verified domain behind it, but the trust
grant treated it as authoritative anyway. On the multi-tenant deployment that is
an account-takeover primitive: anyone able to register one could claim a domain
they do not own, point it at their own IdP, and have a sign-in auto-link to an
existing account on that domain.
Sim's UI always registers org-scoped, so this only reaches direct API callers.
Self-hosted deployments are single-tenant — the operator is the only tenant —
so the org-less path keeps working there.
Also clears the attribute mapping when the protocol changes: claim names are
protocol-specific, so an OIDC override carried into a SAML config would save a
mapping the IdP cannot resolve.
* docs(sso): correct the personal-provider trust note after the hosted gating
* fix(sso): drop the inert SAML algorithm selects, make NameID format clearable
The signature and digest algorithm selects were placebo controls. Tracing
@better-auth/sso 1.6.23, those two values are only read by validateConfigAlgorithms,
mergeSAMLConfig and sanitizeProvider — createSP and createIdP never pass them to
samlify, so nothing they select reaches the SAML exchange. Bugbot separately
noted they could not be cleared, since Better Auth merges with `??` and omitting
a key keeps the stored value. A control that neither applies nor clears should
not exist, so both are removed.
NameID format is genuinely wired (createSP passes it as nameIDFormat) and is
kept, but is now always sent rather than omitted when set to the provider
default. samlify falsy-guards the value, so an empty string reads as unset and
"Provider default" can actually clear a stored override.
The read-only provider view also now shows the SP Entity ID and the ACS label
for SAML — admins land there after saving and need the same two values the form
says their IdP requires.
* docs(sso): tighten the personal-provider note to the self-host path it describes
* fix(sso): forward an empty SAML NameID format so the provider default can be restored
The form sends an empty identifierFormat when the admin selects "Provider
default", but the route dropped it with a truthiness check. Better Auth merges
SAML config with `??`, so an omitted key retains the stored value — the selection
appeared to apply and silently did not.
Forwarding the empty string makes it reach the merge, and samlify falsy-guards
nameIDFormat, so it reads as unset. Selecting the provider default now actually
clears a stored override.
* fix(sso): revoke trust for providers whose domain is spelled with a wildcard
Migration 0268 grandfathered providers by normalizing their domain with
lower + btrim + a stripped leading `*.`, so sso_provider.domain can hold
`*.acme.com` while its verified sso_domain row holds `acme.com`. The revoke on
domain deletion compared the raw column, so such a provider matched nothing and
kept domainVerified after its ownership proof was gone.
The comparison now applies the same normalization 0268 used, so a grandfathered
row is matched the way it was written.
* fix(db): give the SSO index migration the concurrent-build convention it skipped
packages/db/scripts/migrate.ts documents the required shape for CONCURRENTLY
statements, and the previous migration follows it. 0284 did not, and the
omission is silently destructive.
migrate.ts sets a session lock_timeout of 5s, which survives the embedded
COMMIT. CREATE INDEX CONCURRENTLY waits on every concurrent write transaction in
the database — not only ones touching this table — so on a busy database the
build is cancelled with 55P03 and leaves an INVALID index. The retry then
replays the file, IF NOT EXISTS skips the invalid index, and DROP INDEX removes
the only working index on provider_id. The migration journals as applied and
exits 0 with provider_id unindexed and uniqueness unenforced, reopening the
cross-tenant provider resolution this migration exists to close.
Adds SET lock_timeout = 0 around the concurrent statements, a pre-drop of the
target index name so a replay rebuilds rather than skips, and restores the 5s
timeout afterwards. Verified by stranding an INVALID index and replaying: the
end state is a valid unique index with uniqueness enforced.
Also corrects the sso() comment that claimed domainVerified confines linking to
matching email domains. link-account.mjs blocks on
`!isTrustedProvider && !userInfo.emailVerified`, so an IdP asserting
email_verified links regardless of domain — the flag narrows nothing on its own.
* fix(sso): give the Enter shortcut the same guard as the Add domain button
The Enter handler called handleAdd unconditionally while the button was disabled
during an in-flight add, so repeated presses could issue overlapping requests.
Both now read one canAddDomain flag rather than duplicating the condition.
* fix(sso): stop the provider ID being editable after it is saved
Renaming it was never useful and always destructive. The value forms the
redirect URL registered with the identity provider, so changing it breaks
sign-in until the IdP is updated. Worse, the register route selects
create-vs-update by (providerId, organizationId), so a renamed id misses and
registers a SECOND provider; the settings page renders providers[0], so the
duplicate is invisible, there is no delete action to remove it, and existing
account rows still reference the old id.
Editing now shows it as a read-only copyable value, and the create form says up
front that it cannot be changed later.
Also hoists the suggestion list to module scope — it was rebuilding 44 objects
on every keystroke anywhere in the form.
* fix(sso): stop persisting generated IdP metadata so SAML cert rotation works
The route stored an IdP metadata document built from cert + entryPoint even when
the admin supplied none. The form loads that document back into its optional
metadata field and resends it, and on the next save it wins over the certificate
— so rotating a SAML signing certificate through the form appeared to succeed
and changed nothing.
Only metadata the admin actually pasted is persisted now. With none stored,
Better Auth's createIdP builds the IdP from issuer, entryPoint and cert, which
are the fields the form edits. No SAML providers exist in production, so this
changes no live tenant.
* fix(sso): always write SAML IdP metadata so clearing it takes effect on update
Not storing generated metadata fixed new providers but not existing ones: Better
Auth merges SAML config with `??`, so omitting the key let a previously stored
document survive and keep overriding the certificate.
The key is now always written, empty when the admin supplied none. createIdP
falsy-guards it and falls back to issuer/entryPoint/cert, so clearing the field
actually clears it.
* fix(sso): hold the domain proof under a row lock while granting trust
Two fixes from review.
The trust grant folded the ownership test into the UPDATE's WHERE clause, but
under READ COMMITTED the EXISTS subquery is evaluated against the statement's
original snapshot. A delete committing while the UPDATE waited on the provider
row could therefore still see the removed sso_domain row and grant trust after
ownership was gone. The grant now selects the proof FOR SHARE inside a
transaction before writing, so the delete blocks until it commits, and if the
delete committed first the select finds nothing and no trust is written.
Editing a SAML provider also broke on configs written by the previous commit:
hydration used `config.idpMetadata?.metadata || config.idpMetadata`, and
`{ metadata: '' }` is falsy at the property but truthy as an object, so an object
landed in a string field and failed validation on save. It now narrows on the
type and handles both the object and legacy bare-string shapes.
* refactor(sso): write the two merge-sensitive SAML fields the same way
idpMetadata and identifierFormat both exist to defeat Better Auth's `??` merge,
which silently keeps a stored value when a key is omitted, but they were written
differently — one always, one only when defined. Both are now always written,
empty when unset, under one comment explaining why and noting that each is
falsy-guarded downstream.
Also drops a redundant saveDisabled: false; the prop already defaults to false.
* fix(sso): report the row the trust grant actually matched
The grant returned true once it found the proof, without checking that the
provider UPDATE matched anything, so its boolean did not always mean what
callers read it to mean. It now reports the matched row.
* fix(sso): restore provider domain trust when a domain is re-verified
* fix(sso): correct the domain-removal warning now that it disables sign-in
* chore(sso): trim verbose comments
* fix(sso): revert a rejected SSO update instead of leaving it stored
|
||
|
Vikhyath Mondreti
|
293dc5b87c | fix(execution): duplicate execution issue (#6316) | ||
|
Vikhyath Mondreti
|
377702f701 |
fix(uploads): manual file uploads must ignore provenance stamping (#6314)
* fix(attachments): model egress attachments * fix(uploads): manual uploads provenance ignore * further checks on kb * tags fixes * fix more stuff * fix tests * address comments * fix * fix * fix * address timestamp concern |
||
|
Vikhyath Mondreti
|
e0a464dc66 | fix(attachments): model egress attachments (#6312) | ||
|
Theodore Li
|
079b66c277 | fix(tables): pass enriched query schema to agents (#6305) | ||
|
Waleed
|
3510b0c1c6 |
fix(tables): stop remote cell selections painting over the row gutter (#6310)
* fix(tables): stop remote cell selections painting over the row gutter * fix(tables): classify a remote selection as pinned only when both endpoints resolve * fix(tables): classify an off-window selection endpoint by its column |
||
|
Theodore Li
|
0ebbcc7867 | fix(workflows): await execution log finalization (#6309) | ||
|
Waleed
|
721b471f11 |
fix(providers): pin transport policy and lift the 60s cap on Groq and Cerebras (#6306)
* fix(providers): pin transport policy and lift the 60s cap on Groq and Cerebras * fix(providers): cover the option payload, drop the unused discovery constant * fix(guardrails): forward the caller's abort signal to hallucination scoring * fix(guardrails): surface a cancelled scoring run as cancellation, not a failed guardrail * fix(guardrails): return 499 on a cancelled validation instead of a failed verdict |
||
|
Vikhyath Mondreti
|
c530d276b7 |
fix(env): flag combinations for sandboxes (#6308)
* fix(env): flag combinations for sandboxes * more changes |
||
|
Waleed
|
79bfff728b |
fix(combobox): keep the dropdown open while dragging its scrollbar (#6307)
* fix(combobox): keep the dropdown open while dragging its scrollbar * improvement(combobox): move pointer-press notes into TSDoc |
||
|
Vikhyath Mondreti
|
117fe3137b |
feat(code): cli sandboxes, enterprise timeouts, secrets projections, resolver lift, workflow exec cancellations (#6247)
* feat(code): cli sandboxes, enterprise timeouts, secrets projections, resolver lift * fix(execution): harden compatibility and secret diagnostics * fix(execution): harden generated JavaScript literals * fix(execution): align timeout cleanup semantics * fix(tables): decouple stale job cleanup * fix(execution): drain stale workflow backlog * test(sandbox): make deadline assertions timing-safe * fix(execution): lock cleanup candidate batches * fix(execution): preserve cleanup failure metrics * cancel route fixes * separate out mship template and func template * fix * fix(execution): harden secret projection and block runs * fix(workflow): validate draft execution state * run from block ui disabling * feat(copilot): expose Sim sandboxes to mothership * feat(copilot): expose sandbox capability catalog in VFS * Updates * fix legacy logs showing up * fix(copilot): keep sandbox config visible * fix model provenance issues * fix lint' * more lint * more * test(files): align provenance copy query order * consolidate migrations, rollout compat * integration projections * update skills * fix * add provenance linters * fix: address review and compatibility regressions * fix: make tool boundary audit Bun 1.3 compatible --------- Co-authored-by: Siddharth Ganesan <siddharthganesan@gmail.com> |
||
|
Justin Blumencranz
|
5baa7a41ec |
fix(files): uniquify materialized upload names (#6273)
* fix(files): uniquify materialized upload names * fix(files): sync materialized display names * fix(files): return materialized file names |
||
|
Waleed
|
5dbe95e0c1 |
fix(files): reserve image layout space so images stop reflowing on load (#6299)
* fix(files): reserve image layout space so images stop reflowing on load
A markdown image with no stored dimensions reserved zero vertical space until
it downloaded, then snapped to its natural height and pushed content below it
down (cumulative layout shift). Reserve the box up front from the image's
intrinsic aspect ratio instead.
Store intrinsic width/height as workspace_file metadata (not in the markdown —
it stays clean ``), read it synchronously from the already-loaded file
list to reserve a responsive aspect-ratio box on first render, and lazily
backfill it once per image on first view via a write-gated, idempotent PATCH.
The node view falls back to on-load measurement for the first-ever view and for
external images. Images stay fluid (max-width:100%, height:auto).
* fix(files): address review — reserve on stale memo, clear dims on content swap
- onLoad guards on the memoized storedDimensions the render uses (not a fresh
cache read), so a sibling's non-reactive backfill can't leave a view unreserved.
- updateWorkspaceFileContent clears width/height when it swaps bytes, so stale
dimensions can't be reserved for new content (and the null re-enables backfill).
- Keep optimistically-cached dimensions when the PATCH fails (correct measurement;
a 403/transient error shouldn't wipe sibling reservations).
- Test imports the sibling via the absolute @/ path.
* fix(files): re-derive image dimensions on content swap instead of clearing
Clearing width/height to NULL on a content swap reopened the width IS NULL
backfill path, so a late fire-and-forget PATCH for the previous image could
write its stale size onto the new content. Instead, measure the new bytes'
intrinsic dimensions server-side (image-size, headers only) and store those
(or null for a non-image), so the row always matches the current content and a
stale backfill can't apply.
* fix(files): self-heal image dimensions from the browser instead of server-measuring
Round-3 review: server-side image-size returns raw (non-EXIF) dimensions, and
clearing dims on content swap reopened the stale-PATCH race for non-image or
unmeasurable content. Move authority to the browser's own naturalWidth/Height
(EXIF-correct): the node view reserves from it and reports on any mismatch, and
updateWorkspaceFileDimensions overwrites (no width IS NULL gate) so stale values
self-correct on the next view. Reverts the server-side measurement and the
content-swap dimension touch entirely.
* fix(files): clear image dimensions on content swap (completes self-heal)
The self-heal rework left the old image's dimensions in the row after a content
replacement, so the next view of the new bytes reserved a wrong-sized box before
correcting. Clear width/height on the content-swap write so the row never
describes stale content: the next view falls back to the baseline first-load
reflow and the browser's measurement backfills the correct size. No server-side
decode (EXIF-safe), and the client's overwrite-on-mismatch handles a late PATCH.
* fix(files): guard dimension writes by content key so a stale PATCH can't persist
Ties the dimensions write to the storage key the client measured. The key is
regenerated on every content replacement, so an in-flight PATCH measured against
superseded bytes is rejected at the DB (WHERE key = measured key) instead of
persisting the old aspect ratio for new content. Closes the last stale-ordering
window Greptile flagged — the write is now content-version-conditioned, not just
corrected on the next render.
* chore(files): fix stale route TSDoc and hoist a regex literal (cleanup pass)
Post-review /cleanup: the dimensions route TSDoc still described backfill-once
behavior (now overwrite-on-mismatch via the content-key CAS); the bare-pixel
width regex is hoisted to module scope. No behavior change.
* fix(files): reflect the content-version guard outcome in the dimensions response
The route returned success:true even when updateWorkspaceFileDimensions matched
0 rows (the CAS rejected a write whose measured key no longer matches the row).
Return success:<whether a row was written> and widen the contract response to
{ success: boolean }. Not an error path — the client's next measurement persists
once its file list has the new key; this just stops the API claiming a persist
that did not happen.
* fix(files): reconcile the cache when a dimension write is content-version-rejected
Previously the client discarded a success:false (CAS-rejected) response, leaving
its optimistic patch — which is for superseded bytes — lingering in the file-list
cache. On rejection, invalidate the list so the cache reconciles with the new
content (whose real size persists on its next load). Deliberately NOT a retry:
re-sending the old measurement under the new key would write the wrong size. A
transport error / read-only 403 still keeps the optimistic value (it's the real
displayed size).
* docs(files): align stale dimension docs with the overwrite/self-heal behavior
Cleanup audit: the ImageDimensionsSource/reportImageDimensions interface docs and
one route log string still said backfill-once/no-op; the mechanism overwrites on
mismatch to self-correct. Wording only, no behavior change.
|
||
|
Waleed
|
a7d6b96132 |
fix(docs): stop the sidebar drifting when page content resizes (#6301)
* fix(docs): stop the sidebar drifting when page content resizes The sidebar used fumadocs' `sticky` positioning, and a sticky box is bottom-limited by its containing block. #nd-docs-layout ends ~660px above the document bottom because the site footer is a sibling of the layout rather than a grid child, so across the whole footer the sidebar was pushed progressively upward. Any content-height change while the reader sat in that zone then moved it: expanding one FAQ row shifted the sidebar 16.8px at a fixed scroll offset, and collapsing it shifted it back. Pin the sidebar and its divider to the viewport instead. A fixed box ignores both the container's end and the document height, so neither the drift nor the jump can happen. The grid columns are explicit (`0px 300px 1fr 268px 0px`), so taking the placeholder out of flow leaves its track intact and the content column does not move. The footer is already opaque and now out-stacks both, so it slides over them at the end of the page. Measured with Playwright before and after: sidebar delta on expand/collapse 16.8px/-16.8px -> 0px/0px, content column left and width unchanged, and the sidebar holds top:92px at the page bottom on the docs, API-reference, academy and integrations layouts. Mobile is untouched (the rule is desktop-only). * refactor(docs): drop dead grid placement, move footer stacking to the component Review follow-ups. The divider's `grid-row`/`grid-column` stopped doing anything the moment it became `position: fixed` — a fixed box is out of grid layout entirely — so they and the comment explaining the grid span were describing positioning that no longer happens. Verified inert: the divider still computes to left 300px / width 1px / z-index 21 without them. The footer's stacking context also belongs on the footer, not in a global rule matching every desktop `footer` element, so it moves to the component as `relative z-[22]` with the reason in its TSDoc. |
||
|
Waleed
|
7193035a72 | fix(knowledge): scroll the whole chunk editor area instead of the textarea (#6300) | ||
|
Theodore Li
|
21725585d2 |
improvement(tables): raise the max column limit to 1000 (#6295)
Bumps TABLE_LIMITS.MAX_COLUMNS_PER_TABLE from 50 to 1000. Every validation site (schema validation, add-column, bulk add, workflow output columns, and the boundary contract) already derives from this constant. |
||
|
Waleed
|
2ba248472e |
fix(editor): restore caret alignment and the intended type scale across the panel (#6297)
* fix(editor): restore caret alignment and the intended type scale across the panel
Four distinct defects, all surfaced while testing the workflow editor.
**Caret drift in the Start block's Description field.** Its overlay mirror was
built differently from every sibling field: `overflow-hidden` + `truncate` and
no scroll synchronisation, where the working fields use `overflow-x-auto` +
`whitespace-pre` + `syncOverlayScroll`. Once the value passed the visible width
the input scrolled and carried the caret with it while the overlay stayed pinned
at the first character, so the gap grew as you typed. It now has the same
plumbing as its siblings, on its own ref maps so it cannot collide with the value
overlay.
**Overlay mirrors left at 500 over 400 inputs.** #6291 dropped emcn Input and
Textarea to the inherited weight, but the canvas files that mirror them were
reverted from that PR, so 11 overlays kept a hardcoded `font-medium`. A mirror
that renders heavier than the input beneath it misaligns by the weight delta on
every character. All 11 realigned.
**Input text tracking differently from its label.** The UA stylesheet resets form
controls to `letter-spacing: normal`, so inside `.workspace-root` (0.02em) an
input diverged from surrounding text — and from its own overlay — by 0.28px per
character. emcn Input and Textarea now carry `[letter-spacing:inherit]`, which
fixes every mirrored input at the source rather than per call site.
**Weights and type sizes that changed meaning under #6241.** That PR deleted the
tailwind remap of `font-medium` (440 light / 480 dark) without migrating the
~505 call sites written against it, so untouched code jumped to a stock 500. The
panel's editor, toolbar, chat and connections surfaces are swept back to the
inherited weight. The Chat header was also visibly taller than Toolbar and Editor
purely because it used `text-[14px]` — font-size with no paired line-height —
against otherwise byte-identical containers; it and the panel's two other
arbitrary sizes now use named tokens.
Also fixes five JSX conditionals in the workflow MCP settings page that had lost
their braces, so `canManage && ()` rendered as literal text under the server
detail tab.
Verified: typecheck 0, biome clean, 18644/18645 vitest passing (the one failure
is a missing `rg` binary and predates this branch).
* fix(emcn): give chip text fields the same tracking as their mirrors
An audit of the previous commit found the letter-spacing fix was incomplete: it
landed on `Input`/`Textarea` but not on the chip family, so `ChipInput` and
`ChipTextarea` kept the UA `letter-spacing: normal` while any overlay mirroring
them inherited the ambient tracking.
The MCP server form modal is a live instance — its shared `FormattedInput` layers
a transparent `ChipInput` under a visible div, across the server URL and both
header fields, whose values are long by nature. The caret separated from the text
by roughly 0.28px per character.
Fixed on `chipFieldTextClass` rather than the call site, so every chip field
matches its mirror the way `Input`/`Textarea` already do.
* fix(editor): restore the strong-text variant a class sweep welded together
The font-weight sweep in
|
||
|
Waleed
|
0bef1c3099 |
fix(files): stop a cached collab snapshot resurrecting blank lines (#6293)
* fix(files): stop a cached collab snapshot resurrecting blank lines A collaborative markdown file's cold-start seed can come from a cached Yjs snapshot (workspace_file_collab_state.doc_state) rather than a fresh markdown re-parse. The snapshot is a raw CRDT binary, so it preserves top-level empty paragraphs that parseMarkdownToDoc/stripEmptyParagraphs strips from every parse target. The static placeholder always re-parses (clean); a warm seed replays the snapshot verbatim, so a stray blank line appears once the doc settles — and only intermittently, since a stale/cold cache falls through to the clean re-parse. Enforce the same no-top-level-empty-paragraph invariant on the Yjs side: - normalize.ts: stripEmptyTopLevelParagraphs(doc) shared helper. - seed.ts: repair the cached snapshot on read (self-heals legacy snapshots, preserving CRDT client ids; no data migration). - persist.ts: normalize before caching so new snapshots are clean by construction. * refactor(collab-doc): make COLLAB_DOC_FIELD a single canonical constant converter.ts had a duplicate 'default' fragment-name constant; import the now-exported one from normalize.ts so the value TipTap's Collaboration binding depends on lives in exactly one place. Fold the back-to-front loop note into the helper's TSDoc. |