mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-22 05:19:54 +08:00
d152fad78817de265cfecf9835dea2ccc6f197a2
243
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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 |
||
|
Waleed
|
303986f45f |
feat(snowflake): credential-based auth, object pickers, and 9 new operations (#6474)
* feat(snowflake): credential-based auth, object pickers, and 9 new operations
Replace the per-block host + PAT fields with a Snowflake service-account
credential, move the credential picker to the top of the block, back the
object fields with metadata-only pickers, and add nine operations.
- credential: snowflake-service-account token service account (account host +
programmatic access token), verified against the SQL API with the same
headers the tools use
- selectors: database, schema, table, warehouse, execution role, file format
and procedure pickers behind one /api/tools/snowflake/objects route
- new operations: unload_data, list_databases, list_schemas, list_tables,
alter_warehouse, resume_task, suspend_task, list_query_history,
list_copy_history
* fix(snowflake): migrate renamed subblock IDs and authenticate before parsing
- add SUBBLOCK_ID_MIGRATIONS entries so the renamed object fields map onto
their pickers and the removed host/apiKey values are parked
- authenticate the caller before contract validation in the selector route,
per the API route convention
* fix(snowflake): close unload-query breakouts, drop parked secrets, correct docs
- assertBalancedQuery now skips // line comments, $$ dollar quoting and rejects
ambiguous nested block comments; each hid a paren that let an injected
OVERWRITE = TRUE escape the derived table
- always emit OVERWRITE so an injected duplicate is rejected by Snowflake
rather than silently replacing staged files
- _removed_ migration targets now drop the stored value instead of parking it
under a dead key, where export scrubbing (which walks the block config) would
never clear it
- 403 falls back to the shared invalid-credentials message, which names the
network policy and SQL API causes Snowflake does not distinguish in the body
- correct the network-policy-by-user-type claim: only SERVICE_AGENT is exempt
- correct MAX_FILE_SIZE and errorOnly tool descriptions to match the fixed code
* fix(snowflake): stop untouched switches emitting clauses; retarget migration
- an untouched switch serializes as null, and advanced mode emits every
advanced subblock, so alter_warehouse silently sent AUTO_RESUME = FALSE and
permanently disabled auto-resume on the warehouse; normalize optional
booleans to undefined in tools.config.params
- point the subblock migration at the advanced text members: a migrated block
has no credential, so a picker cannot hydrate a stored name, and legacy
fileFormat values were qualified while the picker lists bare names
- add the missing json-object wand type and scope the SQL wand prompt, which
promised bindings that unload_data does not accept
* fix(migrations): sweep already-parked subblock values; align picker 403
- an earlier version of this migration renamed retired fields into _removed_*
keys instead of deleting them, so deployed workflows still hold those values;
they match no oldId, so a dedicated sweep clears them for every block type
- the picker now treats a Snowflake 403 like a 401: it means a network policy
or a disabled SQL API, which the credential validator already reports as a
credential problem rather than a bad request
* fix(wand): add json-array generation type for array-contract fields
The json-object reinforcement tells the model the response must start with {
and end with }, which fights any field whose contract is an array. Snowflake's
rows, matchColumns and procedureArguments all ask for arrays, so they were
being steered toward an object that the JSON parse would then reject.
Adds a sibling json-array type that strips fences the same way but reinforces
brackets, and points the three array fields at it. bindings and filters are
genuine objects and stay on json-object.
* fix(snowflake): unload a table, not an inline query
The COPY INTO grammar places the source immediately before its copy options, so
an inlined query sits one parenthesis from being able to rewrite them. Guarding
that means matching Snowflake's tokenizer exactly, and three successive versions
of the guard were each defeated: // line comments, $$ dollar quoting, and a bare
carriage return, which the scanner did not treat as a line terminator but
Snowflake does. Each fix was a guess at a lexer the public docs do not specify.
Removes the inline-query source instead of guessing a fourth time. A table name
goes through qualifiedIdentifier, which is provably safe. Exporting a query
result now means materializing it first — a view, or CREATE TABLE AS SELECT via
Execute SQL — which the tool description, the block skill and the docs all say.
Also from the final audit:
- optionalBoolean accepts the string forms a direct tool call delivers, matching
the other boolean readers on this block, and its TSDoc no longer states the
serializer rule backwards
- the five JSON editors declare language: 'json', so invalid JSON is caught
inline instead of at execution
- bound the RESULT_SCAN read in SQL, not only by rows_per_resultset
- pin every migration target to a live subblock id, for all blocks
|
||
|
Waleed
|
1c0e82a4e2 |
perf(ci): parallelize repo audits, guard env-dependent tests, and fix the docs generator (#6358)
* perf(ci): parallelize the repo audits and guard env-dependent tests
The 21 independent audits ran as 21 sequential CI steps, each a single-threaded
read-only walk of the tree. scripts/run-audits.ts runs them concurrently:
28s serial -> 5.0s wall locally at 13-way. It buffers each audit's output and
replays only failures, so a green run stays quiet and a red one still names the
audit and shows why. Audits needing a git base ref (block registry, migration
safety) or that write files (drizzle generate) stay as their own steps.
Also fixes 5 tests that fail for every macOS dev and are invisible in CI. They
shell out to python3 using `match` statements and 3.12 f-string nesting, which
need >= 3.10; stock macOS ships 3.9.6, so `bun run test` produced raw Python
SyntaxErrors with no guard and nothing tying them to a missing tool. One also
needs ripgrep, which CI installs and a Mac usually does not.
@sim/testing/environment detects both and the tests skip with a reason via
vitest's ctx.skip(). Under CI it throws instead: these suites deliberately run
the real helper rather than a mock -- the cloud-review path/read-size bounds and
the placeholder compiler's generated Python are only observable that way -- so a
missing tool in CI means a security boundary silently stopped being covered,
which is worse than a red build.
Drops the Codecov upload. The workflow already documented it as a dead path:
nothing generates apps/sim/coverage, vitest runs without --coverage, and
fail_ci_if_error hides it, so it reported green having uploaded nothing.
* fix(ci): raise the python floor to 3.12 and stop the bridge audit serializing the batch
Two review findings, both real.
MIN_PYTHON was 3.10, chosen for the `match` statements the compiler suite
generates. But two of the three guarded tests also use PEP 701 f-strings --
reusing the outer quote, and embedding `#` -- which are 3.12. Verified on a real
3.11 interpreter: the match-guard test passes, the other two fail with
`f-string: unmatched '('` and `f-string expression part cannot include '#'`,
which is exactly the raw SyntaxError the guard exists to prevent. A 3.10 floor
let them through and failed anyway.
The audit parallelization did not speed CI up -- it slowed it down. Serially the
21 audits took ~31s; concurrently the batch took 39.2s wall, because
check:desktop-bridge went from 1s to 39.2s and became the entire wall clock while
the other 20 finished in 9s. It is the only audit that shells out through `bunx`,
which re-resolves the package against the shared install cache -- a network-backed
sticky-disk mount on CI. Cheap when it runs alone, serialized behind the others
when they run together. Spawning the resolved compiler entry point directly
removes that layer.
Verified the audit still fails on a breaking bridge change rather than passing
faster by doing less.
* fix(docs): unbreak the MDX build and read trigger config from the registry
The docs build has been failing on staging since the Smartlead merge:
./apps/docs/content/docs/en/integrations/smartlead.mdx
Expected a closing tag for `<original>` before the end of `paragraph`
Tool descriptions are emitted as prose, and that path escaped only braces --
every table-cell path already escaped angle brackets. MDX reads `<` as the start
of a JSX tag, so a description like 'The copy is named "<original> - copy"' fails
the build outright. escapeMdxProse handles the MDX-hostile characters and leaves
pipes, parens and brackets alone, which are legal in prose and whose escaping
would mangle markdown links.
Trigger configuration now comes from the evaluated registry instead of regex over
source. Static parsing silently dropped every field whose builder assembled its
array imperatively or took a description as a parameter -- all ten Jira triggers
lost `webhookSecret` and `jqlFilter` that way, and Monday lost its config too, so
regenerating the docs was destructive. Reading real objects also deletes 232 lines
of parsing. Note `required` may be a condition object rather than `true`; only an
unconditional `true` renders as Required, matching the previous behavior.
Tool headings now show the tool's name ("A2A Send Message") rather than its id
(`a2a_send_message`), unformatted, across 241 generated pages. Names come from
tools/generated/tool-metadata.ts, which CI keeps in sync. These headings feed each
page's table of contents. a2a.mdx is hand-written, so its headings were updated
directly.
Also consolidates five hand-inlined copies of the escape chain into the
escapeMdxCell that already existed, and drops 44 comments that restated the line
below them. Generator: 4306 -> 4069 lines.
Every refactor step was verified against a golden manifest of all 289 generated
files -- proven deterministic across runs and proven to catch a one-character
change -- so the only output differences are the intended ones.
KNOWN GAP: extractTriggerOutputs still parses source and has the same blind spot;
it already drops one Jira output section on main. Regenerating is now safe for
trigger config but still lossy for trigger outputs.
* refactor(ci): derive the audit list and stop shelling out through bunx
Review pass over the audit runner and the tool guards.
The audit list was hand-maintained alongside package.json with nothing linking
them, and it had already drifted: check:cron-parity exists, passes, and ran in no
CI step at all. The list is now derived from the check:* scripts with an explicit
exclusion map, so a new audit is opted out deliberately rather than forgotten.
That picks up cron-parity — 22 audits now, not 21.
check-realtime-prune-graph.ts still shelled out through `bunx turbo`, the same
pattern that took the bridge audit from 1s to 39s once the audits ran
concurrently. Both now go through scripts/local-bin.ts, which resolves
node_modules/.bin — the same path check:native-typecheck asserts is the native
TypeScript 7 compiler, so the one guarded path is the one that runs.
Audits are spawned as their script rather than `bun run <name>`, which started a
bun process only to read package.json and start a second one.
Tool detection is memoized per process; it was re-spawning python3 on each of the
5 call sites, in every vitest worker. The CI throw is deliberately NOT memoized —
memoizing it would turn every call after the first into a silent skip, which is
the failure mode the guard exists to prevent. Verified it still throws for all
three guarded tests, not just the first.
Also: dropped the environment module from the @sim/testing barrel so
node:child_process stays out of unrelated consumers' module graphs, restored the
per-audit reporting the 21 separate steps used to give (collapsible groups, error
annotations, and a timing table they never had), and trimmed comments that
restated their code or duplicated the runner's own docs.
* fix(devin): give the 11 Devin tools real display names
Every Devin tool had its id as its `name` (`list_session_messages`), so the
generated docs rendered `### list_session_messages` where every other integration
renders a human name. It was the only integration doing this -- 11 of 4427 tools.
Names take the service prefix, matching the majority convention (3200 of 4416
names start with their service).
Also points the ship skill at check:audits instead of hand-listing the audits.
That copy had drifted five behind package.json: cron-parity, import-specifiers,
sql-date-binding, trigger-block-cycle and native-typecheck were all missing, so
shipping never ran them. It was the third copy of that list; there is now one.
* fix(docs): read trigger outputs from the registry too
Closes the gap left by the config fix: extractTriggerOutputs still parsed source,
so triggers whose outputs come from a builder call lost their tables. jira_webhook
had no output section at all.
The registry was not a drop-in, which is why the naive swap deleted 10,298 lines
earlier. The two sides encode nesting differently. A TriggerOutput marks a group
by OMITTING type and holding children as sibling keys:
issue: { id: { type: 'number' }, title: { type: 'string' } }
while the renderer walks the JSON-Schema-ish shape the parser used to synthesize:
issue: { type: 'object', properties: { id: …, title: … } }
formatOutputStructure only descends into .properties, so handing it the raw
registry value collapsed every nested group to one untyped row and dropped its
children. normalizeTriggerOutputs converts between the two, preserving leaves
that already declare properties/items and merging the 13 hybrid nodes that carry
both a type and inline children.
Measured across all 368 triggers before changing anything: 155 identical, 213
divergent, and the divergence was purely the nesting encoding — no node has a
non-string type, and a group never carries its own string description, so
leaf-vs-group classification is unambiguous. That is what makes a nested property
literally named 'description' (42 of them) survive.
Deletes the static path: extractTriggerOutputs, resolveTriggerBuilderFunction,
resolveTriggerOutputsConstant, readTriggerSiblingModules,
getWebhookProviderConstants, plus resolveConstStringValue and matchQuotedProperty
which the config fix had already stranded.
20 output sections recovered (linear 79->93, tiktok 6->11, jira 44->45) and 1698
rows. Verified independently: zero sections lost across all 289 generated files,
no file lost rows, output deterministic across regeneration.
The 96 deletions are all corrections, not losses. 70 are confluence fields the
parser flattened out of `comment: { ...buildContentEntityFields(), parent: {…} }`
and rendered as top-level trigger outputs; they reappear nested under their
parent in the same hunk. 8 are greenhouse key ordering, 6 are intercom
descriptions the parser had dropped, 1 is a vercel row moving position.
Generator: 4069 -> 3903 lines.
* chore(test): silence vite 8 deprecation warnings in the sim vitest config
@vitejs/plugin-react v4 targets pre-rolldown Vite: it sets `esbuild.jsx`
and `optimizeDeps.rollupOptions`, both deprecated under Vite 8's oxc
pipeline, and self-reports that plugin-react-oxc should be used instead.
v6 is that plugin merged back under the original name — it requires Vite
^8, drops Babel entirely, and emits none of those options.
Vite 8 also resolves tsconfig paths natively, so vite-tsconfig-paths is
replaced by `resolve.tsconfigPaths`.
Full apps/sim suite unchanged: 1483 passed / 2 skipped files,
20415 passed / 30 skipped tests.
* refactor(docs): drop 33 more comments that restated their code
Second pass over the generator, e.g. `// Copy icons from sim app to docs app`
above `copyIconsFile()`. Kept the multi-line runs (those carry reasoning), the
ones with concrete examples, and the one marking a deliberate empty catch.
Verified byte-identical output across all 289 generated files.
Generator: 3903 -> 3870 lines, 4306 at the start of this branch.
* refactor(ci): read package.json once in the audit runner
auditScripts() re-read the manifest the module body had already loaded.
* fix(pdl): name the tools directory after the tool ids
People Data Labs declared `pdl_*` tool ids under `tools/peopledatalabs/`. Every
other integration names the directory after its id prefix -- 259 of 260 before
this, and PDL was the only exception.
The docs generator locates a tool's definition by deriving the directory from the
id prefix, so it looked in `tools/pdl/`, found nothing, and returned null for all
11 tools. peopledatalabs.mdx rendered eleven bare `###` headings with no
description, no Input table and no Output table.
Renaming the directory rather than the ids: tool ids are persisted in saved
workflows, so renaming those would break existing users. The directory is
internal -- 15 files' imports.
Fixed at the source rather than teaching the generator a fallback. A special case
would have left the invariant broken and the next integration free to break it
again; now 260 of 260 hold, and the generator needs no exception.
peopledatalabs.mdx: 11 empty headings -> 456 lines. Repo-wide: zero pages with an
empty action body.
|
||
|
Waleed
|
a512a263c8 |
perf(typecheck): run the native TypeScript 7 compiler (#6356)
A bare `tsc` was silently resolving to the JavaScript TypeScript 6 compiler. `apps/sim` depends on `@typescript/typescript6` for its runtime TypeScript AST API, which pulls in `@typescript/old` (an alias of `typescript@6`) declaring its own `tsc` bin. Package managers pick bin winners by lexical sort rather than dependency depth, so `@typescript/old` beat `typescript` and won `node_modules/.bin/tsc`. Identical diagnostics, ~10x slower, and it fails silently: the check still passes, it just burns minutes. Both compilers check an identical 11,066-source- file program with byte-identical diagnostics; the only `--listFiles` delta is lib relocation plus TS7 deduping nested .d.ts copies. The `@typescript/native` alias sorts ahead of `@typescript/old` and reclaims the bin. This is the TypeScript team's own recommendation on typescript-go#4567 -- the original blog example was wrong. Every `type-check` script is unchanged; `bunx tsc` and ad-hoc invocations are fixed too. apps/sim cold 83s -> 8.5s; all 23 workspaces 96s -> 9.4s. The alias is invisible-load-bearing: nothing imports it, so removing it looks like dead-dependency cleanup and costs 10x with no visible failure. check:native-typecheck asserts a bare `tsc` reports 7.x and fails CI otherwise. Also drops NODE_OPTIONS=--max-old-space-size=8192 from apps/sim's type-check -- it only ever mattered for the JS compiler's V8 heap. |
||
|
Waleed
|
de02bc6ad5 |
fix(scripts): make the specifier audit path-separator agnostic (#6355)
Review round: isGeneratedPath split the repo-relative path on '/', but
path.relative returns backslashes on Windows, so '.source' and 'node_modules'
never matched a segment and generated output was treated as source. The repo
does support Windows dev — scripts/setup branches on win32.
The finding named one site; there were three. isCompiledSource compared against
'apps/sim/scripts/' with the same assumption, and workspaceFor matched
`${w.dir}/`, which on Windows never matches an absolute path and would have
dropped every file out of its own workspace — silently disabling tsconfig paths
resolution rather than erroring.
Normalized behind a repoPath() helper, with workspaceFor using path.sep against
absolute paths. Reported paths now go through it too, so output is identical on
either platform. spec.split('/') is left alone: import specifiers are always
'/'-separated regardless of host.
Verified by simulating win32 separators through the same predicates, and posix
behaviour is unchanged at 37,437 specifiers.
|
||
|
Waleed
|
10878fbde5 |
fix(utils): drop the .js specifiers Turbopack cannot resolve (#6351)
* fix(utils): drop the .js specifiers Turbopack cannot resolve
Every dev server on staging is currently returning 500 from any route whose module
graph reaches the `@sim/utils` barrel:
Module not found: Can't resolve './errors.js'
> 1 | export { getErrorMessage, getPostgresErrorCode, toError } from './errors.js'
Import trace:
./packages/utils/src/index.ts
./apps/sim/lib/embeddings/client.ts
./apps/sim/lib/knowledge/embeddings.ts
./apps/sim/app/api/knowledge/route.ts
`packages/utils/src/index.ts` addresses its siblings as `./errors.js` while the files
are `./errors.ts`. webpack rewrites that through `resolve.extensionAlias`; Turbopack has
no equivalent (vercel/next.js#82945). `next build` is webpack and `next dev` is
Turbopack, so this passes CI and breaks every local dev server — #6317 went green.
Nothing required the extensions: the repo is on `moduleResolution: "bundler"`, and no
other package barrel uses them.
Two changes, either of which fixes the symptom; both are here because they fail
differently:
- `packages/utils/src/index.ts` drops all 12 `.js` specifiers. Fixes the barrel for
every current and future consumer.
- `apps/sim/lib/embeddings/client.ts` imports `chunkArray` from `@sim/utils/helpers`
rather than the barrel. #6317 added the only bare-barrel `@sim/utils` import in the
monorepo; the subpath form is the documented convention (CLAUDE.md, "Common
Utilities") and resolves to one module instead of pulling twelve.
`scripts/check-import-specifiers.ts` fails the build on either shape and runs in CI.
Verified it goes red by restoring both halves of the bug. It scans only bundler-compiled
source — vitest and standalone `bun run` scripts resolve `.js` -> `.ts` themselves, so
flagging their specifiers would be noise.
Verified against a real dev server with production env: `/api/knowledge`,
`/api/tools/embeddings` and `/api/workflows/[id]/deploy` all go 500 -> 401, `/workspace`
renders, and the Turbopack log is free of resolution errors. `tsc --noEmit` clean,
`packages/utils` 147/147.
* refactor(scripts): resolve specifiers instead of pattern-matching one mistake
The first version banned `.js` specifiers by regex, which catches the bug that happened
and nothing adjacent to it. This runs the actual resolution algorithm with Turbopack's
rules — extensionAlias deliberately absent — and fails on anything that does not land on
a real file.
That covers the whole "Module not found" class rather than one shape of it: `.js`
specifiers, typo'd paths, files moved or deleted with a stale importer left behind, `@/`
aliases pointing nowhere, and `@sim/*` subpaths a package does not export. Verified
against three synthetic breakages the regex version passed clean:
'@/lib/webhooks/providerz' — '@/' alias matches a tsconfig path but nothing is there
'./does-not-exist' — no file at that path
'@sim/utils/chunking' — @sim/utils does not export './chunking'
Getting to zero false positives on 37,307 specifiers needed three things the naive
version got wrong:
- tsconfig `paths` are per-workspace. `@/*` is `apps/sim/*` inside apps/sim but
`apps/realtime/src/*` inside apps/realtime, and apps/sim maps `@sim/db/*` straight at
the package directory, legitimately bypassing that package's exports map. One
hardcoded alias produced ~30 false positives in apps/realtime alone.
- `exports` maps have wildcards. `@sim/emcn` publishes `"./*": "./src/*"`, so
`@sim/emcn/components/code/code.css` is valid despite no literal entry.
- TSDoc contains example imports. `packages/db/triggers.ts` documents
`import { ensureRowCountTriggers } from '@sim/db/triggers'` — a subpath the package
deliberately does not export. Comments are now blanked in place, preserving byte
offsets so reported line numbers stay exact.
* fix(scripts): close three coverage gaps in the specifier audit
Review round 1 on #6351. All three findings were real and all three let the exact
regression this guard exists for slip through.
- Reported line numbers were one early. `SPECIFIER_RE` opens with `(?:^|\n)`, so
`m.index` is the newline ENDING the previous line, not the start of the statement.
`./helpers.js` on line 13 was reported as line 12. Anchoring to the specifier's own
offset is exact, and for a multi-line import it points at the `from '...'` line —
where the reader needs to look anyway.
- `require()` was not scanned. This repo uses lazy requires deliberately to break import
cycles: `tools/params.ts` reaches `@/blocks` that way and `blocks/blocks/agent.ts`
reaches `@/blocks/registry`, 22 first-party call sites in total. Those edges resolve
exactly like static ones, so a bad specifier in one fails identically. Verified by
pointing `tools/params.ts` at a non-existent module and watching the audit catch it.
- `apps/docs` was not scanned, despite being a second Next.js app with its own
`next.config.ts` — so it carries identical Turbopack exposure. Now covered, and clean.
Side-effect imports and dynamic `import()` were called out in the same round but are
already covered: the optional `from` group in `SPECIFIER_RE` matches bare `import '...'`,
and `DYNAMIC_RE` handles `import('...')`. That review ran against
|
||
|
mzxchandraandWaleed Latif
|
dc5bab6e54 |
feat(embeddings): multi-provider Embeddings block on a shared core (#6317)
* feat(embeddings): multi-provider Embeddings block on a shared core
The Embeddings block was OpenAI-only with a bare fetch: no batching, no
retry, no metering, and no hosted-key support. Meanwhile the knowledge-base
indexing path already had a real multi-provider engine. Nothing bridged the
two, so the block could not reach Gemini and the KB engine could not be
reached from a workflow.
Extract the shared core into lib/embeddings/ first, then build breadth on
top of it, so both the KB path and the block resolve models and providers
from one catalog and one set of adapters instead of a third parallel
implementation.
- lib/embeddings/: catalog, client, key resolution, batching, L2
normalization, and adapters for OpenAI, Azure OpenAI, Gemini, Cohere,
and Mistral
- lib/knowledge/embeddings.ts becomes a thin KB wrapper with its exported
signatures unchanged; the 1536-dimension vector invariant does not move
- one tool per provider from a shared factory, behind a single
/api/tools/embeddings route and contract
- new `embeddings` block type; the `openai` block is left functionally
untouched and only leaves the discovery surfaces via hideFromToolbar
plus sunset.replacedBy, so placed instances keep working unmigrated
- openai_embeddings is now an alias of embeddings_openai, so legacy
instances pick up batching, retry, and metering with no visible change
* fix(embeddings): report an unsupported dimension as a client error
The route validated the model and the provider match up front but left
`dimensions` to be checked inside embed(), where resolveDimensions throws
and the generic catch maps it to 502. A typo in the block's dimension
field, or a reference expression resolving to an out-of-range value, was
reported as an upstream gateway failure rather than bad input.
Resolve dimensions in the route alongside the other boundary checks and
return 400. The throw stays the single source of the message, so the two
call sites cannot drift.
Adds route tests covering auth, the response shape, each boundary
rejection, input normalization, and the 502 path for genuine provider
failures.
* fix(embeddings): only send a dimension when the caller asked to reduce
resolveDimensions() returns the model's native size when no reduction is
requested, and that resolved value was handed straight to the adapter. The
adapters guard on `dimensions !== undefined`, so the field was always
populated and always sent.
Models that support Matryoshka reduction accept their own native size, so
this was invisible for text-embedding-3-*, gemini-embedding-001,
embed-v4.0, and codestral-embed. Models that do not support the parameter
at all reject it outright: every unreduced request to text-embedding-ada-002
and mistral-embed failed with a 400, which is both of the models whose
catalog entry has no supportedDimensions.
Track the caller's explicit reduction separately from the resolved
dimensionality. The resolved value still drives reporting and billing; only
the requested one reaches the wire.
Found by driving the live provider matrix against all four providers.
* test(knowledge): de-flake the sync-engine suite
Every test dynamically imported the module under test, so the first one to
run paid the whole cold-load cost inside its own 10s timeout and failed
intermittently under load.
The dynamic imports were working around a hoisting problem: mockMapTags is
a top-level const read by a vi.mock factory, and vi.mock is hoisted above
it, so a static import of the module under test crashes with a
use-before-initialization error. Declaring the mock through vi.hoisted()
removes that constraint, which is the pattern the testing guidelines
already call for.
One static import replaces 42 dynamic ones. The file drops from ~15s to
~2s and passed 5 consecutive runs.
* fix(embeddings): drop a capability the selected model no longer offers
The per-model Dimensions and Task Type dropdowns each share one subblock
id, and nothing clears a stored subblock value when its dependsOn fields
change — dependsOn only feeds rendering. A choice made for one model
therefore outlives a switch to another.
Picking 3072 on text-embedding-3-large and switching to -3-small left 3072
stored while the dropdown offered at most 1536, and the block forwarded it.
Same for a task type: 'similarity' chosen on Gemini survived a switch to
Cohere, which has no equivalent input type.
The guards only checked that the model declared the capability at all, not
that the value was one it lists. Check membership so a stale value falls
back to the model's native size, or is omitted, instead of being sent and
rejected. The user cannot have deliberately chosen an option the dropdown
stopped presenting.
* feat(embeddings): use the latent-constellation mark for the block icon
Replaces the scatter-plot-on-axes placeholder with a centre node, four
neighbours, and the rays between them — a point and its nearest neighbours
in embedding space, which is what the block actually produces. The axes
mark read as a generic chart and said nothing specific to embeddings.
Nodes are filled so they hold their shape at small sizes. The rays carry
less weight than the nodes to keep the hierarchy, but at 1.6/0.9 rather
than the 1.4/0.75 they were drawn at, so they do not thin out to loose
dots in the 14px block-search row.
Kept byte-identical between the app and docs icon sets.
* fix(embeddings): declare the outputs the legacy openai block returns
openai_embeddings became an alias of embeddings_openai, so the legacy
block's runtime payload gained `provider` and `dimensions`. Its declared
outputs still listed only embeddings/model/usage, so the tag picker never
offered two fields every run demonstrably returns, and downstream blocks
could not reference them.
Declaring them is additive and does not touch execution. Asserts the
legacy block's output keys match the replacement's, since both run the
same tool and neither should expose fields the other lacks.
* fix(copilot): resolve same-id subblock variants before validating
A block may declare one field id several times, each variant conditioned
on another field — the embeddings block declares model, dimensions, and
taskType once per provider, and the image and video generators do the
same. Validation keyed a map by id alone, so whichever variant was
declared last silently became the validator for every write to that
field.
Programmatic edits to an embeddings block were therefore checked against
Mistral's option lists whatever the saved provider: `text-embedding-3-small`
was rejected as not one of mistral-embed/codestral-embed, and dimensions
valid only elsewhere (3072, 768) could not be set at all. Values that
happened to overlap the last variant passed, so automation saw partial
success rather than a clean failure.
Keep every candidate per id and pick the one whose condition holds,
evaluating against the mutation's inputs merged over the block's saved
values so a partial write still resolves. When no condition matches, fall
back to the union of all variants' options rather than guessing.
Conditions still never gate whether a field may be written — that was a
deliberate choice and a hidden field stays writable. They only select
which definition describes the field, and an unresolved condition widens
the accepted set instead of narrowing it.
* fix(copilot): prefer a conditioned variant over an unconditioned catch-all
An unconditioned same-id variant matches every set of values, so it would
shadow a genuinely selected variant purely by being declared first. Prefer
a variant that actually asserted something about the current values.
No block in the registry currently declares a catch-all ahead of a
conditioned variant on a field where it would change validation, so this
is a guard against the pattern rather than a fix for a live case.
* chore(embeddings): scope this branch to the multi-provider block
Two changes made while building the Embeddings block are not part of it and
ship separately, so their files are restored to staging here:
- copilot edit-workflow validation resolving same-id conditional subblock
variants. The embeddings block surfaced it, but it is a platform fix
affecting ~20 blocks that declare a field id more than once, and it
narrows what programmatic edits accept — that deserves its own review.
- the sync-engine test de-flake, which is unrelated test hygiene.
Both are preserved in full on feat/embeddings-full-snapshot.
Note this restores the reported bug where a programmatic edit to an
embeddings block validates model/dimensions against the last-declared
provider variant. The block is unaffected in the editor and at runtime.
* fix(embeddings): honor per-model token limits and bound the JSON input path
Review round 1.
Batching used one 8,000-token constant for every model, inherited from the
knowledge-base engine this branch extracted. `batchByTokenLimit` truncates
any single text above the limit it is given, so that constant both sent
oversized input to models with a lower ceiling and silently dropped content
models with a higher one accept:
- Gemini declares 2,048, so a 3,000-token text passed through whole and the
provider rejected it, surfacing as a 502. This also affected knowledge-base
indexing on staging, which uses the same constant.
- Cohere declares 128,000, so anything past 8,000 was truncated for no reason.
Batch against the selected model's own `maxInputTokens` instead. Using the
per-input ceiling as the per-batch budget also keeps every individual text
within it.
The contract bounds the array arm of `input`, but a JSON-encoded array
arrives as a plain string and `normalizeInput` only expands it after
validation — so neither the 1,000-input cap nor the non-empty checks applied
to the reference-expression path the route was written to accept. `"[]"`
also reported success with no vectors. Re-check the normalized list so the
bounds hold for both shapes.
* chore(embeddings): regenerate tool metadata for the new embedding tools
CI's tool-metadata:check gate failed: registering embeddings_openai,
embeddings_gemini, embeddings_cohere, and embeddings_mistral left the
generated tool-ids/metadata/outputs artifacts stale.
* fix(embeddings): project before batching, and keep the sunset block's docs icon
Review round 2.
Projection ran inside callEmbeddingAPI, after batchByTokenLimit had already
measured and truncated the original text. The projector rewrites resolved
secrets to placeholders, which changes length, so batching sized against a
string that was never sent: a lengthening projection then pushed input past
the model's ceiling and the provider rejected it, and a shortening one
discarded document content that would have fit.
Project once up front, then batch the projected text, so truncation measures
what actually goes to the provider. This also keeps projection to exactly one
call per embed(), so no retry can re-project.
Separately, marking the legacy openai block hideFromToolbar dropped it from
the generated docs icon map, which only retains hidden blocks when they are
versioned. integrations/openai.mdx is deliberately kept — docsLink is baked
into every placed instance — so BlockInfoCard lost its icon and fell back to
a text tile. A sunset block keeps its docs page for the same reason a hidden
versioned block does, so the generator now treats it the same way.
The sim-side integrations map still omits it, which is intended: that feeds
the discovery page a sunset block should not appear on, and placed blocks
render from the registry's own icon reference.
* fix(embeddings): override stale block params instead of omitting them
Review round 3.
The generic handler merges the params() result over the original inputs
(`{ ...inputs, ...transformedParams }`), so omitting a key leaves the stale
value in place. The previous round dropped an unsupported taskType or
dimensions by omission, which was therefore a no-op through the executor
path: a reduction or task type chosen for one model still reached the tool
after a model switch.
Rewrite each stale field to an explicit `undefined`, which does override in a
spread.
Same class of bug for `model` itself, which was forwarded whenever present
without checking it belongs to the selected provider. Every provider's model
dropdown shares the `model` id, so switching provider kept the previous
provider's model and failed at the route as a mismatch. It now falls back to
the provider's default unless the saved model actually belongs to it.
Tests assert the merged result rather than the returned object, since the
return shape alone cannot distinguish an omitted key from an overridden one —
which is exactly why the previous fix looked correct and was not.
* fix(embeddings): discount the batch ceiling when the tokenizer is foreign
Review round 4.
Batching measures with tiktoken, which only has encodings for OpenAI models —
every other id falls back to cl100k_base. Gemini's 2048, Cohere's 128k, and
Mistral's 8192 were therefore enforced in OpenAI token units, so an input near
one of those ceilings could still be rejected upstream or trimmed more than
needed.
A true fix needs per-provider tokenizers, which the repo does not have:
estimateTokenCount is a chars-per-token heuristic, and truncation needs a real
encode/decode pair to slice on a token boundary. So the ceiling is discounted
for foreign tokenizers rather than trusted exactly.
The discount is one-sided on purpose. Overshooting means the provider rejects
the whole request; undershooting only trims a text that was already at the
limit, so the margin errs toward the second.
resolveBatchTokenCeiling is a pure function tested directly, rather than
inferred from truncation behavior, so the guarantee holds per model as the
catalog grows.
* fix(embeddings): keep the batch ceiling exact and warn before truncating
Review round 5. Reverts the safety margin from round 4.
The two review findings were in direct tension: round 4 flagged that a
foreign model's ceiling is measured in tiktoken units, and the margin added
to absorb that error reintroduced the round 3 harm — valid content truncated
below the provider's declared limit.
The margin was the wrong trade. It swapped a loud failure for a silent one:
an undercount surfaces as a provider rejection the caller can see and act on,
while shortening an embedding's input produces a degraded vector that is
indistinguishable from a good one at every layer above it. Silent quality
loss in a retrieval index is the worse outcome, and it is also the harder one
to ever notice.
So the declared ceiling is applied exactly, and truncation is no longer
silent: an input above the limit now logs a warning naming the model, the
limit, and whether the count was approximate. hasApproximateTokenCount
records which models are counted with a foreign tokenizer without being used
to shrink anything.
The tokenizer imprecision itself remains, and cannot be fixed without
per-provider BPE the repo does not have — estimateTokenCount is a
chars-per-token heuristic, and truncation needs a real encode/decode pair to
slice on a token boundary.
* refactor(embeddings): drop dead surface and enforce OpenAI's item cap
Audit follow-ups on the multi-provider embeddings work:
- Enforce OpenAI's documented 2048-entry `input` array cap in the OpenAI and
Azure adapters. Nothing bounded item count on the OpenAI path — batching
bounds tokens per request, so a batch of many short inputs could exceed it.
- Make the provider item cap single-source. It was declared both on the catalog
entry and on the adapter, read through a `??`; the adapter is the wire-protocol
owner, so the catalog copy is gone.
- Have the knowledge-base view call `getKbEligibleModels()` instead of
re-deriving the same `kbEligible` filter inline.
- Remove dead surface: the unused `EMBEDDING_TASK_TYPES` constant,
`EmbeddingToolDefinition`, `HOSTED_KEY_PROVIDERS`, and the five request-body
fields (`workspaceId`, `workflowId`, `executionId`, `userId`,
`useHostedCostTracking`) the route never reads.
- Trim `@/lib/embeddings` to what callers outside the module use.
- Drop the route's manual request-id plumbing; `withRouteHandler` supplies it.
- Fix two comments that had drifted onto the wrong declaration.
* fix(embeddings): normalize reduced Cohere output; correct OpenAI token ceiling
Second validation pass against provider documentation.
- Cohere: normalize locally when `output_dimension` reduces below native.
Cohere documents the parameter as Matryoshka truncation but never states that
it renormalizes, and an unnormalized vector silently skews cosine similarity.
`l2Normalize` is idempotent, so this is a no-op if Cohere already returns unit
vectors and a correctness fix if it does not. Covered by a test that fails
without it.
- OpenAI: raise the per-input ceiling from 8191 to the 8192 the API reference
documents, so a maximal input is no longer truncated by one token.
- Share the OpenAI response type with the Azure adapter instead of declaring an
identical copy, mirroring how the mail providers share `_nodemailer`.
- Rewrite the Gemini item-cap comment to say the 100-item limit is observed
rather than documented, which is what Google's reference actually supports.
Docs: add a manual intro to the Embeddings page covering providers, models,
inputs, outputs, and comparability rules. The generated Input tables are empty
because `createEmbeddingTool` builds params programmatically and the docs
generator only reads literals, so the manual section carries that reference.
* fix(embeddings): split per-input and per-request token limits; close provider gaps
Four gaps found in the validation pass.
Gemini token counts were estimated, not measured. `BatchEmbedContentsResponse`
carries `usageMetadata.promptTokenCount`; without reading it the client fell back
to tiktoken, which has no Gemini encoding and silently used `cl100k_base` — the
wrong tokenizer on a count knowledge-base runs bill against.
`maxInputTokens` was doing two jobs: the per-input ceiling that decides
truncation, and the per-request budget that decides how many inputs share a
batch. These are different provider limits, and conflating them meant Cohere
packed batches against its 128k per-document ceiling while OpenAI's documented
300,000-token request cap went unenforced. They are now separate fields.
Truncation moves out of `batchByTokenLimit` and into `embed`, so it happens once,
against the per-input ceiling, and always logs. The request budget is floored at
that ceiling — a budget below it would truncate inputs the provider accepts.
Batch sizes are unchanged everywhere except Gemini, which rises from 2048 to the
8192 the other providers already used.
codestral-embed now offers its documented 3072 maximum. Its API default is 1536,
so the offered sizes straddle the default; the catalog invariant relaxes from
"native size first" to "native size present", which is what the block relies on.
The Mistral API-key field no longer differs from the other three. Sim stocks
`MISTRAL_API_KEY` — `mistral_parse` already hides its key field on hosted — so
one field with `hideWhenHosted` replaces the conditional pair.
Docs: correct the API-key row, which described the old Mistral-only behavior.
* refactor(embeddings): derive block options from the catalog; use shared helpers
Findings from a four-angle quality review.
Reuse: `splitByItemLimit` and `processWithConcurrency` were reimplementations of
`chunkArray` (`@sim/utils`) and `mapWithConcurrency`
(`@/lib/core/utils/concurrency`), so `lib/embeddings/batching.ts` is gone. That
helper's doc forbade a throwing mapper; embedding legitimately wants a failed
batch to fail the call, since a partial vector set is not a usable result, so the
contract is reworded to cover both intents rather than forked.
The block no longer hand-copies the catalog. Its model, task-type, and dimension
dropdowns are derived from `EMBEDDING_MODELS`, which deletes roughly 150 lines of
literals that had to be kept in step by a drift test. The comment claiming this
was impossible was wrong: `generate-docs.ts` only reads `subBlocks` looking for
an `id: 'operation'` entry, which this block does not have. Verified by
regenerating — `embeddings.mdx` and `integrations.json` come out byte-identical.
Single-sourced two maps that were stated twice: BYOK provider ids (which encode
the non-obvious gemini -> google mapping) and the per-provider default model.
The route previously took its default from `getModelsForProvider(provider)[0]`,
which silently depended on catalog key order.
Azure's `endpoint` and `apiVersion` are required on their own context type
instead of optional on the shared one, so the adapter can no longer be built
without them and emit an `undefined/...` URL.
Also: contract enums now `satisfies` the catalog unions so they cannot drift,
the barrel exports only what callers outside the module use, the redundant
`requestedDimensions` field is a parameter, the bare `getEmbeddingModelInfo()`
call is a named `assertKbEmbeddingModel`, and the route checks payload size
before scanning entries rather than copying the body first.
* docs(embeddings): correct comments that drifted from the code
A comment pass over the feature found four that no longer matched what they sat
on, all introduced by earlier rounds of this work.
The contract's `satisfies` note promised that adding a catalog provider could
not leave the wire enum stale. It cannot deliver that: `satisfies` proves every
listed member is valid, not that the list is exhaustive, so an addition stays
silently absent. Reworded to say what it does and does not catch.
The client cited Gemini as a provider that omits usage, which the Gemini adapter
now contradicts — it reads `usageMetadata.promptTokenCount`. Every adapter
defines `parseTokens`, so the fallback is about a response lacking a usage block,
not about a particular provider.
`l2Normalize` documented only Gemini, though Cohere now calls it for a different
and stronger reason, and "normalizes in place" read as mutation when the function
returns a copy.
The route's new size-guard comment claimed it avoids copying the payload; nothing
there copies. The real reason is that summing lengths gates before the per-entry
character scan.
Also: split the derived-sub-block TSDoc so both constants carry hover text, gave
the payload cap its own doc, dropped one comment that restated a signature, and
tightened two long blocks without losing a fact.
* fix(docs): generate tool inputs for factory-built tools
The four embeddings tools rendered header-only Input tables. `extractToolInfo`
finds a tool's `params` by regex over the tool's own file, and these files hold
nothing but a `createEmbeddingTool({...})` call — the params live in the
factory's module. There was already a fallback for a same-file `...spread` base,
so this adds the cross-module equivalent: follow the factory's import and read
`params` from there.
Two things surfaced once the tables populated.
`hosting` was not in the set of keys that terminate the `params` capture, so the
non-greedy match ran past it to `request:` and swallowed the whole hosting block.
Every tool with a `hosting:` section between `params:` and `request:` was
publishing `pricing` and `rateLimit` as if they were user-facing inputs — this
drops those rows from eight unrelated integration pages as well.
The shared apiKey description was a template literal, which the regex emitted
verbatim as `${name} API key`. It is now a static string, matching how every
other tool in the repo declares one.
Docs: the Embeddings page keeps a prose intro in its MANUAL-CONTENT block like
other integrations, with the hand-written input/output tables removed now that
the generated ones are correct. The sunset `openai` page loses its
`encodingFormat` row — page generation skips hidden blocks, so that page is
frozen and would otherwise keep advertising a parameter the aliased tool no
longer accepts.
---------
Co-authored-by: Waleed Latif <walif6@gmail.com>
|
||
|
|
b04fee8aa7 |
fix(deployment): prevent trigger registry initialization crash (#6342)
* fix(deployment): initialize block registry before triggers
* fix(triggers): break the triggers <-> blocks initialization cycle
Replaces the import-order guard from the previous commit with the structural fix.
Block configs spread `getTrigger('...').subBlocks` while their module body runs, so
`blocks/*` depends on `triggers/*` by design. Thirteen edges closed the loop back the
other way, which made module evaluation order load-bearing: enter the graph through
`@/triggers` and a block config calls `getTrigger()` before `TRIGGER_REGISTRY` is
initialized, throwing
ReferenceError: Cannot access 'TRIGGER_REGISTRY' before initialization
Eleven deployment routes crashed on import: `POST /api/workflows/[id]/deploy`, the v1
public and admin deploy/rollback/activate routes, both deployment-version routes, and
the three custom-tool deployment routes. All of them funnel through
`lib/webhooks/deploy.ts`, which stayed safe only because it imported a value from
`@/blocks` — biome sorts that above `@/triggers`, so the safe barrel always evaluated
first. #6272 deleted that import as unused cleanup and took the whole surface with it.
The reverse edges came from two places, both layering violations rather than anything
inherent to triggers:
- `triggers/index.ts` imported the mock-payload generator from `trigger-utils`, which
imports `@/blocks` for unrelated helpers. The generator is pure, so it moves to
`lib/workflows/triggers/mock-payload.ts` and both callers import it there.
- Eleven trigger modules statically imported the editor's Zustand stores to read
sub-block values inside `fetchOptions`/`fetchOptionById`. Those reads now go through
`triggers/editor-state.ts`, which loads the stores with a dynamic `import()` —
resolved when the resolver is called, not during module evaluation, so it carries no
initialization-order obligation.
Side effect: `@/triggers` drops from 744 statically reachable modules to 526. The block
registry, the workflow Zustand stores and their React Query graph are no longer pulled
into every server module that imports a trigger.
`scripts/check-trigger-block-cycle.ts` fails the build if a static edge returns, and
reports the shortest offending chain. The existing suite could not have caught this —
`deploy.test.ts` mocks both `@/blocks/registry` and `@/triggers`, and `vitest.setup.ts`
mocks `@/blocks/registry` globally, so it passed 18/18 against the broken code.
---------
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
Co-authored-by: Waleed Latif <walif6@gmail.com>
|
||
|
Waleed
|
8c49d35a9c |
fix(scripts): make the sql Date-binding audit precise and crash-proof (#6340)
* fix(scripts): make the sql Date-binding audit precise and crash-proof Resolve the drizzle `sql` tag from its import binding, scope Date bindings lexically, tolerate unparseable files, accept the allow annotation above a multi-line template, and scan the root scripts directory. * fix(scripts): honor shadowed bindings and defaulted destructured Dates * fix(scripts): audit drizzle sql tags bound through a dynamic import * chore(scripts): drop the sql Date-binding unit tests and the exports that served them * chore(scripts): drop the script unit tests and the exports that served them |
||
|
Waleed
|
3e3d8605fc |
fix(uploads): drop the stray 'use server' directive that enables Server Actions app-wide (#6335)
* fix(uploads): drop the stray 'use server' directive that enables Server Actions app-wide `file-utils.server.ts` was the repo's only `'use server'` module, and the sole reason Next's `hasServerActions()` returned true. With actions registered, Next loses its early-404 escape hatch for Server Action requests — and it classifies a request as an action from headers alone, with no body inspection and no auth. Any unauthenticated `POST` with `Content-Type: multipart/form-data` to any App Router path therefore took the non-fetch action path, which bare-throws and surfaces as an HTTP 500. Nothing invokes these functions as Server Actions: every one of the ~77 importers is server-side, with zero `'use client'` importers. The directive was a misuse of `'use server'` where "server-only module" was meant — the `.server.ts` suffix already carries that convention. Extends check-client-boundary-imports.ts to fail on any `'use server'` directive so this cannot regress. * fix(scripts): match boundary directives that carry a trailing comment A directive keeps its meaning when a note follows it on the same line, so strip a trailing '//' or block comment before matching. Shared by the 'use client' and 'use server' detectors. |
||
|
Waleed
|
2ba455647b |
fix(db): bind every raw-sql Date through its column encoder (#6337)
* fix(db): bind every raw-sql Date through its column encoder `drizzle()` overwrites postgres-js's temporal serializers (OIDs 1082/1083/ 1114/1184/1182/1185/1115/1231) with an identity function because drizzle maps timestamps itself through the column's `mapToDriverValue`. A raw `sql` template carries no column context, so an interpolated `Date` skips that mapping, reaches the identity serializer unchanged, and the wire encoder throws `ERR_INVALID_ARG_TYPE`. The pools' `prepare` / `fetch_types` options are irrelevant: the serializer swap happens for all four combinations. Five live sites still interpolated a bare `Date`, the stale schedule-job filter among them — it has no try/catch, so a database async backend would surface a 500 from the schedule tick. Bind each cutoff with `sql.param(date, column)`. The testing `sql` mock's guard cannot see untested code or the tests that override the drizzle-orm mock, so add `check:sql-date-binding`: a Babel-AST audit over apps/** and packages/** that resolves Date-valued bindings per file and rejects any that reach a raw template unbound. Correct the mock's comment, which attributed the failure to postgres-js under `fetch_types: false`. * fix(scripts): require the documented sql-date-bound annotation form and a reason |
||
|
Vikhyath MondretiandSiddharth Ganesan
|
117fe3137b |
feat(code): cli sandboxes, enterprise timeouts, secrets projections, resolver lift, workflow exec cancellations (#6247)
* feat(code): cli sandboxes, enterprise timeouts, secrets projections, resolver lift * fix(execution): harden compatibility and secret diagnostics * fix(execution): harden generated JavaScript literals * fix(execution): align timeout cleanup semantics * fix(tables): decouple stale job cleanup * fix(execution): drain stale workflow backlog * test(sandbox): make deadline assertions timing-safe * fix(execution): lock cleanup candidate batches * fix(execution): preserve cleanup failure metrics * cancel route fixes * separate out mship template and func template * fix * fix(execution): harden secret projection and block runs * fix(workflow): validate draft execution state * run from block ui disabling * feat(copilot): expose Sim sandboxes to mothership * feat(copilot): expose sandbox capability catalog in VFS * Updates * fix legacy logs showing up * fix(copilot): keep sandbox config visible * fix model provenance issues * fix lint' * more lint * more * test(files): align provenance copy query order * consolidate migrations, rollout compat * integration projections * update skills * fix * add provenance linters * fix: address review and compatibility regressions * fix: make tool boundary audit Bun 1.3 compatible --------- Co-authored-by: Siddharth Ganesan <siddharthganesan@gmail.com> |
||
|
Theodore Li
|
35fd4ef42f |
improvement(self-host): simplify capability setup configuration (#6230)
* feat(self-host): add capability-aware setup * fix(self-host): preserve capability compatibility * fix(copilot): honor preview availability server-side * improvement(self-host): centralize capability resolution * fix(self-host): preserve integration availability paths * fix(testing): align capability-aware config mocks * improvement(self-host): simplify capability setup configuration * fix(setup): preserve unowned storage overrides * fix(self-host): reconcile storage and allowlists * fix(integrations): preserve connect deep links |
||
|
WaleedandBohdan Vilishchuk
|
47f5fee8cb |
fix(setup): launch the docker app the CLI is actually pointed at (#6253)
* fix(setup): detect OrbStack vs Docker Desktop before relaunching the daemon ensureDocker() always ran `open -a Docker` to relaunch a stopped daemon on macOS, which silently no-ops for OrbStack users (no Docker.app bundle exists), leading to a misleading "GUI license acceptance" timeout error. Now it checks the docker CLI's active context first (accurate regardless of install location) and falls back to checking for OrbStack.app, so the wizard launches and messages the app that's actually installed. * fix(setup): don't let an installed OrbStack override an explicit Docker Desktop context macDockerApp() fell through to the OrbStack.app existence check whenever docker context show returned anything other than "orbstack" — including a known, explicit context like "desktop-linux". With both apps installed but Docker Desktop active and stopped, this launched OrbStack while daemonUp() kept polling Docker Desktop's socket, timing out with OrbStack-flavored guidance for a Docker Desktop problem. The path fallback now only runs when the context command gives no answer at all (null); any resolved context is trusted outright. Flagged identically by Greptile and Cursor Bugbot on PR #6250. * fix(setup): fall back to the installed app when the context isn't OrbStack Context detection only fell back to the app bundle when `docker context show` failed outright, so an OrbStack-only Mac sitting on the `default` context still resolved to Docker Desktop — the same 90s hang this fix exists to remove. Treat an explicit OrbStack selection as the only positive context signal and otherwise pick whichever app is installed. Read `DOCKER_HOST` first: it overrides the active context, so the context name is not authoritative while it is set. * fix(setup): require OrbStack to be installed before selecting it A context or DOCKER_HOST left behind by an OrbStack uninstall selected an app that can never launch, turning a working Docker Desktop start into a guaranteed 90s timeout. Gate the OrbStack signal on the bundle being present and fall through to whichever app is. Look in ~/Applications as well as /Applications while here — Homebrew casks honour --appdir, so a user-local install is not unusual and a hardcoded /Applications check would misread it as "not installed". * fix(setup): resolve the docker app through LaunchServices, not fixed paths A Homebrew `--appdir` can put OrbStack anywhere, so enumerating install directories will always have a tail that reads a present app as missing and sends setup to the wrong one. Fall back to LaunchServices when the well-known directories miss: that is the same lookup `open -a` performs, so availability now agrees with what the launch will actually do. * fix(setup): settle the docker app with open(1) instead of probing for it `path to application` can raise a modal "Where is …?" picker when the name does not resolve, which in a terminal wizard reads as a hang. Drop it: the launch itself already answers the question, since `open` exits non-zero when macOS knows no such app, instantly and without UI. That inverts the design. Rather than predict which app is installed and then launch it, pick a provider, try to start it, and let the exit code correct a guess — so the directory probe no longer has to enumerate every possible install location to be right. An explicit OrbStack selection is now never redirected to Docker Desktop. The CLI is addressing OrbStack's socket, so `docker info` keeps failing no matter how well Docker Desktop starts; the earlier fallback only replaced a 90s timeout with a differently worded one. Say the context is stale and how to fix it instead. * fix(setup): honour `required` when the docker app fails to launch db.ts and redis.ts call ensureDocker(false) and branch on the boolean to offer an external Postgres or Redis instead. Throwing past that aborts the whole wizard when a working non-Docker path was on the table, so every post-confirm failure now warns and returns false unless Docker is required. That covers the 90s-timeout throw too, which ignored `required` before this branch existed — leaving it as the one path that still aborts would make the flag mean two different things in one function. Also name DOCKER_CONTEXT in the stale-selection hint. It overrides the config context, so `docker context use` alone leaves the CLI pointed at OrbStack and the next run fails identically. * improvement(setup): don't tell CLI-runtime users to install Docker Desktop Having the docker CLI but neither GUI app is exactly what a colima or Rancher Desktop user looks like, and the failure told them to install Docker Desktop — advice for a problem they don't have. Name the situation accurately and add starting an existing runtime as an option. --------- Co-authored-by: Bohdan Vilishchuk <iamtheflex@gmail.com> |
||
|
Emir KarabegandWaleed Latif
|
9b9da81a27 |
improvement(platform): drop lucide-react for the in-house icon set, flatten the type and border scales, and retire scheduled tasks and workflow references (#6241)
* border styling * improvement(platform): migrate off lucide-react, flatten the font-weight scale, and retire scheduled tasks and workflow references * chore(platform): drop the dead schedule client layer and repair stale rule and skill docs Follow-up cleanup for the platform commit, which removed the workspace scheduled-tasks surface and migrated off lucide-react. Both left dead tails that type-check clean, so nothing flagged them. Six mutation hooks in hooks/queries/schedules.ts lost their only consumer when the scheduled-tasks page was deleted: useDisableSchedule, useResumeSchedule, useDeleteSchedule, useExcludeOccurrence, useUpdateSchedule, useCreateSchedule. They are removed along with the three contract objects that served only them — disableScheduleContract, excludeOccurrenceContract, deleteScheduleContract. disableScheduleBodySchema and excludeOccurrenceBodySchema are deliberately kept: both are members of scheduleUpdateSchema, the discriminated union the live PUT /api/schedules/[id] route parses. Dropping them would collapse the union and 400 the disable and exclude_occurrence actions. The schedule-calendar tree and its utils stay unmounted for later reuse. Its TSDoc now says so, since it has no importer and would otherwise read as dead code on the next sweep. The add-enrichment skill templated an import from lucide-react, a dependency the platform commit deleted, so running it produced an unresolvable import. It now points at @sim/emcn/icons, matching all five shipped enrichments. The emcn-design-review skill and several rule files still pointed at apps/sim/components/emcn/**, which moved to packages/emcn/**. Also corrects the documented Chip variant list — it advertised a ghost variant that never existed and omitted border — repoints the sim-url-state date-parser example at an inline snippet now that its source file is gone, and normalizes the one strokeWidth the icon migration left at 1.5 in bubble-chat-delay. * fix(platform): mark the resource chrome as client components `skills/page.tsx` is a Server Component, and this branch moved its `IntegrationTabsHeader` import onto the `@/app/workspace/[workspaceId]/components` barrel. That barrel re-exports `SortDropdown` from `resource-options`, which calls `useState`, so the server graph now reaches a client-only module and `next build` fails. `resource-header` has the same latent problem (`useState`, `useEffect`, `useRef`). Both files are genuinely client components, so they get the directive rather than the page dropping the barrel import — local feature barrels are the convention here. Also drops a stale `lucide-react` mention now that the dependency is gone. * chore(scheduled-tasks): remove the scheduled-task logic Scheduled tasks are retired. This removes the `sourceType = 'job'` half of `workflow_schedule` from the application, leaving the workflow Schedule trigger (`sourceType = 'workflow'`) untouched. Gone: - the job orchestration layer (`lib/workflows/schedules/orchestration.ts`) and the agent-job runner in `background/schedule-execution.ts` - the job claim/dispatch half of the schedules execute tick - POST /api/schedules (job creation) and the job branches of GET /api/schedules and PUT/DELETE /api/schedules/[id] - the copilot job tools and handlers, the `scheduledtask` resource type and chat-context kind, and the VFS `jobs/` materialization - the scheduled-task analytics events and the job variant of the schedule-disabled email Kept on purpose: `scheduled-tasks/components/schedule-calendar/**` and `scheduled-tasks/utils/**`, which the agents module will reuse. `packages/db/schema.ts` is deliberately untouched — the columns stay for now and come out in a follow-up with a proper expand/contract migration. The generated copilot catalog and VFS snapshot types are regenerated from the matching copilot PR, which removes the tools and the `jobs` snapshot field at the source. Verified: 23/23 type-check, biome, api-validation, production build, and the full vitest suite (18361 passing; the one failure in executor/handlers/pi/cloud-review-tools.test.ts predates this branch). * fix(sidebar): derive the settings and switcher widths from SIDEBAR_WIDTH This branch moved `SIDEBAR_WIDTH.DEFAULT` from 248 to 238 but left two hardcoded `248px` chrome widths behind, so both sat 10px wider than the live sidebar: - the workspace-switcher menu, which is meant to line up with the sidebar column it drops out of - the standalone settings sidebar, whose own comment says to keep it in step with the in-workspace chrome Both now read `SIDEBAR_WIDTH.DEFAULT` directly rather than repeating the number, so the next change to the constant cannot leave them stale again. * fix(schedules): stop the API accepting actions it no longer handles Adversarial pass on the scheduled-task removal found a real regression in PUT /api/schedules/[id]. Removing the job-only `update` and `exclude_occurrence` handlers left them in `scheduleUpdateSchema`, so those bodies still parsed. The handler chain is `disable` first and then an unguarded fall-through to reactivate, so an `action: 'update'` request would have silently REACTIVATED the schedule instead of being rejected. Both actions are dropped from the discriminated union, so `parseRequest` now rejects them with a 400. Their bodies, response types and the orphaned `createScheduleContract` (its POST route is gone, and nothing imported it) go with them. * chore(landing): retire the scheduled-tasks marketing surface The feature is gone from the product, so the marketing pages stop selling it. - deletes the `/scheduled-tasks` landing page and its calendar-loop hero, and the `LandingPreviewScheduledTasks` panel - drops the view from the landing preview: the `SidebarView` member, the nav entry and its now-unused Calendar icon, the callout label, both render branches, and the staged chat copy in `workflow-data` - removes the navbar and footer links and the sitemap entry - removes the route from `LANDING_ROUTES`, the COEP exemption list that must list every `app/(landing)` route `/scheduled-tasks` is indexed, so it 301s to `/workflows` rather than starting to 404 — that is the surface that still carries scheduled execution via the workflow Schedule trigger. Left alone deliberately: `demo-scheduler` is the Cal.com booking embed for the demo page, unrelated to this feature, and the scheduling library article is a generic SEO piece that never pitched it. * perf(chat): stop the resource picker fetching schedules it no longer shows Dropping the `scheduledtask` group from the add-resource dropdown left `useWorkspaceSchedules` behind, so the picker still issued a workspace schedules request whose result never reached a group. Worse than a wasted request: `schedulesPending` was still in the hydration gate, so the whole picker waited on that response before it could settle, and `schedules` was still a `useMemo` dependency, re-running the group build when it resolved. The hook and its route stay — `/api/schedules?workspaceId=` still correctly lists workflow schedules, unlike `createScheduleContract`, whose route this branch removed. * chore(scheduled-tasks): drop the leftovers the removal stranded An independent audit of the branch turned up dead code and stale docs that the compiler cannot see — nothing behavioural, but all of it rots silently. - README still sold the feature: the "Scheduled tasks" tile, the prose listing it as a workspace surface, and the now-unreferenced screenshot. The landing surface went in c61770a8c; this tile was missed. - `resource-content.tsx`: `SCHEDULE_STATUS_LABEL`, `formatScheduleInstant` and `ScheduledTaskField` were orphaned when the schedule render branch went. - `computeNextRunAt`: zero callers, including tests — its only consumer was the removed agent-job runner. - `applyScheduleUpdate`'s `allowCompleted` option: no call site passes it, and its comment described self-completion, which no longer exists. The guard stays (legacy `sourceType='job'` rows still carry `status='completed'` until the DB follow-up); it is simply unconditional now. - Three TSDoc blocks still described a create-job route and "opening a scheduled-task artifact". Type-check re-run with --force, since a cached turbo replay is not a check. --------- Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
Siddharth Ganesan
|
5ab5f2c7ed |
feat(browser, terminal): implement browser driver, password manager, terminal features (#6196)
* icon styling * feat(desktop): isolate chat browser and terminal sessions * feat(desktop): uncap browser and terminal tabs * feat(desktop): polish browser and terminal resources * improvement desktop * fixes * updates * fixes * fix * update tests |
||
|
Waleed
|
3de63c94e3 |
feat(self-host): align Docker Compose with Helm and overhaul self-hosting docs (#6225)
* feat(self-host): align Docker Compose with Helm and overhaul self-hosting docs Docker Compose shipped no scheduler, so scheduled workflows, every polling trigger, connector syncs, the outbox, and data drains silently never ran. Adds a cron service running the same 18 jobs the Helm chart schedules as CronJobs, and closes the remaining behavioral gaps between the two paths: bundled Redis in the chart, no hosted plan caps in chart defaults, pinned image tags, and fail-fast secrets. A CI check keeps the schedulers in sync. Also rewrites the self-hosting docs: 14 new pages, 8 updated, reorganized into Install / Configure / Operate. * fix(self-host): drop bun install from chart CI, remove air-gapped and backup docs The scheduler-parity check pulled a full dependency install into the chart-validation job, which fails building isolated-vm on that runner. Rewritten to use only node builtins so the job installs nothing. Also removes the air-gapped and backup/restore pages, and stops pinning a concrete release in the docs so the examples do not go stale each release. * fix(helm): bundle Redis in secret-manager modes unless the URL is supplied Suppressing Redis whenever a secret mode was active left those deployments with no Redis at all — REDIS_URL is optional there and both shipped examples omit it. The chart now steps aside only on a detectable signal: an explicit app.env.REDIS_URL, an ESO remoteRefs.app.REDIS_URL mapping, or the new redis.provideUrl=false opt-out for a pre-created Secret it cannot read. * fix(compose): derive realtime BETTER_AUTH_URL from NEXT_PUBLIC_APP_URL realtime read BETTER_AUTH_URL directly and fell back to localhost while simstudio derived it from NEXT_PUBLIC_APP_URL, so setting only the public origin left realtime authenticating against http://localhost:3000. * fix(helm): deliver bundled REDIS_URL via ConfigMap so an operator value always wins Injecting REDIS_URL as an inline container env made it beat every envFrom source, so a REDIS_URL held in a pre-created Secret or synced by External Secrets was silently shadowed and traffic moved to a fresh in-cluster Redis. Kubernetes resolves duplicate envFrom keys by letting the last source win, so the bundled URL now ships as a ConfigMap listed before the app Secret. Any operator-supplied value overrides it without the chart needing to read it, which also removes the redis.provideUrl flag the previous attempt required. * docs(helm): spell out the egress rule external datastores need The default NetworkPolicy allows 443 plus the bundled Postgres and Redis by pod selector. Anything you run outside the chart on another port needs its own rule, which is easiest to miss when REDIS_URL arrives via a Secret the chart cannot inspect. Adds a copyable example to the production checklist and the security guide. * feat(helm): add networkPolicy.allowExternalEgress for managed datastores The default policy allows 443 plus the bundled Postgres and Redis by pod selector, so a managed datastore on another port needs a hand-written CIDR rule — awkward when REDIS_URL arrives via a Secret the chart cannot inspect. Adds an opt-in switch that drops the port restriction while still blocking the cloud metadata endpoints. Defaults to false, keeping this chart stricter than the common chart default of unrestricted egress. |
||
|
Theodore Li
|
3f70096841 |
improvement(self-host): gate email verification on a mail provider, add self-host settings, land setup on signup (#6216)
* fix(auth): skip email verification when no mail provider is configured Signup pushed /verify unconditionally, stranding self-hosted deployments with no mail provider on a screen no email could ever satisfy. Derive one server-side effective value (verification enabled AND deliverable) and read it from Better Auth enforcement, signup routing, and the verify page. * feat(settings): add a self-host section with the managed Chat keys link Self-hosters had no in-app pointer to the managed service that issues their Chat keys. New Settings > System > Self-host section, gated on `requiresSelfHosted` so it is absent on hosted Sim, containing only that link. * improvement(setup): land the wizard handoff on signup A freshly provisioned deployment has no accounts and / renders the marketing landing page, so the bare origin left operators hunting for the CTA. Single-source the URLs and point every open-Sim handoff at /signup across all three modes. * improvement(settings): mark the self-host section with a sprout Server was already doing double duty for MCP servers and Mothership, and the icon set ships no botanical glyph, so the mark is a text emoji. * improvement(settings): draw the sprout as an emcn line icon, move to Platform The emoji rendered in the platform's own colors, so it was the one glyph in the nav that ignored --text-icon. Replaced with a hand-drawn emcn Sprout (24 grid, 1.55 stroke, currentColor) matching the house style, renamed the tab to Self hosting, and regrouped it under Platform — self-hosting is deployment-wide, not per-workspace. Still self-hosted-only. * improvement(settings): drop the section header from self hosting One row does not need a section label, and removing it takes the divider with it. The body is now the Chat keys row and its managed-keys link, nothing else. |
||
|
mzxchandraandWaleed Latif
|
87aeca6f0c |
feat(zoho-desk): add Zoho Desk integration (#6157)
* feat(zoho-desk): add Zoho Desk integration
Add a full Zoho Desk integration: tools, block, icon, and a webhook trigger.
Tools (tools/zoho_desk): list/get/update tickets, list/add comments,
list/get threads, get contact, list organizations, and download attachments
as UserFiles via an internal route. Registered in tools/registry.ts.
Block (blocks/blocks/zoho-desk.ts): operation dropdown, OAuth credential,
an organization selector backed by GET /organizations, per-operation fields,
and BlockMeta templates. Wires the Zoho Desk trigger.
OAuth (zoho-desk provider): authorize/token at accounts.zoho.com with
access_type=offline + prompt=consent; the Desk REST base is derived from the
token response api_domain and persisted so calls honor data residency instead
of assuming desk.zoho.com. Every call sends Authorization: Zoho-oauthtoken and
the orgId header.
Trigger + webhook handler (triggers/zoho_desk, lib/webhooks/providers/zoho-desk.ts):
Sim creates and tears down the Zoho Desk webhook subscription. Inbound events
are verified with JWT RS256 (X-ZDesk-JWT) against the data-center JWKS, ACKed
via the durable queue to meet Zoho's 5s deadline, and fail loudly on
Free/Standard editions that cannot create webhooks.
* fix(zoho-desk): OAuth PKCE, DC scope-marker parsing, SSRF, and e2e fixes
OAuth: forward code_verifier in the custom getToken (PKCE is enabled, so the
exchange must echo the verifier or Zoho rejects the request with invalid_request).
Surface Zoho's error/error_description, which it returns in the JSON body with
HTTP 200, instead of collapsing every failure into "no access token".
Data-center base parsing: better-auth persists Zoho's scopes comma-joined with no
spaces, so the greedy \S+ marker regex swallowed the whole scope list into the
host. Stop the capture at a comma or whitespace in both read sites (token route
and webhook handler), so apiDomain resolves to the real Desk host.
Attachment SSRF: replace the permissive host regex (which accepted attacker
domains like zoho.attacker.com) with a strict Zoho-apex suffix allowlist.
Block: guard Number() pagination so a non-numeric typo can't send NaN; add the
ignoreSourceId -> sourceId loop-guard header to update_ticket (matching add_comment).
Organizations route: surface fetch/Zoho failures with a real status instead of a
200 with an empty list, so the org selector no longer fails silently.
* fix(zoho-desk): webhook creation, attachment naming, and HTML content handling
Webhook trigger (verified end-to-end against a live Enterprise org):
- Omit ignoreSourceId; Zoho rejects a non-Zoho UUID with INVALID_DATA. Drop
the generateId() fallback and its providerConfig persistence.
- Answer Zoho's create-time notification-URL probe via the existing pending
webhook verification mechanism (GET/HEAD matchers) so subscription creation
no longer 405s.
- mapZohoWebhookError now surfaces Zoho's real errorCode / message / field
errors instead of a catch-all edition message, and attaches an HTTP status so
4xx flow through NonRetryableDeploymentError while 429/5xx stay retryable.
- Propagate the real status through deploy.ts so failed creates don't retry-loop.
get_attachment polish:
- Return the downloaded file's name under `name` (ToolFileData key) instead of
`filename`, and derive it (explicit -> Content-Disposition -> URL segment ->
fallback) so attachments are no longer stored as "untitled".
- Gate the add_comment-only `contentType` param so it isn't sent to get_attachment.
HTML content handling (Zoho content fields emit raw HTML):
- Add a Zoho-local html-to-text converter mirroring the Outlook dual-field
pattern: when contentType is 'html', derive a plain-text `contentText`
alongside the untouched raw `content` + `contentType`; plainText mirrors.
- Apply to comments (list/add), threads (list/get), the ticket description
(descriptionText), and the webhook trigger payload.
Trigger org selector: Organization is now a credential-scoped combobox that
lists the connected account's Zoho Desk organizations.
* fix(zoho-desk): review round - DC-base derivation, org-loader resilience, batched-event visibility
- deriveZohoDeskBaseFromApiDomain: preserve an already-regional desk.zoho.<tld>
api_domain instead of falling back to the US (.com) data center, and map the
DC TLD from any zoho(apis).<tld> host - keeps Desk calls in the right data
center for residency.
- fetchZohoDeskOrganizationOptions: wrap the token/org fetch in try/catch and
degrade to an empty list (the org field is a free-text combobox, so manual
entry still works) instead of hard-failing the selector on token/DC/network
errors.
- formatInput: warn (not silently drop) if Zoho ever delivers more than one
event in a single payload.
* fix(zoho-desk): harden attachment download against redirect-based SSRF/token leak
Replace the raw fetch in the attachment route with secureFetchWithValidation
(the same guarded fetch the copilot file-download tool uses). The download URL
is user/LLM-influenced and Zoho may redirect, so auto-following redirects could
send the OAuth token / orgId to an untrusted or internal host. The guarded fetch
pins the resolved IP, blocks private/reserved targets on every hop, drops the
Authorization header if a redirect leaves the origin (stripAuthOnRedirect), and
enforces the 50MB cap while streaming. The strict Zoho apex allowlist still
gates the initial origin as defense in depth.
* fix(zoho-desk): only add the edition hint when Zoho's error indicates it
mapZohoWebhookError appended the "requires Professional edition or higher"
guidance to every 403, but a 403 can also mean a wrong org, a missing scope, or
a bad token. Gate the hint on Zoho's own errorCode / message matching the
permission/edition pattern instead of the bare status, so unrelated 403s surface
Zoho's real reason without the misleading suffix. Adds a test for the
non-edition 403 path.
* fix(zoho-desk): stop duplicating /api/v1 when resolving a relative attachment href
A relative attachment href that already starts with `api/v1` (as Zoho's hrefs
often do) was concatenated onto getZohoDeskApiBase (which ends in /api/v1),
producing `/api/v1/api/v1/...` and a failing download. Extract a tested
resolveZohoAttachmentUrl helper that uses absolute hrefs as-is and strips a
leading slash + `api/v1/` prefix from relative ones before joining, so the path
is correct for absolute, root-relative, and api/v1-prefixed hrefs alike.
* fix(zoho-desk): reject an empty update_ticket PATCH with a clear error
update_ticket built its PATCH body from optional fields via filterUndefined, so
a call with no fields set sent `{}` and surfaced an opaque Zoho failure. Guard
the body builder to throw an actionable "provide at least one field" error
before the request. Adds a test for the empty and populated body paths.
* fix(zoho-desk): fall back to the credential Desk domain in webhook JWT verify
verifyAuth chose the JWKS host from providerConfig.apiDomain and otherwise
defaulted to the US host (desk.zoho.com), so a non-US webhook row missing
apiDomain would verify against the wrong JWKS and reject legitimate events. When
apiDomain is absent, resolve it from the OAuth credential's __zoho_domain__ scope
marker (mirroring deleteSubscription). The persisted-apiDomain fast path stays
DB-free to respect the 5s delivery deadline. Adds tests for both paths.
* fix(zoho-desk): apply the Zoho host allowlist to the organizations route
The organizations route built its URL from the client-supplied apiDomain and
attached the OAuth token without the https-Zoho-host allowlist the attachment
route already enforced, so a session-access caller could point the server at an
arbitrary origin and leak the token. Extract the shared isZohoHost allowlist and
an assertZohoUrl guard into tools/zoho_desk/utils (two consumers now), guard the
organizations URL before fetching, and refactor the attachment route to reuse
the shared helper. Adds tests for the allowlist and guard.
* fix(zoho-desk): propagate provider 4xx in the stable webhook prepare path
The v2 stable deploy preparation flattened every registration failure (except
path conflicts) to HTTP 500, so a provider-attached permanent 4xx - e.g. Zoho's
edition/validation failures from createSubscription - retried instead of failing
the deploy terminally. Propagate the attached status (`?? 500`), matching the
legacy save path's status-aware mapping so both deploy paths route 4xx through
NonRetryableDeploymentError.
* fix(zoho-desk): make createSubscription config failures non-retryable
createSubscription threw plain Errors (no status) for missing orgId, event type,
or credentials, and for a Zoho success with no webhook id - so the deploy outbox
mapped them to 500 and retried permanent configuration failures. Attach a 4xx
via statusError (400 for missing config/credentials; 422 for the no-id anomaly,
where a retry risks duplicate webhooks) so they fail the deploy terminally like
the mapped Zoho API 4xx responses. Tests assert the 400 status on the guard paths.
* fix(zoho-desk): enrich prevState with contentText symmetrically with payload
formatInput derived plain-text contentText only on payload, so an update event
for a comment/thread left prevState as raw HTML while payload carried
contentText - inconsistent shapes for before/after comparisons. Apply
withDerivedContentText to prevState too. Test asserts both are enriched.
* docs(zoho-desk): regenerate integration docs
Regenerate zoho_desk.mdx from the current tool definitions: removes the stale
add_comment `ignoreSourceId` input row (the field was dropped because Zoho
rejects arbitrary values) and adds the derived `contentText` / `descriptionText`
plain-text fields on comments, threads, and tickets.
* fix(zoho-desk): validate the persisted Desk base against the strict host allowlist
deriveZohoDeskBaseFromApiDomain trusted any host matching `desk.zoho.[a-z.]+`,
so a crafted api_domain like `desk.zoho.com.attacker.com` passed and was
persisted as the credential's `__zoho_domain__` REST base - later receiving the
OAuth token on every Desk tool/webhook call. Gate the derivation on the strict
isZohoHost apex allowlist (which rejects that lookalike), extracted with
assertZohoUrl into a dependency-free host-allowlist module so the auth
token-exchange path validates hosts without pulling in the tool utilities. The
attachment and organizations routes now import the shared guard from there.
Also: formatInput now emits the normalized null trigger shape for an empty/
malformed event array instead of leaking a raw `[]` to downstream steps. Tests
cover the empty-array shape and the lookalike-host rejection.
* fix(zoho-desk): correct API field names, scopes, and host validation
Validation pass against Zoho's published Desk API surfaced six defects that
typecheck, lint, and the existing suite all passed over, because each one fails
silently against the live API rather than erroring.
Wire-name mismatches (Zoho ignores unknown keys, so all three were silent):
- update_ticket sent `customFields`; the ticket PATCH body names it `cf`.
`customFields` exists only as a deprecated alias on other Desk resources and
on the separate validate-field-updates endpoint, so updates reported success
and applied nothing.
- ZOHO_DESK_TICKET_PROPERTIES and ZOHO_DESK_CONTACT_PROPERTIES advertised a
`customFields` output; both resources return `cf`. The declared field always
resolved undefined and the real one was undeclared.
- list_tickets sent `departmentId`; the query param is `departmentIds`, so the
department filter was dropped and every department's tickets came back.
Content handling:
- deriveZohoContentText matched `contentType === 'html'`, but Zoho spells the
discriminator per resource: comments use `html`, threads use the MIME form
`text/html`. Every thread's `contentText` was therefore raw markup - the exact
opposite of the field's purpose. Now normalized across both spellings,
parameterized values, and casing, with regression tests.
Scopes (least privilege):
- Desk.tickets.ALL -> Desk.tickets.READ + Desk.tickets.UPDATE. No tool creates
or deletes a ticket; ALL additionally granted ticket DELETE.
- Dropped Desk.search.READ (no search tool exists) and Desk.webhooks.READ /
.UPDATE (the provider only creates and deletes), plus their orphaned
SCOPE_DESCRIPTIONS entries.
Host validation - the webhook provider was the only token-carrying path not
anchored to the Zoho apex allowlist, including the JWKS fetch, where an
unrecognized host would have stood in as the JWT issuer:
- createSubscription, deleteSubscription, and verifyAuth now route their base
through a shared allowlist check.
- getZohoDeskApiBase validates rather than trusting injection precedence.
- The organizations route uses secureFetchWithValidation with
stripAuthOnRedirect, matching the attachment route it had diverged from.
Block and trigger:
- The trigger's department field is renamed `triggerDepartmentIds`; sharing the
`departmentIds` id let a value typed as a list_tickets filter become the
webhook subscription's filter when switching modes.
- `isPublic` no longer serializes onto all ten operations, matching the existing
gating for `contentType`.
- from/limit reject negatives and fractions instead of forwarding them.
- update_ticket gains description, resolution, and classification (all already
declared as outputs), and a departmentId input so a ticket can be moved.
Accuracy corrections to user-facing text, all against the published parameter
tables: `from` is 0-based (0-4999, default 0), not 1-based; per-endpoint limits
are tickets 1-100/10, comments 1-100/50, threads 1-200/100; sortBy lists Zoho's
actual allowed values; the two `include` sets genuinely differ per endpoint;
status and priority accept comma-separated lists.
Also: path IDs are trimmed via requireZohoDeskId so a pasted trailing space
fails with a clear message instead of a %20 404; comment `commenter` and thread
`status`/`isDescriptionThread`/`visibility`/`canReply` are now declared;
ZOHO_CLIENT_ID/SECRET added to the oauth test env; docs page gains a
MANUAL-CONTENT intro covering capabilities, the Professional-edition webhook
requirement, and the US-data-center limitation.
Not verified from documentation, needs a live account before merge:
- the OAuth scope for the attachment content sub-path (Zoho publishes none, and
there is an unanswered SCOPE_MISMATCH report against it)
- 12 of the 17 offered webhook event ids (5 are confirmed); Ticket_Delete is
documented but not offered
- the ticket `descriptionContentType` key, and the POST /api/v1/webhooks body
shape, neither of which appears in any reachable Zoho reference
* chore(zoho-desk): regenerate tool metadata
The param and description corrections in the previous commit changed the
generated tool surface, so tool-metadata:check failed in CI. Regenerated;
the diff is two Zoho-only lines.
* fix(zoho-desk): stop posting null for untouched update_ticket fields
`filterUndefined` strips only `undefined`, but an untouched subBlock never
arrives as `undefined`: the workflow serializer initializes every subBlock value
to `null` (stores/workflows/utils.ts) and extractBlockParams writes those nulls
straight into tool params, with nothing between the serializer and request.body
filtering them.
Reproduced against the real serializer and block with only `status` set:
basic {"subject":null,"status":"Closed"}
advanced {"subject":null,"status":"Closed","priority":null,...,"cf":null}
`subject` leaks even in basic mode because it declares no `mode`, so
shouldSerializeSubBlock never drops it. Zoho documents subject as a writable
field, so every status-only edit either failed the PATCH or blanked the ticket's
subject; in advanced mode the whole update surface nulled out, including `cf`.
Two things hid this. The empty-PATCH guard was unreachable from the block (the
body always carried at least `subject`), and the existing test called buildBody
with fields *absent* rather than null - the shape the block never produces - so
it could not fail on the real path.
Replaces filterUndefined with a local omitUnset that drops undefined, null, and
'' (a cleared input means "leave unchanged", not "set to empty"). Adds three
tests using the real serializer shape, all verified to fail before the fix.
Also fixes the same null-blindness in the block's param mapping, where
Number(null) === 0 injected from=0 on every operation, and corrects the shared
limit placeholder, which claimed max 100 while list_threads allows 200.
* feat(zoho-desk): add Self Client service-account credential
Adds a second way to connect Zoho Desk, alongside the interactive OAuth flow: a
Zoho Self Client, pasted as client id + client secret + organization id. Built
on the existing client-credential-accounts framework rather than a new credential
path, so it behaves like the Zoom Server-to-Server and Box CCG accounts already
in the repo - a short-lived token minted on demand, no refresh token.
Two Zoho behaviors the generic framework does not cover:
- `scope` must be COMMA-separated on Zoho's token endpoint; a space-separated
list is rejected as an invalid scope. The list comes from
getCanonicalScopesForProvider('zoho-desk'), so the Self Client and the OAuth
flow can never drift apart on scopes.
- Zoho reports OAuth failures in the JSON body, frequently with HTTP 200
(e.g. {"error":"invalid_client"}), so the success body is inspected for an
`error` field before the token is read - a status-only check would accept a
failed mint.
deriveZohoDeskBaseFromApiDomain moves out of auth.ts into the dependency-free
host-allowlist module so the minter and the OAuth path share one derivation
instead of duplicating it, and the mint response's api_domain now flows through
to tools as `apiDomain` (the SA branch of the token route previously returned
none, so SA calls would have assumed desk.zoho.com).
Docs: hand-authored zoho-desk-service-account.mdx following the existing
*-service-account.mdx pages, registered in meta.json and in the generator's
keep-list so stale-page cleanup does not delete it.
Known limitation, documented in the descriptor helpText and the docs page:
webhook triggers still require an OAuth connection. Webhook provisioning resolves
credentials through getCredentialOwner/refreshAccessTokenIfNeeded, which is
OAuth-account-only for every provider in the repo - not a Zoho-specific gap.
Unverified from documentation, needs a live Zoho org before merge:
- the `ZohoDesk.` soid prefix. Zoho documents only the syntax
{servicename}.{zsoid} with a single CRM example; no first-party doc states the
Desk prefix. normalizeZohoDeskSoid passes through any value already containing
a '.', so an operator can paste a corrected full soid without a code change.
- whether zsoid is the same identifier as the Desk orgId header value.
- whether the client-credentials endpoint accepts Desk.webhooks.CREATE/DELETE
for a Self Client.
- whether the mint response populates api_domain for Desk (documented for CRM);
if absent the derivation falls back to the US Desk host.
* fix(zoho-desk): derive descriptionText for ticket-shaped payloads
Cursor Bugbot: webhook ticket events reached workflows as raw HTML with no
plain-text sibling. `withDerivedContentText` only looked at `content` /
`contentType`, but ticket resources carry their body on `description` /
`descriptionContentType`, so trigger output disagreed with get_ticket.
The helper now derives both, which also removed two inconsistencies on the tool
side: get_ticket had its own inline copy of the derivation (now one shared
implementation that cannot drift), and update_ticket returned its PATCH response
raw despite the shared output map declaring descriptionText.
`descriptionContentType` remains the one field name unconfirmed in any Zoho
reference. It degrades safely - an absent key makes deriveZohoContentText return
the value unchanged, so descriptionText mirrors description rather than breaking,
exactly as get_ticket already behaved - and it is now one helper to correct if
Zoho names it differently.
* feat(zoho-desk): let the service account pick its data center
Zoho's accounts server is per region, and the integration pinned every call to
the US host. For the interactive OAuth flow that is currently unavoidable -
better-auth's authorize/token URLs are static per provider - but the service
account mints its own token, so the region can simply be chosen. This makes the
Self Client the only way a non-US Zoho org can connect.
Adds an optional `dataCenter` field to the client-credential framework. Optional
matters: ClientCredentialAccountFieldId and ClientCredentialAccountFields are
shared with Zoom, Box and Salesforce, whose descriptors and minters are
unchanged. Blank keeps the previous behavior (US), so existing credentials are
unaffected.
Only us/eu/in/au are offered - the four regions where both the accounts server
and the Desk REST host are confirmed. CA is deliberately absent: Zoho's accounts
docs say accounts.zohocloud.ca while Zoho's own Desk SDK says accounts.zoho.ca,
and the two cannot both be right. JP/SA/CN/UK lack a confirmed Desk host.
The Desk base is now derived from the selected region rather than inferred from
the mint response, which also removes a dependency on `api_domain` being
populated for Desk (Zoho documents it for CRM only). When `api_domain` IS present
and disagrees with the region, it wins - it is authoritative about where the
token actually works - and the mismatch is logged so a mis-selected region is
diagnosable. deriveZohoDeskBaseFromApiDomain gains a `try` variant returning
undefined so an untrusted api_domain can no longer masquerade as an authoritative
US answer and silently override a correct region.
A wrong region fails loudly rather than silently: the minter runs as verification
on both create and reconnect, so the credential is never persisted in a broken
state. Because Zoho reports it as `invalid_client` - a Self Client only exists on
its own region's accounts server - the operator hint for that code now names the
data center as a candidate cause.
Copy is scoped per path rather than blanket "US only": the OAuth service
description, trigger setup instructions, and the docs intro now say which path
each limitation applies to, and the service-account page documents the four
regions with a sign-in-domain to region-code table.
* fix(zoho-desk): strip ticket description HTML, classify body-reported refresh failures
Final validation pass findings.
descriptionText never stripped anything. It was gated on a
`descriptionContentType` discriminator that Zoho does not send: the Ticket_Add
webhook sample ships `"description": "<div>Description</div>"` with no such key,
and the ticket GET/PATCH response field lists have no content-type sibling
either. So get_ticket, update_ticket, and every webhook ticket payload emitted
descriptionText as a byte-identical copy of the raw HTML, while the declared
output promised stripped text.
The tests did not catch it because they fabricated the shape - both fixtures
constructed `descriptionContentType: 'html'`, a key Zoho never emits, proving the
branch works without proving it is ever taken. Ticket descriptions are HTML by
convention, so the strip is now unconditional (html-to-text is a near-identity on
genuinely plain text), an explicit descriptionContentType is still honored if
Zoho ever adds one, and the fixtures now use Zoho's real shape with no
content-type key anywhere.
A body-reported refresh failure was unclassified. Zoho answers a revoked refresh
token with HTTP 200 and `{"error":"invalid_client"}`; refreshOAuthToken only
checked `data.ok === false` (a Slack-ism), so the request fell through to the
"no access token" guard and returned no errorCode. isTerminalRefreshError could
therefore never recognize invalid_client as terminal, the credential was never
marked dead, and every later execution retried a refresh that cannot succeed -
with the user shown "No access token in refresh response" instead of a reconnect
prompt. The body is now classified before the status is trusted, matching what
the token exchange and the service-account mint already did. That guard also
stopped logging the whole response body, which carries live tokens on a partial
success.
Also: an unrecognized dataCenter now fails with a named error instead of quietly
resolving to US and surfacing as an opaque invalid_client (blank still means US);
the webhook JWKS cache is bounded, since its key derives from a providerConfig
field that SYSTEM_MANAGED_FIELDS protects from diffing but not from being
written; and the attachment `size` output no longer asserts bytes, a unit Zoho
documents as KB.
* feat(zoho-desk): canonical selectors and BlockMeta skills
The block picked its organization with an ad-hoc `combobox` + `fetchOptions`.
Only five blocks in the repo did that, and the other four are core blocks
(agent/credential/function/logs) - no other OAuth integration used it. Every
other resource a user has to identify was a bare short-input taking an opaque
numeric id.
Zoho Desk now uses the same machinery as the other 25 selector providers:
hooks/selectors/providers/zoho-desk/selectors.ts registered in the selector
registry, consumed from the block as basic selector + advanced manual input
sharing one canonicalParamId, for organization, update-ticket department, and
the list-tickets department filter. The trigger's org field moves to the same
selector. zoho-desk-org-options.ts is deleted rather than left beside the new
path, so blocks/ has zero fetchOptions usages outside the core blocks.
Wire params are unchanged (orgId, departmentId, departmentIds, assigneeId,
ticketId, contactId) - this is a UI change, not an API change.
The organizations route now resolves the credential server-side. It previously
had the browser fetch an access token and POST it back, which an earlier audit
flagged as the one place a Zoho token left the server; the new selector-credential
resolver keeps it server-side for both the OAuth and service-account credential
types and re-anchors every outbound host to the Zoho apex allowlist.
No agents selector: the endpoint is documented but its OAuth scope is not, and
the nearest evidence points at Desk.agents.READ, which we do not request. Adding
it would force every existing Zoho Desk user to reconnect for a convenience
field, so assigneeId stays a manual input until the scope can be confirmed
against a live org.
Adds the skills array BlockMeta was missing - 227 of 300 blocks declare one and
this did not. Seven skills, each grounded in a use case Zoho or the ecosystem
actually advertises (auto-triage, SLA escalation, digest, AI draft reply,
customer context, engineering handoff, knowledge-gap report) and each exercising
only tools in tools.access. CSAT surveys, ticket creation, dedup and keyword
search were deliberately left out: the integration has no tool for them, and a
skill implying an unsupported action is worse than a shorter list.
* feat(zoho-desk): agents selector and free-text trigger organization
Three improvements that were previously deferred only to avoid forcing existing
users to reconnect or orphaning saved workflows. This integration is unmerged and
has no users, so the constraint does not apply and the better option wins.
assigneeId was the last field still asking for an opaque numeric id. It is now a
canonical selector pair backed by a new zoho_desk.agents selector, which required
adding the Desk.agents.READ scope - the reason it was skipped before. Route
follows the departments one exactly: auth before parseRequest, host anchored to
the Zoho apex allowlist, secureFetchWithValidation with stripAuthOnRedirect, and
a page drain capped at 20 pages with 204 treated as end-of-list.
Scope caveat: Zoho publishes no explicit scope line for the list-all
GET /api/v1/agents. Every other endpoint in the Agents module documents
Desk.agents.READ (get by id, get by email, roles/{id}/agents), and it is the only
agents-module scope Zoho defines, so that is the basis. Inference across a module
rather than a direct quote - worth one live call before merge, same as the
existing attachment-scope note.
The trigger regained free-text organization entry, lost when the org field became
a selector. The earlier concern - that a manual value would land under its raw
subBlock id and never reach the provider - turned out not to hold: buildProviderConfig
already collapses canonical pairs and writes the active member under the canonical
key. The real gap is narrower and does exist: when canonicalModes pins the group
to basic while only the manual field has a value, the collapse deletes the
canonical key even though the required-field check passes, so the deploy succeeds
and then fails at subscription time. resolveConfigOrgId closes that, with a test.
The block/trigger `orgId` id overlap stays shared, now with a comment. Two earlier
audits disagreed; renaming turns out to be the wrong call. buildCanonicalIndex has
an explicit guard for trigger-mode reuse and blocks.test.ts codifies it as a valid
pattern, orgId means the same portal in both modes (unlike departmentIds, which is
correctly distinct), and a separate triggerManualOrgId would put two advanced
members in one canonical group - getCanonicalValues takes the first non-empty, so
a stale tool-mode value could silently supply the trigger's organization.
* fix(zoho-desk): make the attachment cap reachable, unbreak selector paging
Final audit round.
The 50 MB attachment ceiling could never be hit. This route returns the file as
base64 inside its JSON body, and the executor reads internal tool responses
through readToolResponseBody, capped at 10 MB. Base64 inflates 4/3, so ~7.5 MB
of raw bytes is the real ceiling - and the old limit meant a larger attachment
was downloaded, encoded and serialized in full (peaking near 250 MB of live
allocation, with nothing bounding concurrent downloads) purely to be rejected
afterwards. The cap is now the reachable size, so the limit enforces itself while
the bytes are still streaming, and an overflow returns 413 with the actual
ceiling instead of a generic 500. Raising it properly means uploading in the
route and returning a file reference, as the WhatsApp media route does - not a
bigger constant.
Selector paging assumed a 0-based `from`. Zoho's docs contradict themselves:
the pagination section says "range 0-4999, default 0" while the listing examples
read as 1-based ("from=5 and limit=50 retrieves records 5 to 54"). Under the
1-based reading, stepping by exactly the page size re-fetches the boundary record
and the dropdown shows a duplicate per page. Rather than pick a base that cannot
be confirmed without a live tenant, the department and agent drains dedupe by id,
which is correct under either reading.
The organization list was unpaginated, and Zoho's listing APIs default to ten per
page. An account with more accessible portals silently got a truncated dropdown,
and since every other selector and every tool call is gated on orgId, a missing
portal was unreachable except through the advanced manual field. Both the
selector route and list_organizations now request the documented maximum.
Docs: regenerated so the trigger table includes manualOrgId, and two
service-account claims are hedged to match what the code already says it cannot
verify - that zsoid equals the Desk orgId header value, and that every tool works
under the requested scopes (Zoho publishes no scope for the attachment content
sub-path).
Also: status and priority move out of advanced mode - they are the fields most
often changed on a ticket update; the custom-fields wand prompt now ends with the
required "Return ONLY" clause; and the shared-orgId rationale comment cites the
mechanism that actually applies (buildCanonicalIndex dedupe plus the first-non-
empty rule in getCanonicalValues) rather than a blocks.test.ts branch that never
evaluates this pair.
* fix(zoho-desk): five-audit round - serializer trigger-advanced leak, scopes, paging
Five independent audits (OAuth/scopes, tools-vs-docs, block/selectors,
blast-radius, /validate-trigger). Findings, most severe first.
A trigger-mode field was a live tool-mode required param. `shouldSerializeSubBlock`
excluded `mode: 'trigger'` but not `'trigger-advanced'`, so the trigger's required
`manualOrgId` validated on every tool operation. Reproduced against the real
serializer: with the Organization field pinned to advanced, running
List Organizations failed with "Missing required fields: Organization ID" - a
field that operation does not even render, and which the user could not clear
without switching operations. Fixed in the serializer rather than locally,
because the Google Sheets/Drive/Calendar pollers have the identical shape.
`limit=200` on /organizations was an undocumented parameter I added by
extrapolating from /departments and /agents. Zoho documents NO parameters for
that endpoint and its sample is a bare GET; the other siblings cap at 100 and
Zoho answers out-of-range with 422. Since orgId gates every tool and both other
selectors, a 422 there would have made the whole integration unreachable. Reverted
to Zoho's documented shape.
`descriptionText` was HTML-stripping plain text. The previous round made the strip
unconditional after finding Zoho sends no `descriptionContentType`, but Zoho's REST
samples show plain descriptions while only the webhook payload is HTML - and the
webhook path runs this over contact/account/department bodies too. html-to-text is
not identity on plain text: it decodes entities and deletes tag-shaped content
("a < b > c", XML snippets). Now sniffs for markup first.
`omitUnset` made every documented field-clear impossible. Zoho's own PATCH sample
uses `"classification": ""` and `"productId": ""` to clear. Dropping `''` meant no
scalar field could be cleared. Now drops only undefined/null - the serializer-null
case it was written for - and forwards `''`.
status/priority leaked between operations. One shared subBlock served both the
list_tickets filter and the update_ticket value, and subBlock values survive an
operation switch, so a filter of "Open,On Hold" could be PATCHed onto a ticket and
an update value could silently filter a later list. Split per operation.
Auth: `invalid_code` added to TERMINAL_ERRORS - it is Zoho's code for a revoked
refresh token, so without it the previous round's refresh fix never actually
dead-flagged the credential it was written for. The shared refresh body-error
branch now also requires `!data.access_token`, so no provider can have a
successful refresh misclassified. The token route now uses the validating
`extractZohoDeskBaseFromScope` instead of a private regex with no https/allowlist
check - that value is injected into every tool call. Scope list falls back to the
requested scopes when Zoho omits `scope`, which would otherwise flag every
credential as needing reconnect. The Self Client mint no longer sends
`aaaserver.profile.READ`, a scope that grant never uses.
Trigger: `includePrevState` now set for every *_Update event, not just tickets -
it defaults to false, so prevState was permanently null for contact/agent/task/
article updates while the trigger advertised it. `departmentIds` is only sent for
events Zoho documents as accepting it, and the field is conditioned accordingly.
Empty filters serialize as `null`, matching Zoho's examples, rather than `{}`.
JWKS fetch bounded to 1.5s - jose's default is 5000ms, exactly Zoho's whole
delivery deadline, and Zoho publishes no retry. The create-time validation POST
fallback is now matched by the pending-verification probe. Ticket_Delete added.
All 17 webhook event ids, the POST /api/v1/webhooks body contract, and the JWT
claim/JWKS specifics are now confirmed verbatim against Zoho's webhook
documentation - previously 12 of 17 events and the entire subscription contract
were unverified.
* revert(zoho-desk): back out both shared lib/oauth changes
Reverting two changes to shared OAuth code because their premise is inferred
rather than proven, and neither meets the bar for touching a path every provider
runs.
`refreshOAuthToken` body-error branch. The premise was that Zoho reports refresh
failures with HTTP 200 and an `error` body. That is documented and empirically
confirmed for the authorization-code EXCHANGE (see the comment on getToken in
auth.ts), but I never confirmed it for the REFRESH grant specifically - and if
Zoho returns a proper 4xx there, the existing `!response.ok` path already
classifies it via extractErrorCode, making the branch dead code that every one
of the ~34 providers still executes on each refresh. A shared branch whose only
justification is an unverified inference about one provider is not worth its
blast radius.
`invalid_code` in TERMINAL_ERRORS. Same problem, worse downside: the code is
sourced from a Zoho community post rather than official docs, TERMINAL_ERRORS is
consulted for every provider, and a false positive marks a credential dead for an
hour. Not adding it simply preserves today's behavior (retry rather than
dead-flag), so reverting costs nothing that was previously working.
Both are cheap to reinstate, correctly scoped, once a live Zoho account shows
what a revoked refresh token actually returns.
Kept: the token-redaction on the "no access token" warn, which is an unambiguous
improvement independent of Zoho.
Also kept, deliberately, is the serializer `trigger-advanced` exclusion - that one
rests on a reproduced bug rather than an inference, and it aligns the serializer
with the convention the rest of the codebase already follows (blocks.test.ts
treats `trigger` and `trigger-advanced` identically in six places, as does the
copilot block-metadata tool, and blocks/types.ts documents trigger-advanced as
"the advanced side of a trigger field").
* fix(zoho-desk): carry the stored data center through a credential reconnect
A reconnect rebuilds the service-account secret blob from the submitted fields
only, and the connect modal never prefills - correctly, since for every other
field in this family the stored value is a secret the admin must retype. The
data center is the first non-secret member of that set, so it was being silently
dropped: rotating a client secret on an EU/IN/AU credential moved it back to the
US accounts server, where the next mint fails with an opaque invalid_client.
performUpdateCredential now reads the stored dataCenter out of the existing blob
when the caller does not supply one. The read is failure-tolerant - an
undecryptable or unparseable blob yields undefined rather than throwing, so it
can never block a reconnect, and the provider default applies as before.
Raised independently by three reviewers; I twice argued it was acceptable because
the mint fails loudly rather than corrupting silently. That was true and beside
the point - the operator still had to guess why.
* fix(zoho-desk): delta-audit findings - prevState scope, status leak, HTML sniffer
An audit of the commits the earlier five audits never saw. All four findings are
in code written as fixes for those audits, which is where this branch has
repeatedly introduced new problems.
`includePrevState` was sent for Ticket_Comment_Update. The previous commit gated
it on an `_Update` suffix and claimed Zoho supports it on every update event.
Zoho's webhook doc lists the attribute on Ticket/Contact/Agent/Task/Article update
events but NOT on Ticket_Comment_Update, which documents only `departmentIds`.
That made it an undocumented filter key on a live subscription create - the same
class of risk the same commit reverted `limit=200` for, so it failed that commit's
own stated bar. Now an explicit set rather than a suffix rule.
The status/priority split did not stop the leak it was written for. The mapping
used `operation === 'list_tickets' ? filterValue : updateValue`, whose bare else
covers all eight other operations - so a stale Update Ticket status was forwarded
into get_ticket, list_comments and the rest. Harmless on the wire (those tools
ignore it) but exactly the stale-value pattern the neighbouring gates exist to
prevent. Both fields are now scoped to the two operations that declare them.
The HTML sniffer destroyed plain text. `/<[a-z!\/][^>]*>/` fires on any `<`
followed by a letter with a later `>`, so realistic ticket bodies lost content:
"if x<y then z>0" became "if x0", and "replace <username> with the real name"
lost the placeholder. It now requires a real element - a paired tag, a
self-closing tag, a comment/doctype - or an entity, and the entity arm covers hex
references it previously missed. Regression tests verified by reverting to the
loose pattern and watching them go red.
The reconnect data-center carry-forward is scoped to client-credential providers.
As written it added a DB read plus a decrypt to every service-account reconnect
for every provider - Slack, Atlassian, all token-paste providers - to carry a
field only Zoho has.
Also: the JWKS cache-bound TSDoc had been orphaned onto the wrong constant by an
earlier insertion, and `cooldownDuration` was dropped since it restated jose's
default while only `timeoutDuration` needed justifying.
* test(zoho-desk): cover the webhook subscription filter rules
The subscription filter logic had no test coverage at all, and it is where the
last two rounds both found bugs - includePrevState on an event Zoho does not
document it for, and departmentIds sent to events that accept no filters.
Adds six cases against the real createSubscription: includePrevState is set for
each of the five documented update events and NOT for Ticket_Comment_Update,
departmentIds is kept for a filterable event and dropped for one that is not, and
an event with no filters serializes as null rather than an empty object.
Verified the guard bites: reverting PREV_STATE_EVENTS to the `endsWith('_Update')`
rule turns the Ticket_Comment_Update case red.
The Ticket_Comment_Update assertion checks the with-departments case as well as
the bare one - asserting only `not.toHaveProperty` on the bare filter would pass
vacuously, since that filter is legitimately null.
---------
Co-authored-by: Waleed Latif <walif6@gmail.com>
|
||
|
|
13772565a3 |
fix(chat): show deployment passwords to admins (#6177)
* fix(chat): show deployment passwords to admins
* fix(chat): reject whitespace-only passwords
* fix(chat): preserve password visibility on regenerate
* fix(chat): harden password reveal and close deployment lockout paths
Follow-ups from a security review of the password reveal endpoint. The
permission model itself was correct — the reveal is gated on workspace
admin via the canonical resolver, so derived org-admin access is honored.
These address secret handling and validation around it.
- Cap set-path passwords at the same 1024 chars the chat login accepts.
Neither the input nor the schema bounded length, so a longer password
saved fine and then failed the login POST on length before auth ran,
locking every visitor out permanently.
- Discard the revealed password when the field is hidden. It previously
stayed in state and in the input's DOM value with Copy still armed, so
the field read as hidden while still handing out the plaintext.
- Evict the decrypted password from the mutation cache on unmount, and
correct the TSDoc claiming it was never retained — it sat in the
MutationCache for the default five minutes after the modal closed.
- Validate the password inside performChatDeploy, the writer both callers
must use. The copilot deploy_chat tool bypasses the route contract and
could still store a whitespace-only or over-long password, or create a
password-protected chat with no password at all.
- Stop echoing raw decryption errors from the reveal endpoint.
- Only persist a new password when the chat ends up password-protected;
PATCH { authType: 'email', password } used to re-arm the secret that the
auth-type branch had just cleared.
Also replaces the hand-rolled copy state with useCopyToClipboard, which
fixes an unawaited clipboard write that surfaced as an unhandled rejection
and a "Copied" confirmation shown even when the write failed.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(chat): correct password-change confirmation gate and stale reveal error
Addresses both open Bugbot findings.
- shouldConfirmPasswordChange keyed on "a chat exists" rather than "a password
exists", so switching a public chat to password protection for the first time
asked the admin to confirm changing a password that was never set. It now
takes the existing-password signal the component already computes.
- A failed reveal left "Failed to load the current password" on screen while the
admin typed or generated a replacement, because the mutation only drops its
error on the next attempt. Editing or regenerating now resets it.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Waleed
|
03649e934c |
refactor(dev): remove the minimal-registry escape hatch (#6163)
* refactor(dev): remove the minimal-registry escape hatch `dev:minimal` existed because the tool registry was 71-82% of every workspace route's module graph and aliasing it away was the only way to make dev bearable. The metadata work removed that reason, so the hatch now buys almost nothing: before this stack 31.7s -> 20.0s cold (-37%) after this stack 22.9s -> 20.7s cold (-10%) A 10% cold-compile win, on the run that happens once — restarts are ~4.2s either way — is not worth what it costs. `tools/registry.minimal.ts` and `blocks/registry-maps.minimal.ts` are 283 lines of hand-curated duplicates of the real registries that **nothing keeps in sync** (no lint, no CI check, no test); they are correct today only because someone remembered. And the mode is actively misleading: it silently drops ~250 services and ~280 blocks, so anything reproduced under it may not reproduce for real. Removes both files, the `SIM_DEV_MINIMAL_REGISTRY` branch from `next.config.ts` (including the whole `webpack()` hook, which existed only for this), and the `dev:minimal` / `dev:full:minimal-registry` scripts. Verified after removal: `tsc` clean, boundary + metadata + skills + monorepo gates pass, and `next dev` starts and serves the canvas at 22.6s cold / HTTP 200. * fix(setup): stop the wizard offering the removed minimal-registry mode The setup wizard prompted for a dev server on machines under 16GB and **defaulted** to `dev:full:minimal-registry` — a script this stack deletes. Anyone running `bun run setup` on a low-RAM machine would have accepted the default and hit "Script not found", which is exactly the contributor the mode existed to help. Repointed at `dev:full:capped`, which still exists and caps Node at 4GB without dropping ~250 integrations — a strictly better answer to the same question. The hints were also stale: they warned the full registry "can use 4-5GB+ on its own", which was true when a dev server sat at 11.5GB. It now sits at ~4GB, so they say that instead. Missed by an earlier sweep because the pattern searched for `dev:minimal` and `registry.minimal`, and this string is `dev:full:minimal-registry` — the two halves reversed. Re-swept across every file type for all spellings: zero references remain. Also audited every script value the wizard can return, so the class of bug is checked, not just this instance. |
||
|
Waleed
|
e8894a8764 |
perf(tools): guard the tool-registry client boundary in CI (#6156)
* perf(tools): guard the tool-registry client boundary in CI
The registry was 71-82% of every workspace route's module graph, and the two
edges that put it there were invisible at the call site: `providers/utils.ts`
imported `mergeToolParameters`, and `mcp-dynamic-args.tsx` imported
`formatParameterLabel`. Neither import looks remotely like "pull in 4,700
modules of SDK clients", which is why this needs a lint rather than a convention.
`check-tool-registry-boundary.ts` walks the value-import graph (skipping
`import type`, which is erased) from the workspace layout and the four routes
that mount inside it, and fails if `@/tools/registry` is reachable — printing
the exact chain that reintroduced it.
Verified it fails: reintroducing a `getTool` import in `serializer/index.ts`
exits 1 and names the chain through `stores/workflow-diff/store.ts`; removing it
returns to 0.
There is deliberately no allowlist. The fix for a failure is always to move the
symbol the file actually needs into a registry-free module, not to exempt the
route.
Documents the guard in the tool-registry-boundary skill.
* fix(tools): close two edge-detection gaps in the registry boundary guard
Review found the walker missed two forms, both verified against a matrix of
every import/export shape:
export * as ns from '…' namespace re-export — the star branch had no alias
import('…') dynamic import
A dynamic import splits the registry into its own chunk rather than the route's
initial one, so it does not show up in cold-compile time — but it still puts
4,300 tools' worth of executable config on a client path, which is what this
guard exists to prevent. It counts as reaching the registry. No such import
exists today; this is purely closing the hole.
Adding both raised the measured counts (tables 1,217 -> 1,261, files
1,310 -> 1,419) because lazily-loaded modules are now counted. The registry
stays unreachable from all five entries.
Also checked and rejected: side-effect imports (`import '@/x'`) were reported as
missed, but are matched both standalone and after another import — the `from`
clause is already optional.
* fix(tools): resolve extensionful specifiers in the boundary guard
`resolveSpecifier` probed `base + ext` and `base/index + ext` but never `base`
itself, so an already-extensioned specifier resolved to null and its edge
vanished from the walk — `import { tools } from '@/tools/registry.ts'` would
have passed the guard silently.
Not theoretical: `executor/execution/block-executor.ts` already imports
`@/executor/human-in-the-loop/utils.ts` with the extension, so real edges were
being dropped. Counts rise slightly now that they are followed (canvas
2,023 -> 2,029).
Verified: the extensionful import exits 1, and removing it returns to 0.
* fix(tools): discover guard entries instead of listing them
Review caught the guard checking the wrong shell: it named
`app/workspace/layout.tsx` as "the shared shell every route mounts inside", but
that file only wraps `SocketProvider`. The real shell is
`app/workspace/[workspaceId]/layout.tsx`, which pulls in `WorkspaceChrome`, the
loaders and the providers — and it was never checked.
Worse, layouts are composed by Next.js convention rather than imported, so a
page's graph never reaches its layout at all. Walking pages alone left every
layout module outside the guard.
So entries are now discovered: every `page.tsx` and `layout.tsx` under
`app/workspace`, 35 of them instead of a hand-written 5. A list goes stale
silently; discovery cannot. Refuses to pass vacuously if the walk finds none.
Immediately found a real edge the hand-written list had missed — the settings
route reaching the registry through a dynamically-imported access-control panel
(fixed in the previous commit). Full walk takes ~2s.
Also restores the extensionful-specifier fix, which a bad merge had dropped from
this file. Re-verified both directions: an extensionful `@/tools/registry.ts`
import exits 1, removing it returns to 0.
* fix(tools): restore the dynamic-import and namespace-alias edge detection
A bad merge during a rebase reverted this file to a pre-fix revision, silently
dropping `DYNAMIC_IMPORT_RE` and the `export * as ns from` alias branch that
earlier commits on this branch had already added. The guard still passed, which
is the worst way for a lint to break — it simply stopped following edges.
Caught it because the per-route counts fell after the rebase (files
1,424 -> 1,314, logs 1,610 -> 1,545) rather than staying put. A guard that
reports fewer modules after a no-op merge is not passing, it is blind.
Now verified against every bypass form rather than the one I happened to think
of, so a future regression of this kind fails loudly:
CAUGHT extensionful import { tools } from '@/tools/registry.ts'
CAUGHT dynamic import('@/tools/registry')
CAUGHT ns re-export export * as ns from '@/tools/registry'
CAUGHT side-effect import '@/tools/registry'
CAUGHT plain named import { tools } from '@/tools/registry'
clean tree passes
* fix(tools): traverse require() edges in the boundary guard
Review flagged `require()` as an untraversed edge form, and it is not
hypothetical here — this codebase uses lazy `require('@/…')` to break import
cycles, including from a client-reachable file (`tools/params.ts` reaches
`@/blocks` that way). Those edges are as real as static imports; a `require` of
the registry would have walked straight past the guard.
The audit now covers every form a module can be reached by, each verified rather
than assumed:
CAUGHT plain named import { tools } from '@/tools/registry'
CAUGHT side-effect import '@/tools/registry'
CAUGHT extensionful import { tools } from '@/tools/registry.ts'
CAUGHT ns re-export export * as ns from '@/tools/registry'
CAUGHT dynamic import('@/tools/registry')
CAUGHT require require('@/tools/registry')
clean tree passes
No new violations surfaced — the 35 guarded page/layout graphs stay clean with
require edges followed.
|