mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-22 05:19:54 +08:00
d5701d5b2bc2d60ce94de9ab0f34b9e398dcab15
6200
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Theodore Li
|
e4019fa39e |
fix(files): preserve principals when serving generated documents (#6654)
* fix(files): preserve principals when serving documents * fix(files): address principal serve review findings |
||
|
Waleed
|
ec70c4d85b |
improvement(nav): cut prefetch and session-recorder waste (#6656)
- Seed the workspace list instead of prefetching it. The empty-list case was signalled by throwing inside queryFn, which the retry: 1 default re-ran the entire read to re-derive, a retry delay later. Log the failure path, which was silent — contract drift would have degraded into every viewer waterfalling with nothing in the logs. - Drop non-painted nodes from rrweb snapshots via slimDOMOptions. Enumerated rather than true/'all' so headTitleMutations stays off and replays keep document.title. Prefetch concurrency and await semantics are unchanged, so sidebar paint timing matches staging. |
||
|
Waleed
|
29853fbbcf |
improvement(docs): make the API reference read as code and unify its type token (#6653)
* improvement(docs): make the API reference read as code, and unify the type token The API-page font override matched every span/div/p inside the page, which outranks the .font-mono class on specificity, so every parameter name, type, and identifier silently rendered in the body sans face. Exclude .font-mono so code tokens stay monospace. Consolidate the three divergent type-slot treatments — plain scalar, union, and schema reference each carried their own chip definition, differing in size, weight, face, and box height — onto one code token that reuses the docs inline-code recipe and the platform's 20px chip height. Demote the row metadata: 'required' and 'header' were filled pills, 'required' on the error token, making a constraint the loudest element on the page and a page of required parameters read as a page of alarms. Both are now uncontained text, leaving the type token as the only box on the row. Pin the two 'application/json' labels to one treatment; the Request Body and Response headers rendered the same string at different weights and faces. * improvement(docs): mono status-code tabs, and match fumadocs' lucide icons to emcn Status codes in the example panel are numeric literals and render as code everywhere else on the page, including the Response header's own trigger, but fumadocs rendered the strip in the body sans face. Language tabs sit in a separate container and stay sans — those are product names, not code. fumadocs draws a few lucide glyphs on API pages that its client-component overrides do not expose (the heading anchor and the code-block copy button). emcn strokes at 1.55 and lucide at 2, so those icons read heavier than every icon around them; match the weight. * fix(docs): align the auth type chip with every other property row, and wrap example code The auth row collapses its real `<token>` type and renders the chip through ::after, so the span is only a wrapper — but it still matched the type-token rule and kept that rule's border, height, and gap. The border drew a second empty box around the real chip and the gap opened in front of it, because the collapsed text remains an anonymous flex item; together they pushed the chip right by roughly 8px that no other row had. Example-panel code overflowed sideways instead of wrapping: fumadocs sizes the block with `w-max`, so it grew to its longest line inside a 400px scroller and the existing pre-wrap never applied. Cap the width, switch break-all to overflow-wrap anywhere so only unfittable tokens split, and reserve room for the copy button fumadocs floats over the first line. * revert(docs): let example code overflow instead of wrapping Wrapping restarts every continuation line at column zero, and in a JSON body indentation is what carries nesting depth — so a wrapped response misreports its own structure. A hanging indent keeps the depth but needs the shiki lines forced from flex rows to blocks, which breaks the line rhythm. Removes the pre-wrap rules rather than repointing them: fumadocs sizes the block with w-max, so the previous rule never took effect and overflow was already the behaviour on the page. * fix(docs): tighten array type tokens and keep the union separator legible An `array<T>` slot holds its angle brackets as bare text nodes, which become anonymous flex items, so the slot's gap prised `array<` and `>` away from the type they wrap. Drop the gap and let the union separator carry its own margin; this also makes the auth row's gap override redundant. The separator was dimmed twice, by a muted token and again by opacity, which on the dark chip fill left `string | null` reading as `string null`. * fix(docs): restore the hidden API key description, and drop dead API-reference CSS The rule hiding the trailing `In: header` line matched `p:has(> code)`, which is a shape, not a target — every scheme description in our specs cites a status code, so the whole explanation of personal vs workspace-scoped keys was display:none on every API reference page. Match the last child instead, and shorten the description to one line now that it renders. The dropdown trigger's hover rule had been left below a new id-qualified base rule that outranked it, so the trigger could no longer change colour on hover. Removes what does not run: the four `::-webkit-scrollbar` rules (specifying a non-auto scrollbar-width makes Chromium ignore them, and Firefox never had them) and an `order: 2` block whose selectors and declaration the type-token rule above it already carried. Names the two values the API reference repeats — the monospace stack, written out eleven times, and the 12.5px code size, written nine — as --font-mono-stack and --text-code. Also drops four !important declarations that already won on specificity, a --text-muted fallback that can never fire, and a lucide selector subsumed by the one beside it. * refactor(docs): define the API-reference chrome once, and cut the commentary The metadata face — size, leading, weight, mono stack — was written out in seven rules that a comment asked future readers to keep in sync by hand; it is now one rule those seven consume, each adding only its own colour, content, and order. The auth row's chip likewise re-derived all eleven declarations of the type token and now joins that rule, keeping only its label. Comments were running longer than the rules they documented — 88 added comment lines against 73 declarations. Trimmed to the load-bearing facts: cascade traps, browser behaviour, and the bugs a rule prevents. Dropped the block narrating why the wrap rules were reverted, which duplicated its own commit message. Also retires a scrollbar token left unreferenced by the webkit removal, moves the last two fumadocs colours in our own components onto platform tokens, and brings the callout icon to 1.55 so the docs really do have one icon weight. * fix(docs): keep the union separator in the type token's own face The `|` between union members is a classless span, so the page-wide `span:not(.font-mono)` rule assigned it the body sans face while the members beside it stayed mono — one chip rendering in two faces. Applies the inherit reset to every descendant of a type token rather than just its links, so anything fumadocs nests there later is covered too. |
||
|
Waleed
|
738006db33 |
fix(emails): re-export the wordmark onto its original canvas (#6651)
#6648 shrank the header wordmark by re-exporting the raster edge-to-edge at 168x80, replacing a 272x164 file whose mark was inset. Same URL, new proportions — so every email already delivered, which keeps the width=68 height=41 it was sent with and refetches that URL forever, now stretches the mark ~20%, and a client still holding the old bytes squashes it ~22% into the new box. That is the squished logo on staging. Re-export onto the original 272x164 canvas instead, with the outlines at their own aspect (the previous asset was itself 3.9% squashed) and the textBody fill. Old mail renders against its own numbers as before; the header takes a 43x26 box for 20.7px of ink. A test pins the canvas, since that shape — not the display size — is what old mail depends on. |
||
|
Theodore Li
|
2da80150b8 |
fix(enrichment): require work email company domain (#6531)
* fix(enrichment): require work email company domain * fix(enrichment): show exhausted cascades as not found |
||
|
Theodore Li
|
51df824451 |
fix(copilot): align principal lifetime with orchestration (#6649)
* fix(copilot): align principal lifetime with orchestration * fix(copilot): align workflow lifetime expectation |
||
|
Waleed
|
f890c89978 |
improvement(emails): size the header wordmark to the landing navbar's rule (#6648)
* improvement(emails): size the header wordmark to the landing navbar's rule The email header rendered the wordmark at 34px of ink against 16px body copy, and inked it #1a1a1a while every other line of the email uses #434343. Size it by the rule the landing navbar states: the mark stands 2px above and below its neighbouring text (18px against 14px chip labels), so 20px against the email's 16px body copy. Rasterize it from the same brand outlines the navbar renders, filled with the email's own textBody token, so the two surfaces cannot drift. * chore(emails): drop the wordmark generator script The raster is committed at 4x, so the display box can be retuned without re-exporting it — the script only ever ran by hand, and the test pinning the asset to an exact multiple of the box would have forced a pointless regeneration on any size tweak. Keep the shared outlines the navbar and 8 other surfaces already render, record how the asset was produced on the size constant, and assert the property that actually matters: the asset out-resolves its display box. * chore(branding): re-export the branding barrel through the absolute alias Matches the repo's absolute-import rule and the majority of lib barrels (billing, table, uploads and five others), instead of leaving this file split between relative and absolute re-exports. * docs(branding): correct the wordmark comments that named the removed script Two TSDoc blocks still pointed at scripts/generate-email-wordmark.ts as how the email raster is produced. Point them at the committed export and the size constant that records its fill and scale instead. |
||
|
Theodore Li
|
aeb77ddf14 | fix(files): allow document compiler to read referenced images (#6647) | ||
|
Theodore Li
|
1faac4e8ef |
fix(tools): sanitize database execution errors (#6645)
* fix(tools): sanitize database execution errors * fix(tools): retry transient permission failures * fix(tools): preserve preflight cancellation |
||
|
Vikhyath Mondreti
|
9f8d4d1310 |
fix(billing): checkout guard, admin panel case (#6641)
* fix(billing): checkout guard, admin panel case * fix(billing): serialize checkout admission * fix(billing): release checkout admission claim |
||
|
Waleed
|
e56b28857c |
fix(workflow): draw a highlighted edge over the ordinary ones (#6642)
* fix(workflow): draw a highlighted edge over the ordinary ones An edge's z came from the nesting depth of the container it belongs to, and a highlighted edge kept that depth like any other. A line one level deeper therefore sat above it and painted straight through the highlight, cutting it in half wherever the two crossed. Give a highlighted edge — selected, or connected to the selected card — the top tier of the edge band instead. Depth only ever ordered edges against each other, and once the user has picked one out, being drawn whole matters more than which container it came from. The tier stays inside the band, below the cards, deliberately: highlighted edges were elevated over the cards once before and drew across the chrome of their own endpoints. A line belongs behind cards, knobs and the action-bar swell whether or not it is highlighted, so ordinary edges give up the top of the band rather than the band being widened into the cards. * fix(workflow): elevate the connection preview edge with the rest It renders highlighted — its data carries `isConnectedToSelection` — but it was the one call site left taking a depth tier, so the line being drawn could be crossed by an ordinary edge in a deeper container. Highlighted now means elevated with no exception. Also drop the export on the highlighted tier: nothing outside the module reads it, and the band's tiers are an implementation detail of `getEdgeZIndex`. * fix(workflow): give the edge highlight one definition The z-index elevation I added checked canvas selection only, while the edge darkens for panel focus too — a block open in the editor lights its edges, and those stayed depth-tiered, so an ordinary edge could still cut through the highlight. The bug I set out to fix, on the path I had not covered. The condition already existed in two places and the second one carries a comment saying it must mirror the first exactly, because a knob checking fewer conditions than the line leaves a dark line running into a light knob. Adding the z would have made a third copy, and the finding here is what the third copy gets you. One predicate now, in `edge-highlight`, used by the line, the knobs, and the z. The canvas subscribes to the panel store rather than reading `getState()`, since the z has to be recomputed when the open block changes. |
||
|
Waleed
|
c58a6427f7 |
fix(workflow): stop a nested block jumping when it leaves its container (#6644)
* fix(workflow): stop a nested block jumping when it leaves its container `getNodeAbsolutePosition` added the container's header and padding to a child's position. Those are already in the position: React Flow places a child at its parent's origin plus its own coordinates, and `clampPositionToContainer` is what holds it clear of the chrome, flooring it at `LEFT_PADDING` and `HEADER_HEIGHT + TOP_PADDING`. Counting them twice put every nested node 16px right and 66px below where it actually renders. Visible as a block dropping down-right the moment it is dragged out of a Loop, and as a block landing off-target when dragged into one from the canvas. Also skewed container hit-testing during a drag and the bounds `fitView` focuses on. Two callers already knew: both subtracted the same three constants straight back off to recover a relative position. They now take the difference of two absolutes, which is what a relative position is. A third place, React Flow's child `extent`, had its own copy of the numbers — a fourth distinct header height, 42, against the 40 the card renders — and now reads the same constants as the clamp, so a drag stops where a drop would put it. `positionAbsolute ?? getNodeAbsolutePosition(...)` in the fit-view path can also stop disagreeing with itself: React Flow's own answer carries no offset, so the two branches returned points 66px apart for the same node. * refactor(workflow): type the node-utilities block map `useNodeUtilities` took `Record<string, any>`, so the test fixtures had to be cast to reach it and nothing in the hook was checked against a real block. Typing it as `Record<string, BlockState>` surfaced an unsafe read straight away: the cycle walk re-read `blocks[currentId].data.parentId` after the `while` condition had tested the same optional chain, on a map where both links are optional. It now reads the value once and breaks on absence, which is what the condition was trying to express. The fixtures follow the hook's own parameter type, so they stay honest without a cast on either side. |
||
|
Waleed
|
1fa40b8118 |
feat(v2): complete and align the v2 API surface (#6643)
* fix(v2): close four validation holes in the logs and billing surfaces
Each of these answered a caller-supplied value with a 500 or a silently
wrong result instead of a 400.
- `GET /api/v2/logs` accepted any string as `startDate`/`endDate`. The
route constructs a `Date` from it, so `?startDate=abc` reached the
driver's timestamp mapper as an `Invalid Date` and 500'd. Both bounds
now carry `.datetime()`, matching the sibling run list so one timestamp
works on both collections. This narrows the accepted set: a date without
a time and an offset-bearing timestamp are now rejected, and the field
descriptions say "UTC ISO 8601" rather than overpromising "ISO 8601".
- `v2BillingStatusQuerySchema` was the only non-strict query schema in its
family, so a mis-cased `workspaceID` was stripped and the caller got
account-scope billing in place of the workspace scope it asked for — a
wrong answer about money, served as a 200.
- An unresolvable `cursor` on `/api/v2/billing/logs` applied no cursor
condition and restarted the sequence at page 1 while still reporting
`hasMore`, so a pager holding a cursor across a deploy loops over the
first page and counts the same credits on every lap. It is now a 400.
The message does not reuse `INVALID_CURSOR_MESSAGE`, which names
`sortBy`/`sortOrder` params this collection does not accept.
- The logs `status` field disagrees with the run resources for the same
run: the run projection overlays `paused` from `paused_executions`,
so an ordinary human-in-the-loop pause reads `paused` there and
`pending` here. Reconciling would mean joining `paused_executions` in
this read and silently moving live runs between two buckets of a
shipped field, so the divergence is documented on the contract instead.
* feat(v2): expose the MCP tool plane and page the MCP server list
Registering an MCP server through v2 dead-ended: nothing on the public
surface ever ran tool discovery, so connectionStatus, toolCount, lastError,
and lastToolsRefresh stayed at their registration defaults and there was no
way to read a server's tools without opening the UI.
Adds GET /api/v2/mcp-servers/{id}/tools over a thin use case composed from
the existing mcp_servers.tools.discover operation, resolveServerContext, and
mcpService.discoverServerTools. It is personal-API-key-only — discovery
resolves the acting user's own OAuth credentials, which a workspace key
cannot supply — and the contract says so rather than letting callers meet an
unexplained 403. Discovery failures are classified instead of collapsing
into a 500: an unreachable or cooling-down server is a retryable 503, a
stale OAuth grant is a 401.
Also pages GET /api/v2/mcp-servers. It was the one unbounded list on the v2
surface, classified full-set on a bounded-by-construction rationale that
only holds for folder lists; nothing caps how many servers a workspace
registers.
* feat(v2/tables): strict row bodies, a filtered row count, and round-trippable required columns
Three tables gaps from the v2 capability evaluation.
Strictness. Every v2 tables request body is now `.strict()`. The row family
was the whole hole: `POST /query` sent v1's `filter` key answered 200 with a
fully unfiltered page, because Zod strips unknown keys unless told not to. The
same laxity covered the row create/update/delete/upsert/find bodies, the
run and cancel-runs bodies, the enrichment body, and — outside the row family
but the same class — the column delete, view create/update, and export bodies.
A contract sweep now walks every body-bearing tables contract and fails if one
of them stops rejecting an unrecognized key.
Filtered row count. `POST /api/v2/tables/{tableId}/query/count` answers the
question v1's `includeTotal`/`totalCount` answered and the `{data, nextCursor}`
envelope has nowhere to put: how many rows a predicate matches. It binds the
existing `queryTableRows` use case with `includeTotal: true, limit: 1` — no new
domain logic and the same `tables.rows.query` read policy. The use case types
`totalCount` as nullable because paged callers can decline it; this route always
asks for it, so a null is treated as a broken invariant rather than presented as
a fabricated zero.
Required columns. `required` is accepted on create-table, add-column, and
update-column, matching v1. v2 emitted the flag on every read while stripping it
from every write, so a column could not round-trip. Enforcement was already
complete: turning it on over rows with null, missing, or empty cells is rejected
by the domain.
* test(skills): pin the workspace-API-key split as structural, not accidental
A workspace API key can create a skill it can then never update or delete,
which no sibling resource does — so the asymmetry reads like an oversight
worth widening. It is not. Skill edits are authorized by the per-skill
editor row belonging to the acting user, which is why update/upsert/delete
declare a 'read' floor rather than 'write': workspace role is not the
authority. A workspace key carries no user subject, so allowing one replaces
a 403 with an unclassified PrincipalSubjectUserRequiredError that the v2
surface renders as a caller-reachable 500.
Records the reason on the registry and pins it, so the next reader finds the
argument instead of flipping the flag.
* feat(v2): read deployment state, and undo a file delete
Two v2 reads that existed only as a side effect of a mutation.
`GET /api/v2/workflows/{id}/deployment` publishes the state the deploy,
undeploy, and rollback responses carry, plus `needsRedeployment` — which
those responses structurally cannot carry, because they answer at the
moment the draft and the live version are equal. A caller that lost the
mutation response, or that polls from another process, had no way to ask.
Reuses `readWorkflowDeploymentStatus` behind `workflows.read`, the same
use case the internal status and deploy GETs already adapt.
`DELETE /api/v2/files/{fileId}` was a soft delete with no way to see what
it archived and no way to reverse it. `GET /api/v2/files?scope=archived`
pages the archived set and `deletedAt` on the file resource dates each
one; `POST /api/v2/files/{fileId}/restore` reverses the delete through
the existing `files.restore` operation. Restore is not a pure undo — it
returns the file to the root and renames it on a collision — so the use
case now reads the file back and both the response and the OpenAPI
description say what actually came back rather than what was deleted.
`scope=all` is rejected on the list for the reason the internal contract
already gives: it drops the `deleted_at` predicate and cannot use the
partial index. `scope=archived` combined with `folderPath` 404s when the
containing folder was archived too, which the contract documents.
* fix(v2): keep the unresolvable-cursor rejection a 400 on every surface
The cursor rejection lived in shared billing core but was an OrchestrationError
only, which the session-only GET /api/users/me/usage-logs cannot project: that
route is raw withRouteHandler and readTypedError matches instanceof HttpError,
so any signed-in caller typing ?cursor=x got a 500. UnknownUsageCursorError is
an HttpError carrying the OrchestrationError as its cause, so the v2 route still
renders BAD_REQUEST off the cause chain and the internal route answers 400.
Also closes the other half of the run-list parity: an inverted window on
GET /api/v2/logs is now a 400 instead of a silently empty page.
* fix(v2/tables): sweep union bodies per member and name the shapes on a rows 400
Review follow-ups on the strictness work.
The sweep was vacuous on the one union body it covers. Parsing
`{ notAContractField: true }` against `v2CreateTableRowsBodySchema` and looking
for `unrecognized_keys` anywhere in the issue tree is satisfied by either member
alone, so dropping `.strict()` from the single-row branch shipped green —
reproduced, 36/36 passing with the regression in place. The sweep now flattens a
union body into its members and asserts each one separately; removing `.strict()`
from either branch now fails a case that names it.
`POST /rows` answered an unknown key with `Invalid input`, the exact message the
v2 conventions name as failing the actionable-error rule, because a union
surfaces `invalid_union` first. The union now carries a message naming both
accepted shapes; the per-member failures still ride along in `details`.
Two TSDoc corrections. The `required` docstring claimed the domain rejects
turning the flag on over rows with empty cells — true of the update path, false
of add-column, which applies the flag as given (the same shape `unique` already
had here). And `.strict()` binds the top level only, so the view `config` object
and the shared sort-spec elements still strip unknown keys; both docstrings now
say so instead of implying full coverage.
* fix(v2): classify MCP discovery failures by type, not by substring
The tool-discovery error policy consumed categorizeError's status, whose
fallback is a substring match on the upstream message. Three consequences,
all caller-visible:
- A ZodError from the builder's own response `.parse` contains `invalid_type`,
so a Sim-side response-schema defect answered 400 "Invalid request
parameters" and suppressed the builder's 500 and its unhandled-error log.
- An upstream `Invalid params` or `not found` became the caller's 400/404 on a
request the contract had already validated.
- A stale OAuth grant to the third-party server answered 401, the status this
surface reserves for a missing or invalid Sim API key, so a client would
rotate a credential that was never the problem.
The policy now dispatches on the MCP error families and returns null for
anything else. Reauthorization is a 409 carrying
`details.code: MCP_SERVER_REAUTHORIZATION_REQUIRED`; an unreachable, slow, or
cooling-down server is a 503 with a constant message.
Also: widen the shared server path-param description now that it covers tool
listing, map the list query explicitly so no undeclared `cursor` reaches the
use-case input, and document the endpoint's write side effects.
* merge: bring in the MCP tool plane workstream
* feat(v2): make knowledge tags usable and let documents be updated
v2 accepted tag slots on upload and filtered search by tag display name,
but no response ever returned a tag value and nothing listed the
vocabulary, so a shipped feature dead-ended in the public API. A document
that failed processing could only be deleted and re-uploaded, and
retiring 500 documents cost 500 requests.
- GET /api/v2/knowledge/{id}/tags returns the vocabulary (display name,
slot, field type) as a full-set list.
- Document list and detail responses carry `tags`, keyed by display name
exactly as search keys its result metadata. Writes stay slot-keyed; the
tags endpoint is the mapping and the contract documents the split.
- PATCH /api/v2/knowledge/{id}/documents/{documentId} renames, enables,
disables, retags, or requeues processing. Derived indexing state is not
writable: asserting `processingStatus` on an unindexed document would
corrupt search. A retry may not ride along with field updates.
- PATCH /api/v2/knowledge/{id}/documents bulk-enables or bulk-disables.
Bulk delete is deliberately absent — that operation records no semantic
audit, and a public bulk delete would empty a knowledge base leaving no
DOCUMENT_DELETED entries.
- The document list accepts the same name-based `tagFilters` as search;
the name-to-slot resolver moves out of search into a shared helper, and
the filters are stamped into the offset cursor scope so a replayed
cursor cannot cross a filter change.
- Search accepts `rerankerEnabled`, `rerankerModel`, `rerankerInputCount`
and returns `rerankerScore`; `rerankerApiKey` and `skipUsageBilling`
stay unexposed. Every result now names its `knowledgeBaseId`.
knowledge.tags.list flips from workspaceApiKey 'deny' to 'allow' (and
gains the workspace_api_key principal kind) so it matches the sibling
reads knowledge.documents.list / read / search. The vocabulary is
required input for two operations a workspace key can already perform.
Every tag write stays human-delegated.
* fix(v2): name every 403 cause, unfork boolean params, close nested strictness holes
Four cross-cutting consistency gaps on the v2 public surface.
**403s now carry a machine-readable cause.** The conventions skill mandated
`error.details.code` on 403 and nothing emitted one, so a client had to
string-match prose to tell "raise this member's role" from "this workspace
refuses personal keys" from "buy an enterprise plan" — four different
remedies behind one status, and every message reword a silent break. The
vocabulary is a closed set, `FORBIDDEN_DETAIL_CODES`, with a `Record` of
descriptions beside it that the generated OpenAPI 403 description is built
from, so a code cannot reach the wire unpublished. Refusals throw
`ForbiddenOperationError` in the domain and `v2CaughtOrchestrationError` —
the function every v2 error policy falls through to — attaches the code, so a
route cannot forget it. The audit-log resolver distinguished four causes and
collapsed them into one; it now names each.
Cross-tenant refusals deliberately get no code: they are concealed as 404 and
naming their cause would hand back the existence signal the concealment
withholds.
**Two boolean query params rejoin the majority.** `?includeDeparted` and
`?includeOutput` were `'true'`/`'false'` string enums inherited from the
internal shapes they reused, while four sibling params were real booleans.
Both move to `booleanQueryFlagSchema`, which still coerces both strings — a
strict widening, so an existing caller is unaffected, and the spec stops
telling callers to send a string.
**Two nested strictness holes close.** `.strict()` binds the top level only,
so `sort: [{ field, direction, nulls: 'last' }]` was answered 200 with the
null-ordering request dropped, and an unknown key inside a saved view's
`config` was accepted and discarded — the headline `filter` bug one level
down. `sortSpecSchema`'s element and both view-config schemas are now strict.
Safe on the read side because `normalizeStoredViewConfig` projects the
schemaless stored blob onto the declared keys first, so a legacy row cannot
turn into a 500.
The two sort dialects stay as they are. `/logs` and `/workflows/{id}/runs`
have one sortable column, so there is no `sortBy` to pair with; renaming
`order` breaks every caller and an alias is a second spelling of one thing
with undefined precedence. Both contracts and the skill now state the rule.
* style: format the files the workspace-scoped lint gate does not reach
`turbo run lint:check` runs `biome check .` per workspace, so `scripts/` at the
repo root is outside the graph and four changed files were unformatted — one of
them a merge artifact from reconciling the route baseline across branches.
* fix(v2): collapse the four knowledge document projections onto one null-tolerant summary
Extracts toV2DocumentSummary in app/api/v2/knowledge/utils.ts and composes the
list, upload-acknowledgement and detail presenters from it. toV2TaggedDocument
serialized uploadedAt with a bare .toISOString(), so a document with no upload
timestamp threw where every sibling returned null and the contract declares the
field nullable.
Also consolidates the two Zod strictness walkers onto one shared introspection
helper that unwraps wrappers and expands unions, closing the hole where a
union-shaped schema answered null and was skipped by the pagination sweep.
* fix(v2): stop HEAD driving MCP discovery, and unbreak the updatedAt keyset page
B1: Next aliases HEAD onto GET, which RFC 9110 permits only because GET is safe.
The MCP tool-discovery GET is not: it opens a live connection to the registered
endpoint and writes the outcome onto the server row. The v2 JSON builder gains a
headSafe option, default true, and the discovery route declares itself unsafe —
a HEAD is authenticated and rate-limited, then answered bodiless.
B2: a discovery status write stamped updatedAt, which this branch added as a
keyset sort, so any concurrent discovery duplicated and skipped servers across a
caller's pages. Discovery liveness already has lastConnected, lastToolsRefresh,
lastError and statusConfig.
B4: a public refresh now skips the positive cache but keeps the failure cooldown,
so it cannot be used to drive a connection attempt per request at a failing
endpoint. An explicit user action on their own server keeps the full bypass.
B6: the consecutive-failure counter is incremented SQL-side rather than read,
incremented and written back, and the success branch carries the same workspace,
liveness and staleness guard the failure branch already had.
* fix(v2): bound the bulk update echo, close the search leak, and make the docs true
B3: a selectAll bulk document update echoed every changed identifier, which the
request does not bound — a 100k-document knowledge base produced a multi-megabyte
array, materialized and then element-wise validated. The use case now reports
whether the selection was unbounded and the presenter omits the echo.
A1: the knowledge search presenter spread the whole use-case result, which also
carries userId, workspaceId, a cost breakdown and a live secret-trace registry.
Only Zod's default key-stripping kept them off the wire. Projected explicitly.
P1-a: GET /knowledge/{id}/tags advertised all 17 slots while the document PATCH
accepted only the seven text ones. The writer already coerces every slot type,
so the PATCH now takes all 17 in their declared types, with a 400 where a
malformed value used to silently clear the tag.
P1-b: both new PATCHes deny workspace API keys and now say so.
P1-c: the two table query reads declare maxBodyBytes and now document the 413.
P1-d: getWorkflowDeploymentV2 loses its legacy suffix.
C3: deletes two orchestration error mappers with no callers that mapped
'forbidden' with no details.
D2: a stored null in table_views.config survived the pick and failed the
response schema.
Also folds the six 'bounded set' paraphrases onto one FULL_SET_LIST constant,
shares the run-window date bound between the logs and runs lists so their
documented parity is enforced rather than asserted, adds the missing barrel
export for FORBIDDEN_DETAIL_CODE_DESCRIPTIONS, and strictens two response
schemas whose peers were already strict.
Migrates 40 v2 route tests onto the shared @sim/testing harness: 26 asserted a
rateLimitSubjectIds shape v2 auth never returns, 26 asserted the wrong
refillRate, 33 could not exercise their 401 path at all, and 6 hard-wired the
rollout gate to null.
* fix(mcp): bound the connect handshake, and stop the 403 description over-claiming
B5: the connect clamp was getMaxExecutionTimeout(), the workflow ceiling of
seven days, so the real bound became the server row's own timeout — which the
registration contract permits up to 300s — times the connect retries. A slow
server could hold a Node request for roughly twenty minutes. Connecting is not a
workflow run, so the handshake now shares the one-minute ceiling tools/list
already applies to itself.
C2: the generated 403 description asserted that error.details.code names the
cause on every 403. Nine domain refusals still throw a bare forbidden
OrchestrationError and reach the wire codeless, so the wording now says 'where
the cause is one a caller can act on'. Reparenting those throws is left as a
deliberate change: one of them is a cross-tenant refusal that belongs in the
codeless class and would change its status.
* chore: reconcile the route ratchet with staging
* style: sort imports and format the three files biome flagged
* fix(openapi): import the forbidden-code constants from their module, not the application barrel
The barrel also re-exports the authorized use-case layer, which loads
@sim/db at import time. That pulled a database connection into the
OpenAPI spec check, so check:audits failed wherever DATABASE_URL is
absent, including CI.
|
||
|
Waleed
|
128054ea51 |
fix(workflow): stop subflows resizing themselves after every load (#6639)
* fix(workflow): stop subflows resizing themselves after every load
A container sized itself from its children, and when a child had not yet
reported a height it used `estimateBlockDimensions` in its place — a guess of
`ceil(subBlockCount / 2)` rows, which read a 39-field Gmail card as 276px tall
against the 112px it draws. The container painted that number, the real height
arrived a frame later, and it visibly resized between the two. Nothing is
persisted, so it happened on every refresh.
A card's height depends on what it actually renders — which rows survive its
conditions, whether it draws a summary sentence, and for a reactive field even
a credential it has to fetch — so the card is the only thing that can know it.
Size only from heights the children have themselves reported, and hold the
container at its current size until they have. `getBlockDimensions` keeps the
estimate for the callers that only need a rough box (clamping a drag, placing a
paste) and is now that same lookup plus the fallback.
Also stop `calculateContainerDimensions` counting the container's chrome twice.
Child coordinates are relative to the container's own origin and are already
held clear of the header by `clampPositionToContainer`, so a child's far edge is
the distance to cover and only the trailing padding is owed on top. Adding the
header and leading padding again left every container 66px taller and 16px
wider than its contents.
* fix(workflow): gate container sizing on this session's reported layout
Two holes in the measurement gate, both from reading the wrong field.
`height` is a persisted column and `data.width` / `data.height` persist a
container's last size, so a block that has not reported yet can still carry last
session's numbers — reachable through paste, import, and checkpoint restore. The
gate treated those as reported and sized from them.
Nested containers had it worse: an inner container with an unreported descendant
handed back its 500x300 default as though it were measured, so the outer
container sized to that and resized again once the descendant filled in — the
same two-step this change exists to remove.
`layout` is in-memory only and written by exactly the two places that know: a
card through `updateBlockLayoutMetrics`, a container through
`updateNodeDimensions`. Reading it means "reported during this session" and
nothing else, and an inner container that is still waiting reports null, so the
outer one waits with it.
`getBlockDimensions` keeps the persisted height and the estimate as fallbacks —
its callers only need a rough box, where a stale height still beats a guess.
* Revert "fix(workflow): gate container sizing on this session's reported layout"
This reverts commit
|
||
|
|
2805a8def9 |
feat(windchill): add document integration (#6577)
* feat(windchill): add document integration * fix(windchill): align tool contracts and docs * fix(windchill): use official integration icon * fix(windchill): correct response and paging semantics * fix(windchill): align execution and API contracts * refactor(windchill): inline route authentication * fix(windchill): correct OData query encoding, content download, and cleared-field handling Validated the integration end to end against PTC Windchill REST Services 2.7 documentation and fixed every divergence found. Protocol correctness: - Encode OData query spaces as %20 rather than the form-encoded `+` that URLSearchParams emits. Every multi-token $filter and $orderby reached Windchill as a literal `+` and could not match. - Download content through the documented typed navigation `<content>/PTC.ApplicationData/Content/URL`, which returns a signed vault URL, instead of a `$value` segment that WRS does not implement. The resolved URL is pinned to the configured HTTPS origin. - Terminate every Stage 2 CacheDescriptor_array entry with `;` to match the documented grammar. - Raise the $top bound to Windchill's documented 2000 maximum, keeping 200 as the default page size. Cleared-field handling: - The executor merges raw block inputs before the block's param transform, so omitting a key could not clear it. A cleared numeric or boolean field reached the URL builder as '' and threw, and cleared optional strings failed contract validation. Coercions now emit an explicit undefined, and the internal-route body strips blanks centrally. Robustness and contracts: - Bound the document-structure walk to the depth actually requested. - Loosen response schemas that re-applied request-side bounds to provider-returned values, which turned committed mutations into opaque parse failures. - Return contract-shaped bodies for oversized, malformed, and unhandled request failures. - Normalize downloaded content types and drop charset parameters. Presentation and docs: - Square the icon to a centred tile on white. - Replace WT.Document and PATCH-compatible jargon with plain language. - Fix canvas sentence noun stutters on the bulk operations. - Correct the revision skill's unverified working-copy claim to read the OID back rather than assume it, and add retirement and stale-checkout skills. - Add a manual intro section to the integration docs page. * fix(windchill): align tool copy with the docs page and rebase the route baseline Tool descriptions feed both the integration catalog and the generated docs page, so the plain-language pass had to reach them too: drop WT.Document and PATCH-compatible from the operation copy, and correct the $top bound the descriptions still advertised as 200. Correct the docs intro's attachment wording, gloss OData on first use, and attribute the bulk-atomicity claim to PTC's documented behavior. Raise the API route-count baseline, which staging advanced while this branch was behind. * feat(windchill): add update common properties Name, Number, and Organization are rejected by the PATCH-based update operation, and the rejection message told users to reach for Windchill's UpdateCommonProperties action that the integration did not expose. Add it. PTC documents UpdateCommonProperties as a bound DocMgmt action taking an Updates wrapper, available when hasCommonProperties is set on the Documents entity, and refused while the document is checked out. The subblock and param descriptions carry that constraint, and the rejection message now names the operation that does the job. * test(windchill): assert block and tool params stay aligned for every operation Validating the new operation surfaced that nothing enforced the block-to-tool alignment the review process had been checking by hand. Assert it for all 27 operations instead: every required tool param has a required, non-advanced input under that operation's condition, and no operation shows an input its tool cannot accept. Both fail on a deliberately broken condition or a dropped required flag. --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Waleed
|
9aa16e381c |
improvement(workflow): smooth the running hatch and sit it in the slot's own box (#6638)
* improvement(workflow): smooth the running hatch's slanted edges The marks read as stepped rather than slanted. A repeating gradient is sampled once per pixel with no coverage term, so a hard colour stop on an edge 15° off vertical can only land wholly on one side or the other — there is no partial value to soften the transition, and the staircase is the whole edge on a mark this thin. Ramp each edge over 0.75px, roughly a device pixel, instead of switching colour at a single offset. That hands the rasterizer the intermediate values antialiasing would have produced: measured deviation of the edge from its own straight line falls from 0.28 device px — pure quantization — to 0.05. The ramps are centred on the offsets the hard stops used, so the 50%-coverage line does not move: same 75° lean, same 24/2 rhythm, same 26px scroll period. * improvement(workflow): sit the running hatch in the slot's own box The hatch was inset 4px into a 24px row, so it stood 16px tall inside a swell whose slots are 24px — it read as a shorter bar floating inside the row rather than as the slots themselves filling, and its right end stopped short of where a hovered slot's fill ends. Span the row instead. The row already sits inside the container's 2px/3.2px inset, so occupying it outright puts the hatch in exactly the box a slot's hover fill occupies: same height, same padding in from the swell on every side. The end taper has to move with it, since its two numbers were read off the slot's diagonal at the old overlay's top and bottom (y=4 and y=20). Continuing that same edge — slope 20/24 — across the full row gives 20px in at the top and flush at the bottom, so the hatch still ends on the slot's own diagonal. * fix(workflow): feather both hatch edges, not just one The trailing ramp straddled the period boundary. Anchored at 0, the mark's leaving edge ramped 24.735 → 25.485, but a repeating gradient truncates at its own wrap, so it was cut at 25.11: half the feather, and its 50%-coverage line pulled 0.19px inward. That edge stayed sharper than the other and the gap rendered 1.75px instead of 1.93px. Run the period centre-of-mark to centre-of-mark instead, so both ramps sit strictly inside it. The stop list still tiles backwards from its first stop, so the marks land where anchoring at 0 put them — measured pitch is unchanged at 26px and both edges now carry the full 0.75px. |
||
|
Waleed
|
1b635417fc |
fix(v2): give the keyset cursor's timestamp an explicit SQL type (#6636)
Handing back the `nextCursor` from any timestamp-sorted v2 list and passing it straight in returned 500. The keyset compares millisecond-truncated timestamps on both sides, and the bound cursor value went out as a bare placeholder — which Postgres types as `unknown`. `date_trunc` is overloaded across `timestamp`, `timestamptz`, and `interval`, so `date_trunc(unknown, unknown)` matched no single candidate and the statement failed outright. The value was already validated; it just carried no type. Cast it to the column's own SQL type inside `timestampKey`, so all twelve call sites across six modules inherit the fix. Derived from the column rather than hardcoded, which keeps a `timestamptz` column's offset honoured too. The millisecond truncation is unchanged — it is what stops the page's own last row being re-admitted. |
||
|
Justin Blumencranz
|
7c2ba46cbe | fix(cmdk): keep the first result focused and the top fog stable across re-ranks (#6635) | ||
|
Justin Blumencranz
|
0e65ca32ed |
fix(copilot): surface document render failures (#6629)
* fix(copilot): surface document render failures * Address PR review feedback (#6629) - validate render errors against workspace-file provenance before returning details\n- cover blocked provenance with a regression test * Address PR review feedback (#6629) - mark every non-throwing render failure as a failed dynamic read\n- cover all soft render failure paths with producer-level tests\n\nNote: pre-existing type-check failures in HEIC and provider files are not addressed by this PR. |
||
|
Waleed
|
8a0b32862e |
fix(blocks): give a block one tile everywhere it is listed (#6634)
A block's tile disagreed with the card it named. The canvas brands only third-party integrations and gives everything first-party its role accent, but the command palette, connection lists, tag menus and output pickers all painted straight from the catalog `bgColor` — so Webhook Trigger showed green in the palette and blue on the canvas it was about to be dropped onto, and the five roleless first-party triggers showed catalog blues where the canvas shows neutral. Read the canvas rule from one place (`hasBlockAccent`) and render it through one component (`BlockTile`), then point every surface that lists a block at them: canvas, editor header, preview, toolbar, palette, connection picker, terminal, logs trace rows, connection lists, tag menus, output pickers and the tables workflow sidebar. Folds in the duplication the split had grown: three copies of `TagIcon`, five hand-rolled tile divs, the toolbar's second encoding of the accent rule, a third icon-contrast helper on its own brightness threshold, and the dead `showColoredIcon` prop every caller passed. Tiles now share the chip radius, and the tile forces its own icon colour so popover and command rows painting `[&_svg]:text-*` can no longer wash out a pale brand tile. Large detail headers (preview panel, trace-view detail) keep their own treatment and are left for a follow-up. |
||
|
Waleed
|
7022e2ef4f |
fix(workflow): stop the coloured knob leaving a barb at each shoulder tip (#6632)
The connection knob is a recolour of one stretch of the card outline, so its path has to be the outline's own path. Two things pulled it off: Its span was cut at the exact point the bulge falls under the visibility threshold, which is not one of the points the silhouette sampled — so the knob sat on a grid of its own. And a span clamped its first and last control points to the bare perimeter tangent, where the silhouette derives every control point from the samples either side of it, so those two segments bowed differently from the curve they were painted over. The knob was left still flat where the silhouette had already begun its descent, and the uncovered sliver of dark stroke read as a small spur poking off the shoulder. It is clearest on the Error output's outer shoulder, against the card's bottom-right corner. Measure the span against the interval the silhouette resamples the bulge over rather than in whole pixels, and sample a step wide on each side before trimming back, so every emitted segment has the neighbours the silhouette had. The knob's commands now come out identical to the silhouette's, which the tests pin — including for merged intervals and odd tab lengths, where measuring in whole pixels would still have landed half a step off. The painted footprint is unchanged. |
||
|
Justin Blumencranz
|
c411b1d5b3 | fix(workflow): allow dual-mode blocks inside loop/parallel subflows (#6633) | ||
|
Vikhyath Mondreti
|
f4ee41bf24 | improvement(blog): simplify provenance post structure (#6631) | ||
|
Siddharth Ganesan
|
3ae83b17f7 | fix(desktop): fix desktop prod ci #6628 | ||
|
Waleed
|
81108d5a4e |
fix(v2): serve HEAD, advertise PATCH, and document the reachable 403 (#6623)
* fix(v2): serve HEAD, advertise PATCH, and document the reachable 403 Three HTTP-semantics defects on the v2 surface, all found by probing the published contract rather than the happy path. **HEAD answered 500 on every v2 endpoint.** Next implements a missing `HEAD` export by aliasing it onto `GET` and dropping the body when it sends, so a route's `GET` legitimately runs with `request.method === 'HEAD'`. The builders' method guard compared that against the contract's declared method and threw, so `HEAD /api/v2/workflows` and every sibling replied 500 — which is what health checkers, uptime monitors, link checkers, and some CDNs send, all of them reading the API as hard-down. RFC 9110 §9.3.2 makes HEAD identical to GET but for the body, which is exactly what running the GET path produces. Fixed once in `methodMatchesContract`, shared by all five route builders; every other mismatch stays a hard error so a handler exported under the wrong verb still fails loudly. **CORS advertised `GET,POST,OPTIONS,PUT,DELETE`** while the v2 spec has 17 `PATCH` operations, so a browser preflight for any of them was rejected. It also advertised `PUT`, which two operations use — the shape of a hand-maintained list outgrown by its surface. The list stays hand-written because middleware cannot import the contract tree without pulling Zod into the edge bundle, but it is now pinned by a test that sweeps the real contracts and fails on any method it omits. **Six operations omitted a 403 their siblings documented** — three knowledge reads and three file-upload operations. Traced from the code rather than the spec: `requirePermission` throws `NoWorkspaceAccessError` for no access at all (concealed as 404) but `InsufficientWorkspacePermissionsError` for access below `minimumRole` (a real 403), and `PersonalApiKeysDisabledError` reaches every operation a personal API key can call. So 403 was reachable on all six and the omission was an accident of hand-assembled error lists, not a policy. They now use the shared `RESOURCE_ERRORS` / `RESOURCE_CONFLICT_ERRORS` sets, and two operations spelling those same sets by hand were normalized onto them. All 128 documented operations now declare 403. The rules for HEAD, for the 403/404 split, and for using the shared error sets are recorded in `.agents/skills/v2-api-conventions/SKILL.md`. * test(proxy): update the CORS policy assertion to the served method list `proxy.test.ts` pinned the previous hand-written method string, so widening `resolveApiCorsPolicy` to advertise PATCH and HEAD left it asserting a list the middleware no longer returns. The literal is kept rather than imported from `proxy.ts` so the test still pins the exact wire value independently of the implementation. * refactor(v2): retire the error sets that could omit Forbidden The three knowledge reads and three upload operations lost their `403` by assembling `[...VALIDATED_ERRORS, ...]` by hand, and `VALIDATED_ERRORS` / `STANDARD_ERRORS` were the only exported sets that omit `Forbidden`. Migrating the last consumers to the shared `RESOURCE_*` sets left both unreferenced, so deleting them turns the fix from a one-time cleanup into an invariant: there is no longer a building block from which a workspace-scoped operation can assemble an error list without `Forbidden`. Regenerating the specs produces no diff, so the migration is output-neutral. Also folds `method-match.test.ts` into `definition.test.ts` to match the repo's `feature.ts` -> `feature.test.ts` convention, types `contractMethod` as `HttpMethod` so a contract declaring `HEAD` is unrepresentable, and drops the duplicated Next-aliasing rationale so `methodMatchesContract`'s TSDoc is its single home. * fix(cors): expose the API response headers a browser client needs Without `Access-Control-Expose-Headers` a browser can read only the six CORS-safelisted response headers, so the rate-limit budget, the `Retry-After` a 429 or 503 asks the caller to observe, and the request/run correlation ids were all on the wire but invisible to `fetch()`. Server-to-server callers were unaffected, which is why it went unnoticed. Exposed on the default `/api` policy only. The per-route `CORS_RULES` entries are wildcard-origin public endpoints and opt in individually if they ever need it, so this does not widen what an anonymous cross-origin caller can read from them. |
||
|
Waleed
|
6541a22aa6 |
fix(v2): tell a caller when to come back on every failure meant to be retried (#6625)
* fix(v2): tell a caller when to come back on every failure meant to be retried Three related gaps in retry signalling, found auditing the v2 surface against RFC 9110/6585 and against how Stripe, GitHub and Google's AIPs handle the same problems. **No 503 carried `Retry-After`.** Every one of them — the three route builders' `unhandledErrorResponse`, the execute and resume routes, and `serviceFailureResponse` — funnels through `v2Error`, so the default lands there, keyed on the response *status*: `Retry-After` is defined against the status, and the status is the only half of the code/status pair a client sees. A caller that supplies its own value still wins. RFC 9110 §15.6.4 makes this a `MAY` rather than a `SHOULD`, so it is a deliberate improvement, not a conformance fix: without it a client's only defensible policy on a 503 is an immediate retry, and Sim raises 503 exactly when a dependency is too degraded to absorb one. **A 429 that already knew its wait threw it away.** The admission descriptors declare `retryAfterSeconds` per denial, but mapping a descriptor onto a preprocess error copied only `statusCode`, `code` and `retryable`. A concurrency denial therefore reached the client as a bare 429 with no `Retry-After` despite the policy layer having named the wait five seconds earlier. The value now travels `descriptor.retryAfterSeconds` → `PreprocessExecutionError.retryAfterMs` → `ExecuteWorkflowServiceFailure.retryAfterMs` → `serviceFailureResponse`, so the transport reads a number the policy owns instead of re-guessing one. The 503 default is now only the floor for paths with no policy signal. **One failure must not advise a retry at all.** `ASYNC_ENQUEUE_AMBIGUOUS` is a 503 whose enqueue may have succeeded — it deliberately retains its execution-ID claim because a job may already exist. Telling that caller to come back in five seconds invites a client with no `X-Run-Id` to start, and bill, a second run of the same workflow. It opts out via `omitRetryAfter` and returns the run id so the caller reconciles instead. `ADMISSION_RETRY_AFTER_SECONDS` is reused rather than restated, so the execute route's capacity 429 and every other surface's 503 cannot drift apart. Also records the audit in `.agents/skills/v2-api-conventions/SKILL.md`: the retry rule, the cursor-tampering invariants, and reasoned rejections of RFC 9457 problem+json, the `RateLimit-*` draft fields, renaming `X-RateLimit-*` under RFC 6648, 422-for-semantic-validation, `Location` on 201, ETag/`If-Match`, and `merge-patch+json` — each with the spec text and the industry evidence, so they are not re-litigated. `Deprecation`/`Sunset` on v1 is left open pending a retirement date, which is a product decision. * docs(v2): name the one 503 that omits Retry-After in the shared contract The shared ServiceUnavailable description claimed every 503 carries the header, which the ASYNC_ENQUEUE_AMBIGUOUS response deliberately does not. It now says the header is normally present and names that exception, so the published contract matches the runtime behaviour for all 128 operations. |
||
|
Waleed
|
9dfd9db3b0 |
feat(xai): wire reasoning effort through the Grok adapter (#6627)
* feat(xai): wire reasoning effort through the Grok adapter The catalog never declared reasoningEffort for xAI and the adapter never sent reasoning_effort, so the flag was dead for every Grok model. Values are per-model and verified against the live API rather than the docs, which are wrong in three places: grok-4.5 does accept xhigh, grok-4.3 supports the parameter at all (undocumented) including none, and grok-4.20-0309-reasoning rejects it outright despite being a reasoning model. Also corrects grok-4.5's missing cachedInput and drops an inline comment the new provider TSDoc now covers. * test(xai): type the provider test helper instead of casting to any * fix(agent): correct reasoning-effort copy that still claimed GPT-5 only |
||
|
Theodore Li
|
afa02939bb | fix(docs): include API key header in generated code samples (#6630) | ||
|
Waleed
|
74212ef333 |
feat(models): add grok-4.6 and make it the xAI flagship (#6624)
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. |
||
|
Waleed
|
878856b6a9 | chore(ci): declare least-privilege permissions on the desktop e2e workflow (#6622) | ||
|
Waleed
|
5cf60f6a41 |
fix(v2): give every collection one pagination contract and close four envelope holes (#6620)
* 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.
|
||
|
Waleed
|
0c4fb1374a |
fix(copilot): keep a file-preview failure from stripping a tool call's arguments (#6621)
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 |
||
|
Waleed
|
7b3e6a63b0 |
fix(executor): carry child provenance across the workflow agent tool result (#6619)
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.
|
||
|
Waleed
|
082deb2349 |
fix(executor): copy the env map at the workflow-tool boundary (#6618)
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. |
||
|
Waleed
|
2054947503 |
test(export): pin the table and tool-param loss the sanitized export accepts (#6613)
* 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. |
||
|
Waleed
|
366829b6b0 |
fix(v2): derive the log and run status enums from the persisted status list (#6612)
* 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
|
||
|
Waleed
|
47f143016e |
fix(docs): restore api-reference URL continuity and fix translated SDK bodies (#6617)
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? |
||
|
Waleed
|
34d65df7d6 |
fix(sdk): make the 0.2.0 SDK release safe to publish (#6616)
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
|
||
|
Waleed
|
5a63eb8567 |
fix(copilot): clamp the legacy int4 size when materializing a chat upload (#6615)
`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". |
||
|
Waleed
|
b33f8e82fe |
fix(executor): give the workflow agent tool the caller's env and PII policy (#6611)
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.
|
||
|
Waleed
|
8f85ded35f |
fix(workflows): pin a stored block retry policy when loading it (#6614)
`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. |
||
|
Waleed
|
d60b6c6273 |
fix(mcp): stop refusing tool results authored by another workspace member (#6610)
`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. |
||
|
Waleed
|
d96f3f95c8 |
perf(db): drop 0287's two zero-row scans from the ACCESS EXCLUSIVE hold (#6609)
drizzle-orm 0.45.2 runs every pending migration inside ONE transaction (node_modules/drizzle-orm/pg-core/dialect.js:60-71), and migrate.ts:212 sets `statement_timeout = 0`. So the ACCESS EXCLUSIVE that 0287:2's ADD COLUMN takes on `workflow_blocks` is held, unbounded, through 0288 and 0289 to COMMIT — `lock_timeout` at migrate.ts:213 bounds acquisition only, exactly as that file's own TSDoc at :76-78 says. Every editor load, workflow save, executor block read, and realtime canvas op queues behind it platform-wide, and migrations run before image promotion (ci.yml:113-133), so the stall lands on 100% old-version traffic. Two of the three statements inside that hold did nothing. `data.errorEnabled` never existed in a released version — `git log origin/main -S errorEnabled` returns zero commits across main's entire history — so both statements filtered on it match zero rows, and the file's own comment said as much. There is no index on the `data` expression, so each was a full sequential scan of the whole table. Measured on PostgreSQL 17.9 against a 328 MB / 200k-row fixture built to the same bytes-per-row shape as the reported production table: 0287:2 ADD COLUMN 0.5 ms metadata-only, takes AccessExclusiveLock 0287:11 edge backfill 21 ms Nested Loop -> Index Scan on the PK 0287:20 (deleted) 47 ms Seq Scan, 200,000 rows removed, 0 matched 0287:22 (deleted) 47 ms Seq Scan, 200,000 rows removed, 0 matched A concurrent primary-key SELECT started 50 ms into the transaction was blocked 53-219 ms before and 22-25 ms after — the latter indistinguishable from the 21-33 ms control with no migration running at all. The two deleted statements accounted for 80,520 buffer accesses (~629 MB of in-lock I/O on that fixture) and 81% of the transaction's work. Editing an already-merged migration in place is safe here specifically because drizzle writes `hash` but never reads it back: the skip test at dialect.js:56-63 compares `created_at` against `folderMillis` only. The edit is therefore a no-op on every database that already applied 0287 (staging, dev, branch DBs) and takes effect only where it has not run. `meta/_journal.json` and the snapshot prevId chain are untouched. The only casualty is a stale `data.errorEnabled` key on branch databases a developer created it on. Nothing reads it: save.ts:50 and load.ts:89 both read the `error_enabled` COLUMN, and load.ts passes `data` through untouched. Alternatives rejected, each checked against the code rather than assumed: - An embedded `COMMIT;` to end the transaction early. 0289's four CREATE TYPE, its CREATE TABLE, and its three CREATE INDEX all lack IF NOT EXISTS, so a mid-batch failure in autocommit leaves them applied-but-unjournaled and the replay dies on 42710 — which migrate.ts:219 only retries for 55P03. That turns a transient stall into a wedged deploy. - Moving the statements to a new file after 0289. All pending files share one transaction, so it buys exactly zero lock reduction. - Rewriting the surviving backfill as `WHERE id IN (SELECT source_block_id FROM workflow_edges WHERE ...)`. Measured both: planner-equivalent. `source_handle` is unindexed, so both forms seq-scan `workflow_edges` (7.5 ms, identical) and then index-scan `workflow_blocks` on the primary key — it never scans that table. The IN form adds a HashAggregate and 1,170 more buffers, so it is marginally worse. Left as shipped. - Promoting the `data-backfill` lint from warn to annotate. `readAnnotation` only requires a non-empty reason and 0287 already supplies one per statement, so the rule would fire zero findings. 0288's nullable `retry` column is correct as-is and unchanged. Both delete-and-reinsert save paths on the deployed version (save.ts:30-63 and the realtime REPLACE_STATE handler) reset `error_enabled` to false and `retry` to NULL for a workflow saved by an old replica during the rollout; the ordinary realtime block upsert does not, because its `set` clause omits both columns. That residue is tolerable: everything the surviving backfill writes is re-derived from the edge set at workflow-block.tsx:754 and lib/workflows/persistence/utils.ts:196-201, and `retry` ships in this same release so it has no installed base. `bun run check:migrations origin/main` drops from three data-backfill warnings to one. The real migrator applied all 289 journal entries to a fresh PostgreSQL 17.9 database with the edited file, producing `error_enabled boolean not null default false` and `retry jsonb`. |
||
|
Waleed
|
d1bc99a9c4 |
improvement(integrations): add Managed Agents templates and guard docs links (#6608)
* 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. |
||
|
Waleed
|
892401a8d0 |
fix(uploads): sign Azure upload URLs create-only (#6607)
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.
|
||
|
Waleed
|
b5d9e93797 |
fix(mcp): audit an upsert that rewrites or revives a server (#6602)
* 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. |
||
|
Waleed
|
068422b3ff |
refactor(audit): derive updatedFields through one shared helper (#6604)
* 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. |
||
|
Waleed
|
a40e379784 |
chore: remove the stray .agiloft-spec working notes (#6605)
Transcribed Agiloft REST docs committed by accident in #6562 alongside the alrest repointing. Nothing imports or links it, it sits in a dot-directory at the repo root rather than anywhere docs live, and the integration it informed has shipped. Contents are the vendor's own public examples — placeholder credentials and localhost hosts only, so nothing sensitive was exposed while it was public. |
||
|
Waleed
|
e65345b63d |
test(table): pin the executor auth pairing on the table read route (#6603)
* 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. |
||
|
Waleed
|
0d640aabbc |
fix(uploads): make execution attachment completion replay-safe (#6601)
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.
|