6493 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Theodore Li	4c0bd944dc	feat(slack): launch v2 triggers and backfill custom bots (#6873) ... * feat(slack): launch v2 triggers and backfill custom bots * fix(slack): propagate legacy webhook dispatch failures * fix(slack): continue shared legacy webhook fanout * fix(slack): acknowledge filtered webhook deliveries * fix(slack): finalize custom bot migration rollout * fix(slack): dedupe migrated bots per workflow * fix(slack): harden custom bot rollout * fix(slack): acknowledge permanently ignored deliveries * fix(slack): retry failed webhook deliveries	2026-08-25 14:11:02 -04:00
Waleed	656840a1b1	feat(api): make the platform operable headless over v2 (#6912) ... * feat(v2): download run output files by API key Adds GET /api/v2/workflows/{id}/runs/{runId}/files/{fileId}, closing the async-run loop for headless callers. A run's output carries UserFile URLs pointing at /api/files/serve/..., which rejects x-api-key outright, so an async run that produces a file previously had no byte path out for an API key at all. The file is addressed by the id the run reported and resolved against the run's own recorded execution data, from which the storage key is read. The request never supplies a storage key, so the endpoint cannot be aimed at bytes the run did not produce. Resolution deliberately reads the materialized-but-undisplayed recording, because the display projection strips exactly the `key`/`context` fields a byte read needs. Also hardens normalizeStartFile to derive a file's storage key only from a validated internal serve URL, discarding any caller-supplied `key`/`context`. A workspace API key has no human subject, so the executor resolves its actor to the workspace billing owner (preprocessing.ts -> resolveSystemBillingAttribution); verifyFileAccess then authorizes a workspace-context key as that owner, whose reach is not bounded by the key's workspace. Accepting an attacker-authorable key made that substitution exploitable as a confused deputy. Normalization is all-or-nothing, so a forged file now drops the whole files input. * feat(workflows): one graph-write door, principal-derived audit source, and v2 authoring endpoints Extract replaceWorkflowNormalizedState as the single persistence primitive for a workflow graph replace and route both the internal editor save and the Copilot edit tool through it, so neither can skip state preparation, the row lock, the lastSynced stamp, or custom-tool extraction by choosing a different entry point. Derive the audit source from the acting principal instead of hardcoding 'copilot', then widen workflows.variables.apply_operations and workflows.bulk.move to every principal kind. Add GET/PUT /api/v2/workflows/{id}/state, POST /operations, /duplicate, /restore, PATCH /variables, and POST /api/v2/workflows/move over surface-neutral application use cases; move the edit engine to lib/workflows/editing. * test(workflows): cover the graph-write primitive, audit source, and the v2 authoring surface Pin the two-doors fix (preparation runs, the row is locked, custom-tool extraction is post-commit and best-effort) and the false-audit fix (a session principal writes source: 'session', a delegated one writes its service). Both were verified to fail with the fix reverted. Add the application matrix for replaceWorkflowState, applyWorkflowOperations, readWorkflowGraph, and restoreWorkflow — role floor, principal-kind rejection before canonical load, asserted-scope concealment, lock, validation, atomic conflict, plan gate, and audit-then-notify ordering — plus route tests for every new endpoint. * test(workflows): pin the internal graph-write door and the v2 list scope Characterize saveWorkflowNormalizedState's statuses, messages, and notification after the persistence extraction, and cover the new scope filter on GET /api/v2/workflows including a cursor replayed under a different scope. * feat(api): add v2 block, tool, connector-type, and enrichment catalogs Adds six read endpoints under /api/v2 that publish Sim's code-defined catalogs: GET /blocks, GET /blocks/{blockId}, GET /tools, GET /tools/{toolId}, GET /connector-types, and GET /enrichments. These read like static reference data and are not. What a caller may place is decided per workspace by its permission-group integration allowlist, per organization by which unreleased blocks have been revealed, per deployment by ALLOWED_INTEGRATIONS, and per workspace again by the workflows it has deployed as blocks. So all six are plain defineWorkspaceOperation reads at minimumRole 'read' with workspaceApiKey 'allow' — the exact policy of credentials.providers.list — and every response keeps Cache-Control: private, no-store, because an unrevealed preview block's existence must not leak across organizations through a shared cache. Trigger blocks ride as ?capability=trigger rather than a second endpoint, and workspace custom blocks ride inside /blocks discriminated by `source`, so "what may I place?" stays a one-call question. The block projection is extracted out of the Copilot get_blocks_metadata tool and rewritten onto @/tools/metadata and @/tools/metadata-outputs. That cuts the tool's own @/tools/registry edge as a side effect: its module graph drops from 6,756 to 1,318, and the new routes land at 1,673-1,734, next to the shipped /v2/credentials/providers baseline of 1,668. Supporting changes: - scripts/sync-tool-metadata.ts derives hostedApiKey ('always' | 'conditional' | 'none') from each tool's `hosting`. The config itself stays excluded because it holds closures, but "does Sim host the key" is a first-order authoring question, so the answer is emitted. - getCopilotToolDescription takes hostedApiKey as an option instead of reading `hosting` off the tool, so both an executable ToolConfig and the generated metadata can answer it through one shared derivation. - principalUserId / allowedIntegrationTypes move out of lib/credentials/application/provider-catalog.ts into lib/integrations/principal-scope.server.ts. Two copies of the workspace integration gate would diverge first on the workspace-key path, which has no user for permission groups to key on. - scripts/check-tool-registry-boundary.ts walked page.tsx/layout.tsx under app/workspace only, so a route importing the executable registry passed green. It now walks a list of entry sources, seeded with the four catalog route subtrees and the shared projection barrel. Routes are covered per subtree rather than wholesale because 122 of ~1,130 route files legitimately execute tools. Registry sweeps parse every block, tool, connector type, and enrichment through its published response schema and compare against the wire round-trip. They caught a real drift while being written: an operation's inputs were typed as a union of the tool-param and block-input shapes, and the union resolved to whichever member matched first, silently dropping a block input's `schema`. * docs(api): stop publishing a 413 GET /workflows/{id}/state cannot emit * feat(v2): read upload-session state Adds GET /api/v2/files/uploads/{uploadId}. Only DELETE was exported, so a caller that lost track of a transfer could abort it but could not ask whether the session was still alive, already finalized, or failed — the resume story was missing. Runs on a new files.upload.read operation at minimumRole 'read' rather than reusing uploadCancel, which is a 'write': asking about a session must not require permission to destroy it. The GET is a control leg like every other, so it carries the signed upload token and re-authorizes the caller's present workspace permission through reauthorizeWorkspaceUploadPurpose instead of resolving the session on its id alone. * fix(api): reconcile v2 catalog and workflow-authoring integration Merging the catalog and workflow-authoring branches surfaced four issues that neither produced in isolation. - Route and OpenAPI counters were bumped to the same value on both branches, so git merged them as one change while the merged tree holds the sum. Corrects the route ratchet to 1142 and the workflows document to 29 operations (152 total), then regenerates the OpenAPI documents and the CLI surface from the reconciled contracts. - The seven new workflow operations were published in the spec but absent from the workflow API reference groups, which `check:openapi` rejects. - `route-policies.ts` reached `WorkflowOperationsNotAppliedError` through `apply-workflow-operations`, dragging the edit engine — and its diff and comparison dependencies, which reach a client OAuth hook — into every route that uses the shared workflow error policies. The class moves to its own leaf module, mirroring `WorkflowImportError`, and each importer now takes it from there. - The operations route test shadowed that class inside its module mock, so `instanceof` matched a fake and the assertion pinned a message the production class never emits. It now uses the real class and asserts the real message. * feat(v2): extract ZIP archives over the public API Adds POST /api/v2/files/{fileId}/extract and widens files.extract_archive from principalKinds ['session'] / workspaceApiKey 'deny' to admit personal and workspace API keys at the unchanged 'write' role. The widening is an authorization change, so the justification lives in the operation's TSDoc: extraction grants no capability an API key lacks, since every file it writes could be created one at a time through files.create and files.upload.create, both already 'allow' at the same role. It only collapses many calls into one. The previous ['session'] restriction read as an artifact of the UI having been the only caller. Delegated services stay out — no copilot or executor caller exists and admitting one is a separate decision. The response is counts plus the destination folderPath, never the extracted files: a large archive would otherwise materialize thousands of objects into one body. Callers page GET /api/v2/files?folderPath=... instead. The use case returns the internal display path and the adapter projects it to a v2 path, keeping the use case surface-neutral. * feat(v2): extract file text over the public API Adds GET /api/v2/files/{fileId}/text. Text extraction previously sat behind checkInternalAuth on /api/files/parse, a route that also mixes in external-URL fetching, execution-file upload, and multi-file aggregation, so it could not be reused. The parse call is lifted into a thin application use case instead. Runs on the existing files.read_content operation unchanged — it is already workspaceApiKey 'allow' at the read role, and turning bytes it already authorizes into text grants no further reach. `degraded` is a required, non-optional boolean on the response. The legacy doc and ppt parsers deliberately return best-effort or placeholder content rather than throwing, so an omittable flag would let a client that never checks it treat guessed text as extracted text. It is reported honestly rather than converted into an error, because the parsers' behaviour is deliberate and characterization-tested. The read is bounded on its input at 25 MiB before extraction rather than on its output after, given the parsers' documented DoS history; a caller may lower the ceiling but never raise it. * feat(v2): restore archived folders and list the archived set DELETE /api/v2/files/folders archives recursively, so a recursive delete was unrecoverable over the API: the archived files stayed visible through GET /api/v2/files?scope=archived, but nothing could rebuild the folder structure. Adds POST /api/v2/files/folders/restore, path-addressed like the rest of the v2 folder family, and a `scope` selector on the folder list so a caller can find the archived path to hand it. `scope` extends the files folder-list query rather than the shared v2ListFoldersQuerySchema: only workspace files have an archived folder set, so adding it to the shared schema would give tables, workflows, and knowledge a parameter they ignore. GET /api/v2/files/folders is a FULL_SET_LIST, not paged, so no cursor binding changes — list-pagination.test.ts passes unchanged. Restore resolves the archived folder from its path by scanning the archived set rather than walking the live tree, which by definition does not contain the folder being restored. The folder-restored analytics hook now reports the folder actually restored rather than the requested selector, which carries no id on a path-addressed surface. * feat(v2): bulk-download a file selection as a zip Adds GET /api/v2/files/bulk-download, an adapter over the existing downloadWorkspaceFileItems use case and its internal binary route. Path collision: a static segment beside [fileId] permanently shadows a file whose id equals it, and workspaceFileIdSchema does accept [A-Za-z0-9_-]+. Rather than invent a new shape, this follows the existing bulk-delete sibling: the hyphenated form cannot be produced by either minted id shape (UUID v4 or wf_<shortId>), so the shadowed id is unreachable in practice. Documented on the contract so the reasoning is not lost. Folders are addressed by path, matching the rest of the v2 file surface. The paths resolve against the folder set the selection already loads, so it costs no extra query, and a path matching no folder is rejected rather than silently dropped — a misspelled folder must not yield a zip of whatever else was selected. The empty-selection and folder-count guards now account for folderPaths, which a path-only selection would otherwise have tripped. Selections are comma-separated only: v2 rejects a query parameter sent more than once, so a repeated-parameter form would never reach the schema. Pinned by a test so the contract cannot advertise a form the boundary rejects. * feat(v2): expose run output files and optional inline bytes on the runs read GET /api/v2/workflows/{id}/runs/{runId} now reports the files a run produced, each with the downloadPath that fetches its bytes, and can inline them as base64 on request. Gated by includeOutput, matching `output`'s nullability: a caller that did not ask for output does not receive a file list it did not request. The async execute request's rejection of includeFileBase64 is deliberately left alone — at submit time the run has not happened, so there is nothing to inline; reading a finished run is the first moment the question means anything. Inlining is capped per file at the executor's 16 MiB inline ceiling, which a caller may lower but never raise. A file above it answers 413 naming that file's downloadPath, so the caller is told exactly how to get the bytes rather than being left stuck. The descriptor deliberately omits the storage key — files are addressed by id and the key is re-derived from the run's recording — and omits an expiry, which the recording does not carry and which would be fabricated if published. The route becomes headSafe: false, since inlining reads object storage. The builder enforces that this requires the use case to expose authorize(), so HEAD still answers from a real authorization rather than from authentication alone. * feat(v2): permanently delete an archived file DELETE /api/v2/files/{fileId} only archives — the OpenAPI says its stored bytes are never removed — so there was no way to actually destroy a file over the API. Adds the repository primitive, application use case, operation, and DELETE /api/v2/files/{fileId}/permanent. A distinct path rather than a flag on the ordinary delete: a query parameter that turns a recoverable archive into an irreversible destruction is set by accident, and the two acts carry different minimum roles, which one route declaration cannot express. The file must already be archived; a live file answers 409 naming the archive step, so no single request can turn a live file into lost bytes. minimumRole 'admin', which forces workspaceApiKey 'deny' since the workspace-key ceiling is 'write' — the desired policy anyway: unattended credentials should not destroy bytes. Row first, then object. The two legs commit independently, so one can survive a crash between them: deleting the row first leaves at most an orphaned object for the storage sweep, while the reverse would leave a live row pointing at bytes that no longer exist — a file that lists and opens but can never be read. A failed object delete is therefore reported as objectDeleted: false rather than thrown, because the request has genuinely succeeded once the row is gone. Both directions are pinned by failure-injection tests, verified to fail when the order is reversed. Audited as a distinct FILE_PERMANENTLY_DELETED action, not a reuse of FILE_DELETED, which records the recoverable archive step. * feat(api): v2 log analytics, itemized cost, filters, and sortable query Adds the aggregate and rich-read halves of the public logs surface, and fixes three defects the existing reads carry. Aggregate analytics. `GET /api/v2/logs/stats` returns time-bucketed run counts, success rate, error count, mean latency, and the window bounds, per workflow and for the workspace. The first-party route was a raw handler with inline SQL and inline aggregation, so it is split into a repository (`lib/logs/stats-queries.ts`), a pure aggregator (`lib/logs/stats.ts`), and an application use case. That route keeps its legacy authorization — it answers a caller without workspace access with a zeroed 200, where v2 conceals the workspace as a 404 — and consumes only the two surface-neutral halves. `segmentCount` had no `.int()`, `.min()`, or `.max()`, so `0` divided by zero and `1e9` allocated two billion-element arrays: both caller-reachable 500s. Bounded on both contracts. `workflows` is capped, with the workspace totals still computed from every workflow and the cut reported as `workflowsTruncated`. Detail reads gain the itemized `cost.items` ledger (`null` and `[]` are distinct answers and both reachable) and `workflowInput`, restoring a v1→v2 regression. The list gains `workflowName` and `status` filters, and `includeJobRuns`, which unions Chat and Sim-agent job runs into the sequence behind a new `kind` discriminator — without it a job run is indistinguishable from a run whose workflow was deleted. A filter no job row can answer drops the branch outright rather than meaning two things across the union. `POST /api/v2/logs/query` carries the additional sort columns. `GET /logs` is untouched: its single `order` param rests on there being exactly one sortable column, and both escapes from that are ruled out, so the rich read gets its own endpoint — the split the table surface already ships. It uses the shared keyset scheme with the two nullable sort columns read through a sentinel, since a keyset cannot compare against null. `folderPaths` now covers a folder's whole subtree on the public path, as it already did everywhere else; it previously omitted every nested run with no error. The path strings did not change, so a folder-scope version is stamped into the cursor and in-flight tokens restart rather than silently skipping rows. Also fixes `folderName`, which ILIKEd `workflow.name` — a copy of the clause above it — and so searched workflow names instead of folders. `buildLogSortCursorCondition`'s `IS NULL` disjunct is documented and pinned: under `NULLS LAST` the null block is only reachable through it, so removing it as a duplicate-row fix makes those runs unpageable. Ratchets: route count 1142 -> 1144; logs OpenAPI operations 2 -> 4; total operations 152 -> 154. * feat(api): v2 tables run state, dispatch polling, batch update, bulk, archive Closes the headless gaps on the v2 tables surface. - Per-cell run state is now readable through an opt-in `includeRunState` on `GET /rows`, `POST /query`, and `GET /rows/{rowId}`. The default projection is byte-identical; a page whose sidecar outgrows its byte budget is a 413 rather than a silent truncation. - Run dispatches are addressable: `GET /tables/dispatches/{dispatchId}` publishes the column's full four-state domain so polling a finished run is not a 500, and `GET /tables/{tableId}/dispatches` lists what is in flight. - `POST /rows/batch-update` takes one distinct patch per row. Its transaction moved out of the Copilot-only module into a surface-neutral use case both surfaces now call. - `GET .../enrichment/{groupId}` publishes the provider cascade, cost, and timing behind one enrichment cell. - `POST /tables/bulk-move` and `/bulk-delete` reach the existing bulk use cases, which now accept folders by canonical path and resolve them inside the application layer. - `DELETE` is recoverable: `scope=archived` on the table list plus `POST /tables/{tableId}/restore`. * feat(api): expose knowledge chunks, tag writes, archive/restore on v2 Closes the knowledge cluster's remaining public-surface gaps. Chunks: list/read/create/update/delete/bulk under `/api/v2/knowledge/{id}/documents/{documentId}/chunks`. `queryChunks` gains an `id` tiebreaker on every sort so the list pages on a keyset rather than an offset — `tokenCount` and `enabled` are both non-unique, so a page boundary inside a run of equal values used to repeat or drop the tied rows. The internal offset caller is unchanged; the two positioning schemes share one read. Tag definitions: create, update, delete, next-slot, usage, and the document-scoped save and cleanup. Without them a caller could write a tag value into a slot with no definition and then had no way to name it, so tag-filtered retrieval was unbuildable end-to-end. `v2KnowledgeTagSchema` gains `id`, without which PATCH and DELETE are unaddressable. The document-scoped DELETE is pinned to `action: 'cleanup'`: the domain's `'all'` deletes the whole knowledge base's tag vocabulary from a document path. Archive/restore: `GET /api/v2/knowledge/archived` as a sibling route rather than a `scope` param — the two reads bind different operations and a v2 route declares one — plus `POST /api/v2/knowledge/{id}/restore`. `knowledge.restore` is a new workspace operation carrying `delete`'s policy, since an operation's inverse must not be harder to reach; the internal session route now delegates its workspace branch to the shared use case and keeps only the legacy personal one. Also: `POST .../documents/from-workspace-files` surfaces `addWorkspaceFiles`, so a file already in workspace storage no longer has to be re-uploaded byte-for-byte to be indexed; the `chunkingConfig` write widens to the first-party five-key schema with its refines and separator bounds, while the response stays `.catchall` so a legacy JSONB row cannot 500; and `CONNECTOR_MANAGED_RESOURCE_READ_ONLY` joins `FORBIDDEN_DETAIL_CODES` now that the bare 403 on connector-managed chunk writes is wire-reachable. Document upsert is deliberately not included. * feat(api): add v2 credential rotation and a gate-exempt capabilities endpoint PATCH /api/v2/credentials/{credentialId} rotates service-account secret material or renames a credential in place, preserving the credential id so existing workflow, deployment, paused-run, connector, and webhook references keep working. Re-posting to POST /api/v2/credentials answers 409, and delete-and-recreate mints a new id, so rotation previously had no door. The route is adapter-only: updateWorkspaceCredentialUseCase already owned the rotation, its audit projection, and credentials.update. It gains one additive assertedWorkspaceId field for the v2 workspace assertion, and the per-principal credential-type table that deleteCredentialUseCase already applied is lifted into requireManageableCredentialType so both operations share it. Without it a personal API key could rename an env_workspace row and toV2Credential's throw would surface as a caller-reachable 500. CredentialProviderOperationError now maps to 503 with Retry-After when the provider is unreachable, instead of the 400 its OrchestrationError('validation') base projected. A transient outage rendered as a permanent input error invites a caller to revoke a working credential. GET /api/v2/meta reports the calling key's rollout cohort, type, and expiry. It is the one route declaring the new typed gate: 'exempt' option, because the rollout gate and the unknown-path catch-all answer byte-identical 404s and a gated /api/v2/meta could never resolve that ambiguity. Authentication still runs first, so the only fact disclosed is one about the caller's own credential. * feat(api): publish deployment lifecycle and workflow-MCP v2 surfaces Adds the four deployment-lifecycle operations v2 was missing, and the workflow-as-MCP publishing surface, both as adapters over application use cases that already existed. Deployment lifecycle: - PATCH /api/v2/workflows/{id}/versions/{version} relabels a version. Deliberately not the internal route's body-shape dispatch between "rename" and "promote to live". - POST .../versions/{version}/activate promotes a version. Same use case as rollback under a different transition, on its own path because the two mean opposite things to a caller. - POST .../versions/{version}/revert overwrites the draft. Accepts the literal `active` alongside a version number. - PATCH /api/v2/workflows/{id}/deployment toggles unauthenticated public execution. `workflows.public_api.update` widens from session-only to session plus personal API key: it is an admin-role change the same accountable human may make from a script. Workspace keys stay denied. Its EE refusal now carries PUBLIC_SHARING_NOT_ALLOWED instead of a bare forbidden. Workflow MCP servers: - /api/v2/workflow-mcp-servers list, create, update, delete, plus publish and unpublish of a workflow as a tool. Named apart from /api/v2/mcp-servers, which registers the external servers Sim calls. - The six mcp_servers.workflow_deployments operations widen from ['delegated'] to admit sessions and personal API keys; roles and the workspace-key denial are unchanged. - The server list gains keyset pagination, matching its external sibling, since nothing caps how many a workspace publishes. - Server, tool, and workflow reads move out of the use case into lib/mcp/queries. Route ratchet 1150 -> 1160; OpenAPI operations 161 -> 171. * feat(api): extract chat deployments and publish the v2 surface Chat deployment was a shipped module with no public API and two authorization systems: `lib/workflows/application/chat-deployments.ts` had deploy/undeploy extracted, but only Copilot used them — the REST routes reimplemented workflow authorization inline, and `PATCH /api/chat/manage/[id]` additionally owned password encryption, the auth-type field-clearing matrix, identifier uniqueness, the redeploy-gating protocol with two 409s, a raw db.update, and a manual recordAudit. New `lib/chat-deployments` domain: - `chat_deployments.list/read/update/delete`, keyed on the deployment whose workspace is derived by joining its workflow. Creation stays `workflows.chat.deploy`, which is keyed on the workflow. - The PATCH extraction, including the field-clearing matrix and the asynchronous-cutover invariant the route had hand-mirrored from `performChatDeploy`. - One `buildChatDeploymentUrl`, replacing three constructions that had already drifted onto two different host helpers. There is no chat subdomain, so nothing publishes a host. - Repository reads moved out of the use cases into `lib/chat-deployments/queries`. Internal routes are now adapters over those use cases. `GET /api/chat` is deliberately not migrated: it scopes by `chat.userId` while every other chat operation authorizes by workspace admin, and reconciling the two is a product decision. `PATCH` keeps its 400 for an identifier collision through a typed `ChatIdentifierInUseError`; v2 reports the 409 the condition actually is. v2 surface at `/api/v2/chat-deployments`: list, create, read, update, delete. Workspace-scoped, keyset-paged, and a stored password is never readable — reads carry `hasPassword` only, and the session-only reveal endpoint deliberately has no v2 counterpart. Also: an email- or SSO-gated chat with an empty allow-list is now refused in the use case rather than only at the internal boundary, since it is unenterable; and the doc comment on `processHostedKeyCost` claiming a `usageLog` write is corrected — no such write exists. Route ratchet 1160 -> 1165; OpenAPI operations 171 -> 176. * fix(api): close three review findings, two of them caller-reachable - Run output files are filtered to keys under the run's own execution prefix. The recording they came from is not a trustworthy key source: the start block copies every caller-supplied input field verbatim into its output and `collectUserFilesById` accepts anything carrying the `UserFile` shape, so a caller could name any storage key and have the download and base64 paths — neither of which authorizes per file — serve it back. - `getBlock` reads own keys only. `BLOCK_REGISTRY` is an object literal, so `constructor`, `toString` and friends returned inherited functions that every consumer then treated as a block, turning a path segment into a 500. `getToolMetadata` already guarded this way. - A folder-scoped log page no longer unions in every job run in the workspace. The guard read `filters.folderIds`, which the public surface never sets — it carries the folder filter in `folderScope` — so the page contradicted the contract's promise that job runs are dropped whenever a filter they cannot answer is set. Also: the log cursor stamps `includeJobRuns` only when it is on, so its `.default(false)` no longer puts a constant in every fingerprint and rejects cursors minted before it existed; and the `folderName` subquery is scoped to the workspace and to workflow folders instead of scanning the whole `folder` table. * fix(api): close two more review findings, one an authorization bypass - `workflows.operations.apply` no longer admits a workspace API key. The use case authorizes against three per-user policies — the EE permission config, block visibility, and credential reachability — and all three take a human subject. An actorless key has none, and both substitutes fail open: attributing to the workspace billing owner evaluates the batch as the least-restricted account in the workspace, and passing no user makes `getUserPermissionConfig` return `null`, which every caller reads as unrestricted. Either way a workspace constrained by an allowlist was edited as though it were not. Personal keys keep the capability, so headless editing is unaffected for a credential that names a human. - `GET /workflows/{id}/state` reads its variables through `parseWorkflowVariables`, and the stored variable response schema drops the two assertions the column cannot honour. The column has carried a JSON string and a legacy array as well as the current record, the realtime `variable.add` op types `type` as `z.any()`, and the parser writes `name` through verbatim — so the input bounds on the read turned a stored workflow into a 500 on the endpoint that opens it. The write schema keeps them, which is where they can still be honoured. - `GET /workflows?scope=archived` projects folder paths tolerantly. Archiving a folder cascades onto the workflows inside it but leaves their `folderId` dangling — which is why restore has to null it — so the strict projector threw a bare `Error` and took the whole page down with no cursor able to step past the row. * fix(files): bind Start-block file keys to the executing workspace The Start block derived a file's storage key by parsing the caller's own `url`, which `isInternalFileUrl` matches on any host and `extractStorageKey` returns verbatim — so a request body could name any tenant's bytes. The key is now accepted only when its own layout names the workspace the execution runs in, and every file is dropped when the execution carries no workspace. Also bounds `includeFileBase64` with an aggregate response ceiling and a worker pool instead of an unbounded `Promise.all`, scopes the bulk download's authorization resource to the workspace when folder paths are requested, makes the folder-restore selector mutually exclusive at the type level, and names the bound in the `maxBytes` validation message. * fix(api): close v2 log review findings Cursor scope: `scope` on the workflow and table lists carries `.default('active')`, so it entered every fingerprint as a constant and refused every cursor minted before the param existed — with the "cursor does not match the requested filters" 400, which is actively misleading for a caller that changed nothing. Both now stamp the default as absent, so only a caller who asked for `archived` gets a new sequence. Dashboard stats: `maxWorkflows` capped the response, not the allocation. Segment series are now densified after the cut instead of before, so returning 200 series no longer materializes one `segmentCount`-length array per workflow in the window. The aggregate still sums every workflow, now from the sparse per-workflow maps. Cost keyset: `cost_total` is an unconstrained `numeric`, so its anchor travelled through `Number()` and was compared back at full precision — rows differing beyond float64 collapsed onto one anchor. Adds `decimalKey`, which carries the digit string and binds it `::numeric`. Run detail: `cost_total` is a backfilled projection, so a run predating the backfill reported `cost: null` even with a real ledger, making `items` unreachable for exactly the runs the ledger explains. Falls back to the ledger total. Also caps the log folder-path index reads at MAX_FOLDERS_PER_WORKSPACE like every other reader, publishing the folder-tree 413 on the four log operations; reverts a dead `status` widening in `v2CommaListSchema`; drops an unread `executionData` select; corrects the segment-count and searchLogs prose; and replaces the sort-cursor SQL-text assertions with a two-page walk over a fixture with a null block. * fix(api): close knowledge v2 review findings - widen knowledge.list_archived to the delete/restore policy so a workspace API key can discover what it may restore - escape LIKE wildcards on the now-public chunk search - derive tag slot capacity from TAG_SLOT_CONFIG per field type - type updateKnowledgeBase's chunkingConfig as ChunkingConfig and project every declared field explicitly - attribute a restore to the calling surface instead of a literal 'api' - gate 'knowledge chunks batch-update' behind --yes, since it can delete - present the tag-cleanup action from the parsed request rather than faulting on the domain result after the delete committed - unbind asserted-scope workspaceId from the nested knowledge cursors, matching the table-row lists - add executed-SQL coverage for the chunk keyset * fix(catalog): close the catalog and registry-boundary review findings The module-graph ratchet treated an entry with no baseline row as informational, so the six catalog routes and the projection barrel were unratcheted while the summary still read "within their module-count baseline". An unbaselined entry now fails --check, the summary counts only what was actually compared, and the baseline is re-recorded. The Copilot block-metadata tool — the reason the shared projection exists, 6,756 modules down to 1,321 — was in no guarded subtree. It is now an entry source and a catalog boundary root. Catalog behaviour: - hostedApiKey is gated on the deployment, so a self-hosted install reports none instead of promising 127 tools' keys it will never supply - block detail resolves an unversioned base type to its newest version and projects through the viewer's visibility, so it can no longer 404 a block the list contains or name it differently - offset-cursor ordering compares code units rather than the process locale - projections copy every array they publish instead of handing out the registries' own - an options function returning a thenable throws rather than silently widening the providers-store substitution across the event loop - a throwing block projection costs the Copilot tool one block, not all of them - the trigger-kind log returns to debug: chat/manual/api are entry-point kinds, not authoring defects Also sweeps the custom-block detail branch against its response schema, guards each projection module rather than the dead barrel over them, and drops a provably dead branch in processHostedKeyCost. * fix(workflows): close v2 workflow-authoring review findings - Read a blockless draft back as an empty graph. `PUT /state` of `{ blocks: {}, edges: [] }` — the contract's own published example — deletes every block row, and the loader answers `null` for a blockless workflow, so the following `GET /state` answered 404 while the list endpoint still showed the workflow. Existence is the workflow row's to decide; the null is now projected as an empty graph. - Rewrite the `readWorkflowGraph` authorization test so it can fail. It called `authorize?.()` and asserted only a negative, so deleting `authorize` or replacing it with a no-op both passed — the invariant the head-safe `HEAD` path depends on. - Route `setWorkflowBlockEnabled` through `replaceWorkflowNormalizedState`, the same door the other two graph writes use, instead of writing the normalized tables itself without state preparation or custom-tool extraction. - Count applied operations directly. Enablement refusals landed in the same skipped-item array and were subtracted from the operation count, which `Math.max(applied, 0)` then masked when it went negative. - Give a `disabled_ancestor` refusal its own member of the published skip enum instead of reporting it as `block_locked`. - Refuse an `atomic` batch whose credential or hosted API key would be stripped, and carry the dropped inputs in the 409 details. - Publish the whole lint report — `sources`, `sinks`, `orphanBlocks`, `emptyOutgoingPorts`, `invalidBranchPorts`, `invalidConnectionTargets`, `fieldIssues`, and the `kind` discriminator on unresolved references — rather than only free-text reference prose. - Stop reporting unresolved lint references as `inputValidationErrors`. `collectUnresolvedReferences` is read-only, so those values stay persisted; they were double-reported, and falsely as dropped inputs. - Replace two unfalsifiable negative-principal tests, which used a principal kind `Exclude`d from `PrincipalKind`, with a reachable one. - Nits: drop a stranded TSDoc block; assert the membership predicate in the selector-validator admin test; make the HEAD test assert a representation; assert the sanitized graph is what `replaceWorkflowState` writes; add a route test rejecting `baseGraph` in a v2 body; carry `principalAuditSource` on restore/duplicate/moveBulk audit; unify the two `base64MaxBytes` ceilings on `MAX_INLINE_MATERIALIZATION_BYTES`. * fix(api): close seven v2 review findings, one an authorization bypass Raise mcp_servers.workflow_deployments.update_server to admin: its body carries isPublic, and a public server executes with no Sim credential, so a write member could remove authentication from every workflow it publishes. create_server already grants the same visibility at admin. Pin the widened operations in registry tests — the six workflow-MCP ones, the four workflow widenings, and files.extract_archive. Reject secret fields on a credential that has no rotatable secret instead of dropping them behind a 200, classify a non-transient provider 4xx as caller error rather than a retryable outage, and reconcile a provider outage to 503 with Retry-After on all three surfaces. Give /v2/meta a declarative principal policy through a new defineOperation factory, carry the key expiry on the auth context instead of reading the api_key table from the application layer, and make the impossible principal branch an invariant error rather than a codeless 403. Enforce the rollout-gate exemption at definition time, against the one contract path it is reserved for, and remove the gate parameter from the exported admission helper so the builder is its only door. * fix(tables): bound the run-state sidecar, and close six v2 review findings Enforces the 2 MiB run-state ceiling INSIDE the sidecar drain rather than over its materialized result, refuses the unbounded query form paired with it, and normalizes the two stored blobs the v2 surface publishes from bare `as` casts. - The run-state budget now travels into `loadExecutionsByRow`, which drains row ids in bounded chunks and refuses before fetching the next one. The post-hoc `requireBoundedRunState` walk is gone: it measured a spike that had already happened, and re-serialized every entry to do it. - Both row reads that accept `includeRunState` cap the page at `V2_MAX_RUN_STATE_ROW_LIMIT`, and `POST /tables/{id}/query` additionally refuses the flag paired with `limit: 0`. - `runState.status` and the enrichment cascade blob are projected onto the published shape before presentation; both were caller-reachable 500s on a well-formed read. - `sim tables bulk-delete` now gates behind `--yes`, and the CLI sweep that should have caught it covers destructive non-DELETE forms. - `POST /tables/{id}/restore` is idempotent (200, no audit) like its knowledge sibling, and bulk folder selection deduplicates after resolution. - The batch-update backstop keeps the looser Copilot ceiling and says so in TSDoc: it is a backstop no surface reaches, because the contracts stop a v2 caller at 1000 and the Copilot tool stops itself at 5000. Each caller sees the bound that actually applies to it; neither surface's cap moved. * fix(chat-deployments): close v2 chat review findings Fixes the chat-deployments slice of the v2 review, several of which are regressions the application-operation extraction introduced. - Stop a `500` on schemaless JSONB: the response now declares a stored shape without bounds and `toV2ChatDeployment` projects `customizations`, `outputConfigs`, and `allowedEmails` onto it. The request schemas keep `.strict()` and their bounds. - Restore the specific validation message on `POST /api/chat` and `PATCH /api/chat/manage/[id]`, and the deleted test that pinned it. - Restore `chat_deployments.read` to workspace `admin`; the detail read serves the visitor gate. - Narrow the list projection so `chat_deployments.list` can stay a `read` operation reachable by a workspace API key: `allowedEmails`, `hasPassword`, and `customizations` are gone from the list entry and available only from the admin-gated detail read. Serialized field by field so a field added to the detail shape cannot reach the list by default. - Classify create-path failures: `performChatDeploy` carries an `errorCode`, so an in-flight deployment is a `409` and an invariant failure a `500` instead of every refusal being a `400`. - Delete the callerless `GET /api/chat`, which served the encrypted password column with no response contract. - Propagate undeploy infrastructure failures instead of concealing them as `404`, and return `ChatDeploymentView` from both delete paths. - Name `CHAT_AUTH_MODE_NOT_PERMITTED` on the create path. - Guard `getBaseUrl` inside `buildChatDeploymentUrl`, which otherwise throws on a self-host with no `NEXT_PUBLIC_APP_URL`. - Correct the published allow-list claim: a replacement `allowedEmails` is applied after the auth-type clear, so it does survive. - Assert `workspaceId` on the v2 detail routes and reconcile the concealment TSDoc with what the error policy actually renders. - Move `resolveActiveWorkspaceApplicationContext` to the workspaces domain so chat-deployments no longer imports workflow application code. * test(credentials): pin the reconciled provider-outage status The internal route alone answered 502 where the v2 surface, the shared status helper and `PROVIDER_OUTAGE_CODES`' own TSDoc all say 503. The test pinned the divergence; it now pins the reconciliation, including the `Retry-After` a 503 carries. Corrects a stale comment that still named 502 as the value callers see. * fix(executor): restore cloud-storage Start files, dropped by the key rule The ownership check accepted a key only when it could be parsed out of an internal `/api/files/serve/...` URL. But the server-side uploader for run inputs returns a *presigned cloud* URL whenever object storage is configured, whose path is the bucket key — so every chat-deployment attachment, API `files[]` payload and generic-webhook file field resolved no key, and because normalization is all-or-nothing the entire `files` input was dropped with no error. It passed locally and under vitest only because the uploader falls back to an internal URL when no object storage is configured, which is exactly why no test caught it. The test is ownership, not provenance: a key is accepted when its own layout names the executing workspace, whether it arrives directly or is parsed out of the URL. Neither field has to be trusted, since both are caller-authored and both are held to the same check. A payload whose key and URL disagree is refused rather than resolved in the caller's favour — a genuine uploader writes the two consistently, so only a forged pairing is turned away. `context` is now derived from the accepted key rather than read from the payload or the URL's `?context=`, so an owned key can no longer be labelled with a bucket its bytes do not live in — the hardening the previous comment claimed but did not perform. * fix(api): close four defects the fix pass introduced - `resolveLatest` built a `RegExp` from the caller's block id and read the registry with a bare lookup, so the catalog detail route — moved onto it by the version-alias fix — routed around the `ownBlock` guard added for exactly this. `GET /api/v2/blocks/%5B` was a `SyntaxError` 500 and `.../constructor` an inherited function. Matched by string comparison now, the way `tools/tool-ids.ts` resolves the same convention, and read through `ownBlock`. - The run-state byte budget was applied inside `queryRows` rather than at the callers that publish it, so the first-party table grid — which reads run state at five times the row limit and publishes no ceiling — turned a large page into a hard failure, with an error naming a parameter it does not expose. The budget is now an explicit option the public reads pass and internal callers omit. - Three graph-write CLI commands shipped ungated because the sweep meant to catch them matched only the names already enumerated, so it could never fail. It now forces every non-`GET` operation into a destructive or non-destructive list, and the three carry confirmations. - Unbinding `workspaceId` from the knowledge-documents cursor was right on the merits and wrong in effect: the value is constant per sequence, so removing it changed the fingerprint and refused every cursor already in flight. Restored there; the chunks list is new in the same change and keeps the cleaner reading. * fix(api): resolve a detail read to a version the viewer can see `getLatestBlockForViewer` took the newest version and then hid it, which inverted the contradiction it was written to close: `slack_v2` and `table_v2` are preview-gated while their v1 deliberately stays in the toolbar, so an unrevealed viewer got a `404` on a detail read for a type `GET /api/v2/blocks` was listing in the same breath. It now walks versions newest-first and answers with the first one visible to that viewer. Also: - The chat password guard ran after `performFullDeploy`, so a request that could never succeed burned a real workflow deployment version and then answered 400. Its two sibling gate guards already refuse ahead of the deploy; this one now does too. - The Copilot sub-block serializer published the registry's own `options` and `dependsOn` arrays by reference. Pre-existing, but the catalog projection this parallels copies every array it publishes precisely because they are process-global and shared by every request. * fix(api): restore the locked read-modify-write and the password validator Two findings verified as real regressions against staging, out of ten checked — the rest were pre-existing, latent, or false. `setWorkflowBlockEnabled` read the graph outside the row lock and wrote it back inside a later transaction. The editor's own save takes that same lock, so an autosave committing in the window was silently discarded: this operation writes a whole graph, not a delta. The persistence primitive now accepts a reader that runs after the lock is taken, and the toggle re-reads and re-decides there. Its lock predicate is also scoped to the workspace and to a live row again, so a workflow archived mid-flight is refused rather than written. The v2 chat-deployment contracts inlined their own password rule twice instead of using `chatDeploymentPasswordSchema`, losing the refusal of a whitespace-only password — which the internal contract rejects precisely because it strands the deployment behind a password the visitor form will not submit. Both sites use the canonical validator now. * fix(api): one folder projection, one dynamic-provider list, honest 413s - `toV2Folder` existed twice, and the second copy had been written without the name/path invariant — so a row the list read refuses loudly would have been served with a mismatched pair by the restore read. One definition, guard included. - The catalog projection restated `DYNAMIC_MODEL_PROVIDERS` and had drifted by one member. Derived from the canonical list instead. - The tables reads documented a `413` for run state that they cannot emit — the budget became opt-in, and the row limit is the bound now — so the claim is removed rather than declared. The workflow run read has the opposite problem: it genuinely emits one, on a single file *or* the run's inlined total, and declared neither. Now declared, and the sentence covers both. - Reclassifies the operations staging added into the destructive sweep, so the triage stays exhaustive. * fix(v2): classify storage and uniqueness failures, drop permanent file delete - remove the permanent file-delete endpoint; the platform offers no such action in the UI, and its manager wrote outside a transaction with no storage accounting - extract a generated document's text from its compiled artifact rather than its generation source, matching the download path; a `.pdf` source was a 500 and a `.docx` source returned generator JavaScript as clean content - report a run file whose object retention has already swept as 404 rather than 500, on both the inline base64 read and the download stream - report a knowledge tag that loses at a unique index as 409, naming whether the slot or the display name is taken - gate `workflows versions revert` behind a CLI confirm; it overwrites the draft graph and was classified non-destructive * feat(v2): report lint from both graph writes and add dry-run previews - `PUT /workflows/{id}/state` now returns the same `lint` report as `POST /operations`; an agent authoring a graph from scratch needs the findings at least as much as one editing incrementally - extract the report into one shared builder so the two writes cannot drift, and one shared presenter so the wire shape is identical - skip the credential/tool reference pass when the caller has no human subject, rather than resolving it against the workspace billing owner: that would misreport what the workflow can reach and disclose another person's grants. `lint.notes` says when it was skipped - add `?dryRun=true` to both graph writes: validates and lints, persists nothing, records no audit, notifies nobody. A query param, not a body field, since the body of a PUT is the resource itself - CLI: a dry run no longer demands `--yes`; requiring confirmation to preview a change teaches callers to pass `--yes` reflexively - CLI: name the graph commands for their verbs — `workflows state get`, `workflows state replace`, `workflows operations apply` — instead of the derived `state list` / `state update` / `operations create` - document when to use `rollback` vs `versions/{version}/activate` on both * chore(docs): sync generated docs manifest for the new CLI pages * feat(v2): add the missing workflow-MCP reads and align bulk naming - add `GET /workflow-mcp-servers/{serverId}` and `GET /workflow-mcp-servers/{serverId}/tools`. The resource could be PATCHed and DELETEd but never read, and its tools could be published and unpublished but never listed — the server list reports tool names only, so nothing published the `workflowId` that addresses a tool for deletion. Both mirror `mcp-servers` beside them, and carry that family's workspace-API-key denial rather than the wider `mcp_servers.read` policy - rename `POST /tables/bulk-move` to `POST /tables/move`, so tables matches the shipped `files` resource exactly (`move` + `bulk-delete`) - name the CLI commands for their operations instead of the derived `... create`: `tables move`, `workflows move`, `tables bulk-delete`, and `tables rows update-each` for the per-row batch, which sits beside the existing filter-based `tables rows batch-update` `POST /tables/{id}/rows/batch-update` keeps its name: a distinct payload per resource is precisely AIP-234 BatchUpdate, and `bulk-` would have collided one word away from the filter form. * fix(v2): correct documented statuses and a caller-reachable 500 - duplicating a workflow into a locked destination folder answered 500: `FolderLockedError` is a plain Error carrying `status = 423`, which the v2 error policy does not classify. Converted to OrchestrationError('locked') at the application boundary, matching the bulk-move path - restore workflow promised a 413 for an oversized folder tree that its response list never published; the cap is real, so the status now is too - move workflows and apply variables documented 409/423 they cannot emit: every per-item lock and conflict is reported in `failed`, not thrown - bulk download and delete knowledge tag can both 409 and did not say so; cleanup tag definitions cannot and did say so - apply workflow operations denies workspace API keys but never documented it - the dry-run responses are not byte-identical to a committed write: `needsRedeployment` describes the pre-write state and persistence warnings cannot appear. Reworded rather than overclaimed - read file text and get file upload are head-safe, so the "HEAD skips the effect" sentence did not apply to them * fix(v2): guard tag field-type changes and publish the 415 every body route can return - `PATCH /knowledge/{id}/tags/{tagId}` accepted a `fieldType` incompatible with the slot the tag already occupies. Slots are enumerated per field type, so a text tag could be relabelled `number` and every later read would interpret its values as the wrong type. Create checked this; update now runs the same two checks - derive `415` from the contract the way `413` already is: the JSON builder answers UNSUPPORTED_MEDIA_TYPE for any body under a content type it cannot read, so all 100 body routes could return a status none of them published - give version activation its own result component instead of publishing it as `RollbackResult`; the shipped rollback keeps that name - correct descriptions that promised behaviour the code does not have: a `processingStatus` field never returned, a `gmail_send` resolution example that short-circuits, bucket widths that overflow the window, a bulk tag save that relocates rather than overwrites, and per-server tool names actually gathered under a page-wide budget - drop 409 from three knowledge and upload reads that cannot emit it * docs(v2): correct the upload transfer contract and 16 other published claims The upload transfer step was documented as Sim's own data plane on every deployment: "success is 204" and "a failure is the v2 error envelope". That holds only when Sim stores objects itself. With object storage configured the URL is the provider's presigned URL, so S3 and GCS answer 200 and Azure 201, and a failure is the provider's XML — a client written to the old text reads a successful cloud upload as a failure. Also states that part ETags do not need retaining: completion takes no body because Sim lists the parts from the provider itself. Other corrections, all to shipped descriptions rather than behaviour: - DELETE table and bulk-delete files archive rather than erase, and neither said so; bulk delete also cannot emit the 409 it declared - complete knowledge upload published a 402 only the create leg can raise - billing status conceals a foreign workspace id as 404, not the 403 its TSDoc and description both claimed - audit entries null a folder's resourceId and strip folder ids from metadata at every level; neither redaction was documented - details=full adds the workflow summary to workflow runs only, never to job runs; GET /logs folderPaths covers a subtree like its two siblings; getLog now carries the retention sentence - list secrets returns description too, and the logs and resources documents described only part of what they serve * improvement(api): consolidate the v2 surface and close seven defects Endpoint consolidation: - Fold POST /logs/query into GET /logs; add sortBy/sortOrder, cap the comma lists, and move the list onto the shared keyset codec - Fold GET /knowledge/archived into GET /knowledge?scope=archived, matching files, tables, and workflows - Re-home chat deployments as a singleton under the workflow they belong to; keep the workspace-scoped discovery list - Move the tag-definition writes off the document path onto /knowledge/{id}/tags, where they already acted - Nest the table export and dispatch reads under their parent table Defects: - Publish isPublicApi on the deployment read; it was write-only, so a workflow could be opened to unauthenticated execution unauditably - Stop publishing raw storage keys and an unusable URL in log files - Classify a chat-identifier unique violation as 409 rather than 500 - Fall back to the root path instead of throwing when a knowledge base's folder is archived - Cap bulk-download at the ceiling it actually enforces - Normalize variables through one helper on both graph write paths - Fix a folder-name log filter that matched workflow names Naming and gaps: - Rename /files/{id}/extract to /unarchive, /rows/find to /rows/search, /rows/batch-update to /rows/bulk-update, /columns/run to /dispatches - Type the last six generic [id] path segments - Add table folder restore and id-addressed dispatch cancel * chore(audits): record the v2 catalog routes in the boundary baseline * fix(api): accept a null chat password and correct three published claims - performChatDeploy validated `password: null` as a password, so the replace-shaped chat PUT answered 400 for every mode that owns no password — public (the default), email, and sso. The declared payload type has always allowed null, and the stored value is cleared by authType regardless, so null needs no validation of its own. The route test could not catch it: it mocks the orchestration module and pinned the exact null the real guard refused. - Redirect the two docs slugs this branch retired that were genuinely published: findTableRows and runTableColumns. - The table folder restore described an idempotent no-op for an already active folder; it answers 404. Say so, and say where the path comes from, since the tables folder list cannot yet report archived folders. - Name the customizations exception to the chat PUT's replace semantics. - A cursor-binding case used status=error, which is a level and not a status, so it failed contract validation and never reached the cursor check. Use an accepted value and pin the reason, not just the status. * chore(cli): classify the new v2 chat operation as non-destructive * feat(cli): expose canonical resource URLs * fix(api): close three caller-reachable failures found by the final probe - GET /files/{fileId}/text called parseBuffer unguarded, and parseBuffer signals every failure as a bare Error that no v2 policy classifies. A zero-byte upload or a mislabelled archive was an unhandled 500. Empty bytes now answer empty text — a zero-length file has no text — and unparseable bytes answer 409, matching the rendered-artifact resolver. - GET /workflows/{workflowId}/state asserted write-side bounds over stored data. workflow_blocks.name and .type are bare text() and the realtime rename op accepts z.string(), so a block renamed past 255 characters made the workflow unreadable, and unrepairable, over v2. The read shape now takes the same input/stored split the variable schema already had. Stored subflow conditions are coerced in the loader beside the existing numeric guards. - GET /knowledge stamped scope into the cursor fingerprint unconditionally. scope defaults to active and is new on that list, so every cursor the deployed build handed out would have been refused with a message saying the caller changed a filter they never sent. Its siblings already carry the guard and the comment. Also: POST /workflow-mcp-servers answers 201 like every other v2 create; GET /logs/stats reuses the log list's entry ceilings; the execute and resume routes install the media-type-aware 415 they publish; and a rationale citing an endpoint that never reached the wire is corrected. * fix(tables): carry the dispatch terminal timestamps through the stale sweep Staging's abandoned-dispatch recovery builds its own `DispatchRow`, and this branch had added `completedAt`/`cancelledAt` to that shape for the id-addressed dispatch read and cancel. The merge was textually clean and left the new mapping short two fields. * fix(workflows): deny workspace API keys on the graph replace `PUT /workflows/{workflowId}/state` stores blocks and their tool wiring wholesale, but the policies deciding which of those a member may add — the EE permission config and block visibility — take a human subject. A workspace API key has none, and both substitutes fail open: the billing owner is a different, typically less-constrained person, and passing no user makes the permission lookup return null, which every caller reads as unrestricted. That made the replace a second graph-write door storing what its sibling `POST /workflows/{workflowId}/operations` refuses, which denies workspace keys for exactly this reason. Both doors now agree. Personal keys keep the capability, so headless authoring is unaffected for a credential that names a human. * chore(api): drop the enrichment catalog endpoint GET /api/v2/enrichments listed the code-defined table enrichments. The per-row enrichment run detail stays; only the catalog read goes. Removes the route, contract, response and query schemas, the semantic operation, the use case, the projection module, its registry-boundary entry, and the CLI command. The "not found" and "blank search" cases it covered are repointed at the connector-type sibling so the shared behaviour stays tested rather than deleted with it. * improvement(api): make the workflow operations endpoint self-describing Two things stood between this endpoint and a caller who has only the published spec. The accepted `params` keys existed only in the edit engine's source. The spec said "the accepted keys depend on the target block type", so a caller reading it could create a nameless empty block and nothing more — while the Copilot tool catalog, over the same engine, has always spelled out the envelope. That guidance now lives in the contract, shared by the add, edit, and insert_into_subflow parameter schemas so the two surfaces cannot describe one engine differently: `inputs` keyed by sub-block id, `retry`/`triggerMode`/`advancedMode` beside it rather than inside it, `connections` keyed by source handle, and `removeEdges` for dropping one edge without restating the rest. A `block_id` that is not already a UUID is replaced with a minted one, and the mapping was computed and then dropped. A caller could not reference the block it had just created except by re-reading the graph and matching on name. The engine now returns it and the response publishes it as `mintedBlockIds`, with the in-batch versus cross-request rule stated in the operation description. * fix(api): close the pre-merge scan's blocking findings Docs and public wire, all of it permanent surface once released. - Two published tag groups, Catalog and Meta, had no sidebar entry in any locale, so six operations shipped unbrowsable. Added to all six, and a test now fails when a published tag has no entry. - deleteWorkflowChatDeployment pointed callers at DELETE on /deployment; the undeploy verb is on /deploy. - PUT /state still promised workspace API keys a degraded lint pass after the operation began rejecting them outright. It also lived in a single-quoted string, so the shared clause would not have interpolated. - POST /tables/move took targetFolderPath as nullable-but-required, which rendered the CLI flag as a required `<json|@file>`: `--to /Archive` failed to parse and omitting it failed outright, while the docs said "omit for root". Now optional and a plain string, matching POST /files/move; the route supplies the null the use case wants. - Table dispatch status and row run state published `cancelled` beside imports, exports, and job state publishing `canceled`, and the note explaining the split was wrong about its own sibling. Both new schemas now publish `canceled`; the stored column is unchanged and mapped at the presenter. The shipped `cancelled` count field is left alone. - applyWorkflowVariables can answer 423 and both workflow-MCP deletes can answer 409; none declared it. Hand-assembled error lists replaced with the shared sets. - /files/{fileId}/unarchive became /unzip. `extract` reads as "extract text" and `unarchive` reads as the inverse of restore on a resource where archived means soft-deleted; unzip collides with neither and is what the implementation calls itself. * improvement(cli,docs): finish the naming and clear the enrichment leftovers - Four single-record GETs derived to `list` while returning one thing: meta, a workflow's chat deployment, log stats, and file text. Renamed to `meta status`, `workflows chat status`, `logs stats`, `files read`, matching the `workflows deployment status` correction already in this branch. None of the old spellings shipped. - Dropped five `renamedFrom` aliases pointing at spellings that never existed, each of which built a hidden command and a permanent deprecation warning for argv nobody could have typed. `tables rows find` keeps its alias — that one really shipped. - Removed the enrichment catalog from three prose sites left behind when the endpoint went: the resources spec description, its Catalog tag, and the contract and pagination-test comments. - Documented the six new command groups in the CLI index table. That page is a guide page, so the docs staleness check cannot flag it. - Published the `customizations` exception to the chat replace semantics. It was in the route TSDoc and invisible to every caller reading the spec, which is where the claim "Replace, not merge" is made. * improvement(cli): use batch- for the tables bulk delete, matching its siblings The CLI renames a bulk form only when it would collide with its singular sibling, and uses AWS's `batch-` prefix when it does — `files batch-delete`, `tables rows batch-delete`, `knowledge chunks batch-update`. `tables delete` exists, so `tables bulk-delete` was that same rename reaching for the other word, and the only `bulk-` command on the surface. No `bulk-` CLI command has ever shipped, so this costs nothing now and would be permanent later. `files bulk-download` keeps its name: there is no `files download` to collide with, and it is one archive rather than N operations. Its config block now says why it exists at all, since the command is never built — the builder skips non-JSON response modes, but the contract sweeps still read the entry and require the folder-path field to be marked. * fix(workflows): allow operations on blockless drafts * feat(workflows): add manual and run-from-block execution * fix(cli): update workflow run description test * fix(tests): align fixtures with current contracts * fix(api): derive chat activity from workflow deployment --------- Co-authored-by: Theodore Li <theo@sim.ai>	2026-08-25 04:19:34 -04:00
Vikhyath Mondreti	1e1298b7f7	feat(library): Best Multi-Agent Frameworks for Production in 2026 (#7065) ... Co-authored-by: Sim Pi Agent <pi@sim.ai>	2026-08-24 23:33:33 -07:00
Waleed	174c77384a	improvement(tools): support duplicate provider instances (#7064) ... * improvement(tools): support duplicate provider instances * fix(tools): cover duplicate Pi tool instances	2026-08-24 23:30:34 -07:00
Waleed	011f26d9f3	refactor: remove dead code, orphan modules, and two unused dependencies (#7063) ... * refactor: remove dead code, orphan modules, and two unused dependencies Every symbol below was classified per reference hit as: own declaration / barrel re-export line / vi.mock stub / real production caller. Only symbols with zero real callers are removed. Orphan modules (nothing imports them): - scheduled-tasks/components/schedule-calendar/ — 11 files. Zero references repo-wide, not even a barrel line. - terminal/components/filter-popover/ — re-exported through a barrel, consumed by nothing. - 6 iso-marks primitives (IsoCubeGrid, IsoCubeRow, IsoFourBox, IsoGridPlane, IsoStackedPlanes, IsoStar). mothership.tsx imports only the four Iso*Illustrations, which do not use these. - 4 copilot user-input hooks (useMentionKeyboard, useTextareaAutoResize, useMentionInsertHandlers, useCaretViewport) — barrel line only. Their six siblings in that directory are live and stay. - lib/workflows/application/duplicate-workflow.ts — an orphaned authorized use case. Live duplication goes through lib/workflows/persistence/duplicate.ts, which exports a same-named duplicateWorkflow; the 23 apparent references are all to that one. Nothing imports the application path. Dead exports: - lib/compare/data/feature-catalog.ts (SIM_FEATURES, 939 lines) plus the SimFeature type, FeatureCategory union, and featuresByCategory/featuresByTag helpers that only it used — one unshipped feature-comparison catalog. - createExecutionCallbacks (execution-events.ts) — declaration only, zero other hits repo-wide. Stale vi.mock targets — mocks whose module no longer exists, so they are silent no-ops that make a test look protected when it is not: - @/background/logs-webhook-delivery and @/app/api/webhooks/utils in the webhook trigger route test (its generateRequestHashMock and validateSlackSignatureMock hoisted vars went with it) - @/lib/workflows/subblocks/options in blocks/blocks/logs.test.ts - @/lib/uploads/setup in the S3 client test - @/lib/uploads/setup.server in the files delete and parse route tests — parse/route.test.ts mocks the real @/lib/uploads/core/setup.server on the very next line Dependencies: three and @types/three. Zero imports anywhere, and absent from every config, script, workflow, Dockerfile and chart. Both arrived with the speech-to-speech voice mode, which has since been removed. Deliberately kept: build-chat-animation/ reads as an orphan but build-callout.tsx documents it as parked unwired for reuse. * chore: update lockfile for the three/@types/three removal CI runs bun install --frozen-lockfile, so dropping the two dependencies from apps/sim/package.json without regenerating bun.lock fails the install step. The diff is the two entries plus three's transitives (@dimforge/rapier3d-compat, @tweenjs/tween.js, @types/stats.js, @types/webxr, @webgpu/types, meshoptimizer). One change deserves a note: top-level fflate moves 0.8.3 -> 0.4.8. It was hoisted to 0.8.3 only because @types/three required ~0.8.2; with that gone the remaining top-level consumer is posthog-js, which asks for ^0.4.8. Every other consumer keeps its own pinned nested entry (@shuding/opentype.js 0.7.4, @smithy/middleware-compression 0.8.1), and no source file imports fflate directly, so no resolution changes for anything that was already installed.	2026-08-24 22:54:02 -07:00
Waleed	bdda083f93	refactor: consolidate three drifted copies, close a gate blind spot, fix stale docs (#7062) ... * refactor(workflows): one owner for new-workflow sort order The same ~35-line query — parent condition for workflows and folders, two parallel min(sortOrder) reads, fold to a min, subtract one, fall back to 0 — existed three times: lib/workflows/utils.ts inline in createWorkflowRecord lib/workflows/orchestration/... as a file-private nextWorkflowSortOrder lib/workflows/persistence/duplicate inline, inside the duplicate transaction The first two are character-identical modulo the table alias. The third had drifted: it omits isNull(workflow.archivedAt), which the other two apply, so a folder whose lowest-sortOrder workflow is soft-deleted positioned a *duplicate* differently from a *create*. The folder-side query agrees in all three, which marks it as a copy-paste slip rather than intent. Promotes the helper to lib/workflows/sort-order.ts, taking an optional DbOrTx so the duplicate path can keep reading inside its transaction. Its own module rather than utils.ts because duplicate.test.ts and workflow-lifecycle.test.ts both mock '@/lib/workflows/utils' wholesale — from a separate module the real query still runs under those suites, so their existing sort-order assertions keep their meaning and needed no edits. Note the archived-row behavior itself is not unit-testable here: the shared dbChainMock does not evaluate WHERE predicates. The guarantee is structural — one query builder instead of three means the predicate can no longer drift. * fix(ee): case-fold the stored integration allowlist ee/access-control re-implemented the allowlist intersection instead of calling intersectIntegrationAllowlists, and lost the case-folding: normalization only happened on the envAllowlist !== null branch, so with ALLOWED_INTEGRATIONS unset a stored config went through untouched. Callers compare against blockType.toLowerCase(), so a stored 'Slack' failed to match 'slack' and the block was denied. The access-control UI writes block.type directly and block types are lowercase, so this is not reachable from the UI — but allowedIntegrations is a bare z.array(z.string()) on the wire, so any API client can store mixed case. Replaces the fork with the shared helper. Adds two tests; the first fails against the old code. * fix(queries): forward the abort signal to getFullOrganization useOrganization destructured `signal` from the queryFn and passed it to fetchOrganization, which named the parameter `_signal` and never used it — so the org detail fetch could not be cancelled. Switching orgs rapidly left every prior request in flight, free to resolve out of order. Better Auth takes cancellation two ways and both are already used here: fetchOptions on the params object (session.ts:21) and a second argument (admin-users.ts:129). Uses the former. This was the only `_signal` under apps/sim/hooks. The existing transition test asserted the exact call shape, so it now asserts intent via objectContaining. Adds a test for the signal itself; it fails against the old code. * fix(canvas): select from useWorkflowRegistry instead of subscribing whole check-zustand-v5-selectors matched /use[A-Z]\w*Store\(/, and exactly one Zustand store in the repo is not named with a Store suffix — useWorkflowRegistry. So the store behind the canvas went unchecked, and two bare whole-store subscriptions had accumulated in the action bar, re-rendering it on every registry mutation (clipboard, hydration, pendingSelection, activeWorkflowId). Every other call site in the repo already uses a selector. Widens the pattern to (?:Store|Registry) and fixes both call sites. The widened gate reports these two and nothing else, so there is no cleanup tail. * docs: correct comments that name symbols which no longer exist Each of these points a reader at an identifier that is not in the repo: - table/import-data.ts, table/service.ts — `acquireTablePositionLock` and `nextAutoPosition` were removed with the service.ts split; the surviving lock is `acquireRowOrderLock`, which import-data.ts already imports and calls. service.ts's mention is load-bearing: it exists to tell the reader which other lock this one mirrors, for lock-ordering. - resources/orchestration/restore-resource.ts — named `performRestoreFolder` (the callee is `restoreFolder`) and described a `'workflow'` default it falls back to. There is no such default: resourceType is required and the config lookup is a bare index. Describing a fiction is how a future reader talks themselves into relaxing the total Record to a Partial. - knowledge/search/queries.ts — cited apps/docs/app/api/chat/route.ts, deleted with Ask AI. The k=60 it pins against now lives in the docs search route. - rate-limiter/hosted-key/queue.ts — documented a `waitForHead` method the class does not have; the queue exposes `checkHead` and the polling loop is private to the consumer. - logs/log-views.ts — a "Level 1.5 / 2 / 3" scheme that appears nowhere else; the real contract is the five named views. Dropped, and the three banner rules with it (CLAUDE.md bans banner separators). Comment-only apart from the log-views banners. * refactor(ui): derive three values instead of storing or memoizing them - import-modal: browserId and profileId were state corrected by two effects when a reload dropped the selection. That commits and paints one frame in which the profile still belongs to the previously selected browser — and Import is enabled during it, submitting via a `profiles.find` that searches every browser's profiles. Both now fall back during render, and `selected` searches only the current browser's profiles. Covered by the existing 'never leaves a profile selected that belongs to another browser' test. - workflow.tsx: isWorkflowEmpty was a second useMemo over the same [blocks] dep computing exactly !hasBlocks, allocating its own Object.keys array. Both feed primitives, so neither memo bought identity stability. - thinking-loader: an effect seeding cycleVariant whenever variant is defined, which `shown = variant ?? cycleVariant` can never read. On the one transition where cycleVariant becomes visible (variant going undefined) the cycling effect assigns it in every branch — settle, reduced-motion, and tick — in the same flush, so the seed was never observable. * fix(review): exclude deleted folders from sort order; make the allowlist tests real Three findings from cubic on #7062, all valid. 1. nextWorkflowSortOrder consulted the folder minimum without excluding soft-deleted folders. Because the helper returns min - 1, a deleted folder holding the lowest slot ratchets the floor down permanently — the same class of bug as the archived-workflow one this PR set out to fix, on the other half of the query. lib/folders/orchestration.ts already documents this exact rationale for the folder-creation side of the same algorithm, and the uploads folder manager filters it too; this was the outlier. 2. The two new allowlist tests did not call setEnterpriseOrgWorkspace(), so resolution never reached the group queries and validateBlockType returned early. They passed against the unfixed code when run in isolation and only appeared to fail in a full-file run, where mock state leaked from earlier tests. Verified with 'vitest -t': both now fail without the case-folding fix and pass with it. 3. The restore-resource comment this PR rewrote was itself wrong. A Partial map does not defer the failure into the cascade — the lookup widens to FolderResourceType | undefined and the error lands on the restoreFolder call site. Reworded to say that, and why keeping the check at the mapping matters.	2026-08-24 22:53:32 -07:00
Waleed	167fcb483b	refactor: remove dead orchestration entry points and no-op tests (#7060) ... * refactor(orchestration): remove seven unreferenced perform* entry points Each had exactly one declaration, a barrel re-export, and no caller anywhere in the repo — no route, no application use case, no tool handler, no test. Their Params/Result interfaces went with them where nothing else consumed them; PerformCredentialResult, PerformUpdateWorkflowParams and PerformUpdateWorkflowResult stay, since live functions still use them. Removed: performDeleteCredential, performGetWorkspaceFileShare, performUpsertWorkspaceFileShare, performMoveRenameWorkspaceFile, performUpdateTableDescription, performUpdateWorkflow, performUpdateWorkspaceFileContent. * test: drop assertions that cannot fail Four tests asserted nothing about the code under test: - app/api/copilot/methods/route.test.ts was the directory's only file — it asserted expect(true).toBe(true) against a route that does not exist. - tools/index.test.ts carried a block self-documented as existing "to maintain test count". - mcp/storage/memory-cache.test.ts closed a delete-a-missing-key case with expect(true).toBe(true); it now asserts the call resolves without throwing. - realtime/src/index.test.ts checked typeof roomManager.x === 'function' and typeof process.on === 'function', both of which tsc already proves. Removing the realtime cases leaves a real gap: index.ts registers uncaughtException, unhandledRejection, SIGINT and SIGTERM handlers with no coverage. Better to have that gap visible than papered over by a test that would pass with the handlers deleted.	2026-08-24 21:58:41 -07:00
Waleed	ea9207d367	fix(sse): rotate workspace streams without gaps (#7061) ... * fix(sse): rotate workspace streams without gaps * test(sse): cover delivery across rotation	2026-08-24 21:58:17 -07:00
Waleed	1e24a6ad6e	chore(skills): drop Cursor Bugbot from the babysit review loop (#7059)	2026-08-24 21:04:54 -07:00
Waleed	ef42424b2b	fix(sse): bound workspace SSE connection lifetime (#7058) ... * fix(sse): bound workspace SSE connection lifetime Teardown ran only from the request abort listener and the stream cancel callback, both of which fire only when the runtime reports a client disconnect. Nothing else bounded the connection, so a missed report left the pub/sub handler, the heartbeat timer, and the stream's undrained queue held for the life of the process. Add a jittered lifetime ceiling checked on the existing heartbeat tick, tighten reclaim for a vanished consumer via desiredSize, remove the abort listener on every teardown path, and run full teardown when a heartbeat enqueue fails. Log the close reason so opens minus closes is observable. * fix(mothership): resync chat caches after an SSE reconnect gap task_status events are transient and never replayed, so any window with no open connection can drop a create, rename, delete, or completion. The chat hook reconnected silently and reconciled nothing, leaving list and detail caches stale until an unrelated action refreshed them. Resync on reconnect, on the first open of a re-subscription, and on a first open that only succeeded after an error, matching the pattern useMcpToolsEvents already uses for the same gap. * fix(mothership): keep reconnect resync off locally streaming chats The resync invalidated every chat detail, including one whose stream this client is rendering optimistically. Refetching there replaces the local transcript with a server copy that does not yet hold the in-flight message, which is exactly what status events avoid via shouldSkipDetailInvalidationForStreamEvent. Filter the detail invalidation with the same isLocalOptimisticActiveStream check. Those chats reconcile when their own stream finishes. * fix(mothership): only skip resync for a stream still running Optimistic markers alone were the skip condition, but a finished turn can leave activeStreamId and its live-assistant message cached when finalization skips detail invalidation for a queued follow-up. That chat would then be excluded from every future resync — permanently, since only a refetch clears the markers, and the resync was the refetch. Gate the skip on a non-terminal streamSnapshot status so it covers turns that are genuinely still streaming. Exports isTerminalStreamStatus, which was already the private check for this in effective-transcript. * fix(sse): raise the ceiling and narrow reconnect resync to the lists Deciding from cache whether a chat is still streaming is not reliable — the optimistic markers outlive the turn, and each refinement of that predicate exposed another state where it answers wrongly. Drop it: the resync now invalidates only the workspace lists, which is always safe, and chat detail reconciliation stays as it is today rather than being half-solved here. Raise the ceiling to 4h, matching lib/realtime/event-stream-route.ts. A healthy client is drained and so is never unread; the unread check is what reclaims a vanished consumer, and it does so within minutes. A short ceiling would therefore only force reconnects on the connections that are working, and every reconnect is a window where a transient event can be missed. Retention stays bounded by the ceiling instead of by process uptime.	2026-08-24 20:46:09 -07:00
Waleed	49a3399dd2	fix(ui): unify branded error pages (#7057) ... * fix(ui): unify branded error pages * fix(desktop): match canonical chip chrome * test(ui): update error chip mock * fix(desktop): package offline font reliably	2026-08-24 20:42:41 -07:00
Vikhyath Mondreti	b7b0be8eef	fix(memory): close app-service leak paths behind the per-task memory ramp (#7056) ... * fix(memory): close app-service leak paths behind the per-task memory ramp Prod app tasks climb from ~3.0 GB to 9+ GB average (20.6 GB worst task) with uptime and reset only on deploy. The growth lives in the main Next.js process — the isolated-vm worker is a separate, bounded child and its disposal already runs in finally on every path. Four fixes at the sites that can actually accumulate there: - copilot stream teardown (lib/copilot/request/lifecycle/start.ts): the activeStreams registration, the 250ms Redis abort poller, and the SSE keepalive were acquired outside the try whose finally releases them, so a throw before the lifecycle started (e.g. resetBuffer on a Redis blip) or a throw inside the ordered teardown orphaned two immortal intervals and the registration. Add an idempotent backstop in the orchestration's outer finally; the ordered teardown sets a flag so the normal path pays nothing. - large-value cache (lib/execution/payloads/cache.ts): expiry was enforced only inside later cache calls, so on a quieting instance the last entries — parsed object graphs worth a multiple of their JSON-byte accounting — sat indefinitely instead of for the 15-minute TTL. Add a self-retiring, unref'd sweep interval, and export occupancy stats. - memory telemetry (lib/monitoring/memory-telemetry.ts): add the large-value cache occupancy and detached-context count to the periodic snapshot so the JSON-bytes-vs-heap amplification and context retention are readable from the same log line as heapUsedMB. - BYOK rotation cursors (lib/api-key/byok.ts): the tenant-keyed cursor Map had no delete, TTL, or ceiling. Bound it with an LRU; evicting an idle pool's cursor just restarts its rotation at index 0. - collab-doc converter (lib/collab-doc/converter.ts): the DOM guard read the bundled module's `window` binding while the install wrote `globalThis.window` — the same bundler mismatch documented for TipTap in next.config.ts — so a runtime where the two disagree re-allocated a multi-MB jsdom window on every conversion. Guard and install now go through globalThis, and the jsdom window is a module singleton either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * improvement(execution): idle-TTL touch-on-read + LRU eviction for the large-value cache Entry lifetimes were absolute-from-insert and eviction order was insertion order, so a value a live run kept referencing could expire or be pressure-evicted mid-use — while a genuinely idle entry survived the full window. Every authorized read now refreshes the expiry and moves the entry to the back of the eviction order: expiry and eviction only ever take entries nothing has read for a full TTL, and pressure eviction takes the least-recently-used recoverable entry. Strictly fewer mid-execution misses; TTL values, the admission budget, and the sole-copy (non-recoverable) eviction protection are unchanged. Touching stays behind the scope check so an unauthorized probe cannot extend a lifetime. Module TSDoc now records the standing constraint: the warm pass runs once per execution start, so the TTL must outlive the warm-to-first-reference gap — do not shorten it until warming is per-block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-08-24 19:34:30 -07:00
Theodore Li	7b761ba560	feat(setup): add incremental Chat setup (#7043) ... * feat(setup): add incremental Chat setup * fix(setup): retain existing Compose Chat config	2026-08-24 19:22:42 -04:00
Justin Blumencranz	49e2bb5da7	fix(copilot): show custom block names in read tool rows (#7044) ... * fix(copilot): show custom block names in read tool rows * fix(copilot): refresh custom block metadata after hydration	2026-08-24 15:06:02 -07:00
Waleed	3ce99530f8	chore(legal): update privacy policy (#7050) ... * chore(legal): update privacy policy * fix(legal): remove source metadata	2026-08-24 14:48:55 -07:00
Waleed	626f6e5f1b	fix(consent): enforce consent-aware analytics (#7049) ... * fix(consent): enforce consent-aware analytics * fix(consent): preserve script config literals * fix(consent): adapt readonly scripts for provider	2026-08-24 14:17:30 -07:00
Vikhyath Mondreti	0a5b3801ea	feat(secrets): let workspace secrets opt out of redaction (#7045) ... * feat(secrets): let workspace secrets opt out of redaction * fix(secrets): certify no sandbox exemptions once the registry is incomplete * feat(secrets): carry visible secret values on the v2 list and document visibility * fix(secrets): read visible values by own property so prototype-named secrets cannot poison the list	2026-08-24 13:46:57 -07:00
Waleed	3251bf13a6	fix(ci): make the utils gate see the wrapped forms of what it bans (#7047) ... `check-utils-enforcement.ts` scanned line by line, and every idiom it bans is a multi-token expression the formatter wraps at 100 columns. So it printed `✓ No banned patterns found` while eleven files carried the wrapped form of e instanceof Error ? e.message : fallback which CLAUDE.md mandates `getErrorMessage` for. The same class as the two blind spots already fixed in check-react-query-patterns. Patterns now run against the whole file, with match offsets mapped back to line numbers by binary search over the line-start table — verified against every offset of a multi-line fixture. Eight of the eleven are now `getErrorMessage(error, fallback)`. `auto-layout-utils` collapses a redundant `instanceof ApiClientError` arm on the way, since that class extends `Error`; `upgrade.ts` keeps its `rawBody ?? message` arm, which the helper cannot express, and only its tail collapses. The other three stay, because the helper genuinely does not fit, and they carry a `// utils-lint-allow: <reason>` annotation — the same escape hatch check-react-query-patterns already has, which this gate lacked: - the two auth routes return the message to an unauthenticated caller, so a non-Error throw must surface the fixed copy rather than its own text. `getErrorMessage` passes a thrown string straight through, which is the disclosure shape #7015 closed. - `e2b.ts` probes E2B's own error shape — a record-like carrying `message` or `value` — which has no equivalent. An annotation with no reason does not suppress, so the hatch cannot be used to silence a finding without saying why. Also corrects the header, which claimed Biome's `noRestrictedImports` covers "crypto named imports". It lists only `nanoid` and `uuid`. Named crypto imports pass both gates deliberately — server code building cipher IVs wants node's crypto, not the cross-context wrapper — and the comment asserting otherwise would mislead the next person auditing this. Verified the gate can fail in both directions: reintroducing a wrapped ternary reports it, and emptying an annotation's reason reports it too.	2026-08-24 13:31:16 -07:00
Waleed	aba681133a	refactor: remove five more dead prop chains (#7046) ... * refactor(auth): drop the isProduction prop nothing on the signin path reads Same shape as the `isWorkflowRunning` removal: declared, required, threaded through every layer, and never read at the end of the chain. `SocialLoginButtons` declares `isProduction: boolean` as a REQUIRED prop and never reads it, so every caller had to produce and forward a value that was discarded. Neither `login-form` nor `signup-form` reads it either — each only declares it, destructures it, and passes it down. `signup-form` forwards it twice, through its own inner `SignupFormContent` hop. With the chain gone, `getOAuthProviderStatus` has no consumer for the `isProduction: isProd` it returned: the pages destructured it only to forward it, and `/api/auth/providers` already takes just the three availability flags. So the return value and its `isProd` import go too. `isProduction` stays alive where it is genuinely used — `verify-content.tsx` branches on it and hands it to `useVerification`, and imports `isProd` directly rather than through this helper. That path is untouched. Found by the rule enabled in #7037: it was the only `.tsx` unused-parameter warning in `apps/sim`. (cherry picked from commit 331c2ed2ed) * refactor: drop two more props declared, threaded, and never read Same shape as the two already in this PR, found by sweeping the rest of the unused-parameter list for params callers actively compute and pass. `FieldItem.level` is the worse of the two. It is a required `level: number` that the component never reads, and `FieldTreeNodes` exists to thread it: declared, destructured, handed to `FieldItem`, and incremented on every recursion (`level={level + 1}`) from a `level={0}` seed. So a depth counter was carried through an arbitrarily deep tree to feed a component that ignores it. Indentation comes from the nested wrapper divs (`ml-1.5 pl-2.5`, `ml-3 pl-2.5`), not from the counter — removing it changes no rendering. `useMentionMenu`'s `onContextSelect` is a required prop carrying the TSDoc "Callback when a context is selected". The hook never invokes it, so that contract is unimplemented and a future caller would reasonably rely on it. Only the dead hand-off goes there. `addContextNotified` stays: the caller invokes it directly at five sites, and the ref sinks behind it keep its identity stable for those. Context selection has always worked because the caller does the work itself, not because the hook calls back. (cherry picked from commit 110ba76df3) * refactor: drop two more dead prop chains in the sub-block editor `GroupedCheckboxList` declares `title` (required) and `maxHeight` and reads neither. It renders its own hardcoded copy instead — `Select PII Types to Detect` for the header and `PII types` for the field label — so a block author who sets `title` on a `grouped-checkbox-list` subBlock gets silence, and the `maxHeight = 400` default implies a scroll ceiling that is never applied. Both props go, along with the two values `sub-block.tsx` was passing. `flatTagList` was threaded through the recursive tag renderers to a dead end: declared on `NestedTagRendererProps`, inherited by `FolderContentsProps`, destructured in both, forwarded once more, and read by neither. Its real consumer is `flatTagIndexMap`, built from it at the top level and documented "Map from tag string to index for O(1) lookups" — so the array was being carried alongside its own index through arbitrary nesting depth. The top-level memo and its length checks stay; only the descent goes. Note the component's copy is PII-specific while its name and props present as generic. Renaming it is a separate call, not made here. Both removals were caught mid-flight by `tsc`: my line patterns also matched a live `flatTagList` on `KeyboardNavigationHandler` and a live `title` on `Switch`, which is exactly why the type-check runs before the commit and not after. (cherry picked from commit 5db44f347b) * refactor(custom-blocks): drop the workspaceId three mutation hooks never use `usePublishCustomBlock`, `useUpdateCustomBlock` and `useDeleteCustomBlock` each take `workspaceId?: string` and never read it. `custom-block-detail.tsx` passes it to all three. The parameter looks like it was meant to narrow the invalidation to `customBlockKeys.list(workspaceId)`, but `lists()` is the level CLAUDE.md's targeted-invalidation rule actually prescribes, and it is a correct superset. So the invalidation is right as written and the parameter is simply vestigial — removing it is the honest fix, and narrowing the key would be a separate call with its own risk of under-invalidating. Worth recording that these three were reported to me as having zero callers and therefore being dead exports. They are not: the search that produced that claim omitted `apps/sim/ee`, where all three are used. (cherry picked from commit 2d0854a587)	2026-08-24 13:30:52 -07:00
Bill Leoutsakos	445ef62880	fix(oauth): bind update access to selected credential (#6999) ... * fix(oauth): bind update access to selected credential * fix(oauth): guard unresolved connector credentials * fix(oauth): clear stale connector return context * fix(oauth): fail closed when reconnect target disappears * fix(oauth): wait for reconnect credential lookup * fix(oauth): refresh resolved connector credential --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>	2026-08-24 12:32:26 -07:00
Justin Blumencranz	472532e1a3	feat(tables): confirm view deletion (#7041)	2026-08-24 12:17:59 -07:00
Waleed	6d313a4a94	refactor(renderer): drop the isWorkflowRunning prop the views never read (#7040) ... Removing only the unused binding and keeping the prop was half a fix. The views declared it, the app passed it, and nothing read it — so the prop was dead, and dead code does not become live by being documented. Its TSDoc claimed it "holds every block's action swell open". That behavior does not exist in either view. Keeping the prop on the chance someone wants it later is the speculative-generality smell: if the toolbar should pin open during a run, that gets implemented deliberately and the prop comes back with logic behind it. Removed from both view interfaces and from both call sites. The store subscription stays — `workflow-block.tsx` and `subflow-node.tsx` each passed the same value twice, once to the dead view prop and once to `ActionBar`, which has 28 real reads and is what the surrounding TSDoc is actually describing when it says the flag "only swaps Run for Stop and disables mutations". `workflow-edge-view` uses it too and is untouched. The renderer test that passed it loses the argument. Worth noting it set the flag to stage a workflow run, and since the view ignored it those two cases were never exercising the run state they name.	2026-08-24 12:04:56 -07:00
Vikhyath Mondreti	cc611180be	improvement(provenance): attribute stored-envelope display reads to their execution (#7039) ... * improvement(provenance): attribute stored-envelope display reads to their execution A display materialization of an execution log imports the row's stored provenance envelopes into throwaway registries, and each import of an incomplete envelope re-emitted the registry's own summary — per envelope, per view, carrying counts and a workspace but never the execution id. A reader repeatedly materializing the same stored rows produced hundreds of identical lines that could not say which executions to go look at, and the volume scaled with views of a state that was fully recorded when the run wrote it. Verified against production before changing anything: essentially no new incomplete envelopes are being stored since the writer fix shipped, and no data drains exist — the stream is bounded re-reads of old rows through the display paths, not a live producer. The display registries are now staged — the existing concept for a registry that filters one value for a caller that reports against the real boundary — and each display function reports once per materialization with the execution id, workflow, workspace, and the parts that could not be vouched for. Severity is preserved: an incomplete stored envelope stays at warn, a malformed one stays at error. Projection behavior is unchanged everywhere — incomplete and malformed envelopes still fail their values closed exactly as before; only the reporting moves to the boundary that knows the execution. * improvement(provenance): fold the incomplete-envelope predicate and pin dual-site reporting Review pass over the previous commit: one helper instead of three copies of the incomplete-envelope check, the staged TSDoc generalized to cover both of its uses, and the block-outputs entry point's two-site reporting of one run envelope documented and pinned rather than left implicit. * improvement(provenance): classify every unusable stored envelope at the display boundary Review findings from the first round, both accepted: a present-but- malformed block or run envelope was withheld with no attributed line, and a complete envelope whose entries fail decryption latched the staged registry with only the unattributed entry-level error. Fault classification moves into the one import helper the display paths share, which now returns the registry and the fault together: absent is not a fault, unparseable is malformed, unable-to-vouch is incomplete, and a complete envelope whose registry latched during import — entry decryption is the only latch on that trusted path — is undecryptable. Every consumer reports through the same table, severity per kind, so the exact-value loop stops being the only site that could name a malformed envelope. Withholding behavior is unchanged at every site.	2026-08-24 11:58:11 -07:00
Vikhyath Mondreti	04380b79a5	fix(webhooks): requeue deliveries dropped by retryable setup infrastructure failures (#7038) ... * fix(webhooks): requeue deliveries dropped by retryable setup infrastructure failures * fix(webhooks): restore terminal log on failed requeue and make retry backoff abort-aware	2026-08-24 11:28:27 -07:00
Waleed	cae80c5f20	chore(lint): turn on the rules that would have caught the dead code (#7037) ... Three rules were off, so nothing enforced them. Measured, fixed the sites, and enabled them where the cost is bounded. `noAccumulatingSpread` — 2 violations, both real O(n²) reducers, both now `Object.fromEntries`. One duplicates a block's subBlocks on every block duplication; the other rebuilds a Record from every workspace env var. Enabled repo-wide. `noUnusedVariables` / `noUnusedFunctionParameters` — 633 repo-wide, but only 6 under `packages/`. Fixed those 6 and enabled both at error for `packages/**` via an override, which permanently covers 979 files. `apps/sim`'s remaining 627 are left deliberately: that is a sweep of its own, and a rule enabled with 627 outstanding warnings teaches people to ignore it. This is the class of rule whose absence let the dead code in #7019 accumulate — eleven unread loggers, a whole unimported file, write-only locals — none of which any gate could see. Two of the six were in `workflow-renderer`, where the fix is narrower than it looks. `isWorkflowRunning` is destructured-but-unread in both the block and subflow views, and the app passes it from `workflow-block.tsx` and `subflow-node.tsx`. Its TSDoc claimed it "holds every block's action swell open"; nothing reads it, so that behavior does not exist. Removing the prop breaks the callers and implementing it is a UX decision — there is adjacent logic deliberately not pinning the toolbar during a handoff. So only the unused binding goes, and the TSDoc now says what is true. Not enabled: `noDocumentCookie` (3 sites, and its fix is the CookieStore API, which is a browser-support call) and `useExhaustiveDependencies` (384 errors).	2026-08-24 11:10:19 -07:00
Waleed	edf07ec3cd	fix(vllm): support LM Studio endpoints (#7036) ... * fix(vllm): support LM Studio endpoints * fix(vllm): validate compatible base URLs * fix(vllm): guard discovery URL validation	2026-08-24 10:58:07 -07:00
Waleed	fbeea53bec	fix(files): normalize encoded embedded ids (#7035)	2026-08-24 09:51:26 -07:00
Waleed	efe8a14bcc	fix(ci): give push builds a base their audits can actually read (#7033) ... Push builds fail the migration audit: ✗ Migration safety check could not run. Cannot diff against 'HEAD~1'. `actions/checkout` sets no `fetch-depth`, so it defaults to 1 — a single-commit clone in which `HEAD~1` does not resolve. Both diff-based audits named `HEAD~1` as their push base, so neither has ever had a base to read. The migration audit answered that with `✓ No new migrations to check` and exit 0, so it had never run on a push build at all; #7022 made it say it could not run instead, which is what surfaced this. The block-registry check reports `⚠ … skipping` on the same input — visible, and equally never run. `HEAD~1` was the wrong base regardless. It names the last commit, so a push carrying several commits audits the tip and lets every earlier commit through: 3-commit push, HEAD~1 base: mig3.sql 3-commit push, before base: mig1.sql mig2.sql mig3.sql The base is now `github.event.before` — the tip the branch had before the push, which is what GitHub provides for exactly this. It is fetched by SHA at depth 1; the audits diff two tips and need no common ancestry between them. Resolved once in a step both audits read, so the two cannot drift apart. `HEAD~1` survives only as the fallback for an all-zero `before` (a new branch, with no predecessor to diff), which is what `fetch-depth: 2` now covers. Verified: both audits accept a raw SHA base and pass; the multi-commit case above is a real reproduction, not a description.	2026-08-24 01:12:27 -07:00
Bill Leoutsakos	528b34f564	fix(workflow): hide idle nested subflow end handles (#6976) ... * fix(workflow): hide idle nested subflow end handles * perf(workflow): avoid repeated subflow edge scans * perf(workflow): stabilize subflow edge selector --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Waleed Latif <walif6@gmail.com>	2026-08-24 00:58:03 -07:00
Waleed	67fc2aeb30	fix(ci): require a default export before treating a file as a route entry (#7028) ... * fix(ci): require a default export before treating a file as a route entry Follow-up to #7026, which added `error.tsx` to the entry filenames and with it picked up `[workspaceId]/components/error/error.tsx` — named like a boundary, and not one. It exports `ErrorShell` and `ErrorState` for the thirteen real boundaries to use; Next would reject it as a boundary for having no default export. Counting it inflated the coverage number and would have recorded a shared component in the graph-weight baseline as though it were a route. The filename was never the right test. Every convention-composed entry must default-export the thing Next renders, so that is the discriminator now. Entry count goes 60 → 59, and all thirteen real `error.tsx` boundaries still walk. Also adds `template.tsx` and `default.tsx`. Neither exists under `app/workspace` today, so this changes nothing now — but the enumeration claims to cover what Next composes, and leaving two out makes that claim false the day someone adds one. Both raised in review on #7026 (Cursor and Greptile respectively); I merged before reading them, so this lands separately. * fix(ci): count every form that declares a default export `export { default } from './page'` is a valid Next entry and the regex required `as default`, so such an entry would have dropped out of the walk and skipped both the registry gate and the graph-weight ratchet — silently, which is the dangerous direction for a discriminator to fail in. Latent rather than live: the form appears once under `app/workspace`, in a barrel, not in an entry filename. Four forms now count — `export default …`, `export { default } from`, `export { default, … } from`, and `export { X as default }`. `export { default as X }` still does not: it re-exports another module's default under a name and leaves this one without one. Verified all ten variants, including that last distinction. Raised by both Cursor and Greptile on #7028.	2026-08-24 00:46:50 -07:00
Waleed	82b02fb4d9	improvement(ui): standardize modal default actions (#7029) ... * improvement(ui): standardize modal default actions * fix(ui): keep aggregate deletes on safe default	2026-08-24 00:43:08 -07:00
Waleed	1aa714cf91	fix(ci): stop failing the API audit for adding a compliant route (#7027) ... `totalRoutes` sits at 1162 and the repo has exactly 1162 routes, so the next route fails CI whether or not it is contract-backed: API validation audit failed: - route count increased from 1161 to 1162 The invariant worth holding is that every route has a contract, and `nonZodRoutes` states exactly that. It is 0, and it rises the moment a route ships without one — `zodRoutes === totalRoutes` today, so the total adds no information the other two counters do not already carry. What it adds instead is a habit. The only way past it is editing the number, and this file holds seven other baselines that work only while nobody bumps a baseline casually. The total is still printed; it is no longer a failure. Verified both directions: a compliant new route passes where it previously failed, and a route without a contract still fails through `nonZodRoutes`.	2026-08-23 19:58:43 -07:00
Waleed	f37c24e541	fix(ci): walk every route entry the workspace app composes, not just pages and layouts (#7026) ... The tool-registry guard collected `page.tsx` and `layout.tsx`, and Next composes three more entries by convention: `error.tsx`, `loading.tsx`, `not-found.tsx`. Twenty-six exist under `app/workspace` and none was walked. `error.tsx` is always a Client Component — Next requires it — so a registry edge there reaches the browser bundle exactly as one from a page does. Coverage goes from 34 entry graphs to 60. Nothing new is reported: the hole was unexploited, and closing it costs nothing. The root deliberately stays at `app/workspace`. Widening it to `app` reports `(interfaces)/resume/[workflowId]/[executionId]/page.tsx`, a Server Component (`runtime = 'nodejs'`, `force-dynamic`) whose `PauseResumeManager` import resolves server-side and never reaches a client bundle. The guard cannot distinguish server from client entries, so it stays where its premise holds.	2026-08-23 19:19:31 -07:00
Waleed	cc16d23d90	fix(react-query): close the lint's blind spots, and the drift they hid (#7020) ... `check-react-query-patterns.ts` reported a clean strict zone while never looking at part of it. Two gaps in one regex: `\buseQuery\s*\(` does not match `useQuery<Row[]>({ ... })` — a type argument sits between the name and the paren. Twenty query calls carry one, ten of them inside the zero-tolerance zone, so that zone's "0 violations" was partly a statement about what the scan could see. `useQueries` was absent from both the call pattern and the file pre-filter, where `\buse(Query|...)\b` rejects it on the trailing `s`. All sixteen call sites were unscanned, and its options nest one level deeper — inside a `queries` array — so it needs its own pass per entry rather than one that reads the wrapper and takes a single `staleTime` anywhere inside as covering them all. With both closed, three real violations surfaced: - `knowledge-base-selector` served `knowledgeKeys.detail(id)` with an inline `60 * 1000` while `useKnowledgeBaseQuery` serves the same cache key from `KNOWLEDGE_BASE_DETAIL_STALE_TIME`. The two agree only by coincidence, and TanStack resolves staleTime per observer, so tuning the constant would have left this component on the old window for the same entry. - The same call dropped the `AbortSignal`, which `fetchKnowledgeBase` accepts. - `use-permission-config` gave `staleTime` as a literal with no named constant. The new `stale-time-literal` category makes the second half of the CLAUDE.md rule enforceable — it required a named constant, and only the presence of `staleTime` was ever checked. `0` is exempt: it is the sentinel for "always refetch", not a window anyone keeps in step with a prefetch. Verified the new rules can fail by reverting each fix and watching the audit report it, then restoring.	2026-08-23 19:09:03 -07:00
Waleed	7e0d8681d2	fix: surface an unbilled run, and clamp the google-docs page cap (#7025) ... Two places where a failure is reported as something smaller than it is. **A run that is never billed logs as a notification problem.** The usage safety net re-records billing when an earlier step threw before the single record call, and its own failure went into a bare `catch {}`. With a degraded database the user lookup throws first, the re-record hits the same database and is swallowed, and the only line emitted reads "Usage threshold notification check failed (non-fatal)" — which is true of the outer failure and badly wrong about the inner one. It now logs at error with the execution and workflow ids, and says the run may be unbilled. The outer warn still covers the email path it was written for. **google-docs can ask Drive for a negative page.** `remaining` was `maxDocs - previouslyFetched` unclamped, where its google-slides twin carries `Math.max(0, …)` under the comment "Last-page precision". Both then run `if (documents.length > remaining) documents = documents.slice(0, remaining)`, and a negative `remaining` makes that guard true for any non-empty page while `slice` counts from the end — keeping the leading documents and dropping the trailing ones, where the cap says to keep none. Reachable when `maxDocs` is lowered while a sync cursor persists. google-drive guards the same case with an early return; google-docs had neither.	2026-08-23 19:08:01 -07:00
Waleed	0cbff0e01b	fix(settings): report a failed settings write instead of reporting success (#7023) ... The PATCH catch answered `{ success: true }` with 200, so a failed upsert was indistinguishable from a saved one. `useUpdateGeneralSetting` is optimistic: `onMutate` writes the new value into the cache and calls `syncThemeToNextThemes`, and `onError` restores the previous settings. `requestJson` only throws on a non-2xx, so `onError` could never run — the rollback and its theme re-sync were unreachable code. A user toggling a consent-shaped setting (telemetry, email opt-out) saw it applied and it was not saved, until a later refetch quietly reverted it. The catch now returns 500, which is what the mutation was already written to handle. Left alone deliberately: GET still falls back to `defaultUserSettings` on error. Failing it would take the settings page down on a transient read, and the value of changing it is a separate judgement from this one. Covered by a route test that drives the failure through the real handler. Verified it fails when the 200 is put back.	2026-08-23 19:07:42 -07:00
Waleed	297e970b09	refactor: delete code nothing reaches (#7019) ... `biome.json:101-102` turns off `noUnusedVariables` and `noUnusedFunctionParameters`, so none of this was ever going to be flagged. Everything here was confirmed by grepping the symbol across `apps/` and `packages/` and finding only its own declaration; `tsc --noEmit` then proves each deleted binding was unread, since a read one fails to compile. - Eleven module-scope loggers that nothing logs through, with the now-orphaned `createLogger` import each left behind. - `execute-platform-context-use-case.ts` — the whole file. No importer, no barrel, and neither export is named anywhere. - `routeToolCall` and, once it goes, `ToolRoute` and `ToolRouteTarget` with it. The catalog accessors around them stay live. - `processPastChat`, superseded by `processPastChatFromDb`. It carried the last `boundary-raw-fetch` exemption in the file. - `withMessageId`, pasted into three server tools and called in none. - Write-only locals: `activeSubagent` (assigned twice, read never — the scoped maps replaced it), `resolvedReadPath`, `workflowPath`, and `workflow` in an execution-core destructure. - `ACCEPTED_AUDIO_TYPES` / `ACCEPTED_VIDEO_TYPES`, never wired to an accept attribute the way their live sibling is. - Unused `catch` bindings in `error-extractors.ts` and `defaults.ts`. `diff-engine.ts` drops a `proposedSubKeys.includes(key)` guard that the `!proposedSub` check three lines down already covers: a key absent from the proposed block reads back `undefined` there, and so does a key present with a nullish value. Same answer on every input, without the O(n) scan per iteration.	2026-08-23 19:03:27 -07:00
Waleed	95d08d2a49	fix(ci): stop the migration safety audit from passing on a branch it never read (#7022) ... * fix(ci): stop the migration safety audit from passing on a branch it never read The zero-downtime audit reports the same empty file list for 'this branch adds no migrations' and 'I could not diff against the base', and the second prints as `✓ No new migrations to check` with exit 0. Reproduced on this checkout: $ bun run scripts/check-migrations-safety.ts origin/does-not-exist-branch ✓ No new migrations to check. exit=0 `changedMigrationFiles` returned `[]` whenever `git diff` failed, with a comment deferring the decision to the caller — but the caller only recognised a missing git binary (`git rev-parse HEAD === null`), never an unusable ref. CI supplied exactly that input. `git fetch --depth=1 … 2>/dev/null || true` hid a failed fetch, leaving `origin/<base>` absent, so a PR adding a destructive `DROP COLUMN` would clear the only guard on production DDL with a green check. Two halves: - The audit now distinguishes the cases. Absent git is still the one legitimate skip and is checked before the diff; a diff that fails with git present raises `BaseRefUnusableError` and exits 1. - The fetch is its own step with no `|| true`, so a failure fails the job. Depth stays 1: without a merge-base the audit diffs the two tips, which under `--diff-filter=AM` is exactly the migrations new on the branch. Covered by a test that runs the script end to end, since the defect was in the exit code rather than in any function's return value. Verified it fails when the throw is reverted to `return []`. * fix(ci): fetch the base ref once, and stop swallowing the failure The same `git fetch --depth=1 … 2>/dev/null || true` appeared in both base-ref audits. Fixing only the migration one would have left the identical defect a few steps above it. Neither audit can tell an absent base ref apart from a branch that changed nothing. The block-registry check at least degrades to a visible `⚠ Could not diff against base ref — skipping`; the migration audit printed `✓ No new migrations to check` and exited 0. Both now share one fetch step that fails the job when it fails.	2026-08-23 18:59:54 -07:00
Waleed	cc087498ae	refactor(utils): add slugify and adopt it at the eight sites that hand-rolled it (#7018) ... The same three-step derivation — lowercase, collapse each non-alphanumeric run to a hyphen, strip the leading and trailing one — sat in eight files. Two of them carried a TSDoc line whose only job was to warn that they mirrored a third (`instance-org.ts`: "Derives a slug the same way the admin organization API does"; `consolidate-users-into-organization.ts`: "Mirrors the slug derivation used by POST /api/v1/admin/organizations"). A comment asserting two implementations agree is the shape duplication takes when it cannot be checked. All eight were semantically identical. Two anchored the strip with `-+` rather than `-`, and one followed it with a `--+` collapse, but `[^a-z0-9]+` has already collapsed every run by that point, so neither could ever match more than the single-hyphen form. Nothing changes. Truncation stays at the call sites. Four of them bound the result — at 24, 64 and 80 — and only `copy-chats.ts` strips again afterwards, because slicing can land mid-run and leave a trailing hyphen the earlier strip never saw. Folding a `maxLength` into the helper would have had to pick one of those behaviors and silently impose it on the others. `artifact-stylesheet.ts` keeps its copy: it lives inside the `SIM_ARTIFACT_SHELL` template literal and runs in the viewer's browser, where there is no import to resolve.	2026-08-23 18:57:52 -07:00
Waleed	49593b3191	refactor: replace hand-rolled utilities and dead code with the shared forms (#7021) ... * refactor: replace hand-rolled utilities and dead code with the shared forms Each of these has a mandated helper or an established accessor in the repo that the site predates or missed. All are behavior-preserving: - `omit()` for the three `Object.fromEntries(Object.entries(x).filter(...))` block-input filters, which also recovers the `Omit<T, K>` typing that `Object.fromEntries` erases to an index signature. - `getErrorMessage()` for the inline `instanceof Error` message ternary. - `getBlock()` for two `getAllBlocks().find((b) => b.type === x)` scans, one of them inside a loop over selected tools. The same file already resolves the same values through `getBlock`. - A memoised `Map` for three `.find()`-by-id scans over the workspace skill list, one of them inside a render `.map()`. - `SELECTOR_SEARCH_STALE` for three copy-pasted `15 * 1000` literals. They are deliberately shorter than `SELECTOR_STALE`, so this is a new named constant rather than a fold into the existing one. - Tailwind classes for the static half of two duplicated anchor styles, keeping only the genuinely dynamic `left`/`top` inline. - Dropped the unused `catch` bindings on three intentional JSON-parse swallows. `panel.tsx`'s run-button gate loses a `TODO`-stubbed `hasValidationErrors = false` and the `isWorkflowBlocked` term built on it. That term was dead twice over: it reduced to `isExecuting`, and the enclosing expression is already guarded by `!isExecuting`. * fix: guard the registry lookups, and scope the search-stale doc to its callers `getBlock` normalizes its argument with `type.replace(...)`, so it throws on `undefined` where the `getAllBlocks().find(...)` it replaced returned `undefined` harmlessly. Both call sites can be reached without a type: `tool-input` reads `state.blocks[blockId]?.type`, which is undefined once the block is deleted while the panel is mounted — and `Record` indexing hides that from the compiler, so it would have thrown during render. `agent-handler`'s `tool.type` is optional and the compiler did catch it. Also index the skill lookup in `resolveSkillsLabel`, which runs a `.find()` inside a `.map()` for every block on the canvas — the case the memoised map in `skill-input` addressed for one component while leaving the hot path. `providers/utils.ts` keeps its `getAllBlocks().find(...)`: it takes the registry as an injected dependency precisely so a client-reachable module never imports it, and reaching for `getBlock` there would cross that boundary. The new constant's doc claimed search-backed selectors take a shorter window. Several still sit on `SELECTOR_STALE`, so it now describes the value its three callers share rather than asserting a rule the tree does not follow. * fix: guard the second registry lookup in tool-input `selectedTools` validates only `value[0]?.type` and then casts the whole array, so a persisted workflow whose later rows lost their `type` yields `undefined` here — the cast is what makes the compiler believe otherwise. `getBlock` normalizes with `type.replace`, so that throws during render.	2026-08-23 18:41:41 -07:00
Waleed	fc7aa66a39	fix(api): withhold internal failure messages from internal route responses (#7015) ... An orchestration result carrying `errorCode: 'internal'` holds whatever text the fault happened to have — `workflow-lifecycle.ts` catch-alls return `toError(error).message`, which is the driver's failed SQL. Three application helpers projected that straight into an `OrchestrationError`, and the internal route policy rendered its message into a 500 body, so raw SQL reached clients. The v2 envelope already scrubbed the same failures; internal routes did not. `messageForOrchestrationError` already encoded the rule and two sites honored it. The three that hand-rolled it disagreed, and `workflow-vfs` disagreed with itself: it defaulted the code with `?? 'internal'` but compared the raw `errorCode` against `'internal'`, so an uncoded failure was classified internal and still rendered its own message. Pair the two in `throwOrchestrationFailure` so a code and its message cannot disagree, and scrub at the internal route boundary as well, matching v2 — no call site authors a curated `internal` message, so nothing legitimate is masked, and site N+1 cannot reopen this by forgetting the rule.	2026-08-23 17:18:03 -07:00
Waleed	a20a5465ef	perf(db): optimize recurring query paths (#7014) ... * perf(db): optimize recurring query paths * perf(logs): batch keyset export reads * fix(workspaces): type nullable member count targets * fix(db): harden query performance changes * fix(logs): guard export stream cancellation	2026-08-23 17:15:10 -07:00
Vikhyath Mondreti	9cecf0b837	improvement(provenance): aggregate and attribute unrecorded durable reads (#7017) ... Fail-open on unrecorded durable provenance rests on one compensating control: the audit entry telling the people who own the secrets that a read proceeded unvouched. An audit of all four surfaces found the control incomplete in exactly the places this closes, and confirmed the policy itself sound — so nothing here changes what any read or write does, only what gets recorded about it. Knowledge was the one surface with no audit trail at all: the per-record import reports without a workspace, and the report skips the audit row when it cannot name one, so fail-open knowledge reads emitted one error log line per record and zero audit entries. Both importers now count unrecorded records while the surface is open and report once per read with the workspace, actor, and count — the shape memory and tables already use. The search read reports once across chunks and rendered metadata, and only when the registry did not latch, since a latched read never reaches a model. Fault returns stay silent; those reads fail closed. Memory had the one silent local degrade: a record whose canonical hash outgrows its bounds, or whose entries fail normalization, was stored unknown with nothing logged anywhere — the table writer logs its equivalent. The binding now logs the cause at error where it is decided. An incoming unknown stays silent; its producer already reported. The memory list contract gains the page ceiling every other list already has (max 1000, matching the table convention); no caller in the repo passes a limit at all, and the route is internal-auth only. Workspace-file audit rows now carry the acting user where the caller already holds one — copilot vfs, the agent and mothership handlers, and the provider attachment filter. Everywhere else, including principals with no user to name, the actor stays null, which the report type has always permitted. Two comments catch up with the code: the file sidecar stores three statuses since the absence/taint split, and the mounted-file scanner's scan-overflow-to-taint is deliberate where the registry scan over-approximates — that scan only narrows an already-sound candidate set, while this one decides whether egress redaction would suffice for bytes the same matcher just failed on.	2026-08-23 13:08:23 -07:00
Waleed	1cb9c868ce	improvement(chat): speed up conversation navigation (#7011) ... * improvement(chat): speed up conversation navigation * fix(chat): prefetch direct navigation intent * fix(chat): preserve quick-click prefetch intent	2026-08-23 11:15:25 -07:00
Waleed	b44d285607	fix(files): download a markdown file as a zip only when it really has assets (#7009) ... * fix(files): download a markdown file as a zip only when it really has assets A document that merely mentions an embed URL in prose or an inline code span counted as having attachments, so any document about the files API downloaded as a zip whose assets/ folder was empty. - Detect embeds with the markdown lexer instead of scanning raw text, so only real image embeds count: prose, code spans, fenced samples, and links no longer do - Choose the export format after resolving assets rather than from the candidate count, so a missing, unreadable, or oversized embed falls back to the plain document instead of an empty zip - Move the document scan out of the copilot tool tree into lib/uploads/server, where both file routes already live, and drop two pass-through wrappers - Share one <img> src reader between the clipboard handlers and the scan - Walk tokens explicitly: marked's walkTokens concatenates per token and costs O(n^2), measuring 5.4s on a 254KB document against 14ms here, on a path anonymous public-share traffic reaches * fix(files): keep an embed id spelled as the document spells it Decoding the id let a percent-encoded embed resolve and bundle its asset while the rewrite, which searches the document for that id, found nothing — the zip kept an API URL that renders as a broken image offline. Keys stay decoded; they are matched against stored keys, not against document text. * fix(files): resolve an export asset by its stored id, rewrite by its spelling An embed carries two representations and they are not interchangeable: metadata resolves by the stored id, while the rewrite finds the embed by searching the document for the spelling it used. Using one for both either drops a percent-encoded asset or bundles it behind a link still pointing at the API. * fix(files): resolve an embed by its stored id wherever one is read from a document The export bundler decoded an embed's spelling before looking it up, but the file-agent's embeddability warning did not, so a percent-encoded embed the export resolves and bundles could still be reported as one that will not survive an export. Both now share one helper. Request-supplied ids are untouched: their route contracts already constrain them to the plain id charset, so there is no spelling to decode.	2026-08-23 11:15:14 -07:00
Vikhyath Mondreti	ed335d4137	improvement(pricing): list Sandboxes as a Max and Enterprise feature (#7013)	2026-08-23 10:53:43 -07:00
Waleed	f3867694b8	fix(files): allowlist the schemes a markdown link may target (#7012) ... `normalizeLinkHref` rejected only `file` for a `scheme://` target, so any other scheme was returned unchanged. `scheme://` is well-formed for every scheme, so the check let through spellings that are not navigable targets at all. - Keep a scheme only when it is http(s), ftp(s), mailto, or tel; drop the rest - Leave an existing link alone when a committed target normalizes away, rather than unsetting it — the editor seeds that field with the current href, so committing an untouched one previously removed the link Detection is unchanged for relative, anchor, protocol-relative, and bare-domain targets. A document's stored markdown is untouched: normalization runs on the render and edit paths, never on parse or serialize, so a target that is refused still round-trips verbatim.	2026-08-23 10:31:42 -07:00
Waleed	465bdbd12b	fix(settings): keep billing header stable (#7010)	2026-08-23 04:16:12 -07:00
Theodore Li	d831c0937c	fix(security): isolate rejected OTP attempts (#7008) ... * fix(security): isolate rejected OTP attempts * fix(security): make OTP requests non-enumerating * fix(security): defer OTP delivery work	2026-08-23 03:22:57 -04:00
Vikhyath Mondreti	33feaa158a	fix(inbox): stop an unattributed sender inheriting owner write authority (#7006) ... * fix(inbox): stop an unattributed sender inheriting owner write authority resolveInboxExecutionActor refuses to name a raw-secret actor when the sender matches no workspace member, then hands the run ws.ownerId for everything else. That identity also supplies userPermission, which is what executeTool gates on, so the owner's admin satisfied every requiredPermission check. In headless mode the client-routed workflow tools fall back to their registered server handlers (see the comment in tool-executor/executor.ts), so create_workflow, edit_workflow and run_workflow — all requiredPermission 'write' — were reachable. runWorkflowFromCopilot then executes with enforceCredentialAccess and the owner as actor, which resolves the owner's workspace and personal secrets. An allowlisted external correspondent could therefore reach, through a workflow it had the agent build and run, exactly what the null secret actor refuses for a direct mount. Cap the run's tool permission at read when no member owns the message. An attributed message is unchanged and still uses the sender's own permission, so a read-only member emailing the inbox still cannot run or edit anything. Read rather than none because answering an external correspondent from workspace context is the point of the inbox; only mutation and execution are withheld. The owner identity itself stays: billing attribution and workspace reads need a real user. This separates that need from the authority that came with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(copilot): bar the headless client-tool fallback below write Client-routed tools carry no catalog requiredPermission because the browser runs them through the workflow APIs, which authorize the caller's own session. The headless fallback in executeTool has no session and runs under the request's principal instead, with nothing standing in for that check. So the read cap from the previous commit did not reach run_workflow, run_workflow_until_block, run_block or run_from_block: all four are route 'client' with no requiredPermission, unlike create_workflow and edit_workflow. An unattributed inbox sender could therefore still run an existing workflow, which executes with enforceCredentialAccess under the workspace owner and resolves the owner's workspace and personal secrets. Derive the requirement at the gate instead: a client-routed tool taking the headless fallback requires write. Interactive callers never reach this branch, so the browser path is unaffected. The catalog itself is generated from the copilot contracts repo and cannot carry this rule, which only applies to the fallback. Also corrects the inboxToolPermission doc, which claimed run_workflow gates on requiredPermission 'write'. It does not; it is gated here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-22 23:23:44 -07:00