mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-31 03:47:12 +08:00
fe0b92bb6db87b4733ddf31c00bde15ce8f40d76
264
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Vikhyath MondretiandClaude Opus 5
|
fe0b92bb6d |
feat(secrets): show where a secret is referenced, beside its usage log (#6947)
* feat(secrets): show where a secret is referenced, beside its usage log
"See usage" answered who has run something with a key. It could not answer the
question a rotation actually starts from — where is this wired in — because a
secret four blocks depend on but nothing has executed yet has no usage rows at
all, so the panel read "This secret has not been used yet" for a live key.
The usage view now carries two tabs. Logs is the existing trail, unchanged and
still the default, since that is what the header action has always opened.
References is new: the blocks that name the secret as {{KEY}}, grouped under
their workflow, then the custom tools and MCP servers whose own bodies carry it.
Detection is the workspace-fork remapper's. remapSubBlocks already walks nested
tool-input params, resolves canonical basic/advanced pairs, and skips dormant
and condition-hidden members, so calling it per block inherits every rule a
fork already obeys. Only the aggregation is new: scanWorkflowReferences
collapses its output to unique (kind, sourceId) pairs and discards the workflow
— right for building a mapping table, wrong for locating a key. Nothing under
ee/workspace-forking changed.
- Candidates come from strpos(sub_blocks::text, name) > 0, deliberately not
LIKE: `_` is a LIKE single-character wildcard and nearly every env key
contains one, so SB_ACTION_ROUTER_SECRET would match text it does not occur
in. The prefilter can over-match but never under-match; the scanner decides.
The plan is an index scan on workflow by workspace, nested-looped into
workflow_blocks, so cost tracks the workspace rather than the table.
- Scope gates the read but does not narrow it. A {{KEY}} names a key, not a
scope, so the same sites answer for a workspace secret and the personal one it
shadows; narrowing here would report a personal secret as unreferenced the
moment a workspace variable of the same name existed.
- References reports one field per block, not a list. The remapper dedupes a
block's references by (kind, sourceId), so a block naming the secret twice
yields one entry — the type says so and a test pins it, because the row
renders that field as its whole description.
- Reads live state, not deployed: a draft workflow referencing the key must
show. Blocks are capped and the cap is reported as `truncated` rather than
silently trimming the list.
- Authorization is the existing usage gate, renamed requireSecretTrailReadAccess
and shared verbatim, so the two tabs can never disagree about who may look.
UI is existing primitives only — ChipModalTabs for the strip, DetailSection per
workflow over RESOURCE_LIST_STACK rows, IntegrationTile for the block glyph so a
block reads here as it does on an integrations row, SettingsEmptyState for the
gates. No new component, no new class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): close the reference-scan scope bypass and bound its output
Review round 1.
- use-cases.ts: `scope` was a caller-controlled assertion the reference scan never
narrowed by, so `scope=personal` returned from the shared gate before any check
and handed any workspace member the admin-gated reference map for any workspace
secret. The usage trail can trust that scope because it filters the read by
`secretOwnerUserId`; a name-based workspace-wide scan cannot. References now
authorize on what the NAME resolves to — a workspace secret under that name is
admin-gated outright, and absent one the caller must actually hold a personal
secret of that name, which also stops a member enumerating arbitrary names.
`scope` is dropped from the input, the contract, the hook and the query key
rather than merely ignored: a parameter that does not exist cannot be asserted.
The trail gate keeps its old name and a note saying why only a scope-narrowed
read may reuse it.
- scan.ts: the prefilter matched the bare name, so `API_KEY` also read every block
holding `{{API_KEY_TEST}}` or the words "the API_KEY value" — and those false
positives counted against the row cap, so on a workspace with enough of them
genuine references sorted later were never read at all. It now matches the
reference syntax (`{{name}}`, with the whitespace ENV_REF_PATTERN allows), so a
candidate is a real occurrence and the cap means what it says. A name outside the
env-key charset short-circuits, which is also what makes it safe to inline into
the regex unescaped. Verified against a real workspace: the exact key still
returns its 16 blocks, its prefix now returns 0 where it previously matched all
16, and a metacharacter name touches no query.
- scan.ts: capping tool and server ROWS did not bound the output — one MCP server
emits an entry per matching header plus one for its url, so 200 rows could expand
past the contract's 400-entry bound and make the route reject its own response,
turning a successful scan into a 500 and the tab into "Could not load
references." Emission now stops at the bound and reports `truncated`.
- secret-references-panel.tsx: the empty-state early return preceded the truncation
banner, so a capped scan that filtered everything out claimed the secret was
unreferenced. Both paths now share one note, and silence from a capped scan reads
as absence of evidence rather than evidence of absence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): cover unicode whitespace, legacy keys, and the shadowed-personal tab
Review round 2 — all three follow from round 1's own fixes.
- scan.ts: the syntax prefilter anchored on `[[:space:]]`, but the two engines
disagree about what whitespace is. `ENV_REF_PATTERN`'s `\s` accepts U+00A0,
U+202F and U+3000; Postgres `[[:space:]]` matches only the ASCII set. So a value
pasted with a non-breaking space inside the braces is a reference the executor
resolves and the prefilter silently dropped — the one failure direction this
feature must never take, since the answer it gives is "unused, safe to delete".
Anchoring on `[^[:alnum:]_]` instead accepts every whitespace encoding while
still rejecting a longer key on either side, and needs no code-point list that
could drift. It can admit a non-reference like `{{-NAME-}}`; that costs one
candidate row, and the scanner re-checks every candidate regardless. Erring loose
here is deliberate. (Greptile's `{{\tAPI_KEY\t}}` example was already handled —
tab is ASCII — but the unicode half of the finding was real.)
- use-cases.ts: the gate read `keyAccess.knownKeys` as "a workspace secret exists
under this name", but that set only covers names with an `env_workspace`
credential row. A legacy value written before the ACL existed has no row and
still wins at run time, so it fell through to the personal branch and handed a
non-admin the reference map for exactly the oldest keys. It now reads the
authoritative `workspace_environment.variables` map through a new
`hasWorkspaceEnvValue`, which is documented against `knownKeys` so the two are
not confused again. `getWorkspaceEnvKeyAdminAccess` keeps its existing contract —
its `knownKeys` still answers the ACL question its other callers ask.
- secret-references-panel.tsx: a personal secret shadowed by a same-named workspace
variable could open the view (its owner may read their own Logs) but References
always hit the workspace refusal and rendered a generic load error — a tab
offered in a state where it cannot succeed. The refusal is correct; the tab now
states the shadowing instead of asking for a map it will be denied, reusing the
wording the detail page already shows. No request is made in that state.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): re-check the reference gate's volatile input after the scan
Review round 3.
The name-resolution gate reads whether a workspace value exists, then scans. A
workspace secret created between the two makes the map now in hand admin-gated,
so a personal owner could receive it without workspace-secret administration.
The window is small and the data is derivable — a workspace member can already
open every workflow and read its `{{KEY}}` references — but the gate's stated
contract is that references follow the same predicate as revealing the value, and
a point-in-time check that can be overtaken does not honour that. An advisory lock
or a snapshot transaction would serialize a read-only view against secret writes
for it, which is the wrong trade.
Instead the one volatile input is re-read after the scan and the request fails
closed if it flipped. `requireSecretReferencesReadAccess` now reports which branch
authorized: an `admin` grant holds however the name resolves and pays nothing,
while a `personal` grant — the only one resting on absence — is re-checked. A
non-admin loses nothing they were entitled to keep; the request is refused the way
it would have been a moment later.
Adds a `listSecretReferencesUseCase` suite covering both denial paths, the legacy
value, the personal owner, the admin short-circuit, and the race itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): accept JSON-escaped whitespace in the reference prefilter
Review round 4.
The prefilter reads a `::text` rendering of a JSON column, and `jsonb::text`
renders a real tab inside a string value as the literal pair `\` `t`. `t` is
alphanumeric, so `[^[:alnum:]_]` could not consume it and the row was discarded
before `ENV_REF_PATTERN` ever ran — the References tab omitting a live reference
and reporting `truncated: false` while doing it.
Round 3's fix was verified against a raw text value rather than the JSON
rendering, which is exactly why it looked correct: `E'{{\tAPI_KEY\t}}'` matches,
`jsonb_build_object('v', E'{{\tAPI_KEY\t}}')::text` does not.
The gap between `{{` and the name now accepts three encodings at once — raw
characters (covering every Unicode space, which Postgres `[[:space:]]` misses),
JSON two-character escapes, and `\uXXXX` (how a vertical tab survives the same
rendering). Verified against the real jsonb rendering: tab, newline, carriage
return, vertical tab and form feed all recover, U+00A0 / U+3000 / space / plain
keep matching, and `{{API_KEY_TEST}}`, `{{MY_API_KEY}}` and prose are still
rejected — so the row cap keeps meaning what it says.
Plain-text columns (`custom_tools.code`, `mcp_servers.url`) carry no JSON
escaping, but tool code is JavaScript source and can contain the same escape
sequences literally, so the one predicate is right for every column.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(secrets): land the References link on the block, and name its field
Feedback round.
- Logs leads the tab strip. It was already the default tab; the order now says so.
- The usage view drops its resource heading for a plain "Usage" title. The back
chip already names the secret, so the tile and the subtitle underneath were
saying it a second time. `CredentialDetailLayout` gains an optional `title`
that renders the same element, class and column position the settings shell
gives `SettingsPanel` — which is how the sibling Forks "Activity" view titles
itself. Existing callers pass nothing and are unchanged.
- A block row now lands on the block instead of the workflow's default framing.
The editor had no URL params at all, so `?block=` is its first: read once on
arrival, acted on, and stripped. It is a navigation signal rather than canvas
state — the carve-out in sim-url-state.md is about pan, zoom, selection and
drag, which are socket-synced or high-frequency; this is neither, and it rides
in the link so a middle-click or reload keeps it where an in-memory handoff
could not.
The consuming effect mirrors the note-search reveal in the same file, including
the three details that make that one work: read from `displayNodes` so a target
arriving before its node mounts is retried on the mounting commit, route
selection through `resolveSelectionConflicts`, and latch in a ref. It also
claims `userFocusedWorkflowIdRef` the way a node click does, because `onInit`
re-reads that inside its own rAF and would otherwise `fitView` over the camera —
and that ref is reset by exactly the `workflowIdParam` change a deep link causes.
The panel opens for free: `syncPanelWithSelection` already follows selection.
`useSearchParams` needs a Suspense boundary and the editor's ancestry has none,
so the read lives in a leaf under its own `fallback={null}` rather than wrapping
the editor and adding a `loading.tsx` to its mount path. `next build` passes.
- A tool-input reference showed `tools-tool-0-code`. Those
`{subBlockId}-tool-{index}-{paramId}` keys are documented as an ephemeral,
client-only projection of the canonical `tool-input` value and are not meant to
be persisted, but older rows carry them — so the scanner reported whichever the
record yielded last. They are dropped before scanning, which is right even where
the two disagree: `tool.params` is what executes, so a mirror the canonical no
longer matches describes a reference that no longer runs.
- The row now shows the field's label from the block config rather than its
storage id — "API Key", "Tools", "Code", "Bot Token" — falling back to the id
when the block or field is unregistered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): make the reference prefilter exactly as tight as the authority
Review round 6.
The gap between `{{` and the name accepted any non-word character, so
`{{-API_KEY-}}` and `{{"API_KEY"}}` matched in SQL while `ENV_REF_PATTERN`
rejects them. The previous commit called that free — "costs a candidate row and
nothing else" — which was wrong: a candidate row is a slot under
BLOCK_SCAN_LIMIT, so enough near-misses sorted earlier exhaust the cap before a
genuine reference is read, and the tab reports a live key as unused. That is the
same failure the tightening in round 1 was meant to remove, reintroduced by the
round 4 loosening that fixed JSON-escaped whitespace.
The gap now enumerates exactly the whitespace `\s` accepts, in each encoding it
can arrive in: `[[:space:]]` for raw ASCII, `\\[tnrf]` and `\\u000[bB]` for the
JSON escapes, and an explicit class for the Unicode spaces Postgres emits
verbatim but `[[:space:]]` does not match.
That class is generated from a code-point table rather than written literally.
Writing it by hand put a run of invisible characters in the source — a reviewer
cannot check them, and a formatter or editor can silently mangle them. The table
is the readable form and `toPgEscape` renders it.
Verified against the real jsonb rendering, 17 cases: raw space, tab, newline,
carriage return, vertical tab, form feed, U+00A0, U+202F, U+3000 and an embedded
reference all match; `{{-API_KEY-}}`, `{{"API_KEY"}}`, `{{API_KEY_TEST}}`,
`{{MY_API_KEY}}`, prose and an across-braces span all do not. Every candidate the
SQL admits is now a real occurrence, so the cap counts references and nothing
else.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): cap the reference scan on results, not candidates
Review round 7.
The prefilter now matches reference syntax exactly, but `remapSubBlocks` filters
further on semantics SQL cannot see: it drops dormant canonical members and
condition-hidden fields. So a block whose only `{{KEY}}` sits in a hidden field is
a genuine candidate that yields nothing, and with the cap counting candidates,
enough of those sorted earlier displaced active references out of the answer.
Unlike the previous two rounds this is not fixable by tightening the prefilter —
no SQL predicate can evaluate canonical modes or field conditions. So the cap
moves to what it should have counted all along: blocks REPORTED. Candidates are
read a page at a time up to a ceiling far above the result limit, so filtered rows
are absorbed as extra reads instead of taking result slots.
Paging rather than one large read because the alternative is holding every
candidate block's `sub_blocks` in memory at once; peak memory is now one page.
`blockId` joins the ordering as a final tiebreak, since OFFSET paging over a
non-unique sort can repeat or skip rows across pages — which here would
double-report a block or silently lose one.
This does not make the scan unconditionally complete, and the ceiling says so:
bounded work and guaranteed completeness cannot both hold, so the only real
choice is where the bound sits and whether it counts something the reader can
see. It now counts results.
Query plan re-checked with the OFFSET in place: still an index scan on workflow by
workspace, nested-looped into workflow_blocks. Test added that pins the fix — 2,500
prose candidates sorted ahead of one real reference, which the previous cap
dropped entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): drop the paging that caused drift, and stop false truncation
Review round 8.
- Paging removed. It bought headroom and paid with drift: `OFFSET` is positional,
so a block renamed, inserted or deleted between page queries shifts the result
set, and the scan skips a live reference or reports one twice. That is a worse
failure than the one paging was added to fix, and it was self-inflicted last
round. Candidates are read in one statement again — one statement is one
snapshot, so neither skew nor duplication is possible — with the ceiling
lowered to 4,000 so a single read stays a sane amount of memory. Result-capping
survives, which was the actual point: filtered rows are still absorbed as extra
reads rather than taking result slots.
- `truncated` no longer fires on an exact landing. The block path now uses the
limit-plus-one read and strict `>` the resource paths already used, so a scan
that ends precisely on a bound reports complete instead of warning about
references that were all returned.
- The deep-link target is released when its block does not exist. It was cleared
only on a match, so a link to a since-deleted block left the id set with the
param already stripped: the effect re-checked on every canvas update forever and
shadowed a later link to the same block. Once any node has mounted the canvas is
populated, so an id still absent is gone and the target is dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): gate the deep-link release on the workflow being ready
Review round 9.
Round 8 released a deep-link target once `displayNodes` was non-empty, reading
that as "the canvas is populated, so a missing id is deleted". It is not: arriving
from another workflow the store still holds that graph, so nodes are present while
the linked workflow is still hydrating — and a valid `?block=` target was dropped
before its own blocks ever mounted.
The file already had the right predicate. `isWorkflowReady` pins
`hydration.phase === 'ready'`, `hydration.workflowId === workflowIdParam` and
`activeWorkflowId === workflowIdParam`, which is exactly "the graph now loaded is
this workflow's". Absence is only conclusive under that, and a node count never
was — it says something mounted, not whose.
This is the second fix to this release condition in two rounds, both from guessing
at a readiness signal instead of using the one the component already computes for
the same question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Siddharth Ganesan
|
58aa6379e0 |
feat(cli): add chat command (#6937)
* feat(cli): add chat command * fix(cli): harden chat command execution |
||
|
Waleed
|
00d8a3fbfe |
fix(knowledge): refund a processing attempt whose dispatch never happened (#6938)
* fix(knowledge): refund a processing attempt whose dispatch never happened `markDocumentsQueued` spends one attempt from the processing budget on every dispatch, and `clearDocumentsQueued` already withdraws the queue stamp on the one path that proves nothing was dispatched — but it left the attempt spent. The budget exists to stop re-billing a document that keeps failing the same way in processing. An attempt that never reached a worker teaches it nothing, so an infrastructure outage — a Trigger.dev region error, an exhausted quota — burned the allowance without a single run. MAX_PROCESSING_ATTEMPTS such outages dead-letter the document, and the connector sweep filters on `processingAttempts < MAX_PROCESSING_ATTEMPTS`, so automatic recovery stops for a document that was never processed once. Refunded in the same guarded statement as the stamp, so it can only ever give back the charge this call made, and floored at zero. Also corrects the stale-lock TTL doc, which still described the reclaim as measuring `updatedAt` after it moved to `COALESCE(syncLockLeaseAt, updatedAt)`, and the attempt-budget rationale, which predates the refund. * chore(audits): re-record the route module-graph baseline Every workspace route had drifted past what the baseline records, uniformly: the shared `[workspaceId]/layout.tsx` grew +28 and each route inheriting it grew +27 to +29. The knowledge page carries +18 of its own on top, which put it +46 over a +44 allowance and was the only entry actually failing. Bisecting the growth across the last five commits on staging shows 2249 → 2251 → 2252 → 2253 → 2255 — one or two modules per unrelated feature PR, not a single regression dragging in a fat dependency. That is the organic creep the `max(25, 2%)` ratchet is meant to absorb and the re-record is meant to settle. Counts only: all 34 entries are preserved and every entry's gateway set is unchanged, so the registry-reachability gate and the per-entry ratchet keep exactly the strength they had. |
||
|
Vikhyath Mondreti
|
ab66ce9149 |
fix(admin): harden dashboard billing operations (#6914)
* feat(admin): make dashboard billing operations durable * fix(admin): close dashboard recovery gaps * fix(admin): preserve member operation lock order * fix(admin): harden durable operation boundaries * fix(admin): report member operation failures accurately * test(invitations): cover locked seat admission * feat(enterprise): gate owner activation on acceptance * fix(enterprise): keep owner activation recoverable |
||
|
|
42f6287911 |
feat(byok): add organization-wide key inheritance (#6834)
* feat(byok): add organization key management * feat(byok): inherit organization keys at runtime * feat(byok): add organization scope to BYOK settings * fix(byok): refresh org key state after mutations * fix(byok): hide stale inherited status badges * chore(db): drop colliding byok migration ahead of staging merge Staging independently claimed 0293. Remove ours so the merge is clean; it is regenerated at the next free index right after. * chore(db): regenerate byok migration at 0296 Staging claimed 0293-0295 during the merge; the regenerated SQL is byte-identical to the dropped 0293. * docs(byok): document organization scope, precedence, and the full provider list The BYOK section described workspace-scoped keys only. Add the organization scope, its Enterprise requirement, the per-provider precedence rule, what an entitlement lapse does, and the Pi sandbox exposure. Refresh the provider table from the settings page, which had drifted from 14 to 34 entries. * feat(byok): open organization keys to every organization plan Organization BYOK was gated on Enterprise, but an organization is the only thing that can hold the keys, so every plan that can own an organization should qualify — Pro for Teams, Max for Teams, and Enterprise. Add checkOrgPlan/resolveOrganizationPlan beside the Enterprise pair rather than widening checkEnterprisePlan, so the Enterprise-only gates (Access Control, whitelabeling) are untouched, and restore resolveOrganizationEnterprisePlan to module-private now that BYOK no longer needs it. * perf(byok): cache the organization entitlement, not the key material getBYOKKey runs once per agent block and once per hosted-capable tool call, so a loop over N items resolved N times — and each organization-inheriting resolution paid three sequential billing queries on top of the two key reads. Split the two reads by staleness tolerance. Key rows stay fresh, because revocation must be immediate. The entitlement is a billing gate that tolerates bounded staleness in the harmless direction (a lapsed organization keeps using its own key for <=60s), so cache it per organization with an in-flight share so concurrent blocks issue one query set. The management surfaces keep reading it fresh, so an organization that just upgraded is never told otherwise. Also run the block check and subscription read in parallel inside resolveOrganizationPlan, and carry the resolved scope on BYOKKeyResult so a log line can say whether a run used the workspace's key or an inherited one. * feat(byok): let workspaces store the Z.ai and Cohere keys the runtime reads Both ids were already in the BYOK contract enum and both are resolved at execution time — getApiKeyWithBYOK reaches 'zai' (GLM models are in the hosted catalog, so the BYOK branch runs), and 'cohere' backs both the Embeddings block and Knowledge Base reranking — but neither appeared in the settings list, so there was no way to store the key either path looks for. Cohere had no icon; add one from the official multi-color mark so it stays legible on a light and a dark page. Cohere's embed-v4.0 is kbEligible:false, so the description says 'Embeddings and Knowledge Base reranking' rather than claiming KB embeddings. * improvement(byok): shorten the workspace scope chip to 'Workspace' It sits beside 'Organization', so the scope reads from the pair; 'This' only added width. * fix(byok): do not cache a billing outage as an unentitled organization resolveOrganizationPlan maps a failed billing read to false, which is indistinguishable from a real plan lapse. The entitlement cache stored that, so one transient outage held the gate shut for the full TTL and every inheriting run silently fell back to a metered hosted key — and the cache's rejection path, which exists to prevent exactly this, was unreachable. Give the resolver the onError option its neighbours already have and let the cached read ask for 'throw', so a failure stays out of the cache and the next resolution retries. Behavior for the call that saw the error is unchanged: getBYOKKey still fails closed. Reported by Cursor Bugbot. * fix(byok): propagate the subscription read's failure too The previous commit threaded onError through resolveOrganizationPlan's own catch, but getOrganizationSubscriptionUsable soft-fails to null on its own, so a failed subscription read still arrived as an ordinary 'no usable subscription' and returned a successful false — which the entitlement cache then stored for the full TTL. Thread the option into that call as well. Test it at the billing layer rather than the cache layer: the entitlement test mocks resolveOrganizationPlan wholesale, so it could never have caught this. Verified the new test fails against the previous commit. Reported by Cursor Bugbot. * refactor(byok): cache the entitlement with LRUCache, like copilot entitlements The hand-rolled version reinvented three things the codebase already has a canonical answer for. lru-cache is a declared dependency of apps/sim and lib/copilot/entitlements.ts already caches an entitlement with it — by storing the in-flight Promise, which is what makes concurrent callers collapse onto one resolution with no in-flight bookkeeping at all. TTL and the size bound come from the library. That removes the second Map, the manual eviction (and its interaction with an in-flight entry), and the dead value-while-refreshing state: 23 executable lines. The one thing the library does not cover is dropping a rejected promise so a billing outage is not cached for the TTL, which is kept and pinned by a test that fails without it. TTL expiry is no longer re-tested — that is the library's behavior, not ours, and lru-cache reads its clock at module load so faking timers never moved it. * refactor(byok): coalesce the entitlement read with the shared singleflight lib/concurrency/singleflight.ts is the codebase's coalescing primitive and oauth/credential-service.ts already pairs it with a read-through cache. Adopting that shape fixes a case caching the promise directly did not: a *hung* billing read wedged every caller for the full 60s TTL, where coalesceLocally evicts and rejects at its settle deadline. It also removes the hand-rolled rejection eviction — the cache is written only on the success path, so an outage leaves no entry by construction. The cache now holds booleans, which introduces the one trap worth a test: a truthiness check would read a cached false as a miss and re-query billing on every resolution for lapsed organizations. Pinned. * fix(byok): keep an abandoned entitlement producer from writing the cache coalesceLocally does not cancel a producer it timed out — its docstring says so explicitly — so writing the cache from inside the producer let a late billing result overwrite a fresher answer a retry had already cached, and hold it for a full TTL. Move the write onto the value the caller actually received. A caller that timed out throws before reaching it, so an abandoned producer now resolves into nothing. The test reproduces the overwrite and fails against the previous shape. Reported by Cursor Bugbot. --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> |
||
|
|
ea70f8dcf1 |
feat(harmonic): add contact workflow integration (#6902)
* feat(harmonic): add contact workflow integration * fix(harmonic): sync docs manifest * fix(harmonic): address integration review findings * feat(harmonic): add the missing people endpoints and fix two error paths Extends the integration from 4 to 13 tools, covering every non-deprecated people-scoped Harmonic endpoint, and repairs two defects found by validating the existing tools against Harmonic's OpenAPI and API reference. New tools: - Enrich Person (POST /persons) — the only path from a LinkedIn URL or email a workflow already holds to a Harmonic contact. - Get Person, Get Company Employees — account-based sourcing; employees returns URNs that chain into Batch Get People. - Saved-search net-new results and their acknowledgement, so a monitor stops reprocessing the entire result set on every poll. - Bulk email enrichment: submit, poll, and quota, plus Get Enrichment Status. Fixes: - The error extractor dropped Harmonic's string and object `detail` envelopes. A tool that names an extractor gets no fallback chain, so every FastAPI abort surfaced as "Request failed with status 403". The enrichment 404 also carries the scheduled `enrichment_urn`, which was being discarded — that URN is the only handle on the job, so it is now kept in the message. - The saved-search selector failed the whole dropdown instead of degrading: the response cap was half the sibling value on an endpoint that is unpaginated and returns every saved search with its full query object, and the option ceiling threw rather than truncating. Raised to 1MB and switched to truncate-and-warn, matching the other data-driven selectors. Clearing net-new results now requires an explicit scope. Harmonic treats an absent `entity_urns` as "clear everything", so an empty field would have silently discarded the backlog. Scope deliberately excludes company-side, deal, typeahead, network, and Scout streaming endpoints, and every endpoint retiring on 2026-11-05. --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Theodore Li
|
a99f61bee9 |
feat(api): add v2 resource management endpoints (#6900)
* feat(api): add v2 resource management endpoints * fix(cli): gate destructive v2 commands * fix(test): update v2 request-slice count * feat(cli): add shared workspace profiles |
||
|
|
d9cfd7c68e |
improvement(mothership): v0.9 (#6815)
* checkpoint
* Checkpoint
* dot fixes
* Make async tool resume delivery recoverable
* Support split table tools and option recovery
* Harden VFS mutation handling
* feat(platform): platform subagent support — docs corpus VFS, search_docs, account context
Squash of the feat/platform-agent branch (sim side): mounts the Sim docs
corpus in the copilot VFS, wires search_docs and retires the legacy docs
search tools, and syncs the generated tool catalog and trace contracts for
the platform subagent.
* Align Copilot tools and resource handling
* Expand workflow log query support
* checkpoint
* Port desktop-improvements-0 desktop and browser-agent work
* fix(desktop): keep browser-agent input alive through live-SPA re-renders
* Harden workflow sanitization and Slack setup
* feat(desktop): coordinate clicks, caret insertion, and drag for the browser agent
* Revert subagent group eager auto-collapse
* fix(chat): keep sends FIFO across the streaming-to-idle drain gap
* Add the steering backend surface for mid-turn sends
* Sync generated contracts for async subagent orchestration
Pulls the mothership tool catalog (wait_agents / tail_agent / steer_agent /
interrupt_agent), trace spans (chat.async_subagent.*, chat.orchestrate.*), and
trace attributes (copilot.async_subagent.*) into the generated TS contracts.
* Add display titles for the async subagent orchestration tools
wait_agents / tail_agent / steer_agent / interrupt_agent get natural-language
running titles (naming the agent id being waited on, tailed, steered, or
stopped) and a Steering→Steered completed-verb rewrite.
* Show orchestrator-chosen subagent names on agent groups
A subagent_start whose payload data carries a name (the orchestrator's new
name trigger parameter) now labels the agent group with that mission name —
the agent-type icon stays. The name flows through the live stream path, the
turn model (AgentNode.displayName) and its serialize/rebuild round-trip, and
persisted transcripts (PersistedContentBlock.name), so reloads keep the label.
* Improve Copilot error handling and logging
* Backfill the subagent display name from the second start event
The dispatch-time subagent_start fires before the trigger args (and therefore
the name parameter) have streamed; the phase-3 start re-announces the lane with
the name. The block builder was dropping that duplicate wholesale, losing the
name on streaming providers — now it backfills subagentName onto the existing
block instead. (The home turn-model path already reconciled this case.)
* Support Slack bot connection flow
* Harden Copilot error and VFS handling
* Harden VFS resource operations
* Show 'Waiting for the first of N agents' for mode-any waits
The wait_agents title ignored the mode argument, so an any-mode wait over
three agents read 'Waiting for 3 agents' while the model narrated waiting for
the first — contradicting the transcript.
* Collapsed-by-default agent cards with live intent status lines
Subagents now narrate their work through <intent>3-5 words</intent> tags (a
fleet-wide prompt protocol on the mothership side). The turn model streams
each subagent's text through a split-safe tag parser: complete tags update the
agent's currentIntent and disappear from the prose, tags split across deltas
are carried until their close arrives, and a tag that never closes flushes
back as plain text.
The agent card renders as one line — display name (or agent label) plus the
latest intent, replaced inline as the agent shifts gears — and never
auto-expands; expanding to the full tool log is a deliberate click. Only an
outstanding permission prompt or a browser hand-back forces a group open.
Intents persist on the subagent block (and through the legacy persisted-
message paths) so reloads keep the last status, and a renamed reinvocation
now takes the latest name instead of pinning the first.
* Add the internal in-band tool execution route for live mothership turns
POST /api/copilot/tools/execute (INTERNAL_API_SECRET, Go→Sim) runs one
sim-server tool through the same server tool router the resume driver uses
and returns the result synchronously — no checkpoint. This is what lets
background (async) subagents write files/tables/knowledge, and lets the main
lane keep streaming (instead of checkpoint-pausing and killing every
background run) while async agents are live.
* Persist resource side effects for in-band tool execution
Files/tables created through the internal execute route now register on the
chat's resources exactly like the resume driver's executions — the route runs
the same handleResourceSideEffects pass (persistence only; an out-of-band
route has no live event sink, so mid-turn chip pushes are a follow-up).
* Extract intents from group text on every path, sync and async
The turn-model intent filter only fires for span-scoped subagent lanes, but
this surface also delivers subagent text through the legacy block path — so
<intent> tags flowed through unparsed and rendered as prose rows. Groups now
extract intents from their accumulated text at append time: the last complete
tag becomes the card's status line and every complete tag is stripped from
the rendered prose. Covers span-scoped, legacy, and persisted-reload paths
for both synchronous and background delegations.
* Fall back to the live tool title for the agent card status line
Persisted data proved tool-first subagents (grok search agents) emit zero
prose, so intent tags never stream no matter what the prompt says. The
collapsed card now always narrates: the agent's own <intent> tag when present,
else the latest tool's display title while the lane is live.
* Catch subagent <intent> tags in the server relay
The relay's subagent text handler now runs the split-safe intent extraction
as chunks stream: the latest complete tag is stamped onto the lane's persisted
subagent block (subagentIntent) and stripped from the stored prose, so live,
persisted, and replayed views all agree. Per-lane carry handles tags split
across chunks; a never-closing tag flushes back as plain text.
* Drop the tool-title fallback: the status line is the agent's intent
With the intent protocol now injected into every spawn's task message, agents
open with an <intent> tag; the card shows that narration or nothing.
* Replace intents with live tool-title status lines on agent cards
Intent parsing is fully removed (turn model, relay handler, persistence
fields, group extraction). The collapsed card's status is the latest tool
call in its RUNNING phrasing — never the completed rewrite, which stays in
the expanded log. Parallel tools show the most recently started still-running
title with a +N for concurrent siblings; between rounds the last title stays
frozen; a closed lane shows the bare name. Nested agent cards compute their
own status recursively from their own items.
* Keep the main Sim lane live-expanded; collapse only real subagent cards
The mothership group is the turn's own narration, not a delegation card —
collapsing it hid main-lane text and tools until manual expand, which read as
mis-ordered streaming while async subagents interleaved. It keeps the
original live-expand behavior and no status suffix.
* Persist subagent lane lifecycle blocks from the span handler
Lane-scoped span events route to the span handler, which only recorded trace
side effects — no subagent start block was ever persisted (verified: a
seven-agent run stored 104 blocks with zero starts). Grouping then fell back
to keying lane content by agent NAME, so a respawned agent of the same type
merged invisibly into the first one's card until it resolved. The handler now
persists the start block (spanId-keyed and deduped, carrying the display
name) and stamps endedAt on close, giving every invocation its own card.
* Name agents in orchestration titles; '+ n more' overflow format
wait/tail/steer/interrupt titles humanize the slugified agent ids back to
their display names ('Waiting for the first of Digest Workflow Build + 4
more'), and the agent card's parallel-tool suffix uses the same '+ n more'
format.
* Harden in-band tool execution and resources
* Route in-band execution through the comprehensive tool dispatcher
The internal execute route used the bare server-tool router, which rejects
VFS tools with 'Unknown server tool: read/glob/grep' — so nearly every
background agent's first discovery call failed (102 in-band calls in one run,
dozens rejected). It now uses the relay's executeTool dispatcher: registered
handlers (VFS, function execute) with permission checks and param
normalization, falling back to the app tool router — the same surface
foreground execution gets.
* Harden chat stream transition handling
* Harden VFS provenance and resource writes
* Standardize tool environment references
* Harden browser panel and chat cleanup
* Descriptive, user-language tool titles across the board
House rules applied everywhere: use every argument the call carries, never
name internal machinery, and never lead with Getting (the Got rewrite is
deleted so it cannot return).
- Deployments name the workflow: Deploying {workflow} as API/chat app/MCP tool
- Workflow reads name the part: Reading {workflow} meta/state/deployment/notes;
generic reads always name the file (Reading {leaf}), never bare Reading file
- Block runs name block and workflow: Running {block} in {workflow}, Running
from {block} in {workflow}, Running {workflow} until {block}, and
Enabling/Disabling {block} in {workflow}
- The six split-table tools get per-operation verbs (Adding column {name},
Updating rows, Wiring automation, Creating view {name}) instead of a wall
of Querying table
- The manage quartet drops X-action system-speak for gerunds
- get_* internal names become user language (Checking run settings, Tracing
block inputs, Reading the deployed version); web_fetch says Fetching
- Scheduled-task titles removed entirely (feature deleted from the Go catalog)
- New verb rewrites: Fetched, Traced, Wired, Configured, Looked, Rotated
* Deploying {workflow} as chat, not as chat app
* Loader gerunds; mv names both ends; mkdir names the folder
search_integration_tools -> Finding the right integration;
load_integration_tool -> Loading {integration} tools; load_skill ->
Loading skill {name}; run_enrichment -> Looking up {subject}. mv prefers the
model's phrasing, else reads 'Moving {files} to {destination}'; mkdir reads
'Creating folder {name}' from the path.
* Overflow counts read '+ n', dropping 'more'
* Unify workspace find and search
* Scale desktop title bar with page zoom
* Serialize account and organization truth into the copilot VFS
Workspace standing, membership, billing, org role, access-control
restrictions, published-block provenance, and fork topology were reachable
only through three parameterless tools (or not at all). They are ambient
read-only facts, so they belong in the VFS where they are greppable, cost no
tool round-trip, and every agent that can read gets them — the same move that
retired get_blocks_and_tools and list_user_workflows.
Adds account/{workspace,workspaces,members,billing}.json (always mounted) and
organization/{organization,access-control,custom-blocks,forks}.json (only when
the workspace is org-hosted). Every file projects an existing use case or util
after getOrMaterializeVFS's access assert — no new queries, no new
authorization. One relation per file, cross-referenced by id-and-name stub, so
overlapping facts cannot disagree. Volatile content (billing, access control,
forks) is lazy, so numbers are read-time fresh and unasked-for reads cost
nothing.
Projection follows the viewer: member emails are admin-only, fork detail
requires workspace admin on a forking-enabled org, and the whole organization/
namespace is absent for a personal workspace — which is itself the answer.
Retires get_account_billing, get_enterprise_context, and list_user_workspaces
along with their handlers; display titles stay for transcript replay.
* Fix insert_text refusing an editable field focused inside a frame
describeFocusedEditable descended shadow roots but not frames, while
activeElementReadback descends both. Focus inside a same-origin frame therefore
surfaced to the first as the FRAME element — not an input, not contentEditable,
not a canvas, no textbox role — so it fell through to 'not-editable' and
insert_text refused a field that press_key had just typed a character into.
Two functions answering 'what is focused' with different answers is the bug;
the descent loops now match exactly.
The refusal also names what actually held focus (tag, role, contenteditable).
A bare 'not-editable' gave the agent nothing to act on, so it guessed at the
cause — a real run spent twenty rounds on the wrong theory and had to be
stopped by the user.
* Keep retired browser takeover renderable in history
The tool is gone from the catalog, so its generated constant went with it and
every path that referenced it stopped compiling. Deleting those paths instead
would have silently downgraded every past transcript containing a takeover card
to a generic tool row, and dropped the no-timeout budget that an in-flight
takeover still needs while a rolling deploy finishes.
retired-tools.ts gives the literal a documented home that says what it is and
why it survives its tool.
* Follow the agent into a tab it opened to work in
browser_open_tab created the page with activate: false, so the agent worked in
a tab the user could not see while the panel sat on a page where nothing was
happening. The panel now follows a tab the agent deliberately opened.
Scoped to that tool only. A page spawning its own tab (popup, target=_blank) is
the site grabbing the view rather than the agent choosing a workspace, and
stays in the background as before — two existing tests pin that and caught the
first version of this change, which moved both.
A tab the user claimed still wins over both: the work starts in the background
instead of pulling the page out from under them mid-read.
* Make the browser tools agree with each other
An audit of the module found the frame-descent bug was one instance of a
pattern: six independent definitions of 'is this editable' and seven of 'what
is focused', disagreeing with each other. A tool refusing what its sibling
accepts on identical page state is invisible at runtime — the agent follows a
snapshot that says one thing into a tool that says another.
- browser_type now accepts role="textbox" like browser_insert_text does. The
snapshot advertises those elements as [textbox] with a ref, so refusing them
meant rejecting exactly what the outline told the model to type into. Both
the native and synthetic paths, and their descendant scans.
- pressKeyOnPage descends shadow roots and frames like every other focus
reader. It was dispatching synthetic keys at the shadow host or <iframe>
element, where they bubble but never reach the editor, while reporting
success — and contradicting the activeElement reported beside it.
- not-editable and ambiguous-editable name what was found: the element's tag
and role, and the candidate fields. Both had the data and discarded it, which
is what turns one blocked step into twenty rounds of guessing.
- obstructedAfterNavigation requires a dialog that ARRIVED with the
navigation. It compared against nothing, so every SPA route change under a
persistent role=dialog reported a successful click as obstructed. The test
that covered this asserted the false positive; it now pins both directions.
- browser_insert_text observes the top document when typing inside a frame,
like every other input tool. A submit that navigates the top page was
invisible to its frame-scoped observation.
* Let hover actually see what it mounted
Four independent defects made browser_hover blind to the most common thing a
hover produces — a row's action bar — so it reported no effect on a hover that
worked, and the agent fell back to clicking pixels off screenshots.
- The popup scan matched only role=tooltip/menu/listbox. Slack's message
shortcuts bar is a labelled toolbar/group, so it registered as nothing at
all. Added toolbar, menubar, labelled group, and [popover].
- The baseline was captured BEFORE prepareElementSurface scrolled the target
into view, so scrollChanged was always set by the tool's own probe. That
pinned every unproductive hover to 'background DOM churn' instead of the
honest 'nothing happened', and hid scrolling the hover really caused.
Re-baselined once the scroll settles and before the pointer moves.
- The MutationObserver attached only on the first observation, while the roots
list is rebuilt every call and grows as shadow roots mount. Components that
appeared later were never observed, so their DOM changes raised no revision.
Roots are now observed as they show up.
- observationTruncated was computed and never read, so a scan capped at 12k
nodes reported 'nothing appeared' with the same confidence as a complete
one — and portalled overlays live at the end of <body>, exactly what the cap
drops. Hover now says the page was too large to scan and to confirm visually.
* Stop the browser agent acting on the wrong element, and say why it refused
Four findings from the module audit, the first of which could silently do the
wrong thing rather than merely fail.
- A ref whose node is gone is re-adopted by structural resemblance, matching on
ORIGIN only so a pushState between snapshot and act does not kill every ref.
That leniency also let a ref to a row control in one view rebind to the
identical control in a view the app had since navigated to — acting on the
wrong message, signalled by nothing louder than recovered: true. Adoption now
requires the same path; a view swap reports the ref stale, and the caller
re-snapshots. Revalidating a still-connected node stays lenient, because that
is literally the node the model chose.
- A hit INSIDE the requested element is its own nested control, not an overlay.
Both produced 'covered by X — close or move the overlay', advice that cannot
be followed because there is nothing to close. Nested hits now say so and
point at retargeting.
- browser_click_at, browser_insert_text, and browser_drag listed targetChanged
in their effect formulas, but none passes an elementId, so no targetState is
ever captured and the term was always false — coverage that read as real.
Removed, with a test pinning the dependency.
- The seven effect formulas are deliberately NOT collapsed into one predicate:
drag must trust domChanged where others must not, hover must ignore field and
focus changes, click counts focus only for editables. Forcing one would make
each tool wrong differently. The differences are now documented in one place
next to the shared computation, so divergence is a declared policy rather
than an accident.
* Let edit_workflow configure block retries
* Updates
* Always focus the resource the agent is working on, and its browser tab
The resource panel had a carve-out: an already-open browser session declined
to replace another selection and only got an attention marker, so agent
browser work happened off-screen. The panel now follows the agent to whatever
it touches — browser included — and an event can still opt out explicitly.
The browser panel also follows the agent BETWEEN tabs: the store already
tracked automationTabId (and the strip marked it), but the visible tab never
changed. It now switches when the agent's target tab changes, so watching the
agent never means hunting for the tab it moved to. Keyed on the target
changing rather than on it being set, so a user who browses elsewhere
mid-run is only pulled along when the agent itself moves.
* Never paint a browser snapshot at stale geometry (the modal-open flash)
Opening a modal locks scroll, which removes the window scrollbar and reflows
the panel — so a capture taken before the lock describes a rect the panel no
longer occupies. The handshake painted that frame anyway and only then
retried, so the replacement landed visibly offset from the page it stands in
for: the flash. A capture is now checked against the host's live rect before
it is painted; a mismatched frame is skipped and re-captured at the settled
layout instead (modal retries go 2 -> 3 to absorb the extra settle).
* Name the workflow in deployment and workflow-scoped tool titles
'Checked deployment status' never said which workflow — nor did the deployed-
state read, run settings, block outputs/inputs, redeploy, promote, or the
global-variable write. These tools carry a workflowId (often defaulting to the
current workflow), so only the client can resolve a name: the enrichment layer
now resolves it for the whole workflow-scoped family and passes it as
workflowName, which every workflow title already reads.
Titles: Checking {workflow} deployment status, Reading deployed {workflow},
Checking {workflow} run settings, Reading {workflow} block outputs, Tracing
{workflow} block inputs, Redeploying {workflow}, Promoting {workflow} version
{n} to live, and 'Adding workflow variable {name} in {workflow}' — each
falling back to its unnamed form when no workflow resolves.
* Name the block that ran; never fall back to a raw block id
run_block and set_block_enabled carry only a blockId, so their titles would
have printed an opaque UUID ('Running 7f3a2b91-… in Invoice Sync'). The
enrichment layer now resolves blockId against the workflow store the same way
it already did for run_from_block's startBlockId, and the base titles no
longer accept an id as a name — an unresolved block reads 'Running block'
rather than a UUID.
* Add the missing Removing -> Removed rewrite
The table work introduced 'Removing automation'/'Removing enrichment' with no
past form, so those rows kept their present tense after completing.
* Name the target resource in the remaining tool titles
Table tools keep their operands nested under args and identify the table by
id, so their rows said 'Adding rows' with no hint where: enrichment now lifts
the nested args and resolves tableId against the cached workspace table list,
giving 'Adding rows to Runtimes', 'Adding column status in Runtimes',
'Reading views of Runtimes'.
Also: a block-schema read names the block instead of the file ('Loading
Slack', 'Loading Google Sheets tips'); browser type/insert show the text they
send, middle-ellipsized; downloads name the file; library-docs searches name
the library and query; knowledge-base searches include the query; generated
media names its output file; and diff_workflows, list_deployment_versions,
and publish_custom_block joined the workflow-name enrichment set.
* Bubble nested agents' tool calls into the parent's status line
The collapsed status only scanned a group's OWN tool items and skipped nested
agent groups, so a parent that had delegated froze on its last own tool while
its child did the actual work — the line described nothing that was running.
Status now walks the whole subtree: any tool at any depth counts, the most
recently started running one is shown, and the rest become the same '+ n'
overflow. With nothing running it falls back to the last tool at any depth,
so an idle parent still reflects where its subtree got to.
* Align nested tool call status rows
* Revert branch-local KB connector error-message edits
Restores apps/sim/connectors/ to staging state. Two copilot-focused commits
on this branch (
|
||
|
Justin Blumencranz
|
0b71717241 |
perf(icons): reduce and guard SVG path precision (#6839)
* perf(react): reduce SVG path precision * fix(react): preserve Sim wordmark precision * fix(icons): preserve Quartr scale * test(icons): ratchet SVG path precision * fix(icons): make precision exceptions local * perf(icons): enforce three-decimal paths |
||
|
Vikhyath MondretiandClaude Opus 5
|
2ced737976 |
refactor(sub-blocks): make a registered selector the single source for a remote option list, and close the fork-sync reconfiguration gap (#6878)
* fix(workspace-forking): stop double-labelling a custom block's inputs, and derive their controls from the canvas
Two problems with how a repointed custom block's inputs render in the sync modal.
The field title printed twice. The row wrapper already draws the label and its
required marker for every dependent field — `DependentFieldSelector` takes a
`title` only to phrase its placeholder and renders a bare combobox. The
custom-block branch used `ChipModalField`, which owns a label of its own, so every
input showed its name twice. It now renders bare controls like its sibling does.
The control was chosen by re-reading the raw field type instead of asking the
function that already answers this. `subBlockTypeForField` decides what a Start
field becomes on the canvas; the modal had a parallel switch that had already
drifted, rendering a `file[]` input — an upload on the canvas — as a plain text
box, which would write a bare string into a field expecting file references.
`subBlockTypeForField` is now exported and the modal derives from it, so the two
cannot disagree about what a field IS; the modal only decides how that kind draws.
A file input is explicitly `unsupported` rather than falling through: it renders
disabled, saying it is set in the workflow, instead of inviting a value that
cannot work. A test walks every type a Start field can declare and asserts the
modal's choice follows the canvas's, so a type added later surfaces here rather
than silently becoming a text box.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(workspace-forking): resolve a custom block's inputs against the target environment, and stop a re-sync wiping its uploads
A repointed custom block's inputs are configured at sync time, but the modal
drew them as bare text fields against no environment at all:
- `{{SECRET}}` had no completion, and no way to know which secrets exist in the
workspace the value is written INTO.
- `<block.output>` had no completion. The canvas dropdown reads the workflow
open in the editor; on the fork settings page there is none, and the workflow
that matters is the target's.
- A `file[]` input has no control here (it is an upload on the canvas), so it
had no stored override — and the block was rebuilt from overrides alone, so
every sync silently dropped the target's uploaded files.
`WorkflowReferenceScope` lets a surface supply the workflow a reference resolves
against. Absent a provider, the hooks read the live editor stores exactly as
before, so the canvas is unchanged. The scope splits graph from values on
purpose: reachability cannot change with the text being typed, and the
validation hook runs in every reference-aware sub-block editor at once, so
subscribing it to live sub-block values would re-render all of them on every
keystroke. A test pins that split.
`replaceCustomBlockInputs` now seeds from the target block when it is ALREADY
the mapped type, layering the configured values on top. That keeps an input the
modal cannot offer a control for, and leaves a field the user simply did not
touch alone; a field they explicitly emptied stores `''`, which is an override
and still wins. Under a DIFFERENT current type nothing is carried over — those
values are keyed by another block's field ids, which is the orphaning this
function exists to prevent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(workspace-forking): stop a required file input deadlocking Sync
Both PR bots flagged this and they were right. A repointed custom block's
`file[]` input renders as a disabled control — it is an upload on the canvas,
and there is nothing to type here — but the Sync gate still demanded a
non-empty value for every REQUIRED dependent. So a custom block with a required
file input turned Sync off permanently, while the field's own hint told the
user to go set it in a workflow they could only reach BY syncing.
`isForkSyncConfigurableField` is the one predicate for "can the modal put a
value in this field", used by the gate and by the per-kind status badge so the
two cannot disagree. Skipping the gate is only safe because the sync no longer
clears the field: the target keeps what it has, and a genuinely missing value is
still caught by the block's own required-field validation at run/deploy time —
the same fallback every other unconfigured required field already relies on.
Also gives the disabled control an `aria-label` (the row's visible label is a
sibling, not associated), closing the second review note.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(sub-blocks): make a registered selector the single source for a remote option list
`dropdown` and `combobox` could only load a remote list through a per-block
`fetchOptions(blockId)`, which resolves its credential by reading the live
workflow store. That works on the canvas and nowhere else — which is why the
fork sync modal cannot offer those fields, and why every one of those fetchers
turned out to be a hand-rolled duplicate of a selector that already exists
(`triggers/gmail/poller.ts` calls the very contract `gmail.labels` wraps).
Both controls now accept `selectorKey`, resolved through the registry inside
`useFetchedOptions`. Deliberately NOT a second code path: the registry is
presented through the same two function shapes the props already describe, so
the existing lifecycle — request-id guards, dependency-scope reset, label
hydration — is reused verbatim, and paginated selectors drain through the same
`loadAllSelectorOptions` that search/replace and value resolution already use.
`isDynamic` replaces the `fetchOptions &&` test the controls used to decide
whether the fetched list or the static `options` array is authoritative; that
question outlives the prop it was asking about.
No block or trigger changes yet, so nothing moves off `fetchOptions` in this
commit: subblock `type`, `multiSelect`, and the stored value shape are all
untouched and no existing workflow is affected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(triggers): move every credential-scoped option list onto a registered selector
Each of these `fetchOptions` resolved its credential with
`readSubBlockValue(blockId, 'triggerCredentials')` — a live-workflow-store read
— and then called the very selector contract a registered selector already
wraps. They were duplicates that only worked on the canvas.
Migrated: webflow sites/collections (x4 triggers), clickup workspaces, gmail
labels, outlook folders, and all six hubspot pickers. 425 lines of duplicated
fetch logic deleted.
The missing piece each one needed was `canonicalParamId: 'oauthCredential'` on
its credential subblock: `buildSelectorContextFromBlock` keys the context on a
subblock's CANONICAL id, so without it `context.oauthCredential` was never
populated and the block had no way to reach its credential except the store —
which is what forced the hand-rolled fetcher in the first place.
Five new hubspot selectors. `hubspot.pipelineStages` reads the pipelines
contract and narrows, because HubSpot returns stages inside the pipeline
payload rather than behind an endpoint of their own; sharing the one response
is also what keeps a stage list from ever describing a pipeline its sibling
picker is not showing. `objectType`/`customObjectTypeId`/`pipelineId` join
SelectorContext, and `resolveObjectType` keeps HubSpot's own `contact` default
so an untouched dropdown still lists properties for what it visibly shows.
Subblock `type`, `multiSelect`, and stored value shapes are unchanged, so
existing workflows are unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(triggers): move the table trigger's column picker onto table.columns
`fetchTableColumns` resolved the workspace from the active-workflow store and
the table id by reading two subblocks by name, then refetched the table list to
find one table's schema. The registered `table.columns` selector takes both from
the context — `tableSelector`/`manualTableId` already carry
`canonicalParamId: 'tableId'`, so the canonical pair resolves on its own — and
reads the table detail query directly.
Deletes the helper and the four imports it was the only user of.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(managed-agent): move its four pickers onto registered selectors
All four read one route distinguished only by `resource`, with the credential
pulled from the store by name. They are now `managedAgent.agents` / `.vaults` /
`.memoryStores` / `.environments`, and `lib/managed-agents/subblock-options.ts`
is deleted entirely.
The environment filter (cloud vs self_hosted expose different fields, so mixing
them offers choices the rest of the form cannot honour) moves into the selector
with `environmentType` on the context.
Also decouples two things `canonicalParamId` was conflating. It is both a
block's serialized PARAM NAME and the key `buildSelectorContextFromBlock` reads,
so making this block's pickers resolvable appeared to require renaming its
shipped `credential` param to `oauthCredential` — a rename that would change the
serialized shape of every existing managed_agent block, and one that
`blocks.test.ts` correctly refused. A picker should not be able to force a param
rename, so the context now reads a credential off the subblock TYPE when no
canonical id supplied one. It only fills a gap: a block that declares
`canonicalParamId: 'oauthCredential'` has already resolved it, including the
basic/advanced active-member logic the type check cannot express.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(sub-blocks): delete fetchOptions — a sub-block's options are a selector or derived, never both
Completes the migration. `fetchOptions`/`fetchOptionById` are off `SubBlockConfig`,
off both controls, and out of `useFetchedOptions`, leaving exactly two ways a
sub-block gets its options:
selectorKey — a registered selector. The ONLY way to load a remote list.
Parameterized by an explicit SelectorContext, so it works on the
canvas, in the fork sync modal, and anywhere else.
options — a static array, or a pure function of the block's own values.
No I/O.
Reading the remaining callsites showed most of the "derived" ones were nothing of
the kind — they were workspace-scoped remote fetches wearing a local-looking
signature. Those became seven `workspace.*` selectors (credential providers,
credential groups + their per-group providers, secret names, raw secret names,
sandboxes, trigger types) plus `providers.openrouterEmbeddingModels`. Only the
agent block's three capability dropdowns were genuinely derived; `options` now
takes the block's values so they can say so directly. The parameter is optional,
so every existing zero-argument options function is untouched.
`imap.mailboxes` is the one selector whose account is typed rather than stored.
Its password is deliberately absent from the query key: a query key identifies a
resource, a credential authorizes access to it. `oauthCredential` is safe there
because it is only an id — a typed password is a secret, and keys are cached and
surfaced by devtools. Host, port, TLS and username already identify the mailbox
list uniquely; the password rides the body exactly as before.
`selectorExcludeSelf` replaces the one thing a shared `sim.workflows` selector
could not express. It is a declared flag rather than a blanket rule because the
answer differs per field: the Sim trigger never receives events about its own
workflow, while the Logs block legitimately reads the logs of the workflow it
runs in.
Deletes `lib/workflows/subblocks/options.ts` and `triggers/editor-state.ts`
entirely — every caller was a `fetchOptions` resolver. The live-registry test for
the trigger vocabulary moves to the selector that now owns it, keeping its
lazy-import cycle guarantee under test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(workspace-forking): make every fork-clearable sub-block reconfigurable at sync time, and lint that it stays so
`clearDependentsOnRemap` wipes every transitive dependent of a remapped parent,
and a credential mapped between environments changes value on EVERY sync — so a
dependent the sync modal could not offer was re-emptied on every push, with
nowhere to set it that stuck. Setting it in the target did not survive. 36 fields
were in that state.
The selector migration closed most of it; this closes the rest. The collector now
also emits plain text dependents (`short-input` / `long-input`), which need no
selector — just somewhere to type — and the modal's no-selector branch renders
them through the same control it already drew for custom-block inputs. It
deliberately does NOT emit the manual half of a selector-backed canonical pair:
that pair already represents the field once, and its manual member is verbatim by
policy, so offering both would show one concept twice and invite writing into the
inactive half.
`forkDependentControl` replaces the direct `customBlockInputControl` call in the
view, because `fieldType` now means two different things: a custom-block input
declares a Start FIELD type (`string`, `file[]`), while every other no-selector
dependent is a canvas SUB-BLOCK whose own type says it. They agreed by accident
before; now they are classified separately.
`check:fork-dependent-coverage` fails when a sub-block under a
credential/knowledge-base/table anchor is none of: selector-backed, a canonical
pair member, a preserved name-based type, or text. 656 dependents, zero
uncovered, no baseline — verified to fail by seeding a regression. Picked up
automatically by `check:audits` (all 30 green).
Documented in `/add-block`, `/add-trigger`, and `.claude/rules/sim-integrations.md`,
including the two rules the checks enforce: a secret never enters a selector's
query key, and a fork-clearable dependent must be reconfigurable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(sub-blocks): stop selector-backed fields rendering undefined options, and pass derived values to ComboBox
Both found by Bugbot on the migration commit; both real, both mine.
A selector-backed field carries no static `options` — that is the point — but
`Dropdown` and `ComboBox` still read it on first paint, before any fetch resolves,
and `allOptions.map(...)` is unconditional. Every field moved to `selectorKey`
(Function sandboxes, Managed Agent pickers, OpenRouter embeddings, Logs
workflows, the migrated triggers) would throw on mount. The type said the prop was
required, so nothing caught it: the callsites pass `config.options`, which is
optional on `SubBlockConfig` and now genuinely absent.
Fixed on the controls rather than by restoring `options: []` to every migrated
sub-block: the absence is correct, so the component owns the default. `options`
is optional on both prop types and falls back to a shared empty array, which also
keeps a stable identity for the memo.
`ComboBox` never got the `options({ values })` wiring `Dropdown` received, so
agent's reasoning-effort, verbosity and thinking-level lists — all comboboxes —
silently stayed on their generic fallback instead of narrowing to the selected
model. Wired the same way, reading the block's own values from the store.
`selector-backed-subblocks.test.ts` pins the invariants against the real registry:
a named selector exists and can list, a selector-backed field never also declares
static options, and a field whose selector is gated on context declares the
`dependsOn` that rebuilds it. That last one immediately caught a third bug —
`clickup.triggerWorkspaceId` had no `dependsOn`, so its list would have loaded
once, empty, and never refetched once a credential was picked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(selectors): restore the credential-group provider label resolver, and probe getQueryKey for missing dependsOn
Two findings from a final adversarial pass over the migration, both the same
class as the three the review bots caught: something a `fetchOptions` sub-block
declared that its replacement selector quietly does not.
`credential-group.providerFilter` had a `fetchOptionById`;
`workspace.credentialGroupProviders` had no `fetchById`, so the canvas card
summarising several stored provider ids lost every label. The field is
multi-select, which is exactly when a label has to resolve without the full list.
The `dependsOn` assertion in `selector-backed-subblocks.test.ts` only probed
`enabled` against three hand-listed context fields, which is why it caught
`clickup.triggerWorkspaceId` and would have missed the rest. It now probes
`getQueryKey` as well — a selector's key names every context field its RESULT
depends on — and derives the sub-block-sourced set from
`SELECTOR_CONTEXT_FIELDS` rather than a literal. Verified by deleting a real
`dependsOn`: it fails naming the field and the fields it depends on.
Also checked and NOT changed: `display.ts` and the copilot dropdown validator
both guard `options` before use, so stripping `options: []` does not reach them.
The validator's behaviour does shift from "reject every value" (an empty
`validIds` array matched nothing) to "skip validation", which is a relaxation
rather than a regression. `function.sandboxId` kept its `dependsOn: ['language']`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: re-record the page-graph baseline after staging's growth consumed its allowance
CI's "Repo audits" step failed on `check:tool-registry-boundary`. Measured before
touching anything, because the reported growth (+32 and +42 modules on two
routes) looked like this branch had dragged the selector registry somewhere new.
It had not. Recording a baseline on clean `origin/staging` and diffing against
this branch attributes the growth precisely:
this branch: +1 to +4 modules per route, +35 total across 25 routes
staging: the rest
Staging's six merged commits landed both failing routes at exactly their
tolerance — knowledge/[id] at +31 of an allowed +31, layout at +41 of +41 — so
`check:tool-registry-boundary` passed there with nothing left over. This branch's
+1 tipped both past the line. The next PR to touch anything would have tripped it
just the same, whatever it contained.
The +1..+4 is the selector consolidation's real cost: `selectorRegistry` is one
static object, so a page reaching any selector reaches every provider, and this
branch adds four (hubspot, managed-agent, imap, workspace). That is the same cost
the 27 existing providers already impose, and it is what buys one option-list
mechanism that works off the canvas.
Also tried deferring the workspace provider's data-layer imports to fetch time.
Reverted: this checker follows dynamic imports, so the numbers did not move,
leaving only a Promise.all-of-imports shape that reads worse than the 27 sibling
providers it sits next to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(workspace-forking): actually apply text dependents, and resolve labels against the full selector context
Both from Bugbot; both real, both mine.
**Text dependents never persisted.** `applyDependentOverrides` allowlisted
`dependsOn && selectorKey`, so the plain text fields the collector started
emitting were offered in the modal, stored, and gated on by the Sync button —
then dropped on apply. The field stayed wiped on every push and the typed value
went nowhere, which is the exact treadmill the feature existed to end.
The cause was the rule being written twice. `reconfigurableDependentIds` is now
the single definition of "a dependent the modal can offer AND the sync can write
back", used by the collector and by the apply side. A test asserts the two agree
by round-tripping through `applyDependentOverrides`, and fails against the old
allowlist.
**Provider labels stayed raw ids.** `useDynamicSubBlockOptionDisplayName` called
`fetchById` with a `workspaceId`-only context, which silently fails any selector
scoped by a sibling — `workspace.credentialGroupProviders` needs the group before
it can name a provider, so the `fetchById` restored last round returned null
every time. It now builds the block's real context with
`buildSelectorContextFromBlock`, the same one the canvas uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(queries): scope the sub-block label cache by the selector's own context
Follow-on to
|
||
|
|
e3a4874ece |
feat(integrations): add Bitbucket Cloud (#6860)
* feat(integrations): add Bitbucket Cloud * fix(bitbucket): enforce selector workspace slugs * fix(bitbucket): overfetch small pipeline log tails * fix(bitbucket): harden provider edge cases * fix(bitbucket): accept provider diff redirect specs * fix(bitbucket): stop advanced-field leakage and harden log, status, and selector paths Splits the `closeSourceBranch` advanced subBlock into per-operation ids. Advanced fields serialize without evaluating their condition, so a value set on Create Pull Request reached Merge Pull Request and closed the source branch unprompted. Also: - read step logs through the byte-capped server transport and map an empty-log 416 to an empty result, keeping a genuine 416 an error - trim a step log's partial leading line after the character cap rather than before, and never return an empty log when the retained window held content - surface Bitbucket's `error.detail` alongside `error.message` - treat commit-status `key`/`state` as nullable so one malformed row cannot drop a page - match repository `full_name` case-insensitively and reject dot segments in a workspace slug before the outbound request - type `reviewerAccountIds` as the comma-separated string it is - trim optional Bitbucket query strings; correct the token lifetime to two hours --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Theodore Li
|
1372977d07 |
feat(setup): publish standalone self-hosting package (#6849)
* feat(setup): publish standalone self-hosting package * fix(setup): refresh discovered compose installs * improvement(setup): unify repository command * fix(setup): harden standalone package launch * Update README.md * fix(setup): isolate standalone compose installs * fix(setup): restore default stopped installs |
||
|
Vikhyath Mondreti
|
483ff12b6f |
feat(billing): align enterprise reporting periods (#6851)
* feat(billing): align enterprise reporting periods * fix(billing): harden enterprise reporting flow * fix(outbox): preserve handler compatibility * fix(billing): bound enterprise provisioning reads |
||
|
Waleed
|
4f9d5f33b0 |
improvement(search): search every folder, and document real API error bodies (#6861)
* improvement(search): search every folder, and document real API error bodies Search on Files, Tables, and Knowledge was ANDed with the open folder, so a query only ever matched that folder's direct children — and the query was not cleared when you entered a folder, filtering the folder you just opened down to the same matches. A non-empty query now searches the whole workspace, a Location column names each result's folder, and opening a folder ends the search. Also gives GET /api/v2/files a `recursive` flag, and replaces the single shared OpenAPI error example — which showed `BAD_REQUEST` under every status tab — with one real body per status. * fix(search): discard the search term on clear instead of masking it `useSearchFilterValue` returned the debounced term whenever the input was non-empty, so clearing only hid the settled needle. The mask lifted on the next keystroke while the debounce still held the pre-clear term — opening a folder and typing within the window searched the whole workspace for the query the user had just abandoned. A clear now resets the settled term rather than hiding it, adjusted during render so the reset is visible to the render that follows the clear. The initial state is seeded from the first value so a deep-linked `?search=` still filters on the first render. |
||
|
|
681a8ec427 |
feat(resources): empty-state graphics for knowledge, tables, logs, and files (#6828)
* feat(resources): empty-state graphics for knowledge, tables, logs, files, skills Four of the resource pages (knowledge, tables, logs, files) had no empty state at all — `Resource.Table` painted column headers over a blank scroll area and stopped there. Skills had a `: null` branch for zero data. Adds a graphic per resource, drawn in the editor vignette's recipe: take the product's own primitives, shrink them, strip the content to skeletons, and let the composition bleed off the frame edges. - Knowledge — a document fanning into the chunks it is embedded as, using the editor's 6px smooth-step connector language in --workflow-edge - Tables — a sheet of cells running off two edges with one cell in an edit ring - Logs — runs stacked newest-first, their trace spans staggered into a waterfall - Files — a folder held open with one file still above its dashed landing slot - Skills — a skill card opened far enough to show the tools bundled inside `Resource.Table` gains a sanctioned `emptyState` slot rendered below the column headers when `rows` is empty, so the chrome guarantee still holds. Each page shows the graphic only for true zero-data — never for a search or filter that matched nothing, never inside an empty subfolder, and (logs) never before the first page of runs lands. Also ports the shared `EmptyState` frame from the editor branch so this branch stands alone, and adds a review-only /empty-states-preview gallery route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * improvement(tables): redraw the empty-state graphic in the house grayscale Matches the workflow editor's vignette and the landing feature graphics, which between them use no brand colour at all — every one of them is built from neutral tokens. Two corrections: - The blue edit ring is gone. Nothing in the reference graphics carries a hue, and it was the loudest element on the page. - `--surface-4`/`--surface-5` are near-white in light mode (#f5f5f5/#f3f3f3), so skeleton geometry built on them dissolved on a white card. Bars now mix `--text-secondary` into transparent at graded strengths — a real mid-grey that inverts with the theme, which is the idiom the editor vignette already uses for the one bar it needs you to see. Also drops the full-composition mask. The editor vignette keeps its block fully opaque and fades only the connector strokes leaving the frame; masking everything is what made the miniature read washed rather than deliberate. The card is crisp now and the continuation is drawn the way a real table draws it — an overflow fade at the edge the columns run off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * improvement(tables): strip the empty-state graphic to ruled lines and a corner fade Minimal pass. The card is gone — no border, no fill, no header shading, no type squares. What is left is the grid itself: hairline rules in `--border-1`, ink bars at two strengths, and the one cell held in an edit ring. With no card fill the grid sits directly on the page, so it can dissolve into the background instead of ending at a border. The fade is the landing page's own idiom — two gradients intersected (`mask-composite: intersect`), crisp at the top-left and gone through the bottom-right, the same construction `workflow-graph-preview` uses. Two placement notes: - The grid is offset right of frame centre. A diagonal dissolve puts the visual mass toward its opaque corner, so centring the geometry would leave the graphic reading left of the copy beneath it. - The selected cell sits in the quadrant the fade leaves fully opaque. A selection ring dissolving mid-stroke reads as a rendering fault, not a detail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * improvement(tables): make the selected cell opaque and a shade darker The ring mixed `--text-secondary` into `transparent`, so the grid rules running underneath showed through its own stroke. Mixing into `--bg` instead holds the same apparent value while staying opaque, and still inverts with the theme. Raised 32% -> 46% so it reads as chrome rather than more content, and added a stacking context: neighbouring cells are later siblings, so their rules were painting over the ring's right and bottom edges. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * improvement(tables): round the empty-state grid's crisp corner 6px on the top-left only — the one corner the fade leaves intact, and the same radius the workflow editor's vignette uses. The other three dissolve, so there is nothing there to round. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(knowledge,tables): redraw knowledge's empty state and add docs/create chips Knowledge gets the same treatment tables just went through: no brand colour (the `--brand-knowledge` accent is gone), no card chrome, ink mixed from `--text-secondary`, and the landing page's intersected corner fade. The graphic is a document and the chunks it is embedded as. Its fade is held back further than the tables grid on both axes — the document has to stay whole for the graphic to mean anything, so only the chunk grid may trail off. The three chunks the edges actually land on are the only filled ones; filling the whole first column left a chunk with no edge feeding it. Both empty states now carry two chips in the frame's action slot — a docs link and the create action, each running the same handler as the header's primary chip and inheriting its disabled state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * review(knowledge): three candidate depictions, and lead with the create chip Chip order swapped on both empty states — the primary action reads first, the docs link second. Adds a review-only `knowledge-alternates.tsx` rendered in the preview gallery, because the document-to-chunks graphic is not landing. Three directions: - A. the embedding mesh — the landing hero's own knowledge-base panel already draws a base this way (`stage-kb.tsx`), so this is the house depiction rather than a new invention - B. a stack of documents — the most literal reading, at the cost of colliding with what the files empty state wants to draw - C. a query and the passages that answered it — depicts what a base is for, which is what the description copy actually promises Delete this file and the gallery entries once one is chosen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(knowledge): draw the empty state as an isometric set of volumes Replaces the document-to-chunks diagram, which read as a workflow graph rather than as a knowledge base. Built on the landing page's iso-illustration recipe rather than a new one: `ISO_STROKE` contours (`--text-subtle` mixed toward `--text-muted`) at the shared 3.2 stroke width, faces filled from the three-tier surface ramp brightest-on-top, round caps and joins. Geometry is authored in a large unit space so that 3.2 lands as a hairline once scaled to empty-state size — the same reason the landing marks draw 3.2 into a ~526-unit viewBox. The projection and faces are computed rather than hand-authored as path data, so the volumes stay coherent when the geometry is retuned. No corner fade here. The fade belongs to repeating structures that mean the same thing cropped — the tables grid keeps its meaning with two columns or four. A discrete object does not, which is also why the workflow editor's vignette keeps its block fully opaque. Drops the three candidate depictions now that the direction is settled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * improvement(knowledge): brand the front volume instead of laying a page beside it Drops the loose page on the ground and puts the knowledge-base mark on the front volume's cover — the same `Database` glyph the sidebar and the page header use, so the empty state names its own resource. The mark is laid into the cover's plane rather than drawn over it. The cover is the face at max x, spanned by the volume's depth across and its height up; walking those two edges gives the face's basis vectors in projected space, and an affine matrix built from them maps flat artwork into the face. So the glyph skews with the isometric, and because both vectors derive from the box, retuning the volumes carries the mark with them instead of stranding hand-fitted path data. Its stroke is pre-divided by the same factor the matrix scales by, so the glyph's contours land at the volumes' weight rather than four times it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * improvement(knowledge): bore through the front volume, and fade the set back Replaces the mark on the cover with a hole through it. The bore is authored as a plain circle in the cover's own plane and skewed into an ellipse by the face matrix. Its far mouth is the same circle stepped back through the volume: boring straight back is a world step of `-w` along x, and solving the cover-plane matrix for the local offset that produces it gives `(+w, -w)`. The sliver of near mouth the far mouth fails to cover is exactly the wall you see down the hole, so the depth falls out of the geometry rather than being drawn by hand. Down the hole the near mouth is floored in a tone darker than any outer face — the wall turns away from the light — and the far mouth is painted in the cover tone of the volume standing behind it, because looking through a hole in the front volume lands on that volume's face, not on the page. Corners stay square. Rounding was tried and reverted: rounding each face separately notches every corner where three faces meet, and rounding the silhouette instead cost a clip per volume for a softness the set did not want. The tables grid's corner fade is applied along the other diagonal. There it dissolves toward the bottom-right because a grid keeps its meaning cropped; here the set recedes up and to the left and the front volume carries the bore, so anchoring at the bottom-right eats into the back of the stack and reads as more volumes behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(resources): logs and files graphics, and settle the set as one collection Logs is an activity feed — newest run lifted onto its own card, older ones settling behind it. The relative stamps are the only literal text in any of these graphics; everything else stays skeleton, so nothing here has to be translated or kept true. Files is a folder with sheets standing proud of its front panel. Depth comes from the surface ramp rather than shadow, which would need separate light and dark recipes where the ramp inverts on its own. The tab's diagonal is filleted at both ends and every outer corner shares one radius — mixing radii, or running the diagonal into square junctions, made the corners fight at this size. Consistency pass across the set: - Titles are the resource name alone. "No tables yet" earned nothing the description does not already say. - The knowledge mark is mirrored so its bore faces left. Rebuilt on the geometry rather than flipped, since a flip would have put the shading on the wrong side. Its contours are thinned and mixed toward `--border-1`: the landing marks are the focal art of their section, but this one sits beside a ruled grid whose lines are 1px, and full-weight contours read as ink next to it. - The logs feed is sized to the same ~148px footprint as the rest. The frame centres graphic and copy together, so a taller graphic pushes its title out of line with the others' and the set stops reading as one thing. - Every empty state carries its create action and a docs link, each running the same handler as the header's primary chip. - Fades run whichever way the subject recedes: the tables grid to the bottom-right, the knowledge set up and right, the logs feed down, the folder up. Fixes a duplicate React key in the knowledge mark — the volumes stack along y now, so keying on `box.x` gave every one of them `0`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(skills): drop the skills empty state Removed at request. The skills list goes back to rendering nothing for zero data, which is what it did before this branch. Takes `vignette.tsx` with it — the shared stage and skeleton bar were left over from the first pass, and skills was the last thing still importing them once the other four graphics were redrawn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * cleanup(resources): fix empty-state flashes and share the iso ramp Drops the review-only preview route and gallery, which the branch always meant to delete before merging. Three ways the zero-data graphic painted over a workspace that has content: - The gate read the instant URL search term while `rows` is filtered by the debounced one, so clearing a search that matched nothing showed the full "you have nothing yet" state for one debounce window. - Nothing gated on the list still loading. Knowledge and tables hydrate from a server prefetch that is allowed to seed nothing, and the files list deliberately seeds nothing above 300 rows — so the emptiest-looking screen was shown to the fullest workspaces. - The filters are part of the query key and every list keeps the previous key's data, so `isLoading` is false across a filter change. Only the placeholder gate suppresses the graphic during that refetch. Also folds the re-declared isometric fills and stroke back onto the shared `iso-illustration-style` source they were copied from, so a change to the iso ramp reaches this mark too; only the stroke width still diverges. The static face paths move to module scope, the bore interior becomes a named component so its note is TSDoc rather than a JSX comment, and the four identical docs chips become one. * simplify(resources): reuse the iso recipe and let the frame own its layout Hides the empty-state graphic behind `error` as well. A failed load also leaves `rows` empty, and inviting someone to create their first item is the wrong answer to a request that did not complete — all four pages only logged the error, so the zero-data copy was what a failed load actually rendered. `iso-illustration-style` moves out of the landing route group to `components/iso/`. Importing it from a workspace route was the only workspace-to-landing edge in the app, one directory away from an `iso-marks` barrel that pulls ~10KB gzipped of illustration components — a hazard for whoever needs the second constant. The contour recipe is now shared too: `createIsoLineProps` takes an optional stroke width, so the knowledge mark stops re-declaring it and only its weight diverges. `EmptyState` owns the action row's layout, so the three pages with two chips drop their wrapper div and every empty state's chips sit identically. Its unused `className` prop goes with them. Also drops the `height` prop that had one caller passing its default, and the `CORNER` constant that promised single-sourcing the path's four bare literals did not honour. * simplify(resources): decide list emptiness in one place "This list holds nothing" was derived in four pages, each with the same seven clauses under the same nine-line comment. Adding the `error` gate one commit ago took four identical edits, and the skills empty state that was reverted off this branch would have made it five copies. `isResourceListEmpty` now owns the rule and the reasoning behind each gate. Logs omits the folder argument because it has no folder navigation; the other three pass theirs. `Resource.Table` also wraps the slot in its own growth box, so the empty state centres because the table says so rather than because the node handed to it happened to carry `flex-1`. * chore(audits): re-record the page module-graph baseline The four empty-state graphics and the shared frame add **+10 modules** to each of the five routes that render them — measured against `origin/staging`, not against the recorded baseline: files/[fileId] 1958 -> 1968 files 1958 -> 1968 knowledge 2167 -> 2177 logs 1727 -> 1737 tables 1817 -> 1827 The baseline itself was last recorded in #6697, and staging has drifted up to +29 on tables since — inside the max(25, 2%) tolerance on its own, but close enough that this +10 tipped it over. So the failure was the stale baseline meeting a small real addition, not a heavy import. The other 29 entries move only by that accumulated drift. The graphics stay eagerly imported on purpose: an empty state is the first thing a new workspace paints, and deferring ~4KB gzipped behind a chunk request would trade a shared, already-fetched module for a visible pop on the one screen where the product has to look like it works. * fix(resources): hold the empty state until folders resolve Folder rows share the list with resource rows, so a workspace whose only contents are folders has an empty `rows` until the folder tree lands — and got the "create your first item" graphic in the gap. The resource list's own loading gates never covered it because the folder tree is a separate query. `useFolderNavigation` already exposes `foldersResolved` (`isSuccess && !isPlaceholderData`) for exactly this hazard — it guards the ancestry index against evicting a folder id it has not loaded yet. Knowledge and tables pass it straight through; files reads the same two flags off `useWorkspaceFileFolders`, which it calls directly. Logs omits it, as it has no folders. --------- Co-authored-by: andresdjasso <andresdjasso@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Vikhyath MondretiandClaude Opus 5
|
521348b529 |
feat(secrets): record which secrets each run resolves, and surface it per secret (#6823)
* feat(secrets): record which secrets each run resolves and surface it per secret
Redaction stops a value at a boundary but cannot stop code that never emits it —
a Function block can print a key one character at a time and nothing ever matches
the secret. That is undecidable in general, so this adds the other half of the
posture: attribution.
Every run now records which configured secrets it actually resolved, under whose
identity, through which surface (workflow, Sim agent, MCP). The data already
existed in ResolvedSecretTraceRegistry.addActiveEntry and was persisted only for
paused runs; this persists it for every terminal path.
Execution logs cannot answer this. They store the whole available encrypted
environment rather than what a run referenced, they evidence a secret only where
value-matching redaction happened to fire, and they expire under
logRetentionHours — while "who has touched this key" outlives any single run.
- secret_usage: per-UTC-day rollup keyed by workspace, secret, scope, owner,
source, workflow, actor. A one-minute schedule touching three secrets would
otherwise write thousands of rows a day, which is also why this is not
audit_log. workflow_id/actor_user_id use '' sentinels rather than null so the
unique key works on Postgres 14 without NULLS NOT DISTINCT, and are not FKs:
they are historical facts, and an onDelete would rewrite a key column.
- secret_owner_user_id is part of the key. Two people can hold a personal secret
under one name and a shared personal secret resolves for a caller who does not
own it, so name and scope alone do not identify a secret. It is NOT the actor:
a scheduled run resolves the workflow owner's personal slice under the
workspace's execution actor.
- Direct environment reads are now detected in JS (TypeScript AST), Python
(tokenizer-checked) and shell (quote/heredoc-scanned), so a secret read as
environmentVariables['K'] or $K enters the run's provenance instead of going
unredacted. Each detector prescans for names that are actually configured
secrets before paying for a lex or quote-frame pass.
- Copilot integration tool calls are covered: resolveCopilotEnvReferences
substitutes {{SECRET}} into user-only params, which is a real use.
- See usage lives behind a credential-admin gate, using the same predicate that
reveals the value; members get a disabled chip explaining why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(audit): register the secret-usage route in the validation baseline
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): keep rollup metadata with its run, and stop shadowed bindings faking usage
Review round 1.
- record.ts: last_execution_id/last_trigger were assigned unconditionally while
last_used_at was chosen by greatest(), so two runs completing out of order split
one row between them — the newer run's timestamp beside the older run's execution
id, making "View log" open a run the row does not describe. Both are now guarded
on the timestamp actually advancing, so the row's metadata always belongs to the
run that owns its timestamp.
- javascript.ts: a local binding named environmentVariables (declaration, parameter,
destructured binding, or bare reassignment) made reads off the user's own object
look like mounted-secret reads. Any such binding now disables detection for the
file; the AST already had parent pointers, so this is a kind check during the
existing walk.
- python.ts: same class of bug with no parser available, so the rule is an allowlist
— every mention of the binding must be a literal subscript or .get(), otherwise
detection is off for the file. This also subsumes the cross-line attribute case
(other.\n environmentVariables['K']), which the previous space-and-tab look-behind
missed.
Under-reporting is the safe direction here: a trail that claims a use that never
happened is worse than one that misses a use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(db): format the generated migration snapshot
CI runs lint:check across every workspace; the drizzle-kit output in packages/db
had never been through biome, so the branch was green locally (where lint had
only been run inside apps/sim) and red on CI. Whitespace only — both files are
byte-for-byte identical once parsed, and drizzle-kit still reports no pending
schema diff against the reformatted snapshot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): detect every rebinding of the environment identifier, not just declarations
Review round 2. A bare `for (environmentVariables of rows)` has no declaration to
key off, so the previous check missed it and reads of the loop value were still
recorded as secret usage.
Rather than extend the hand-rolled node-kind list, this reuses the pair the same
file already applies to reject a placeholder in a write position:
isDeclarationIdentifier covers declarations, parameters, destructured bindings and
imports, and isWriteIdentifier covers every assignment operator, ++/--,
destructuring targets, and for-in / for-of initializers.
That also closes four forms neither the review nor the original check named:
logical (||=) and nullish (??=) assignment, and object and array destructuring
assignment. Six of the eight added cases fail against the previous check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): apply the rebinding rule to shell, and say when a run's log is gone
Review round 3, plus the docs that were left claiming the old behavior.
- shell.ts: a script that writes a configured name (API_KEY=local, export/local/
readonly, read, for, unset) expands its own value from that point on, not the
mounted secret, so recording it claimed a use that never happened. Every mention
of the name must now be a `$NAME` / `${NAME}` expansion, matching the allowlist
shape the Python detector already uses. Applied per name rather than per file:
JavaScript and Python shadow one object holding every secret, whereas rebinding
one shell variable says nothing about the rest.
- The usage trail deliberately outlives execution logs, so a row routinely names a
run whose log has been pruned. The read now left-joins workflow_execution_logs on
its unique execution_id and reports availability, and the panel renders the chip
disabled with the platform tooltip instead of linking into an empty Logs view.
Three states: no run to link, a run whose log is gone, and a live link.
- Docs said a direct environmentVariables/$KEY read does not activate masking,
which this branch changes. Corrected in credentials.mdx, function.mdx and the
logging FAQ, and the recognition limits are now written down: runtime-built
names, reassigned bindings, and reads that cannot be told apart from text.
Added a "See usage" section covering who can see it and why an empty trail
means "nothing recognized" rather than "never used".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): writing a name is not reading it, and a bare mention is not a rebinding
Review round 4.
- javascript.ts / python.ts: `environmentVariables.API_KEY = 'x'` and
`delete environmentVariables.API_KEY` touch the name without ever reading the
mounted value, but the detectors matched the member access and recorded a use
that never happened. JavaScript now asks the same isWriteIdentifier the
placeholder rewriter uses (its parameter is widened to ts.Node — the body
already walked generic nodes, so this is a type change, not a behaviour one)
plus a delete check; Python excludes a subscript followed by `=` and a `del`
target.
- shell.ts: requiring every mention of a name to be an expansion also fired on
text that binds nothing — a comment naming the key, or `echo "API_KEY=$API_KEY"`
where the literal is an argument rather than an assignment — and dropping those
cost masking on a genuine read. It now looks for actual writes: an assignment at
command-word position, a binding builtin, `printf -v`, or a `for` target.
The two directions are not symmetric, which is why this errs toward detecting
the read: missing a write records a use of a secret the script only had in its
environment, a misleading audit row and nothing more, since masking still
searches for the real value and will not find it. Over-detecting a write
suppresses masking on a value that does reach the log.
This also makes the code match what the docs already described — skipping after
a rebinding, not after any mention.
13 tests added; 11 fail against the previous code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): an update reads before it stores, and a del target may be parenthesized
Review round 5. The first of these is a regression from round 4.
- javascript.ts: reusing isWriteIdentifier to answer "is this a read" was wrong.
That predicate answers the rewriter's question — is this a target the
substitution must refuse — so it treats every assignment operator alike, which
is correct there and wrong here: `+=`, `||=`, `??=`, `++` and `--` all load the
current value before storing, so they are genuine reads and were silently
losing their masking. Only a plain `=` stores without reading. Replaced with a
purpose-named predicate, and isWriteIdentifier's parameter is narrowed back to
ts.Identifier now that nothing else needs it widened.
A test committed last round asserted the wrong behaviour for `+=`; it has been
corrected rather than left to pin the bug.
- python.ts: `del (environmentVariables['K'])` slipped past a check that looked
only at the characters immediately before the match. It now isolates the
enclosing logical line and tests whether that is a del statement, which also
covers `del((x))`, `del(x)`, `del a, x`, and a del after a semicolon.
12 tests added or corrected; 10 fail against the previous code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): stop excluding Python writes, which kept leaking in the unsafe direction
Review round 6. Greptile found that `del environmentVariables[environmentVariables['K']]`
had its inner access — which computes a key, so it is a genuine read — skipped
along with the delete, leaving that value unmasked.
The narrow fix was another textual rule. Instead this removes the write and delete
exclusions from the Python detector entirely, because they were optimizing the
wrong direction.
`resolvedSecretNames` feeds `outputSecretMatcher`, an exact-value matcher over the
output. Naming a secret the code never read costs nothing there: the matcher scans
for a value that does not appear. Failing to name one that was read leaves it
unmasked. The two error directions are therefore not comparable, and the
exclusions bought only audit-trail tidiness while every heuristic they needed has
so far leaked into the dangerous side — first a parenthesized target, now a nested
read. A `del` or an assignment is reported like any other access.
JavaScript keeps its exclusion: a real AST answers the question per node, with no
text to misread, and it has produced no such hole.
Net 30 lines removed from python.ts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): report recognized reads instead of proving they are not reads
Review round 7. Greptile flagged both directions at once — false usage from
reporting a write target, and unmasked secrets from the file-wide shadow flag —
so I traced what the signal actually drives before choosing.
The chain: the compiler's names feed outputSecretPlaintextsByName and the
exact-value matcher, NOT context.resolvedSecretNames, which starts empty. After
execution activateOutputSecretProvenance scans the output and adds only names
whose plaintext actually appeared; those become __resolvedSecretNames, which
tools/index.ts turns into recordResolved calls, which is what the usage trail
reads.
So a compile-time false positive produces no usage row on the ordinary path — it
only hands the matcher a value the code never emits. It does produce one on the
!projection.safe fallback, where the system already over-approximates by design.
A false negative, by contrast, keeps the value out of the matcher entirely, so a
genuinely read secret is never masked on any path.
That asymmetry decides it, so every "prove this is not a read" mechanism is gone:
- javascript.ts: the file-wide shadow flag. A helper declaring its own
environmentVariables discarded genuine reads of the mounted binding everywhere
else in the file — Greptile's security finding, and real.
- python.ts: the allowlist requiring every mention to be a subscript or .get().
Same hole: passing the dict to a function suppressed unrelated reads.
- shell.ts: the rebinding check. It had the same hole in a form nobody flagged —
`echo "$API_KEY"; API_KEY=local` dropped the first read, which is of the real
secret.
What stays is the question of whether the text is code at all — strings, comments,
single quotes, quoted heredocs — plus the receiver check that `other.environment
Variables['K']` is a different object, and JavaScript's node-precise write/delete
exclusion, which cannot suppress a read elsewhere.
Net 215 lines removed across the three detectors and their tests. Docs updated:
the rule is now stated as reporting rather than proving, and that See usage may
occasionally list a secret the code had available but did not read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(secrets): drop the last write-vs-read special case
`environmentVariables` is a plain object deserialized from the run payload
(route.ts:206), not a handle on the stored secret. Assigning to it changes
nothing outside the sandbox and is discarded when the run ends, so separating a
write from a read bought almost nothing while leaving JavaScript as the one
language still trying to prove a read is not a read.
Every language now follows the same rule: report a recognized read of a
configured secret name. The only exclusions left are facts rather than
inferences — the text is not executable (string, comment, single quote, quoted
heredoc), the receiver is a different object, or the name is not statically
knowable.
Docs note that assigning to the binding does not edit the secret.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(secrets): ship only the fields the trail actually shows
Five fields crossed the API and reached no reader: usageDate, firstUsedAt,
actorEmail, workflowId and actorUserId. The panel renders the timestamp, the
trigger, what used the secret, the actor's name, the run count and the run link;
everything else was projected, serialized and discarded.
first_used_at is dropped from the table as well. Nothing read it, and inside a
per-day bucket "first used that day" says nothing next to "last used that day" —
so it was a column written on every run for no question anyone asks. The upsert
loses its least() with it. Migration regenerated; the identifier columns behind
the joins stay, they simply are not returned.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): report referenced code secrets, not only ones that surface in output
The Function route activated a secret's provenance — and therefore its usage row
and downstream masking — only when the exact value appeared in the result,
stdout, or error. That gate made the trail miss silent use entirely: a key that
authenticates an API call and is never echoed reported nothing, and so did the
founding scenario of this feature, a key exfiltrated character by character. The
innocent run that echoed a key got a row; the run worth catching did not.
Activation now follows the referenced set the compiler already computes: resolved
{{KEY}} bindings plus recognized direct reads, filtered to configured values —
the same set the unsafe-projection fallback already activated. An extra name only
hands the output matcher a value that never appears; configured-but-unreferenced
values are still never included. The output-scan activation path and its surface
helper are deleted rather than kept alongside.
One old test pinned the gate ("does not activate a referenced secret that does
not cross the Function result"); it now asserts the reverse, with the reasoning
attached. Two new tests pin the char-split exfiltration and the silent API-call
case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): shell escaping is backslash parity, not adjacency
Review round 8. `\\$API_KEY` is an escaped backslash followed by a LIVE expansion
— bash prints `\` plus the value — while `\$API_KEY` is an escaped dollar and
stays literal. Checking only the character adjacent to `$` read every even run as
escaped, dropping a real read from usage and masking alike; verified against
bash before fixing.
The scanner now counts the run of backslashes before the `$` and skips only odd
runs, the same parity rule logicalLineEndAfterContinuations in this file already
applies to line continuations. Six-case parity table added; the three even-run
cases fail against the previous check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): recognize destructured environment reads
Review round 9. `const { API_KEY } = environmentVariables` delivers the value by
name with no property- or element-access node in the AST, so the member-access
walk missed it entirely — and a missed read leaves an emitted value unmasked,
the dangerous direction.
The AST walk now also recognizes the declaration form (shorthand, renames,
defaults, string-literal keys), the assignment form ({ KEY } = env), and a
...rest element — which names no key but takes every value, so it reports every
configured name; the alternative left `const { ...all } = env; return all`
entirely unmasked. A computed key stays unrecognized, the same runtime-name
boundary as a computed subscript, and a receiver that is not the bare identifier
is not attributed.
Nine cases added; the six positive ones fail against the previous walk.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): one receiver rule for destructured reads, parentheses included
Review round 10. Two accurate findings, folded into a generalization instead of
two more special cases:
- A parameter default (function f({ API_KEY } = environmentVariables)) and a
binding-element default are the same by-name delivery as a variable
declaration. The detector now keys on the ObjectBindingPattern itself and
checks its parent's initializer, so every declaration position follows one
rule instead of per-kind arms.
- Parentheses group without changing the receiver, so (environmentVariables) is
unwrapped before the identifier check — in the destructuring arm AND the
member-access arm, which had the same hole unreported.
Declined the for-of-over-array-literal finding: the receiver there is a
container, not the environment object, and following data flow through
containers has no fixed point — the same documented boundary as aliasing and
computed keys. A test pins the boundary so it reads as chosen, not missed.
Eight cases added; the seven receiver-rule cases fail against the previous code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(secrets): a dot in prose is not a qualifier, and a literal computed key is a subscript
Review round 11. Both findings were implementation-narrower-than-rule, fixed by
consulting authorities the detectors already had rather than adding new ones:
- python.ts: the receiver walk crosses whitespace so a parenthesized `other.` on
a previous line is seen — but it landed on a comment's final period
(`# Load the value.`) and discarded the genuine read on the next line. The
landing position is now checked against the same lexer ranges that filter the
candidates, which is also why the receiver check moves after lexing.
- javascript.ts: `const { ['API_KEY']: key } = environmentVariables` is the
element-access rule in pattern position, so a computed key holding a string
literal resolves like a literal subscript; any other computed key keeps the
runtime-name boundary a computed subscript already has.
Eight cases added; the comment-period case and all three literal-computed-key
cases fail against the previous code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Waleed
|
9328e66d69 |
feat(account): let users delete their own account (#6831)
* feat(account): let users delete their own account Adds a GDPR self-serve account deletion path: a preflight that reports what deletion would remove and every reason it would be refused, and a confirmed delete that erases the account and everything only it can reach. Deletion refuses while the account is still entangled rather than reassigning its content. Most tables reference user.id with ON DELETE CASCADE, and those cascades do not distinguish content in the account's own workspace from content it created inside somebody else's, so each blocker names the existing action that untangles it (leave the workspace, leave the organization, cancel the plan) — all of which already hand work over on their own tested paths. * fix(account): make deletion atomic and re-check privacy at delete time Reorders the teardown so nothing irreversible happens before the deletion is certain: anchors are handed over first (the fallible step, while everything is still recoverable), the workspace and user deletes now share one transaction, and the object-storage purge runs only after that commits. The workspace delete also re-checks inside the transaction that each workspace is still private, so a membership granted between the preview and the delete aborts the whole thing instead of destroying the new member's access. * fix(account): close deletion gaps found in review - Run the whole teardown in one transaction. The billing and ownership handovers now take the caller's transaction, so a refused deletion can no longer leave a workspace reassigned for a deletion that never happened. - Fail closed on a subscription read error. getHighestPriorityPersonalSubscription defaulted to returning null, which read as "no plan" and would have erased an account Stripe was still billing. - Erase the account's profile picture. It is personal data under our own storage prefix; an external provider avatar is left alone. - Enforce the storage purge cap while collecting keys rather than after, so an oversized account cannot exhaust memory before the cap applies. |
||
|
Waleed
|
1e2572dd8a |
fix(desktop): guarantee the update feed resolves the newest release (#6827)
The stable channel reads a GitHub release list it shares with web-app releases, SDK tags, and legacy prereleases, but only ever looked at the first 30 entries. Once enough unrelated releases stack on top, the feed 404s and every stable shell silently stops updating. Walk pages (100 per page, up to 5) until one yields a release for the channel, and fail the feed rather than serving an older build when a page cannot be read. Also point the update gate's manual download at a new /api/desktop/update/download redirect, which resolves through the same channel selection. It previously opened GitHub's repository-wide latest release, which can be a tag carrying no desktop artifact at all. |
||
|
Justin BlumencranzandWaleed Latif
|
0c34e69fdc |
feat(files): upload and safely extract ZIP archives (#6782)
* feat(files): support zip extraction * fix(files): harden zip extraction safety * fix(files): batch extraction notifications * fix(files): defer rollback storage cleanup * fix(files): restore reliable drag uploads * fix(files): use explicit archive extraction route * fix(ci): account for archive extraction route * refactor(files): bound every archive extraction and trim the extractor's option surface `maxMaterializedItems` was opt-in, so only the new unzip route bounded its output tree — the copilot `materialize_file` and `POST /api/tools/file/manage` extract paths had no cap on folder creation at all. An archive within MAX_ARCHIVE_ENTRIES can still imply far more folders than files, so the cap now defaults to MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS and applies to all three callers. `materializedRootFolderCount` was a hand-maintained number that had to agree with what an opaque callback would create, and the callee could not check it; drift surfaced only as an over-limit archive slipping past the cap. It is now derived from whether `prepareRootFolder` ran, so the contract is just "the callback creates exactly one folder". Also single-sources the ArchiveError -> HTTP status map (it was copied into both the internal error policy and the tools route), drops IdempotencyService config that only `executeWithIdempotency` reads (the extraction lease uses atomicallyClaim/release, so no result is ever stored), hoists the duplicated predicates in purgeCreatedWorkspaceFile and archiveWorkspaceFileFolderIfEmpty so a lock and its write cannot diverge, and names UPLOAD_SESSION_LOCAL_PUT_MAX_BYTES rather than overloading the multipart part size as the local single-PUT ceiling. Adds coverage for the two guards nothing exercised: the re-validation of the segments `prepareRootFolder` actually returned, and the default cap applying with no caller opt-in. UI: the drop overlay used --surface-4 unconditionally, which renders grey over the light-mode canvas; matches the canonical overlay's --white/dark:--surface-4 and swaps arbitrary px type sizes for named tokens. * fix(files): bound the extraction write loop so it cannot outlive its lease Cursor Bugbot flagged two related holes, both rooted in the write loop being unbounded: 1. `maxDuration` is a Next.js route-segment config that serverless platforms enforce and self-hosted deployments do not. A slow extraction (up to 1000 sequential uploads) could therefore outrun the six-minute lease, and `IdempotencyService` reclaims an expired in-progress claim — so a second unzip of the same archive could start beside the first. 2. Nothing rolls back a process killed mid-pass-2, so a timeout stranded the destination folder and every file written so far. `decompressArchiveBufferToWorkspaceFiles` now takes an `AbortSignal` and checks it between entries in both passes, and the extraction use case supplies a 180s deadline. The abort unwinds through the existing all-or-nothing rollback, so the work stops on our terms with the tree cleaned up, well inside both the route's 300s budget and the 360s lease. That closes (1) outright — the holder can no longer outlive its lease on any platform — and converts (2) from a stranded partial tree into a clean rollback for the slow case that actually triggers it. A SIGKILL still cannot be caught; that needs a durable job and is out of scope here. The overrun surfaces as a caller-fixable 413 naming the archive rather than an opaque 500 from the raw DOMException. * fix(files): only remap the deadline abort itself, and stop overclaiming rollback Two follow-ups on the budget deadline, both reported by Cursor Bugbot: `deadline.aborted` stays true for the rest of the request once the timer fires, so it cannot decide whether *this* error was the abort. An `ArchiveError` or storage failure thrown mid-entry after the timer fired was being relabelled as a timeout and returned as a 413, hiding the real cause. The catch now matches the thrown value against `deadline.reason` — `throwIfAborted()` throws exactly that object, so the check is identity-exact and cannot capture an unrelated failure. The message also claimed a rollback that has not necessarily happened: the budget covers the archive download too, so it can fire before the first write, when there is nothing to roll back. It now says the unzip was cancelled and claims nothing about what was written. Including the download in the budget is deliberate — the lease it has to fit inside starts earlier still — so the TSDoc says that rather than "the extraction itself". --------- Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Waleed
|
c8f559ae77 |
fix(workflows,connectors): close pre-merge audit findings (#6783)
* fix(workflows,connectors): close pre-merge audit findings Recover subblock values orphaned by the id renames in this release, and stop truncated knowledge-base listings from reporting themselves complete. - Add operation-scoped subblock id migrations so a saved workflow's stored value survives a rename. Cloudflare create/update DNS record, ServiceNow read record, and Okta deactivate/delete previously lost their stored value: the create path substituted a seeded default (an A record where the user chose CNAME, and unproxied where they chose proxied), and the update path silently no-opped while reporting success. A migration is used rather than a legacy-id fallback so no subblock id carries two value spaces at runtime. - Webflow, Zendesk: a listing that stops for a reason the connector cannot rule out now reports as capped instead of exhausted. A malformed envelope, an unfollowable continuation link, or an absent collection list previously read as a complete listing and let deletion reconciliation hard-delete every document past the truncation point. - Sentry: pin the listing window in the request rather than inheriting the server default, so the range cannot silently narrow into hard deletes. - Fork sync: a parent re-pick no longer writes a blank over a hidden optional dependent's stored target value, and a required field stays on screen once it is filled. Add hook-level coverage for the submitted payload. - Fork file copy: a file whose name is already taken in a reused target folder is de-duplicated instead of dropped. - Delete an orphaned Shopify OAuth route that built a credential from unsigned cookies. It had no writer, no caller, and no inbound link. - Tailwind: drop two content globs that scanned 5.4k files to emit one unused rule, keeping the ones that fix brand tile icon color. - Correct the API route-count baseline, add an Evernote docs redirect, align library copy with the language rules, and fix a stale turbo filter. * fix(connectors,forking): trim the audit fixes to their minimum A legitimacy review found several changes closed no live defect, and two introduced problems of their own. - Zendesk: narrow the cursor fix to a signal change. Treating a missing meta envelope as truncation had also made the walk follow links.next and keep paginating, and the ticket cursor has no page-depth valve, so a source advertising a next page with no meta could loop without terminating. The page-fetch set now matches the previous behavior; only the flag is new. - Zendesk: drop the search next_page branch. The existing count check already caps every case where a missing key could lose documents. - Webflow: drop the empty-collections flag. The sync engine already blocks the first sync on an empty listing and reconciles only when a second sync agrees, which handles a transient fault better and still removes documents when a source is genuinely emptied. The flag short-circuited that and suppressed reconciliation permanently. Restore the previous loud failure on a non-array envelope, and drop the unreachable collection-id filter. - Webflow: soften a docstring that claimed pagination.total is always present. It is documented optional, so its absence proves nothing either way and treating it as unprovable truncation is the fail-safe reading. - Sentry: drop the pinned statsPeriod. Sentry's issue search floors every query at 90 days in the executor regardless of the request, and the endpoint this release moved away from hit the same floor, so there was no window to close. Keep the tests and the docstring recording that. - Fork copy: drop the renamed counter, which no caller reads. - Repair check-block-registry, which stopped exempting migrated subblock ids when the migration map became an array — `in` was testing array indices. - Drop mdx from a Tailwind content glob that emits nothing, and loosen an exact compiled-SQL assertion to the invariant it was pinning. * fix(migrations): keep a ServiceNow write body off the read projection Review findings from the first round. - A legacy ServiceNow block can hold a Create/Update Record JSON body under `fields` while its stored operation is Read Records: the id served both value spaces before the rename, and a subblock value is not cleared when the operation changes. The scoped migration moved that body onto `readFields`, where it would reach the wire as sysparm_fields. Migration entries can now carry a `whenValue` predicate for the case where the stored operation alone cannot separate two value spaces, and the ServiceNow entry uses it to move only a plausible comma-separated projection. - Type the fork copy test harness instead of using `any`, without weakening it: every predicate shape it does not model still throws rather than matching. - Correct the dependent-omission comments. Omitting a parent-invalidated field preserves the target's stored value on Save and across an undo, where the parent nets out unchanged; on a Sync the written state is source-derived, so what it prevents there is an explicit blank reaching the fields the remap's clearing pass does not cover, nested tool params in particular. Okta's migration scope is left as-is: `okta_remove_user_from_app` and the sendEmail split shipped in the same release, so no saved block can hold legacy state for it, and widening the scope would promote an activation-era value onto the deactivation switch. Tests document the boundary. * chore(forking): move the fork-sync changes to their own PR The dependent-omission fix and the fork file-copy de-duplication are reviewed separately in #6787. They are the only changes here that overlap #6776, and they carry their own design tradeoff, so they should not ride along with the unrelated audit fixes in this PR. * fix(migrations): separate a ServiceNow write body from a projection by parsing The guard tested for a `{` or `[` prefix, so a stored scalar body — `true`, `"short_description"`, `42` — read as a field list and was promoted onto `readFields`, where it would go out as sysparm_fields. A Create/Update Record body is JSON and a projection is a bare comma-separated field list, which is never valid JSON, so parsing is the whole test rather than a guess at its opening character. Ambiguity still resolves to "not a projection", leaving the value where the Create/Update control owns it. * test(connectors,credentials): tie two assertions to what they actually prove - Webflow: a non-array collections envelope reaching `for...of` throws, which is the intended loud failure. Assert the spec-mandated TypeError plus a single request and no write-back, rather than matching V8's wording. - Credentials: the second guard test cannot observe "not deleted" — the proxy driver replays canned rows — so name it for what it does verify, that the reference check carries no workspace predicate and an empty RETURNING logs nothing. Making the driver decide the outcome would fake the database. - Drop `vi.importActual`; a plain `drizzle-orm/pg-proxy` import works now that `drizzle-orm` is un-mocked. * fix(migrations): identify a ServiceNow projection by its own shape Recognising a write body was the wrong way round. A saved body is not always well-formed: it can be a half-typed draft or carry an unquoted block reference, so neither "opens with a brace" nor "fails to parse as JSON" identifies one — and a body misread as a projection is moved to readFields with its original key dropped, losing the draft. Match the projection instead: a comma-separated list of ServiceNow field names, which are word characters plus the dot of a dotted walk. A brace, quote, colon, angle bracket or interior space fails that shape. Parsing then removes the bare scalars that satisfy it by accident. |
||
|
Waleed
|
ae2147645c |
fix(cli): resolve findings from a full command-surface audit (#6788)
* fix(cli): resolve findings from a full command-surface audit Exercised all 147 commands against a live deployment. Fixes the defects that surfaced, plus the docs and generator drift they exposed. Transport - Stop following redirects. A bare domain that 301s to www silently converted POST to GET and dropped the body, so reads worked while every write failed with a misleading validation error and login returned 405. Both the client and the device flow now explain the redirect and name the endpoint to configure, rather than carrying credentials off-origin. - Report a non-JSON response as one instead of printing the HTML page. - Name the personal-API-key remedy on a workspace-key refusal, reading the machine-readable code the API actually sends. - Drop union-branch noise from validation errors that contradicted itself. - Show paging progress on stderr for multi-page fetches. Output - Clamp record values for table only. text is the format built for pipes, and it was truncating signed URLs and tool source mid-value. - Infer timestamp, duration, bytes and boolean formatting for API-owned keys so undeclared commands stop printing raw ISO and float ms. Skips user-defined table cells and leaves json/yaml on the raw payload. - Render a declared-but-absent field as an em dash; billing credits were vanishing silently. Paths, naming and validation - Percent-encode folder paths per segment and decode them for display, so a folder reads and types as the name shown in the app. - Reject a malformed endpoint where it is set and where it resolves, instead of crashing with a URL parse trace. - Request the detail level logs list's own columns need; its workflow column could never populate. - Rename three commands that described themselves wrongly and align two flags with their siblings. Old spellings still work: hidden, warned on stderr, and kept out of help and docs. - Verify whoami against the API, separating a bad key from an unreachable endpoint, and report the workspace by name. - Correct the --yes help text, which advertised skipping a prompt that does not exist. Docs - Teach the docs generator that a flag required by the runtime is required, and that hidden commands are not documented. * fix(cli): clear the paging progress line when a page fails Progress is written without a trailing newline so it can be overwritten in place, and both paging loops cleaned it up only on success. A page that threw part-way through left `fetched 1200…` on the line the error was then printed onto, so the two ran together. * fix(cli): name a working API root when an endpoint redirects The suggested endpoint was the redirect target's origin, which drops a path prefix. A self-hosted deployment reached at https://host/sim was told to set https://www.host — not an API root, so following the advice replaced one broken endpoint with another. Derive it by stripping the request's own path from the target instead, so a prefix survives, and say nothing about --set-endpoint when the target resolves to the endpoint already configured: a trailing-slash or path normalization redirect keeps the origin, and naming the value the caller already has explains nothing. The login poll shared both faults and now shares the helper. |
||
|
Waleed
|
025ea4d2bd |
fix(docs): serve JSON-LD in the HTML, fix sidebar spacing, and tighten the CLI guides (#6763)
* docs(cli): use -g for the install, and cut the prose that was not pulling weight
`--global` is valid but `-g` is what every comparable CLI documents, and the
long form only came from the package README. Also drops the yarn tab: it read
`yarn global add sim`, which works on Yarn 1 only — Yarn 2 removed global
installs, so that command fails for anyone on a modern Yarn. Adds `npx sim` for
running without installing.
The guides had accumulated design rationale that belongs in code comments rather
than user docs — why the filter grammar is JSON, why the config section naming
is asymmetric, why an unexpected error keeps its stack trace. Surveying how gh,
Vercel, Turborepo, Deno, Bun and Supabase write theirs, none carry that kind of
justification, and callouts are reserved for content whose absence produces a
wrong result rather than for general asides.
So: 1016 lines to 763, and 12 callouts to 3. The three that remain are the
pairing-code check, that `sim logout` does not revoke the key, and the
`--limit 100` default on `batch-delete`/`batch-update`, which silently truncates
a larger match. Troubleshooting drops the entries whose error message already
contained its own fix and keeps the seven whose cause is not obvious.
* fix(docs): render JSON-LD as native script tags so it reaches the HTML
All four structured-data blocks — WebSite, TechArticle, BreadcrumbList,
SoftwareApplication — were rendered with `next/script`, which never emitted a
script tag. Measured on a production build, `/api-reference/getting-started`
contained zero `<script type="application/ld+json">` elements; the payload
existed only in the `__next_s` client-injection queue and the RSC flight data,
so anything reading the served HTML saw no structured data at all. React was
also logging "Encountered a script tag while rendering React component" on every
page.
`next/script` is for loading and executing JavaScript. JSON-LD is data, and
Next's own guidance is a native `<script>` in the component. `serializeJsonLd`
already escapes the `<` character to its unicode form, which is the
sanitization that guidance calls for, so only the element changes.
Same build, after: three valid tags per page with `WebSite` in `<head>`, and the
injection queue gone entirely.
* fix(docs): scope the flush-separator rule to a container's first separator
`[data-separator]:not([data-separator] ~ [data-separator])` was meant to keep the
first sidebar group flush against the top padding, but `~` only reaches siblings,
so it also matched the first separator inside every expanded folder. Under
Self-Hosting, "Install" lost its top margin and crowded the "Architecture" link
above it — 25px of gap where "Configure" and "Operate" below it had 40px.
`:first-child` expresses the intent directly. Only the four sidebar roots open
with a separator; every nested folder starts with a page, so the intended case
still goes flush and nothing else changes.
* fix(docs): move the flush-separator rule onto the separator component
Keeps the styling with the component that owns it, per the repo standard, and
lets the global rule be deleted outright rather than corrected — `global.css`
now only loses a rule in this PR. Tailwind's `first:` variant compiles to the
same `:first-child` selector, so behavior is unchanged: the build emits
`.first\:mt-0:first-child{margin-top:0}` and the prerendered HTML carries the
class on the separator.
|
||
|
Waleed
|
fed891f69d |
docs(cli): add a CLI docs section generated from the command tree (#6762)
* docs(cli): add a CLI section, generated from the command tree
The `sim` CLI shipped with no coverage in the docs site. Adds a fourth
top-level tab for it, and moves Academy last.
The command reference is generated. `sim` exposes 147 leaf commands across
33 groups, most of them derived at runtime from the v2 route contracts, so a
hand-written reference would be wrong the week after it was written. The
generator walks the command tree `buildProgram()` hands to commander — the
same tree the terminal parses — rather than re-deriving it from the contract,
which would be a second implementation free to describe commands nobody can
invoke. `check:cli-docs` is a zero-arg `check:*` script, so the existing audit
runner picks it up and stale pages fail CI.
Generating against the real tree surfaced a collision it had been hiding:
`bulkUpdateKnowledgeDocuments` and `updateKnowledgeDocument` both derived to
`sim knowledge documents update`. Commander resolves a duplicate to the first
match, so the bulk form shadowed the single-document one and its flags were
unreachable while still appearing in `--help`. The bulk form is now
`batch-update`, matching how `tables rows batch-delete`/`batch-update` already
handle the same REST overload, and the generator fails on any duplicate path
so the next one cannot land silently.
Five hand-written guides cover install, auth, configuration, output formats,
and scripting. Also corrects two commands in the package README that do not
exist as documented (`tables columns <tableId>`, and `--sort score:desc`,
which is JSON).
* docs(cli): document every flag from the contracts, add troubleshooting and a single-page reference
The command reference was structurally complete but said almost nothing: 223 of
377 flags rendered as "Set sort by" because the CLI only ever read flag help
from its own contract overrides, and fell back to restating the flag name.
The prose already existed. The v2 route contracts carry 931 `.describe()` calls
and the OpenAPI specs publish all of them — 327 parameters and 282 body
properties, 100% coverage — but the generated operation table dropped every one,
carrying only a per-operation summary. It now carries the field descriptions,
the path-parameter descriptions, and positional help, so `--help` and the docs
explain a flag the same way the API reference does. Placeholder descriptions are
now zero, and 147/147 commands, 377/377 flags and 130/130 arguments are
documented.
`check:cli-docs` fails on a request field with no `.describe()` rather than
letting it render as documentation that says nothing.
Also in this pass:
- Commands are root-level sidebar entries under a Commands heading rather than
a folder, and headings are the command's description, so the table of
contents distinguishes entries at the first word instead of repeating
"sim knowledge documents …" fourteen times. A guard fails the build if two
descriptions on a page collide, since they would share an anchor.
- A single-page `Complete reference` carrying all 147 commands, for in-page
search and for agents fetching `/cli/reference.mdx`. It keys on exact command
paths because descriptions are only unique within a group.
- A troubleshooting page, with every message copied from the source.
- Table columns are sized by a local component; the flag column was starved
while descriptions kept most of the row empty.
- The prerelease install channels are dropped from the docs and the package
README, which is what npm renders.
* fix(docs): match the CLI tab by path segment, and escape backslashes before pipes
`pathname.includes('/cli')` also matches `/integrations/clickup` and
`/integrations/clickhouse`, so both existing integration pages lit the CLI tab
and unlit Documentation. Matching is now per path segment. Anchoring to the
start would not work either — a non-default locale prefixes the path, as in
`/ja/cli` — so the segment is matched wherever it sits.
Table cells now double a backslash before escaping pipes. A value ending in one
turned `a\` + `|` into `a\\|`, which the table parser reads as an escaped
backslash followed by an unescaped pipe, splitting the cell early. Nothing in
the command surface contains a backslash today, so this was latent rather than
visible.
The reference page's global options table is two-column and was being wrapped in
`CommandTable`, which sizes the second column for the `Required` cell of the
three-column tables and crushed the description into 5.5rem. It now matches the
overview page, which leaves that table unsized.
|
||
|
Waleed
|
6a29a9e2f4 |
feat(mssql): add Microsoft SQL Server integration (#6739)
* feat(mssql): add Microsoft SQL Server integration
Add a Microsoft SQL Server block backed by six tools (query, execute,
insert, update, delete, introspect), mirroring the existing PostgreSQL
and MySQL integrations.
Connections go through the `mssql` (Tedious) driver: `connectionTimeout`
is top-level while `encrypt`, `trustServerCertificate`, and
`instanceName` live under `options`, and `port` is omitted when a named
instance is used. Values are bound as `@paramN` via `request.input()`;
no user value is interpolated into SQL. Identifiers are bracket-quoted
after validation and WHERE clauses run through the shared injection
guard.
Introspection reads INFORMATION_SCHEMA plus the `sys.indexes` catalog
views for tables, columns, primary keys, foreign keys, and indexes.
The icon is a placeholder database cylinder drawn with `currentColor`
until the real brand mark lands.
Requires `bun install` for the new `mssql` / `@types/mssql` deps.
* feat(mssql): use the SQL Server brand mark on a white tile
* fix(mssql): pin the validated IP and correct the introspection catalog reads
Tedious exposes `options.connector`, a hook that replaces its own
resolve-and-connect path, so the connection can be pinned to the address
`validateDatabaseHost` already approved instead of re-resolving the
hostname. `server` stays the hostname because tedious derives the TLS
`servername` from it independently of the connector, so SNI and
certificate validation survive the pin. This brings MSSQL in line with
the PostgreSQL and MySQL tools.
Named instances are dropped: tedious resolves them with a UDP SQL Server
Browser lookup issued outside the connector, and node-mssql deletes
`port` whenever `instanceName` is set, so no configuration leaves a
named instance pinned. A named instance is reachable through its static
TCP port.
Introspection fixes:
- index key columns now filter on `key_ordinal > 0`; INCLUDEd columns
and partitioning columns both report `0` and were being returned as
key columns, ordered ahead of the real ones
- foreign keys resolve through `sys.foreign_keys` /
`sys.foreign_key_columns` rather than
`INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS`, whose join to
`TABLE_CONSTRAINTS` has no row when a key references a unique index
and so dropped the key entirely
- `is_unique` is a `bit`, which tedious maps to a boolean, so it is
coerced rather than compared
- schemas come from `sys.schemas`, which needs only `public` and carries
no metadata-visibility caveat
The WHERE-clause guard also covers `WAITFOR TIME`, `OPENQUERY`,
`OPENXML`, the legacy `master..sys*` compatibility views, and extended
and OLE-automation procedures beyond `xp_cmdshell`.
Regenerates the docs and catalog artifacts the icon change left stale.
* chore(mssql): commit the lockfile entries for mssql and its tedious dependency tree
* fix(mssql): make the Query operation genuinely read-only
The block label, tool description, and docs all present Query as SELECT-only
while the route ran whatever T-SQL it was given, so an agent picking
mssql_query because "it is only a SELECT" could delete rows. Screen the
statement for mutating keywords with string literals stripped, which also
catches the WITH ... DELETE form that a leading-token check would miss.
Also switch the tool barrel to absolute imports per the repo convention.
* fix(mssql): compose the shared WHERE guard and close the semicolon-less batch gap
The local validateWhereClause re-derived an older copy of the shared patterns
and scanned raw text, so it missed a bare 1=1 and false-positived on prose in a
quoted value. Delegate to validateSqlWhereClause, which masks string literals
first, and keep only the SQL Server surfaces it has no reason to know about.
T-SQL needs no statement terminator, so every semicolon-anchored stacked-query
check reads straight past `id = 1 DROP TABLE dbo.users`. Screen for a bare
statement-introducing keyword to close that; word boundaries leave ordinary
column names like updated_at and deleted_at untouched.
Export maskSqlStringLiterals so the dialect layer masks the same way the shared
guard does rather than carrying a weaker single-quote-only copy.
* fix(mssql): screen administrative T-SQL and reject batches in the read-only path
The previous round left two keyword lists maintained separately, and both were
short: DBCC, KILL, CHECKPOINT, USE, and DENY were in neither, so
`SELECT 1; DBCC SHRINKDATABASE(...)` and `id = 1 DBCC SHRINKDATABASE(...)`
both got through. Collapse them into one MSSQL_STATEMENT_KEYWORDS shared by the
query and WHERE screens so a keyword cannot be covered in one place and missed
in the other, and add the administrative commands.
Also reject any second statement after a semicolon in the Query path outright.
That closes SELECT 1; <anything> structurally instead of by naming the anything,
so the blacklist no longer has to be exhaustive to hold.
* fix(mssql): reject SQL comments in the read-only query path
A block comment placed inside a keyword splits it as far as a lexical scan is
concerned, so keyword coverage cannot settle whether the server rejoins the
halves. Refuse comments in the Query path instead of modelling the tokenizer.
A SELECT sent through this operation has no need for one, and Execute Raw SQL
still accepts them. Masking leaves comment markers intact, so a literal
containing -- still passes.
* fix(mssql): close the masker-desync bypasses and correct the catalog reads
Every T-SQL screen runs over the shared literal masker, which was written for
the MySQL dialect. Three ways to desynchronise it let real SQL hide inside what
the masker believes is a string, two of which survived the existing even-quote
check:
- a backslash before a quote. T-SQL has no backslash escape, so the server
closes the literal where the masker swallowed the quote and runs the rest as
code. `a='x\' DELETE FROM dbo.t WHERE b='y'` holds four quotes and masks the
DELETE out of the keyword screen entirely, so the read-only Query operation
would run it.
- a double quote inside a bracketed identifier, which the bracket rule missed
because it only looked for single quotes.
- any unbalanced double quote or backtick, which the parity check did not cover.
All three now fail closed. Introspection also filters hypothetical and disabled
indexes, which were reported as if they were live, and resolves the referenced
side of a foreign key through sys.schemas so a cross-schema reference is no
longer an ambiguous bare table name. Values bound through request.input are
serialized when they are nested JSON, which the driver otherwise rejects with a
bare "Invalid string.".
* test(mssql): cover the block param merge and the operation-to-tool map
Asserts on the merged `{ ...inputs, ...buildParams(inputs) }` the generic tool
handler forwards rather than the mapper's return, since a key the mapper omits
keeps its raw subBlock value through that merge. Pins the TLS toggles to their
string form end to end — a switch subBlock would serialize `'false'`, which is
truthy, and the route contract would coerce the user's off into on — and checks
that duplicate subBlock ids agree on their seeded default.
* fix(mssql): release a pool whose connect failed, and allow a keyword with no trailing space
Only a pool handed back to the route reaches its `finally`, so a pool whose
connect rejected leaked its tarn resources — one per attempt when a bad
credential is retried. It now closes itself, and a failure to close cannot mask
the connect error the caller needs.
The read-only screen also anchored on `\s` after the opening keyword, which
refused valid reads like `SELECT*FROM dbo.users` and `SELECT(1)`. A word
boundary accepts those while still refusing `SELECTX`, and cannot loosen the
screen — the keyword and batch checks run over the whole statement regardless.
* chore(mssql): regenerate tool metadata and the integration catalog after rebase
Artifacts rebuilt with the generators rather than hand-merged, so they carry
both the mssql entries and the tools that landed on staging in parallel.
* fix(mssql): reject a parenthesised or negated constant tautology in a WHERE clause
The shared guard recognises `OR 1` but not `OR (1)`, `OR ((1))`, `OR NOT 0`, or
`OR NOT (FALSE)` — a parenthesis or a NOT between the operator and the constant
hides it. Both patterns require the constant to be the whole parenthesised term,
so a real disjunct such as `OR (1 = priority)` is untouched.
This narrows the gap rather than closing it, and is not meant to close it: an
always-true expression is not lexically decidable in general, which is why the
WHERE screen stays documented as defense-in-depth rather than a boundary.
* chore(mssql): regenerate tool metadata after rebase onto staging
Rebuilt with the generators so the artifacts carry the servicenow and
crowdstrike tools that landed on staging alongside the mssql entries.
* fix(mssql): screen trigger and state statements, and drop the space anchor on Execute
DISABLE and ENABLE were missing from the shared statement list, so
`SELECT 1 DISABLE TRIGGER dbo.audit ON dbo.users` passed the read-only screen as
a semicolon-less batch and turned auditing off. SET, BEGIN, COMMIT, and ROLLBACK
are added with them, since session and transaction state are reachable the same
way. FETCH is deliberately left out: OFFSET ... FETCH NEXT is the standard paging
clause, and screening it would reject the ordinary paged SELECT.
Execute Raw SQL anchored its allowlist on `\s`, which refused `EXEC(@sql)` —
the ordinary form of dynamic SQL, on the one operation meant to run it. It now
uses `\b`, matching the read-only screen.
|
||
|
Justin Blumencranz
|
7f936dc02a |
feat(tooling): enforce docs freshness and modernize agent skills (#6756)
* feat(docs): fail CI when generated integration docs are stale * fix(docs): don't flag delete-then-recreated trigger pages in check mode * docs(skills): require docs:check in the integration authoring skills * chore(skills): migrate agent commands to native skills * fix(skills): clean orphaned Claude projections |
||
|
Waleed
|
9e67655b23 |
feat(servicenow): semantic incident, change, catalog, approval, CMDB, and knowledge tools (#6747)
* feat(servicenow): add semantic incident, change, catalog, approval, CMDB, knowledge, and directory tools
The ServiceNow block only exposed generic Table API CRUD, so every real task
started with "which table is that on?". This adds 27 semantic tools that wrap
the same Table API plumbing under the names customers actually use.
- Incidents: create, get by number or sys_id, search, update, resolve, close,
and append a work note or customer-visible comment.
- Change: create, get, list, update, move state, and list change tasks through
the documented Change Management API.
- Service catalog: browse items, order one via the Service Catalog API
order_now endpoint, and list or get requested items.
- Approvals: list pending approvals for an approver, approve, and reject.
- CMDB: search CIs on any class, read a CI with its inbound and outbound
relations through the CMDB Instance API, and list cmdb_rel_ci rows.
- Knowledge: search and read articles through the Knowledge Management API.
- Directory: find a user by email or user name and list group members, which
is what fills assigned_to and assignment_group.
Reference fields are the usual source of confusion, so every semantic read
defaults to sysparm_display_value=all — a reference comes back as both its
sys_id and its label — and every semantic write exposes
sysparm_input_display_value so a display name can be written instead of a
sys_id. Coded state values are exposed as labelled dropdowns built from one
constants module rather than raw integers.
The shared instance-URL, Basic Auth, sysparm, envelope, and error handling now
live in tools/servicenow/utils.ts, and the existing eight generic tools were
moved onto it rather than keeping their own copies.
* fix(servicenow): stop per-operation subblock defaults colliding on a shared id
Subblock initial values are seeded into block state keyed by subblock id, so
two subblocks sharing an id leave one stored value and the last definition
wins. Three ids were duplicated with differing defaults:
- `displayValue` was defined twice, unset for the generic Table API tools and
`all` for the semantic ones. The semantic definition won, so a new block set
to Read Records or Aggregate Records sent `sysparm_display_value=all` — a
wire change to two already-shipped tools.
- `state` was defined four times. The Approval State definition won, so every
new block carried `state=requested`, which Create Incident wrote to the
incident and Move Change State used instead of its own `-5` default.
Give the colliding controls their own ids and map them back to the tool params
per operation, so the generic tools keep their original request shape and each
semantic operation keeps its own default.
Also correct descriptions that overstated what the API does: the LIKE operator
is not documented as case-sensitive, List Requested Items has no requester
filter, and the Change Management API task shape differs from the Table API.
Adds tool tests covering the refactor invariants for the eight pre-existing
Table API tools and the display-value separation.
* feat(servicenow): read a change request's real next states from the instance
The change tools describe state transitions using the base-system codes, which
only hold on an instance that has not customized its change model. ServiceNow
publishes an endpoint that answers the question directly for the record in
hand, so use it rather than keep assuming.
GET /api/sn_chg_rest/change/{sys_id}/nextstates returns the states reachable
from the change request, the instance's own state-value-to-label map, and, for
model-driven changes, each transition with the conditions it has and has not
met. The tool flattens the per-target-state grouping ServiceNow returns (each
transition already carries from_state and to_state, so nothing is lost) and
derives the states whose conditions currently pass.
Also record the sourcing for the coded values in constants.ts: the change
states and close codes are published as a table, but the incident state codes
are not — only 6 (Resolved) appears in the docs — so mark the rest as defaults
rather than guarantees. Note that sysparm_input_display_value also reinterprets
date and time values in the caller's timezone instead of GMT, which matters for
the change start and end dates.
* docs(servicenow): stop asserting undocumented coded values in placeholders
The additional-fields examples used hold_reason with a coded value of "1".
ServiceNow documents the On hold reason choices by label only — Awaiting
Caller, Awaiting Change, Awaiting Problem, Awaiting Vendor — and publishes
neither the column name nor the codes, so the example was asserting something
unsourced. Use a field whose value is caller-supplied instead, and record the
On Hold requirement on the incident state control using the labels the docs
actually give, including that Awaiting Caller makes Additional Comments
mandatory.
* fix(servicenow): drop phantom parent fields from the catalog order output
order_catalog_item read parent_id and parent_table off the order_now response.
Those fields belong to submit_producer, a different Service Catalog endpoint;
the documented order_now result is sys_id, number, request_number, request_id,
and table. Both outputs were therefore always null.
* fix(servicenow): correct what knowledge search returns as an article id
Search results carry a table-prefixed identifier — "kb_knowledge:9e528db1..."
— not a bare sys_id, while GET /knowledge/articles/{id} accepts only a bare
sys_id or a KB number. The output described it as a sys_id and the tool
description told callers it was what they needed to fetch the article, so
chaining the two tools on that field would fail. Point callers at the KB
number instead. Relevancy score is documented as a number, not a string.
* docs(servicenow): cite the page that actually documents approval statuses
The approval state constants pointed at the classic-approvals landing page,
which does not list the statuses. Approval status is documented separately and
names four — Requested, Approved, Rejected, and Not Requested.
* fix(servicenow): stop constant interpolation leaking into tool descriptions
The docs generator and the client-facing integration catalog read tool
descriptions from source rather than from the evaluated module, so a
template literal like `state ${INCIDENT_STATE.RESOLVED}` shipped to users
verbatim: `apps/sim/lib/integrations/integrations.json` and the published
ServiceNow integration page both rendered `${INCIDENT_STATE.RESOLVED}`
instead of `6`. Inline the base-system coded values in the description
text; the constants stay in use everywhere behavior depends on them.
Also drops an escaped `\'` in the `inputDisplayValue` description for the
same reason, and adds a standing guard test asserting no subBlock id
carries two different seeded defaults — the invariant behind the
per-operation defaulting bug, now checked structurally rather than only
through the four per-operation cases.
* refactor(servicenow): type the shared response boundary instead of any
`parseServiceNowResponse` returned `any`, so every tool reading `data.result`
did unchecked property access — a shape change on the instance side would have
produced a wrong-typed output silently rather than a type error.
Introduces `ServiceNowEnvelope` (`result?: unknown`) as the parser's return
type and narrows the record index signatures from `any` to `unknown`. Adds
`toRecordObject`, `readString`, and `readNestedNumber` so the tools that read
individual fields narrow deliberately at the point of use.
This surfaced five genuinely unchecked reads: Order Catalog Item, Get Knowledge
Article, and Search Knowledge were declaring `string | null` / `number | null`
outputs while emitting whatever the instance sent, and Get Change Next States
assigned an unvalidated object to `Record<string, string>`. Each now coerces or
drops a non-matching value rather than passing it through.
* fix(servicenow): publish the shared tool params and stop offering inert controls
The docs generator reads tool source rather than importing it, so the shared
`params.ts` consts the semantic tools spread were dropped from every published
Input table — 27 of 35 ServiceNow tools listed no instance URL, username, or
password at all. Follow a spread into the module it is imported from so those
rows are published; ten other integrations gain the rows they were missing for
the same reason.
Two controls were dead on arrival: Additional Fields was offered on Move Change
State and Add Incident Comment, and neither tool read it. Wire it through the
change transition, which needs it, and drop it from the comment tool, whose body
is exactly one journal field.
Every coded-value control was a select-only dropdown, so a customized instance's
state or close code was unreachable — sharpest on Move Change State, whose
target state is required and whose real codes come from Get Change Next States.
Make them comboboxes.
Also correct two doc claims ServiceNow does not publish (the incident state
citation pointed at a page that does not exist and compares the legacy
incident_state field; closing an incident is not documented as requiring
itil_admin), replace Record<string, any> with checked narrowing that surfaced
two unsound widenings, and document that List Change Tasks returns a fixed
{value, display_value} shape under `tasks` rather than `records`.
* fix(servicenow): stop one subblock id from carrying two value spaces
Subblock values are stored per block keyed by id, so an id reused across
operations keeps its value when the operation changes. Incident and change
shared `state`, and `closeCode`, `closeNotes`, `comments`, and the knowledge
search phrase were each reused for a different value space — so an incident
state could be written onto a change request, an incident close code sent as a
change close code, or an encoded query searched as knowledge text.
Give each value space its own subblock and republish it to the tool param from
the operation that owns it, the way targetState and approvalState already work.
The generic Table API ids stay exactly as they are, since renaming one would
orphan the stored value of every workflow already using those shipped tools.
The previous guard only compared seeded defaults, which is why this class stayed
hidden; the new one asserts against the merged params a tool actually receives.
* fix(servicenow): point the canvas sentences at the renamed subblocks
The split of the colliding subblock ids left the operation sentences anchored on
ids that no longer exist, so those clauses would silently drop from the card.
* fix(servicenow): validate collection members and split the fields projection
toRecordArray cast every member of a successful response, so a null or scalar in
a collection was handed to the next block as a record while the tool reported
success and its declared output said that could not happen. Members that are not
plain objects are now dropped, and knowledge articles and change transitions get
the same narrowing. The two response types that described an unverified inner
shape now say what is actually checked.
The 'fields' subblock also carried two value spaces: a JSON body on Create and
Update Record, a comma-separated projection everywhere else. Operations added
since read a separate returnFields control, so a body can no longer arrive as a
projection or the reverse. The shipped ids are untouched, since renaming one
orphans the stored value of every workflow already using those tools.
|
||
|
Waleed
|
0cd87bad2f |
feat(resources): multiselect on tables and knowledge, spring-loaded folders (#6721)
* feat(resources): multiselect on tables and knowledge, spring-loaded folders Tables and Knowledge lists get the checkbox multiselect Files already had — selection, shift-click ranges, select-all, and a shared bulk action bar for move and delete. Dragging a resource onto a folder row and resting there now opens that folder, so nested filing is one gesture (macOS Finder spring-loading). Works on Files, Tables, and Knowledge. Selection, the action bar, the drag payload, the drag ghost, and drag teardown are extracted to shared modules; Files migrates onto them rather than keeping its own copies. Bulk move and delete land as single authorized operations that take folders and resources together, so a mixed selection commits once instead of fanning out. Fixes two latent UI bugs: the drop-target outline referenced --accent, an HSL-channel token only valid via hsl(), so it silently rendered as currentColor; and rows painted hover and selected with the same surface token, making the two states indistinguishable. * chore(audits): re-record route ratchet after merging staging * fix(bulk): reject a move target inside the moving subtree and report contained folders deterministically * improvement(resources): neutral drop affordance, longer spring delay, and a body drop target * fix(resources): drop into the open folder on Files, return on an unused spring-open, and guard bulk caps * fix(drag): end a drag on pointer resume instead of an idle timer * feat(resources): drag onto breadcrumbs to move back up, and round the drop ring * fix(resources): tint the list region on drop instead of ringing it * refactor(folders): share the spring-navigation lifecycle so Files returns too * fix(breadcrumbs): accept a drag on the open-folder crumb too * fix(files): keep the view in a spring-opened folder when an OS upload lands there |
||
|
|
337a53f12c |
feat(cli): Sim CLI with AWS-style profiles and a platform key exchange (#6147)
* improvement(api): pull in the v2 external endpoint surface Cherry-picks improvement/v2-endpoints ( |
||
|
Theodore Li
|
6006870f02 |
feat(credentials): add v2 credential lifecycle APIs (#6664)
* feat(credentials): add v2 OAuth connection APIs * fix(credentials): preserve active OAuth connection links * fix(credentials): bind OAuth links to connection intent * feat(credentials): complete v2 credential lifecycle * fix(credentials): make disconnect idempotent * fix(credentials): stabilize oauth draft retries * fix(credentials): bind oauth callbacks to drafts * fix(credentials): fail closed on oauth completion * fix(credentials): bind shopify completion to oauth state * fix(credentials): align custom oauth reconnects * fix(credentials): centralize application authorization * fix(credentials): keep OAuth draft intent immutable * fix(credentials): allow renamed reconnect targets * fix(credentials): close OAuth draft edge cases * fix(credentials): fail closed without breaking auth * fix(credentials): preserve migrated route behavior * feat(credentials): add provider search * fix(credentials): prevent stale secrets and drafts |
||
|
Theodore Li
|
8fb571ac54 |
feat(credentials): add managed credential groups (#6697)
* feat(credentials): add managed credential groups * fix(audit): sync credential group mock * fix(credentials): serialize enrollment revocation * fix(credentials): isolate managed delegation * fix(credentials): serialize invitation lifecycle * fix(credentials): preserve enrollment lifecycle * refactor(credentials): migrate groups to application boundary * fix(credentials): serialize enrollment readiness * fix(credentials): preserve completed reconnect state * fix(credentials): revalidate policy before grant persistence * fix(credentials): prioritize expired invitations * fix(credentials): redirect unavailable oauth starts * fix(credentials): clarify managed oauth boundaries * fix(settings): complete feature flag test mocks * fix(credentials): clarify enrollment actions and entitlement errors * fix(credentials): preserve entitlement failure reasons * fix(credentials): refine managed oauth flow * fix(lint): use optional chain for pagination |
||
|
Waleed
|
3848f97b4c |
fix(grafana): validate against the API docs, add data source querying and contact-point CRUD (#6712)
* fix(azure-data-explorer): correct the tags ingestion-property example
The example rendered as tags="[''daily'']" — doubled single quotes from an
escaping slip, which is not valid Kusto. The reference writes a tags list
as tags='["TagA","TagB"]': single outer quotes with the JSON array's own
double quotes inside.
The clause builder already handled that form; only the example text was
wrong. A template literal avoids the escaping entirely, since the metadata
generator reads the source verbatim and would otherwise carry the
backslashes into the description the model sees.
Adds a test asserting the reference's exact multi-property clause
round-trips, including the comma inside the quoted array.
* fix(grafana): correct response contracts, required alert fields, and outbound request hardening
Validated against Grafana's HTTP API reference and, where the docs
contradict themselves, against the Go wire structs.
Response shapes the tools got wrong:
- update_annotation declared an `id` that was always 0; a patch returns only
a message, so the request's annotation id is echoed and labelled as such
- delete_folder discarded the numeric id Grafana returns and presented an
input-echoed uid as if it came from the API
- delete_dashboard fabricated `id: 0` / `title: ''` via `||` on absent fields
- the contact-point `provenance` description was inverted: "api" means
API-managed, empty means it stayed UI-editable
Requests that could not succeed:
- create_alert_rule left noDataState and execErrState unset and invisible to
the model, but Grafana's validator rejects an empty value outright, so every
model-driven create failed. Both are now sent with Grafana's own defaults,
and skipped for recording rules, which take a different validator
- get_data_source routed a numeric input at /api/datasources/:id, which exists
only behind an off-by-default feature toggle. UID only now
- list_annotations did not trim the dashboard UID, so a padded value matched
nothing
Outbound hardening on the three proxy routes:
- the service-account token was re-sent to redirect targets; the shared fetch
only drops it when asked, so stripAuthOnRedirect is now set
- no timeout was passed, leaving two sequential hops at the 5-minute default
- upstream error bodies were interpolated whole into the tool result, putting
up to 10MB of HTML into logs and traces; now truncated
- UID path segments are URL-encoded so they cannot re-target the request
- update_folder sent both `version` and `overwrite: true`, which Grafana treats
as alternatives, making the freshly fetched version decorative and silently
clobbering a concurrent rename
- replaced the `any` casts with narrowed types
Block surface:
- 25 outputs the tools emit were undeclared and so unreferenceable downstream;
get_data_source had 13 of its 18 unreachable
- `version` was typed string though the dashboard, folder, and data-source
producers all emit a number
- the dashboard title field was shown only for create, so a dashboard could
never be renamed through Update Dashboard
- six list outputs were typed json rather than array
* fix(grafana): let the health check report ill-health, and disambiguate block outputs
The data source health check could only ever report health. Grafana answers an
unhealthy source with HTTP 400 carrying the same {status, message} payload as a
healthy one, and the tool framework converts any non-2xx into an opaque tool
error — so the diagnostic the caller actually wants was unreachable. The check
now goes through an internal route that reads the verdict off either status and
reports it as a successful check, while a failure carrying no verdict (missing
data source, bad token, plugin with no health endpoint) stays a real error. The
plugin's `details` payload is surfaced too.
Also on that route, matching the other three: an outbound timeout, redirect
auth stripping, a truncated upstream error, and a URL-encoded UID.
Block output descriptions: ten keys are emitted by several tools with different
meanings and were described for only one producer — `database` meant both a
data source name and a health status, `annotations` both an annotation list and
an alert rule's summary map. Eleven `json` outputs were opaque although the
tools already document their inner fields. All rewritten to name every producer.
Smaller alignment fixes:
- the same EmbeddedContactPoint.settings field was typed `object` in list and
`json` in create
- list_contact_points mapped non-nullable uid/name/type through `?? null`;
Grafana returns an empty string, which is what create already assumed
- create_alert_rule sent `orgID`, which Grafana overwrites from the
authenticated context, and `Number()` on a non-numeric value put NaN -> null
in the body
- the three update routes declared `output` as required though the auth
short-circuit omits it, and did not declare the `details` they emit on a
validation error
* feat(grafana): complete contact-point CRUD, and add folder move and rule-group read
Four operations the integration was missing, taking it to 29.
update_contact_point / delete_contact_point close a real gap: contact points
could be listed and created but never corrected or removed. Two things worth
recording, because the published docs get both wrong:
- both verbs answer 202 with only a message, not the object. The rendered docs
claim delete returns 204; the current spec and handler both say 202. So the
UID is echoed from the request, the way delete_folder and update_annotation
already do
- update is a full replace with no PATCH counterpart, so name, type, and
settings are all required and the description says so. Omitting
disableResolveMessage resets it
X-Disable-Provenance is exposed on update only. Its polarity is the opposite of
the alert-rule case: omitting it always succeeds, while sending it against an
API-provisioned contact point is rejected — with 403, not the 409 rules use. It
is not exposed on delete at all, because that handler never reads stored
provenance and the endpoint takes no such parameter.
move_folder reuses get_folder's mapping verbatim — same DTO. It always sends
the parentUid key, since Grafana reads an empty value as "move to the root",
which a conditionally-omitted field could not express.
get_alert_rule_group surfaces the group evaluation interval, the one alerting
knob the per-rule operations cannot reach. It reuses the shared mapAlertRule for
the nested rules, and the interval is documented as an integer of seconds.
* feat(grafana): add data source querying, and ground the skill and templates in real tools
query_data_source closes the largest gap in the integration: 29 tools could
read dashboards, folders, and alert configuration, but none could read a metric
value. It posts to /api/ds/query and returns both the raw response and the
frames flattened into rows.
The flattening is derived from the documented layout rather than any data
source's field names: a frame carries schema.fields[] alongside data.values[],
where values[i] is the whole column for fields[i], so zipping them by position
works for Prometheus, SQL, or anything else with a backend.
A failed query is a 400 by Grafana's own status table, so it stays a tool
error — unlike the health check, where the failure status carries the answer.
That also lets four templates and the review-firing-alerts skill stop promising
things the integration could not do. Three templates assumed a metric-query
tool, which now exists. The fourth, and the skill, assumed live alert instance
state, which the provisioning API never returns — they now derive firing rules
from alert-state annotations, which are documented to carry newState and
prevState, and say so explicitly rather than implying a live snapshot.
Deliberately not added: a tool over /api/prometheus/grafana/api/v1/rules for
live instance state. That endpoint appears on no Grafana HTTP API doc page, its
response is only readable from Go internals and test assertions, and the
instance-level state casing differs from the rule level with no documented
contract. Not something to build an output schema on.
* fix(grafana): declare the two block outputs the earlier fixes introduced
Renaming update_annotation's phantom `id` to `annotationId` and adding
`details` to the health check both created outputs the block never declared, so
neither was referenceable downstream. Caught by re-running the output-coverage
check over both integrations; the block now covers all 64 keys the 30 tools emit.
* fix(grafana): make Update Contact Point actually usable from the block
The new replace operation could never succeed. contactPointType and
contactPointSettings were widened to cover it, but contactPointNameNew was
left create-only — and the update maps `name` from that field, so the required
parameter was never supplied.
disableResolveMessage had the same gap, and it matters more than it looks:
the update is a full replace, so a block-driven update was silently clearing
resolve suppression on every contact point it touched. Both fields are now
shown, and required where the API requires them.
Also states a reason on each intentionally-unconstrained response field —
Zod issue objects, alert query stages, notification settings, recording-rule
config, and data-source health detail are all genuinely opaque, but that was
left implicit.
|
||
|
Waleed
|
a7115e87ee |
feat(integrations): add Azure Data Explorer (#6701)
* feat(integrations): add Azure Data Explorer Add a 14-operation Azure Data Explorer (Kusto) integration covering KQL queries, schema and metadata discovery, table management, inline and query-sourced ingestion, ingestion-failure triage, and arbitrary management commands. Authentication uses a Microsoft Entra service principal through an internal proxy route, since the Kusto token audience is per-cluster and cannot be expressed as a static-scope OAuth provider. * fix(azure-data-explorer): only read partial-failure status from the QueryStatus table Scanning every returned table for Severity and StatusDescription columns misread an ordinary query as a failed request whenever the user's own result selected columns of those names — a common shape for a log table. Failure detection now consults only the table the response's table of contents names as QueryStatus, and primary-result selection reuses the same index instead of re-reading it. * fix(azure-data-explorer): keep the Show Operations and Show Table Details cards from painting empty check:canvas-sentences flagged the Show Operations sentence: it anchored `core` on operationId, which is an advanced-mode optional field, so an untouched card resolved to nothing and painted empty. Show Table Details had the same shape in milder form — table is optional there, since omitting it describes every table, leaving a dangling preposition. Both now lead with literal copy and treat their field as an optional refinement. Also simplifies the primary-table condition to a single `!= null` check. * fix(azure-data-explorer): authenticate sovereign clusters against their own Entra authority The cluster allowlist accepted Azure China and US Government hosts, but every token request went to login.microsoftonline.com. Those clouds are isolated instances with their own Entra endpoints, so a sovereign cluster passed URI validation and then could never obtain a token. Each Kusto service domain is now declared alongside the authority that issues tokens for it, so the two cannot drift apart, and the authority is part of the token cache key. * improvement(azure-data-explorer): warn that ingest-from-query matches columns by position Kusto aligns an ingested query result to the target table on column type and order, never on column name, so a query projecting the right columns in the wrong order lands data in the wrong columns without erroring. Surfaces that in the tool description and param the model reads, in the wand prompt that generates the query, in the rollup skill's steps, and in the docs. Also verifies the target schema first rather than after. * chore(azure-data-explorer): drop the unsourced kustomfa host from the cluster allowlist Every other entry traces to a Microsoft reference — the Kusto connection-string doc, the national-cloud endpoint tables, and the Fabric KQL-database REST reference. kustomfa.windows.net does not, and the connection-string doc states the trust boundary as hostnames ending in kusto.windows.net. An allowlist should only hold hosts we can justify, so this drops it and records the sourcing standard for anything added later. * fix(azure-data-explorer): handle commas inside quoted properties and empty extent IDs Two defects in the shared command helpers: buildWithClause split the property list on every comma before validating, so a value that legally contains one — a docstring sentence, or a tags array with more than one entry — was torn in half and rejected. Splitting is now quote-aware, and an unterminated quote is rejected outright rather than swallowing the rest of the clause. transformColumnListResponse dropped empty strings, but `.ingest inline` reports "no data shards were generated" as a single record carrying an empty extent ID. A no-op load therefore looked like a missing column instead of an empty result. Only non-strings are skipped now. |
||
|
|
1424809614 |
feat(netsuite): add Oracle NetSuite integration (#6476)
* revise netsuite integration * fix(netsuite): align selector route with snowflake * test(netsuite): remove selector route coverage * test(netsuite): align coverage with snowflake * fix(netsuite): complete integration validation * refactor(netsuite): align integration with codebase patterns * test(netsuite): correct async job citation * fix(netsuite): address final audit findings * fix(netsuite): surface upsert/transform Location, relax task link check Oracle documents the Location response header for create and update, and both tools already require it. Upsert and transform also produce a record but Oracle documents no response headers for either, so they dropped the header entirely and the new record's ID was unreachable. Add a `resource-optional` location mode that captures Location when NetSuite sends it and never fails when it is absent, and wire it to upsert and transform along with their tool and block outputs. Async task discovery rejected the whole response if any task link carried a rel other than `self`, collapsing the picker into a 502. Oracle documents a `self` link per task but never guarantees it is the only one, so skip other relationships and fail only when no self link exists. Also use the shared `truncate` helper in the error sanitizer per the repo convention instead of an inline slice. * fix(netsuite): validate SuiteQL pages against their documented shape The shared collection-page validator required links, items, count, hasMore, offset, and totalResults on every 200, and a missing field turns a successful call into a reported failure. Oracle documents all six for record collections and SuiteAnalytics dataset pages, but its SuiteQL reference lists only links, count, offset, totalResults, and items. A documented SuiteQL response that omits hasMore would therefore have been rejected. Split out a suiteql-page validator that requires the five documented SuiteQL fields and type-checks hasMore only when the account returns it. Record collections and dataset pages keep requiring all six. * chore(netsuite): regenerate tool metadata after rebase on staging The rebase conflicted only in the generated tool-id, tool-metadata, and tool-output artifacts, which NetSuite and the newly landed LogRocket integration both extend. Regenerated from the merged registries: the result is staging's catalog plus the 27 NetSuite tools, with LogRocket's entries intact and no other tool changed. --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Waleed
|
6de8ba2504 |
fix(v2): close the correctness gaps an end-to-end audit found (#6655)
* fix(v2): stop a third-party tool description from 500ing MCP discovery
`v2McpToolInputSchema` declared `description: z.string().optional()` inside a
`.catchall(z.unknown())` object, and a declared key beats the catchall. The MCP
SDK's own `ToolSchema.inputSchema` does not declare `description` at all, so any
value — including the JSON `null` a Python server emits for an absent one —
passes its validation and reaches Sim unchecked. The builder's outbound `.parse()`
then threw, and the discovery error policy correctly declines to classify a
Sim-side schema defect, so the endpoint that completes MCP onboarding answered a
bare 500. The key is dropped and left to the catchall; `type`, `properties`, and
`required` stay pinned because the SDK enforces those at least as tightly.
Also in the v2 resources family:
- The single-resource query schemas for MCP servers, skills, custom tools, and
secrets are now `.strict()`, matching every list in the same family. A mistyped
flag was silently ignored behind a 200.
- `openapi/resources.ts` re-derived `RESOURCE_ERRORS` and
`RESOURCE_CONFLICT_ERRORS` inline in 21 of 22 operations. They now import the
shared constants; the generated spec is unchanged, which is the point.
- The internal MCP refresh route stamped `updatedAt` alongside `lastToolsRefresh`.
`updatedAt` means "configuration last changed" and is a public keyset sort, so
a refresh moved rows out from under an in-flight page. `updateServerStatus`
already held that invariant; the route now matches it.
- The discovery cooldown is a typed `McpServerCooldownError` rather than a
substring search for `cooldown`. `McpConnectionError` interpolates the server's
display name into its message, so a server named after the word was reported as
a transient cooldown when its connection had genuinely failed.
* fix(v2): close correctness gaps in the workflows deployment surface
Deploy and rollback bodies were plain objects, so a misspelled key was
stripped rather than rejected. On rollback that is silent misbehavior:
an omitted `version` legitimately means "reactivate the preceding
version", so `{"versoin": 5}` rolled back somewhere else and answered
200. Both v2 bodies, the run-read query, and the versions cursor are now
strict.
Deployment versions are an `integer` column, but the path param, the
versions cursor, and the v1 body each bounded it differently or not at
all — an out-of-range value overflowed the comparison into an
unclassifiable 500. One exported bound now covers all three.
Resume admission raised bare `Error`s for a stale contextId or an
already-resumed run, which the resume surfaces could not classify and
reported as 500. They now use the sibling `ResumeAdmissionError` already
in that file, carrying 404/409/400 and whether an automatic retry can
clear the refusal.
Docs corrections: rollback publishes the 409 its webhook-path conflict
already produces; deploy/undeploy/rollback reject a workspace key with
403, not the concealed 404 they documented; the workflows OpenAPI module
imports the shared error sets instead of re-deriving them; import and
the folder ops explain their folder-tree 413. The export route is marked
`headSafe: false` so a HEAD probe stops filing a WORKFLOW_EXPORTED audit
event for an export that never happened. `runId` is one bounded schema
across the run and log resources.
* fix(v2): conceal knowledge upload existence, tighten knowledge/files bounds
Security: the four knowledge document-upload routes rendered a bare upload
error policy with no resource concealment, while every sibling knowledge route
uses one. Because the use case resolves the knowledge-base context before
workspace authorization, the unconcealed 403 told any valid API-key holder that
a knowledge base exists in a workspace it cannot reach — the exact signal
GET /api/v2/knowledge/{id} withholds by answering 404 either way. All four now
use the composed concealing policy, which also renders the 415/402/413 the
route-local renderer already handled; that duplicate renderer is deleted.
Contracts:
- POST /knowledge/search is strict. It was the only non-strict v2 request body,
so a mis-cased rerankerEnabled or topK returned 200 with the key stripped,
changing what the caller was billed and silently disabling reranking.
- The document list takes limit, cursor, and search from the shared v2 schemas.
search was an unbounded, empty-accepting v1 string, so ?search= answered 200
with a full page here and 400 on GET /knowledge, and the term reached an
unindexed filename LIKE scan with no ceiling.
- The 16 non-strict single-field workspace query slices across both families are
strict, matching GET /knowledge/{id}/tags.
- GET /audit-logs takes workspaceIdSchema instead of a bare string (?workspaceId=
was forwarded as a filter and returned zero rows) and the shared run-window
bounds for startDate/endDate.
Documentation:
- listAuditLogs drops the 404 it has no code path to emit.
- upsertFileShare describes its workspace-key refusal as the 403 it renders;
the operation denies the key by principal kind, which the concealment policy
does not rewrite.
- The 12 body-reading knowledge and files operations publish the 413 their
pre-validation body read raises, and the file list publishes the folder-tree
413 its now-capped path index raises.
Correctness: queryWorkspaceFilePage loads its folder path index under
MAX_FOLDERS_PER_WORKSPACE like the workflow, table, and knowledge lists. An
uncapped index does not fail on truncation, so a real folder outside the read
rows resolved to undefined and answered "Folder not found".
* fix(v2): publish the reachable 413 on body-carrying resources ops
`parseRequest` buffers a JSON body through `parseJsonBody` under
`DEFAULT_MAX_JSON_BODY_BYTES` before any schema runs, and the v2 builders supply
`V2_PARSE_DEFAULTS.payloadTooLargeResponse`, so every operation whose contract
declares a body already answers 413 above the cap. The resources family
published it on none of them. A status a caller cannot see in the spec is a
status they will not handle.
Adds `RESOURCE_BODY_ERRORS` and `RESOURCE_CONFLICT_BODY_ERRORS` to the shared
sets and applies them to the seven affected operations: createMcpServer,
updateMcpServer, createSkill, updateSkill, createCustomTool, updateCustomTool,
and setSecret. All seven are `defineV2JsonRoute` handlers on non-GET methods
with no `parseOptions` override, so the 413 is genuinely reachable on each. The
new sets are opt-in rather than folded into the base sets precisely because
reachability is not automatic — an operation with no body, or one whose payload
reaches it through an uncapped path, would be publishing a response that can
never arrive.
A sweep test pins the invariant across the resources, billing, and logs
documents. It is one-directional by construction: several bodyless operations
publish 413 for their own folder-tree and render ceilings, so the converse would
flag correct documentation.
Also completes the shared-constant consolidation started in
|
||
|
Waleed
|
c49751b32e |
perf(prefetch): stop calling our own API over the wire during server render (#6657)
* perf(prefetch): read the data layer instead of calling our own API over the wire
Four server-render prefetches went out over HTTP to our own routes. With
INTERNAL_API_BASE_URL unset in prod, getInternalApiBaseUrl() falls back to the
public base URL, so each was RSC -> public HTTPS -> load balancer -> back into
the app, awaited inside the render with a second round of auth.
- /home fetched the workflow folder list that the workspace layout had already
fetched, under the identical query key. Since getQueryClient() builds a new
client per call on the server, the two never deduped: same data, twice a
request, once directly and once over the wire. Dropped; the layout's entry
already hydrates it.
- /home cached raw route JSON under workspaceFilesKeys.list, while
files/prefetch.ts seeds that same key from listWorkspaceFilesWithShares. The
contract declares the date fields z.coerce.date(), so consumers hold Dates —
a file record's type depended on which page the viewer landed on. Now reads
the same function files/prefetch.ts does.
- tables and knowledge folder reads now call listFoldersForWorkspace, matching
the sidebar prefetch.
These reads carry no authorization of their own, so each surface proves the
viewer through getWorkspaceHostContextForViewer first and caches nothing when
it fails, leaving the client fetch to reach the route for the real 403. Both it
and getSession are cache()d and already resolved by the layout, so the proof
costs no extra queries.
Left on the wire, deliberately: the tables and knowledge lists, whose cached
shape is the serialized wire shape, and pinned items and members, which have no
exported data-layer function.
* improvement(prefetch): skip the viewer proof when there is no session
Passing an empty-string userId ran a real permission query that could only
return null. Take an optional userId instead and skip straight to the
unauthorized path, matching how the home prefetch is called.
* perf(prefetch): finish removing self-HTTP prefetches and delete the legacy helper
Converts the last four server-render prefetches that called our own API over
HTTP, and deletes prefetch-internal-fetch.ts now that nothing imports it.
- knowledge bases: runs the route's own listInternalKnowledgeBases use case
with a principal from the same internalSessionAuth policy the route declares,
then projects through the same presenter and contract. Not a bypass of the
application boundary — the same path, called in-process.
- tables: extracts the route's list projection into lib/table/wire.ts as
toTableListItem, which the route and the prefetch now both call. This matters
because listTablesContract's response schema is a passthrough z.custom, so a
client fetch caches the route's JSON verbatim. Seeding listTables() directly
would have put Date objects and the server-only metadata field under a key the
hook never sees them on.
- pinned items: extracts the route's inline query into lib/pinned-items/queries.ts
as listPinnedItemsForUser, which the route now calls too.
- workspace members: getWorkspaceMemberProfiles already existed; the prefetch
calls it directly.
normalizeColumn moves from app/api/table/utils.ts to lib/table/wire.ts with ten
importers repointed. That also removes a pre-existing lib/* -> app/api/* boundary
violation in lib/table/import-runner.ts. No response shape changes: the v1/v2
edits are import-path moves only.
Every converted read proves the viewer first and caches nothing when that fails,
so an unauthorized viewer's client fetch still reaches the route for the real
403. Authorization equivalence was checked by unfolding both paths to
checkWorkspaceAccess rather than assumed.
* improvement(prefetch): collapse the duplicated folder prefetch and unify the call shape
- Extract prefetchResourceFolders. The same eight-line folder prefetch was
written three times, varying only by resourceType, with the key, stale time
and mapper kept in sync by hand.
- Adopting it removes the conditional spread from the tables and knowledge
prefetches. Tables can now early-return, matching prefetchFilesBrowser:
prefetchResourceListChrome already self-guards on the same cached host
context, so a null context meant the function did nothing either way.
- Take userId as string | undefined everywhere and guard inside, so every
prefetch module has one calling convention rather than two.
- Export toWireTimestamp and use it for the create-table response's own copy of
the same idiom, and drop a cast that the extraction made dead: the parameter
is already TableDefinition, whose schema is TableSchema.
- Read params and the session concurrently on the tables and knowledge pages,
matching the files page, and drop TSDoc that restated each prefetch's own.
* fix(prefetch): keep the tables list on its route and cut the executor edge
Reading listTables from a page prefetch put the executable tool registry into
the Tables page server graph — ~4,700 modules, which check:tool-registry-boundary
rejects. lib/table/service reaches workflow-columns by several independent
paths (directly, and through jobs/service and rows/service), so severing one
edge is not enough; untangling that belongs in its own change.
- The tables list goes back through GET /api/table, with the reason recorded so
the next person does not repeat the attempt. Folders and chrome on that page
stay on the data layer.
- stripGroupDeps moves to its own leaf module. It is a pure projection over a
WorkflowGroup, but living beside the group runtime meant every importer of
lib/table/service paid for the executor to get it.
Net effect on the Tables page graph: 2,186 modules to 1,742.
* perf(prefetch): finish the migration, delete the legacy helper, ratchet page graphs
Answers the question the previous commit left open: the tables list did not have
to stay on HTTP. lib/table/service reached the executor through
jobs/service -> rows/service -> workflow-columns, for one symbol.
pendingDeleteMask is a delete-visibility SQL clause with no executor
involvement, so it moves to its own leaf and that chain is cut. The tables
prefetch now reads the data layer like every other one, and
prefetch-internal-fetch.ts is deleted: nothing in the app calls its own API over
HTTP during a server render any more.
stripGroupDeps likewise moves to a leaf rather than being re-exported through
workflow-columns, so its importers no longer pull the executor to get a pure
projection.
React Query mechanism fixes, all found by audit:
- settings/[section] fired two prefetches without awaiting them. Only a settled
query is dehydrated, so those were shipped mid-flight; a rejection hydrated
into an error state retryOnMount: false never retries, leaving the panel
broken for the session. Awaited now, and the pending-dehydration opt-in is
removed since nothing streams.
- The viewer profile was prefetched by both the layout and the settings page.
Separate server QueryClients mean that was a real second read per request.
- prefetchSubscriptionData was dead, and hand-rolled an unannotated raw fetch.
- retry is scoped to the browser. Query core defaults it to 0 on the server;
stating one value for both opted awaited prefetches into a retry backoff. The
gcTime default is dropped entirely — 5 minutes is already the browser default,
and setting it explicitly overrode the server's Infinity, leaving a live timer
and payload per request.
check:tool-registry-boundary now also ratchets per-page module counts against a
committed baseline, attributing a regression to the import that caused it via a
dominator tree. It caught a +444 regression in this branch by hand; it would
have caught it in CI. Its import regex also missed bare side-effect imports,
so `import '@/tools/registry'` could have slipped past it entirely.
Prefetch guidance added to .claude/rules/sim-queries.md.
* fix(prefetch): correct the extracted module's db imports and stale rationale
Audit findings from the migration.
- pending-delete-mask imported its schema tables from @sim/db rather than
@sim/db/schema, which the module it came from was careful to split. The
global test mocks are bound per-entrypoint and only the schema mock exports
tables, so every suite that reaches pendingDeleteMask would have failed on a
missing mock export. Restored to the original convention, and the same split
applied to the new pinned-items queries module before it grows a test.
- The settings prefetch and page justified awaiting with a mechanism this
branch removed — pending queries being shipped with their promise. Only a
settled query is dehydrated now, so an unawaited prefetch is dropped from the
payload entirely. Same conclusion, correct reason, and no longer contradicting
the rule this branch added.
- Removed the doc block left orphaned above validateSchema when stripGroupDeps
moved out of workflow-columns.
Skill projections regenerated after trimming the boundary skill.
* chore(table): drop a section separator comment
Separators like these are non-TSDoc decoration that CLAUDE.md already rules
out. This is the only one in a file this branch touches; the rest of the repo
is swept separately.
* chore: remove section separator comments
CLAUDE.md already rules these out ("No ==== separators. No non-TSDoc
comments"), but 546 of them had accumulated across 48 files. They decorate
rather than explain, and they drift: a separator says "Validation" while the
code beneath it moved elsewhere, as one in workflow-columns already had.
Pure deletion — no source line was touched, and lines inside template
literals were skipped so nothing in a generated string changed.
|
||
|
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.
|
||
|
|
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
|
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
|
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
|
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
|
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. |
||
|
Justin Blumencranz
|
a72457b9f5 | fix(docs): preserve items response fields (#6587) | ||
|
Waleed
|
9b9f4ee596 |
fix(v2-api): close three secret disclosures, make the surface consistent, and align docs with signatures (#6560)
* fix(v2-api): close two secret disclosures and align docs with signatures
Two P0 disclosures, five correctness bugs, and the standardization and
guard work that came out of auditing them.
**Secret disclosure — workflow version state.** `GET /api/v2/workflows/{id}/
versions/{version}` served the deployed graph unsanitized, so a read-role
workspace API key received plaintext block-password values and OAuth
credential ids. The sibling export route has always sanitized. Every other v2
response is protected structurally because the builder re-parses it, but this
field is `z.custom<WorkflowState>()` — a predicate that validates nothing —
which is why it survived earlier audits. Sanitization now lives in the use
case, secure by default, with a named `includeCredentialValues` opt-in that
only the session-authed deploy-preview route sets.
**Secret disclosure — MCP headers.** The internal list and update routes
returned custom `Authorization` headers verbatim to any read-role member;
headers are stored unencrypted. Values are now gated on write permission and
projected through one shared helper. The settings UI genuinely prefills from
them, so blanking outright would wipe headers on unrelated edits — write-only
headers plus encryption at rest are the follow-up.
Correctness:
- v2 execute ignored `X-Sim-Via`, resetting the call chain on every hop and
defeating the recursion guard. Wired on both the keyed and anonymous paths.
- v2 knowledge search accepted `searchMode` and dropped it, silently serving
vector-only results for a hybrid request, and allowed 50MB bodies where
internal caps at 2MiB.
- v2 run cancel never released the plan concurrency slot and half-cancelled
group runs; a group conflict now returns 409 instead of reporting success.
- v2 table row writes stamped no secret provenance, so the next internal read
reported the whole page incomplete. `secretProvenance` is now required on
the primitives, making the next omission a compile error.
- Folder conflicts and malformed paths returned 500; they are 409/404/400 now.
`FolderPathError` splits from `FolderHierarchyError` so a corrupt stored
tree stays a 500 and stays in 5xx alerting.
Standardization and documentation:
- `PUT /files/{id}/share` -> PATCH. The resource is not round-trippable
(`hasPassword`, never the password), so merge-on-omission is the only
implementable semantics.
- ~40 spec truthfulness fixes: a 410 the API cannot emit, eight 423s with no
lock guard, ~30 reachable-but-undocumented 404/400/413s, and six inverted
field claims. Eleven operations that always reject a workspace key now say
so — four of them answer 404, so a workspace key was told the resource did
not exist.
- `NAME_PATTERN` lost its `/i` through `z.toJSONSchema`, publishing 15
patterns that reject names the runtime accepts. Every generated client
rejected any capitalized table or column name, and two of the spec's own
examples failed the spec's own schema.
Guards, so these classes cannot recur:
- `check:route-verbs` (new) cross-checks all 212 builder routes' exported verb
and path against their contract. The builders only compare at runtime, so a
half-done rename previously passed CI and 500'd in production.
- Example validation now runs against the published JSON Schema with formats
on, covering 225 nodes instead of 100 — this is what caught the regex bug.
- The list-pagination sweep is union-aware and fails loudly on a schema it
cannot introspect, rather than counting it compliant.
* refactor(v2-api)!: flatten the single-resource response envelope
BREAKING: 31 endpoints that returned `{ data: { <resource>: T } }` now return
`{ data: T }`.
This corrects drift, not a design decision. PR #5273 added skills, custom
tools, MCP servers, secrets, and knowledge nested while adding workflows,
files, and logs flat — and in the same commit wrote the `v2/shared.ts`
docblock declaring `single resource: { data: T }` is the standard. The nested
half appears to have been modelled on the v2 tables surface (#6067), which
landed twelve days earlier. Lists were already `{ data: T[], nextCursor }`, so
flat single-resource is what actually matches them; nesting made every client
destructure a layer that carries nothing.
Doing it now because the cost only grows: `v2-api` is still dark-launched, so
today this breaks no one. After GA it needs a deprecation window.
Payloads that carry real information were deliberately left alone — this was a
classification exercise, not a mechanical sweep. Unchanged: delete
acknowledgements (`{ id, deleted }`, `{ path, deleted, deletedItems }`), the
knowledge search envelope (which echoes query, knowledgeBaseIds, topK and
totalResults alongside hits), upload payloads carrying signed tokens and
transfer instructions, bulk-operation counts, `{ row, operation }` upserts,
named acknowledgement scalars (`{ dispatchId }`, `{ cancelled }`), and
`{ columns: [...] }` — a collection, where a bare `{ data: T[] }` would be
indistinguishable from the list envelope but without `nextCursor`.
Also flattened the two file-share responses, which were not in the original
survey: leaving them would have put one resource in two shapes on one path.
`GET /files/{id}/share` now returns `{ "data": null }` when a file has never
been shared.
No consumer is affected. Both SDKs touch exactly two v2 endpoints — execute
and run status — and both were already flat. No docs MDX, client hook, or
internal caller reads a changed response; Copilot table tools call the
application use cases directly rather than the HTTP surface.
The shared `v2FolderSchema` is untouched: every folder flatten was achievable
at the response site, which is itself evidence flat was the intended shape.
* fix(v2-api): close a third secret disclosure and make concealment coherent
**Secret disclosure — run snapshot.** `GET /api/v2/logs/{runId}` returned
`workflowState` straight from `workflowExecutionSnapshots.stateData`, which is
the workflow graph: `blocks[].subBlocks[].value` holds `password: true` field
values and `oauth-input` credential ids. Nothing on that path sanitized it, and
the field was typed `z.unknown()`, so the builder's response parse stripped
nothing. A read-role workspace API key could read plaintext credentials.
This is the third instance of one pattern, and the pattern is the finding: the
builder protects every response by re-parsing it, so the only fields that can
leak are the ones typed `z.unknown()` or `z.custom()`. Both prior disclosures
sat behind exactly such a field. The snapshot is now sanitized in the use case
and the field is typed object-or-null. An inventory of every remaining
`z.unknown()` in the v2 contracts is in the PR description; two carry data with
no projection behind them and are named there as follow-ups.
**Concealment was bypassable.** `createV2ResourceConcealmentPolicy` rewrites
resource-authorization failures to 404 so a caller cannot probe for existence.
Workflows and files applied it on every verb; tables and knowledge applied it
only on reads. A caller could therefore probe with PATCH, read the 403, and
learn the resource exists — the read-side concealment bought nothing. Nine
mutation sites now conceal, plus the three table-column verbs, which were
inconsistent with their own sibling sub-resources.
`lib/logs/api/route-policies.ts` was a second, divergent implementation that
sniffed `response.status === 403` and so also swallowed workspace-policy
denials the canonical helper deliberately preserves. It now uses the helper. A
third such sniff survives in the upload-control helper and is noted as a
follow-up.
Also:
- `DELETE /tables/{tableId}/rows/{rowId}` returned the bulk `{deletedCount,
deletedRowIds}` shape while nine sibling single-resource deletes return
`{id, deleted}`. It now matches them.
- Nine operations can 404 on an unknown folder path and did not document it;
`createWorkflow` could 413 on an oversized folder tree and did not; getting a
run can 409 when trace data was truncated and did not.
- `queryTableRows` documented a 413 it cannot emit and `resumeWorkflowRun` a
423 with no lock guard anywhere in its path — the same un-producible-status
class already cleared for 410 elsewhere.
- Execute's 409 description covered only the run-id case after the
recursion-guard fix added a second cause, and named a code the route does not
emit: the wire carries `error.code: CONFLICT` with the specific cause in
`error.details.code`. `x-sim-via` is now a declared request header.
- Deploy and rollback published examples that were impossible: `isDeployed:
true` beside `activeDeployment: null`, where the route computes the former
from the latter.
- `afterRowId`/`beforeRowId` were published on row insert and silently dropped
by the route, so a positional insert became a tail append.
- A generated document whose script fails permanently answered "still being
generated, try again" forever; the underlying cause is now preserved.
* docs(v2-api): correct eleven false or misleading spec claims
Structural parity between contracts and specs is CI-enforced; semantic truth is
not. These are claims the spec made that the code does not honour.
Outright false:
- `DELETE /files/{fileId}` said it deletes "the stored bytes". It archives:
the row is retained with a deletion timestamp and the bytes are never
removed. Restore exists, but only on the internal API, so the description now
says so rather than implying v2 offers it.
- Execute documented `409 EXECUTION_ID_CONFLICT` in three places. The wire
carries `error.code: CONFLICT` with `error.details.code: RUN_ID_CONFLICT`;
only v1 ever emitted the documented string.
- The files spec claimed every endpoint uses the canonical envelopes while
`GET /files/{fileId}` returns octet-stream.
- The shared timestamp rule justified itself with a rendering claim that is
false — 29 bare-form sites publish `format: date-time` identically. The real
difference is runtime validation, so the rule now says that. It was softened
rather than enforced: responses are re-parsed, so adding `.datetime()` to a
field whose producer can emit a non-ISO string turns a working read into a
500, and that could not be proven for all 29 without a much larger audit.
Misleading:
- The billing ledger silently defaults to a 30-day window, so a client
paginating to `nextCursor: null` believes it has the whole ledger.
- Deleting a connector-backed knowledge document does not delete its chunks —
the row survives as excluded and the embeddings remain.
- `listTables` said "all tables"; it is keyset-paged with a default limit.
- `GET /files/{id}/share` omitted the `data: null` never-shared case its own
schema and example already declare.
- The share PATCH matrix omitted two hard 400s, so following it literally
against a never-shared file fails.
- Five knowledge operations render a canonical folder path back and can 413 on
an oversized tree without carrying the sentence that says so.
Also: the upload-control helper was a third implementation of concealment by
sniffing `response.status === 403`, which masks workspace-policy denials the
canonical helper deliberately preserves. It now uses the shared policy, so
those denials keep their 403. And the shared docblock's search-field
enumeration was presented as exhaustive while omitting two lists, and its
error-envelope claim omitted the two upload data-plane routes that emit a bare
`{error: string}` — both now carry the carve-out the CI allowlist already had.
* test(v2-api): align upload concealment test with cross-tenant-only semantics
#6557 narrowed `createV2ResourceConcealmentPolicy` to conceal only the three
cross-tenant authorization classes, deliberately letting a same-workspace
policy denial keep its 403 so the caller learns why. My test predated that and
asserted a workspace-key denial was concealed as 404.
Split into two cases that pin the distinction rather than paper over it: a
cross-tenant reach conceals, a workspace-key policy denial does not.
* fix(v2-api): accept the redacting log status and envelope the knowledge-search 413
The v2 log presenters parsed status against a five-value enum, but the
execution logger persists a sixth, redacting, while a finished run's output
is scrubbed. Any such row failed the response parse; on the list route one
row 500'd the whole page. The enum is now derived from
PersistedWorkflowExecutionStatus with a compile-time exhaustiveness
assertion, so a future status is a type error rather than a production 500.
POST /api/v2/knowledge/search declared maxBodyBytes without
payloadTooLargeResponse, so its 413 returned a bare string instead of the v2
error envelope. It now matches the sibling deploy/rollback routes.
* fix(uploads): restore archive extraction folder parity
Archive extraction into workspace files/ was rewritten onto the authorized
application-operation boundary, and three behavioral regressions came with
that move. Together they broke every archive containing a subdirectory, and
100% of copilot extract() calls (materialize-file always passes
rootFolderSegments: [baseName], and its catch only handles ArchiveError).
1. Non-canonical folder path. The extractor joined the folder segments with
"/" and passed the result as `path` to createWorkspaceFileFolderOperation.
That path reaches requireNonRootFolderPath -> parseFolderPath, which
requires a leading "/" and byte-for-byte canonical per-segment encoding,
so "bundle/data" threw FolderPathError before anything was written — and
a folder name containing a space or a reserved character would still have
thrown after merely prefixing a slash.
2. exactName: true. createWorkspaceFileFromBuffer was told to demand the
exact leaf name, which sets maxAttempts = 1 and raises FileConflictError
when the name already exists. The extractor's rollback then deleted every
file written so far, so one colliding name destroyed the whole
extraction. Reachable today for flat archives through the unzip action of
POST /api/tools/file/manage. Restored to auto-suffixing via
allocateUniqueWorkspaceFileName.
3. Wrong folder primitive. createWorkspaceFileFolderAtPath creates exactly
one leaf, conflicts on an existing path, and requires the parent to exist
already. The extractor never creates intermediates and caches by full
path, so the first nested entry asked for a folder whose parent was never
created. The correct semantics are ensureWorkspaceFileFolderPath: walk
every segment, reuse what exists, create only what is missing.
Rather than bypass the operation boundary by calling the manager primitive
directly, this adds ensureWorkspaceFileFolderPathOperation — an authorized
application use case under files.folders.create that expresses "ensure this
whole chain exists" — and routes the extractor through it with raw decoded
segments, so no path string is built and no encoding can be malformed.
archive.test.ts previously mocked the folder operation and asserted the
broken shape (path: 'bundle'), which is why this shipped. The suite now
fakes the workspace-file store in memory while enforcing the real rules:
folder paths run through the production parseFolderPath family, the
create-one-leaf operation conflicts and requires a parent, and exactName
governs conflict vs auto-suffix. Nested, reuse, encoded-name, and collision
cases are covered and each fails against the pre-fix code.
* chore(files): tidy archive extraction cleanup
* fix(uploads): roll back folders archive extraction created
Extraction now materializes folders before uploading files, but the failure
path only deleted the extracted files — every folder the call created was left
behind. That is not cosmetic: `materialize_file` guards re-extraction by looking
up the root folder path and refusing when it has any child, so a half-extracted
nested archive turned every retry into "already extracted — delete that folder
first" until a human cleaned up the tree by hand.
The rollback must delete only folders this call actually inserted, never one it
reused: extracting into an existing path is normal (a sibling entry, an earlier
successful extraction), and deleting a pre-existing folder would destroy
unrelated user data. `ensureWorkspaceFileFolderPath` already distinguishes the
two while walking the segment chain, so it (and its application operation) now
reports `createdFolderIds` alongside the leaf id. The extractor accumulates
those ids in creation order and, on failure, deletes them in reverse — parents
are recorded before their children, so reverse order is deepest-first and a
parent is never removed out from under a child. Folder cleanup is best-effort
like the existing file cleanup, so a cleanup failure never masks the original
error.
* fix(billing): withhold the payer credit pool from v2 status readers
`GET /api/v2/billing/status` resolved the workspace's payer and projected
that payer's pooled allowances — credits used, credit limit, credits
remaining, and the payer entity's storage usage and quota — to any caller
holding only `read` on the workspace, including a personal API key. The
payer pool is shared across every workspace that payer funds, and the
platform already treats it as privileged: the workspace credit-availability
surface computes `canViewPayerPool` from `canManageWorkspaceBilling` and
substitutes member-scoped or null figures for everyone else. The new
versioned endpoint had no equivalent gate.
`credits` and `storage` are now projected only to a caller who may manage
the resolved payer's billing: the billed account holder of a personally
hosted workspace, an admin of the hosting organization, or a workspace API
key, which only a workspace admin can provision. The endpoint stays at
`read` so a plain member keeps the plan, period, and standing the workspace
UI already shows them, and an exceeded pooled limit still reports as
`limit_exceeded` without disclosing the numbers behind it. Both fields are
nullable on the wire and in the regenerated OpenAPI spec.
The decision lives in the application use case, resolved from canonical
workspace state, not in the route: billing authority is payer identity and
organization role, which the workspace permission ladder cannot express —
a plain workspace `admin` is deliberately not enough.
* chore(api): remove the unused public API route builder and dead endpoint labels
`withPublicApiRouteHandler` and 27 `ApiEndpoint` union members landed together
in #5273, but the v2 surface shipped on `defineV2JsonRoute` + `v2RateLimits`
instead. The builder had no production caller — only its own test — and the v2
rate limiter never reads an `ApiEndpoint` label, so those members were never
emitted to telemetry by symbol or by string literal.
Remaining members are exactly the labels a v1 route passes to `checkRateLimit`
or `authenticateRequest`. Drops the now-unreachable `hasZodUsage` branch from
the API validation audit; no ratchet metric moves (route total stays 1093).
* fix(billing): deny the payer pool to actor-less workspace API keys
The first pass gated `credits` and `storage` on billing authority for
personal API keys but let a `workspace_api_key` principal through
unconditionally, which left the excluded role a way back in. Any workspace
`admin` may mint a workspace API key, and a workspace `admin` is
deliberately not a billing manager, so an admin who reads `null` as
themselves could mint a key and read the full pool with it. On an
organization-hosted workspace that pool is the organization's, spanning
workspaces the admin has no standing in.
Billing authority is payer identity or an organization admin role — a
property of a person. A workspace API key is deliberately actor-less, so it
can never satisfy it and now reads both fields as `null`. Attributing the
key to its creator was rejected: it would launder the same workspace-admin
role, it breaks when the creator's authority is revoked while the key lives
on, and substituting a key's owner for the acting principal is what the
application operation boundary forbids. The reasoning sits in TSDoc at the
decision point.
The key keeps the plan, period, and standing it needs to monitor a
workspace, including `limit_exceeded` and `billing_blocked`. No in-repo
caller reads `credits` or `storage` from this endpoint. The payer storage
pool is now read only once disclosure is authorized, so a caller who may
not see it no longer triggers the query at all.
* fix(folders): bound the workflow folderId-branch path index reads
`createWorkflow` and `updateWorkflow` each resolve a folder two ways inside one
function. The folderPath branch goes through `resolveWorkflowFolderPath`, which
loads the path index with `maxRows: MAX_FOLDERS_PER_WORKSPACE`; the folderId
branch loaded it with no bound at all, issuing a `SELECT` over every active
folder row in the workspace. In `updateWorkflow` the unbounded read and the
bounded fallback sit thirty lines apart in the same function.
Passes the cap at both sites, matching the read sites that already opt in.
Exceeding it throws `FolderCollectionLimitExceededError` rather than truncating,
because a partial path index resolves real folder paths to `undefined` and
re-roots resources at the workspace root.
`maxRows` deliberately stays opt-in rather than becoming the default. Folder
creation does not refuse at the same ceiling on every path — `POST /api/folders`
goes through the `createFolder` name/parentId variant, which passes no
`maxFolderRows`, so the count guard in `executeCreateFolderAtPath` never runs
and a workspace can already hold more than `MAX_FOLDERS_PER_WORKSPACE` folders.
Defaulting the bound would make every path-index consumer throw for a state the
product allows to exist. Reconciling reader and writer is a separate change with
a user-facing limit, not a chore.
* chore(billing): tidy payer-pool concealment cleanup
* fix(api): reject an undecodable offset cursor on v2 table rows
GET /api/v2/tables/{tableId}/rows coerced an undecodable pagination cursor to
offset 0 and re-served page one. A client paging forward reads that as a fresh
first page and can loop over it forever. Every sibling v2 cursor list — logs,
files, workflows, workflow runs, workflow versions, workspace members, tables,
knowledge documents — already rejects with a validation error instead.
Extracts the offset-cursor decode both offset-paginated v2 routes had inlined
into `decodeOffsetCursor`, next to the existing `decodeSortedCursor`, so the
reject-don't-restart rule has one home.
* fix(api): restore v1 table error-response parity and stop internal message leak
The v1 table routes were rewritten to consume `lib/table/orchestration`
results, and two response behaviors drifted from what the live API returned.
Information disclosure: an unclassified failure's `outcome.error` carries
whatever text the fault happened to have. Drizzle wraps a throw raised inside
a transaction in an error whose own message is the failed statement and its
bound parameters, so `DELETE /api/v1/tables/{tableId}` and
`DELETE /api/v1/tables/{tableId}/rows/{rowId}` returned that verbatim in the
500 body to any API-key holder. Previously these returned a fixed generic
string.
Lost `lock` field: the 423 body used to be `{ error, lock }`. The delete,
row-delete, and column-update routes (v1 and internal) dropped the lock kind
the orchestration result already computes, leaving clients unable to tell
which lock to clear.
Both are fixed at one altitude: `orchestrationOutcomeErrorResponse` in
`app/api/table/utils.ts` is now the only way a table route projects an
orchestration failure onto the wire. It renders the route's fallback for an
unclassified failure and the real message for a classified one (validation,
not-found, conflict, locked keep their specific text), and carries `lock` on a
423. A future route cannot reintroduce either bug by hand-spelling the body.
Duplicate table names on `POST /api/v1/tables` keep answering 409 rather than
reverting to the previous 400. 409 is the correct semantic, and every other v1
duplicate-name surface (knowledge, files, workflow import) already answers 409;
the tables 400 was the outlier. v1 tables appears in no published OpenAPI
document and no in-repo client branches on the status, so the compatibility
cost is limited to a caller matching 400 specifically for a name collision.
* fix(skills): only reject a built-in name collision on an actual rename
The built-in-name guard ran on every update that carried a `name`, without
comparing it to the skill's current persisted name. Skills created before the
guard existed can legitimately carry a built-in's name (they simply shadowed
the built-in at read time), and the skill modal always submits the full object
including the unchanged name — so every save of such a skill returned 400 with
"The skill name ... is reserved by a built-in skill", with no way to fix it
short of renaming.
Move the guard in `updateSkill` to after the canonical row is loaded and run it
only when the submitted name differs from the current one. Creating a skill
with a built-in name, and renaming an existing skill into one, are still
rejected. The check stays in the shared orchestration primitive because that is
the only layer both the internal `/api/skills` adapter (via `performUpdateSkill`)
and `updateSkillUseCase` (v2 + Copilot) pass through, and it is where the
current name is in hand.
* chore(tables): tidy v1 error projection cleanup
* chore(skills): tidy collision guard cleanup
|
||
|
Waleed
|
be5db68644 |
fix(agiloft): repoint the block at the alrest surface and fix EWLogin (#6562)
* fix(agiloft): make the block work, and align it with the REST documentation
The native Agiloft block could not authenticate against any instance. A
customer reported it; production traces for their workspace confirm every
failure mode verbatim. Fixing that exposed a second, larger problem, and a
per-endpoint audit against the full published documentation found the rest.
Authentication
- EWLogin sent only $KB/$login/$password as query parameters. A live instance
answers `400 EWWrongDataException ... One has to specify $table, $KB, $lang
parameters`. $table is required even though only $KB/$login/$password/$lang
are documented. Parameters now travel in a form-encoded body, which the docs
permit and which keeps the password out of URLs and access logs.
- The authentication scheme is read from the login response and trimmed;
Agiloft returns it as "Bearer " with a trailing space.
- EWLogout was missing $lang.
Surfaces
- Record create, read, update, search and saved-search now use the endpoints
that accept the token EWLogin issues; the legacy operations authenticate from
inline credentials, which is what that surface expects. Nothing sends both
forms at once — the documented 400 for doing so is what the original report
had run into.
- EWSelect passes credentials in a POST body, one of the five operations
documented to support it.
- Attachment retrieval uses the documented EWRetrieve endpoint, with
filePosition rather than position, and no longer needs a login/logout pair.
Defects found in the audit
- remove_attachment reported zero on every call: its body is the EWREST
assignment form but the route ran JSON.parse then Number(), yielding NaN.
- The EWREST parser could not read EWActionButton's documented response, which
puts both assignments on one line.
- EWLock treated any 200 as success, including the documented
{error, error_description} envelope, and invented an 'UNKNOWN' status.
- EWTable discarded the linked-field details, required flag and text field type
it had asked for, making includeLinkedInfo inert.
- select_records had no result ceiling at all; both it and search now cap and
report a truncated flag rather than reporting a capped length as a total.
- Optional string inputs rejected null, so a blank Page field failed validation
before any request was made.
- Upsert treated the documented 202 async acknowledgement as a missing-ID
failure, and returned no callback ID for the caller to poll.
- Every response contract required an output that the 401 and 500 paths never
return.
Coverage added
- Table and field discovery (EWTable), upsert (EWUpsert), async status
(EWAsyncStatus), natural language search (EWNLPSearch), action buttons
(EWActionButton), the REPLACE_WITH_ANOTHER delete rule with its substitute
records, $async on upsert, and <fieldName>$overwrite on attach.
- Reads with a named field list go through the search projection; an unfiltered
contract record runs to roughly 184KB and swamps downstream agent context.
- Errors are readable: Agiloft wraps failures in HTML around a typed exception
and an internal task id, and the JSON endpoints now request real status codes
rather than a 200 the caller has to interpret.
Not implemented: $searchSQL and $operationHints=NOLOCK are EWRead/EWUpdate
parameters and those operations do not run on that surface here; EWQuestion,
EWHotlinks, EWOData, EWBroadcast and webhook registration have no documentation
beyond their names.
Verified against the published documentation, not against a live instance.
* fix(agiloft): give natural language search a sentence that paints
check:canvas-sentences failed: the nlp_search card resolved to nothing on an
untouched canvas, so it painted empty. Its only basic-mode field was the
long-input query, and the field list is advanced, so every segment dropped.
The sentence now leads with the knowledge base, matching the shape List Tables
already uses — both operations are knowledge-base scoped rather than
table-scoped, so it also reads more accurately.
* fix(agiloft): stop retrying refusals, and expose the outputs the new operations return
Five findings from review that had gone unanswered.
An Agiloft refusal was surfacing as HTTP 500. readAlrestJson throws when the
envelope reports success:false, the route catch mapped that to 500, and the
tool runner retries 500s — so a create the server had already rejected could be
retried and duplicate the record. Refusals now return a settled failure with the
message intact; genuine faults still 500.
list_tables could not run in its primary mode. EWTable is knowledge-base scoped,
but some instances reject EWLogin without a $table, so whole-knowledge-base
discovery failed at login with nothing to fall back to. It now says what the
caller can do about it rather than surfacing the raw login error.
Upsert corrupted structured values. Every field went through String(), so a
multi-value field collapsed into one joined string instead of the documented
repeated key/value pairs, and an object silently wrote "[object Object]" into
the record. Arrays now encode as repeated pairs and objects are refused, since
Agiloft documents no encoding for them.
Two outputs were invisible in the editor. `records` was conditioned on
search_records alone, so natural language search results could not be chained,
and `callbackId` on run_action_button alone, so a queued upsert's callback could
not be wired into Async Status even though both values exist at runtime.
|
||
|
Waleed
|
440a68bff9 |
improvement(audits): skip the sql-date-binding parse for files without drizzle-orm (#6554)
check:sql-date-binding Babel-parsed all 13,941 source files in apps, packages, and scripts. A violation can only come from an `sql` tag resolved through a `drizzle-orm` import, and both the static and dynamic resolvers match the specifier as a string literal, so a file that never names the module cannot produce one. Only ~590 files do. Skipping the parse for the other 92% of bytes takes the audit from ~4.5s to ~0.8s and drops it out of the four slowest audits, taking check:audits from 6.0s to 5.3s wall and 38.0s to 32.9s serial. Output is unchanged. |
||
|
Waleed
|
6fdb1459c4 |
fix(v2-api): standardization (#6542)
* fix(v2-api): stop leaking resolved secrets in logs and serving doc source Two regressions shipped with the v2 API (#5273) where v2 diverged from the v1 path it replaced, plus the hardening that fell out of auditing them. **v2 logs bypassed secret redaction.** `getPublicLog` and `listPublicLogs` called raw `materializeExecutionData`, while every other reader — v1 list and detail, CSV export, `fetch-log-detail`, both data-drain sources — calls `materializeExecutionDataForDisplay`, which applies the resolved-secret provenance projection. Both v2 routes then serialize `traceSpans` and `finalOutput` straight onto the wire, so unredacted secrets could reach the public API. Swapped to the display projection and threaded the principal's subject user into the read context. **v2 file download served generation source.** `GET /api/v2/files/{fileId}` streamed `file.key` raw. AI-generated docs store their generation source as the primary file, so a raw download yields source text under a `.pdf` name — a file the recipient cannot open. Generated docs now resolve to their compiled artifact; ordinary uploads still stream and are never materialized, gated on the recorded generation-source type rather than the extension. The resolve is capped at MAX_RENDERED_DOCUMENT_BYTES, and a still-compiling artifact returns a retryable 409 rather than a 500. Also in this change: - Reconcile the two v2 verbs that used PUT for PATCH semantics: `PUT /v2/knowledge/{id}` and `PUT /v2/tables/{tableId}/rows` are both all-optional partial updates. Breaking for API-key clients, but the surface is dark-launched behind the `v2-api` gate and no in-repo caller issues PUT. - Close the OpenAPI coverage blind spot that hid two routes: contract discovery was a non-recursive read of the flat `contracts/v2/` directory, so a contract in a subdirectory — or beside its non-v2 siblings, which is where the uploads contracts live — escaped the gate. The sweep is now recursive over the whole contracts tree, and the two upload data-plane routes are named in an explicit allowlist with reasons and staleness guards. - Extract `needsRenderedArtifact` so the "recorded type is authoritative, extension is fallback" rule has one home instead of being duplicated. - Extract `DocCompileUserError` into a leaf module so recognizing it no longer drags `app/api/**` and `next/server` into application modules. - Correct the stale pagination docstring in `contracts/v2/shared.ts` and pin the paged/full-set split in a test so it cannot drift again. * fix(v2-api): absolute imports for the extracted doc-compile error Review follow-up. - Use the `@/lib/...` alias for `doc-compile-error` in the three modules that imported it relatively. The repo requires absolute imports, and having all four consumers share one specifier also removes any chance of two module instances resolving apart and breaking `instanceof`. - Memoize the v2 list-pagination sweep. It re-imported the whole contracts tree once per test and timed out against the default 10s limit under load; it now sweeps once and declares an explicit timeout. Its failure message also still pointed at an enumeration in `v2/shared.ts` that this branch replaced with a pointer to the test itself. * fix(v2-api): correct three inaccurate claims found in verification None of these change behavior; each is a comment or test-config assertion that was not true as written. - The artifact resolver's TSDoc implied the byte cap prevents an oversized artifact being materialized. It does not: the artifact-store fetch is not streaming-bounded, so the bytes are resident before the ceiling rejects them. Say what it actually guarantees. - `v2/shared.ts` pointed at per-contract documentation for the two lists that still filter in memory. Neither contract documents it, so name the two lists and what they do inline instead of pointing at a page that does not exist. - The knowledge update contract said "every field of the body is optional"; `workspaceId` is required. Narrow the claim to mutable fields. - Scope the pagination sweep's extended timeout to the one test that pays for it, so a genuine hang in the other two surfaces in 10s rather than 60s. |
||
|
|
263e3ca67e |
improvement(external-endpoints): v2 versions with clean signatures + updated docs based on openapi spec (#5273)
* v0.6.29: login improvements, posthog telemetry (#4026) * feat(posthog): Add tracking on mothership abort (#4023) Co-authored-by: Theodore Li <theo@sim.ai> * fix(login): fix captcha headers for manual login (#4025) * fix(signup): fix turnstile key loading * fix(login): fix captcha header passing * Catch user already exists, remove login form captcha * improvement(external-endpoints): v2 versions with clean signatures + updated docs * feat(usage): accept X-API-Key on usage-logs list + export /api/users/me/usage-logs and /export now use checkHybridAuth — the same auth /api/users/me/usage-limits already accepts — so external monitors can read summary.bySourceCredits (the source breakdown of usage-limits' aggregate currentPeriodCost) instead of estimating Copilot spend by subtraction. Workspace-scoped keys are pinned to their own workspace's slice of the ledger: the filter defaults to the key's workspace and an explicit mismatch 403s. Both endpoints documented in openapi-core.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(billing): dedicated v2 usage endpoints; keep internal usage routes session-only Replaces the earlier X-API-Key enablement on /api/users/me/usage-logs with a dedicated public surface, so the internal Billing-settings endpoints can evolve with the UI while external monitors get a stable versioned contract: - GET /api/v2/billing/usage — current-billing-period summary with bySourceCredits (the source breakdown external monitors need to watch e.g. Copilot consumption without estimating by subtraction), plus limitCredits and plan - GET /api/v2/billing/usage/logs — cursor-paged credit ledger in the v2 envelope - workspace-scoped keys are pinned to their own workspace's slice; personal keys read the account ledger The public wire is credits-only: usage-logs rows now carry a hasCost boolean instead of dollarCost (the Billing UI only needed the >0 signal), and the rateLimit block is removed from the usage-limits response and docs (deploy-modal tab relabeled accordingly). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(docs): validate OpenAPI specs against the Zod contracts in CI The specs in apps/docs are hand-authored because they carry what Zod never defines — error envelopes, status codes, prose, examples — so they can't be generated; check:openapi validates them instead: - spec integrity: $refs resolve, operationIds unique, 2xx documented, no orphaned component schemas - v2 conventions: every /api/v2 operation documents 401 + 429 and every 4xx/5xx resolves to the canonical { error: { code, message } } envelope - contract cross-check: contracts are auto-discovered from lib/api/contracts/v2 (each carries its method + path); doc<->contract coverage both ways, query/body/response field diffs via z.toJSONSchema - examples: documented request/response examples must parse with the matching contract's actual Zod schemas First run caught real drift, fixed here: 16 stale orphaned schemas in the core spec, the v2 billing ops referencing v1-shaped error components, deploy/rollback examples missing the required nullable lifecycle keys, CreateTableBody missing folderId, a legacy-grammar delete-rows example, and four knowledge document ops missing their required workspaceId query param. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * fix(docs): recursive field diff in check:openapi + the deep drift it found A mutation test showed the doc<->contract field diff only compared top-level properties, so a typo inside the { data } envelope passed. The diff now descends through matching object properties and array items (both sides must expose a property set — passthrough contracts and prose-only docs end the descent instead of false-positive), with the Zod JSON-schema root doubling as the $defs context. Deep drift it immediately caught, fixed here: select-column config (options/multiple) missing from every tables column schema, AddColumnBody hand-rolling a third column shape (now composed from ColumnInput, with position/workflowGroupId as the per-op extensions the contracts actually admit), chunking strategyOptions undocumented, and the deployment lifecycle fields (activeDeployment/latestDeploymentAttempt) missing from DeploymentState. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * fix(security): close the triggerType rate-limit bypass on workflow execute Caller-supplied triggerType flowed unchecked into preprocessExecution, whose checkRateLimit default turns OFF for 'manual'/'chat' — so any API-key caller, and any anonymous public-API caller billed to the workspace owner, could execute unthrottled by sending {"triggerType":"manual"} (async runs also skipped the worker-side check via admissionCompleted). External callers may now only send the redundant 'api' value; internal JWT callers ('workflow'/'mcp') are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * refactor(execution): extract enqueue/status/cancel into shared libs Prepares the v2 execution surface: handleAsyncExecution's queue logic moves to lib/workflows/executor/enqueue-execution.ts (slot/claim semantics encoded in a discriminated outcome, not HTTP statuses), the execution-status read to execution-status.ts, and the order-sensitive cancel machinery to lib/execution/cancel-workflow-execution.ts. The v1 routes re-render identically — their suites pass unmodified. Also: preprocessExecution gains rateLimitCounter ('sync'|'async') and its 429 now carries code RATE_LIMIT_EXCEEDED + retryAfterMs (previously indistinguishable from the concurrency 429 and Retry-After was discarded); and the duplicate cancel contract in contracts/logs.ts is unified on the full 5-value reason enum — its narrower copy made requestJson throw a client ZodError when cancelling a paused HITL run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(execution): callable execution service + structured error classifier executeWorkflowService composes the same libs the v1 route holds inline (call-chain guard, execution-id claim, LoggingSession, preprocessing, deployed-state load + file-field processing, timeout-bound executeWorkflowCore, output hydration/compaction) for the deployed-state caller class — the seam the v2 execute route and in-process internal callers share, making the HTTP endpoint syntactic sugar. classifyExecutionError stops discarding the block context that buildBlockExecutionError already attaches at throw sites: failed runs now yield {message, code, blockId, blockName, blockType} with a stable append-only code enum (TIMEOUT/CANCELLED/USAGE_LIMIT_EXCEEDED/ INVALID_INPUT/BLOCK_EXECUTION_FAILED/CHILD_WORKFLOW_FAILED/ OUTPUT_TOO_LARGE/EXECUTION_FAILED), so callers route on error class instead of substring-matching messages — the single place raw errors are interpreted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(api): POST /api/v2/workflows/[id]/execute Thin route over executeWorkflowService: X-API-Key or anonymous public-API auth (sync/stream only for anonymous), strict body with body-flag async (no mode headers on v2), SSE passthrough for stream, and the execution resource response — executionId always present, in-band run failures are status:'failed' with the structured {message, code, blockId, blockName, blockType} error, sync timeout is status:'failed' + TIMEOUT instead of v1's 408, and a Response block's payload stays inside output (authors never control response status/headers on this origin). Async debits the async bucket and the 202 statusUrl points at the v2 executions resource. Adds CLIENT_CLOSED_REQUEST/SERVICE_UNAVAILABLE to the v2 error codes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(api): v2 executions status + cancel with queued backfill GET /api/v2/workflows/[id]/executions/[executionId] is the single status URL for sync and async runs: before the async worker writes the durable log row, status is backfilled from the job queue (deterministic job id) as 'queued'/'running' — closing v1's 202-to-pickup 404 window — and failed runs carry the structured error object. POST .../cancel renders the shared cancellation lib in the v2 envelope with the tightened 5-value reason enum. Both authenticate via the shared resolveV2WorkflowAccess (X-API-Key, authz masked as 404, allowPersonalApiKeys honored). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(execution): workflow tool + MCP bridge run in-process workflow_executor (workflow-as-agent-tool) short-circuits in executeTool through WorkflowBlockHandler — the same invocation boundary canvas child workflows use — mirroring the deployed_block_executor precedent. The MCP serve bridge calls executeWorkflowService directly instead of fetching its own execute endpoint; deployment-version pinning, MCP response-size rejection, and the actor override become typed options instead of header sniffing. Both callers drop the double admission slot and duplicate top-level log row the HTTP hop cost, and failed child runs now surface the structured error + child executionId so parents and MCP clients can route on error class and hand providers a reproducible handle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(infra): CORS + CSP coverage for the v2 execute path /api/v2/workflows/:id/execute gets the same wildcard-origin, credential-free CORS policy as v1 (the default credentialed policy would block browser API-key calls and open a cookie CSRF surface) with X-Sim-Stream-Protocol allowed and no X-Execution-Mode (async is body-selected on v2), plus the COEP/COOP/CSP header block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(ui): deploy modal + copilot advertise the v2 execute surface All 20 API-tab snippets move to POST /api/v2/workflows/{id}/execute with the nested {"input": ...} body, async as the "async": true body flag (X-Execution-Mode gone), status polling against the v2 executions resource, the third tab renamed Usage and pointed at /api/v2/billing/usage, and {data} envelope unwraps in the printed responses. Fixes the latent baseUrl derivation (endpoint.split('/api/workflows/')) that would have silently built garbage URLs under a v2 endpoint, and deletes dead code (exampleCommand across 3 sites, getAsyncExampleTitle). Copilot deploy/manage/serializer endpoint builders and the api_trigger bestPractices example follow (the latter also drops its hardcoded staging host). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * docs(api): document the v2 execution surface Adds execute, execution status, and cancel to openapi-v2-workflows.json with the structured ExecutionError schema (append-only code enum + block attribution) and the ExecutionResource contract, documenting the rules that differ from v1: modes are body-selected, a failed run is HTTP 200 with status 'failed', an executionId always means data (never the error envelope), queued status is visible immediately, and Response-block payloads stay inside output. Registers the three pages in the generated workflows meta.json and bumps the route-count baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * feat(api): gate the whole /api/v2 surface behind one flag; UI stays on v1 Every v2 route now runs exactly one check immediately after auth — v2ApiGateError — and answers 404 when the `v2-api` flag is off, so the surface is invisible until it is deliberately rolled out. The gate is keyed on userId only: a workspace/org-keyed check would have to read membership for a caller-supplied id before authorization runs, and its 404-vs-403 split would leak cohort membership (the trap the per-domain table gate worked around by running late). The two executions routes inherit it from the shared access resolver; the tables-specific gate is removed so no route checks twice. `tables-v2-api` stays, now gating only the internal predicate-grammar route /api/table/[tableId]/query — note v2 tables routes move to the unified flag, so enabling them is a `v2-api` decision now. Reverts the deploy modal, copilot handlers, and api_trigger example to the v1 execute endpoint: v1 works unchanged, and the UI must not advertise a surface most users would get a 404 from. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CiHhAk2R1NryaS3R8n2yFz * fix(executor): restore child-cost aggregation dropped by the staging merge Staging's custom-block rewrite deleted `aggregateChildCost` from workflow-handler.ts, and git merged that file cleanly — but this branch's workflow-tool-runner.ts, added for the v2 execute migration, still imports it. A silent semantic conflict: no marker, broken build. Taking staging's rewrite is correct, so the helper is defined locally in its one remaining consumer rather than resurrected in the file staging just rewrote. Same four lines over the still-exported `calculateCostSummary`, so a failed child workflow keeps billing the hosted-key spend it consumed instead of reporting $0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(tables): make lib/table/orchestration the single implementation (#6134) * refactor(orchestration): move the shared error contract out of lib/workflows OrchestrationErrorCode and statusForOrchestrationError are the contract every lib/[resource]/orchestration module returns against, but they lived inside the workflows module, so resource-neutral code (lib/folders) already had to import from a workflow path. Moved to lib/core/orchestration/types. Adds a 'locked' class mapping to 423. Both tables and workflows have a lock that forbids a mutation, and each caller was translating that to a status itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(tables): make lib/table/orchestration the single implementation Column update was implemented four times — the UI route, v1, v2, and the copilot table tool — each calling the same column services but owning its own guards, error mapping, and audit. The copies had drifted, and the drift was the bug: v2 was missing both guards, only the copilot copy minted stable option ids, and only v1/v2 audited. performUpdateTableColumn, performDeleteTable, and performDeleteTableRow now own that logic; all ten call sites reduce to auth, parse, call, render. The guards are asserted once in lib/table/orchestration rather than four times against four routes. Behavior this consolidates, previously true on only some paths: - The typeChanging guard. updateColumnType early-returns on an unchanged type and drops any options sent with it, so restating the current type alongside new options silently discarded them. v2 had no guard at all and, since its contract shares v1's body schema, accepted options and ignored them. - The select-unique guard. Each write is its own locked transaction, so a rename or type change paired with a constraint write that is going to fail commits first and then throws, half-applying the schema change. - Stable select-option ids. Cells reference the option id, so an edit that re-sends an option by name has to reuse it or every cell holding it is orphaned. Only the copilot path did this; normalizeSelectOptionsInput moves to lib/table/select-options and now covers every caller. It preserves a supplied id, so it is a no-op for the fully-formed options the HTTP contracts accept. - required forwarded into the type and options writes, so a conversion validates against the constraint the same request is setting. - An audit on every successful update. The UI route and the copilot tool emitted none. - Single-row delete through the row service. v2 did a raw db.delete, skipping assertRowDelete and deleteOrderedRow, so a delete-locked table returned 200 and the row-count bookkeeping never ran. - The delete actor handed to deleteTable, which audits only when a row was actually archived. v1 and v2 omitted it and audited themselves outside that check, emitting TABLE_DELETED for a no-op delete of an archived table. Failure classes come back as OrchestrationErrorCode; v2 renders them through a new v2ErrorForOrchestration, mirroring statusForOrchestrationError on the v1 and UI surfaces, so a given failure maps to the same status everywhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(tables): bind the column-update tests to the orchestration function The base's route tests assert which column service each payload reaches — the behavior that now lives in performUpdateTableColumn. They mocked the `@/lib/table` barrel; the orchestration module imports the service directly, so they mock that too and keep asserting the same thing through the extracted implementation. The orchestration tests move onto the base's semantics: writes address the stable column id, a rename rides inside the write it accompanies rather than running first, and the currency guards replace the non-select options guard the service now owns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(copilot): drop the column-type import the delegation made dead Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(tables): move the audit log out of the table service `lib/table/service.ts` wrote its own audit rows, so whether an operation was audited depended on which function a caller reached for rather than on a user having performed it. That is what let v1 and v2 audit a no-op delete, and what made `deleteTable`'s optional `actingUserId` double as an audit opt-out flag. Worse, most sites fell back to `actingUserId ?? createdBy`, so an unattributed call was logged against the table's *creator*. The copilot `mv` path passed no actor at all: renaming someone else's table recorded them as the renamer. Audit now lives in the orchestration functions — performDeleteTable, performRenameTable, performMoveTableToFolder, performUpdateTableLocks — and the services just write. Internal callers (folder cascade, import rollback) keep calling the service and are silent by construction rather than by remembering to omit an argument. Two services now return what the audit needs: `deleteTable` reports whether it actually archived a row, so a repeat delete logs nothing; `updateTableLocks` returns the before/after locks, since only the locked write can observe the transition its description names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tables): restore audit provenance and conflict status in orchestration Moving the audits into the orchestration functions dropped three things the routes had been carrying, and added one the orchestration now owns twice. - The v1 and v2 column-update routes passed `request` to `recordAudit`, so their audit rows recorded the caller's IP and user-agent. The orchestration function had no way to receive it. Every table orchestration function now takes an optional `OrchestrationRequestContext` and every HTTP route forwards it; the copilot and VFS callers, which have no request, omit it. - `classifyTableMutation` matched `TableConflictError` on "already exists" appearing in the message and reported it as `validation`, turning the UI route's 409 on a duplicate table rename into a 400. It now matches the type, the way `performRestoreTable` already did. - `captureServerEvent` ran on every delete while the audit was gated on a row actually being archived, so a repeat delete of an archived table still reported `table_deleted`. Both now hang off the same evidence. - The copilot delete path kept its own `captureServerEvent` from when the service did not emit one, double-counting every copilot table delete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a * fix(tables): say which type a no-op column update restated A copilot `update_column` payload whose only content was the column's current type used to return success with the live schema, while the v1, v2, and UI routes rejected the same payload with "No updates specified". Delegating to `performUpdateTableColumn` unified them onto the routes' rejection — correct, but the message tells the caller its request was empty when it named a type. The orchestration function now reports the same thing `updateColumnType` reports when it loses this race concurrently: the column is already that type, re-issue without the type change. An empty payload still reads "No updates specified". Drops the copilot's `outcome.table ?? tableForUpdate` fallback with it — the comment described the no-op that can no longer reach that line, and a success always carries a table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a * refactor(tables): classify failures by type instead of by message text The table module decided HTTP statuses by searching error messages for phrases. `VALIDATION_MESSAGE_FRAGMENTS` and `ROW_WRITE_ERROR_PATTERNS` held 32 substrings between them, and fifteen more lists were inlined in routes — 83 matchers over 17 files, each its own copy of the guesswork and already drifted apart. It made message wording load-bearing: `TableRowLimitError`'s own doc comment noted that its text had to contain "row limit" for a route to answer 400, and adding "already exists" to a rename message silently demoted a 409 to a 400 (the bug fixed one commit ago, by adding another special case). Services now throw `OrchestrationError`, which carries the transport-neutral `OrchestrationErrorCode` the layers above already speak. Classification is one `instanceof` in `orchestrationErrorResponse` (UI + v1) and `v2CaughtOrchestrationError` (v2). Every pattern list is gone. Wording is free to change; an unclassified error still becomes a generic 500, which is what an unexpected fault should be. `asOrchestrationError` walks the `cause` chain rather than testing the caught value directly: drizzle wraps a throw raised inside a transaction callback in a `DrizzleQueryError` whose own message is the failed SQL, so a bare `instanceof` would drop every failure raised inside `withLockedTable`. That is the same reason `rootErrorMessage` had to dig for a root cause before. Three throws stay bare `Error` deliberately — `Table ID mismatch`, `Workspace ID mismatch`, and `Failed to build upsert conflict predicate` are internal invariants no consumer classified, and they keep falling through to a 500. `Insufficient capacity` was in the pattern list with no producer anywhere in the codebase. Status changes, all deliberate: - `'forbidden'` joins the code union so the table-row-limit ceiling keeps its 403; without it this refactor would have flattened it to 400. - import-async's table-limit rejection: 400 -> 403, matching the two other create routes it had drifted from. - Renaming a table to an invalid name: 500 -> 400. `validateTableName` messages don't contain "Invalid", so no matcher ever caught them. - Restoring a table that isn't archived, or into an archived workspace: 500 -> 400. - A duplicate *column* name stays `validation`/400 rather than becoming a 409 like a duplicate table name. Both v1 and the orchestration have always answered 400 for it; changing a published status is not this refactor's job. The twelve tests that changed were asserting the substring mechanism itself, constructing plain `Error`s with magic strings. They now assert the real contract, plus new cases pinning that identical wording carrying no classification stays internal and keeps its message off the wire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGzbVDZpe2dEALbu2BUU8a --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * feat(api): add v2 endpoints for MCP servers, skills, custom tools, folders, and credentials (#6150) * feat(api): add v2 endpoints for MCP servers, skills, custom tools, folders, and credentials * fix(api): correct credential role, skill permission bar, MCP url identity, and custom-tool conflict mapping * fix(api): align credential mutation gating, provider-outage status, and unique-violation conflicts * fix(api): close unique-violation, revival, orphan-write, and env-rename gaps * fix(api): treat every provider-outage code as unavailable on create and update * fix(credentials): use the shared outage predicate on the session update path * fix(contracts): anchor the predicate double-cast annotation to the cast `check:api-validation:strict` counted 9 unannotated double-casts against a baseline of 8, failing CI. The predicate leaf schema was annotated, but the annotation sat above the declaration while the checker anchors on the line carrying the cast — five lines below, at the close of the object literal. The scanner walks back at most three lines and stops at the first non-comment one, so it hit `value: z.unknown().optional(),` and never saw the reason. Splitting the object schema from the cast puts them adjacent, so the existing reason binds. No behavior change — the cast, the schema, and the reasoning are unchanged. Also lowers the rawJsonReads ratchet 6 -> 5 to match the current count, which had drifted down; leaving it high lets a removed raw read silently come back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): point the orchestration error contract at its moved module #6150 branched before #6134, so skill-lifecycle.ts imports @/lib/workflows/orchestration/types — the module #6134 moved to @/lib/core/orchestration/types. Git merged a file deletion on one side with a new file referencing it on the other: no textual conflict, broken build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(knowledge): make lib/knowledge/orchestration the single implementation (#6154) * refactor(knowledge): make lib/knowledge/orchestration the single implementation Knowledge base create was implemented four times — the internal route, v1, v2, and the copilot tool — and the orchestration around the shared write had drifted. Extract it the same way lib/table/orchestration was: services write, orchestration decides which writes run, guards them, audits them, and returns a transport-neutral failure. Behavior converged, not preserved: - One chunking default (DEFAULT_CHUNKING_CONFIG). The agent defaulted minSize to 1 against the API's 100, so identical input produced differently-chunked knowledge bases depending on who created it. The agent path now chunks at 100. - Every successful mutation is audited inside the orchestration function. The copilot tool called recordAudit zero times, so agent-created knowledge bases, document uploads, updates and deletes left no audit trail at all. - Failures classify by class, not by message text. The knowledge service errors are OrchestrationError subclasses and storage-quota rejections throw a shared StorageLimitExceededError, replacing four separate message greps for "already exists" / "does not have permission" / "storage limit". delete_connector reported the opposite of what happened. It reached the route through an internal HTTP self-call that sent no query string, so the route's keep-documents default always applied while the agent told the user the documents had been removed. The self-call is gone — all four connector operations run in-process — and the orchestration returns the real counts. Also: - OrchestrationErrorCode gains 'payload_too_large' (413 / PAYLOAD_TOO_LARGE). Without it, dropping the storage-limit message match would have regressed the documented 413 on knowledge base create and document upload to a 500. - messageForOrchestrationError renders a route's own wording for an unclassified fault, so a driver's message no longer reaches the client on a 500. - v1 and v2 knowledge base update now forward actorUserId, which the service requires for a workspace move; both omitted it. - The connector DELETE route reads deleteDocuments through parseRequest. Its contract declared z.boolean(), which would have rejected the string a query param actually is. - Drop the 409 from POST /api/v2/knowledge/{id}/documents in the OpenAPI spec. Nothing on the upload path throws a conflict; it was only ever reachable by the message match this change removes. Behavior change worth noting: a v1/v2 PUT carrying only the workspaceId scope field and no actual updates now returns 400 rather than 200 with the unchanged knowledge base. Deliberately deferred: document update remains internal-only. Extracting performUpdateKnowledgeDocument makes exposing it on v1/v2 a contract and a route away, but that is a new public surface rather than part of this consolidation. * fix(knowledge): make connector create atomic and stop flattening failures Review round 1 on #6154. - Resolve the billing payer before the connector is committed, not after. A malformed attribution header rejected post-commit left a live connector behind a 500, and a retry created a duplicate plus duplicate sync work. Manual sync resolves before writing its audit for the same reason. - Let the source-config validator carry its own failure class. Collapsing every rejection to `validation` flattened the connector PATCH route's 401 (stale stored credential) and 409 (missing workspace context) into a 400. - Add `unauthorized` to OrchestrationErrorCode. It is the class that 401 was already expressing on this route, and the v2 vocabulary already had UNAUTHORIZED; only the shared union was missing it. - Report a knowledge base that exists but failed to archive as failed, with the reason, rather than as not found. The copilot delete loop folded every non-not-found failure into `notFound`, telling the user it was never there. - Route copilot failures through the same message helper the HTTP surfaces use, so an unclassified fault's raw text (a driver's failed SQL) no longer reaches the agent verbatim while the UI and public APIs get the generic wording. * feat(api): expand the public v2 files surface (#6160) * feat(api): expand the public v2 files surface Adds folder support, rename/restore, move, bulk archive, share, and content replace to /api/v2/files, so managing files by API no longer stops at upload + download + archive-one. Routes are thin: auth -> parse -> perform* -> serialize. Share and content replace get their orchestration extracted first so the session routes and the public ones cannot diverge on the effective-authType resolution, the EE public-sharing gate, or the storage-quota classification. Presigned upload stays session-only: presign does an advisory quota check and the real debit happens in the separate register step, so a caller that never registers leaves unaccounted bytes with no reaper. The buffered multipart path debits inside uploadWorkspaceFile's own transaction. * fix(files): classify folder and content failures instead of 500ing them Bugbot round 1. The v2 routes map errorCode straight to a status, so every manager failure that arrived unclassified became a 500 for what is really a caller-fixable 400 or 404. - Folder manager throws OrchestrationError: missing target/folder -> not_found, reparent cycle / self-parent / restore-into-archived-workspace -> validation. - File manager does the same for the in-transaction 'File not found' paths that the earlier pass missed. - updateWorkspaceFileContent's outer catch re-wrapped everything in a bare Error, which stripped the class off StorageLimitExceededError and the new not_found alike. It now rethrows a classified failure untouched and attaches cause to the generic wrap, so asOrchestrationError can still walk the chain. - Every remaining perform* gained the asOrchestrationError branch. - renameWorkspaceFile returned the pre-update read, so the v2 PATCH reported a stale updatedAt; it now returns the timestamp it actually wrote. Docs: upload auto-suffixes a duplicate name rather than rejecting it, matching the in-app uploader. The description claimed 409 and was simply wrong. * fix(files): surface a failed upload read-back as the real error getWorkspaceFile swallows a query failure and returns null unless throwOnError is set, so a transient blip on the post-upload read reported as 'file could not be read back'. Distinguish the two: a real null after a just-committed write is an invariant break, a query failure is itself. * revert(api): drop the dedicated v2 file-folder routes File folders already live in the shared folder table as resourceType 'file' (#6045 cut them over, #6051 dropped workspace_file_folders), and the remaining file-specific folder machinery is being folded into the generic folder engine. Publishing /api/v2/files/folders/** would pin that transitional split into a public contract we'd then have to keep or break. Files stay folder-aware — folderId/folderPath on the projection, folderId on upload, and the move route — because a folder id is a folder.id and survives the unification untouched. Folder management belongs on /api/v2/folders once that surface serves resourceType 'file'; until then there is no v2 way to enumerate file folders, which is the deliberate gap. The orchestration classification fixes stay: the internal routes and the copilot file-folder tools still call those perform* functions. * fix(files): classify upload failures instead of matching their wording Bugbot round 2. uploadWorkspaceFile had the same outer-catch rewrap that updateWorkspaceFileContent did, so a blown storage quota reached the route as a bare Error and the v2 handler recovered the status by substring-matching the message. Any rewording silently demoted a 413 to a 500. - uploadWorkspaceFile rethrows a classified failure untouched and attaches cause to the generic wrap. - FileConflictError is now an OrchestrationError('conflict'), so a duplicate name classifies like every other conflict. Its 'FILE_EXISTS' discriminator had no readers and is gone; the instanceof checks elsewhere still hold. - The v2 upload handler uses v2CaughtOrchestrationError, dropping all three string matches. Also documents that bulk-archive is best-effort: unknown or already-archived ids are skipped rather than failing the call, and deletedItems is what actually happened. That asymmetry with the single-id DELETE was undocumented. * feat(api): add search, filtering, and sorting to the v2 list endpoints (#6189) * feat(api): add search, filtering, and sorting to the v2 list endpoints One convention across every v2 list, documented on lib/api/contracts/v2/shared.ts: `search` (case-insensitive substring on the resource's natural name field), `sortBy` + `sortOrder` (per-resource enum, never a free string), and enumerated resource-specific filters. Reuses the sortBy/sortOrder pair v2 logs and v2 knowledge-documents already ship rather than inventing a third dialect alongside the Logs filters and the Tables predicate grammar. Every filter and sort is pushed into SQL. GET /api/v2/files previously read the whole scope and sorted/sliced it in JS; it now goes through a new queryWorkspaceFiles that filters, orders, and bounds the page in one query. Cursors are stamped with the sort they were minted under, so replaying one under a different sort is a 400 instead of silently duplicated or skipped rows. * fix(api): validate v2 cursor key values and compare timestamps at ms precision Two review findings, fixed at the root by making a keyset key own its cursor codec instead of hand-writing a decoder per sort. Cursor key values are caller-controlled, and matching the sort stamp and key count was not enough: an unparseable timestamp or a non-numeric size reached the query as an Invalid Date or NaN and surfaced as a 500. Each key now type- checks its own value and rejects a cursor it cannot hold, which both routes render as the documented 400. Timestamp keys now order and compare on date_trunc('milliseconds', col). Postgres keeps microseconds and defaultNow() populates them, but a cursor value round-trips through a millisecond-only JS Date — comparing the raw column against the truncated value re-admitted the page's own last row, duplicating it and stalling pagination outright at a page size of one. Reachable today via workspace_files.updated_at, which insertFileMetadata leaves to defaultNow(). * feat(api): complete the v2 workflows resource with versions and CRUD (#6184) * feat(api): complete the v2 workflows resource with versions and CRUD Adds version listing/detail plus create, update, and delete to the v2 workflows surface, which previously covered only execution and deployment. - GET /api/v2/workflows/[id]/versions — cursor-paginated, newest first - GET /api/v2/workflows/[id]/versions/[version] — version + pinned state - POST /api/v2/workflows, PATCH and DELETE /api/v2/workflows/[id] All six delegate to the existing orchestration and persistence helpers; no new domain logic. * fix(api): check folder containment before lock state; reject malformed version cursors assertFolderMutable walks a folder's ancestor chain without filtering on workspace, so inspecting it before containment let a caller tell a locked folder in someone else's workspace (423) from a nonexistent one (400). Create and update now assert containment first, matching the ordering import-workflow.ts already uses. A version cursor that decodes to JSON without a numeric version filtered every row out and returned an empty page with nextCursor null, which reads as a clean end-of-list. Malformed cursors are now a 400. * refactor(api): page workflow versions in the persistence helper listWorkflowVersions read every version row and the route filtered and sliced the result in memory, so the response was bounded but the query was not. It now takes optional limit/afterVersion, turning the cursor into a real keyset query; the route asks for limit + 1 and only trims the has-more probe. Both params are optional, so the internal, v1 admin, and copilot callers are unchanged. Also restores the untouched GET handler in [id]/route.ts to its original formatting — collapsing its signature had re-indented the whole body and buried the actual additions in whitespace churn. * feat(api): expand v2 tables with stateless multipart transfers (#6188) * feat(api): expand the public v2 tables surface Adds 16 operations so a v2 caller can do what the internal surface can: rename/move/lock a table, restore it, manage saved views, run enrichment columns, look up rows, and import/export with observable job control. Extracts lib/table/orchestration/import.ts (performTableCsvImport, performCreateTableFromCsv) and lib/table/export-stream.ts from the first-party routes, then repoints those routes at them, so v1 and v2 cannot drift on what an import or export actually does. events/stream, metadata and dispatches stay internal — they are editor state, not public API. * fix(api): make v2 table PATCH all-or-nothing and name the lock in every 423 Greptile P1: PATCH applied locks, rename and move as three sequential transactions, so a folder rejected mid-request left the earlier writes persisted while the response reported failure — and the schema-changed signal was skipped, leaving open clients on stale state. Every rejectable condition now runs before the first write, and the signal fires whenever anything did land. Cursor: v2TableLockError dropped the lock kind, so async import, column run, enrichment and table mutations returned a bare LOCKED. A table has four independent locks, so the caller could not tell which to clear. * fix(api): report the lock kind on classified 423s too, not just thrown ones The previous commit named the lock only where the rejection was thrown and caught at the route boundary. Where it instead arrives as a classified `errorCode: 'locked'` outcome — delete table, delete row, update column, and the table mutations — the kind was dropped, so those 423s stayed unactionable while their neighbours improved. The orchestration results now carry `lock`, and a shared `v2TableOrchestrationError` renders both arrival paths into the same `{ code, message, details: { lock } }` body. `details` is omitted rather than sent null when the kind is unknown, so a caller branching on it sees absence instead of a phantom value. * fix(api): make async table imports observable, not just startable `POST /import-async` pointed callers at `GET /api/v2/tables/jobs` to track progress, but that endpoint filters to `type = 'export'` — imports are derived onto the table itself, one write job at a time, and exports get a separate list precisely because they are excluded from that derivation. The public Table shape omitted those derived fields, so an async import could be started and cancelled but never observed to completion, failure, or progress. That is the gap the import/export/job-control set was meant to close. Table now carries `job` — id, type, status, rowsProcessed, error, or null when idle — and the import-async docs point at the table rather than the export list. * feat(api): make v2 table PATCH state which operations landed on failure Greptile held the PR at 4/5 on the residual non-atomicity and named two acceptable resolutions: make PATCH atomic, or have the contract adopt and expose partial-success explicitly. Atomicity would mean threading one transaction through renameTable, moveTableToFolder and updateTableLocks — three shared service functions with four non-test callers including the first-party route and two copilot tools — and deferring their per-operation audits to commit time. That is a refactor of shared write paths well outside this PR. So the contract states it instead. Every rejectable condition is already pre-validated, so a failure here is a genuine fault; when one follows a successful operation the error now carries `details.applied` listing what is live. Absent when nothing applied, so its presence always means "these changes took effect despite the error". Documented on the operation. `v2ErrorForOrchestration` gained the optional `details` this needs. * fix(api): make table lock flags read-only on the public v2 surface The new PATCH /api/v2/tables/[tableId] accepted a `locks` object, gated on workspace admin plus the table-locks feature. That still lets an API key clear the guard placed there to stop it: `write` is the floor for the endpoint, and admin keys are ordinary API keys, so a lock is no longer a boundary the key cannot cross. Locks stay readable on the table resource and enforcement is unchanged (a locked verb still returns 423). Changing one is now a first-party admin action only. The v2 body is declared here rather than reusing the first-party updateTableBodySchema, which keeps its `locks` field so the UI can still toggle them. It is .strict(), so a request carrying `locks` is rejected with a 400 naming the field instead of silently succeeding without applying it. * fix(api): keep reporting applied operations when the PATCH re-read fails The composite table PATCH promises that `error.details.applied` names the operations that are live despite an error, but `applied` was scoped inside the try. A rename or move that committed and was then followed by a throw in the final re-read — or a re-read finding the table archived — returned a bare 500/404 with no details, telling the caller nothing had landed. It would then retry into a duplicate-name conflict or repeat the move. `applied` is now function-scoped so every post-write exit carries it: the 404 on a missing re-read, a thrown lock error, a classified orchestration error, and the generic 500. `v2TableLockError` gains the same `extraDetails` parameter `v2TableOrchestrationError` already had. * feat(api): add workflow group writes to the v2 tables surface v2 exposed GET /groups but none of the writes, so the public API could run an enrichment or workflow column and read its binding, but never create one. A caller could add a plain data column and trigger the machine; wiring the two together still required the UI. Adds POST/PATCH/DELETE on /api/v2/tables/[tableId]/groups. The group is the unit that fills columns — one group feeds several — so creating one creates its output columns in the same call, matching the first-party shape rather than inverting it onto the column endpoint. Four departures from the first-party body, all public-surface concerns: - group.id is optional and server-generated. The UI mints an id to render optimistically; a public caller has no such need and a client-chosen id is a collision waiting to happen. - outputColumns[].workflowGroupId is dropped from the body and stamped from the resolved group, so it cannot disagree with it. - autoRun defaults to false. First-party defaults true so a UI add fills cells immediately; here it would make one POST fan out a metered run across every existing row. - A group naming neither a workflowId (type manual) nor an enrichmentId (type enrichment) is a 400 rather than a half-specified group the route has to guess about. Also rejects an outputColumns entry no group output feeds — the two arrays are joined by column name, and the first-party client builds both from one picker so it cannot desync, but a public caller can. Workspace containment on workflowId is asserted before it is persisted, on create and on any update that re-points the group; without it a table becomes a way to invoke workflows the key cannot otherwise reach. * improvement(api): make v2 table import and export async-only Drops the three synchronous entry points: POST /tables/[tableId]/import, POST /tables/import-csv, and GET /tables/[tableId]/export. Sync import tied a write to the lifetime of an HTTP request. The body *was* the data, so it carried a 10 MB cap that Next silently truncates past — a partial import reporting success. It also had no job, so a timeout mid-write left rows in place with nothing to poll and nothing to cancel. The async path reads the file from storage instead: upload via POST /api/v2/files for a key, start with POST /import-async, watch GET /tables/[tableId] -> job, stop with POST /job/cancel. Sync export carried no such hazard, but one shape per operation beats two: with both removed the surface has exactly one way to move a table in or out, and the CLI wraps the extra calls. This also removes the last multipart handling in v2 tables. Those were the only routes bypassing parseRequest — form fields were parsed by hand against separate form schemas, outside the contract system every other v2 write goes through. Create-a-table-from-CSV is now two calls: POST /tables, then /import-async with createColumns. csvImportModeSchema is append|replace, so there is no single-call create. Route baseline 1064 -> 1061. * docs(api): correct the import-async note about upload size limits The docstring claimed there is no synchronous upload endpoint and so no request-body size cliff. Both are wrong: POST /api/v2/files is a synchronous multipart upload with a 100 MB cap, and it is the only v2 upload path (presigned is deliberately absent). What async-only actually bought: the cap went 10 MB -> 100 MB, it fails on an explicit size check and a bounded body read rather than a proxy cap that silently truncates, authorization completes before any body is buffered, and the table write is a job that can be watched and cancelled. * feat(api): unify file and table transfers * improvement(api): make multipart transfers stateless * fix(api): make table import completion retries idempotent * feat(v2-tables): paginate the table list `GET /api/v2/tables` returned every table in the workspace in one response — it used the cursor envelope but hardcoded `nextCursor: null`, and had no `limit`. That was defensible when tables were only created through the UI; `POST /api/v2/tables` is public now, so a script can create them in bulk and the list has no way to ask for less. Adds `queryTables` alongside `listTables` rather than changing it, so the internal callers that genuinely want the whole scope are untouched — the same split `queryWorkspaceFiles` / `listWorkspaceFiles` already uses. Filter, order and slice all run in the query, so a `search` never costs a full-workspace read. A cursor whose values don't bind raises a validation error instead of being coerced to "no filter", which would have silently served page 1 under a resumed cursor. The keyset closes on `id` so a page boundary inside a run of equal names or timestamps stays stable. The shared `LimitQuery` doc component said "Maximum rows to return"; it now serves the table list too, so the wording is resource-neutral. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(api): add multipart knowledge document uploads * fix(api): keep usage admission at knowledge upload session creation * feat(knowledge): wire knowledge base uploads to multipart sessions * fix(knowledge): refuse to abort an upload once a document is bound * fix(uploads): prevent multipart cleanup races * Unify file creation and signed upload sessions (#6264) * feat(uploads): unify signed upload sessions * fix(uploads): preserve attachment storage semantics * feat(files): add authored file creation * fix(uploads): omit hoisted S3 metadata headers * feat(api): add file metadata endpoint * improvement(api): scope folders to resource paths (#6284) * improvement(api): scope folders to resource paths * fix(files): serialize folder resolution with uploads * fix(files): release folder lock before upload setup * fix(api): normalize folder paths and unblock resource mutations * fix(api): make resource cleanup and metadata consistent * improvement(uploads): persist multipart sessions in postgres * fix(db): store table row trigger timestamps in UTC * improvement(api): default folder deletion to non-recursive * fix(billing): unify chat usage source * improvement(logs): expose trace spans on log detail * fix(logs): parse list trace spans * improvement(api): replace workflow jobs with execution resources (#6294) * improvement(api): replace workflow jobs with execution resources * fix(api): preserve legacy jobs while preferring v2 executions * fix(api): make execution polling resume-aware * fix(ui): hide async examples for public workflows * fix(api): bridge resume queue visibility lag * feat(api): add v2 workflow resume endpoint * fix(api): project pending resume attempts * fix(api): prefer terminal logs over stale resumes * improvement(api): unify v2 resource query layers (#6319) * improvement(api): unify v2 resource query layers * fix(api): address v2 review findings * fix(api): preserve cancelled queue status * fix(api): guard cancelled job transitions * fix(api): close v2 resume and log gaps * feat(api): rename v2 executions to runs * feat(api): split credentials and secrets * feat(api): add workspace metadata and email attribution * improvement(api): consolidate public v2 route handling * improvement(files): centralize operations across APIs and Copilot (#6392) * improvement(files): unify rename authorization * chore(skills): add file operation migration guide * improvement(files): consolidate file operation authorization * improvement(files): extract shared operation foundation * improvement(api): simplify internal route declarations * improvement(files): centralize application authorization * refactor(api): share workspace file name validation * refactor(files): centralize copilot application calls * docs(skills): generalize application operation migration * improvement(api): centralize remaining v2 resource operations (#6412) * improvement(api): centralize v2 resource operations * fix(api): preserve custom tool conflict errors * improvement(api): migrate policy-sensitive v2 reads (#6410) * improvement(workflows): centralize v2 application operations (#6411) * refactor(api): migrate v2 knowledge operations (#6413) * refactor(api): migrate v2 knowledge operations * fix(knowledge): fail upload completion on dispatch errors * fix(knowledge): preserve upload retry and VFS errors * improvement(tables): centralize v2 application operations (#6414) * improvement(tables): centralize v2 application operations * fix(tables): preserve run validation and signals * feat(auth): add scoped internal executor delegation (#6459) * feat(auth): add scoped internal executor delegation * fix(auth): derive delegation lifetime from one timestamp * Include share status in file metadata * feat(auth): centralize delegated identity policy (#6462) * improvement(copilot): consolidate application adapters (#6450) * improvement(api): harden application route boundaries (#6451) * improvement(api): harden application route boundaries * fix(folders): reject creates at workspace cap * fix(knowledge): enforce trusted workspace scope (#6452) * fix(knowledge): enforce trusted workspace scope * refactor(knowledge): declare v2 body lifecycle * finish knowledge application migration * refactor(knowledge): compose copilot batch commands * fix(knowledge): parse connector query flags * fix(knowledge): finalize partial batch effects * fix(knowledge): align merged application boundaries * fix(knowledge): close application boundary review gaps * style(knowledge): satisfy branch biome checks * fix(knowledge): page connector documents in editor * refactor: enforce Copilot table application boundary (#6453) * refactor: enforce copilot table application boundary * fix(tables): finish application boundary migration * fix(tables): restore scoped copilot imports * fix(tables): compose copilot commands atomically * fix(tables): preserve workflow group scheduling * fix(tables): complete fixed copilot composition * fix(tables): reject enrichment output mutation * fix(tables): complete authorized application boundary * fix(workflows): migrate Copilot application boundary (#6455) * fix(workflows): migrate Copilot application boundary * fix(workflows): finish delegated application migration * fix(workflows): encode VFS folder aliases * fix(workflows): close application composition gaps * fix(workflows): preserve VFS validation errors * fix(workflows): complete application boundary migration * test(workflows): format canonical binding coverage * fix(workflows): scope executor metadata reads * fix(workflows): bind executor metadata targets * improvement(skills): align application operation guidance (#6532) * feat(api): expose v2 resource owners * fix(api): distinguish visible resource authorization failures (#6537) * feat(api): generate v2 OpenAPI from contracts (#6509) * feat(api): generate v2 OpenAPI from contracts * fix(api): preserve string boolean wire defaults * fix(api): document file download headers * fix(docs): use TypeScript CLI with Next.js * fix(docs): avoid client-rendered theme script * fix(api): document departed audit default * feat(api): replace legacy core docs with v2 * feat(api): generate v2 OpenAPI from contracts * feat(api): refine generated v2 OpenAPI docs * fix(docs): align localized v2 execution examples * fix(ci): restore Helm diff and sync audit mock * fix CI regressions after staging merge --------- Co-authored-by: Waleed <walif6@gmail.com> Co-authored-by: Theodore Li <theodoreqili@gmail.com> Co-authored-by: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com> Co-authored-by: Theodore Li <theo@sim.ai> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Theodore Li
|
5478a690cc | improvement(setup): complete knowledge and update flows (#6521) | ||
|
|
245ad1bd46 |
feat(workflows): new workflow block card, progress indicator, colors, dsl for natural language preview, retry configs (#6458)
* improvement(workflow): refine canvas interactions and rendering
* fix(workflow): keep outputs on the right, focus newly created blocks
Connection anchors: an output now always leaves a card from the right.
The cursor swell lets a drag start on any edge, but the left side is the
input, so anchoring an outgoing edge there drew a line out of the input
port and read as a second input. `normalizeCursorSourceHandleId` resolves
every drag to the right anchor, `normalizePositionedSourceHandleId`
collapses `source-left` alongside the legacy vertical anchors (so data
from the API, an older client, or a stale save self-heals on load), and
only the right-side source anchor is mounted.
Drops in `onConnectEnd` are always source -> target. The branch that
reversed the edge for a drag starting on an input could never run: the
`target` handle is `isConnectableStart={false}` and the positioned side
anchors are `isConnectable={false}`, so React Flow never reports an input
as a drag origin. Removed it and its now-unused imports.
A newly created block is centered once its node mounts and is measured,
so a card added from a drag-release, the block menu, or the toolbar is
never left off-screen or under the editor panel.
The editor panel's block icon uses the same type accent as the card's
badge instead of the block's legacy `bgColor`, which had left the panel
on the old per-integration brand colours.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(workflow): floor header-only card height, adopt brand tag palette
The Start card intermittently collapsed after load, squashing the
action-menu tab so its icon row sat over the card.
`.workflow-drag-handle` is the host the border renderer measures, and both
it and the header row took their height from `blockHeight && blockHeight >
0`. `blockHeight` comes from the deterministic-dimensions pass and is
already floored at MIN_PAINTED_HEIGHT (48), but it is absent on the first
frames — and with no floor the host collapsed to its natural content
height (25.5px for a header-only trigger, exactly the title's line box).
The border builds its perimeter from `host.offsetHeight`, so that window
painted a sub-floor card: too little straight edge remained on the
vertical runs for the action-menu tab, which collapsed into the corner
arcs. Whether you saw it depended purely on whether the dimension publish
had landed, which is why it reproduced on one workflow and not another.
Floor all three: the host, the header row (so `items-center` centres the
title and type tag rather than pinning them to the top), and the border's
own `offsetHeight` read.
Also raise ACTION_MENU_CONTENT_READY_THRESHOLD to 0.9. At 0.8 the 24px
icon row was revealed while the swell had only reached 22.4px of its 28px
— shorter than the row it contains. Secondary to the above, but a real
overflow window on its own. The test now pins the ratio rather than the
constant.
Tag palette moves to fixed brand values (hex, not derived oklch) with two
inks — #F8F8F8 on dark fills, #1A1A1A on light. Tones are renamed to match
what they render. `green` (2.55:1) and `orange` (3.15:1) sit under WCAG AA
against their paired ink; both are deliberate brand decisions and are
documented in the component.
Deploy and Run take two new Button variants rather than className
overrides, so `tertiary` stays green everywhere else.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* improvement(workflow): polish workflow canvas interactions
* fix(workflow): restyle loop drop target outline
* fix(workflow): shorten human block catalog label
* fix(workflow): canonicalize realtime edge handles
* code cleanup
* sizing fixes
* improvement(blocks): sentencify every block
* change tebse
* improvement(workflow): land notes UI and execution progress, consolidate duplicates
Ports the notes canvas editing and execution progress-indicator work, then
removes the parallel paths it arrived with so each concern has one owner.
Fixes found while consolidating:
- Note height was measured while the card was expanded to NOTE_EXPANDED_WIDTH,
where text re-wraps shorter, and published as the node's compact height. The
collapse then animated to a height never measured at the compact width.
- The edge pulse glow filter used the default objectBoundingBox units, so a
straight horizontal edge — what an auto-laid-out chain produces — resolved the
filter region to zero height and stopped the glow rendering entirely.
- A subflow's inner Start pill still read isNodeSelected while its border read
usesSelectedVisuals, so the two disagreed during execution.
- The Run/Stop button's disabled prop gated only Run while its handler cancelled
unconditionally, offering a Stop the cancel route answers with 403.
Consolidated:
- One note editor. The view's built-in textarea was unreachable in production
(the app always injects the markdown editor) and was kept alive only by tests
asserting against it; renderContentEditor is now required.
- onBlur/onCancel collapse to onEndEditing — content persists per keystroke, so
there was never a draft for a cancel path to discard.
- DEFAULT_NOTE_COLOR, the note height bounds, the note content reader and the
card width each had two or three definitions; each now has one.
- Removed with zero consumers: graphite/graphiteSubtle button variants,
data-subflow-selected, inputPlaceholderClassName, an effect that could never
fire, and getNoteColorOption's unreachable fallbacks.
Restores the role='status' announcement the progress rewrite dropped, and hardens
isNoteColor against inherited Object keys.
Co-Authored-By: Claude <noreply@anthropic.com>
* improvement(workflow): reuse the platform markdown editor in notes
Notes carried their own TipTap wiring — a second markdown editor that
reimplemented, more thinly, what `RichMarkdownField` already does for the skill
modal, skill fields and the deploy version description. It is now a ~20 line
skin: the Note supplies its type scale and per-colour selection tint, and the
field supplies the extension set, frontmatter held out-of-band, the round-trip
safety gate and its raw-source fallback, and markdown paste.
`RichMarkdownField` gains two additive props, both defaulting to today's
behaviour so the file editor is untouched: `surface` ('field' | 'bare') and
`proseClassName`. All three existing consumers pass an explicit `minHeight` and
no `surface`, so they take the original path unchanged.
Exiting the note editor moved to the card, because the editor's `/` and `@`
menus consume Escape to close themselves and ProseMirror checks `editorProps`
before plugin handlers — intercepting it inside the editor would have broken
both menus. The card now honours Escape only when nothing already consumed it,
which also let `onEndEditing` leave the injection contract.
The note editor is lazy now, matching every other consumer: it was pulling
TipTap and the full extension set into the canvas's initial chunk.
Also:
- One `areRunFromBlockDependenciesSatisfied`. The ActionBar, the canvas context
menu and the run-from-block handler each carried a byte-identical copy, and
the handler expressed the snapshot requirement differently, so the affordance
and the action could disagree. Each copy also re-scanned `edges` once per
incoming edge, on every ActionBar on the canvas.
- Reduced motion is one `usePrefersReducedMotion` in @sim/emcn rather than a
sixth ad-hoc `matchMedia`. The edge pulse now stops rendering instead of
hiding: `motion-reduce:hidden` is `display: none`, which left four SMIL
timelines running per edge.
- The pulse glow bleed covers the canvas minimum zoom. The strokes are
`non-scaling-stroke`, so the 6px tail spans 3/zoom user units — 30 at 0.1.
Co-Authored-By: Claude <noreply@anthropic.com>
* improvement(workflow): port canvas styling from workflow-updates
Ports the 14 styling commits your colleague added since the last sync, leaving
the ~68 staging PRs on that branch alone — those are platform/core work, not
this. Cherry-picked individually rather than merged so each conflict was small
enough to reason about.
What came in:
- Core block colors unify behind a two-level map: block type -> semantic role
-> accent, replacing the flat per-type table. Adds `purple` and `content`
tones to ChipTag, and a shared `WorkflowTypeIcon` that replaces the
hand-rolled ChipTag + accent lookup at each discovery surface.
- Native triggers take semantic colors; the deployments block moves to the
shared Rocket icon and drops its now-unused `iconColor`.
- Running-state polish: loader artwork and position, stop hover in dark mode,
the loader blended into the execution swell, and tooltips suppressed for
actions that are hidden mid-run.
- The toolbar drag preview clones the rendered icon container instead of
rebuilding a bgColor tile, so it matches what the canvas paints.
- The sidebar shows route-derived workspace identity instead of a skeleton
while the full record loads.
Conflict resolutions worth knowing:
- The running-loader artwork went through the shared `Loader` and back to the
custom SVG on their branch; the second commit is the intent, so that is what
landed — keeping our `role='status'` announcement layered on top.
- Two commits carried the lucide-react -> in-house icon migration along with
them. That migration is a staging change we have not taken, so our imports
stayed on lucide: adopting it in two files would leave the icon set split
across the app.
- `getMappedWorkflowTypeAccent` referenced a constant their refactor removed.
It had no consumers left once the search modal moved to `WorkflowTypeIcon`,
and their branch deletes it too, so it is gone here.
Co-Authored-By: Claude <noreply@anthropic.com>
* feat(workflow): make the subflow Start swell a real connection source
Dragging an edge into whitespace opens the add-block picker, but starting that
drag from a loop/parallel Start pill did nothing: the pill's border swell was
visual-only. Regular blocks and the container's own exit mint a draggable
cursor handle from their swell; the pill rendered only its invisible 14px
static strip, so grabbing the glowing affordance started no connection at all.
Everything downstream already worked and was nearly unreachable:
- the drop hit-test skips subflow containers, so a release inside the loop
opens the picker there (z 2000, above containers)
- handleToolbarDrop parents the new block into the container at the drop point
- it already carries the exact boundary rule for this source: a container
start handle only wires to a child of that container
The pill now runs the same cursor-handle machinery as the container view, with
one deliberate difference: its temporary handle carries the branch-cursor form
of the start id. The plain cursor id normalizes by block type — for a
container that is `loop-end-source`/`parallel-end-source`, the exit — so a
swell drag from Start would have persisted as an edge leaving the container.
The branch form passes `loop-start-source`/`parallel-start-source` through
normalization verbatim on both the picker and direct-connect paths; a test
pins that contract.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(notes): stop the field's prose classes recoloring bare-surface editing
Opening a note for editing shifted the text and turned it black: the
ProseMirror root unconditionally carried `rich-markdown-prose
rich-markdown-field-prose`, which pin the field's own ink and type ramp —
`--text-primary` at 15px/25px, then 14px/22px — overriding the card's
`text-current` at 14px/20px the moment the editor mounted.
`surface='bare'` means the host owns typography (the Note card mirrors its
rendered view via `proseClassName`), so on that surface the root now carries no
shared prose classes. The field surface is untouched. Edit mode inherits the
note colour's ink — including the caret — and sits on the same metrics as the
read view.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(workflow): give the in-flight connection line contrast inside containers
The drag line was drawn but camouflaged: its default stroke was the
resting-edge grey (#e0e0e0), which disappears against a loop body's opaque
`--surface-3` fill (~1.1:1) — so dragging an edge inside any container, nested
included, showed nothing. The z-order was never the problem; the connection
line layer already sits above every node.
The default token is now `--text-muted`, one value with contrast on every
canvas surface, still lighter than the `selected` variant so the variant
hierarchy holds. No per-surface special-casing.
Resting edges inside containers share the same camouflage (`--workflow-edge` on
`--surface-3`) — left alone deliberately: recoloring placed edges is a design
decision, not a bug fix.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(notes): match edit mode to the read view, and land the caret where clicked
Three defects, all from the read and edit views being built independently.
1. Blocks jumped up ~12px on entering edit mode. Streamdown wraps its output in
a container carrying `space-y-4` plus first/last margin resets, which outrank
the per-element margins in NOTE_COMPONENTS — so that wrapper, not those
margins, is what the read view actually paints. The editor had no equivalent.
The rhythm is now named (NOTE_MARKDOWN_FLOW), passed to Streamdown
explicitly so a dependency upgrade cannot move the read view out from under
the editor, and mirrored on the ProseMirror root. Tailwind's JIT only sees
literal strings so the mirror cannot be composed from the constant; a test
pins the two together instead, and fails if either side drifts.
2. The caret was barely visible: it inherited the note's 75%-opacity ink. The
palette owns per-colour chrome, so it now names the caret alongside the
selection tint.
3. The caret always landed at the document end. The read view sits under a
full-bleed overlay that must swallow the click to enter editing, so the
point never reached the editor and `autofocus: 'end'` was all that was left.
The view now forwards that point and the field resolves it through
`posAtCoords` on create — after the DOM is laid out, which `autofocus`
cannot wait for. Keyboard activation carries no point and still lands at the
end.
`autoFocusAt` is additive on the shared field and defaults to null, so the file
editor and the other three consumers are unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(workflow): stop edges rendering behind top-level subflows
Containers are z-indexed by nesting depth, so a top-level subflow is 0. Edges
derived their z from their parent container — `+1`, or 0 with no parent — so a
root-level edge landed on exactly the same z as a root-level subflow. Equal
z-index falls back to DOM order, and React Flow paints the nodes layer after
the edges layer, so the container's opaque body won: any edge crossing a
top-level loop or parallel was drawn behind it, in-flight or persisted.
Edges now sit in their own band above the whole container scale and below
cards, keeping both the deeper-container-wins ordering and the rule that a line
always passes behind card chrome. This is why the edge became visible only once
a block was dropped: the new block is selected, and an edge inside the
container was already `containerZ + 1`, clear of the tie.
The in-flight connection line is declared in the same scale rather than
inheriting React Flow's stylesheet default of 1001, which is both below a
selected container child and outside the scale this file owns. Its stroke moves
to `--text-secondary`, the token the canvas already uses for an active edge —
the previous `--workflow-edge` grey is ~1.1:1 against a subflow body.
Co-Authored-By: Claude <noreply@anthropic.com>
* feat(notes): paste and drop images through the workspace-file pipeline
Traced the file editor's image path end to end and reused it verbatim rather
than minting a note-specific source: insertImages -> useUploadWorkspaceFile ->
POST /api/workspaces/{id}/files/presigned -> direct-to-S3 PUT -> workspace_files
row -> the editor persists the workspace-scoped
/api/workspaces/{id}/files/inline URL, which the serve route authorizes by
workspace membership and the embedded-image-ref machinery already recognizes
for share rewriting and referenced-by-doc tracking.
The shared field gains an optional `uploadImage(file) -> {url, alt} | null`
capability. With it, image paste/drop uploads sequentially and inserts each at
the evolving position, mirroring the file editor's flow, with a bail if the
editor unmounts mid-upload; without it, the existing swallow-guard on file
drops is unchanged, so the skill modal, skill fields and version-description
consumers behave exactly as before. The upload mutation owns its own toasts.
The note host wires the capability with folderId null, so note images land in
the workspace Files root — visible, manageable and deletable there like any
other upload. The note read view renders images through its Streamdown
components map with the card's own sizing.
Co-Authored-By: Claude <noreply@anthropic.com>
* improvement for notes, subflows
* fix(uploads): surface the server's message when a multipart upload is refused
A file over the 50MB direct-PUT threshold goes through multipart initiate, which
is where the storage quota is enforced — but the client threw away the response
body and reported `Failed to initiate multipart upload: Payload Too Large`. That
is the string the upload mutation puts in its toast, and it names neither which
limit was hit nor by how much, so the one place that answer surfaces didn't have
it.
It now prefers `errorBody.error` exactly as `getPresignedUploadInfo` already does
on the single-PUT path, and passes the body through as the error's details.
Control flow is unchanged: still throws, still `MULTIPART_ERROR`, and the
cloud-storage-absent branch above still claims its 400 first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(notes): restore GFM in the note read view
Streamdown's `remarkPlugins` prop REPLACES its default plugin list rather than
extending it, and remark-gfm is one of those defaults. The note passed
`[remarkBreaks]` — so the read view silently lost every GFM construct: task
lists, tables, strikethrough and autolinks.
The editor writes all of them (it has TaskList, TableKit and Strike), so a note
round-tripped through editing came back as raw source the moment editing closed:
`- [x] HELLO` rendered as a disc bullet followed by the literal text `[x] HELLO`.
`NOTE_COMPONENTS` has carried table/thead/tbody/tr/th/td entries this whole time
that could never fire.
Restoring the plugin is only half of it: remark-gfm marks a checklist
`contains-task-list` and emits a native checkbox, which under the note's generic
`ul` styling renders a checkbox sitting behind a disc bullet — the same defect
the editor had before the chrome/typography split. The read view now drops the
marker and indent for a task list, lays the row out as a flex line, and styles
the checkbox to match `.rich-markdown-nodes input[type="checkbox"]` declaration
for declaration, tick clip-path included, so the two views agree either side of a
click.
`remark-gfm` is now a declared dependency of the renderer package rather than one
borrowed transitively from streamdown.
Five tests cover the GFM surface — checked/unchecked boxes, no literal `[x]`, the
marker only dropped for checklists, tables, strikethrough — and four go red with
the plugin removed.
Checked the other three Streamdown call sites (Chat, the chat interface renderer,
the changelog): none override `remarkPlugins`, so none were affected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(canvas): drop dead markers and share the tile-brightness maths
Review pass over the branch against staging.
Dead code removed:
- `tileIconColorClass` in the renderer package — never called; only its
`isLightTileColor` sibling is.
- `data-connection-selector-search-frost`, `data-workflow-cursor-edge` and
`data-workflow-cursor-source-side` — written on three elements, read by no
stylesheet, selector or test.
- `CHIP_TARGET_SELECTOR_TYPES`, `MAX_CHIPS` and `chipPriority` were exported from
`canvas-rows.ts` but only used inside it.
Consolidated the one real divergence: the renderer package carried a hand-copied
mirror of the app's perceived-brightness maths, because it may not import app
code. The copy had already drifted — it dropped the `white`/`black` keyword
handling, so a block shipping `bgColor: 'white'` would render a white
`currentColor` icon on a white tile on the canvas while every other surface drew
it black. No block ships one today, which is exactly why nothing caught it. The
function now lives in `@sim/utils/color` and both sides import it; only the
0.75 threshold stays local to each.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(canvas): share the z-scale, fix the preview's edge layering, and re-home strays
The preview canvas carried its own z numbers and had the collision the editor
canvas was fixed for: containers at nesting depth, top-level cards at an implicit
0, and edges at 0/5/10 by execution status — so a default edge tied with a
top-level subflow and painted behind it, while a success edge painted over
unselected cards.
The scale now lives once, in `@sim/workflow-renderer/canvas-layers`, and both
canvases read it. The preview keeps its status ordering, expressed inside the
shared edge band rather than as a second set of magic numbers.
Placement and duplication:
- `perceivedBrightness` moved to `@sim/utils/color`, with its unit test, and its
consumers import it directly. It had been re-exported through
`lib/colors/brightness.ts`, and the renderer package kept a hand-copy.
- `filterAcyclicEdges`/`wouldCreateCycle` were pass-through wrappers in the
workflow store's utils over the real implementations in `@sim/workflow-types`.
Deleted; the three consumers import the source.
- `lib/ui/glass-surface.ts` was a one-constant, one-consumer app-wide module, and
its consumer then aliased it a second time. Collapsed into the navbar shell.
- `nested-subflow-node` was set on nested container nodes in both canvases with no
stylesheet, selector or test behind it.
`packages/workflow-renderer` now has its own vitest config, so the four mount
tests for its components live with the components instead of in
`apps/sim/lib/workflows/**`. That immediately earned its keep: `apps/sim`
excludes test files from type-check, and once these were checked, tsc found three
`SubflowNodeView` renders being handed a `renderContentEditor` prop it does not
accept — a copy-paste from the note cases that had been silently ignored.
Verified: type-check 23/23, 21,143 app tests + 49 renderer + 147 utils, biome
clean, all 23 audits pass (`check:bare-icons` imported the moved helper and was
repointed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix field noun bug + notes
* fix(notes): let a note service the canvas actions the panel editor cannot
The panel editor clears any note put in front of it and renders nothing, but the
block menu still routed Rename and Open Editor through it.
Rename latched the editor's rename state onto the note — `handleStartRename`
reads the store directly, so it saw the id the menu had just set — and nothing
reset it when the clear ran. `handleSaveRename` writes to `renamingBlockIdRef`,
so the header went on showing a rename field over whatever was selected next and
saved that name to the note. Open Editor was a plain no-op that opened an empty
pane.
Rename now goes to the card, which expands and opens its own title — the same
menu-to-card routing Add Image already used, so both events now live in one
`lib/workflows/notes/canvas-requests.ts` and `add-image.ts` keeps only its
markdown concern. Open Editor is hidden for notes.
The panel editor also drops any rename whose block stops being the selected one.
That is belt-and-braces for notes now, but it closes the same hole for ordinary
blocks, where only the input's blur ended a rename and blur only fires if it held
focus. A rename interrupted that way is now discarded rather than left pending.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(canvas): author sentences for Snowflake and Mintlify, repoint Instagram's
Staging's two new integrations shipped without a `canvasPresentation`, so their
39 operations painted the field rows the rest of the canvas has stopped using.
The Instagram break is the more interesting one: staging renamed the insight
metrics subblock `metrics` -> `insightMetrics` while this branch was adding
sentences that named `metrics`. Both hunks merged cleanly — the union check
reports the file as an exact union — and the result was two clauses pointing at
a field that no longer exists, which resolves to nothing with no error and no
log. Only `check:canvas-sentences` sees it.
Two Snowflake sentences say something the block does not do, so they anchor
elsewhere: `taskName` filters `list_task_runs`/`get_task_run` rather than keying
them, and `table` filters `introspect_schema` — blank means "every one", not "not
filled in yet", and a core chip would have claimed otherwise.
Coverage is back to 4727/4727 operations across 321/321 blocks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(canvas): keep the block-type tag naming its type after a rename
The header tag dropped its label whenever the block's title already said the
same word, so the same block read two ways depending on nothing the user did
deliberately: a freshly dropped Wait showed a bare icon, and its second copy —
auto-named "Wait 2" — showed "Wait". The tag looked like a badge that appeared
on rename rather than a fixed part of the header.
It now always names the type, which is what loop and parallel containers already
do with their own tag, so every card on the canvas reads the same way.
`blockName` was only ever read for that comparison, so the prop is gone rather
than left behind for a future reader to wonder about.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(deploy): compare edge handles by port, not by spelling
Two places answer "does this need redeploying?" and they load their sides
differently. The client diffs the live store against `/api/workflows/[id]/deployed`;
the server diffs the normalized tables against the version's raw jsonb. Only
some of those paths run handles through `loadWorkflowFromNormalizedTables`, so a
snapshot holding a side-anchored id (`source-right`) met a canonical one
(`source`) on the other side and the set comparison read it as every edge being
removed and re-added.
Each answer therefore differed, and they arrive on separate query timelines: the
button reads the client's, the modal badge reads the server's, so the state
flipped between Live and "Update deployment" with whichever query landed last
until both settled.
`normalizeEdge` now canonicalizes both handles, so the comparison cannot tell
two spellings of one port apart no matter how its inputs were loaded. The
existing normalization in `materializeDeploymentState` stays — that path also
feeds React Flow, which needs the handle it mounts to match.
The preview's error port had the mirror problem: it rendered for every
non-trigger block regardless of `errorEnabled`, so a card with no error row grew
a red knob anyway. It now gates the way the editor canvas does, keeping the port
mounted when an error edge already leaves it so React Flow cannot drop that edge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(deploy): stop counting the error flag twice in change detection
`errorEnabled` has two homes. It persists inside the block's `data` jsonb — the
realtime server `jsonb_set`s it there, and load mirrors it back onto the block as
a field — so it reached the diff twice, and only some paths populate the copy.
`setBlockErrorEnabled` writes the mirror alone, so right after toggling the port
the live block said `errorEnabled: true` with `data.errorEnabled: false`, while
the snapshot the deploy had just taken from the tables said true in both. The
diff read the stale `data` and reported the workflow as changed the instant it
finished deploying — then a state refetch rehydrated the block and it agreed
again. That is the flip between Live and "Update deployment": the button and the
modal read two different queries, so each landing swapped the answer. A block
created in-session had the same shape from the other side, its `data` carrying no
key at all against a persisted `false`.
Excluded from `normalizeBlockData` alongside the other fields that are duplicated
out of the block's own state. The block field is still compared on its own, with
`!!`, so absent and `false` agree and turning the flag on is still a change.
Fixing the store to write both homes was the other option and is not taken:
nothing reads the in-memory `data.errorEnabled` (save and load both let the block
field win), so it would add a second copy that only the diff could see — which is
the shape of this bug, not its fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(blocks): give the error-output flag a column instead of two homes
`errorEnabled` had no column, so it persisted inside the block's `data` jsonb
and was mirrored onto the block as a field on load. Every writer had to route
its `data` through `withPersistedErrorEnabled` or silently drop the toggle, the
realtime op `jsonb_set`, and change detection saw the same value twice — which
is what made the deploy badge flip between Live and "Update deployment" after
toggling the port.
Its siblings — `enabled`, `horizontal_handles`, `advanced_mode`, `trigger_mode`,
`locked` — are all boolean columns; `data` is for React Flow and subflow state.
The flag belongs with them, so it now has `error_enabled` and one home. The
shuttle helper, its `BlockData` mirror, the store's fallback read, and the
comparison exclusion the duplication forced are all gone.
Backwards compatibility, since released versions draw the error port with no
toggle in front of it: a block already wired to an error edge HAS the output on,
because there was no other way to draw that edge. That rule is now stated in
three places and none may be narrowed to read the flag alone —
- the migration backfills `error_enabled` from the edges, so live rows are true
before any new code reads them;
- `materializeDeploymentState` derives it for a version's frozen jsonb, which the
migration cannot reach — otherwise every workflow deployed before the toggle
would ask to be redeployed once;
- `workflow-block.tsx` keeps it at render time for states that reach the canvas
through neither (imports, copilot edits), where unmounting the port would make
React Flow drop the edge leaving it.
The migration also moves any `data.errorEnabled` a developer created on this
branch onto the column and strips the key; both statements match zero rows in
production, where it never shipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(canvas): realign the Snowflake and Dynatrace sentences with staging's blocks
Both breaks are the class the union check cannot see: separate hunks of the same
file merged cleanly, and the result names fields that no longer exist. A sentence
that does resolves to nothing, with no throw and no log.
Snowflake's rewrite (#6474) moved database, schema, table, warehouse and
procedure onto canonical selector pairs, so seven clauses anchored on ids that
are gone. Each now names both members of its pair, which is also what keeps the
card readable for someone working in advanced mode. Its nine new operations have
sentences.
Dynatrace (#6463) scoped the mute reason to the operations that mute, because
unmuting accepts exactly one — so the two unmute sentences were asking for a
field their card no longer shows. They drop the clause.
Coverage is 4736/4736 operations across 321/321 blocks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(api): drop the block-data error flag from the workflow contract
Left behind by the consolidation: the flag no longer lives in `data`, and a
schema that still declares it there invites the mirror back through the wire.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(deploy): compare edge handles by port, so a falsy one cannot read as changed
`loadWorkflowFromNormalizedTables` now runs handles through the canonicalizer,
which falsy-coalesces — so an edge persisted with `sourceHandle: ''` loads as no
handle at all. The server diffs that against the deployment version's raw jsonb,
which still has `''`, and the set comparison reads one edge as removed and
another added. Every workflow holding such an edge would ask to be redeployed
the moment this ships, for nothing. Two write paths use `?? null` rather than
`|| null`, so `''` is reachable.
Canonicalized inside `normalizeEdge` rather than at either call site: the two
sides are loaded by different paths and only some of them normalize, so the
comparison has to be unable to tell two spellings of one port apart however its
inputs arrived.
This is the change reverted in
|
||
|
Theodore Li
|
56910c002e |
feat(embeddings): add OpenRouter support (#6396)
* feat(knowledge): add OpenRouter embedding fallback * fix(knowledge): preserve successful embedding batches * feat(embeddings): add OpenRouter provider * fix(knowledge): bill only platform embedding tokens * test(embeddings): include OpenRouter provider * feat(embeddings): load OpenRouter model catalog * fix(embeddings): preserve legacy provider default * fix(embeddings): batch OpenRouter requests * fix(embeddings): reset stale OpenRouter model |