Verified every field against xAI's live API with the staging hosted key:
context_length 500000, $2.00/$0.50/$6.00 per 1M, temperature capped at 2,
tool calling and max_completion_tokens both accepted.
Also feature the latest blog post.
* fix(v2): give every collection one pagination contract and close four envelope holes
A fractional `limit` reached Postgres as `LIMIT 2.5` and answered 500 on both
`GET /workflows` and `GET /audit-logs`: each list re-declared the param inline,
and these two copies lost their `.int()`. The same divergence left `limit`
validated five different ways and five collections emitting `nextCursor` while
accepting no `limit` at all, or accepting one and silently discarding it.
Adds `v2PaginationFields()` in `contracts/v2/shared.ts` — a bounded integer
`limit` and an opaque `cursor` — and adopts it across all 17 paged lists, so the
family cannot drift again. `/files`, `/logs` and `/tables` keep the truncate-and-
clamp leniency they published, now as an explicit named mode rather than three
hand-rolled copies.
Gives `/skills`, `/custom-tools`, `/secrets`, `/credentials` and `/knowledge`
real pagination using the existing cursor codecs: a keyset for the four whose
page comes from one ordered SQL read, and the offset cursor for `/skills`, whose
merge of the static builtin registry into DB rows cannot be expressed as a SQL
keyset. Each keyset sort now ends in a unique `id`; knowledge tie-broke on
`createdAt`, which cannot separate rows sharing a millisecond.
Two correctness fixes pagination forced: the secrets visibility filter moved from
a post-query JS pass into SQL, because trimming rows after the page is cut
returns fewer than `limit` while `nextCursor` claims more; and the skills list
stopped selecting the 50k-char `content` column only to discard it.
Also restores the canonical error envelope where it had holes: a malformed JSON
body returned a bare `{"error":string}` because the envelope was a per-route
opt-in only 8 of 77 routes remembered, and an unknown `/api/v2` path returned an
HTML 404. Both are now defaults — `V2_PARSE_DEFAULTS` on the builders and the two
raw routes, and a catch-all whose body is byte-identical to the rollout gate's so
an unknown path stays indistinguishable from an ungated one.
Consolidates the keyset paging block (`resumeKeyset`/`keysetPage` in
`list-query.ts`) that had been open-coded in six modules, and folds the bespoke
`InvalidWorkflowListCursorError` into the `OrchestrationError` every other list
already used.
Prevention: the contract sweep in `list-pagination.test.ts` now also asserts that
every paged list rejects a fractional `limit`, that every list query is
`.strict()`, and that the three clamping lists still truncate. The fractional-
limit assertion is what caught `/audit-logs`. Documented in
`.agents/skills/v2-api-conventions/SKILL.md`.
405 responses still carry no `Allow` header — Next.js generates those before any
handler runs. Recorded as a known gap.
* fix(v2): bind the offset cursor to the query state it counts positions in
An offset names a position in one exact sequence. `GET /skills` accepted a bare
`{offset}` cursor and applied it to whatever sequence the next request asked
for, so following `nextCursor` with a different `search`, `sortBy` or
`sortOrder` silently skipped rows, repeated them, or landed past the end and
returned an empty page while the cursor implied more.
Fixed in the codec rather than the route so the sibling could not keep the gap:
`decodeOffsetCursor` now takes a scope stamp and rejects a cursor minted under a
different one, which is what `decodeSortedCursor` has always done for keysets.
`offsetCursorScope()` builds the stamp from every param that filters or orders
the sequence; `limit` is excluded because it selects how much of the sequence to
return, not what the sequence is, so paging with a different page size still
works.
`GET /knowledge/{id}/documents` had the identical latent gap and gets the same
treatment — the compiler surfaced it as soon as the signature changed.
The workspace file preview adapter runs while the tool-call frame is still
on the wire, so the execution context it gets is turn-scoped and carries no
toolCallId — the file delegation requires one, so resolving a path target
threw on every call. The SSE handler swallows that throw and abandons the
rest of the event, so the frame never registered its arguments and the call
was later dispatched with an empty payload, failing schema validation.
- bind the frame's own tool call id before entering the file use cases
- resolve the preview target best effort, matching the preview base load
- stop a preview failure from dropping the tool-call frame in the stream loop
- drop the synthetic toolCallId the stream fixtures put on a turn context
A workflow invoked as an agent tool resolves `{{VAR}}` to the real decrypted
value in the child run, but the tool result handed back to the model vendor was
projected through a registry that had dropped those entries — so the plaintext
crossed to the vendor verbatim.
Mechanism. Model-facing projection runs `registry.forkForPropagatedEntries()`,
which keeps only entries a result explicitly carried. `EnvResolver` records a
resolution without `propagated`, unlike every other boundary that hands a value
onward. The child shares the caller's registry object, so `blockLogs`, trace
spans and error diagnostics still redact (they read the unforked registry) —
only the model fork loses them. The exposure is wider than the child's final
output: `mapChildOutputToParent` puts the full `childTraceSpans` — every child
block's inputs and outputs — into the returned object, and `postProcessToolOutput`
strips only `__`-prefixed keys.
The previous implementation ran the child over HTTP: the execute route emitted
`__resolvedSecretTraceProvenance`, the tool imported it as `propagated: true`,
and `transformResponse` curated the body so `childTraceSpans` never crossed. The
in-process branch returns before any of that.
Fix. `runWorkflowTool` exports committed provenance for the value it returns and
imports it back with `{ trusted: true }`, which marks those entries propagated —
the same crossing the custom-block branch already performs in `workflow-handler`.
Output-projection only: the returned result is unchanged, and the child executes
exactly as before. Redacted values render as `{{NAME}}`, matching the literal the
model saw before this regression.
Values shorter than `MIN_SUBSTITUTABLE_LITERAL_LENGTH` are still not redacted
anywhere — that floor governs detection as well as substitution, and is a
documented accepted cost. A test pins the behavior rather than leaving it silent.
Follow-up hardening to #6611, which began forwarding the invoking run's
environment variables into a workflow run as an agent tool.
A runtime audit of that change confirmed nothing today writes through
`ctx.environmentVariables`, so this is not a live defect. But `tools/index.ts`
was the only consumer handing the map across an execution boundary by
reference, and it hands it to the longest-lived consumer there is: the child
holds it for its entire run. `agent-handler`, `function-handler`,
`condition-handler` and `providers/utils` all copy via `normalizeStringRecord`
before handing the map anywhere. A future write through the child's reference
would corrupt the parent's env and every later sibling tool call in the same
agent turn — a cross-run bug with no local symptom.
A shallow spread is exact here: the value is typed `Record<string, string>`,
and the sub-Executor already re-copies it through `normalizeStringRecord`
(`executor.ts:73`), so the child receives a byte-identical map either way. The
spread also subsumes the previous `?? {}`, since spreading `undefined` yields
`{}`.
The test mutates the forwarded map and asserts the parent context is unchanged;
it fails without the spread.
* test(export): pin the table and tool-param loss the sanitized export accepts
#6591 enabled `redactOpaqueCredentialInputs` on the workflow export path,
closing a real leak. It also made export lossy for tables and unauthoritative
tool params, and nothing pinned that trade in either direction.
Adds round-trip fixtures (an api block with two table sub-blocks, an agent
block with a custom tool) plus assertions for the current loss, records the
security/usability trade on the flag that governs it, and deletes a duplicate
`sanitizeForExport` in credential-extractor that omitted the redaction flag
and had zero production importers.
No behavior change.
* test(export): make the env-ref leak sweep load-bearing and drop test any
The sweep asserted against a token the fixture no longer contained, so it
passed vacuously. Both the fixture and the assertion now read one symbol,
which is the only form that cannot drift.
Also types the re-imported block lookup instead of casting through any.
* fix(v2): derive the log and run status enums from the persisted status list
`GET /api/v2/logs` and `GET /api/v2/logs/{runId}` parse the raw
`workflow_execution_logs.status` column against a six-value enum that omits
`paused`, so a run holding that value returns 500. The list response is
validated whole-page, so one such row 500s every page it lands on, and the
row is durable until the run is resumed, cancelled, or failed.
`paused` is not written by an ordinary human-in-the-loop pause — that path
persists `pending` (logging-session.ts:1180). It is written by
`PauseResumeManager.markResumeAttemptFailed`, which fires on any
`ResumeAdmissionError`: a workspace over its usage limit, an archived or
undeployed workflow, or a concurrent resume losing the claim race. That is a
routine business path.
The enum was supposed to be protected by an `AssertNever` exhaustiveness gate,
but the gate was vacuous: it compared against `PersistedWorkflowExecutionStatus`,
a hand-written union that was itself missing `paused`, because the write goes
through a raw `sql` CASE fragment Drizzle cannot type-check. Adding `paused` to
both lists would leave the same vacuous gate in place for the next status.
Instead, `PERSISTED_WORKFLOW_EXECUTION_STATUSES` becomes the single runtime
source of truth, `PersistedWorkflowExecutionStatus` is derived from it, and both
v2 contracts derive their enums from the const rather than re-declaring them.
Both surfaces pass the column through verbatim, so their reported set is the
persisted set by definition — there is no editorial choice for a gate to force,
only the question of whether a newly persisted status should be public, which
the option-list tests now pin. The `[...V2_PERSISTED_RUN_STATUSES, 'paused']`
append on the runs contract is deleted rather than adjusted; it would otherwise
be a duplicate.
Alternatives rejected:
- A `.catch()` or `safeParse` in the presenters is dead code:
`v2-json-route.ts:271` re-parses the whole body with the same schema.
- Normalizing `markResumeAttemptFailed` to write `pending` would remove the
distinction the resume claim query at human-in-the-loop-manager.ts:973 relies
on, and leaves the contract wrong for any other future status.
- Typing the Drizzle column does not help: the offending write is a raw `sql`
fragment, and `packages/db` cannot import the app's status list.
The v2 workflows spec changes are reordering and description only — the value
set there already contained `paused`. The v2 logs spec gains `paused`, which is
additive and safe while the whole `/api/v2` surface is behind the off-by-default
`v2-api` flag; it must land before v2 GA, after which it would be breaking.
* fix(v2): document both provenances of a reported paused run status
* fix(v2): stop promising a paused discriminator the response cannot always provide
* fix(v2): describe the paused discriminator as the code actually records it
Two docs-only defects introduced by #5273 (`263e3ca67e`), which re-founded the
public API reference on the v2 surface.
1. Ten translated SDK snippets produce a deterministic 400.
The streaming example in the five translated `api-reference/typescript.mdx` and
`python.mdx` pages was repointed from `/api/workflows/{id}/execute` to
`/api/v2/workflows/{id}/execute` and nothing else was changed — fr/ja/zh
typescript.mdx are literally one-line diffs. `message` stayed at the body root.
That was correct against v1, whose route treats the whole non-control body as
workflow input, but `v2ExecuteWorkflowBodySchema` ends in `.strict()` and the
route parses before executing, so every copied snippet returns
`400 Unrecognized key: "message"`. The same commit fixed the English bodies to
`input: { … }`, so this is an oversight, not a decision. The ten fences now
match `en/api-reference/typescript.mdx:959` and `python.mdx:681`.
Not relaxing `.strict()`: it is deliberate house style across the v2 contract
and is what makes a typo'd option fail loudly instead of silently.
2. Thirty-two published operation pages 404 with no redirect.
Replacing the single v1 `openapi.json` with seven v2-only specs changes page
identity, because fumadocs derives every generated page as
`slugify(tag)/operationId` from the specs at build time. Re-deriving both sets
gives 52 old slugs and 128 new ones: 32 disappear and 20 keep their URL while
silently retargeting v1 -> v2 (`knowledge-bases/updateKnowledgeBase` also flips
PUT -> PATCH). All 52 are in the live sitemap — parsing `<loc>` from
docs.sim.ai/sitemap.xml gives 458 URLs of which 56 are `/api-reference/`: the
four static pages plus all 52 generated ones by name, including every one of
the 32 that die. They are 200 today under an allow-all robots.txt.
The spec swap itself is deliberate and CI-enforced (`check-openapi-specs.ts`
requires every published operation under `/api/v2/`), so restoring the v1
operations is not an option. The missing piece is the redirect map, in a file
that already carried 56 such rules from earlier doc moves.
`permanent: true` (308) is used only for a true 1:1 successor — same operation,
renamed. A 308 is cached indefinitely and effectively unrecallable, so anything
that collapses two pages onto one, changes the identifier model, or lands on a
merely adjacent operation is `permanent: false` (307). That splits 21/11.
Four destinations differ from the mapping proposed in review, each on evidence
from the specs rather than from the operation names:
- `workflows/getJobStatus` is not destination-less. The v2 queued-execution
receipt (`QueuedWorkflowRun`) returns `statusUrl`
`/api/v2/workflows/{id}/runs/{runId}`, so `workflow-runs/getWorkflowRunV2` is
the successor poll target — far better than a generic landing page.
- The three HITL read operations go to `getWorkflowRunV2`, not to the resume
page: `WorkflowRunStatus` carries a `paused` object with `contextId`,
`pausedAt`, and `pauseKind`. Pointing a GET doc at a POST doc would be wrong.
- `human-in-the-loop/listPausedExecutions` goes to `listWorkflowRunsV2`, whose
`status` filter includes `paused`.
- `tables/batchUpdateRows` is 307, not 308. v2 `updateTableRows` is "Update Rows
by Filter" — the successor of v1 `updateRows` (PUT, predicate-based), which
keeps its 308. v2 has no by-id batch update at all, so batchUpdateRows lands
on a genuinely different operation.
3. A guard, so the map cannot rot silently.
`scripts/openapi/docs-redirects.test.ts` recomputes the generated slug set the
way fumadocs does and asserts no `/api-reference/` source shadows a live page
and every destination resolves. Nothing else in the repo reads docs URLs, so a
future spec regeneration would otherwise break the map with no signal. It needs
no wiring: `check-openapi.ts` already runs this vitest config.
The redirect array moves to `apps/docs/lib/redirects.ts` because the guard
cannot import `next.config.ts` — `createMDX()` runs the fumadocs-mdx generator
at import time, which made vitest emit an unhandled build error and warn about
false positives. The 56 pre-existing rules are byte-identical to before,
verified programmatically; `next.config.ts` keeps the same public shape and
Next's own `checkCustomRoutes` accepts all 88 rules.
Open question for the owner, larger than the redirects: all 82 `/api/v1`
route files survive on staging, so a live public API now ships with zero
reference docs, while the documented `/api/v2` surface returns 404 for any
caller outside the off-by-default `v2-api` flag cohort. Is that the intended
end state or transitional?
The v2 SDK migration (#5273, #6564) shipped five breaking changes in both
SDKs but got the release mechanics wrong in three separate ways, and left
one of the two rewrites unable to complete a single successful call.
Versions. packages/ts-sdk/package.json read 0.1.3 -- a patch digit added
inside an unrelated compatibility commit, never deliberated. npm expands
^0.1.2 to >=0.1.2 <0.2.0, so every existing consumer would have picked the
break up on a lockfile refresh: AsyncExecutionResult.jobId renamed to
runId, executionId dropped from that interface, a failed sync run now
throwing instead of resolving {success:false}, the request body reshaped,
and the endpoint moved to /api/v2 with no fallback. 0.2.0 excludes every
existing range, so the upgrade becomes opt-in. packages/python-sdk carries
the identical break and was never bumped at all, so its publish job would
have skipped green at the "version already exists" gate and left the repo
and PyPI silently divergent; it moves 0.1.2 -> 0.2.0 in lockstep, along
with the __version__ string in simstudio/__init__.py, which tracks
pyproject and would otherwise have started lying. setup.py is left at
0.1.1: it is unchanged from main and demonstrably unread (0.1.2 published
from pyproject while setup.py already said 0.1.1). It wants deleting, in
its own commit.
A 404 fallback was considered and rejected. The legacy 202 body's statusUrl
points at /api/jobs/{jobId}, so mapping jobId onto runId would hand the
caller an id that getWorkflowRun cannot resolve against that same old
server -- a successful execute followed by an inexplicable failure on the
next call is a worse contract than a clean 404. Both READMEs instead state
the minimum server version and name the endpoint to check for.
Cancelled runs. packages/python-sdk computed success as status != 'failed',
so a run cancelled out of band reported success=True. The TypeScript SDK
uses a closed whitelist and reports False, and before the migration both
SDKs read the server's own value, which was False -- so this was a Python
regression, not merely an inconsistency. Fixed by mirroring the whitelist.
The v2 contract enumerates exactly completed|failed|paused|cancelled, so
narrowing the blacklist to a whitelist cannot drop a live value, and a
status added later now defaults to "not successful" rather than silently
reporting True. WorkflowExecutionResult gains a status field because
Python, unlike TypeScript, does not throw on 'failed' -- so success=False
alone is ambiguous there in a way it is not in the TypeScript SDK, which
is why status is not added to both.
Rate-limit header. Found while auditing the two SDKs for further
divergence, and the reason the Python bump could not have shipped as it
stood: every authenticated v2 response now carries X-RateLimit-Reset as an
ISO 8601 timestamp (recorded by v2RateLimits.publicApi, stamped by
withRouteHandler). The Python SDK parsed it with int(), raising a bare
ValueError that no handler in execute_workflow catches -- so every
successful v2 execution raised instead of returning. None of the legacy
endpoints the SDK previously called record a rate-limit snapshot, which is
why the latent int() survived until the v2 move. The TypeScript SDK
already branches on the format; _parse_reset_header mirrors it, including
degrading an unrecognised value to 0, because a quota hint must not take
down the call it rode in on.
Timing metadata. The v2 rewrite stopped forwarding startedAt/endedAt, which
main passed through and the TypeScript SDK still reports; restored under
the same startTime/endTime keys the TypeScript SDK uses.
Tests: cancelled/failed/paused status coverage, the ISO reset header, and
the restored metadata keys, each verified red against the unfixed line
first. The TypeScript suite gains matching cancelled/paused and ISO-reset
pins -- they pass against today's source by design, and were confirmed to
fail against a deliberately degraded copy so they are not toothless.
Deliberately not included: a CI guard failing a PR that changes SDK source
without a version bump. It would have caught this twice over, but it is a
new script and workflow rather than a fix to the defect at hand.
Review revision. bun.lock recorded packages/ts-sdk at 0.1.3 and was left
stale by the first pass, so the repo asserted two versions for the same
workspace package -- in a change whose whole thesis is that the version
strings had diverged. It does not break CI (bun 1.3.14 accepts the
mismatch under --frozen-lockfile, confirmed here), but 092311ea68 bumped
the lock in lockstep with package.json, and the next unfrozen install
would otherwise drop the line into an unrelated PR.
_parse_reset_header gated the numeric branch on str.isdigit(), which
accepts characters int() rejects ('²'.isdigit() is True, int('²')
raises) -- and that int() sits outside the try, so the one function added
to stop a quota hint raising could still raise, contradicting its own
docstring. str.isdecimal() is exactly the set int() accepts. The
tolerates-unparseable test is parametrized over both forms and was
confirmed red on '²' against isdigit.
Docs and docstrings: apps/docs api-reference/python.mdx mirrors the
README's dataclass block and was the only copy left without the new
status field. RateLimitInfo now names its units, because reset is epoch
seconds for the legacy integer and milliseconds for the ISO form that v2
sends. execute_workflow's Args entry still described the pre-v2 body
shape ("spread at root level"); every input is nested under input now,
and this is the commit that ships that help() text to PyPI. The
"declared last so positional construction keeps working" sentence was a
maintainer's note that belongs in this message, not in every user's
help(WorkflowExecutionResult).
`materialize_file(operation: 'save')` wrote the HEADed object size straight into
`workspace_files.size`, which is still `integer NOT NULL`. Since the `size_bytes`
widening (0289), a mothership chat attachment may be up to MAX_WORKSPACE_FILE_SIZE
(5 GiB): `upload-session/service.ts` gives `mothership_attachment` that ceiling, and
`finalizers.ts` already dual-writes the row as `size = 2147483647,
size_bytes = <exact>`. Saving such an upload then re-read the true size from
`headObject` and issued `SET size = 3221225472` against int4. Postgres raises 22003;
the retry filter matches only 23505, so it rethrows, the transaction rolls back and
the tool returns `success: false` with no way for the user to complete the save. No
corruption — int4 overflow errors, it never truncates — but the file can never be
saved.
Every other `workspace_files` size writer already pairs
`toLegacyWorkspaceFileSize(bytes)` with `sizeBytes: bytes` (metadata.ts x4,
workspace-file-manager.ts:243/1706, finalizers.ts:367). This call site was simply
missed when the widening landed; the fix converges it with the other six rather than
inventing a third shape. Storage accounting keeps using the exact `verifiedSize`, so
quota and usage are unaffected.
The size source itself also had to widen. `head?.size ?? row.size` fell back to the
clamped int4 column, and since this change now writes `sizeBytes` too, that fallback
would overwrite an exact `size_bytes` with the clamp — the object is gone, so nothing
could recover it, and the row would look internally consistent afterwards. The
fallback is live whenever `hasCloudStorage()` is false, since the early return at the
HEAD miss is cloud-only. Reading `row.sizeBytes ?? row.size` is the same coalescing
shape the readers already use (workspace-file-manager.ts:227, finalizers.ts:399,
metadata.ts:46), and the row comes from a full `select()` so the column is present.
The clamp is derived once next to `verifiedSize` rather than inline in the update
because the value is loop-invariant.
Two sibling writers were examined and deliberately left alone. `copy-files.ts` reads
`task.size` out of the int4 column itself, so it is arithmetically incapable of
overflow, and its missing `sizeBytes` is unreachable behind the 100 MB fork download
cap. `workspace-file-manager.ts:963` takes a caller-supplied size, but its insert
branch needs an orphaned storage object with no `workspace_files` row, and converting
loose external input from a DB error into a JS throw deserves its own review rather
than a release patch; it is the next instance of this bug and should be filed as a
follow-up.
Both new tests were proven red against the unfixed code: the clamp test fails with
"expected 3221225472 to be 2147483647", the fallback test with
"expected undefined to be 3221225472".
A workflow attached as an Agent (or Pi) tool ran its entire child execution
with an empty environment-variable map and no block-output redaction policy.
Mechanism. `tools/index.ts` short-circuits `workflow_executor` into
`runWorkflowTool`, which builds its synthetic parent `ExecutionContext` with
`buildCustomBlockExecutionContext`. That builder was written for the
custom-block (deploy-as-block) path and hardcoded `environmentVariables: {}` —
safe there only because `WorkflowBlockHandler.executeCore` re-derives the
publisher's env inside `if (isCustomBlock)`. The workflow-tool path's synthetic
block carries `metadata.id: 'workflow_input'`, so `isCustomBlock` is false, the
re-derivation is skipped, and `{}` flows through `childEnvVarValues` into the
sub-Executor. `DAGExecutor` has no fallback and `EnvResolver` returns the raw
reference on a miss, so a child block field of `Bearer {{MY_API_KEY}}` was
transmitted to the third party verbatim and 401'd — silently, with the variable
name disclosed. The same builder never set `piiBlockOutputRedaction`, so
`block-executor`'s in-flight masking was disabled for every child block of orgs
that had explicitly enabled that stage. Both landed as unnoticed side effects of
#5273, whose stated goals were admission slots, log rows, cost roll-up and
structured errors; #6539 later patched a third dropped field on the same context
without noticing these two.
Fix. Thread both values through the runner `options` bag — never `params._context`,
which spreads model-reachable `contextParams._context` first and would let a model
inject its own env map or disable redaction. `executeTool` reads them off the
trusted `executionContext`, which also covers the Pi block, whose tool loop calls
`executeTool` with `executionContext: ctx` on the identical path.
`environmentVariables` is required rather than optional-with-a-default. Silent
omission is precisely the failure mode here and in #6539; making it required turns
the next caller's omission into a compile error. `runCustomBlockTool` now passes
`{}` explicitly, so that path is unchanged at runtime. `piiBlockOutputRedaction`
stays optional deliberately: `undefined` is its correct value for the many tenants
with no policy, whereas `{}` for env is a wrong identity rather than a default.
The builder's TSDoc states both halves of that asymmetry.
Identity semantics — this restores function but does not restore main's identity.
On main this tool was an HTTP hop into execution-core, which derived the env from
the CHILD workflow's owner, so the child got the child owner's personal env plus
the child workspace's env. Forwarding the caller's map gives the child the PARENT
CALLER's personal env: a different identity, not a subset. That is the deliberate
choice, because it is byte-identical to the long-standing canvas workflow block,
it is bounded to one workspace by `assertChildWorkflowInWorkspace` on this branch,
and it is the only variant consistent with the parent `resolvedSecretTraceRegistry`
this path already forwards. The narrow case that worked on main and still will not:
a same-workspace child owned by another member that relied on THAT member's
personal environment variable.
The `deployed_block_executor` call site deliberately gets neither value: custom
blocks skip the same-workspace assert and run cross-workspace under the
publisher's identity, so the consumer's env and redaction rules are the wrong
tenant's. A test pins that so a later refactor cannot unify the branches silently.
Tests. Three suites pin the fix itself (runner, builder, `executeTool` dispatch)
and go red without it. A fourth case in `workflow-handler.test.ts` pins the last
hop — `ctx.environmentVariables` -> `childEnvVarValues` -> the sub-Executor's
`envVarValues`, plus `piiBlockOutputRedaction` — on the NON-custom branch. That
hop is untouched staging code, so that case passes either way by construction; it
exists so a future change to the branch that distinguishes the two paths cannot
silently undo this fix downstream of the builder.
Out of scope, deliberately: `enforceCredentialAccess` is dropped by the same
synthetic context, but on main this path ran under an internal JWT with
`useAuthenticatedUserAsActor === false`, so forwarding the parent's value would
TIGHTEN behavior versus main and could break currently-working child runs
mid-release. It needs its own deliberate change — and it now compounds with this
one, since the child runs with the parent's decrypted env while credential-access
enforcement stays off. The `input` redaction stage (masking the LLM-authored
inputMapping) is also not restored — `ExecutionContext` has no field for it and
the canvas workflow block never had it either.
Re-enabling masking inside child runs is a live behavior change for affected
tenants: `redactObjectStrings` runs with `onFailure: 'throw'`, so a child agent
tool call that currently succeeds unmasked can now fail closed, which is main's
semantic restored. This belongs in the release note.
`workflow_blocks.retry` is a jsonb column written verbatim. Three of its
writers never validate what they store: the realtime batch-add and
replace-state ops take untyped block records, and the admin/superuser
import routes persist externally-authored workflow JSON. `load.ts` then
asserted the blob was already a `BlockRetryConfig` and handed it straight
to the HTTP boundary, where `workflowBlockStateSchema` bounds `maxTries`
to 2..5 and `waitBetweenTriesMs` to 0..5000.
That schema is shared between the PUT `/state` body, where the bound is
right, and the GET `/api/workflows/[id]` and `/state` responses, where it
is fatal. The response `.parse` in the shared route builder throws a
ZodError, which is not an `OrchestrationError`, so the error policy
declines it and it falls through to a 500. One out-of-range or partial
stored value therefore made a workflow permanently unopenable, with no
in-product repair — every UI write path reads the workflow first.
The feature already declares clamp-on-read as its contract: the commit
that added it says bounds are clamped on read rather than rejected, the
TSDoc on `resolveBlockRetryConfig` says the same, and `block-retry.test.ts`
asserts it. Execution has always honoured that. Only the read boundary
disagreed, so that is what this fixes: the loader now constructs a real
`BlockRetryConfig` from the blob through `normalizeBlockRetryTries` /
`normalizeBlockRetryWaitMs`, filling defaults for missing fields and
carrying `enabled` across unchanged.
`loadWorkflowFromNormalizedTablesRaw` is the single read choke point for
both apps — `@sim/workflow-persistence` for the Next app and the realtime
server's full-state emit — so one edit repairs every reader, including
rows that are already out of range, and the row self-heals on the next
save. It matches `clampParallelBatchSize` a few lines below, which already
pins a stored subflow value on the same path.
Alternatives rejected:
- Validating on write. It leaves every existing bad row fatal forever, and
it would have to be repeated across three realtime ops plus roughly a
dozen `saveWorkflowToNormalizedTables` callers, none of which share a
validation seam.
- Bounding `BlockRetrySchema` in `@sim/realtime-protocol`. Its own TSDoc is
correct that batch-add and replace-state bypass it, so this closes one
writer and leaves the 500.
- Relaxing the response contract. It stops the 500 but leaves the editor
rendering a number execution will never run. A test now pins the write
bound so that shortcut fails loudly.
- `resolveBlockRetryConfig`. It returns null for a disabled policy, which
would erase the numbers a builder configured every time state is read.
Tests: six cases in `packages/workflow-persistence/src/load.test.ts` (four
red before this change) covering out-of-range enabled, out-of-range
disabled with `enabled` preserved, missing fields, a non-boolean flag, an
untouched in-range policy, and NULL meaning "runs once"; plus a contract
test that the write bound still rejects out-of-range input.
`projectWorkflowToolOutput` required the resolved-secret provenance scope to
carry the ACTING caller's userId, but the executor stamps that scope with the
workflow AUTHOR: on the MCP bridge `execute-service.ts` sets
`isClientSession: false` and `workflowUserId: workflow.userId`, so
`execution-core.ts` resolves `personalEnvUserId` to the author, while the route
builds its scope from `actorUserId`. Author and actor differ in the ordinary
team configuration -- both attach and serve authorize on workspace membership
only, and the workflow row the route selects does not even include `userId`.
The refusal fired only after `executeWorkflowService` had returned, so every
affected `tools/call` ran the workflow, wrote its log row, consumed an admission
slot and resolved billing attribution, and then answered HTTP 500 /
JSON-RPC -32603 'Tool execution failed'. MCP clients retry 500s, re-charging
each time. On a public server the actor is pinned to `server.createdBy`, so a
server whose creator is not the workflow author was permanently broken for every
caller -- including anonymous ones and the creator -- with no recoverable
setting. Provenance always carries a scope even for a secret-free workflow, so
the failure did not depend on the workflow using secrets at all.
The check was collateral of the #5273 rewrite that moved this bridge in-process:
main's `projectWorkflowMcpModelContent` had no scope precondition, the sibling
Copilot bridge added by the same commit has none, and the registry this route
calls documents cross-scope provenance as an ANONYMIZATION signal, never a
refusal.
Comparing the tenant only is the minimal correct fix. Two alternatives were
rejected. Forcing `anonymous: true` on the import is unnecessary and harmful:
the registry's own `scopesMatch` already compares both fields, so a user-only
difference anonymizes every entry and yields the opaque placeholder, whereas
forcing it unconditionally would also strip named redaction from the author's
own calls. Comparing against the workflow author instead would mean re-deriving
the executor's `isClientSession ? sessionUserId : workflowUserId` rule inside a
route -- the same duplication that produced this bug.
Follow-up, not fixed here: this bridge imports the whole provenance bundle where
main used the value-filtered `importCrossingProvenance`, so a very large env
bundle can still hit `MAX_MATCHER_NODES` and produce the same billed 500.
Tests pin cross-author success on both the public and the private API-key
branches, that a cross-author secret redacts to `[REDACTED_SECRET]` while the
author's own call keeps `{{OWNER_TOKEN}}`, and that a workspace mismatch still
refuses. Each was verified to fail against the unfixed route or against the
rejected alternatives.
* improvement(integrations): add Managed Agents templates and guard docs links
The Claude Managed Agents block shipped without templates or suggested
skills, so its integration detail page had nothing to offer and the
"Add to Sim" chat handoff was the only affordance. Add nine templates
and seven skills, each grounded in an operation the block actually
exposes.
Its docsLink also pointed at integrations/managed-agent while the page
is managed_agent, so the link 404'd. Five more blocks had the same
class of bug via a stale tools/ prefix. Nothing validated these,
because the catalog check only compares deployment fields.
Add that validation, and collapse the three copies of the docs-URL
contract onto one exported helper so the checker and the generator
cannot drift apart.
* fix(integrations): resolve docsLink once when checking the vendor allowlist
The stale-allowlist predicate read block.docsLink directly while the main
loop read the resolved link, so an allowlisted block that dropped its
explicit docsLink produced undefined from the optional chain, negated to
true, and was treated as still vendor-linked — the stale entry went
undetected.
Record vendor-linked types during the single pass that already resolves
each link, so both checks agree by construction.
* fix(managed-agents): name every integration a template's alsoIntegrations claims
alsoIntegrations is documented as the blocks a template's prompt
references, and it drives which catalog pages the template cross-lists
on plus the icon cluster on the detail page. Three prompts named a
service only implicitly, or not at all: the runaway stopper claimed
Slack without mentioning it, the PR reviewer said "pull request" rather
than GitHub, and the weekly report said "emails" rather than Gmail.
Name the service in each prompt so the field is accurate and the
templates surface on the right pages.
* fix(integrations): point every visible integration at Sim's own docs
Six blocks had a vendor documentation URL in docsLink — cursor, enrich,
enrow, google_groups, qdrant, and similarweb — which was accidental
rather than deliberate. Each already has a generated Sim page, and each
already carries the vendor's homepage on BlockMeta.url, so the vendor
link in docsLink only sent readers away from our own documentation.
Point all six at their Sim page and drop the allowlist that had been
tolerating them. Every visible integration gets a generated page, so a
docsLink outside docs.sim.ai is now always an error.
getBlobPresignedUploadUrl signed its SAS with BlobSASPermissions.parse('w').
Per Azure's service-SAS reference, `w` is "create or write content" and permits
overwriting an existing blob; `c` is "write a new blob" and does not. main
signed `c` before this signer moved out of core/storage-service.ts, so Azure
deployments lost create-only enforcement in the move.
The `If-None-Match: '*'` the signer returns cannot carry the guarantee on its
own: an Azure service-SAS string-to-sign covers the resource, times,
permissions and the five rsc* response-header overrides, never request headers,
so a client is free to drop it. The header is signed on the other two providers
-- inside the PutObjectCommand on S3, and as x-goog-if-generation-match in
signed extensionHeaders on GCS -- which is why only Azure regressed.
Without this, a signed upload URL stayed a plain overwrite grant on the final
key for its full hour. A caller could replace the object after complete had
already verified size and content type, written the workspace file row and
metered storage from that verified HEAD, leaving durable metadata and billing
describing content that no longer exists.
The multipart block-staging signer keeps `w`: block staging is overwrite-shaped
and matches main.
The existing test asserted parse('w'), so it locked the defect in; it now
asserts create-only, and its name states the guarantee so a future flip reads
as deleting a security property rather than adjusting a value.
* fix(mcp): audit an upsert that rewrites or revives a server
Registering a URL that already exists takes the upsert branch and rewrites the
live row — name, transport, headers, timeout, enabled, auth type, the
connection reset, and the URL's query string, since the server id hashes only
origin and pathname. That branch recorded no audit row at all: the ADDED audit
was gated on `!result.updated`. main recorded ADDED for these (wrong action,
but a row existed), so this restores coverage and fixes the action.
Reachable from the settings POST /api/mcp/servers and from Copilot's
manage_mcp_tool `add`, neither of which passes existingServerBehavior. The v2
POST passes 'reject' so it only reaches the upsert on a revival.
A rewrite is now MCP_SERVER_UPDATED carrying updatedFields; a revival of a
soft-deleted row stays MCP_SERVER_ADDED. updateValues is typed
Partial<$inferInsert> so Object.keys is column-safe. Analytics gating is
unchanged: mcp_server_connected still fires only for a genuine insert.
* fix(mcp): redact audit URLs and drop unwritten columns from updatedFields
Two review findings on the new upsert audit.
The upsert assigns every column unconditionally, so `description` is present
on updateValues but undefined when the registration omits it. Drizzle skips
undefined in .set(), so deriving keys without checking values made the audit
claim a column the write never touched. Filter by value; null stays, since
clearing a value is a write.
MCP URLs carry tokens in their query string — that is why a silent rewrite of
one matters — and audit rows are readable by org admins who need no workspace
MCP access. Newly auditing rewrites would persist those tokens verbatim, so
every MCP audit row now records the URL through sanitizeUrlForLog, which
strips query and fragment. Applied to the add, update and delete rows alike:
redacting only the new path would leave the same credential in the row a
first registration already writes. A null url stays null rather than becoming
an empty string.
* refactor(audit): derive updatedFields through one shared helper
Six copies of Object.keys(updateData).filter(k => k !== 'updatedAt') across
four files decided, independently, which columns an audit row reports. The
exclusion set is an audit convention, not a local detail, so it moves to
@sim/audit as auditUpdatedFields and the exclusion becomes a single edit.
The admin organizations route evaluated the expression twice in one handler
and filed it under the metadata key `fields` while every other site uses
`updatedFields`, so any consumer filtering on updatedFields silently missed
org updates. It now computes once and uses the shared key; nothing reads
metadata.fields.
auditMock carries the real implementation rather than a stub, since callers
under test derive their audit metadata through it. The two suites that
hand-roll an @sim/audit factory source it from there.
* test(audit): pin the testing mock's copy of auditUpdatedFields
@sim/audit devDepends on @sim/testing, so the mock cannot import the real
helper without closing a package cycle. Assert parity from the audit side
instead, where the dependency already runs the safe direction, so a change to
the exclusion convention cannot leave mocked callers validating behavior the
deployed helper no longer has.
* test(table): pin the executor auth pairing on the table read route
fetchTableSchema reaches GET /api/table/[tableId] with a legacy type:'internal'
token, which only works while that route authenticates through
checkSessionOrInternalAuth. Its sibling table routes already moved to the
delegation policy, which rejects that token outright, so migrating this one
without moving the caller in the same change would break every table tool on an
Agent block.
Assert the route still authenticates through the legacy path so that migration
fails here first, and record on the caller why it is deliberately not on
buildExecutorDelegationHeaders yet.
* test(table): assert the Bearer header reaches the legacy verifier
Address review: the pairing test sent no Authorization header and its name
claimed to verify token acceptance, which is pinned separately in
lib/auth/internal.test.ts. Send a representative header, assert it reaches
checkSessionOrInternalAuth unmodified, and scope the name and docs to what this
guard actually covers — the route's choice of verifier.
finalizeExecutionAttachment reported a completedFileId. That marker is what
routes a replayed completion into loadCompletedUploadPurpose, which handled
only workspace_file and threw a bare Error otherwise -- unclassified, so the
route rendered a generic 500. Its structurally identical twin,
finalizeMothershipAttachment, correctly reports nothing.
Both are metadata-backed and idempotent by storage key, exactly as the
finalizeUploadPurpose TSDoc already states, so neither needs the marker: their
replays are correct through the finalizer itself. Drop it from the execution
finalizer so the two twins agree.
loadCompletedUploadPurpose becomes an exhaustive switch, matching the sibling
finalizeUploadPurpose switch, so adding a purpose is a compile error until its
replay behavior is decided rather than a runtime 500. The residual arm throws a
classified UploadSessionError('internal') instead of a bare Error.
markUploadSessionCompleted no longer clears a marker it was not given. A
finalizer that records one inside its own registration transaction --
markUploadSessionFileRegistered does this for workspace_file -- would otherwise
have it overwritten with null, and both the abort guard and the expiry sweep
key on it: cleanupExpiredUploadSessions only treats a finalizing session as
disposable when completedFileId is null. This is a no-op for every current
path, since markUploadSessionCompleted moves the session to completed, which is
neither abortable nor a cleanup candidate.
Latent only. No shipped client replays a completion: the sole producer,
uploadWorkflowAttachments, mints a fresh session per file and never retries,
requestJson does not retry, and a concurrent double-submit is already a clean
409 from claimSession.
Tests pin the invariant rather than the symptom: a Record over the purpose union
is a compile-time gate on which route each purpose replays through, and the
cases assert that idempotent purposes report no marker and reject cleanly if
they ever reach the loader.
The list use case defaulted to a name sort whose defaults were written for the
new v2 contract, so the internal route — whose contract exposes no sort params —
could no longer reach the repository's sortOrder ASC, createdAt ASC ordering.
Surfaces that render the payload positionally (the @-mention Folders group, the
add-resource search results, Copilot's list_file_folders) silently flipped from
newest-first to alphabetical, and the Files browser's SSR prefetch hydrated a
different order than its own refetch.
Push the sort down to the query like the workflow, knowledge, and table folder
lists already do, reusing FOLDER_SORTS. Omitting sortBy keeps the position
ordering; v2 always sends one from its contract defaults. A name sort now also
uses the database collation and the shared createdAt tiebreak instead of a JS
comparator over UTF-16 code units.
Every PATCH /api/mcp/servers/[id] audit row listed oauthClientId,
oauthClientIdProvided and oauthClientSecretProvided — on edits that never
touched credentials — while omitting the connectionStatus/lastConnected/
lastError resets the write actually performed. The route always sends
`oauthClientId: body.oauthClientId || null` and `*Provided: ... !== undefined`,
and null and false both survive a `value !== undefined` filter. The two
*Provided flags are control params, not columns at all.
Only the writer knows which columns a write touched, so updateMcpServer now
returns updatedFields from its updateData and both the internal audit wrapper
and the v2 use case record it. This matches workflow-mcp-lifecycle and
credentials/orchestration, which already report written columns this way.
* fix(tools): bind schema-enrichment reads to executor delegations
Two schema-enrichment callers still sent the deprecated legacy internal JWT
to routes that moved onto delegation-only auth, so both 401'd and swallowed
the failure:
- tools/params.ts fetched a child workflow's input fields with an unsubjected
buildAuthHeaders(), leaving the Agent block's workflow_executor inputMapping
untyped so the model guessed the child's field names.
- tools/schema-enrichers.ts fetched KB tag definitions the same way, which
dropped the tags/tagFilters parameter from the knowledge tools entirely.
Extract the executionId binding rule into executionScopeForTarget so the three
enrichment call sites share one definition.
* improvement(tools): surface tag-definition read failures at error level
Enrichment degrades silently by design, so the log line is the only signal a
credential break leaves. Drain the body on the failure path too.
The internal cancel route mints four outcomes the cancellation service never
produces — queue_cancelled, already_cancelled, active_resume_signal_failed and
cancellation_not_finalized — but the contract enumerated only the five service
reasons. requestJson validates every 2xx against that contract, so a stop that
genuinely applied threw on the client.
Keep the service enum narrow for the public v2 contract, which delegates wholly
to the service and cannot emit the other four, and validate the internal route
against a superset. Route the seven success bodies through one contract-typed
constructor so drift is a compile error, and delete the dead duplicate cancel
contract left behind in contracts/logs.ts.
The v2 routes rewrite DelegatedWorkspaceAuthorizationError,
NoWorkspaceAccessError, and WorkspaceApiKeyScopeAuthorizationError to a
404 so a caller with no reach into a workspace cannot confirm a
resource exists. The internal routes reach the same application use
cases and still answered 403, so the same probe worked from the other
surface. Same-workspace role denials stay 403 on both.
* fix(tables): stop a column retype from nulling empty-string cells
A type conversion rewrote every cell holding '' to null. Main only nulled a
blank the target type could not read; '' is a real stored value that both
string and json columns accept, so string->json and json->string silently
destroyed those cells.
Worse on a required target: countEmptyCells matches only a missing key, SQL
NULL, or '[]', so '' passes the required guard and the rewrite then wrote null
behind a constraint that had just succeeded.
The per-cell decision is now the pure retypeCellRewrite, restoring main's rule:
null a blank only when the target cannot read it, otherwise coerce.
* fix(execution): release the concurrency slot when a group cancel is refused
The stop-the-work effects (durable Redis abort record, queue-job cancel,
in-process abort) all fire before the workflow-group sidecar is consulted, and
none can be undone. When the sidecar refuses the claim we throw a conflict,
which skipped releaseExecutionSlot and stranded the plan concurrency
reservation until it expired.
Every conflict return is a terminal-or-absent state - a missing log row, a log
already completed or errored, or a terminal cell - so a refusal never means the
run is still executing. The slot is released before the throw, keeping the exact
success && !isPausedCancellationPath predicate rather than a blanket finally
that would free reservations for live runs.
* fix(uploads): recover an ambiguous PUT instead of discarding the object
Main recovered an upload whose bytes committed but whose response was lost, via
a verify endpoint. The session client retries the PUT instead, but every
provider now signs a create-only precondition, so the retry returns 409/412,
is classified non-retryable, and the session aborts - deleting the object that
had already landed. A transient blip on the final ack cost the whole upload.
A conflict on a retry attempt is now treated as our own earlier PUT having
committed, and completion proceeds. That is safe because completeUploadSession
independently verifies the object through assertObjectIdentity, which rejects on
uploadId mismatch before anything durable is registered. A first-attempt
conflict still fails loudly.
* fix(folders): enforce the workspace folder ceiling on the create path
Readers bound the active path index at MAX_FOLDERS_PER_WORKSPACE and throw once
a workspace exceeds it, but POST /api/folders reached createFolder, which has no
maxFolderRows field and never counts. A workspace could therefore be driven past
the ceiling, after which the 27 capped read sites failed on a state the product
had allowed.
createFolder now asserts room inside its transaction, right after the mutation
lock, so the count cannot be raced. The refusal is a typed conflict rendering
409 with an actionable message rather than a 500. The check counts rows directly
instead of loading the path index, so an already-over-cap workspace gets a clean
refusal rather than a read error, and no reader gained a cap.
folderMutationStatus also gained the payload_too_large mapping it was missing,
which had been rendering a delete-cascade cap breach as an unexplained 500.
* fix(skills): route internal skill writes through the shared use cases
The internal route made the workspace authorization decision itself, never
consulting the skills operation policy, never loading canonical workspace
context, and recording an audit entry with no operation id or actor projection.
v2 and Copilot already went through the use cases; only this surface did not.
GET/POST/DELETE now authenticate, parse, call the shared use case, and present.
Request and response shapes are unchanged. Two behavior changes fall out: a
write against a deleted workspace is now refused with 404 rather than accepted,
and permission-denial text matches the rest of the platform.
Legacy internal-JWT auth is dropped because no principal kind expresses that
caller and nothing calls it: the whole repo references /api/skills only in two
comments, no tool declares an internalRoute to it, and the executor reads skills
through a direct listSkills call rather than over HTTP.
* fix(folders): enforce the workspace ceiling on the remaining create paths
Folder duplication, admin workspace import, and workspace forking all inserted
folders without consulting the ceiling that 27 read sites enforce, so any of
them could leave a workspace whose reads then fail.
Each now asserts room for the rows it is about to add rather than one at a
time: duplication measures the whole subtree up front, forking counts its bulk
insert, and import counts per segment because that is genuinely one row.
assertFolderCollectionHasRoom gained an additionalRows notion for the bulk case,
and short-circuits when nothing is being added so an over-cap workspace still
reads and still syncs.
Duplication deliberately does not take the folder mutation lock. Holding it
across the copy would block folder creation workspace-wide for an unbounded
time - there is no cap on workflows per subtree and duplicateWorkflow runs
sequentially - and narrowing it is impossible because an advisory transaction
lock cannot be released early; splitting the transaction would leave a
half-copied tree on failure. A rare few-row overshoot near the ceiling is the
better trade, and it matches what forking already does. A test asserts the lock
is absent so re-adding it is a visible decision.
Admin import gained the transaction and lock it never had. Its folder-full
refusal escapes the per-workflow result list, because a full tree is a property
of the workspace and would otherwise be buried as N failures behind a 200.
The fork and promote routes had no catch at all, and withRouteHandler only
classifies HttpError, so a refusal rendered as an opaque 500 - twice over, since
drizzle wraps the throw. Both now project a classified conflict as 409 and
rethrow anything unclassified.
* fix(uploads): bound the signed PUT lifetime and advertise its real expiry
A single-PUT transfer was signed for the whole 24h upload-session TTL, because
expiresAt was reused as both the session lifetime and the signing lifetime.
Multipart part URLs in the same file kept 1h, and the pre-migration presigned
route signed every PUT for 1h, so the widening was unintended rather than a
policy change. No provider clamps below 24h.
The PUT presign is now clamped at the provider boundary by a shared
UPLOAD_URL_TTL_MS, which the part-URL path also uses so the two cannot drift.
An expired PUT URL is deliberately not recoverable: unlike multipart, which
re-signs per part call because its progress is durable, a PUT is not resumable,
so an expired URL and an interrupted PUT have identical recovery. Nothing leaks,
since every provider signs a create-only precondition.
Clamping alone would have made the contract lie: the URL would die an hour
before the session's advertised expiresAt, with nothing telling an integrator
why the 403 happened. The PUT transfer now carries its own expiresAt, mirroring
the multipart part-URL field. It is provider-dependent on purpose - cloud
transfers report the clamped signature expiry, while the local data plane has no
signature and admits against the session, so reporting an hour there would have
been a new inaccuracy in the other direction.
* chore: delete the dead presigned-upload and skill-adapter paths
The presigned upload routes and the internal skills adapters were both replaced
during the v2 migration, leaving their implementations behind with no callers.
Removed generatePresignedUploadUrl and verifyPresignedUploadReceipt with their
three provider helpers, QUOTA_EXEMPT_STORAGE_CONTEXTS and the types it orphaned,
and the performCreateSkill/performUpdateSkill/performDeleteSkill adapters with
recordSkillEvent and statusForSkillOrchestrationError. Each was verified
unreachable across apps, packages, scripts and ee, including barrel re-exports
and string access, not just direct imports.
recordSkillEvent needed the closest look, since deleting an audit writer can
silently drop coverage. The use cases declare the same action, resource, and
description, and the framework adds the operation and actor the old helper
lacked; recordAudit back-fills actorName and actorEmail from the user table
when both are omitted, so the one field the helper passed is not lost.
The self-hosting architecture doc described a directUploadSupported flag on an
endpoint that no longer exists, and now describes the upload-session flow that
replaced it.
* refactor(folders): keep the cheap resource facts out of the schema graph
Reading a folder resource type's label or its lock support meant importing
folderResourceConfig, which imports the db schema for every table it serves and
from there reaches lib/table/service, the executor, and the tool registry.
That mattered as soon as lib/folders/queries needed a label: queries is reached
from workspace-file-manager, which is reached from the files and chat pages, so
one import edge put roughly 4,700 modules into those page graphs and broke the
tool-registry boundary audit.
Labels and lock support now live in a leaf module that imports only a type, and
config composes them so there is still one source of truth. The three folder
routes that pulled the whole config in for a single boolean read the leaf
instead.
* fix(skills): apply an upsert batch in one transaction
The internal skills route looped the batch, calling an independently committing
use case per item. A rejection on a later item left the earlier ones written and
audited while the request reported failure - the compound-mutation rule in
CLAUDE.md exists for exactly this.
No new transaction plumbing was needed: upsertSkills already wraps its whole
item loop in one db.transaction, so the partial commit came from calling it N
times rather than once. upsertSkillBatch now validates and per-skill authorizes
every item before issuing a single write, and createSkill and updateSkill became
thin wrappers over it so v2 and Copilot keep one authority for the rules.
The compound operation declares the read floor that skills.update already used,
and the use case additionally authorizes skills.create when any item lacks an id,
still ahead of every write. A read-only member who is a skill editor keeps their
edit, and creates are not authorized more loosely than before.
Audit projects one entry per committed skill, and analytics moved after the
commit so nothing is reported for a rolled-back item. Note metadata.operation
for these writes is now skills.upsert rather than skills.create/update; the
action field still carries the distinction.
* fix(security): close two disclosure gaps and finish the slot-leak fix
The payer-pool gate only covered the workspace branch. A personal API key that
omits workspaceId takes the account branch, where getHighestPrioritySubscription
resolves an organization subscription from any member row regardless of role -
so a plain member read the organization-wide credit and storage pool by dropping
one query parameter. The account branch is now gated by the same authority, and
the storage pool is not queried when it may not be disclosed. Forcing that
branch self-scoped instead would have downgraded plan, period, and status, which
is what a member needs to see whether the org is blocked.
GET /api/v1/logs/executions/[executionId] emitted the workflow snapshot raw,
carrying password sub-block values and oauth-input credential ids. It now shares
the sanitizer the v2 read already used, extracted so there is one implementation
rather than two. Env-var references are still preserved.
cancelWorkflowGroupExecution itself was unguarded, so an unexpected throw from
its transaction escaped ahead of every release site - the same reservation leak
this branch set out to close, still open on the adjacent path. It now releases
through the shared predicate and rethrows, because a failed transition means the
cell state is unknown and a success-shaped answer would be a lie. The comment
claiming the abort record cannot be taken back was false and now states the real
reason: a refusal is always a terminal-or-absent state.
* fix(v2-api): cover every persisted run status and every capped body
The workflow-runs endpoints carried the same omission the logs contract had:
the execution logger persists redacting, the run schemas did not list it, and
because validation is whole-response one such row returned 500 for an entire
page. Both schemas now derive from PersistedWorkflowExecutionStatus behind the
same AssertNever gate, so a future status is a type error rather than a
production 500. The single-run read keeps its extra queued value, which only it
can observe.
That schema was also serving as the run-list status filter. Widening it would
have accepted a filter value the application input cannot express, so the
reported set and the accepted filter are now separate schemas.
Routes declaring maxBodyBytes without payloadTooLargeResponse fell back to a
bare string with no error code and no private cache header. Rather than patch
the four, the default moved into the builders - all three had the hole - which
covers 58 body-bearing handlers, and a route override still wins. The five
per-route overrides that merely restated the default are gone.
Also documents the 413 on the one knowledge route that has a real body cap, adds
the rollout gate's 404 to the last v2 operation missing it, and rewords the
nextCursor description, which read as though every list were a full-set list.
* fix(api): make the shared traits and lifetimes single-sourced
The folder resource-traits leaf composed labels into config but restated lock
support independently, so the routes reading the trait and the orchestration
reading the config could disagree about which resources lock. Config now
composes both, and supportsLocking is required rather than optional so a new
resource type cannot silently omit it.
The upload commit claimed the PUT clamp and the multipart part URLs shared a
constant and could not drift. That was true only of the advertised expiry - the
three provider signers each hardcoded an hour, so changing the constant would
have moved what we advertise while leaving what we sign, recreating exactly the
mismatch the clamp removed. Each provider now receives the lifetime in its own
unit from the one constant.
Table restore hand-rolled its status map and returned the driver message
verbatim at 500, leaking the failed statement and its bound parameters - the
same defect this branch closed at nine other sites. It and import-csv now use
the shared projection; import-csv's result type also had to carry the lock the
classifier already set, so a 423 can name it.
Deletes v2RowWriteError, which had no callers and would have rendered a locked
table as 400 by discarding the 423 it was handed.
* fix(api): collapse the internal error envelope and restore requestId
The builders shipped two internal error envelopes: internalOrchestrationErrorPolicy
emitted { success: false, error } while internalPlainOrchestrationErrorPolicy
emitted { error }. That split approximated pre-builder behavior, where the shape
depended on which branch failed - guard clauses returned { error } and a route's
terminal try/catch returned { success: false, error }. A per-route policy cannot
express a per-branch rule, so the two disagreed on the same status across families.
Collapse to the bare { error } shape. It is what messageFromErrorBody reads on the
client and what most migrated routes already emitted. requestJson throws
ApiClientError for any non-2xx, so no typed client ever observes the discriminator.
success: true on success bodies is a separate contract and is untouched.
Also restore requestId to internal error bodies. withRouteHandler stamps it on the
bodies it generates, but the builder overrides dropped it, leaving it only on the
x-request-id header - invisible when a user pastes an error out of the UI. It is now
applied at the createJsonErrorResponse chokepoint and in both wrapper overrides, and
is omitted when there is no active request scope.
* fix(api): stamp requestId on internal auth and parse failures
Siddharth Ganesan