mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-21 13:00:04 +08:00
892401a8d0ea661da3491cdb4c76a4b2dff4974b
734
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
892401a8d0 |
fix(uploads): sign Azure upload URLs create-only (#6607)
getBlobPresignedUploadUrl signed its SAS with BlobSASPermissions.parse('w').
Per Azure's service-SAS reference, `w` is "create or write content" and permits
overwriting an existing blob; `c` is "write a new blob" and does not. main
signed `c` before this signer moved out of core/storage-service.ts, so Azure
deployments lost create-only enforcement in the move.
The `If-None-Match: '*'` the signer returns cannot carry the guarantee on its
own: an Azure service-SAS string-to-sign covers the resource, times,
permissions and the five rsc* response-header overrides, never request headers,
so a client is free to drop it. The header is signed on the other two providers
-- inside the PutObjectCommand on S3, and as x-goog-if-generation-match in
signed extensionHeaders on GCS -- which is why only Azure regressed.
Without this, a signed upload URL stayed a plain overwrite grant on the final
key for its full hour. A caller could replace the object after complete had
already verified size and content type, written the workspace file row and
metered storage from that verified HEAD, leaving durable metadata and billing
describing content that no longer exists.
The multipart block-staging signer keeps `w`: block staging is overwrite-shaped
and matches main.
The existing test asserted parse('w'), so it locked the defect in; it now
asserts create-only, and its name states the guarantee so a future flip reads
as deleting a security property rather than adjusting a value.
|
||
|
|
a72457b9f5 | fix(docs): preserve items response fields (#6587) | ||
|
|
7c05e36049 |
fix(api): close five defects found auditing the v2 migration against main (#6575)
* fix(tables): stop a column retype from nulling empty-string cells A type conversion rewrote every cell holding '' to null. Main only nulled a blank the target type could not read; '' is a real stored value that both string and json columns accept, so string->json and json->string silently destroyed those cells. Worse on a required target: countEmptyCells matches only a missing key, SQL NULL, or '[]', so '' passes the required guard and the rewrite then wrote null behind a constraint that had just succeeded. The per-cell decision is now the pure retypeCellRewrite, restoring main's rule: null a blank only when the target cannot read it, otherwise coerce. * fix(execution): release the concurrency slot when a group cancel is refused The stop-the-work effects (durable Redis abort record, queue-job cancel, in-process abort) all fire before the workflow-group sidecar is consulted, and none can be undone. When the sidecar refuses the claim we throw a conflict, which skipped releaseExecutionSlot and stranded the plan concurrency reservation until it expired. Every conflict return is a terminal-or-absent state - a missing log row, a log already completed or errored, or a terminal cell - so a refusal never means the run is still executing. The slot is released before the throw, keeping the exact success && !isPausedCancellationPath predicate rather than a blanket finally that would free reservations for live runs. * fix(uploads): recover an ambiguous PUT instead of discarding the object Main recovered an upload whose bytes committed but whose response was lost, via a verify endpoint. The session client retries the PUT instead, but every provider now signs a create-only precondition, so the retry returns 409/412, is classified non-retryable, and the session aborts - deleting the object that had already landed. A transient blip on the final ack cost the whole upload. A conflict on a retry attempt is now treated as our own earlier PUT having committed, and completion proceeds. That is safe because completeUploadSession independently verifies the object through assertObjectIdentity, which rejects on uploadId mismatch before anything durable is registered. A first-attempt conflict still fails loudly. * fix(folders): enforce the workspace folder ceiling on the create path Readers bound the active path index at MAX_FOLDERS_PER_WORKSPACE and throw once a workspace exceeds it, but POST /api/folders reached createFolder, which has no maxFolderRows field and never counts. A workspace could therefore be driven past the ceiling, after which the 27 capped read sites failed on a state the product had allowed. createFolder now asserts room inside its transaction, right after the mutation lock, so the count cannot be raced. The refusal is a typed conflict rendering 409 with an actionable message rather than a 500. The check counts rows directly instead of loading the path index, so an already-over-cap workspace gets a clean refusal rather than a read error, and no reader gained a cap. folderMutationStatus also gained the payload_too_large mapping it was missing, which had been rendering a delete-cascade cap breach as an unexplained 500. * fix(skills): route internal skill writes through the shared use cases The internal route made the workspace authorization decision itself, never consulting the skills operation policy, never loading canonical workspace context, and recording an audit entry with no operation id or actor projection. v2 and Copilot already went through the use cases; only this surface did not. GET/POST/DELETE now authenticate, parse, call the shared use case, and present. Request and response shapes are unchanged. Two behavior changes fall out: a write against a deleted workspace is now refused with 404 rather than accepted, and permission-denial text matches the rest of the platform. Legacy internal-JWT auth is dropped because no principal kind expresses that caller and nothing calls it: the whole repo references /api/skills only in two comments, no tool declares an internalRoute to it, and the executor reads skills through a direct listSkills call rather than over HTTP. * fix(folders): enforce the workspace ceiling on the remaining create paths Folder duplication, admin workspace import, and workspace forking all inserted folders without consulting the ceiling that 27 read sites enforce, so any of them could leave a workspace whose reads then fail. Each now asserts room for the rows it is about to add rather than one at a time: duplication measures the whole subtree up front, forking counts its bulk insert, and import counts per segment because that is genuinely one row. assertFolderCollectionHasRoom gained an additionalRows notion for the bulk case, and short-circuits when nothing is being added so an over-cap workspace still reads and still syncs. Duplication deliberately does not take the folder mutation lock. Holding it across the copy would block folder creation workspace-wide for an unbounded time - there is no cap on workflows per subtree and duplicateWorkflow runs sequentially - and narrowing it is impossible because an advisory transaction lock cannot be released early; splitting the transaction would leave a half-copied tree on failure. A rare few-row overshoot near the ceiling is the better trade, and it matches what forking already does. A test asserts the lock is absent so re-adding it is a visible decision. Admin import gained the transaction and lock it never had. Its folder-full refusal escapes the per-workflow result list, because a full tree is a property of the workspace and would otherwise be buried as N failures behind a 200. The fork and promote routes had no catch at all, and withRouteHandler only classifies HttpError, so a refusal rendered as an opaque 500 - twice over, since drizzle wraps the throw. Both now project a classified conflict as 409 and rethrow anything unclassified. * fix(uploads): bound the signed PUT lifetime and advertise its real expiry A single-PUT transfer was signed for the whole 24h upload-session TTL, because expiresAt was reused as both the session lifetime and the signing lifetime. Multipart part URLs in the same file kept 1h, and the pre-migration presigned route signed every PUT for 1h, so the widening was unintended rather than a policy change. No provider clamps below 24h. The PUT presign is now clamped at the provider boundary by a shared UPLOAD_URL_TTL_MS, which the part-URL path also uses so the two cannot drift. An expired PUT URL is deliberately not recoverable: unlike multipart, which re-signs per part call because its progress is durable, a PUT is not resumable, so an expired URL and an interrupted PUT have identical recovery. Nothing leaks, since every provider signs a create-only precondition. Clamping alone would have made the contract lie: the URL would die an hour before the session's advertised expiresAt, with nothing telling an integrator why the 403 happened. The PUT transfer now carries its own expiresAt, mirroring the multipart part-URL field. It is provider-dependent on purpose - cloud transfers report the clamped signature expiry, while the local data plane has no signature and admits against the session, so reporting an hour there would have been a new inaccuracy in the other direction. * chore: delete the dead presigned-upload and skill-adapter paths The presigned upload routes and the internal skills adapters were both replaced during the v2 migration, leaving their implementations behind with no callers. Removed generatePresignedUploadUrl and verifyPresignedUploadReceipt with their three provider helpers, QUOTA_EXEMPT_STORAGE_CONTEXTS and the types it orphaned, and the performCreateSkill/performUpdateSkill/performDeleteSkill adapters with recordSkillEvent and statusForSkillOrchestrationError. Each was verified unreachable across apps, packages, scripts and ee, including barrel re-exports and string access, not just direct imports. recordSkillEvent needed the closest look, since deleting an audit writer can silently drop coverage. The use cases declare the same action, resource, and description, and the framework adds the operation and actor the old helper lacked; recordAudit back-fills actorName and actorEmail from the user table when both are omitted, so the one field the helper passed is not lost. The self-hosting architecture doc described a directUploadSupported flag on an endpoint that no longer exists, and now describes the upload-session flow that replaced it. * refactor(folders): keep the cheap resource facts out of the schema graph Reading a folder resource type's label or its lock support meant importing folderResourceConfig, which imports the db schema for every table it serves and from there reaches lib/table/service, the executor, and the tool registry. That mattered as soon as lib/folders/queries needed a label: queries is reached from workspace-file-manager, which is reached from the files and chat pages, so one import edge put roughly 4,700 modules into those page graphs and broke the tool-registry boundary audit. Labels and lock support now live in a leaf module that imports only a type, and config composes them so there is still one source of truth. The three folder routes that pulled the whole config in for a single boolean read the leaf instead. * fix(skills): apply an upsert batch in one transaction The internal skills route looped the batch, calling an independently committing use case per item. A rejection on a later item left the earlier ones written and audited while the request reported failure - the compound-mutation rule in CLAUDE.md exists for exactly this. No new transaction plumbing was needed: upsertSkills already wraps its whole item loop in one db.transaction, so the partial commit came from calling it N times rather than once. upsertSkillBatch now validates and per-skill authorizes every item before issuing a single write, and createSkill and updateSkill became thin wrappers over it so v2 and Copilot keep one authority for the rules. The compound operation declares the read floor that skills.update already used, and the use case additionally authorizes skills.create when any item lacks an id, still ahead of every write. A read-only member who is a skill editor keeps their edit, and creates are not authorized more loosely than before. Audit projects one entry per committed skill, and analytics moved after the commit so nothing is reported for a rolled-back item. Note metadata.operation for these writes is now skills.upsert rather than skills.create/update; the action field still carries the distinction. * fix(security): close two disclosure gaps and finish the slot-leak fix The payer-pool gate only covered the workspace branch. A personal API key that omits workspaceId takes the account branch, where getHighestPrioritySubscription resolves an organization subscription from any member row regardless of role - so a plain member read the organization-wide credit and storage pool by dropping one query parameter. The account branch is now gated by the same authority, and the storage pool is not queried when it may not be disclosed. Forcing that branch self-scoped instead would have downgraded plan, period, and status, which is what a member needs to see whether the org is blocked. GET /api/v1/logs/executions/[executionId] emitted the workflow snapshot raw, carrying password sub-block values and oauth-input credential ids. It now shares the sanitizer the v2 read already used, extracted so there is one implementation rather than two. Env-var references are still preserved. cancelWorkflowGroupExecution itself was unguarded, so an unexpected throw from its transaction escaped ahead of every release site - the same reservation leak this branch set out to close, still open on the adjacent path. It now releases through the shared predicate and rethrows, because a failed transition means the cell state is unknown and a success-shaped answer would be a lie. The comment claiming the abort record cannot be taken back was false and now states the real reason: a refusal is always a terminal-or-absent state. * fix(v2-api): cover every persisted run status and every capped body The workflow-runs endpoints carried the same omission the logs contract had: the execution logger persists redacting, the run schemas did not list it, and because validation is whole-response one such row returned 500 for an entire page. Both schemas now derive from PersistedWorkflowExecutionStatus behind the same AssertNever gate, so a future status is a type error rather than a production 500. The single-run read keeps its extra queued value, which only it can observe. That schema was also serving as the run-list status filter. Widening it would have accepted a filter value the application input cannot express, so the reported set and the accepted filter are now separate schemas. Routes declaring maxBodyBytes without payloadTooLargeResponse fell back to a bare string with no error code and no private cache header. Rather than patch the four, the default moved into the builders - all three had the hole - which covers 58 body-bearing handlers, and a route override still wins. The five per-route overrides that merely restated the default are gone. Also documents the 413 on the one knowledge route that has a real body cap, adds the rollout gate's 404 to the last v2 operation missing it, and rewords the nextCursor description, which read as though every list were a full-set list. * fix(api): make the shared traits and lifetimes single-sourced The folder resource-traits leaf composed labels into config but restated lock support independently, so the routes reading the trait and the orchestration reading the config could disagree about which resources lock. Config now composes both, and supportsLocking is required rather than optional so a new resource type cannot silently omit it. The upload commit claimed the PUT clamp and the multipart part URLs shared a constant and could not drift. That was true only of the advertised expiry - the three provider signers each hardcoded an hour, so changing the constant would have moved what we advertise while leaving what we sign, recreating exactly the mismatch the clamp removed. Each provider now receives the lifetime in its own unit from the one constant. Table restore hand-rolled its status map and returned the driver message verbatim at 500, leaking the failed statement and its bound parameters - the same defect this branch closed at nine other sites. It and import-csv now use the shared projection; import-csv's result type also had to carry the lock the classifier already set, so a 423 can name it. Deletes v2RowWriteError, which had no callers and would have rendered a locked table as 400 by discarding the 423 it was handed. |
||
|
|
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.
|
||
|
|
81e04a8e41 |
fix(agiloft): align the integration with the documented ewws REST interface (#6556)
* fix(agiloft): align the integration with the documented ewws REST interface
The CRUD tools targeted /ewws/REST/{kb}/{table}/{id} with JSON bodies and
guessed at the response by probing `data.result ?? data` and `id ?? ID`.
Agiloft documents that path as a URL convention only -- no method table, no
example call, and no response shape -- and no known client uses it. The EW*
operation family is specified end to end, including exact response bodies, so
every operation now goes through it and parses the documented
`EWREST_key='value';` assignment format.
- EWCreate/EWRead/EWUpdate/EWDelete/EWSearch/EWSelect/EWGetChoiceLineId are
form-encoded and parsed via a shared EWREST parser; the /.json suffix is kept
only on EWAttachInfo, the one operation with a published JSON sample
- EWDelete now sends the deleteRule the docs require, defaulting to
ERROR_IF_DEPENDANTS so a delete fails rather than cascading
- EWRemoveAttachment uses GET; it does not accept DELETE
- EWSearch accepts the documented `search` saved-search label, so saved
searches are reachable for the first time
- Search query help taught AND/OR; Agiloft uses && and ||
- Add run_action_button (POST /ewws/async/EWActionButton) for approvals and
send-for-signature steps
- Drop saved_search: EWSavedSearch has no doc page, so neither its URL nor its
response could be verified and it could only ever return an empty list
- Add force on unlock, filter read fields locally since $fields is
undocumented, correct lock status to LOCKED/NO_LOCK, and stop reporting a
fabricated page size of 25
* fix(agiloft): fail loudly on non-EWREST bodies and keep the retired tool resolvable
- EWSearch and EWSelect report an empty result set as `EWREST_id_length = '0';`,
so a body with no assignments at all is a refusal Agiloft returned with HTTP
200, not an empty result. Both routes now surface it as an error instead of a
successful empty list.
- Re-register agiloft_saved_search as a retired tool. Removing it outright left
workflows saved with operation='saved_search' deriving a tool id the registry
no longer provided, which throws "Tool not found" at execution. It now fails
through directExecution with a message pointing at the Search Records
operation's Saved Search field, without issuing an undocumented request. It
stays out of the operation dropdown so it cannot be chosen for new blocks.
- Guard EWCreate and EWUpdate against oversized record data. Those operations
carry field values in the query string, so a large payload hits the request
line limit; the tool now explains that rather than surfacing an opaque 414.
|
||
|
|
c365b14f73 |
feat(calendly): extend tools with booking, availability, no-shows, and routing forms (#6545)
* feat(calendly): extend tools with booking, availability, no-shows, and routing forms Adds 12 tools verified against the Calendly OpenAPI spec: get_user, get_event_invitee, create_event_invitee, list_event_type_available_times, list_user_busy_times, list_user_availability_schedules, create_scheduling_link, create/delete_invitee_no_show, list_organization_memberships, list_routing_forms, and list_routing_form_submissions. Also fixes issues found while validating the existing tools: - list_webhooks dropped the scope query param the API requires, so every call with scope unset returned 400 - list_event_types could only send active=true, making inactive event types unlistable - user and organization filters now accept a bare UUID or a full URI consistently across every operation - json array params (eventGuests, events) are normalized whether they arrive as an array or a JSON string * improvement(calendly): type the block/tool alignment test instead of using any * fix(calendly): normalize webhook organization and user identifiers |
||
|
|
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> |
||
|
|
a64ce49af9 |
feat(incidentio): add on-call, alert, catalog, and team tools (#6529)
* feat(incidentio): add on-call, alert, catalog, and team tools
Adds 19 tools to the incident.io block, taking it from 46 to 65
operations. Every endpoint, param, and response field is taken from the
official OpenAPI spec at api.incident.io/v1/openapiV3.json.
Who is on call had no reachable answer before: the data lives in
ScheduleV2.current_shifts, and both schedules_list and schedules_show
returned it but never declared it. The new incidentio_on_call_now tool
flattens current and upcoming shifts to one row per person, and the two
existing schedule tools now declare the fields they were already
returning.
Also fixes two pre-existing wiring bugs: the block declared an output
named schedule_override while the tool emits override, and the on-call
handoff skill described a lookup the integration could not perform.
* fix(incidentio): stop the alert filter sentinel reaching the API
The has_notes and include_maintenance_window dropdowns default to the
string "any", meaning "do not filter". The params transform skipped the
key in that case, but the executor merges its output over the raw inputs
(`{ ...inputs, ...transformedParams }`), so the sentinel survived and the
tool sent has_notes[is]=any, which incident.io rejects.
The transform now always assigns the key, mapping "any" to undefined so
it overwrites the sentinel instead of leaving it in place. The tool also
only serializes these filters when it actually has a boolean.
Adds tests covering the sentinel, both real boolean values, and the
documented bracket-operator filter syntax.
|
||
|
|
5478a690cc | improvement(setup): complete knowledge and update flows (#6521) | ||
|
|
daac4f38d4 |
docs(salesforce): correct the setup steps that would strand an admin (#6526)
* docs(salesforce): correct the setup steps that would strand an admin Verified the guide against Salesforce's current UI and docs. Most of it holds; these do not: - The Client Credentials step told admins to check "Enable Client Credentials Flow" under OAuth Policies. On an External Client App that checkbox is under Edit Settings → OAuth Settings; the Policies page holds only the Run As picker, so anyone following it literally hunts for a control that is not on the screen. The FAQ answer inherited the same conflation. - Pre-authorizing the app must go through the profile or a SECOND permission set. A permission set backed by the Salesforce API Integration license cannot hold an Assigned Connected Apps section at all, so the app can never be assigned from the same permission set that grants object access — which produces exactly the "user hasn't approved this consumer" failure that step exists to prevent. This is the likeliest way a JWT setup fails. - Salesforce requires an RSA key of at least 2048 bits; an ECDSA key is silently rejected, and the certificate must stay under 4 KB. - The JWT toggle does not appear until Enable OAuth is on, and the control is "Upload Files". Also scopes the capability promise for the API-only license: SOQL and CRUD on standard objects are supported, reports and dashboards are genuinely unverified in either direction, and Apex Class Access is a permission this license cannot hold, so Tooling API calls touching ApexClass will fail. * docs(salesforce): align the Developer Edition host in the FAQ with the setup section The setup section was corrected to make the `-dev-ed` suffix conditional, but the FAQ still presented it as mandatory — so an admin whose Developer Edition domain lacks the generated suffix would read two contradictory formats on the same page and validate against a host that does not exist. |
||
|
|
ba5dfb1f89 |
feat(salesforce): add JWT bearer flow and sandbox OAuth support (#6508)
* feat(salesforce): add JWT bearer flow and sandbox OAuth support Salesforce integration users could only authenticate through interactive OAuth, which an API-only integration user cannot complete — there is no UI for them to log in to. Adds the JWT Bearer Flow as a second grant on the existing service-account provider, and registers sandbox as its own authorization server so sandbox orgs can connect at all. The assertion is audienced at the org's My Domain URL rather than login/test.salesforce.com: Salesforce ended legacy hostname redirections in Spring '26 and External Client Apps now reject the generic sandbox host with app_not_found. My Domain is valid for Connected Apps and External Client Apps, production and sandbox alike, and is what the Salesforce CLI recommends — so the stored host alone determines the environment. Sandbox credentials are stored under their own provider id, mapped back to the one Salesforce service via additionalProviderIds on OAuthServiceConfig. That is threaded through every resolution point, including the two SQL filters that would otherwise have hidden sandbox credentials from the block picker entirely. Also fixes three latent bugs surfaced along the way: Zoom interpolated an undefined client secret into its Basic auth header, sandbox refresh tokens would have been posted to the production endpoint, and the sandbox connector would have been silently dropped as unconfigured. * fix(salesforce): canonicalize connected provider ids in the copilot credential tool A credential stored under an alternate authorization server was recorded in `connectedProviderIds` under its own id, while the not-connected list compares against the service's canonical id — so a sandbox-only Salesforce user was reported as both connected and not connected. Record the canonical id instead. Also types the JWT test's assertion decoder instead of returning `any`. * fix(salesforce): close reconnect, instance-URL, and key-handling gaps An independent audit swarm found four real defects in the JWT bearer work: - The credential update hook rebuilt its request body from a hand-written allowlist, so `authMethod`, `privateKey`, and `username` were silently dropped. A JWT private key could never be rotated through the UI, and switching grants failed with a generic error. Forwards the whole contract body instead, so a field added to the contract later cannot be lost again. - `getInstanceUrl` guarded only the `sub` claim against login-host origins, so a sandbox id token whose `profile` was rooted at test.salesforce.com yielded the login host as the org's API base. Both claims are now guarded, and a guarded-away `profile` falls through to `sub` instead of ending the lookup. - `canonicalizeServiceProviderId` replaces the previous fold, which also matched family-wide service-account ids and so dropped one arbitrary sibling product (Gmail, Confluence) from the copilot's not-connected list. - The private key was collected in a plain textarea, leaving browser spell check and autofill free to ship it to third parties. Also restores the explicit https check on the userinfo-derived instance URL, anchors the scope marker, caps the accepted RSA modulus, and stops single-grant providers paying for a stored-blob decrypt on every reconnect. Docs: the JWT path no longer tells readers to enable the Client Credentials Flow, and calls out the my.salesforce-setup.com host as the likely wrong paste. Adds coverage for the paths the audit proved untested: partitionClientCredentialFields, credentialProviderMatchesService's alternate-server clause, reconnect carry-forward, the typographic-apostrophe error branch, and the passphrase hint. * fix(salesforce): handle the Government Cloud JWT audience and unassigned-profile errors Verification against Salesforce's own sfdx-core surfaced two gaps: - `gs1` Government Cloud orgs have ordinary *.my.salesforce.com hosts, but Salesforce requires `https://gs1.salesforce.com` as the JWT audience. The host regex accepted them, so they would have failed with an opaque audience error. The token still posts to the org's own host; only `aud` differs. - `invalid_app_access` — Permitted Users is set to admin-pre-authorized but the run-as user's profile was never assigned to the app — is the likeliest misconfiguration and had no hint at all. Also sends `iat`, matching sfdx-core and every mainstream implementation, and softens two TSDoc claims that were stronger than the evidence: Salesforce does not hard-reject a far-future `exp` (its own CLI ships one), and My Domain is the right audience for commercial orgs rather than universally. * fix(salesforce): match sandbox credentials in Chat and the connect draft Two more surfaces resolved a credential to its service by exact provider id: - `credentialsForTarget` compared only `providerId`/`baseProviderId`, so a sandbox-only user's Salesforce chip in Chat read as disconnected and re-prompted them to connect. The alternate ids are passed in by the caller rather than resolved in the module, which is `'use client'` and would otherwise pull the OAuth provider registry into the chat bundle. - `createConnectDraft` resolved the service name by exact id, so a sandbox connect defaulted to the label "My salesforce-sandbox". * fix(salesforce): carry alternate provider ids through chat connect verification The chip's live target was widened to match a sandbox credential, but the post-connect verification leg re-reads the STORED attempt, which did not carry the ids — so completing a sandbox connect from Chat was detected as a failure and the chip was marked failed. The attempt now persists them; attempts written before this simply match as they did, and they expire within 15 minutes. Also marks the auth-method picker required while it is the field blocking submit on a reconnect, so the greyed button has a visible cause. * fix(salesforce): send reauthorize to the server that issued the credential "Update access" derived its provider from the service id, which always yields the primary authorization server. A sandbox credential missing a scope sent the user to login.salesforce.com — where a sandbox-only user cannot sign in at all, and where a user who can sign in creates an orphan production account while the banner never clears. Both credential selectors now pass the selected credential's own provider id, which the connect modal already honours. Also names the alternate provider ids explicitly in the disconnect sweep. That branch is unreachable today (every caller sends an accountId), but it was catching them only by the `{base}-` prefix accident. * fix(salesforce): make the Government Cloud audience check exact, not a prefix `startsWith('gs1-')` was invented from a paraphrase of sfdx-core and would have misrouted an ordinary org like gs1-widgets.my.salesforce.com to the GovCloud audience — breaking a setup that works today. sfdx-core's host signal is the literal gs1.my.salesforce.com; its other signal is the org's createdOrgInstance, which we never see. Matching exactly means a miss falls back to My Domain, which is the behaviour before the branch existed, while a false positive cannot happen. Also replaces the hand-rolled origin regex in getInstanceUrl with URL parsing, which normalizes userinfo, ports, and case before the login-host comparison, and drops two error hints that had no evidence behind them. |
||
|
|
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 |
||
|
|
156ee3ebf2 | fix(provenance): feature flagged, inexact sidecars (#6491) | ||
|
|
4b2412b752 |
fix(auth): stop offering account creation when registration is disabled (#6484)
DISABLE_REGISTRATION blocks /signup server-side, but the invite flow, the login form, the SSO form, and the CLI handoff all kept routing people there, stranding invited users on a dead end. The flag also never covered OAuth account creation, so social sign-in still minted accounts for unknown identities. |
||
|
|
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
|
||
|
|
e6485f522d |
fix(dynatrace): send the only unmute reason the API accepts and request the detail fields the tools map (#6463)
Unmute forwarded the shared muteReason dropdown's FALSE_POSITIVE default, but Dynatrace accepts exactly one unmute reason, AFFECTED. The tool-level fallback never fired because a truthy invalid reason was already supplied, so unmute failed from the block unless the reason was changed by hand. The vulnerability, problem, and attack detail endpoints omit every optional property unless it is named in `fields`, so the descriptions, remediation guidance, affected entities, root-cause evidence, and attacker details those tools map were always null. |
||
|
|
76b535f676 |
chore(snowflake): drop the local write caps in favor of the shared limit (#6461)
The 1000-row and 1 MB bound-value caps duplicated the shared 10MB tool request
body gate on the same axis, at a lower threshold. Nothing fails between the two
thresholds: Snowflake's 1 MB guidance is a recommendation about metadata
retention rather than a hard limit, and statements above it still execute. The
row array is already resident before the statement is built, so the caps also
bounded no allocation the request body did not already bound.
These were removed once before in
|
||
|
|
3096de846e |
feat(integrations): add Snowflake PAT integration (#6407)
* feat(integrations): add Snowflake PAT integration * fix(snowflake): scope block params by operation * refactor(integrations): simplify Snowflake safeguards * refactor(snowflake): isolate statement capabilities * fix(snowflake): localize required user agent * chore(snowflake): limit changes to integration scope * fix(snowflake): correct SQL generation, transport, and param conventions Address defects found by validation against the Snowflake SQL API v2 and SQL reference docs. SQL generation: - lift PARSE_JSON out of the VALUES clause into a projecting SELECT; the previous form is rejected for any object or array value - escape backslashes as well as quotes in string literals, closing a COPY option injection through the user-or-llm stagePath and pattern fields - reject "--" in stage paths, which commented out every following clause - emit COPY INTO clauses in the documented positional order - exclude only view types in introspect_schema so temporary, external, and event tables are visible - use plain equality in MERGE and reject null or duplicate match keys - bound rows and bound-value bytes for every statement, measured in UTF-8 - reject qualified task names, which TASK_HISTORY silently ignores - replace a raw NUL byte in the source with its escape sequence Transport: - read DML stats from the documented top-level ResultSet property - drop Link-header and 391908 paging, which belong to the retired API, and report partition completeness as unknown rather than falsely complete - require a 2xx status before trusting a success SQLSTATE - cap response bodies and fail closed on invalid session context names Conventions: - inline shared params into each tool instead of cross-file spreads, which also lets the docs generator emit host and apiKey - use the official Snowflake brand mark on a white tile * fix(snowflake): emit task history time bounds as literals TASK_HISTORY only accepts bind variables for RESULT_LIMIT and TASK_NAME per BCR-1410, and that change explicitly excludes a bind passed through another function first. A bind in SCHEDULED_TIME_RANGE_START/END is therefore dropped without an error, so the requested window became a no-op and the function fell back to returning the most recent runs. Emit validated literals instead, which also restores Snowflake's seven-day range error. Also reject a fractional skip-file percentage at the block boundary rather than in the builder, and correct the cancel description: a cancelled child marks the task graph run failed, so downstream tasks are skipped rather than continuing. --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain> Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com> |
||
|
|
56200177e6 |
feat(mintlify): add Mintlify integration (#6457)
* feat(mintlify): add Mintlify integration Adds all 18 documented Mintlify REST API endpoints across deployment, automation, agent jobs, prose detection, docs search/assistant, and analytics export. * refactor(mintlify): tighten payload typing and test import |
||
|
|
d317607d2c |
feat(dynatrace): add the write and configuration surfaces (#6398)
* feat(dynatrace): add the write and configuration surfaces Takes the block from 22 operations to 47. The original PR shipped the read paths plus a few ingests; this closes the gaps that made those reads dead-end. The one that was a real defect: security was read-only. The audit-vulnerabilities skill promised "a remediation queue" and then gave you no way to act on it, even though muting is the single most common triage action. Adds mute and unmute, singly and in bulk, plus the remediation items behind a third-party finding, plus the Attacks API so an exploited vulnerability can be traced to the request that exploited it. The rest, by how much they unblock: - Custom tags (read/add/delete). Entity tags already drive every selector in the block; being able to write them closes a loop that was half open. - Settings objects (schemas, list, get, create, update, delete). This is how maintenance windows, alerting profiles, and management zones are configured in modern Dynatrace, so "open a maintenance window before the deploy" was simply unreachable before. The value is a schema-defined blob, so the tool is honestly opaque rather than falsely typed; the docs tell you to mirror an existing object. Update and delete carry the updateToken so a concurrent change fails instead of being overwritten. - Synthetic monitors and on-demand batch execution, which pairs with the deploy-marker tool to gate a release on a smoke test. - Problem comment get/update/delete, and SLO create/update/delete, completing CRUD that was previously half-built. Two structural notes. Synthetic monitors are the only endpoints still on Environment API v1, so `buildDynatraceUrl` grew a v1 sibling and the shared base-URL normalizer now strips either version; the query builder also learned to repeat a param per value, which Synthetic's `tag` needs. And creating an SLO returns 201 with an empty body and the new ID in the Location header, so that tool reads the header rather than parsing nothing. Deliberately excluded: the Grail/DQL query API. It is the long-term successor to the deprecated logs/search endpoint, but it authenticates with a platform token rather than an Api-Token, so it is a second auth path and belongs in its own change. * fix(dynatrace): drill the documented JSON shapes, and require the tag selector Two problems, one found in review and one worth more than it was given. The tag operations could run without an entity selector. All three tag tools declare `entitySelector` required, but the shared block field was only marked required for List Entities, so the block let a workflow reach those tools with an invalid configuration and let Dynatrace do the rejecting. My own structural auditor missed it because it only checked that *some* visible subBlock existed for a required param, not that the specific one was required — that check is now precise, and it confirms these three were the only instances across all 47 operations. The larger one: outputs were declaring `type: 'json'` for shapes the API reference documents in full. Thirty-five of them. The top-level entities were mapped properly, but nested payloads — a problem's evidence and impact analysis, a vulnerability's risk assessment and global counts, an attack's attacker, request, entry point and exploited vulnerability, a remediation item's assessment and mute state, the synthetic execution and failure records, the metric ingest error envelope, the DQL translation — were passed through as anonymous blobs. A downstream block could not reference `attacker.sourceIp` without knowing to guess it. All of those now carry their fields. What stays opaque is now only what genuinely is, and each says why in its description: a settings object's schema-defined value, an entity's type-dependent property bag and relationship keys, caller-supplied synthetic metadata, an audit log's JSON patch, the undocumented partial-success body of log ingestion, and the handful of security-detail shapes the reference names without expanding. * fix(dynatrace): make the synthetic enabled filter tri-state Review catch. `enabled` on List Synthetic Monitors is a three-way filter — enabled, disabled, or either — and I had it as a switch. The URL builder deliberately serializes `false` (there is a test pinning that `evaluate=false` survives), so leaving "Enabled Only" unchecked sent `enabled=false` and returned only the disabled monitors: exactly backwards. Made it a dropdown with Any / Enabled only / Disabled only, matching the monitorType field directly above it, which had the same shape and already used an empty-id "Any" option. The params mapper sends nothing for "Any". Checked the other nine switches rather than assuming. None share the bug: for each of them off genuinely means false, and false is Dynatrace's own default, so serializing it is correct. A test now pins that list so the trap cannot be re-introduced by converting one of them, alongside a test covering all three states of the filter. |
||
|
|
2e7e5ae804 |
feat(dynatrace): add the Dynatrace integration (#6393)
* feat(dynatrace): add the Dynatrace integration
Adds a Dynatrace block backed by 22 Environment API v2 tools, covering the
surfaces an observability workflow actually reaches for:
- Problems: list, get, close, list comments, add comment
- Metrics: query data points, list and get descriptors, ingest line protocol
- Entities: list, get, list entity types
- Events: list, get, ingest
- Logs: search, ingest
- SLOs: list, get
- Application Security: list and get security problems
- Audit log: read
Every request path, query parameter, and response mapping is taken from the
published Dynatrace API reference — no inferred fields. Auth is an access
token sent as `Authorization: Api-Token ...` against a user-supplied
environment URL, so SaaS, Managed, and environment ActiveGate all work.
Two details worth knowing:
`ingest_event` exposes Dynatrace's event timeout as `eventTimeout`, not
`timeout`. The tool transport reserves `params.timeout` for the HTTP request
deadline, so the obvious name would have silently retargeted the wrong knob.
`get_metric` encodes its path segment with `encodeDynatracePathSegment`
rather than `encodeURIComponent`, which leaves the `:` separators in metric
keys and transformation operators intact, matching the docs' own examples.
* fix(dynatrace): close the gaps a validation pass turned up
Three real defects and one usability gap, all found by auditing the tools
against the Dynatrace API reference a second time.
`ingest_logs` double-encoded its payload. `logs` is a `json` param, and a
`json` param arrives as a *string* whenever it comes from a long-input field
or an LLM tool call — only a block-to-block reference hands over a parsed
value. `JSON.stringify` on that string produced `"[{...}]"`, so Dynatrace
received a quoted string where it expected an array. The block hid this in
the UI path by pre-parsing, but the parse lived in `tools.config.params` and
*threw* on malformed input, and it never covered the direct tool-call path at
all. Both tools now normalize through the shared `parseJsonParam`, so the
tool is correct regardless of who calls it, and the block just forwards the
raw value. `ingest_event.properties` had the identical bug.
Path identifiers were not trimmed. A problem or entity ID pasted with a
trailing newline became `%0A` in the URL and 404'd with nothing to suggest
whitespace was the cause.
Errors dropped the part that matters. Dynatrace's ErrorEnvelope carries
`constraintViolations[]`, which names the offending selector or parameter;
the generic `nested-error-object` extractor returns only `error.message`
("Constraints violated."), and which extractor won was left to fallback
order. Adds a `dynatrace-errors` extractor that folds the violations into the
message and pins it on all 22 tools. It sits after `nested-error-object` in
the chain, which already matches this shape, so no other service's error
handling changes.
Adds 21 tests covering URL construction for SaaS/Managed/ActiveGate, cursor
pagination dropping sibling filters, identifier trimming, metric-key colon
preservation, both JSON-param paths, the `eventTimeout` -> `timeout` mapping,
EntityStub flattening, the audit log's dotted `dt.settings.*` keys, and the
204/200 split on log ingestion.
* docs(dynatrace): add the page intro, and pin every response key in tests
Adds a MANUAL-CONTENT:intro block to the generated integration page covering
what the block reaches, how to get an environment URL and a scoped token for
SaaS vs Managed, how selectors work, and how cursor pagination behaves.
Verified it survives `generate-docs.ts` byte-identically.
Also closes the last silent-failure gap the validation pass left open. A
wrong top-level response key does not throw — it maps to an empty array and
reads as "no results", which is indistinguishable from a genuinely empty
environment. Dynatrace is unusually easy to get wrong here: the SLO list
returns `slo` (singular) and the metric query returns `result` (singular).
Adds a table-driven test asserting the documented key for all ten list
endpoints plus the scalar keys of the ingest and single-entity responses.
Confirmed it bites by flipping `data.slo` to `data.slos` and watching only
that row fail.
* chore(dynatrace): type the shared param map as unknown
Review follow-up. `Record<string, any>` in the block's params builder dropped
compile-time checking from every operation's shared params; `unknown` is
enough here since the values flow straight into the tool param maps. Matches
.claude/rules/sim-typescript.md, which sibling blocks (Datadog, Grafana)
still violate.
* fix(dynatrace): stop three silent failures found in a final read-through
All three turn a failed call into something that looks like a successful
empty one, which is the worst shape for an observability integration — you
cannot tell "nothing is wrong" from "the call did not work".
`readJsonBody` swallowed any unparseable body and returned `{}`. A gateway
HTML page, a captive-portal interstitial, or a truncated payload therefore
mapped every field to null and read as "no problems found". Only genuinely
empty bodies are tolerated now (201 from add-comment, 204 from log ingest);
anything else that will not parse raises with a truncated preview.
`ingest_logs` sent `[]` when the payload was missing or empty. Dynatrace
answers 204 to that, so the tool reported `accepted: true` for a call that
shipped no logs. It now fails loudly instead.
`encodeDynatracePathSegment` percent-encoded the whole metric key and then
regex-unescaped `%3A` back to `:`. Same output, but it undoes the encoder's
work and hides the intent. Colons are structural in a metric key, so it now
splits on them, encodes each part, and rejoins — which says that directly.
Each fix has a test, and each test was confirmed to fail in isolation with
only its own fix reverted.
|
||
|
|
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.
|
||
|
|
aae9ce62e7 |
feat(smartlead): add Smartlead integration (#6352)
* feat(smartlead): add Smartlead integration
Adds a Smartlead block with 22 tools covering campaigns, sequences, leads,
analytics, and webhooks.
Every request path, parameter, enum, and response mapping was verified against
the live Smartlead API rather than its documentation, which proved unreliable:
- `POST /campaigns/new` (documented) 404s; the real path is `/campaigns/create`
- `GET /campaigns/{id}` and `/sequences` return bare payloads, not the
documented `{success, data}` envelopes
- `/statistics` returns paginated per-email rows, not the documented aggregate
- `POST /campaigns/{id}/leads` returns import counters under entirely
different field names than documented
- documented `/leads/{id}`, `/top-level-analytics`, `/all-leads-activities`,
`/lead-lists/`, and `/lead-tags/` all 404
Enum values (campaign status, track settings, stop-lead settings, webhook event
types, engagement status) were probed value-by-value against the API.
Notes on the API's shape, encoded in the mappers:
- string-encoded numbers (`total_leads: "1"`, `sent_count: "0"`) are normalized
to numbers so a field never changes type between operations
- `seq_delay_details` is read as `delayInDays` but written as `delay_in_days`
- webhook writes echo `event_type_map`/`category_id_map` objects while the list
endpoint returns `event_types`/`categories` arrays; both map to arrays
- `track_settings` reads back in a vocabulary it will not accept on write
Statistics rows and lead message-history entries pass through unmapped: no
account could produce a non-empty sample, so no field names were invented.
Email-account tools and a webhook trigger are omitted for the same reason.
Adds a `smartlead-errors` extractor since the API's 400s put the useful text in
`message` while `error` is only "Bad Request".
* feat(smartlead): expand to the core workflow surface and fix review findings
Grows the block from 22 to 47 tools and fixes every defect found in review.
New tools (all executed against the live API end to end):
campaign email accounts (list/add/remove), duplicate, delete, CSV lead export,
webhook delete + delivery summary, lead + mailbox statistics, top-level
analytics by date, lead activities, get lead by id, unsubscribe from campaign,
unsubscribe globally, mark complete, delete from campaign, master-inbox
replies, lead lists (list/get/create/update/delete), email accounts, clients.
The endpoint inventory was rebuilt by extracting method+path from all 212
reference pages, which corrected several earlier conclusions: get-lead-by-id is
`/leads/{id}` (not under `/campaigns/`), lead lists are `/lead-list/`
(singular), and lead activities are `/campaigns/all-leads-activities` with no
campaign segment. More documented paths that 404 in reality: lead tags at
`/crm/leads/tags`, and webhook delete at `/campaigns/{id}/webhooks/{id}` —
deletion actually takes the id in the body.
Shapes the docs got wrong again, caught live: `GET /leads/{id}` wraps the lead
in a single-element `data` array; `DELETE .../leads/{id}` answers with the bare
string `success`, not JSON; duplicate returns `newCampaignId`; create/update
lead list take `listName`, and mark-complete takes `campaign_lead_map_id` where
its siblings take `lead.id`.
Review fixes:
- get_campaign, get_campaign_analytics and get_lead_by_email reported an
all-null success for a missing resource, because Smartlead answers HTTP 200
with `{}` (or an empty body) instead of 404. They now fail closed.
- update_campaign_settings silently reset stop_lead_settings and
send_as_plain_text: their dropdown defaults are materialized at block
creation, so every settings update carried them. Both now default to
"Leave unchanged".
- Malformed JSON in Leads/Sequences/Custom Fields resolved to `undefined`,
which overwrote the raw string the executor falls back on and dropped the
field silently. Parsing now raises, and is scoped to the operation that
consumes the field so a stale hidden value cannot fail an unrelated one.
- The four documented import overrides (block/unsubscribe/duplicate/bounce
lists) had no field, so the block's own skill instructions were unexecutable.
- leadId did not distinguish lead.id from campaign_lead_map_id; passing the
latter 404s, and list_campaign_leads surfaces it first.
- Path ids are trimmed and escaped; dead code and a hand-rolled id mapper removed.
Unverified and called out rather than guessed: add/remove email accounts to a
campaign (no mailbox could be connected, so only their error shape was seen),
and the row shapes for statistics, message history, inbox replies, email
accounts and clients — every one of those collections was empty on the
verification account, so their rows pass through unmapped.
* fix(smartlead): correct request params and outputs found in re-validation
Three tools sent a parameter Smartlead's validator rejects outright with 400,
so the affected operations failed whenever the field was filled in:
- get_campaign_lead_statistics paginated with `skip`; the endpoint accepts
`offset` and only echoes it back as `skip`.
- list_lead_activities and list_inbox_replies both sent a campaign filter.
`campaign_id`, `campaignId`, `campaign_ids` and `email_campaign_id` are all
rejected, so the filter is gone rather than advertised and broken.
mark_lead_complete reported `next_sequence: null` on every call, including when
a step remained: `status.nextSequence` is an object, not a number. It now maps
to `next_sequence_id` and `next_sequence_delay_in_days` — verified live
returning step 10093171 rather than null.
get_lead_by_id reused the by-email mapper, so it always claimed the lead belongs
to zero campaigns; `GET /leads/{id}` omits `lead_campaign_data` entirely. It now
declares the narrower shape it actually returns.
A stale advanced `clientId` leaked into list_email_accounts: advanced subblocks
serialize without evaluating their condition, and that tool consumes `clientId`
while sitting outside its condition list. The field is now offered for that
operation too, so the value is visible wherever it is sent.
Two dropdowns had defaults that act on their own. `status` defaulted to PAUSED,
so choosing Update Campaign Status and never opening the dropdown paused the
campaign; it now requires an explicit choice. `pauseLead` sent `false` on every
categorization, which risks resuming a paused lead; it now defaults to leaving
the state alone.
Also counts CSV export rows with a quote-aware scan so a newline inside a name,
location, or custom field no longer inflates the count, and fills in the block
output declarations for the fields the 47 tools actually return.
* fix(smartlead): preserve a zero-day next-sequence delay and render enum values in docs
A next sequence scheduled to send immediately reported no delay at all:
`Number(next.delayInDays) || null` mapped a legitimate 0 to null.
Tool descriptions built enum lists with template literals. The runtime value
and the LLM-facing tool metadata were correct, but the docs generator reads the
description statically, so the public page rendered
`${SMARTLEAD_CAMPAIGN_STATUSES.join(...)}` instead of START, PAUSED, STOPPED.
The five affected descriptions now spell the values out.
* fix(smartlead): stop email-account tools from emitting mailbox credentials
Connecting a real mailbox to the verification account made the email-account
response shapes observable for the first time, and they carry the stored
credentials: `GET /email-accounts/{id}/` and the campaign route return
`password` in plaintext, the list route returns it base64-encoded, and both
carry `imap_password`.
Both tools passed rows through unmapped, so those values would have reached
workflow output, execution logs, and model context. They now select fields
explicitly and omit the credentials.
Verified against the live API: the API response contains the password while the
tool output does not, for both tools.
Also fills in the real email-account fields, which were previously an opaque
array — id, sender identity, SMTP/IMAP host and port, verification state and
last error, sending caps, warmup status, and tags.
* fix(smartlead): remove the dead campaign field that could target the wrong campaign
Removing the campaign filters from list_lead_activities and list_inbox_replies
left their `activityCampaignId` subblock, its params mapping, and its inputs
entry behind. Two problems, the second serious:
- On those two operations the field promised campaign scoping the API cannot
do. Smartlead rejects every candidate key (`campaign_id`, `campaignId`,
`campaign_ids`, `email_campaign_id`), so the value was silently discarded and
account-wide results were reported as scoped.
- Worse, the field is `mode: 'advanced'`, and advanced subblocks serialize
without evaluating their condition. A value left over from listing activities
therefore fed `campaignId` on all 32 campaign operations through the
`params.campaignId || params.activityCampaignId` fallback. Configuring List
Lead Activities with campaign 111, then switching the block to Delete
Campaign and leaving Campaign ID blank, would have passed required-validation
and deleted campaign 111.
Both list tools now also say plainly that Smartlead exposes no campaign filter,
rather than advertising one in their descriptions.
Also: route mark_lead_complete's next-sequence id through the shared numeric
coercion, since Smartlead string-encodes numbers inconsistently and its sibling
field already arrives as a string; re-bind the two enum constants that lost
their last consumer so the literal descriptions cannot drift undetected; and
declare the 17 tool output keys the block was missing — `accounts` most
importantly, which is the entire payload of both email-account tools.
|
||
|
|
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>
|
||
|
|
c3da54470b |
docs(sso): correct callback host and issuer guidance, document Entra SAML and IdP-initiated behavior (#6334)
* docs(sso): use the deployed host in callback and entity ID examples * docs(sso): correct host, issuer, and provider-id guidance; document Entra SAML and IdP-initiated behavior * docs(sso): send Entra federation metadata to the field that reads it |
||
|
|
ab257555d8 |
fix(sso): link Entra sign-ins to existing accounts and enforce unique provider IDs (#6311)
* fix(sso): link Entra sign-ins to existing accounts and enforce unique provider IDs
Better Auth 1.6.23 calls the account-linking handler with trustProviderByName:
false, which disables the trustedProviders allowlist for SSO entirely. Trust now
comes only from the provider's domainVerified flag, which Sim never set — so any
user who already had a Sim account was stranded on "account not linked". Entra
never sends email_verified, so this hit every Microsoft tenant.
Sim already proves domain ownership via sso_domain before a provider can be
registered, so the register route mirrors that decision onto domainVerified.
The column defaults to true so existing providers keep signing in across the
deploy, since enabling the option turns sign-in into a hard gate.
Also enforces the providerId uniqueness Better Auth already assumes: it rejects
any id that exists in any tenant and resolves providers by that column alone, so
a second customer picking "azure-ad" could not register at all and got an opaque
422. Sim now returns a 409 naming a free id, and a unique index makes the
duplicate-row state unreachable.
* fix(sso): revoke domain trust when verification is removed mid-update
The create path re-checks domain ownership after Better Auth persists the
provider and rolls the row back if the verified sso_domain row disappeared in
that window. The update path had no equivalent, so deleting the verified domain
while updateSSOProvider was in flight still set domainVerified, restoring
same-email account-linking trust for a domain the org no longer proves it owns.
The update path has no newly-created row to roll back, so it clears the flag
instead: that denies linking and blocks sign-in on the provider until the domain
is verified again.
* fix(sso): make domain-trust grants atomic and propagate revocation
Greptile flagged that the ownership check and the domainVerified write were
separate statements, so a domain deleted between them still ended with trust
granted. Two changes close it from both sides.
The grant now folds the ownership test into the UPDATE's WHERE clause, so
Postgres evaluates both in one statement and the write matches nothing once the
proof is gone.
Removing a verified domain now clears domainVerified for providers on that
domain, in the same transaction as the delete. This was a standing gap, not just
a race: deleting a domain previously left linking trust set indefinitely.
Together the provider cannot end up trusted without current ownership in either
commit order — if the grant lands first the delete clears it, and if the delete
lands first the grant no-ops.
* fix(sso): report a refused domain-trust grant instead of returning success
The conditional grant could match zero rows if the verified domain was deleted
between the pre-write check and the write. The route ignored that and returned
200, leaving a provider that cannot sign anyone in while telling the admin it
saved.
The grant now reports whether it matched, and that result is the single decision
point on both paths: the create path rolls the provider back, the update path
clears the flag, and both return SSO_DOMAIN_NOT_VERIFIED. This also drops the
separate post-write ownership read, since the UPDATE re-tests ownership itself.
* feat(sso): let admins map IdP claims, and trim setup comments
Identity providers disagree on which claim carries each value — Entra can send
the address as `upn` rather than `email` — and the mapping was hardcoded, so a
mismatch had no fix in the UI at all. Adds an Attribute mapping section for both
protocols, defaulting to each protocol's standard claim names shown as
placeholders, so the common case still needs no input.
Editing an existing provider now loads its stored mapping and only treats a
value as an override when it differs from the default, so a saved custom mapping
is never silently rewritten.
* feat(sso): expose the standard enterprise IdP options in the setup form
Rounds out the form with the options Better Auth already accepts but the UI hid,
so a non-standard IdP no longer dead-ends at a field that cannot be set.
SAML gains signature algorithm, digest algorithm and NameID format. Only SHA-256
and stronger are offered: Better Auth warns on SHA-1 as deprecated and rejects
anything outside its secure set, so weaker choices would only produce failed
saves.
SAML also surfaces the SP Entity ID beside the ACS URL. IdP admins are usually
handed a vendor metadata document; Sim does not publish one, and these are the
two values it would carry.
OIDC gains authorization, token and JWKS endpoint overrides for providers whose
discovery document is incomplete or unreachable. Discovery still fills them in
when they are left blank.
All of these load from the stored config when editing, so re-saving a provider
cannot quietly drop them.
* fix(sso): withhold domain trust from personal providers on the hosted deployment
A personal (org-less) provider has no verified domain behind it, but the trust
grant treated it as authoritative anyway. On the multi-tenant deployment that is
an account-takeover primitive: anyone able to register one could claim a domain
they do not own, point it at their own IdP, and have a sign-in auto-link to an
existing account on that domain.
Sim's UI always registers org-scoped, so this only reaches direct API callers.
Self-hosted deployments are single-tenant — the operator is the only tenant —
so the org-less path keeps working there.
Also clears the attribute mapping when the protocol changes: claim names are
protocol-specific, so an OIDC override carried into a SAML config would save a
mapping the IdP cannot resolve.
* docs(sso): correct the personal-provider trust note after the hosted gating
* fix(sso): drop the inert SAML algorithm selects, make NameID format clearable
The signature and digest algorithm selects were placebo controls. Tracing
@better-auth/sso 1.6.23, those two values are only read by validateConfigAlgorithms,
mergeSAMLConfig and sanitizeProvider — createSP and createIdP never pass them to
samlify, so nothing they select reaches the SAML exchange. Bugbot separately
noted they could not be cleared, since Better Auth merges with `??` and omitting
a key keeps the stored value. A control that neither applies nor clears should
not exist, so both are removed.
NameID format is genuinely wired (createSP passes it as nameIDFormat) and is
kept, but is now always sent rather than omitted when set to the provider
default. samlify falsy-guards the value, so an empty string reads as unset and
"Provider default" can actually clear a stored override.
The read-only provider view also now shows the SP Entity ID and the ACS label
for SAML — admins land there after saving and need the same two values the form
says their IdP requires.
* docs(sso): tighten the personal-provider note to the self-host path it describes
* fix(sso): forward an empty SAML NameID format so the provider default can be restored
The form sends an empty identifierFormat when the admin selects "Provider
default", but the route dropped it with a truthiness check. Better Auth merges
SAML config with `??`, so an omitted key retains the stored value — the selection
appeared to apply and silently did not.
Forwarding the empty string makes it reach the merge, and samlify falsy-guards
nameIDFormat, so it reads as unset. Selecting the provider default now actually
clears a stored override.
* fix(sso): revoke trust for providers whose domain is spelled with a wildcard
Migration 0268 grandfathered providers by normalizing their domain with
lower + btrim + a stripped leading `*.`, so sso_provider.domain can hold
`*.acme.com` while its verified sso_domain row holds `acme.com`. The revoke on
domain deletion compared the raw column, so such a provider matched nothing and
kept domainVerified after its ownership proof was gone.
The comparison now applies the same normalization 0268 used, so a grandfathered
row is matched the way it was written.
* fix(db): give the SSO index migration the concurrent-build convention it skipped
packages/db/scripts/migrate.ts documents the required shape for CONCURRENTLY
statements, and the previous migration follows it. 0284 did not, and the
omission is silently destructive.
migrate.ts sets a session lock_timeout of 5s, which survives the embedded
COMMIT. CREATE INDEX CONCURRENTLY waits on every concurrent write transaction in
the database — not only ones touching this table — so on a busy database the
build is cancelled with 55P03 and leaves an INVALID index. The retry then
replays the file, IF NOT EXISTS skips the invalid index, and DROP INDEX removes
the only working index on provider_id. The migration journals as applied and
exits 0 with provider_id unindexed and uniqueness unenforced, reopening the
cross-tenant provider resolution this migration exists to close.
Adds SET lock_timeout = 0 around the concurrent statements, a pre-drop of the
target index name so a replay rebuilds rather than skips, and restores the 5s
timeout afterwards. Verified by stranding an INVALID index and replaying: the
end state is a valid unique index with uniqueness enforced.
Also corrects the sso() comment that claimed domainVerified confines linking to
matching email domains. link-account.mjs blocks on
`!isTrustedProvider && !userInfo.emailVerified`, so an IdP asserting
email_verified links regardless of domain — the flag narrows nothing on its own.
* fix(sso): give the Enter shortcut the same guard as the Add domain button
The Enter handler called handleAdd unconditionally while the button was disabled
during an in-flight add, so repeated presses could issue overlapping requests.
Both now read one canAddDomain flag rather than duplicating the condition.
* fix(sso): stop the provider ID being editable after it is saved
Renaming it was never useful and always destructive. The value forms the
redirect URL registered with the identity provider, so changing it breaks
sign-in until the IdP is updated. Worse, the register route selects
create-vs-update by (providerId, organizationId), so a renamed id misses and
registers a SECOND provider; the settings page renders providers[0], so the
duplicate is invisible, there is no delete action to remove it, and existing
account rows still reference the old id.
Editing now shows it as a read-only copyable value, and the create form says up
front that it cannot be changed later.
Also hoists the suggestion list to module scope — it was rebuilding 44 objects
on every keystroke anywhere in the form.
* fix(sso): stop persisting generated IdP metadata so SAML cert rotation works
The route stored an IdP metadata document built from cert + entryPoint even when
the admin supplied none. The form loads that document back into its optional
metadata field and resends it, and on the next save it wins over the certificate
— so rotating a SAML signing certificate through the form appeared to succeed
and changed nothing.
Only metadata the admin actually pasted is persisted now. With none stored,
Better Auth's createIdP builds the IdP from issuer, entryPoint and cert, which
are the fields the form edits. No SAML providers exist in production, so this
changes no live tenant.
* fix(sso): always write SAML IdP metadata so clearing it takes effect on update
Not storing generated metadata fixed new providers but not existing ones: Better
Auth merges SAML config with `??`, so omitting the key let a previously stored
document survive and keep overriding the certificate.
The key is now always written, empty when the admin supplied none. createIdP
falsy-guards it and falls back to issuer/entryPoint/cert, so clearing the field
actually clears it.
* fix(sso): hold the domain proof under a row lock while granting trust
Two fixes from review.
The trust grant folded the ownership test into the UPDATE's WHERE clause, but
under READ COMMITTED the EXISTS subquery is evaluated against the statement's
original snapshot. A delete committing while the UPDATE waited on the provider
row could therefore still see the removed sso_domain row and grant trust after
ownership was gone. The grant now selects the proof FOR SHARE inside a
transaction before writing, so the delete blocks until it commits, and if the
delete committed first the select finds nothing and no trust is written.
Editing a SAML provider also broke on configs written by the previous commit:
hydration used `config.idpMetadata?.metadata || config.idpMetadata`, and
`{ metadata: '' }` is falsy at the property but truthy as an object, so an object
landed in a string field and failed validation on save. It now narrows on the
type and handles both the object and legacy bare-string shapes.
* refactor(sso): write the two merge-sensitive SAML fields the same way
idpMetadata and identifierFormat both exist to defeat Better Auth's `??` merge,
which silently keeps a stored value when a key is omitted, but they were written
differently — one always, one only when defined. Both are now always written,
empty when unset, under one comment explaining why and noting that each is
falsy-guarded downstream.
Also drops a redundant saveDisabled: false; the prop already defaults to false.
* fix(sso): report the row the trust grant actually matched
The grant returned true once it found the proof, without checking that the
provider UPDATE matched anything, so its boolean did not always mean what
callers read it to mean. It now reports the matched row.
* fix(sso): restore provider domain trust when a domain is re-verified
* fix(sso): correct the domain-removal warning now that it disables sign-in
* chore(sso): trim verbose comments
* fix(sso): revert a rejected SSO update instead of leaving it stored
|
||
|
|
c530d276b7 |
fix(env): flag combinations for sandboxes (#6308)
* fix(env): flag combinations for sandboxes * more changes |
||
|
|
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> |
||
|
|
dcaa118752 |
improvement(docs): restructure sidebar, align chrome, rename Mothership to Chat (#6296)
Sidebar: 11 separator groups become 5, with each module a collapsible folder that auto-opens on the active page. 61 always-visible rows drop to 16. Groups mirror the app's own nav (Chats/Workspace/Workflows) rather than inventing a taxonomy; Enterprise and Self-Hosting are hoisted out of Platform. Chrome: register the `hover-hover` variant, without which every @sim/emcn hover state silently compiled to nothing; restore the sidebar's Geist font stack; add 11 emcn tokens that were falling back to currentColor; adopt the named type scale; align row geometry, hover tokens and group labels with the app. Rename: mothership/ -> chat/ with redirects for the old URLs. Asset paths, the @mothership.sim.ai domain and the `mothership` log-trigger enum value are deliberately left alone -- they are CDN objects, a real domain, and a live product value. Also removes the page-type badge, drops the "Next" heading from the ToC, and lets FAQ rows open independently so expanding one no longer shifts the page. |
||
|
|
a3a887ad5a |
fix(docs): point the service-account guides at the real connect flow (#6277)
* fix(docs): point the service-account guides at the real connect flow
Fourteen of the twenty service-account guides sent admins to a workspace
Settings → Integrations tab that does not exist — Integrations is a top-level
workspace route, and there is no integrations section in the settings navigation
at all. The same fourteen then told them to search the catalog for
"<Service> Service Account", a name no catalog entry has: the list is derived
from blocks, so the entries are "Airtable", "Monday", "Wealthbox", and search
matches only name and description.
Both steps now match the six guides that were already correct (Box, Zoho Desk,
Zoom, Salesforce, Pipedrive, Atlassian), so all twenty describe one flow. The
per-guide connect labels were already right and are untouched.
Google needed a third fix: its final step said Click **Save**, but the modal's
primary button is `Add {connectNoun}`, which for Google falls back to
"Add service account". It also has no catalog entry of its own, so the search
now points at Google Drive with a note that any Google integration works.
Also adds `invalidCredentialsHelp` for Wealthbox. Its validator rejects a token
that works only over Wealthbox's documented ACCESS_TOKEN header, because Sim's
tools authenticate with Bearer — a deliberate, documented limitation whose
reason reached the server log and never the user, who saw only "Double-check it".
* fix(docs): make the Wealthbox rejection copy true for every failure path
`invalidCredentialsHelp` replaces the generic message for every
`invalid_credentials` rejection, and the Wealthbox validator raises that code on
three paths: a 402 expired trial, a 401/403 where both header styles fail, and a
401/403 where Bearer fails but the ACCESS_TOKEN probe succeeds. The copy
described only the third, so two of the three told the user their token was
valid and pointed at remediation that could not help.
Now leads with what is checkable in all three cases and makes the Bearer note
conditional on the one signal that distinguishes it — the token working
elsewhere.
|
||
|
|
0ab44c5b44 |
improvement(zoho-desk): pick the data center from a dropdown and trim service-account help text (#6271)
* improvement(zoho-desk): pick the data center from a dropdown and trim service-account help text
The Zoho Desk Self Client modal rendered a paragraph of setup steps as the hint
under Client secret, duplicating both the setup guide and two of its own field
hints. Cut it to the one caveat that isn't derivable from the form, and moved it
to the org-identifier field the caveats actually qualify. Data center is now a
dropdown sourced from ZOHO_DESK_DATA_CENTERS.
Same editorial pass across the other service accounts: Zoom, Salesforce,
Shopify, Webflow, Trello and Cal.com dropped setup steps in favor of caveats.
Also adds the documented Zoho Desk params that were missing (list_tickets
assignee/channel/receivedInDays, list_comments and list_threads sortBy,
get_contact and get_thread include), each gated per operation so a stale
subBlock value can't leak into an endpoint that reads the same param name.
* fix(zoho-desk): let an unsupported receivedInDays reach the tool's validation
The block mapper filtered on shape before forwarding, so a fractional or
non-numeric value was dropped and List Tickets then ran with no window at all —
returning the whole queue as though the requested filter had applied. The tool
owns that validation, so the mapper now passes the value straight through.
Adds a block-to-tool seam test: neither side's own tests could catch a value
lost between them.
* fix(zoho-desk): overwrite operation-scoped params instead of omitting them
The block mapper scoped params by destructuring them out of the spread, on the
assumption that a key left out of the return value never reaches the tool. It
does: both call sites merge the mapper's output on top of the original inputs
(`{ ...inputs, ...transformedParams }`), so an omitted key is restored.
The serializer is what actually held this together, and it has a gap — an
advanced subBlock with a retained value is emitted for every operation while the
block's advanced toggle is off, because that branch returns on isNonEmptyValue
without evaluating the subBlock's condition. So a Sort By set on List Tickets
reached List Comments, and a ticket Include reached Get Contact, each rejected
by Zoho. Out-of-range from/limit reached the wire for the same reason.
Every scoped param is now assigned unconditionally, undefined included, so the
merge cannot resurrect a stale value.
Also fixes a crash this branch introduced: clearing the Departments multi-select
stores [], which reached the comma-list normalizer and threw on .split. The
helper now takes arrays, which is what that subBlock actually stores.
The block-to-tool tests now model the real merge rather than the mapper's return
value alone — the previous version passed while production threw on the same
input. Corrects two comments that misstated where Zoho documents customFields
and errorMessage, and splits the shared include subBlock, since Get Ticket
accepts contract and skills and List Tickets does not.
* fix(zoho-desk): do not scope params on the agent-tool path
The previous commit made the mapper assign every operation-scoped param
unconditionally, so the merge could not resurrect a stale value. That is right
on the canvas path and wrong on the agent-tool path, where `operation` is a
sibling of the tool call rather than a member of params: the mapper saw
`operation === undefined`, every gate resolved to undefined, and the merge then
overwrote the model's own arguments with it. A Zoho Desk tool called by an agent
lost every parameter the model supplied.
That path needs no scoping — the tool is already chosen, and the model addresses
tool params by their real names — so it now returns early. Custom fields are
still coerced there, since parsing JSON is a type fix rather than an operation
gate, and that parsing is now shared by both paths.
* fix(zoho-desk): keep the legacy include working on Get Ticket
Splitting the shared `include` subBlock into `include` and `ticketInclude` left
workflows saved before the split reading an empty field, so their Get Ticket
calls silently stopped embedding what they asked for.
Get Ticket now reads `ticketInclude ?? include`. The fallback only goes that
direction: Get Ticket accepts every value List Tickets does plus `contract` and
`skills`, so a legacy value is always valid there, while List Tickets still
reads only `include` and can never receive the two extra tokens it does not
document.
|
||
|
|
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 |
||
|
|
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> |
||
|
|
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. |
||
|
|
b8ec114382 |
improvement(chat): secrets mounting / exposure improvements and controls (#6191)
* fix(copilot): secrets injection into sandbox * improvement(chat): secrets mounting / exposure improvements and controls * fix(secrets): simplify copilot mounting flow * test(secrets): preserve standard tool permissions * fix(copilot): bind workflow tool completions * fix(secrets): preserve own environment keys * fix(copilot): release failed workflow claims * fix(copilot): trust compacted workflow completion |
||
|
|
25e609167f |
fix(search): disambiguate tables and knowledge bases by folder (#6192)
* fix(search): disambiguate tables and knowledge bases by folder The Cmd-K search modal listed tables and knowledge bases without the folder breadcrumb workflows and files already showed, and the table, knowledge base, and search-and-replace pickers in the workflow editor rendered bare names -- so two resources sharing a name in different folders were indistinguishable. Extracts the disambiguation the workflow selector already did into shared collectDuplicateNames + disambiguateLabelByFolder, and shares the search row's folder breadcrumb and its memo comparator, which were duplicated between the workflow and file rows. Also routes folder text through filterAndCap's secondary-rank parameter rather than concatenating it into the name, so an exact name match can no longer be outranked by a folder that happens to fuzzy-match. * fix(zoho-desk): use the real Zoho Desk mark on a white tile The icon was a generic headset placeholder drawn in currentColor, so it never resembled Zoho at all. Replaces it with the mark from Zoho's official logo -- wordmark stripped, viewBox set to the mark's own bounding box so it centers -- and moves the tile to white, matching the other brand-mark integrations. |
||
|
|
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>
|
||
|
|
9064039c19 |
improvement(logfire): scope block outputs per operation and refresh brand chrome (#6178)
* improvement(logfire): scope block outputs per operation and refresh brand chrome - gate each block output on the operations that actually return it - swap in the official Logfire mark, black tile with brand-magenta bare icon - move host to advanced mode and alphabetize the tool registry entries - add track-logfire-llm-cost and verify-logfire-token-target skills * fix(logfire): honor numeric-string limits and surface token validity fields - accept a numeric-string limit so agent-invoked calls stop silently falling back to Logfire's 100-row default - keep an hour-only UTC offset intact instead of producing +05Z - surface expiresAt and spendingCapReachedAt on Get Token Info - document pending_span as a fourth record kind * chore(logfire): regenerate tool metadata and document the step in the skill - regenerate apps/sim/tools/generated/tool-outputs.ts, which CI's tool-metadata:check requires after a tool output change - add the regeneration step and artifact-diff guidance to the validate-integration skill so the gate stops being missed * chore(skills): sync validate-integration projections |
||
|
|
18214158b9 |
fix(sanitization): secret exposure in function and agent trace spans (#6000)
* fix trace span secret sanitization * sanitize workflow output logs * preserve streaming usage estimates * Fix logging session test after staging merge * secrets sanitization correctness * fix(execution): address review regressions * fix(execution): harden secret trace provenance * fix(execution): preserve functional state during trace projection --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai> |
||
|
|
19b0312a04 |
feat(managed-agent): add session lifecycle operations (#6140)
* feat(managed-agent): add session lifecycle operations
Adds an operation selector to the Claude Managed Agents block, backed by
nine new tools alongside the existing run-session behavior:
- create session (non-blocking, seeds initial_events)
- send message to an existing session
- get session (surfaces tool calls awaiting approval)
- list events
- update session (title/metadata)
- interrupt session
- respond to tool confirmation (allow/deny)
- archive session
- delete session
Run Session stays the default, so blocks saved before the selector
existed keep their exact behavior and field layout.
* fix(managed-agent): address review findings on session lifecycle ops
- List Events kept the OLDEST slice when capped, dropping the agent's most
recent reply. Paging is exhaustive again and the cap now keeps the newest
N after ordering, with a `truncated` flag so callers know it is a tail.
- listPaginated returned whole pages past maxItems; it now trims to the
exact cap.
- A whitespace-only title passed the update guard and would have cleared an
existing session title. Blank is now treated as not provided.
- Interrupt had no request timeout and could hang; bounded at 15s while
still honoring the workflow signal.
- Custom-tool gates were surfaced by Get Session but could not be answered,
since they need user.custom_tool_result rather than a confirmation. Adds a
Respond To Custom Tool operation and a `kind` on each pending gate so a
workflow routes to the right one.
- Docs rendered a raw ${DEFAULT_EVENT_LIMIT} placeholder for the default.
* fix(managed-agent): correct truncation flag, gate lookup, custom tool result
- `truncated` was true whenever the history size equalled the limit, even
though nothing was dropped. Event reads now report the untrimmed total and
the flag compares against that.
- Pending-gate enrichment capped its read, which keeps the OLDEST events in
page order — the opposite of where blocking gates live. It now filters to
the ids being looked up as pages arrive, which is both correct regardless
of page order and bounded by the id count. Paging continues on the raw
page so a fully-filtered page is not mistaken for the end of the list.
- Respond To Custom Tool applied one result to every id, so multiple pending
tools would all receive the same output. It now answers a single call per
invocation.
* fix(managed-agent): stop fractional event limits reading unbounded
A limit below 1 passed the positivity check and then floored to 0, which
made `slice(-0)` hand back the ENTIRE history flagged as complete — the
opposite of the requested bound. The limit is now floored before it is
validated, so anything that does not resolve to a positive integer falls
back to the default.
Also hardened the library: a zero or negative cap short-circuits to an
empty result instead of falling through to `slice(-0)`, so no future
caller can hit the same trap.
* fix(managed-agent): stop gate lookup scanning the full tool history
The id filter keeps the collected array tiny, so `maxItems` never trips and
the walk continued to the end of a session's tool history even after every
blocking id had been found. `listPaginated` now takes a `stopWhen` predicate
and the gate lookup ends as soon as it has all the ids it came for.
Also makes a blocked-but-unnamed session observable: when a session reports
`requires_action` with no blocking event ids, `requiresAction` stays true —
reporting false would tell a workflow the session is fine while it is parked
indefinitely — and the dead end is logged and documented instead.
* fix(managed-agent): floor event cap and make metadata clearing explicit
- A `maxItems` between 0 and 1 slipped past the zero guard and became
`slice(-0)` — the whole history — because slice truncates its index toward
zero. The cap is now floored at the library boundary, so no caller can hit
it whatever they pass.
- Update Session documented full metadata replacement but could not express
a clear: an empty map normalizes to "absent". Inferring the clear from
emptiness would be worse, since an untouched table is also empty and would
wipe metadata on every title-only update. Adds an explicit `clearMetadata`
instead, and corrects the parameter's documentation.
* test(managed-agent): pin the HTTP shape of every session endpoint
Method, URL, and beta header for all 11 calls, plus the SSE accept header,
the separate memory-store beta (combining the two is a documented 400), and
content-type only on requests that carry a body. These are the details types
cannot catch and that break silently when a path is "tidied".
|
||
|
|
c5cc6ce26c |
feat(chat): hide the Chat module when NEXT_PUBLIC_CHAT_DISABLED is set (#6137)
* feat(chat): hide the Chat module when CHAT_ENABLED is unset A self-hosted deployment that skipped the chat key still rendered the full mothership Chat UI, landing on the composer and 401ing on every message. Gate it behind a CHAT_ENABLED / NEXT_PUBLIC_CHAT_ENABLED twin, written by the setup wizard alongside COPILOT_API_KEY and validated by the existing FLAG_TWINS doctor check. The flag resolves at module scope on both render passes, so no chat surface renders then disappears. With Chat off the workspace lands on its first workflow (resolved server-side, behind the cached host-context check so no workflow id leaks to non-members), and the chats list, scheduled tasks, editor Chat panel, and chat CTAs are absent. Routes are gated rather than deleted: /home redirects because it is baked into delivered invitation emails and the accept contract. Also fixes two bugs the gate exposed: a persisted activeTab of 'copilot' left the workflow panel blank from first paint, and the panel's handoff listener claimed MOTHERSHIP_SEND_MESSAGE events outside its own gate, silently swallowing "Fix in Chat" messages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * refactor(chat): gate the UI on NEXT_PUBLIC_CHAT_DISABLED, not an opt-in flag CHAT_ENABLED made Chat opt-in, so every existing deployment that already had COPILOT_API_KEY would have lost the module until it set a new variable. Invert to an opt-out so nothing changes for them. That also collapses the twin. The only reason the flag needed a server/client pair was that it projected a secret; NEXT_PUBLIC_CHAT_DISABLED is not one, so getEnv resolves the same value from process.env on the server and window.__ENV in the browser. Gone with it: the FLAG_TWINS entry and its doctor sync check, the two-variable wizard write, and the boot-time throw, whose contradiction (flag on, key absent) can no longer be expressed. Presentation and capability are now separate concerns. NEXT_PUBLIC_CHAT_DISABLED decides whether the surfaces render; COPILOT_API_KEY decides whether the work can run, and gates the paths that need it — the Sim Chat block, prompt-job claims, and inbox access — each failing on its own terms. The wizard writes the opt-out when you skip the chat key, which is the case this started from: a fresh self-host that never configured Chat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * feat(setup): prompt for the chat key in k8s mode The dev and compose flows minted a chat key and wrote the Chat opt-out alongside it; k8s did neither, so a cluster install with no COPILOT_API_KEY in its Helm values rendered a Chat module that rejects every message. Prompt with the same flow and feed both values into `app.env`, which the chart already renders as arbitrary container env. Reading the previous release's key matters here in a way it does not for the file-based modes: `helm upgrade` without `--reuse-values` keeps only what this document carries, so a key the user elects to keep has to be re-supplied or it is silently dropped. Splits the release-values read from the secret-reuse check so both the key and the secrets come from one `helm get values` call, and carries the mothership override across for the same mint-here-validate-there reason the other modes document. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * fix(setup): write app-behavior flags to every env file the app can start from The wizard wrote the Chat opt-out only to the env file its own mode owns, so choosing compose put it in the root `.env` while `bun run dev` reads `apps/sim/.env` and never saw it. Skipping the chat key appeared to do nothing. Mirror values that change how the app behaves — as opposed to where it connects — across both targets. Connection settings deliberately do not go through this: DATABASE_URL and friends differ between the compose stack and a local dev run, which is why this takes an explicit set of values rather than the whole batch. The mirrored file is written even when absent, since missing is exactly the case that stranded the flag, but with seeding suppressed so a compose run leaves a one-line apps/sim/.env instead of a full .env.example for a stack the user is not running. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * fix(compose): forward NEXT_PUBLIC_CHAT_DISABLED to the app container The wizard wrote the flag into the root .env, but compose only passes through variables the service's `environment` block names — and that block listed COPILOT_API_KEY without its companion. Skipping the chat key on a Docker install therefore did nothing: the value sat in .env and never reached the container. Add the passthrough to all four compose files. Reverts the previous commit's mirroring into apps/sim/.env, which treated the symptom — each mode writes only the env file it owns, and that file is now wired correctly. k8s needs no equivalent: its values flow into `app.env`, which the chart renders key by key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * fix(chat): resolve the landing route without blocking on the database Server-resolving the first workflow meant a session lookup, an access check and a query had to finish before anything rendered. A slow or unreachable database left the user on a blank page under a populated sidebar — worse than the instant redirect it replaced, and with no signal that anything was wrong. Redirect straight to `/w` instead and let it pick from the workflow list the layout already prefetches, so the choice costs no round trip and cannot hang. Repoints the sidebar's primary action rather than hiding it: the slot that offered "New chat" now offers "New workflow" and creates one, since with Chat off there is no composer to open but the intent is the same. Sends the CLI key handoff to signup rather than login. It is reached from a terminal — usually the setup wizard standing up a fresh self-host — where the visitor has no account yet. Both auth pages cross-link carrying the callback, so a returning user is one click from login with their destination intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * improvement(chat): address cleanup-pass findings on the Chat gate Effects: the panel's auto-select effect read the copilot chat list while the list query was deliberately skipped, took "empty" for "deleted in another tab", and cleared the user's selection — latching a ref that stopped it ever being restored. Guarded on the same condition as the handoff listener. Memo: `/w` filtered workflows through a useMemo whose array dependency was a fresh `[]` on every render while the query had no data — the exact window the page exists for — so it memoized nothing and re-fired the redirect effect. Keyed on the workflow id instead. Same unstable-default problem on the sidebar's chat list, where it invalidated five downstream memos; given a stable empty constant. Callback: `handleCreateWorkflow` listed the whole mutation object in its deps, which TanStack recreates every render. Harmless until this branch wired it into the top nav, where it defeated `memo(SidebarNavItem)`. React Query: Recently Deleted still fetched archived chats unconditionally and offered restores into routes that now 404. Also surfaces an error state on `/w` — it is the landing route now, so a failed list fetch would otherwise spin forever behind a log line — fixes a spinner using a token undefined in dark mode, and trims comments that restated code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * fix(chat): gate workflow creation on write access, pin the key in schedule tests The zero-workflow landing offered "Create workflow" to every member. Creation navigates optimistically, so a read-only member was sent to a workflow the server had already refused to create, with the failure never surfaced. Gate both entry points — the empty state and the sidebar's "New workflow" row — on the same `canEdit` check the rest of the sidebar uses, and tell read-only members who can make one instead of offering an action that cannot succeed. The schedule-execution tests only passed locally because vitest loads the developer's own `.env`, which supplied COPILOT_API_KEY; CI has none, so the prompt-job claim guard skipped the claims those cases assert on. Pin the key through the env mock so the suite states its own preconditions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha * fix(setup): name both variables in the chat-key failure hint The caller writes the Chat opt-out whenever the prompt returns no key, so the hint's "or set COPILOT_API_KEY yourself" restored capability while leaving the module hidden — the one path where following setup's own advice does not work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c0b19da782 |
feat(pi): install Bun in cloud sandboxes (#6123)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> |
||
|
|
ed23330f88 |
feat(knowledge): opt-in hybrid lexical + vector retrieval for KB search (#6124)
* feat(knowledge): hybrid lexical + vector retrieval for KB search KB search ranked purely on pgvector cosine distance, which retrieves exact tokens (error codes, ticket keys, identifiers, rare product names) poorly. Add a full-text leg over the already-present generated `embedding.content_tsv` column and its GIN index — no migration, no re-indexing — and fuse it with the vector leg by reciprocal rank. Both legs run concurrently and share the same visibility and tag-filter predicates; the lexical leg is best-effort and falls back to vector-only on failure. Hybrid is the default for every caller. `searchMode: 'vector'` on the internal and v1 contracts (and an advanced Retrieval Mode dropdown on the Knowledge block) restores the previous behavior. Both search routes now share one `executeKnowledgeSearch` dispatch instead of duplicating the three-branch retrieval logic. * change(knowledge): make vector the default search mode, hybrid opt-in Every existing caller — workflow block, v1 API, copilot, guardrail RAG — keeps its current ranking. Hybrid retrieval is now requested explicitly via `searchMode: 'hybrid'`. Also routes the copilot knowledge tool through the shared `executeKnowledgeSearch` dispatch so all four callers share one retrieval path, and documents `searchMode` on the public v1 search endpoint in the OpenAPI spec. * docs(knowledge): document the hybrid retrieval mode Regenerates the knowledge integration reference for the new searchMode tool param, and adds a Retrieval Mode section to the knowledge base workflow guide explaining when hybrid beats vector-only. * fix(knowledge): stop rank fusion from starving the lexical leg Rank n in one leg always ties rank n in the other, so ordering the fused list by score alone let whichever leg was scored first take every tied slot. At topK=1 that meant a hybrid search returned exactly the vector-only result and discarded the exact keyword match the mode exists to recover. Selection now orders by score and drains each tie group round-robin, taking from whichever leg has contributed fewest rows so far. The lexical leg is passed first so it wins a total tie, since a chunk the vector leg ranked below its distance threshold is the case hybrid was opted into for. * fix(knowledge): credit a shared hit to every leg that returned it Attributing a row found by both legs to a single leg left the round-robin owing the other leg a slot it had already been served. With a shared rank-1 hit and topK 2, that evicted the lexical-only row — the exact match hybrid was enabled to recover — in favor of the vector-only one. A shared row satisfied every leg that returned it, so every one of them is now charged for it. Tie-breaking prefers the candidate whose least-served leg has been served least, which also removes the arbitrary best-rank attribution. * fix(knowledge): reject a whitespace-only copilot query explicitly The shared dispatch treats a whitespace-only query as absent and throws when no tag filters accompany it, where the previous vector-only call would have embedded the blank string and searched. Tighten the existing guard so the tool returns its normal message instead. * fix(knowledge): fan the keyword leg out per knowledge base The vector leg caps candidates per base once getQueryStrategy sets useParallel, but the keyword leg always ran one global query with a single LIMIT. Searching several bases at once let whichever one ranks strongest lexically consume every slot, so an exact-token hit in a smaller base never reached fusion — the case hybrid exists to serve. The keyword leg now uses the same strategy: per-base queries under the same parallel limit, re-ranked globally on a selected ts_rank_cd. Both legs draw candidates the same way, so fusion combines rankings over the same pool. * perf(knowledge): stop the keyword leg detoasting every match's vector Selecting the cosine distance in the ranking query made Postgres detoast the 1536-dimension embedding and compute a distance for every full-text match before the LIMIT applied, so cost tracked how common the query term was rather than topK. On a 20k-chunk base with a term matching every row that was 61,055 buffer hits against 1,030 for the same query without the projection. Rank on ids and ts_rank_cd alone, then hydrate only the rows that survive the limit. Same results, and the worst case drops to ~27ms end to end. |
||
|
|
48aeac218c |
fix(uploads): set Content-Type once on presigned PUTs; document x-goog-meta-folderid for GCS CORS (#6121)
* fix(uploads): set Content-Type once on presigned PUTs; document x-goog-meta-folderid in the GCS CORS example XMLHttpRequest.setRequestHeader appends on repeated calls (values join with a comma), and GCS is the only provider whose signed uploadHeaders include Content-Type — so single-shot GCS uploads sent 'x, x', which fails V4 signature verification with 403 (headers canonicalize to a comma-separated value that must match what was signed; multipart part PUTs are unaffected since part URLs don't sign Content-Type). The client now sets its default Content-Type only when the server's signed headers don't already carry one, with regression tests for both paths. Also adds x-goog-meta-folderid to the documented GCS CORS responseHeader list — workspace uploads now sign a folderId metadata header, and GCS CORS matches preflight request headers against that list exactly (no wildcards), so the missing entry blocked browser uploads into folders. * chore(uploads): drop inline comment |
||
|
|
ee0157df4b |
feat(tables): add currency column type on a new column-type registry (#6106)
* feat(tables): add currency column type on a new column-type registry
Adds a `currency` column type, and consolidates the per-type knowledge it
would otherwise have been scattered across.
**Currency.** Stores a plain number and carries an ISO 4217 `currencyCode`
as display metadata. That split is what keeps it cheap: filtering, sorting,
uniqueness and CSV export all reuse the numeric paths unchanged, changing a
column's currency rewrites no rows, and the public row output stays a number
rather than a locale-formatted string consumers would have to reparse.
Input accepts the shapes an amount actually arrives in — `$1,234.56`,
`1 234,56 €`, `(12.00)` — so pastes, CSV imports and tool writes land as
numbers instead of being nulled.
**The registry.** Adding this type initially required edits in ~40 places:
32 switch arms under `lib/table`, ~26 UI branches, two hand-maintained icon
maps, and a coercion implementation duplicated four times. Every one of those
failed silently when missed — a missing `jsonbCastForType` arm compares
numbers as text; a missing compatibility arm blocks all conversions.
`lib/table/column-types/` now holds one file per type carrying its label,
icon, badge colour, storage cast, filter operators, coercion, validation,
compatibility and formatting. `Record<ColumnType, …>` on both registries is
the completeness gate: adding a type to the union is a compile error naming
exactly the two files to fill in, and the interface then requires every
field. The 32 switch arms are down to 3.
Two duplicates collapse as a consequence:
- The client no longer mirrors the server's select id-resolution. Those
helpers lived in `validation.ts`, which imports drizzle, so anything
reaching them became server-only and the grid hand-rolled its own copy.
Extracting them to `select-options.ts` lets both sides share one
implementation, so the optimistic cache can no longer disagree with what
gets persisted.
- The two icon maps become one registry read.
It also fixes a live inconsistency it surfaced: currency got a numeric
keypad in the grid's inline editor but a plain text field in the row modal.
Behaviour-neutral by construction: all 1046 tests in the touched areas pass
unchanged, with no test edits.
* test(tables): guard the column-type registry's invariants
Property tests for the registry itself rather than any one type: entries key
by their own id, COLUMN_TYPES stays derived, an unknown type degrades to
string instead of throwing, only opaque-id types restrict filter operators,
only configuration-free types are CSV-inferable, and every type that can
reject a draft has a message to show.
Plus the metadata-ownership matrix, which pins the generic ownership check to
the same answers the hardcoded per-type rules gave.
These target the registry's silent-failure class — a wrong jsonbCast or a
stray operator whitelist used to be invisible until a filter failed in SQL.
Both are verified to fail under mutation.
* fix(tables): read exponent-form amounts and reject bad currency PATCHes up front
Two P1s from review.
Scientific notation lost magnitude. `String()` emits exponent form past 1e21,
so a stored amount round-trips through the editor as `1e+21` — and the
sanitizer treated the `e` as decoration to strip, reading it back as 121. An
untouched cell silently lost 19 orders of magnitude on its next edit. Exponent
form is now taken at face value, but only when the string is wholly a numeric
literal once symbols are removed, so `12 EUR` (whose `E` survives the strip)
still parses through the separator path.
A failed currency PATCH left a partial rename. `renameColumn` commits in its
own transaction before the currency write, so a `currencyCode` the service
would reject — an unsupported code, or any code on a non-currency column —
errored only after the rename had stuck. Both are now caught before the first
write, matching the guard the route already applies to unique-on-select for
exactly this reason.
* refactor(tables): finish the registry migration and drop the dead config
Audit pass over every consumer, closing the gaps the first cut left.
Functional gap: the copilot agent had no currency support at all — it could
create a currency column with no code and could never re-denominate one.
`add_column` and `update_column` now accept `currencyCode`, with the same
up-front validation and the same code-only routing as the HTTP routes.
Config that consumers were still restating, now read from the registry:
- `supportsUnique` replaces the unique-on-select guard stated in three places
(service, both column routes, the copilot tool).
- `editor === 'toggle'` replaces seven `type === 'boolean'` checks in the grid
and expanded popover, all of which meant the same thing.
- `defaultMetadata` replaces the per-type stamping in `addTableColumn` and
`updateColumnType`.
- `sampleValue` replaces the per-type example values in the LLM prompt
scaffolding.
- `storesOpaqueIds` replaces the select filter in the find-row matcher.
Dead config removed: `getTypeBadgeVariant` had zero callers (already dead on
staging), and it was the only reader of `badgeVariant` — so the field, its
union, and all seven values went with it. `inferFromCsv` was read by nothing
but a comment; CSV inference is an ordered heuristic a boolean cannot express,
so it is gone too and `InferredCsvColumnType` is no longer exported.
Fixes a latent crash found on the way: unique-constraint checking normalized a
cell keyed on its RUNTIME type but reconstructed it keyed on the column's
DECLARED type, so a unique `date` column stored a bare `2024-01-01` and then
threw `SyntaxError` parsing it back. Both directions now go through JSON
unconditionally. Pre-existing, unrelated to currency.
Adds the `/add-column-type` skill and a Tables section in CLAUDE.md/AGENTS.md
pointing at it, so the next type is one file plus two registry entries.
* fix(tables): run the column PATCH guards ahead of the rename, not after it
Greptile was right and my previous reply was wrong. The guards were added in
the right shape but the wrong place — below `renameColumn`, which is the
first write and commits in its own transaction. A PATCH combining a rename
with an invalid currency therefore still committed the rename and then
returned 400, exactly the counterexample reported.
Moved the column lookup and all three pre-flight guards above every write.
This also closes the same latent hole for the pre-existing unique-on-select
guard, which sat in the same position.
Adds route tests that assert `renameColumn` was never called on each
rejection path, and that a valid combined rename + currency change still
targets the new name. Verified to fail against the previous ordering.
* fix(tables): make the retype gate and the write path share one parser
A simplify pass over the registry found two real defects and several places
the abstraction was being worked around.
Silent data loss on conversion. `isCompatibleWith` was hand-written per type
and had already drifted from `coerce`, despite the interface promising they
could not: `boolean` accepted '1'/'0'/0/1 in the gate but only 'true'/'false'
in the write path, so converting a column holding "1" reported zero
incompatible rows and then nulled every one of them. `date` drifted the other
way. `isCompatibleWith` is now optional and defaults to `coerce(...).ok`, so
the two are the same code; only `select` overrides, because its rules are
about the column (cleared-vs-required, cardinality) not the value.
`isColumnType` used `in`, which matches inherited keys — `isColumnType('toString')`
was true and `columnTypeById('toString')` returned `Function.prototype.toString`,
which the validator would then call `.validateDefinition()` on. Now `Object.hasOwn`.
`defaultMetadata` only ran on the currency arm of a retype, so a future type
would get its defaults on create but silently not on conversion. It now runs
for every non-select target, carrying forward only metadata the TARGET type
declares it owns — a currency→text conversion no longer strands a currencyCode.
The index doc claimed the registry is kept out of the `@/lib/table` barrel so
44 server modules don't pull `@sim/emcn/icons`. That was false: `constants.ts`
re-exported `COLUMN_TYPES` from the icon-carrying `registry.ts`, and the barrel
re-exports `constants`. `COLUMN_TYPES` now lives in the icon-free `types.ts`;
verified with an import tracer that both are icon-free again.
Also: 5 no-op `validateDefinition`s and 4 duplicated formatters collapsed into
registry defaults; `CURRENCY_OPTIONS` was an eager module-load IIFE costing
~8ms of ICU work on every table API route for a list only the config sidebar
reads, now built on first call; and the skill's validation grep claimed 'should
return nothing' when it returns 8 legitimate hits — it now explains how to tell
a leak from a genuine special case.
* fix(tables): reject a non-leading sign so dates don't parse as amounts
Found by Cursor Bugbot. `parseCurrencyInput` dropped every `-` as decoration,
so an ISO date's hyphens vanished and its digit groups joined: `2024-01-01`
read as 20240101. With the gate now sharing the write path's parser, a
date → currency conversion reported zero incompatible rows and silently
turned every cell into a huge number.
A sign is only meaningful at the front; an interior one means the string is
not a single amount. Leading signs, accounting parentheses, symbols, ISO
codes, grouping separators, and exponent form all still parse — covered by
the existing cases plus new ones, verified to fail without the fix.
* fix(tables): use getErrorMessage in the columns route test mock
`check:utils` bans the inline `e instanceof Error ? e.message : fallback`
form; the mock for `rootErrorMessage` used it.
* fix(tables): rename the column last so a failed write leaves it untouched
Greptile's remaining concern: the pre-flight guards read a schema snapshot, so
a column-type change landing concurrently can still make a later write fail —
and with the rename running first, that failure returned an error with the
rename already committed.
Guards cannot close that window; each write is its own locked transaction and
only the write itself sees the authoritative state. Ordering can. The rename is
the one write that is purely cosmetic, so it now runs last: a failed typed
write leaves the column entirely untouched, and a failed rename leaves the
typed change applied under the old name — the recoverable half. The typed
writes target the column's current name, since no rename has happened yet.
Tests cover both directions: a typed write rejected mid-flight must not rename,
and a successful one must rename strictly after. Verified to fail under the
previous ordering.
* fix(tables): write back coerced values on every conversion
Round 4 findings, all real.
A conversion is allowed exactly when the target type's `coerce` accepts the
value — and `coerce` frequently TRANSFORMS it. Only `select` and `currency`
wrote the transformed value back, so a conversion to any other transforming
type left the cell holding its old bytes under the new type. Converting a
number column to `date` accepted epoch values, stored them unchanged, and then
`(data->>'col')::timestamptz` failed on EVERY query against that column. I
opened this myself by defaulting `isCompatibleWith` to `coerce(...).ok`.
Fixed at the class rather than the instance: the compatibility scan now records
whatever `coerce` produced whenever it differs from what is stored, and one
generic write-back applies it. That subsumes the currency-specific migration
entirely, so it and its helpers are gone. `select` keeps its own id↔name
migrations, which are not coerce-expressible in the outbound direction. The
post-conversion column definition is built once, before the scan, so the
coercion reads the same metadata the stored value is later validated against.
Exponent parsing was ambiguous when followed by text: `1e5 EUR` read as 15.
An `e` with a digit on both sides is an exponent marker, so if the string is
not a clean numeric literal it is refused rather than guessed — the digit on
both sides is what keeps the `E` inside `12 EUR` parsing normally.
A failed rename could still leave a typed change committed. The one rename
failure a caller can cause — a name already taken — is now rejected up front,
leaving only the concurrent-collision race, which no pre-flight check can close
without spanning all writes in one transaction.
* fix(tables): stop a blank cell blocking an optional type conversion
Found by Cursor Bugbot. `''` is incompatible with every numeric type, and the
compatibility scan counted it as a hard blocker regardless of whether the
target was optional — so a text column with a single empty cell could not be
converted to a number at all, and the error said 'to a required ...' either way.
An unreadable-but-empty cell is not a conversion failure. The write path
already turns an unreadable value into null on an optional column, so the
conversion now does the same and records null for it. A required target still
reports it, which the existing guard above already does with the message that
actually fits.
Also pins the two intentional divergences from the pre-registry behavior. A
differential run of the registry against the pre-refactor implementations (55
values x 7 column shapes) found ZERO coercion differences and exactly two
compatibility differences, both deliberate: boolean now rejects the '1'/'0'
conversions the old gate accepted and then nulled, and date now accepts the
epoch numbers its write path always accepted. Tests pin both so neither can be
silently reverted or widened.
* fix(tables): refuse conversions that would invent or destroy values
Final adversarial scan found two data-corrupting conversions, both opened by
defaulting the retype gate to the write path's parser.
number → date destroyed every value. `date.coerce` reads a number as epoch
milliseconds, which is right for one deliberate write and catastrophic applied
to a whole column: 1, 5, 42 became three timestamps in January 1970, and a
Unix-seconds column landed in 1970 rather than the year it meant. Irreversible.
`date` now overrides the gate to reject numbers, restoring the pre-refactor
behavior, and the contract states the rule the override obeys: a gate may be
STRICTER than `coerce`, never looser. Stricter refuses a bulk conversion while
single writes still work; looser is the direction that corrupts.
string → currency invented values. The parser stripped every non-digit and
joined what was left, so `01/02/2024` read as 1022024, `Room 101` as 101, and
`0.1.2` as 12 — a column of SKUs or phone numbers converted with zero reported
incompatibilities. What remains after removing symbols, spacing and an ISO code
must now be only digits and separators, and grouping must be well-formed (a
first group of 1-3 digits, the rest exactly 3). Every legitimate form still
parses, including all the locale variants.
Also generifies the last three metadata leaks: `buildConvertedColumn` strips
and carries back by iterating the key list rather than naming keys (naming them
meant a future type's metadata rode onto a target that rejects it, failing that
column's validation on every later write), `normalizeColumn` forwards metadata
through a shared `typeMetadataOf`, and `filterOperatorsFor` moved onto the
definition — it was a per-type branch inside the registry's own accessor, the
one thing the registry exists to forbid.
Skill corrected: it claimed COLUMN_TYPES derives from the registry (backwards),
promised exactly two compile errors (four once a type owns metadata), used a
grep that missed half the real branches, and never mentioned `import.ts`'s
second coercion path, whose silent default arm is the costliest miss available.
Differential re-run vs the pre-refactor implementations: 0 coercion
differences, 1 intentional compatibility difference (boolean no longer accepts
the 0/1 conversions the old gate accepted and then nulled).
* fix(tables): let the row modal accept formatted amounts again
Found by Cursor Bugbot. I unified the row modal's input type with the grid's
`inputMode` last round, but in the wrong direction: mapping `inputMode:
'decimal'` to `<input type="number">` made the modal reject $1,234.56,
1.234,56 and (12.00) — the exact formats `parseCurrencyInput` exists to accept,
and which the grid's inline editor takes fine.
A native number input and a numeric keypad are different things. Types whose
parser accepts formatted text now say so, and get a text field with
`inputMode='decimal'` — the shape the grid already uses. A plain number keeps
the native input, its spinner, and its validation.
* fix(tables): fold a rename into the write it accompanies
Closes the last partial-update window, properly rather than by pre-checking
around it.
A rename is metadata-only — `renameColumn`'s own comment says so: rows,
metadata, and workflow-group refs all key on the stable column id, so it is a
pure schema write. Nothing forced it to be its own transaction. Running it
separately is what created the window: whichever half committed first survived
a failure in the other, and no pre-flight guard can close a concurrent
collision because only the write itself sees authoritative state.
The four column writes now accept an optional `newName` and apply it through
one shared `applyPendingRename`, which validates the name shape and checks the
collision against the very schema snapshot that write is landing in. A combined
request rides the rename on whichever write runs last, so both halves commit
together or neither does — a concurrent claim on the name now aborts the whole
transaction instead of leaving the other change applied.
The routes also address every write by the column's stable id rather than its
name, so folding a rename into one write cannot break the next one's lookup.
A rename with nothing to ride on still runs standalone.
What remains partial is a type write followed by a failing constraints write —
two independently locked transactions, pre-existing, and untouched by this PR.
* fix(tables): migrate scalar cells when converting a column to select
Found by Cursor Bugbot. `resolveSelectOptionId` stringifies a number or
boolean before matching, so a `number` column whose values equal option NAMES
passes the compatibility gate — but `migrateCellsToSelectIds` only rewrote
JSONB `string` and `array` cells. Those cells stayed raw numbers inside a
select column, where they render as nothing and fail option membership on the
next write.
`data->>key` yields the text form for every scalar, so the existing lookup
already worked; the predicate was simply too narrow. Widened to cover
`number` and `boolean`. The outbound migration is unchanged — cells leaving a
select column are option ids, always strings or arrays.
Pre-existing on staging (both the resolver's scalar handling and the migration
SQL predate this branch), but it lives in a file this PR creates.
Tests pin the resolver behavior the predicate depends on, so narrowing either
one without the other now fails.
* fix(tables): validate a retype's unique against the values it writes
Validated the last partial-update seam with a focused investigation rather
than assuming. The answer was split.
`required` is already safe: `updateColumnType` runs the same `countEmptyCells`
against the constraint the request is about to set, which is why that check
exists.
`unique` was not, and the reachable case commits the unrecoverable half. A
text column holding "5" and "5.0", PATCHed with {type: number, unique: true}:
the conversion succeeds and coerces both to 5, then the separate constraint
write finds duplicates and 400s — with the column already numeric and "5.0"
irreversibly rewritten. A pre-scan of the raw text finds nothing; the
conversion is what manufactures the duplicate. The retype now carries `unique`
and checks it after the write-back, against the values it just wrote.
Constraint changes on a workflow-output column were the same shape — rejected
by the constraint write, after a type change had committed. Now rejected in the
route's pre-flight block, before any write.
The duplicate scan is extracted and shared between both paths for the same
reason `countEmptyCells` is: two copies of one rule is the drift that produced
the original required-check bug.
Deliberately NOT merging `updateColumnType` and `updateColumnConstraints`. They
assert different lock levels (destructive vs schema-only) and only the retype
needs the full row scan, so merging would either force a constraints-only
toggle to materialize every row or reintroduce the branching it was meant to
remove. With both reachable failures pre-validated, what remains at the seam is
concurrent races no in-process check can close.
* fix(tables): don't drop a rename when the write it rides on no-ops
Found by Cursor Bugbot — a bug I introduced folding the rename in.
`updateColumnCurrency` returns early when the code is unchanged, and that
return sat ahead of the rename, so PATCH {name, currencyCode} with the column's
current code answered 200 with the rename silently discarded.
Both early returns now treat a pending rename as work: the currency path only
no-ops when the code is unchanged AND no rename is riding along, and the retype
path applies a rename-only write when the type is unchanged. `applyPendingRename`
signals "nothing to do" by returning the same reference, which is what lets
both detect it cleanly.
Also extracts `persistColumns` — five sites were repeating the same
schema-write-and-return.
* fix(tables): make a combined column PATCH a single transaction
Finishes the fold-in rather than pre-validating around the seam. A retype now
APPLIES the constraints it already validates against — it checks empty cells for
`required` and post-conversion duplicates for `unique`, so it was doing the work
without persisting the result — and the route skips the separate constraint
write when the type changed.
A request combining a rename, a retype and constraint changes is now one
locked transaction: no half of it can commit while another fails. The separate
constraint write remains for requests that do not change type, which is the
only case that still needs it.
Deliberately still NOT merging the two service functions. They assert different
lock levels (destructive vs schema-only) and only the retype needs the full row
scan into memory, so a merged function would force a constraints-only toggle to
materialize every row or reintroduce the branching it was meant to remove.
Folding the payload in gets atomicity without either cost.
* fix(tables): reject a flattened list as an amount; fold constraints into every typed write
Two findings from round 11.
Multi-select converted to nonsense amounts. `selectValueForConversion`
flattens a multi cell to its comma-joined option names, and the parser read
that as a formatted number: options 12 and 34 became 12.34, and 100 and 200
became 100200. No real amount puts whitespace after a separator, but a
delimited list does — so a separator followed by whitespace is now refused.
Every legitimate form still parses, including space-grouped locales.
Combined options-or-currency + constraints could still commit partially. Those
two writes now carry constraints the same way the retype does, through one
shared `applyConstraints` that validates (workflow-output, empty cells for
required, supportsUnique and duplicates for unique) and applies them. The
separate constraint write now runs only when no typed write does. Three copies
of those rules is the drift that produced the original required-check bug, so
they live in one place.
* fix(tables): validate constraints after the migrations that rewrite cells
Self-caught while reviewing my own previous commit, which introduced both.
`updateColumnOptions` ran the shared `applyConstraints` BEFORE its cell
migrations. Those migrations rewrite stored values — a single<->multi toggle
changes the shape, removing an option clears cells — so a `unique` scan read
the pre-migration values, passed, and the rewrite could then produce the
duplicates the scan was meant to prevent. Moved to after the migrations, which
is where `updateColumnType` already had it.
The same commit also left the options path running `required`'s empty-cell
check twice: once in the shared helper and once in its original inline block,
whose comment still described a separate constraint write that no longer runs.
Removed the duplicate — one query, one rule, which is the whole point of the
shared helper.
Also routes the options path through `persistColumns` like the others.
* fix(tables): stop inventing amounts from identifiers; fix the copilot retype
Adversarial pass over the final state, seven real findings.
Two destroyed data. The copilot `update_column` still used the two-transaction
pattern the HTTP routes were fixed for: `unique` was never forwarded to the
typed write, so a retype+unique committed the conversion and then failed the
constraint — the same irrecoverable half. It now rides the typed write, and the
separate constraint write only runs when no typed write did.
And the parser's three-letter strip removed ANY three letters, not an ISO code:
`SKU400` parsed as 400, `ABC1234` as 1234. Converting a column of part numbers
to currency rewrote every cell with an invented value — while the comment two
lines above claimed a SKU was exactly what it prevented. The rule is now that a
letter touching a digit means identifier, not amount; a currency marker is
always separated by a space or a symbol.
That same change fixed a class the review surfaced: the pinned currencies could
not parse their own conventional notation. `R$ 1.234,56`, `1 234,56 kr`,
`1234,56 zł`, `CHF 1’234.56` and Indian lakh grouping (`₹12,34,567.89`) all
work now — these are what Intl emits, so a paste from a spreadsheet was being
rejected.
`updateColumnConstraints` was a fourth copy of the constraint rules the shared
helper exists to unify, and had already drifted: it hardcoded `type ===
'select'` where the helper asks the registry, so a future type declaring
`supportsUnique: false` would have been ignored on that path. It now uses the
helper.
`updateColumnType`'s unchanged-type early return silently discarded every
field except the rename. Callers gate on the type changing, but from a read
taken before the lock — so a concurrent change could land there with real work
pending and answer success. It now throws.
Also: `UpdateColumnCurrencyData` was missing `required`, which only compiled
because the routes pass it through a spread; a missing column returns 404
instead of a 400 reading "of type undefined"; and the comments describing the
old two-transaction architecture are gone.
Verified NOT a bug: CSV export of a currency column writes the raw number, so
export/import round-trips losslessly.
* fix(tables): read the negative and RTL forms Intl actually emits
An Intl sweep across 24 locales found two forms the parser rejected, both from
an ordinary spreadsheet paste.
`Intl` emits U+2212 MINUS SIGN rather than the ASCII hyphen for negatives in
several locales, so `−12,50 kr` read as null instead of -12.5. And it wraps
RTL-locale output in invisible bidi control marks, so `1,234.56 ₪` carried
characters that are not part of the amount. Both are now normalized away.
24 locales x 6 amounts now round-trip, up from 99/100 when the sweep started —
and the test generates them from `Intl` rather than listing them by hand, so a
parser change cannot quietly regress a locale nobody remembered to write down.
Locales that format with their own numeral systems (Arabic-Indic) are still
rejected, and now say so in the docstring. That is a safe failure — null rather
than a wrong value — and supporting them is a wider decision than this type,
since it would also touch `number`, display, and sorting.
|
||
|
|
7798e83489 |
feat(function): custom sandboxes (#6071)
* feat(sandboxes): workspace dependency sets for Function blocks Named package sets a Function block can import from. The server canonicalizes and hashes the list; E2B prebuilds a content-addressed template per set, Daytona installs per execution. Create/edit is gated to Max or Enterprise via the shared workspace entitlement check; execution is deliberately ungated, so a downgraded workspace keeps running what it already built. Also on this branch: - Extract the duplicated dropdown/combobox option-fetch lifecycle into use-fetched-options. Only combobox had the dependency-change reset, so every dropdown with dependsOn + fetchOptions cleared its list and never repopulated until reopened. - Collapse the repeated Max-tier entitlement check onto one hasMaxTierWorkspaceAccess, shared by inbox, live sync, and sandboxes. - Resolve a personal payer's block state through getEffectiveBillingStatus in getBillingEntityBlockStatus, so the client-side Max gates agree with the server-side ones when blockOrgMembers' fan-out is stale. - Carve the Daytona dependency install out of the caller's execution budget instead of stacking on top of it. Co-Authored-By: Claude <noreply@anthropic.com> * chore(db): regenerate the sandboxes migration as 0273 Staging claimed 0271 and 0272 while this branch was out, so the hand-authored 0271_workspace_sandboxes was dropped before the merge and regenerated on top of the merged schema. Same DDL; drizzle emits plain CREATE TABLE/INDEX rather than the hand-added IF NOT EXISTS, which matches the repo default — that idempotent form is only needed for files with CONCURRENTLY ops below an embedded COMMIT. Regenerating also restores the meta snapshot the hand-authored migration never had. Co-Authored-By: Claude <noreply@anthropic.com> * chore(db): drop the sandboxes migration ahead of the staging merge Staging independently claims idx 0273, so remove ours before merging to avoid an add/add conflict on the drizzle migration index. Regenerated at the next free index once the merge lands. Co-Authored-By: Claude <noreply@anthropic.com> * chore(db): regenerate the sandboxes migration as 0275 Staging took 0273 and 0274, so the sandboxes DDL lands at the next free index. The emitted SQL is byte-identical to the dropped 0273. Co-Authored-By: Claude <noreply@anthropic.com> * fix(billing): consolidate the Max-tier entitlement onto one predicate The Max tier was spelled five ways. The odd one out — `isMax`, defined as `isPro(plan) && credits >= 25000` — excluded both `team_25000` and `enterprise`, and it was the sole input to the personal-workspace cap. A delinquent Max-for-Teams org admin got 1 personal workspace while a delinquent Max individual got 10. Only free/pro_6000/pro_25000 were tested, so the two broken tiers were unpinned. Separately, the server gate and the client `hasUsableMaxAccess` were independent copies of the same rule. The settings sidebar renders Sandboxes and Sim Mailer from the client one while the API answers 403 from the server one, so any drift renders a feature unlocked that the API refuses. - `MAX_TIER_CREDITS` is derived from the `CREDIT_TIERS` table; `isMaxTier` in plan-helpers is now the single definition, shared by the server gates, the client derivation, `getPlanTypeForLimits`, `plan-view`, and the cap - `hasWorkspaceTierAccess(id, predicate, { intent, onMissingWorkspace })` becomes the one org-vs-personal payer fork. `intent: 'active-use'` means active and not billing-blocked; `'retention'` means active/past_due with block state ignored, so the inbox teardown guard keeps its fail-open semantics instead of implying them through a duplicated fork - `isWorkspaceOnEnterprisePlan`'s personal branch now applies the status and block checks its own org branch always had, and its TSDoc names its real consumer (copilot BYOK, not Access Control) - the client live-sync gate gained the server's `isHosted` branch, so a self-hosted deploy with billing on no longer locks an interval the API accepts. It reads both flags directly rather than taking one as a parameter the callers sourced from the same module - `sqlIsPro`/`sqlIsTeam` escape the `_` LIKE wildcard, matching the already correct hand-rolled filter in seat-drift - deletes the `TERMINAL_SUBSCRIPTION_STATUSES` and `ENTITLED_STATUSES` shadow constants, and corrects three test mocks that asserted `trialing` was entitled or usable `max-tier-parity.test.ts` asserts the client and server answers match for every plan name. Both new guards were checked against the old code: the parity test fails 3 assertions with the previous predicate, and the self-hosted test fails without the `isHosted` branch. Co-Authored-By: Claude <noreply@anthropic.com> * chore(db): drop the sandboxes migration ahead of the staging merge Staging has claimed 0275 (table_views) and 0276 (drop_legacy_folder_tables) since the last merge, so our 0275_workspace_sandboxes collides on the index. Dropping ours first — the .sql, meta/0275_snapshot.json, and the journal entry — leaves packages/db/migrations byte-identical to the merge-base, so the merge sees no add/add conflict at all. Regenerated on the far side. Ours is the droppable side: plain additive DDL with no hand edits, which drizzle reproduces exactly. Staging's migrations are hand-written and must survive. Co-Authored-By: Claude <noreply@anthropic.com> * chore(db): regenerate the sandboxes migration as 0277 Staging claimed 0275 (table_views) and 0276 (drop_legacy_folder_tables), so the sandboxes migration dropped before the merge comes back on top as 0277. The emitted SQL is byte-identical to what was dropped — the original had no hand edits, so there is nothing to reapply. It is purely additive: two enums, sandbox_image and workspace_sandbox, their two FKs and six indexes. That it regenerated unchanged also confirms the schema.ts auto-merge was correct — had it lost staging's legacy-folder-table drops, drizzle would have emitted CREATE TABLE for them here. Snapshot chain is continuous (0273 -> 0277, each prevId matching the previous id) and the table counts track the DDL: 100 -> 101 (table_views) -> 99 (legacy folder tables dropped) -> 101 (the two sandbox tables). Co-Authored-By: Claude <noreply@anthropic.com> * feat(sandboxes): gate on the enterprise feature flags, drop the rollout switch Sandboxes shipped behind `custom-sandboxes`, an AppConfig rollout flag falling back to a `CUSTOM_SANDBOXES` secret. That made it the only Max-gated surface with no self-hosted path: `INBOX_ENABLED` can force Sim Mailer on for an operator running their own billing, and `ENTERPRISE_ENABLED` turns on the other nine features at once, but neither reached sandboxes. A self-hoster had to find a separately-named variable that was not part of that family, and one running with billing enabled could not enable it at all. Sandboxes now joins the enterprise feature set and the rollout flag is gone: - `sandboxes` is an `EnterpriseFeature` with `SANDBOXES_ENABLED` and its `NEXT_PUBLIC_` twin, so the master switch and the per-feature override both reach it like every sibling - `hasWorkspaceSandboxAccess` takes the inbox's shape exactly — the override wins, then a deployment without billing is unrestricted, then the workspace payer needs usable Max or Enterprise - the settings nav gains `selfHostedOverride`, so the section resolves through the same path as Sim Mailer instead of a second entitlement AND-ed in - `custom-sandboxes`, the `CUSTOM_SANDBOXES` secret, the now-unreachable `SANDBOXES_UNAVAILABLE` 403 copy, and the route's kill-switch branch are deleted Its legacy default is `true`, matching `inbox`: the gate already returns true whenever billing is off, so `false` would leave the nav override disagreeing with the gate that answers the request. Self-hosted builds run on the operator's own E2B/Daytona credentials, so there is no Sim-side cost to withhold — the docs now say so, since enabling the feature without a provider configured is the obvious trap. The new gate tests run with billing enabled on purpose; the `!isBillingEnabled` bail would otherwise answer every case and hide whether the override is wired. Verified by deleting the override line — exactly the one assertion fails. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): let the language menu match its trigger width `matchTriggerWidth={false}` exists for the opposite case — a narrow trigger whose option labels would truncate, letting the menu grow past it. The language field is a full-width form control with two short labels, so the override shrank the menu to "JavaScript" and pinned it to the right edge instead. The default (`true`) is correct here. Every other consumer passing `false` is a genuinely narrow trigger — a role picker in a member row, a table filter chip. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): re-queue a build when resolution finds the image unusable `ensureSandboxImage` only ran when a sandbox was saved, so resolution treated an unusable image as terminal and told the user to go fix a definition that was never wrong. Three states stuck permanently until someone re-saved in Settings: - a build that failed - a build whose worker died mid-flight, stranding the row in `building` - every sandbox created while the deployment ran a `runtime` provider, after a switch to a `prebuilt` one — `runtime` writes no image rows at all, so the whole fleet resolved to "no completed build" with nothing to repair it Resolution now re-queues through the registry's existing idempotent entry point before failing, and says a build is on its way instead of pointing at Settings. The conflict guard already claims only a `failed` row or a stale `pending`/ `building` one, so executions arriving during a healthy build enqueue nothing — no thundering herd from a hot workflow. The registry is imported dynamically for the same reason `sandboxDb` is: it pulls `@sim/db` into the static graph, which this module keeps out of the executor bundle. That also avoids a cycle, since the registry imports `invalidateSandboxResolution` from here. A repair that itself fails is logged and swallowed — it must never replace the build error naming the sandbox. Verified by deleting the repair call: exactly the three new assertions fail. Co-Authored-By: Claude <noreply@anthropic.com> * improvement(sandboxes): let the picker show just the sandbox name The label read "Test · Python · 1 package". The block's own list is already scoped to the language its sibling `language` subblock selects, so the language repeated on every row said nothing, and the package count is decoration next to the name that identifies the sandbox. The language stays for the one caller that cannot filter — agent tool-input renders this field under a synthetic id where the sibling `language` value is unreachable, so its list spans both languages and the name alone is ambiguous. That is the same missing value which disables filtering, so `showLanguage` is derived from it directly rather than passed independently and left to drift. A failed build is still marked: that suffix is the difference between a selection that runs and one that does not. Passing the flag also means dropping `.map(toSandboxOption)` for an explicit arrow — `Array.map` hands the index to the second parameter. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): show the sandbox name on the block card, not its uuid The card printed "443f4934-26ab-44ab-8...". `resolveDropdownLabel` only reads a subblock's static `options` array, and the sandbox picker is a `combobox` whose options load asynchronously, so its array is empty and the raw stored id fell through to the label. Resolved the same way skills and tools already are: a `resolveSandboxLabel` in the display layer, fed from the shared sandbox list query — the same cache entry the picker reads, so this adds no request. Two deliberate scopings: - the query is subscribed only for the sandbox row. `SubBlockRow` is memoized per subblock, and the list query polls while a build is in flight, so an unconditional hook would re-render every row on the canvas on each poll tick - the resolver matches the field id, not just the type. There is no dedicated subblock type for it, and matching `combobox` alone would relabel unrelated pickers An id with no matching sandbox resolves to null rather than a guess, so a deleted sandbox falls through to the caller's placeholder. The template preview surface is left alone: it is explicitly hook-free and passes empty lists for tools and skills too. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): hide the Sandboxes section with no provider configured Entitlement decides whether a workspace may author sandboxes; nothing decided whether anything could run one. A self-hosted deployment with SANDBOXES_ENABLED but no E2B or Daytona credentials got a fully functional tab whose output no Function block could select — the picker is gated on the provider vars, the tab was not. Both navigation planes now drop the section when neither NEXT_PUBLIC_SANDBOX_ENABLED nor the pre-Daytona NEXT_PUBLIC_E2B_ENABLED is set — the same pair the picker's `showWhenEnvSet` reads, so the two cannot disagree. Dropped rather than locked: an upgrade does not conjure a provider. The unified plane drops it in `buildUnifiedSettingsNavigation` rather than in the sidebar's filter, because the sidebar's `selfHostedOverride` short-circuit runs before its `requiresMax` check and would have revealed the tab anyway. It reads the browser twins, not the server's `isRemoteSandboxEnabled`, since this module renders on both sides. The predicate is a function, not a module constant, because the constant form was untestable and ambient: the env mock falls through to `process.env`, and `apps/sim/.env` (gitignored, so absent on CI) sets NEXT_PUBLIC_E2B_ENABLED=true. The nav tests passed locally and failed 6 assertions with the flag cleared. They now pin both flags, so the suite is identical with and without a local env file — verified by running it both ways. Co-Authored-By: Claude <noreply@anthropic.com> * docs(sandboxes): correct three claims the code no longer makes The Sandboxes section described behavior two commits on this branch changed, and led with an internal detail no reader needs. - entitlement is no longer Max/Enterprise only: self-hosted deployments unlock sandboxes with SANDBOXES_ENABLED, and the section is hidden outright when a deployment has no sandbox provider, which is the state a self-hoster is most likely to hit and least likely to diagnose - a build that is not Ready is no longer terminal. It is queued again on the next run, so the advice is to wait and re-run, not to go edit a package list that was never wrong - deleting a sandbox frees its build once nothing else references it. Builds are shared by content, so this is the one place a reader could reasonably assume deletion is immediate Dropped the `ModuleNotFoundError` aside: what the old code did instead is not something a reader needs to know to use the feature. The page is hand-written — `function` has category 'blocks' and is absent from `NATIVE_RESOURCE_BLOCK_TYPES`, so generate-docs skips it and these edits will not be overwritten. Co-Authored-By: Claude <noreply@anthropic.com> * feat(sandboxes): release the provider image when nothing references it Deleting a sandbox only removed its row, leaving the built template in E2B until the 30-day retention sweep — up to a month of paying to store an image nothing could select. Editing a package list had the same effect on the old content address, which is the more common case since every edit re-points the sandbox. `releaseSandboxImage(specHash)` now deletes the provider image and its row from both paths. It reuses the sweep's provider call and its ordering: image first, row second, so a refused delete leaves the row for the sweep to retry rather than orphaning a remote template nothing points at. Two guards make eager deletion safe: - builds are keyed by content, not by workspace, so two workspaces declaring the same package list share one image. The release no-ops while any sandbox still references the hash — otherwise one workspace's delete would break the other's - an in-flight build is left alone rather than raced; the sweep collects it once it settles Called detached from both routes. The row is already committed by then, so the user's action has succeeded whatever the provider says, and awaiting would hold a UI delete open on a remote call the sweep would retry anyway. Every failure inside is logged and swallowed for the same reason. E2B's delete verified against their API reference: DELETE /templates/{templateID} with X-API-Key, 204 on success. The existing implementation already matched, so this commit only adds the call sites and the guards. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): rate-limit the automatic rebuild, drop the one-off status dot Two follow-ups to the resolution repair. The repair had no rate limit. `ensureSandboxImage` re-claims a `failed` row on sight, and a bad package name fails in seconds, so the in-flight guard never closed the window: a workflow on a one-minute schedule would enqueue a build a minute against a package list that will never resolve, each one real provider build compute. Before the repair existed resolution simply threw, so this was introduced with it. The two callers want different things, so the cooldown is opt-in. A save is a person explicitly asking for another attempt and still retries immediately; resolution passes `FAILED_BUILD_RETRY_COOLDOWN_MS` and gets at most one attempt per window no matter how often the workflow runs. Ten minutes: long enough that per-minute runs cannot drive per-minute builds, short enough that a transient registry outage clears within the hour. The status line loses its colour dot. `size-[6px] rounded-full` appeared in exactly one file in the repo, so it was a new primitive rather than a pattern, and it duplicated state the text colour already carries — the label now turns `--text-error` on a failed build, which is what every other status row in settings does. `ChipTag` was the wrong home for this: its variants are `mono`/`invite`, with no semantic tone, so a status version would have meant overriding its chrome from the consumer. Also corrects the docs line this changes: a failed build is retried periodically, and saving is the way to retry now, so "wait a moment and run again" no longer describes it. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): claim the image row and its reference check in one statement Greptile P1. Reading references in one statement and deleting in another left a window — a wide one, since a provider delete is a network call — where a second workspace could declare the same package list, inherit the `ready` row, and have its next run fail against a template already on its way out. Content addressing is what makes that reachable: the image is shared, so one workspace's delete can strand another's sandbox. The reference check now lives in the conditional DELETE itself, so winning the delete is the proof that nothing referenced the hash. A workspace that adopts the hash first makes the delete match nothing and the release becomes a no-op. Claiming the row before the provider call would otherwise strand a template nothing points at if the provider then refused, so that path puts the row back and the retention sweep inherits the retry — the same property the previous ordering had. The sweep is deliberately left as it is: its equivalent window needs a hash unreferenced AND unused for 30 days, and its provider-first ordering encodes the documented retry-on-refusal behaviour this path now reproduces explicitly. No transaction is opened. The provider call sits between discrete statements rather than inside one, so no pooled connection is held across it — which is why this uses a conditional delete instead of the repo's `pg_advisory_xact_lock` pattern, whose lock only releases at commit. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): route the retention sweep through the same image claim Cursor and Greptile both flagged the sweep as still carrying the interleaving just fixed in releaseSandboxImage, and they are right — the reason given for leaving it alone last round does not survive scrutiny. That reason was that provider-first ordering encodes retry-on-refusal, so making the claim atomic would trade a race for an orphaned template. The release path already answers that: claim the row, and put it back if the provider refuses. The sweep can have both properties too. The rarity argument was also weaker than stated. The sweep nominates up to 200 candidates and then works through them eight network deletes at a time, so its check-to-delete gap is seconds to minutes — wider than the window that was just closed, not narrower. Both callers now share `claimAndDeleteImage`, which owns the whole contract: the unreferenced check lives inside the DELETE, the provider call runs only after the claim succeeds, and a refusal restores the row. Having written that ordering twice is what let the two paths drift, so it exists once now. The sweep's query becomes a nomination step only. Its retention cutoff is passed into the claim rather than trusted from the earlier read, so a candidate that stops qualifying mid-sweep fails its claim and is skipped instead of losing its image. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): rebuild a hash adopted while its image was being deleted Greptile's third pass on this path, and a case the previous two did not cover: the adopter starting a *fresh build* rather than inheriting a ready row. Claiming removes the registry row, so between that and the provider delete finishing, a workspace can declare the same package list, get a new row, and start a build under the same content-derived imageRef — which the in-flight delete then removes. The window itself is inherent. The registry row and the provider template are two systems with no shared transaction, so it can be narrowed but not closed. A Redis lock would not close it either: acquireLock returns true when Redis is absent, so it cannot be a correctness guarantee for self-hosted. Holding a Postgres advisory lock would, but only by pinning a pooled connection for the length of a provider call, which is a worse trade. What was avoidable is the adopter finding out the slow way. Its row is new and healthy-looking, so nothing noticed: resolution only repairs a row that is missing or failed, and a failed one waits out the retry cooldown first. The release path now re-checks after the delete and re-enqueues, so the rebuild starts immediately instead of one failed run plus a cooldown later. A build already in flight is left to the conflict guard, since it may still outlive the delete. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): reclaim a ready row whose image was deleted underneath it Greptile found the hole the previous commit left, and it is the case that made the claim in that commit's message wrong: this one is permanent, not transient. If a re-adopted hash reaches `ready` before the in-flight provider delete lands — plausible, since E2B layer caching can rebuild an identical spec in seconds — the row looks healthy while its imageRef points at nothing. Resolution repairs a row that is missing or failed, never one claiming to be ready, so nothing recovers it. The sandbox stays broken until someone re-saves it by hand. `rebuildIfReadopted` called `ensureSandboxImage` with no options, whose conflict guard reclaims only a failed or stale in-flight row, so it silently did nothing in exactly that case. The release path now passes `imageKnownGone`, which widens the re-claim to any settled row rather than only a failed one. It is the one caller that knows the image is gone regardless of what the row says. An in-flight build is still left alone: it either recreates the template it was building or fails into the normal repair path, and resetting it would only add a duplicate build. The three ways a settled row may be re-claimed now sit in one `settledRebuildBranch` helper — any settled row when the image is known gone, a failed one after the cooldown for an automatic caller, a failed one immediately for a person — because inlining the third case is what hid the gap. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): let a same-spec save retry a failed build Cursor Bugbot. `scheduleSandboxBuild` sat inside the changed-hash branch, so a save that did not alter the package list never reached the registry. The comment above it described the opposite — that an unchanged spec finds a ready row and enqueues nothing — which is what `ensureSandboxImage` does, but only if it is called. That made the docs wrong too. They tell a reader to save the sandbox again to retry a failed build immediately, and this branch is exactly why that did nothing: the only way to retry was to edit the package list into a different hash, which is not what someone recovering from a transient registry failure wants to do. The call is now unconditional and the registry decides what a save costs, which is what its conflict guard is for: a ready or in-flight row is left alone, a failed one is re-claimed at once. Releasing the previous image stays behind the hash check, since only a changed hash orphans one. Cache invalidation is unchanged — `scheduleSandboxBuild` already does it, which is why the else branch existed. Co-Authored-By: Claude <noreply@anthropic.com> * docs(sandboxes): correct the image cache's staleness invariant Cursor Bugbot found that a released image can still be served from another replica's cache. The finding is real, and the reason it went unnoticed is that the cache documented an invariant which eager release quietly broke. It claimed a `ready` row is terminal for its spec hash, so a cached hit could not go stale in a way that matters. That held while the only ways a row changed were an edit (new hash) or a delete (caught by the `workspace_sandbox` read). Releasing an image eagerly made a `ready` row disappear with the hash unchanged, so the premise no longer holds and the comment was actively misleading to the next reader. No behaviour change here — the exposure is bounded at IMAGE_TTL_MS on replicas other than the one that ran the release, and it self-heals once the entry expires and the row read finds nothing. Closing it properly needs cross-replica invalidation or a provider-error path that invalidates on "template not found", both of which are larger than a review fix; the comment now says so instead of implying the problem cannot exist. Co-Authored-By: Claude <noreply@anthropic.com> * docs(sandboxes): note that a JavaScript sandbox needs an import to apply Cursor Bugbot pointed out that `useRemoteSandbox` keys on detected static import/require and never on the selected sandbox, so JavaScript without one runs locally and the selection has no effect. Keeping the behaviour: honouring the selection would force those blocks remote, and the large-value-ref guard immediately below would then reject code that runs fine today. Documenting it instead, next to the picker, since a selection that silently does nothing is only surprising if nothing says so. Python is unaffected — it always runs remotely, so its sandbox always applies. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): stop create mode surviving a return to an open sandbox Cursor Bugbot. Create mode and having a sandbox open are mutually exclusive, but nothing enforced it, so both could be set at once — and the screen then lied about which sandbox its Delete pointed at. With `isCreating` true and `selectedId` restored, `baseline` is null, so the editor renders an empty "New sandbox" form, while the Delete action is built from `selected` and still targets the restored sandbox. An admin looking at a blank create form could delete a sandbox it never named. Two ways in, both closed: - Browser Forward after starting a new sandbox restores `selectedId` without going through `closeEditor`. The render-time sync that already drops a stale draft now also leaves create mode, which is the same class of correction and the reason that block exists. - "New sandbox" set `isCreating` without clearing `selectedId`, so the same contradiction was reachable without touching history at all. It now clears the selection, with `history: 'replace'` because switching mode is not a destination. Co-Authored-By: Claude <noreply@anthropic.com> * fix(ci): pin the sandbox flag in the second nav catalog test, bump the chart Two CI failures, both mine. `app/workspace/[workspaceId]/settings/navigation.test.ts` asserts the unified catalog and was left on ambient env. Dropping the Sandboxes section without a sandbox provider made it 26 items instead of 27 on CI, which has no `apps/sim/.env` — the same trap already fixed in the sibling `components/settings/navigation.test.ts`, in the one file that was missed. Fixing it needs `vi.hoisted` rather than the sibling's `beforeEach`, because this file reads `allNavigationItems`, built once at module load; a hook would run after the value it is trying to influence already exists. The chart gate is separate: this branch adds sandbox settings to `helm/sim/values.yaml`, and the workflow requires a Chart.yaml bump whenever `helm/sim/**` changes. Additive config, so 1.3.0 -> 1.4.0 by SemVer. Verified by running the whole suite with the flags forced off, not just the two navigation files — no other test depends on a local env file. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): keep the row restore to a refused delete only Cursor and Greptile, independently, on the same code. `deleteImage` and `rebuildIfReadopted` shared one try/catch, so a rebuild failure after a *successful* provider delete was handled as if the provider had refused: the catch put the claimed row back, `ready` status and all, pointing at a template that no longer exists. That is the one state resolution cannot repair — it fixes a row that is missing or failed, never one claiming to be ready — so it reintroduced the permanent breakage an earlier commit had just closed, through the error path rather than the happy one. Restoring now belongs strictly to a refused delete. Once the template is gone the row stays gone, and the rebuild runs past that catch. The rebuild also swallows its own failures: it follows a delete that already succeeded, so it must not be reported as a failed release, and inside the sweep it must not reject the rest of its chunk. The adopter's next run still reaches the normal repair path. The regression test drives a rebuild failure and asserts no row is restored. It fails against the original shape — rebuild inside the shared try, no inner catch — which is what the two reviewers were describing. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): drop the dead row when a re-adopt rebuild cannot be scheduled Greptile, one layer under the previous fix. Making the post-delete rebuild swallow its own failures kept it from being reported as a failed release, but left the adopter's row claiming a `ready` image whose template is already deleted — the one state resolution cannot repair, since it rebuilds a row that is missing or failed and never one that says ready. So the row is now dropped when the rebuild does not take. That turns the adopter into the missing-row case, which the next execution repairs on its own, instead of a sandbox that stays broken until someone re-saves it by hand. A failure to drop it is logged at error, because at that point two writes in a row have failed and there is nothing further this path can do. Also gives the release tests a default "nothing re-adopted" select. Without it the rebuild threw on an unstubbed mock and the cleanup delete overwrote the predicate the claim assertions read, so two of them were passing on the wrong statement. Co-Authored-By: Claude <noreply@anthropic.com> * feat(sandboxes): repair a missing image at create, where the truth is observable Six review rounds narrowed the window between deleting a shared template and another workspace adopting its content hash, and each fix exposed the next facet. They all share a cause: the registry row and the provider template are two systems with no shared transaction, so any scheme that keeps them in step is guessing. Create is the one step that does not have to guess. It either gets a sandbox or it does not, so a `ready` row pointing at a deleted template now corrects itself the first time it is used, rather than needing someone to re-save the sandbox. - `SandboxImageBuilder.isMissingImage` asks the provider to classify its own failure. Prebuilt-only, because a runtime provider has no image to miss - E2B answers it off `NotFoundError`, which the SDK maps from a 404. The only resource a create names is the template, and the two subclasses that describe other calls — a missing file, an exited sandbox — are excluded. The classifier stays deliberately narrow: treating auth or rate-limit failures as a missing image would turn a provider outage into a build storm - `repairMissingSandboxImage` invalidates the cache, rebuilds with `imageKnownGone` (no cooldown, since this observed the image is gone rather than inferring it), and returns copy telling the author to run again - `ResolvedSandbox` carries `specHash` so the failing execution can name what to rebuild This subsumes the open facets rather than adding another guard beside them: the stale per-replica cache, an adopter left `ready` against a deleted ref, and a rebuild that never took all end at the same place — the next run repairs itself. Co-Authored-By: Claude <noreply@anthropic.com> * fix(sandboxes): key the build trigger by attempt, not by spec Cursor Bugbot. The Trigger.dev idempotency key was the content address alone, so a second attempt at the same spec was deduped against the first: the SDK returns the finished run instead of starting one, and the row that `ensureSandboxImage` just flipped to `pending` sits there with no worker. Nothing can re-claim a `pending` row until it goes stale, so a retry inside the 5-minute TTL did nothing for the next half hour. That silently disabled every repair path — save-to-retry, which the docs name explicitly, and both the resolution and create-time rebuilds. The key's own comment already said it exists "to collapse concurrent saves of the same spec into one build, not to suppress a retry after one failed". The conditional update above it is what actually collapses concurrent saves: only one caller gets a row back, so only one ever reaches the trigger. Keying by the claim's `updatedAt` keeps that property and makes each genuine attempt distinct, while a duplicate delivery of one attempt still collapses. Co-Authored-By: Claude <noreply@anthropic.com> * feat(sandboxes): create a sandbox from the picker, and fix three UI papercuts The Function block's sandbox field now pins a "Create Sandbox" row above its options, matching the "Create Skill" / "Create Tool" rows it sits beside, so authoring a package list no longer means leaving the workflow for Settings. The row is declared by the field (`createAction`) rather than hardcoded by id; block configs are read by the serializer and executor, so the name maps to a modal in the picker rather than carrying a component. Two things the modal has to get right. It seeds the new sandbox's language from the sibling the list is scoped by, or a sandbox created off a JavaScript block would land in the Python list and vanish. And the created option is held locally until a real fetch carries it, or the field would sit on a raw uuid until hydration answered. Also: - The Sandboxes icon was the Logs block's icon (`blocks/blocks/logs.ts`), in both the settings nav and the list rows. It is the Function block's now. - "Default image (no extra packages)" claimed something untrue: E2B and Daytona base images both ship with packages installed. - A new sandbox opened in Python while the Function block defaults to JavaScript. The test pins the two together rather than the literal. Draft shape and helpers moved out of the editor component into `utils.ts` — three consumers now, and it makes the defaults testable without a DOM. * feat(settings): one Max-plan wall, and give the create modal the same one The create-sandbox modal answered a non-Max workspace with a red line under a form it could never submit, and no way to act on it. It now renders the same wall the Settings > Sandboxes tab does — heading, one sentence on what the plan unlocks, and an Upgrade to Max chip — instead of the fields. That wall existed twice already (sandboxes and Sim Mailer), so this extracts it rather than adding a third copy. `SettingsUpgradeNotice` owns the copy rhythm and the route, and `compact` trades the page's full-height centering for a modal's. Both settings consumers now compose it; neither keeps its own markup. The action lands on billing, which `resolveSettingsHref` already redirects to the plan-comparison page for a member who cannot manage billing — so it is a route to explore plans, never a dead end. The chip stays hidden for non-admins, exactly as the settings pages had it. A non-admin on an entitled workspace gets the muted reason rather than the upgrade wall: buying a plan is not what is in their way. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
811a39ec05 |
fix(integrations): show family service accounts on every product they authenticate (#6102)
* fix(integrations): show family service accounts on every product they authenticate
An Atlassian API token authenticates Jira, Jira Service Management, and
Confluence alike, so it is modeled as an `atlassian` pseudo-provider whose
only service is named "Atlassian Service Account". Every credential display
surface resolved through `getServiceConfigByProviderId`, which walks
OAUTH_PROVIDERS in declaration order — so the credential resolved to that
pseudo-service instead of to any product.
The result: adding a service account from the Jira page, through a modal
titled "Add Jira service account", produced a credential that appeared under
neither Jira, JSM, nor Confluence, was titled "Atlassian Service Account" on
its detail page, and lost its brand tile and category on the list. The same
bug hid a Google service account everywhere except Gmail.
- match credentials with `credentialProviderMatchesService`, which accepts a
service's OAuth id or its service-account id
- add `lib/integrations/credential-display.ts` as the single resolver for
catalog join, mark, and copy, replacing three duplicated lookups that keyed
the catalog by OAuth service *display name* — the reason the pseudo-service
fell off the map
- derive "family service account" from the catalog (a service-account id
serving >1 integration) rather than hardcoding vendors, so a new integration
joining a family needs no edit
- title service-account detail pages by credential name, subtitle them with
their reach, and state that reach up front on the connect form
- keep the service description as the detail subtitle for every non-family
credential, unchanged
No schema, migration, contract, or persisted value changes; resolution is
computed at render time from static config. Coverage for all 22 service-account
provider ids is pinned in tests, including that the index and the predicate the
Connected list filters on cannot drift apart.
* chore(icons): use Atlassian's gradient marks for Jira and Confluence
Replaces the flat #1868DB Jira and Confluence marks with Atlassian's gradient
versions, matching the Atlassian mark added alongside them.
- gradient ids go through `useId()` rather than the source SVGs' static ids,
which would collide wherever two of these icons render on one page — the
integrations list and the landing loops both do
- pads the Atlassian viewBox so its artwork fills ~78% of the box, matching the
inset Atlassian ships on the Jira and Confluence marks; without it the mark
renders ~30% heavier than its siblings in the same tile
Visual-only, but these marks render in ~60 files, so it is split from the
credential fix to stay independently revertable.
* fix(integrations): route the editor's service-account setup modal through the shared target
The workflow editor's credential selector passed the OAuth service's own name
and icon straight to ConnectServiceAccountModal, so opening the setup form from
a Jira block titled it "Add Jira service account" while the integrations page
and the chat — both of which already resolve through
`useServiceAccountConnectTarget` — titled the same form "Add Atlassian service
account".
That is the exact confusion this branch set out to remove, surviving on the one
surface that bypassed the shared resolver.
* docs(atlassian): correct the service-account setup path and cover all three products
The setup section could not be followed. It sent readers to a "Settings →
Integrations tab" that does not exist (Integrations is a top-level workspace
module) and told them to search the integrations list for "Atlassian Service
Account", which matches no catalog entry — the catalog lists Jira, Jira Service
Management, and Confluence.
The page also described the credential as covering "Jira and Confluence" while
listing Jira Service Management scopes, and the product now spells the coverage
out in the connect form.
- correct the path: Integrations -> Jira/JSM/Confluence -> Add to Sim -> Add
service account
- name all three products consistently, and state that one service account
covers them
- match the real button label ("Add service account")
|
||
|
|
11c0d3b75d |
feat(organizations): sweep a joiner's owned workspaces into the org on join, disclose it at accept, and add external workspace invites (#5918)
* feat(invites): explicit external members
* update docs
* fix(organizations): atomic admin workspace sweep, removal-impact status in dialog, and preview-unavailable disclosure
Review round 1: the v1 admin add-member now commits membership and the
workspace sweep in one transaction; the remove-member dialog holds confirm
while the credential-impact check loads and shows a caution when it fails;
a failed join preview flags joinPreviewUnavailable so the accept screen
falls back to a generic migration notice. Also aligns the invite test's
react-query mock and repairs two pre-existing docs type errors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(organizations): close the concurrent-workspace escape in the join sweep
Personal workspace creation now serializes with organization joins on the
user's billing-identity lock and re-verifies membership inside its
transaction; both join paths (invite acceptance and the v1 admin add)
re-read the owned-workspace set under that lock after the member insert
and roll the whole join back when it diverged from the advisory-lock plan,
so a workspace created mid-join can never land outside the organization.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(organizations): fail stale-grant member joins and re-resolve the creation race client-side
A member-role org acceptance whose grants all turned stale now rolls back
with workspace-not-found instead of stranding a workspace-less member, and
the workspace resolver treats the creation-vs-join 409 as a signal to
re-resolve (the user is authenticated with org workspaces) rather than
falling into the login path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(invitations): mirror the stale-grant gate in the join preview
The accept-screen preview now returns no-join for a member-role org
invite whose grants all left the stamped organization, matching the
acceptance-side rollback so the disclosure never promises a migration
that acceptance would refuse.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(workspaces): survive the join race on lazy default creation and gate removal on live impact data
The workspace list GET now re-lists (returning the join sweep's
workspaces) when lazy default creation loses the race to an organization
join instead of failing with a 500, and the remove-member dialog gates
its confirm on isFetching so a background refetch can never let an admin
confirm against a stale credential-impact list.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(invitations): surface the accept conflict message and refresh workspace caches post-accept
The accept route now carries the human-readable message alongside the
machine-readable error kind (the client prefers it for server-error), and
a successful accept invalidates workspace queries so the swept workspaces
appear immediately instead of after the stale window.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(billing): sweep archived workspaces in Pro-to-Team conversion and org creation
Every attach call site now passes includeArchived so the archived escape
hatch is closed uniformly — join-attach, admin move, subscription-driven
org provisioning, and manual org creation all sweep archived personal
workspaces into the organization.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(invitations): reject acceptance when the sweep set differs from the disclosed set
The join preview now carries the workspace ids it disclosed, the accept
screen echoes them back as a disclosure token, and acceptance rolls back
with disclosure-outdated (409) whenever the set it would sweep no longer
matches — a workspace created after the preview rendered can never move
without the user seeing the refreshed notice first.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(invitations): send the disclosure token for no-join previews too
A preview that predicted no join still tells the user nothing moves — the
empty disclosed set is now echoed on accept, so a join that becomes
possible between preview and accept (left another org, billing turned
usable, grants un-staled) conflicts with disclosure-outdated instead of
sweeping workspaces without a rendered notice.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(invitations): gate all-stale member joins before disclosure and always refetch removal impact
The all-stale check for member-role org invites now runs before any
mutation and before the disclosure comparison, so an invite whose grants
all left the org fails with workspace-not-found instead of trapping
owners of personal workspaces in a disclosure-outdated retry loop. The
removal-impact query drops its stale window (staleTime 0): every dialog
open refetches while the confirm is held on isFetching.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(organizations): keep-external on org recovery and label archived move candidates
Org creation/recovery now uses the keep-external collaborator policy
(matching Pro-to-Team conversion) so different-org collaborators on
archived workspaces cannot abort it with a conflict, and the admin
workspace-move search and preflight expose an archived flag so internal
tooling can label archived targets.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(invitations): guard the reverse disclosure direction
A will-join notice whose acceptance downgrades to no-join (stale
escalation denial, concurrent other-org membership) now fails with
disclosure-outdated instead of silently succeeding as an external grant —
the disclosure token binds the outcome in both directions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* address comments
* fix
* fix(invitations): one invite surface, coalesced grants, coherent seat model
Consolidate the two invite modals into a single surface and fix the
semantics the split had been hiding.
Invite flow:
- One InviteModal for all three entry points (workspace header, workspace
settings, organization settings), with a workspace multi-select and an
explicit Membership choice that states the seat consequence.
- Coalesce grants instead of 500ing. A partial unique index allows one
pending invitation per (email, organization), so inviting someone to a
second workspace raised a raw 23505. New workspaces now merge into the
pending invitation (with a retry for the concurrent-insert race) and the
invitee gets one link covering everything.
- External collaborators require their own paid plan, checked at invite
time and re-checked on accept, since the invitation lives for 7 days.
Imposed externality is exempt in both places: an invitee already in
another organization is forced external regardless of the inviter's
choice, so the plan gate must not apply to them.
- Every invitation must grant at least one workspace, so accepting always
lands somewhere. Enforced at creation for all roles.
- Revocation is grant-scoped. Since one invitation can span workspaces,
revoking from a workspace's member list withdraws only that grant and
cancels the invitation just when the last one goes. Whole-invitation
revocation now requires authority over all of it rather than admin on
any single granted workspace.
- The accept screen names every granted workspace and states whether the
invitee joins as a member, an admin, or an external collaborator, and
whether that uses a seat.
Seats:
- One rule for the pending-invitation predicate, seat capacity, and the
derived figures; the three counting sites now share it.
- Team seats are elastic (subscription.seats tracks the member count), so
treating that as a cap reported negative headroom on any outstanding
invite. Available seats are clamped and gates branch on whether the plan
actually has a fixed cap.
- POST /api/v1/admin/organizations/[id]/members could never succeed on
Team: it validated N members against N seats. It now skips the cap for
elastic plans, matching invitation acceptance, and reconciles seats
after a committed add.
Also surfaces the External label on the workspace Teammates list, which
already received the flag and dropped it, and removes dead code: the
grantless organization-invite route and contract, three unreferenced
invitation helpers, and the unreachable
ensureUserInOrganization/addUserToOrganization/validateMembershipAddition
cluster.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(invitations): close the two accept-disclosure gaps Bugbot found
Membership notice ignored the join preview. `buildMembershipNotice` keyed off
the invitation's sent `membershipIntent`, but acceptance resolves an internal
invite to external when the invitee already belongs to another organization, or
when the granted workspace changed organizations after the invite went out. The
screen therefore promised "you'll join as a member, which uses one of their
seats" to people who would neither join nor consume a seat. It now keys on the
preview's `willJoinOrganization` — the same signal the migration notice already
used — and falls back to the sent intent only when no preview could be computed.
In-app accept skipped the disclosure entirely. `useAcceptMyInvitation` posted an
empty body, so `disclosedWorkspaceIds` was absent and the server's consent guard
was skipped, and the pending-invitations modal never showed which owned
workspaces would move. Accepting from the workspace switcher (including the
desktop path) could silently sweep personal workspaces into the organization,
bypassing the consent model this PR adds on /invite. The list endpoint now
returns each invitation's join preview, the modal renders the same
membership/migration disclosure as /invite, and accept echoes
`disclosedWorkspaceIds` so the guard applies on both paths.
Both notices moved into lib/invitations/disclosure-copy.ts and are consumed by
/invite and the modal, so the two accept surfaces cannot drift into disclosing
different outcomes for the same invitation — which is how this gap arose.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(invitations): carry the membership outcome in the disclosure token
Empty disclosure skipped membership consent. The token was only the
workspace-id list, so a no-join preview and a will-join preview for someone who
owns nothing both echoed `[]`. Neither guard could tell those apart: the forward
check compares sweep sets, and the reverse check required a non-empty disclosed
set. An invitee who left their other organization between preview and accept
would therefore be silently made a seat-consuming member after being told they
would stay external, and the mirror case could silently demote a promised join.
The accept body now also carries `disclosedWillJoinOrganization`, compared
against the resolved outcome before any write, so consent covers the membership
decision and not just the migration. Both accept surfaces send it.
This was widened by the previous commit: keying the membership notice on the
preview made the screen promise a join outcome the token never verified.
In-app accept errors lacked copy. `getInvitationErrorMessage` omitted
`external-requires-paid-plan`, `disclosure-outdated`, and
`workspace-not-found`, so those failures fell through to the generic "may have
expired" fallback. `disclosure-outdated` became newly reachable in-app the moment
that path started sending the token, so the gap arrived with the fix for it.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(invitations): compare the join disclosure against new-membership creation
The membership consent guard compared the disclosed outcome against
`shouldJoinOrganization`, which stays true for an invitee who already belongs to
the target organization — the invitation's intent is still internal. The join
preview reports no-join for exactly that case, because nothing changes for them.
Every such acceptance therefore failed `disclosure-outdated`, and the retry
re-rendered the same preview, so the invitation became permanently unacceptable.
The guard now compares against whether acceptance creates a NEW membership
(`shouldJoinOrganization && !alreadyMemberOfTargetOrganization`), which is what
the disclosure actually promises and what the preview reports. The
already-a-member predicate is hoisted and shared with the join block below so
the guard and the billing path cannot disagree about it.
Regression test asserts a pre-existing member accepts with a no-join disclosure;
it fails with `disclosure-outdated` against the previous comparison.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(invitations): tell an existing member their standing is unchanged
The join preview reported the same no-join shape for two different outcomes: an
external collaborator, and an invitee who already belongs to the organization
acceptance lands in. `buildMembershipNotice` rendered both as "you'll join as an
external collaborator ... everything you own stays yours", which is wrong for an
existing member — they stay an internal member and simply gain the granted
workspaces.
The preview now reports `alreadyMemberOfOrganization` for that case (a
membership in a DIFFERENCE organization is still the external path, since
acceptance downgrades), and the notice states that standing is unchanged. This is
the same conflation behind the accept loop fixed in
|
||
|
|
94778eb68d |
feat(pi): add update PR mode (#6031)
* feat(pi): add update branch mode * feat(pi): manage pull requests in update mode * docs(pi): clarify closed PR behavior * fix(pi): handle update PR finalization races * fix(pi): accept renamed update PR repositories * fix(pi): clarify shared open PR errors * fix(pi): recheck update PR ambiguity * fix(pi): recreate closed update PRs * fix(pi): follow replacement update PRs * fix(pi): preserve update PR BYOK after staging rebase --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Bill Leoutsakos <billleoutsakos@Mac.localdomain> |
||
|
|
911b958dbd |
feat(exa): refresh Exa integration against current API, retire dead research endpoint (#6074)
* feat(exa): refresh Exa integration against current API, retire dead research endpoint Exa's dev-rel team flagged that our integration was written against a retired version of their API. Validated every claim against the live API with a real key. - /research/v1 returns HTTP 410 RESEARCH_RETIRED, so the Research operation was hard-broken in production. Removed it and added an Agent operation on /agent/runs. Saved workflows on the old operation are routed to Agent so they start working again. - Category dropdown sent values Exa no longer recognizes (research_paper, news_article, movie, song, ...). Exa accepts category as an unvalidated soft hint, so these silently stopped steering results rather than erroring. Replaced with the current taxonomy and remapped legacy values. - Live crawl mode defaulted to 'never', silently forcing cache-only results on every search. Removed the default and exposed maxAgeHours, which replaces the deprecated livecrawl. Exa 400s when both are sent, so they are now mutually exclusive. - numResults was capped at 25 in the UI; the API allows 1-100. - Search types refreshed to instant/fast/auto/deep-lite/deep/ deep-reasoning. Legacy neural/keyword still pass through. - Exposed result id so search results can be chained into Get Contents via ids, plus highlightScores, subpages, entities, extras, statuses, requestId, and outputSchema structured output with grounding. - answer text controls cited-source text, not the answer; fixed the description and the dead query field. - Marked findSimilar and the crawl-date filters deprecated. Both still work, so existing workflows are unaffected. - Copilot search-online never requested page content, so every snippet was empty. Now requests highlights. * fix(exa): address review findings on the API refresh - A run already terminal on creation went through a path that never set success=false, so a failed or cancelled run reported as successful. Both the create path and the poll loop now settle through one function. - Routing exa_research to Agent dropped the research output shape, so saved workflows referencing research[0].text resolved to undefined. The agent tool now also emits that legacy shape. - The Agent operation's inputs are conditioned on both exa_agent and exa_research so the serializer keeps carrying a stored research query; it drops any value whose sub-block condition no longer matches. - Dropped the model to effort mapping and the unused ExaResearchParams: the serializer drops values for removed sub-blocks, so model never reached the params function. * fix(exa): keep legacy research model, add subblock migrations, sharpen outputs The subblock ID stability check caught the removed subblocks — that gate exists precisely to stop removals from breaking deployed workflows. - Restore the research model sub-block, scoped to the legacy exa_research operation so it never shows for new workflows but still serializes for saved ones, and restore the model to effort mapping. Removing it lost the configured research depth, silently falling back to effort auto. - Register useAutoprompt and livecrawl in SUBBLOCK_ID_MIGRATIONS as intentional removals. Neither has a value-compatible replacement: livecrawl is a mode string and maxAgeHours a number, so mapping one to the other would send NaN. - Replace vague json output descriptions with their inner field lists. - Drop the separate compat test file; the coverage that guards real regressions now lives in exa.test.ts. * fix(exa): flag empty Get Contents configs in the editor, keep legacy research depth - A saved research workflow with no stored model fell through to the Agent default of auto rather than the standard depth the old Research operation used. Legacy research now always maps to an effort level, defaulting to medium, and the legacy model sub-block carries the same default the old dropdown had. - Get Contents needed both selectors optional so the ids path is reachable, which left an empty config failing only at run time. URLs is now conditionally required, dropping the requirement when result IDs are supplied, so the editor flags the empty case. The exactly-one check in the request body stays as the backstop. * fix(exa): revert conditional required on Get Contents URLs The conditional required callback did not work and introduced a regression. `isFieldRequired` in webhook deploy calls `config.required()` with no arguments, so the callback never saw `ids` and left URLs required — an ids-only block would have been reported as missing a required field on deploy. `collectBlockFieldIssues` skips sub-block required checks whose id matches a tool param, so it never evaluated the callback either. Both selectors go back to optional with the exactly-one check in the request body, which is what the integration rules prescribe for mutually exclusive alternate identifiers. Added a comment recording why a conditional required cannot express this, so it is not reattempted. |