v0.8.18: sandbox files mounting, ttl tables column type - #7283
Open
waleedlatif1 wants to merge 15 commits into
Open
v0.8.18: sandbox files mounting, ttl tables column type#7283waleedlatif1 wants to merge 15 commits into
waleedlatif1 wants to merge 15 commits into
Conversation
#7275) A Slack-triggered run's subject is the external Slack user, and a schedule or public-API run has no subject at all. list_groups, list_people, and send_invite demanded a Sim user, so every unattended run got "Credential Group user access required" — including reads that need no actor. Authority for an actorless caller comes from the deployment the workspace layer already checks. Invitations no longer name an inviter when there is no person to name, rather than borrowing the run's actor and claiming someone invited when they did not.
* feat(function): mount referenced files into the code sandbox
Reference a file's path in Function block code and it is mounted for you:
`<block.file.path>` resolves to its location on the sandbox filesystem, so
any language can open it. It is the counterpart to `.base64`, which inlines
the bytes and only works in JavaScript, leaving Python and Shell with no way
to read a referenced file at all.
Resolution happens long before a sandbox exists, and mount paths are only
assigned once the whole set is planned together, so the resolver leaves a
marker that the function runtime swaps for the real path — the same shape as
LargeValueRef.
Files written to /tmp/sim/outputs come back as platform file objects, so they
can be attached or uploaded with no intermediate step. Harvesting is automatic
on runs already in the remote sandbox; isolate runs pay nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(execution): cover the sim.* helpers in a real isolate
isolated-vm.test.ts mocks the spawn, so nothing proved the sim.* namespace
was reachable from user code — only that the process plumbing was called.
These run the real worker and assert values cross the boundary both ways.
Pins the fast runtime's global surface, which turned out narrower than
assumed: plain ECMAScript plus fetch, console and sim.*, with no Buffer,
require, process, crypto, TextDecoder, atob or setTimeout. That list is
exactly what decides whether a block needs an import and so moves to the
remote sandbox, so it is asserted rather than described.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: regenerate tool metadata and integration docs
Changing function_execute's params and adding file_write's fileInput made
both generated artifacts stale, which check:audits catches.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(function): address review findings on sandbox file I/O
- Classify harvested output by content, not by file name. Gating the
provenance scan on a filename-derived MIME type let a resolved secret be
written as plaintext under a binary-looking extension and skip the only
guard before upload. Bytes that round-trip as UTF-8 are scannable
whatever they are called.
- Format the .path replacement through the shared context-aware helper.
Returning a bare identifier inserted __blockRef_N literally in Shell and
inside quoted strings instead of the mounted path.
- Enforce the mount ceiling on the combined set. Marker-derived mounts
bypassed the contract's max, which only bounded the explicit files param.
- Reuse one marker per file key, so referencing a path twice mounts once.
- Raise a coded error when the code deletes the output directory, so it
reports as a 400 with remediation rather than an opaque 500.
- Drop declared paths from the discovered set; counting a file in both
rejected a single output larger than half the byte ceiling.
- defineProperty when rebuilding context values, so an own __proto__ key
survives instead of hitting Object.prototype's setter.
- Give the directory sentinel a collision-resistant name, and cap
file_write's fileInput at the destination's own limit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(function): include files in the FAQ's output answer
Adding <function.files> to the outputs table left the FAQ still saying the
block returns two outputs, and the reference FAQ never mentioned that a
file is read by referencing its path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(function): scan every harvested output for resolved secrets
Gating the scan on whether the bytes looked textual was defeatable twice
over: name the file .png, or append a single invalid byte, and a plaintext
secret skipped the only guard before upload.
A lossy UTF-8 decode preserves ASCII runs, so a literal secret is findable
in any buffer. The scan is now unconditional. What remains out of reach is
a secret carried in transformed form, which no substring scan can see —
an inherent limit of scanning rather than a hole in the gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(function): correct provenance, mount caps and runtime file plumbing
Review findings from the sandbox file I/O change, traced to root cause.
A file_write carrying fileInput copied its bytes into a new workspace
file without the source's secret lineage, so a file the platform had
locked as secret-derived became readable again under a new id. It now
derives provenance from the source through the same helper archiving
uses, which also marks a source with no workspace row unknown rather
than empty.
The tool half of that feature had shipped without the block half: the
File block's write mapper listed four params and dropped fileInput, and
no sub-block could supply one, so the documented binary path was
unreachable from the canvas and from an agent holding the File tool.
Adds the canonical pair, maps it, and drops content's unconditional
required now that the two are mutually exclusive sources.
The harvest counted secret names the matcher had already discarded as
too short to identify anything, so a workflow whose every secret was
under the substitutable-literal minimum built no matcher, classified
every output unknown, and refused each one while claiming it held a
secret. The counter now applies the matcher's own predicate.
The execution file index was lazily created on the per-call context
clone, so files a tool produced were recorded onto a throwaway and the
next call in the run never saw them. Materializing it on the source
before the spread makes both objects share one map.
A .path reference inside a quoted Python or JavaScript string was
JSON-encoded, putting literal quote characters inside the path the code
then opened. Mount paths are sanitized to a safe character set, so they
splice raw.
URL mounts now carry a ceiling that curl enforces on the bytes actually
served, rather than trusting a recorded size, and declared sandbox
outputs opt out of the harvest explicitly instead of relying on which
branch returns first. Also stops rebuilding non-plain context values
into stripped objects, and removes an agent tool-description branch that
no longer had a sub-block to read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(file): send one write source, and bound mounts by what they were charged
The write card sent `content` unconditionally, and the contract counts any
defined `content` as "text was provided" — so an untouched Content box,
which serializes as an empty string, collided with a selected file and
rejected every binary write. The mapper now emits only the source the card
carries, leaving the contract to catch filling both or neither.
A generated document that references other files needs a principal to
resolve them; without one the resolver can only serve an already-published
artifact and throws. The write path now passes it.
URL mounts are granted exactly the byte count they were charged against the
aggregate, rather than being charged a reported size while permitted the
global per-file maximum — twenty mounts each claiming a byte could
otherwise be allowed 500MB apiece. An honest size fetches normally and an
understated one is refused.
`--max-filesize` only refuses a transfer up front when the response
declares a Content-Length, so a chunked reply slipped past it. The
delivered file is now measured and removed if it overran, in the same
command rather than a second round trip.
Also documents that naming an explicit sandbox output path excludes the
harvest directory, in both the block docs and the tool description the
model reads.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(file): accept the picker shape, and bound a mount while it downloads
The write path required an already-complete UserFile, but the file picker
stores {name, path, key, size, type} with no id or url — so selecting a
file in the new basic field was rejected as not a file object before any
bytes moved. It now runs through the same normalizer every other operation
in that file applies to its own input, while a block reference or an
agent-resolved id still passes through as-is.
Writing a deliberately empty text file was also rejected, because an empty
string read as "no text". The selected file is what disambiguates now: with
one present an empty Content box means unused, and with none, content
always goes through.
--max-filesize only refuses a transfer that declares a Content-Length, so
the previous size check ran after the whole object had already been written
to sandbox disk. The fetch now streams through a byte cap, so at most one
byte over the limit can ever land, and curl's status travels through a file
so a 403 on an expired URL is still distinguishable from an empty download.
A non-finite mount size made every comparison false, so the aggregate check
passed while the mount was charged the per-file maximum anyway; the size is
resolved once now, before either test.
Mount resolution failures are the caller's files — unreadable, oversized,
or over the aggregate — and now answer 400 with the message naming the
file, rather than a 500. Files already uploaded when a later one in the
same harvest is refused are removed, since the export is all-or-nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(file): clamp a declared mount cap, and demand identity not full metadata
sandboxFiles reaches the sandbox layer from the request body, so a declared
maxBytes is a caller's number. It may now lower its own mount's ceiling but
never raise it past the one that layer guarantees.
The write path required full UserFile metadata, but size is never read
before the download and the download reports the real content type — so a
reference carrying id, key, url and name was rejected over two fields
nothing depends on. It now asks only for identity and fills the rest.
Uploads already made are also discarded when a later upload throws, not
only when a later file is refused for carrying a secret. Both exits leave
the harvest all-or-nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(file): answer null for a key a file input cannot be classified by
fileInputToUserFile normalizes caller-supplied file objects and returns
null for anything it cannot use — but it classified the storage key with
the throwing form, so a key without a recognized context prefix escaped as
a 500 from every operation that normalizes a file input, not just write.
Adds tryInferContextFromKey beside inferContextFromKey, sharing the one
prefix list so a new context cannot be added to half of them. The throwing
form stays right where an unclassifiable key means the platform built one
wrong; the nullable form is for keys that arrived in a request, where an
unrecognized prefix only means this is not a file we can use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The admin workspace move was restricted to personal/grandfathered sources; `assertWorkspaceMovable` refused anything already owned by an organization, so support could only re-home a workspace with manual SQL. Relax that guard to a drift-only check and handle the source organization. `changeWorkspaceStoragePayerInTx` already accepted an arbitrary source payer, so the storage-ledger rebalance needed no change. Moving a workspace between organizations is the first operation capable of separating an artifact from the organization that owns it, so two invariants nothing has ever had to defend are enforced here: - A custom block and its bound workflow always share an organization. `getCustomBlockAuthority` resolves by the consumer's org and `admitCustomBlockChildExecution` skips its concurrency reservation on the strength of that, so a stranded row would run a foreign tenant's workflow under its owner's credentials, billed to the wrong payer. The move unpublishes those blocks through the product's own `deleteCustomBlock` and records the loss in the source organization's audit view. - A fork parent and child always share an organization. `resolveForkEdge` has no org check at all, so the move refuses while a cross-org edge would result. Move-time checks alone cannot hold either invariant, because the writers can commit after them. `publishCustomBlock` now validates and inserts under the organization mutation lock the move holds, and `createFork` row-locks its parent so the move and the organization-attach path — both of which take `FOR NO KEY UPDATE` on that row — serialize against it. Pending invitations block: re-stamping an org-scoped invitation would convert a pending membership in the source org into one in the destination, consuming a seat for an invitation the destination never issued. An entitlement downgrade blocks. When entitlement is subscription-backed, both organizations are re-evaluated under the locks rather than trusting a pre-transaction read that is stale in both directions. The two modes where `resolveOrganizationEnterprisePlan` grants entitlement by deployment configuration are excluded through a shared `isSubscriptionBackedEntitlement` predicate, so a missing subscription row is never misread as a lapse. Both organizations are locked, ascending by id, mirroring `acquireOrganizationUserMutationLocks`. The source id is read optimistically before the transaction and re-verified under the locks, retrying through the existing loop when it moved.
…orkflows Beyond HubSpot and Zapier (#7280) * feat(library): AI Agents for Marketing Automation: Building Agentic Workflows Beyond HubSpot and Zapier * fix(library): use Chat and Sim instead of the prohibited Mothership name The constitution reserves "Sim" for the agent and "Chat" for the surface; "Mothership" names no product surface a reader can find. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Sim Pi Agent <pi@sim.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…7281) * fix(provenance): let a run that never started report why it failed A copilot-run workflow that fails before reaching the engine crossed back with no provenance, which latched the tool's registry and reduced the result to "result unavailable". The caller was told its run failed but not that the workflow was undeployed, or the input invalid, or the slot unavailable — the reasons this layer produces before any block runs, naming no secret because none had been resolved yet. The executor attaches its execution result to every throw, so the absence of one is proof that no block ran: output, logs and error are all undefined and the only content is a message this layer wrote. That is an absence, not an inability to vouch, so the crossing now carries an exact-empty envelope. The message still passes the tool boundary's egress projection against the same registry, so anything that registry knows is still redacted. A run that did execute and could not vouch hands back its incomplete envelope exactly as before, and that still latches. Make the attach total rather than conditional to keep that inference sound. A block failure is already normalized on the way in, so the old `instanceof Error` guard held in practice; what it did not give was a guarantee covering a non-Error raised by the engine's own synchronous work. toError is identity-preserving, so ordinary failures keep their type. The empty envelope moves to the registry module, which owns the vocabulary, replacing a private copy in the logging session so one definition states what "vouched for, naming nothing" is. * fix(provenance): keep the post-run crossing window out of the never-started claim Review round 1, both findings accepted. The post-run crossing runs inside the same try as the executor call, so when that import is what throws, the catch sees an error carrying no execution result — the same evidence a run that never started leaves. The previous condition read that as "nothing crossed" and vouched for it, when in fact an execution exists and its provenance was never imported, which is exactly the content that cannot be vouched for. Record whether the executor returned and require both facts before claiming the absence: not past the executor, and no result attached. Everything else hands back whatever envelope it has, and an incomplete one still latches. The executor test also could not fail against the old gated attach: a block failure is normalized on the way in, so its rejection already arrived as an Error. Drive it through the cancellation subscribe run() awaits before the queue instead, which is its own synchronous work and reaches the catch untouched — the case the total attach exists for. * fix(provenance): carry the run's result through post-execution failures Round 2, cubic's finding accepted — and it was a distinct window, not a restatement of round 1. The executor's post-execution work runs after the run has produced a result but before `executeWorkflow` returns, so a failure there reached callers with no result attached: the run threw nothing itself, and the flag added last round could not be set yet. Every consumer that reads a missing result as "no block ran" was wrong in that window, this crossing included. Fix it where the result lives rather than at each reader. The executor attaches its own on the throws it raises; `executeWorkflow` now does the same for failures raised after it holds one, skipping the case the executor already recorded. Logging and trace spans get the same benefit for free — they read the identical signal. That makes an absent result total again, so the boolean flag goes and the crossing reads one thing: the result from the error, or the one already returned when the failure came later still, from the crossing itself. Only a failure with neither can claim nothing ran. The post-return case now describes content with the run's real envelope rather than latching blind, which is strictly more accurate than either prior behaviour. * fix(provenance): normalize a post-execution failure so it can carry the result Round 3, cubic's finding accepted. The guard added last round required the caught value to already be an `Error`, so a non-Error raised by post-execution work skipped the attach and was rethrown bare — the same hole this branch closed in the executor, left open one layer up by my own change. A Copilot run would have reported an executed workflow as never started and vouched for content it cannot describe. Normalize once at the top of the catch and use that value throughout, including the rethrow, matching what the executor does. `toError` returns an `Error` unchanged, so a custom error class keeps its identity and every ordinary failure is untouched — the existing identity assertion on the rejection path still holds. Two tests: the result reaches an ordinary post-execution failure, and a non-Error one is normalized so it can carry the result too. The second fails against the previous guard.
* feat(billing): record function sandbox usage * fix(billing): charge function user-code failures * fix(billing): correct sandbox trace cost boundaries * fix(billing): tighten sandbox completion boundaries * test(billing): check the metered sandbox amount against a real provider The pricing unit test pins the arithmetic and the conformance suite proves a cost is produced, attached to the right outcomes, and routed — but that suite stubs the provider and mocks Date.now() with a counter advancing one millisecond per call. Under that clock `total > 0` is the strongest claim available, and it holds equally well if the metered window is anchored to the wrong instants or the resource constants are wrong. Bounds the charge between what the sleep must cost and what the wall clock could justify, so a wrong rate, a wrong vCPU/memory constant, and a mis-anchored window all fail. Provider-agnostic via resolveProvider, opt-in behind SANDBOX_BILLING_SMOKE=1 like the sibling smoke suites. Verified against both providers: E2B billed 9.031s of a 9.302s call at $0.1656/hr, Daytona 8.435s of 8.721s at $0.16668/hr — both matching published rates, both excluding ~275ms of Sim-side overhead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(billing): meter the sandbox a cloud Pi session runs in Pi's own sandbox was never metered. withPiSandbox called createSandbox without the meterUsage argument, so only sandboxes created through executeFunctionRequest were charged — and Pi's is the larger consumer by an order of magnitude. A Function block holds one for seconds; a Pi session holds one for a minimum lifetime of 31 minutes. The gap was worst exactly where it was least visible. A Pi coding agent normally runs BYOK, so its model cost is zero by definition, and the ledger bills a model row on total > 0. With the sandbox unmetered, such a run produced a zero-cost model_unbilled row and Sim collected only the flat execution fee while paying its provider for the whole session. Threads a cost sink through PiRunContext, which is the seam backends already receive and the only one that reaches all four cloud modes. The handler owns one sink covering both sandbox sources — Function tools in local mode, the agent's own sandbox in cloud mode — so neither can be dropped where the cost is folded into the block's output. It rides in toolCost for the same reason the Function tool cost already does: that is what survives the BYOK zeroing. Unlike the Function path this charges on creation rather than on a completed session. A Function run is seconds long, so absorbing one the provider failed to deliver is cheap and reads as fair; tens of minutes of Pi compute is consumed whether the agent finished, errored, or was cancelled, and billing only clean endings would mean paying for every other one. A create that throws still costs nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(billing): check the Pi sandbox charge against a real provider The handler test mocks the backend and writes into the cost sink by hand, so it proves the wiring from a backend to the block's cost and nothing more — it would still pass if withPiSandbox never metered at all, which is precisely the bug that path had. Holds a real Pi sandbox open for a known interval and bounds the charge between what that interval must cost and what the whole session could justify. Verified to fail against the original unmetered call with "expected 0 to be greater than or equal to 0.00023", and to pass once the sink is threaded: 5.949s billed of a 6.141s session on E2B. The second case pins the other half of the contract — a caller that supplies no sink is not charged, which is what keeps mothership and other internal Pi sandboxes free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(billing): charge a Pi session only when it completed Aligns Pi with the outcome policy the Function path already applies rather than keeping the divergence the previous commit introduced. A session that ends by throwing — a provider crash, a lifetime limit, a cancellation — is absorbed, because a charge nobody can tie to delivered work is not one worth defending, and consistency across the two sandbox paths is worth more than recovering the cost of runs that failed. A command exiting non-zero is still billed: the callback returns normally there and the agent produced its answer, which is the same reason the Function path bills its own non-zero exits. The window still closes at teardown, so a completed session is charged for the whole time the provider held its sandbox. Verified on both providers, including that the new case fails when the charge is applied unconditionally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(billing): keep the charge on completed runs that fail after execution Three paths dropped cost the sandbox had already earned. A harvest that cannot return what the run produced — more files than the export limit, nesting past the listing depth, or an output directory the code deleted — was excluded from the billable-error set. All three arrive only after the sandbox has executed and all three are the caller's to fix, so they belong with the post-completion export failures the policy already bills rather than the provider failures it absorbs. A completed run whose code wrote one file too many went free. That also left the route with nothing to read: it already consults readTrustedSandboxOutputCost for these errors, so attaching the cost at the sandbox layer is what carries it into the response. Separately, a Function block whose handler succeeded could still fail in the steps that follow it — base64 hydration, and large-value redaction that throws rather than emit unredacted data. Those errors carry no cost of their own, so the completed sandbox went unbilled. The handler's cost is now held across that window, in the same way streamingPartialOutput already is, and used only when the error has none. The new conformance case was confirmed to fail against the narrower catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(billing): carry the Pi charge onto a session its agent failed A backend that returns a result carrying `totals.errorMessage` has already run: the sandbox was billed and the sink holds the charge. But that path throws instead of reaching `buildOutput`, which is what publishes the cost, so the charge was accumulated and then dropped — lost revenue rather than an over-charge. Both failure paths now carry it on the error they raise, the same way the Function handler carries its tool cost, so `handleBlockError` can pick it up. An agent that ran and then reported a failure consumed the same tokens and sandbox seconds as one that succeeded, which is why the cost computation is now shared between the two rather than duplicated. Also corrects the sink's doc comment. Local mode does fill it — the agent runs on the caller's own machine and costs Sim nothing, but a `function_execute` among the Sim tools it calls bills its own remote sandbox into the same total. The new case was confirmed to fail without the attach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(tables): add column type extension points * Address PR review feedback (#7119) - preserve explicit nulls from source-owned conversion normalization - normalize hooked values before select migration - cover null and select conversion rewrites * refactor(tables): short-circuit unlimited column types * refactor(tables): keep CSV coercion in import switch * test(tables): cover rebased column dropdown
* feat(tables): add row TTL expiration * chore(api): regenerate table API artifacts * fix(tables): make TTL cleanup fair across tables * fix(tables): reject rolled-over TTL dates * fix(copilot): remove unrelated catalog drift * chore(helm): bump chart for TTL cron * fix(ci): regenerate CLI table restore type * refactor(tables): keep TTL CSV parsing in import switch * docs(tables): clarify TTL epoch-second storage * docs(tables): clarify TTL cleanup timing * fix(tables): prioritize oldest TTL cleanup rows Page cleanup by created_at and id, with a supporting index, so capped runs make progress on the oldest rows first. * improvement(tables): rename TTL column to Expiration * chore(tables): run TTL cleanup every fifteen minutes * feat(tables): gate row TTL expiration * improvement(tables): align Expiration feature messages * chore(db): format TTL migration metadata * test(tables): cover TTL column dropdown * fix(tables): close TTL extension gaps * fix(tables): keep TTL import coercion lightweight
* fix(timezone): consolidate table wall-clock conversion * fix(timezone): preserve ambiguous date semantics * fix(tables): prevent early TTL expiration * fix(tables): wait for timezone before TTL edits * improvement(tables): improve timezone loading state * test(tables): preserve editor timezone during edits * fix(tables): guard row modal timezone edits
* feat(tables): trigger workflows on row deletes * fix(tables): bound delete trigger snapshots * fix(tables): scope and verify delete triggers
Contributor
|
Too many files changed for review (182 files, 100 file limit). Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
…e integration's output, timeout, and redirect defects (#7276) * fix(elasticsearch): resolve a Cloud ID to the real Elasticsearch host * fix(elasticsearch): reject a malformed parent-domain port in a Cloud ID * fix(elasticsearch): correct get_index output, cluster-health timeout, and redirect credentials * fix(elasticsearch): reject a surviving colon in a decoded Cloud ID component * fix(elasticsearch): stop a cloud deployment falling back to a stale host
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
16 issues found and verified against the latest diff
Confidence score: 2/5
- TTL handling has several user-facing failure modes across
cell-render.tsx,inline-editors.tsx,import-coercion.ts, andttl.ts: invalid timezones can crash table rendering, stale timezone state can change edited instants, and malformed imports or values can be silently blanked or interpreted as January 1, 1970. Validate effective timezones and reject invalid or empty TTL values consistently while reporting import rejections. - File inputs can be mishandled in
execute.tsandresolver.ts: serialized arrays may be mounted as a single file ID, while the same file referenced through both input mechanisms can be transferred twice and collide. Normalize serialized inputs before hydration and deduplicate mounts. - Several operation results can silently report incorrect state:
admin-move.tsloses credential counts after confirmation or reload, andproviders/index.tscan omit failed sandbox spend when usage or precomputed cost is absent. Preserve credential summaries and record tool costs independently of provider aggregates. - Configuration and transaction error handling still exposes avoidable failures in
constants.tsandcreate-fork.ts: an oversized row can exceed the snapshot budget, and a concurrent organization change can surface as a generic 500 instead of a 409. Enforce the tighter size limit and preserveForkErrorthrough the transaction wrapper.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/executor/variables/resolver.ts">
<violation number="1" location="apps/sim/executor/variables/resolver.ts:708">
P2: When the same file is supplied through `files` and referenced with `<block.file.path>`, this creates a second mount request. The runtime transfers the object twice under collision-suffixed paths and can reject an otherwise-valid call at the mount-count or aggregate-size limit. Deduplicate files by storage key before planning mounts.</violation>
</file>
<file name="apps/sim/lib/workspaces/admin-move.ts">
<violation number="1" location="apps/sim/lib/workspaces/admin-move.ts:1057">
P2: After confirmation or reload, the operation response reports zero credentials even when the workspace has secrets, environment variables, or BYOK keys. Preserve or reconstruct the credential summary instead of returning `EMPTY_CREDENTIAL_SUMMARY`.</violation>
<violation number="2" location="apps/sim/lib/workspaces/admin-move.ts:1830">
P2: When an admin reloads a completed move from a personal workspace, the response adds a false “source organization was not persisted” notice. Distinguish a missing property from an explicit `null` before emitting the legacy warning.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx:145">
P2: When a persisted user setting contains an invalid timezone, formatting any non-null TTL cell throws `RangeError` and prevents the table from rendering. Validate or fall back the effective timezone before passing it into the cell formatter.</violation>
</file>
<file name="apps/sim/lib/table/column-types/import-coercion.ts">
<violation number="1" location="apps/sim/lib/table/column-types/import-coercion.ts:10">
P2: When a CSV contains an invalid non-empty TTL, `parseTtlEpochSeconds` returns `null` but this fallback returns the raw string. The import then blanks the cell without invoking `onValueRejected`, so `cellsRejected` and the import rejection summary under-report the data loss. Return `null` for parse failures so the existing rejection hook records the rejected cell.
(Based on your team's feedback about surfacing rejections across all CSV import paths.)</violation>
</file>
<file name="apps/sim/blocks/blocks/file.ts">
<violation number="1" location="apps/sim/blocks/blocks/file.ts:1241">
P2: When `content` resolves to a non-string value while a file is selected, `params` drops it and writes the file successfully instead of letting the file-write schema reject the invalid content. Omit only an explicitly empty string; preserve other defined values for validation.</violation>
</file>
<file name="apps/sim/lib/table/constants.ts">
<violation number="1" location="apps/sim/lib/table/constants.ts:165">
P2: When `TABLE_MAX_ROW_SIZE_BYTES` exceeds `DELETE_SNAPSHOT_BATCH_MAX_BYTES`, this fallback still allows a single row larger than the claimed snapshot budget through trigger dispatch. Reject or cap row-size configurations above the snapshot budget, or explicitly account for the larger single-row payload in the budget.</violation>
</file>
<file name="apps/sim/background/cleanup-table-row-ttl.ts">
<violation number="1" location="apps/sim/background/cleanup-table-row-ttl.ts:254">
P2: When a later table cleanup fails after an earlier table has deleted rows, this function exits before emitting the earlier table's change event, leaving open grids stale. Signal each successful batch or from a `finally` path so partial cleanup remains visible.</violation>
</file>
<file name="apps/sim/ee/workspace-forking/lib/create-fork.ts">
<violation number="1" location="apps/sim/ee/workspace-forking/lib/create-fork.ts:214">
P2: When the parent changes organizations during a fork, Drizzle wraps this transaction error before the route can handle it. The API therefore returns a generic 500 instead of the intended 409; preserve `ForkError` through the transaction cause chain or classify it in the fork route before rethrowing.</violation>
</file>
<file name="apps/sim/providers/index.ts">
<violation number="1" location="apps/sim/providers/index.ts:314">
P2: When the provider omits token usage and precomputed cost, failed Function sandbox spend is silently lost. Initialize a zero model cost or attach tool cost independently before applying this aggregate.</violation>
</file>
<file name="apps/sim/lib/table/column-types/ttl.ts">
<violation number="1" location="apps/sim/lib/table/column-types/ttl.ts:71">
P2: When an unset or malformed TTL cell reaches the formatter or conversion hook, `Number(value)` turns it into epoch zero and can display or convert it as January 1, 1970. Reject `null`/`undefined` and empty non-numeric strings before numeric coercion, then preserve the blank through the callers.</violation>
</file>
<file name="apps/sim/lib/core/utils/timezone.ts">
<violation number="1" location="apps/sim/lib/core/utils/timezone.ts:158">
P2: When a caller passes an empty timezone string, `getWallClockParts` treats it as omitted and uses the runtime's local timezone. Check specifically for `undefined` so invalid timezone input is rejected instead of producing host-dependent wall-clock results.</violation>
</file>
<file name="apps/sim/lib/table/rows/service.ts">
<violation number="1" location="apps/sim/lib/table/rows/service.ts:1921">
P2: When a table has matching webhooks, `deleteRow` waits for lookup, preprocessing, admission, and enqueueing before returning. Dispatch this trigger fire-and-forget, matching the trigger contract and insert/update paths, so a slow trigger cannot delay or time out the DELETE request.</violation>
</file>
<file name="apps/sim/tools/function/execute.ts">
<violation number="1" location="apps/sim/tools/function/execute.ts:170">
P2: When advanced-mode resolution supplies `files` as a JSON string, the generic `file[]` hydrator treats the entire JSON document as one file ID before `normalizeSandboxInputFiles` runs. Parse serialized arrays in the shared file-parameter hydration step, or otherwise bypass that step for this representation so valid mounts reach the new normalizer.</violation>
</file>
<file name="apps/sim/lib/table/dates.ts">
<violation number="1" location="apps/sim/lib/table/dates.ts:336">
P2: When `timezone` is supplied for a valid year from `0000` through `0099`, this call resolves the wall clock using 1900-1999 timezone rules because JavaScript remaps those years in `Date.UTC`. Fix the shared resolver to preserve the full year before resolving, or reject unsupported years before this call.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx:114">
P2: When a regular date editor opens before general settings finish loading, this ref freezes the browser fallback instead of the user's saved timezone. Editing the time/date then writes a different offset and instant; keep date editing gated until the timezone is ready before capturing `initialTimeZone`.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| // The bytes are fetched into the sandbox, so the inline copy would be dead | ||
| // weight in the request body. | ||
| const { base64: _base64, ...fileMetadata } = file | ||
| contextVarAccumulator[varName] = createSandboxFileMountRef(fileMetadata as UserFile) |
There was a problem hiding this comment.
P2: When the same file is supplied through files and referenced with <block.file.path>, this creates a second mount request. The runtime transfers the object twice under collision-suffixed paths and can reject an otherwise-valid call at the mount-count or aggregate-size limit. Deduplicate files by storage key before planning mounts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/executor/variables/resolver.ts, line 708:
<comment>When the same file is supplied through `files` and referenced with `<block.file.path>`, this creates a second mount request. The runtime transfers the object twice under collision-suffixed paths and can reject an otherwise-valid call at the mount-count or aggregate-size limit. Deduplicate files by storage key before planning mounts.</comment>
<file context>
@@ -648,6 +665,106 @@ export class VariableResolver {
+ // The bytes are fetched into the sandbox, so the inline copy would be dead
+ // weight in the request body.
+ const { base64: _base64, ...fileMetadata } = file
+ contextVarAccumulator[varName] = createSandboxFileMountRef(fileMetadata as UserFile)
+ }
+
</file context>
| return formatLocalFieldsAsWall(parsed, offsetMinutes) | ||
| const wallClock = isoWallClock ?? localizedWallClock ?? parseNaiveWallClockAsUtc(trimmed) | ||
| if (!wallClock) return null | ||
| return zonedWallClockWithOffset(wallClock, options.timezone, { |
There was a problem hiding this comment.
P2: When timezone is supplied for a valid year from 0000 through 0099, this call resolves the wall clock using 1900-1999 timezone rules because JavaScript remaps those years in Date.UTC. Fix the shared resolver to preserve the full year before resolving, or reject unsupported years before this call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/table/dates.ts, line 336:
<comment>When `timezone` is supplied for a valid year from `0000` through `0099`, this call resolves the wall clock using 1900-1999 timezone rules because JavaScript remaps those years in `Date.UTC`. Fix the shared resolver to preserve the full year before resolving, or reject unsupported years before this call.</comment>
<file context>
@@ -238,11 +331,12 @@ export function normalizeDateCellValue(
- return formatLocalFieldsAsWall(parsed, offsetMinutes)
+ const wallClock = isoWallClock ?? localizedWallClock ?? parseNaiveWallClockAsUtc(trimmed)
+ if (!wallClock) return null
+ return zonedWallClockWithOffset(wallClock, options.timezone, {
+ ambiguousTime: options.ambiguousTime ?? 'earlier',
+ offsetMinuteRounding: options.offsetMinuteRounding,
</file context>
| * response, which is exactly when the operator most needs to see what the | ||
| * move did and where it came from. | ||
| */ | ||
| const recordedSourceOrganizationId = operationPayload.audit.sourceOrganizationId ?? null |
There was a problem hiding this comment.
P2: When an admin reloads a completed move from a personal workspace, the response adds a false “source organization was not persisted” notice. Distinguish a missing property from an explicit null before emitting the legacy warning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/workspaces/admin-move.ts, line 1830:
<comment>When an admin reloads a completed move from a personal workspace, the response adds a false “source organization was not persisted” notice. Distinguish a missing property from an explicit `null` before emitting the legacy warning.</comment>
<file context>
@@ -986,8 +1820,52 @@ export async function getWorkspaceMoveOperation(
+ * response, which is exactly when the operator most needs to see what the
+ * move did and where it came from.
+ */
+ const recordedSourceOrganizationId = operationPayload.audit.sourceOrganizationId ?? null
+ const sourceOrganization = recordedSourceOrganizationId
+ ? await getSourceOrganization(recordedSourceOrganizationId)
</file context>
| summary: await getMovedWorkspaceSummary(tx, params.workspaceId, destination, { | ||
| sourceOrganization: replayedSourceOrganization, | ||
| sourceOrganizationImpact: EMPTY_SOURCE_IMPACT, | ||
| credentials: EMPTY_CREDENTIAL_SUMMARY, |
There was a problem hiding this comment.
P2: After confirmation or reload, the operation response reports zero credentials even when the workspace has secrets, environment variables, or BYOK keys. Preserve or reconstruct the credential summary instead of returning EMPTY_CREDENTIAL_SUMMARY.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/workspaces/admin-move.ts, line 1057:
<comment>After confirmation or reload, the operation response reports zero credentials even when the workspace has secrets, environment variables, or BYOK keys. Preserve or reconstruct the credential summary instead of returning `EMPTY_CREDENTIAL_SUMMARY`.</comment>
<file context>
@@ -548,17 +1022,50 @@ export async function moveWorkspaceToOrganization(params: {
+ summary: await getMovedWorkspaceSummary(tx, params.workspaceId, destination, {
+ sourceOrganization: replayedSourceOrganization,
+ sourceOrganizationImpact: EMPTY_SOURCE_IMPACT,
+ credentials: EMPTY_CREDENTIAL_SUMMARY,
+ entitlements: {
+ sourceIsEnterprise: false,
</file context>
| const popoverPointerAtRef = useRef(0) | ||
| const timeZone = useTimezone() | ||
| /** Keep one wall-clock interpretation for the lifetime of this edit. */ | ||
| const editTimeZoneRef = useRef(initialTimeZone) |
There was a problem hiding this comment.
P2: When a regular date editor opens before general settings finish loading, this ref freezes the browser fallback instead of the user's saved timezone. Editing the time/date then writes a different offset and instant; keep date editing gated until the timezone is ready before capturing initialTimeZone.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx, line 114:
<comment>When a regular date editor opens before general settings finish loading, this ref freezes the browser fallback instead of the user's saved timezone. Editing the time/date then writes a different offset and instant; keep date editing gated until the timezone is ready before capturing `initialTimeZone`.</comment>
<file context>
@@ -68,9 +110,11 @@ function InlineDateEditor({
const popoverPointerAtRef = useRef(0)
- const timeZone = useTimezone()
+ /** Keep one wall-clock interpretation for the lifetime of this edit. */
+ const editTimeZoneRef = useRef(initialTimeZone)
+ const timeZone = editTimeZoneRef.current
</file context>
| } | ||
|
|
||
| const toolCost = sumToolCosts(response.toolResults) | ||
| const toolCost = sumToolCosts(response.toolResults) + failedFunctionToolCost.total |
There was a problem hiding this comment.
P2: When the provider omits token usage and precomputed cost, failed Function sandbox spend is silently lost. Initialize a zero model cost or attach tool cost independently before applying this aggregate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/providers/index.ts, line 314:
<comment>When the provider omits token usage and precomputed cost, failed Function sandbox spend is silently lost. Initialize a zero model cost or attach tool cost independently before applying this aggregate.</comment>
<file context>
@@ -300,7 +311,7 @@ export async function executeProviderRequest(
}
- const toolCost = sumToolCosts(response.toolResults)
+ const toolCost = sumToolCosts(response.toolResults) + failedFunctionToolCost.total
if (toolCost > 0 && response.cost) {
// Replaced rather than mutated: a provider-supplied cost can be the same
</file context>
| } | ||
|
|
||
| function epochSecondsToIso(value: unknown): string | null { | ||
| const seconds = typeof value === 'number' ? value : Number(value) |
There was a problem hiding this comment.
P2: When an unset or malformed TTL cell reaches the formatter or conversion hook, Number(value) turns it into epoch zero and can display or convert it as January 1, 1970. Reject null/undefined and empty non-numeric strings before numeric coercion, then preserve the blank through the callers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/table/column-types/ttl.ts, line 71:
<comment>When an unset or malformed TTL cell reaches the formatter or conversion hook, `Number(value)` turns it into epoch zero and can display or convert it as January 1, 1970. Reject `null`/`undefined` and empty non-numeric strings before numeric coercion, then preserve the blank through the callers.</comment>
<file context>
@@ -0,0 +1,122 @@
+}
+
+function epochSecondsToIso(value: unknown): string | null {
+ const seconds = typeof value === 'number' ? value : Number(value)
+ if (!isRepresentableEpochSeconds(seconds)) return null
+ return new Date(seconds * 1000).toISOString().replace('.000Z', 'Z')
</file context>
…7284) * improvement(integrations): refresh Slack integration page SEO copy * fix(integrations): correct Slack trigger name and tool categories in SEO copy
appVersion has been pinned at v0.7.44 since chart 1.2.0 while the app moved through forty-odd releases. It is the default tag for every first-party image (app, realtime, migrations, pii, copilot), so a helm install that does not pin image.tag deploys an application far behind the chart shipping with it — and any values key added by a newer chart is silently inert, because the running image has no code that reads it. The release tag is published by the main-branch merge commit that cuts it, so this lands on main together with v0.8.18. The kind install test also stops resolving its images through appVersion. That job installs the default configuration, so it pulled the tag appVersion names — which, on the very PR that raises appVersion, has not been published yet. The install would sit in ImagePullBackOff until --wait timed out. Pinning CI to the published :latest removes the circularity that kept appVersion frozen.
) A .chart document is author-controlled and rendered with setOption() straight into the app document, including on the anonymous /f/<token> share route. confineOptionToCanvas closed the innerHTML/document.write paths but left the navigation ones open: title.link, title.sublink, and a link on a treemap or sunburst data item each reach windowOpen, which assigns the URL to location.href, so a javascript: URL executed on the app origin on one click. Drop the link keys everywhere in the walk alongside toolbox. A chart has no reason to navigate its viewer, so they are stripped rather than scheme-checked, which would still leave an open redirect on an authenticated origin.
Two ceilings on a Function run's sandbox files were charged per source rather than per execution. Mounts: planUserFileMounts assigned a path per element, so one storage key named by two sources became two mounts. `files` is `user-or-llm` and deduped nowhere, so a model repeating an id — or naming a file the code also references with `<block.file.path>` — produced a duplicate that cost a presign, a second transfer of identical bytes, and a second charge against both the byte budget and the 20-file mount ceiling, either of which then refuses a request that fits. Collapse by storage key, first occurrence wins. The contract already requires a non-empty key, so there is no keyless case to carry. Exports: MAX_SANDBOX_OUTPUT_FILES is documented as what one execution may export "whether declared by path or discovered by harvesting", and collectExportedFiles already runs the byte ceiling that way. The count ceiling did not, so a request declaring paths and harvesting a directory could export 20 of each. Count declared and discovered together, with a declared path inside the directory dropped from the discovered set so it is not billed on both sides. With no declared paths — every call execute-request makes, since it sets outputSandboxDir only when nothing declares a sandboxPath — the check and its message are unchanged. The resolver's marker reuse is no longer what keeps a twice-referenced file to one mount; its comment said otherwise. Fixture keys in sandbox-mounts.test.ts were identical across files the tests meant to be distinct; they now differ, which is what those tests always claimed to set up. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.