fix(v2): correct four stale contracts and clear the merge debris behind them

Five of the reported defects were real and four of them were documentation
that had stopped describing its own code.

`cleanCellValue` said only "coerce a raw input value"; it also answers `null`
for anything the column type refuses, and since the multiselect write path
started refusing partial matches that is the difference between a paste
storing one option and blanking the cell. It deliberately does not consult
`salvage`, which would read the same paste as the option that did resolve —
that reading is for writes with no caller to answer, and a typed cell has one.
The pairing is now asserted, so a future helper that "improves" the paste by
salvaging it fails.

`EXECUTE_OPTION_CONSTRAINTS` carried two stacked TSDoc blocks, the second
explaining that the enumeration had moved onto the fields; the body schema
still told a reader the six combinations were enumerated in the constant. The
deployment route's second block orphaned the endpoint documentation above it,
and `list-query.ts` kept the TSDoc for a cursor message that now lives, with
its own rewritten doc, in `cursor-binding.ts`. Two agents left near-identical
essays arguing the same 400-vs-403-vs-409 question about the table ceilings
and concluding that neither status changes; the decision is recorded once, in
`billing.ts`, and `service.ts` points at it.

The credentials use case echoed `sortBy`/`sortOrder` back with a TSDoc
explaining that the presenter needs them, which it no longer does — it reads
`query.*`. The local upload roots move from the data-plane provider to
`core/storage-key.ts`, beside the sidecar suffix, so the cleanup sweep can name
what it reclaims without importing the transport that writes it.

`documents.test.ts` justified sweeping only knowledge and files for the 413 by
saying the same sweep over the other five documents still reported gaps. It
does not: widened to all seven, every body-carrying operation publishes it.

Three reports did not survive checking, and the evidence is recorded where the
next reader will look. An empty rerank result is not the reranker matching
nothing — `rerank` asks for `top_n` over a non-empty document list, so an empty
array means the response carried nothing usable, which is what `unavailable`
already promises. The zero-byte knowledge document is refused on the
upload-session path too, by `validateFile`, under both boundary contracts;
that parity is now pinned, and it fails if the guard is removed. The MCP
re-registration reports exactly the connection fields its SET clause writes,
and the create mutation already drops both caches — what lags is the status
badge, not the tools, because discovery is gated on `connected` for OAuth rows
only.

Author: waleedlatif1

apps/sim/app/api/v2/workflows/[id]/deployment/route.ts

@@ -21,11 +21,9 @@ export const revalidate = 0
  * fallback: it retains the timestamp of a deployment that has since been
  * undeployed, so reading it would report a deploy time alongside
  * `isDeployed: false`.
- */
-/**
- * Deliberately head-safe despite issuing a write.
  *
- * Reading a workflow can trigger a migrate-on-read `workflow_blocks` update when
+ * Deliberately head-safe despite issuing a write. Reading a workflow can trigger
+ * a migrate-on-read `workflow_blocks` update when
  * `applyBlockMigrations` upgrades a stored block. That write is convergent: it is
  * conditional on a migration actually applying, idempotent, and would be issued by
  * the next ordinary read regardless, so a `HEAD` only brings it forward.

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.test.ts

@@ -2,6 +2,7 @@
  * @vitest-environment node
  */
 import { describe, expect, it } from 'vitest'
+import { columnTypeOf } from '@/lib/table/column-types'
 import {
   cleanCellValue,
   dateValueToLocalParts,
@@ -160,6 +161,32 @@ describe('cleanCellValue', () => {
     expect(cleanCellValue('Nope', column)).toBeNull()
     expect(cleanCellValue('Bug, Nope', column)).toBeNull()
   })
+
+  /**
+   * The refusal above is `coerce`'s, not the last word the registry has on the
+   * value: `salvage` reads the same paste as the one option that resolved. That
+   * reading is reserved for writes with no caller to answer — a CSV row, a block
+   * output — and a typed cell has one, so this helper must not reach for it. The
+   * pairing is asserted rather than described so a future helper that "improves"
+   * the paste by salvaging it fails here.
+   */
+  it('refuses a partial multiselect paste the registry could still salvage', () => {
+    const column = {
+      name: 'tags',
+      type: 'select',
+      multiple: true,
+      options: [
+        { id: 'opt_a', name: 'Bug' },
+        { id: 'opt_b', name: 'Docs' },
+      ],
+    } as const
+
+    expect(columnTypeOf(column).salvage?.('Bug, Nope', column)).toEqual({
+      ok: true,
+      value: ['opt_a'],
+    })
+    expect(cleanCellValue('Bug, Nope', column)).toBeNull()
+  })
 })
 
 describe('formatValueForInput', () => {

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.ts

@@ -19,8 +19,18 @@ export function generateColumnName(columns: ReadonlyArray<{ name: string }>): st
 }
 
 /**
- * Coerce a raw input value to the appropriate type for a column.
- * Throws on invalid JSON.
+ * Coerce a value a person typed or pasted into a cell to that column's type.
+ * Throws on invalid JSON, and answers `null` for everything else the column
+ * type refuses.
+ *
+ * `null` is what the server would store for the same value, which is the point:
+ * the optimistic cache and the row that comes back agree. It deliberately does
+ * not consult `ColumnTypeDefinition.salvage`, which reads a refused value
+ * lossily — a multiselect paste naming one option that no longer exists blanks
+ * the cell here rather than storing the members that did resolve. Salvage is
+ * reserved for writes with no caller to answer, and this one has one: a person
+ * watching the cell, who is better served seeing the paste refused than seeing
+ * part of it silently kept.
  */
 export function cleanCellValue(
   value: unknown,

apps/sim/hooks/queries/mcp.ts

@@ -317,6 +317,14 @@ export function useCreateMcpServer() {
         authType,
       }
     },
+    /**
+     * Both caches are dropped, so neither waits out its stale time — but the
+     * refetched row still reads `disconnected`, because the discovery that
+     * moves it runs on the tools query this same invalidation kicks off, after
+     * the list has already come back. The status catches up on the next list
+     * refetch; the tools do not wait for it, since
+     * {@link isServerEligibleForDiscovery} gates only OAuth rows on `connected`.
+     */
     onSettled: (_data, _error, variables) => {
       queryClient.invalidateQueries({ queryKey: mcpKeys.serversList(variables.workspaceId) })
       queryClient.invalidateQueries({

apps/sim/lib/api/contracts/v2/openapi/shared.ts

@@ -257,6 +257,14 @@ export const WORKSPACE_API_KEY_DENIED =
  * {@link WORKSPACE_API_KEY_DENIED} for an operation behind the resource-concealment
  * error policy, which rewrites the authorization failure to a not-found response so
  * the caller learns nothing about the resource.
+ *
+ * Published on no operation today: every one audited so far refuses a workspace
+ * key through its principal-kind list, which raises an error the concealment
+ * policy does not rewrite, so all of them say 403. Kept because a concealed
+ * operation that denies the key through the policy itself would need this exact
+ * wording, and because `scripts/openapi/documents.test.ts` asserts the file-share
+ * description does not carry it — inlining the string there would let the guard
+ * and the wording it guards drift apart.
  */
 export const WORKSPACE_API_KEY_DENIED_AS_NOT_FOUND =
   'A workspace API key is rejected as `404` rather than `403`, because unauthorized resources are concealed; use a personal API key.'

apps/sim/lib/api/contracts/v2/workflows.ts

@@ -816,18 +816,15 @@ export const v2ExecutionErrorSchema = z
 export type V2ExecutionError = z.output<typeof v2ExecutionErrorSchema>
 
 /**
- * The mutually-exclusive execute option matrix, mirrored from the route's
- * post-parse checks in `app/api/v2/workflows/[id]/execute/route.ts`. Kept as one
+ * That the execute options constrain each other, said once. Kept as one
  * exported string so the request-body description and the OpenAPI operation
  * description cannot drift from each other.
- */
-/**
- * The six rejected option combinations used to be enumerated here and pasted
- * onto both the operation and the request-body description, restating what each
- * field already says. A caller reads the constraint where it applies — on the
- * field it constrains — so the enumeration lives on `async`, `stream`,
- * `executionTimeoutSeconds`, `includeThinking`, and `includeToolCalls`, and the
- * operation says only that the options are mutually constrained.
+ *
+ * It deliberately does not enumerate the combinations the route rejects. That
+ * list used to be pasted onto both the operation and the request-body
+ * description, restating what each field already says; a caller reads a
+ * constraint where it applies, so it lives on `async`, `stream`,
+ * `executionTimeoutSeconds`, `includeThinking`, and `includeToolCalls`.
  */
 export const EXECUTE_OPTION_CONSTRAINTS =
   'Each option carries the modes it requires and the modes that reject it; a violated combination is a 400.'
@@ -838,8 +835,9 @@ export const EXECUTE_OPTION_CONSTRAINTS =
  * (triggerType, draft state, deployment pinning) are NEVER wire fields; they
  * are typed options on the execution service.
  *
- * The six rejected option combinations are enumerated in
- * {@link EXECUTE_OPTION_CONSTRAINTS} and enforced by the route after parsing.
+ * The rejected option combinations are enforced by the route after parsing and
+ * described on the fields they constrain; {@link EXECUTE_OPTION_CONSTRAINTS}
+ * only tells a caller that the options constrain each other.
  */
 export const v2ExecuteWorkflowBodySchema = z
   .object({

apps/sim/lib/api/list-query.ts

@@ -57,17 +57,6 @@ export type CursorKey = string | number
 export const INVALID_CURSOR_MESSAGE =
   'cursor does not match the requested sortBy/sortOrder. Restart pagination without a cursor after changing the sort.'
 
-/**
- * Caller-facing message for a cursor that cannot be read back at all.
- *
- * Separate from {@link INVALID_CURSOR_MESSAGE} because that one names
- * `sortBy`/`sortOrder`, and the lists that mint a wrapped domain token accept
- * neither param — `GET /logs` carries its direction in `order`, and
- * `GET /billing/logs` takes no sort param whatsoever. Sending those callers to
- * inspect a knob their operation does not have is the same wrong-signpost
- * problem `UNKNOWN_CURSOR_MESSAGE` was written to avoid on the ledger. The
- * actionable half — restart without a cursor — is identical.
- */
 /**
  * One column of a keyset ordering, with the codec that moves its value through
  * the opaque cursor.

apps/sim/lib/credentials/application/list-workspace-credentials.ts

@@ -25,12 +25,6 @@ export interface ListWorkspaceCredentialsInput {
 export interface ListWorkspaceCredentialsResult {
   credentials: VisibleWorkspaceCredential[]
   nextCursorKeys: CursorKey[] | null
-  /**
-   * Echoed back because the route's presenter receives only this result, and the
-   * cursor it hands out has to be stamped with the sort that produced it.
-   */
-  sortBy: ListWorkspaceCredentialsInput['sortBy']
-  sortOrder: ListSortOrder
 }
 
 export const listWorkspaceCredentials = defineAuthorizedWorkspaceUseCase({
@@ -57,7 +51,7 @@ export const listWorkspaceCredentials = defineAuthorizedWorkspaceUseCase({
         limit: input.limit,
         cursorKeys: input.cursorKeys,
       })
-      return { credentials: page.data, nextCursorKeys: page.nextCursorKeys, ...sort }
+      return { credentials: page.data, nextCursorKeys: page.nextCursorKeys }
     }
 
     const workspaceAccess = await checkWorkspaceAccess(context.workspaceId, principal.userId)
@@ -82,6 +76,6 @@ export const listWorkspaceCredentials = defineAuthorizedWorkspaceUseCase({
       limit: input.limit,
       cursorKeys: input.cursorKeys,
     })
-    return { credentials: page.data, nextCursorKeys: page.nextCursorKeys, ...sort }
+    return { credentials: page.data, nextCursorKeys: page.nextCursorKeys }
   },
 })

apps/sim/lib/knowledge/application/search.test.ts

@@ -352,6 +352,22 @@ describe('knowledge search application use case', () => {
       expect(result.results[0]).not.toHaveProperty('rerankerScore')
     })
 
+    /**
+     * A resolved call with an empty ordering leaves the caller in the same place a
+     * thrown one does — vector order, no `rerankerScore` — so it reports the same
+     * status. It is not "the reranker matched nothing": `rerank` sends a non-empty
+     * document list and asks for `top_n` of it, so an empty array means the
+     * response carried nothing usable rather than a legitimate empty ranking.
+     */
+    it('reports unavailable when the call resolves without a usable ordering', async () => {
+      mocks.rerank.mockResolvedValueOnce({ results: [], isBYOK: false })
+
+      const result = await rerankedSearch(true)
+
+      expect(result.rerankerStatus).toBe('unavailable')
+      expect(result.results[0]).not.toHaveProperty('rerankerScore')
+    })
+
     it('reports skipped for a tag-only search, which has no query to rank against', async () => {
       mocks.getTagDefinitions.mockResolvedValue([
         { tagSlot: 'tag1', displayName: 'team', fieldType: 'text' },

apps/sim/lib/knowledge/application/search.ts

@@ -328,8 +328,18 @@ export const searchKnowledge = defineAuthorizedKnowledgeUseCase({
      * never completes, so only the success path has to move it. A request with
      * nothing to rank — no query text, or no candidate rows — is `skipped` rather
      * than `unavailable`: the reranker was never the obstacle. Anything else that
-     * was asked for and did not run is `unavailable`, including a request that
-     * reaches here with no model, which no HTTP contract can now produce.
+     * was asked for and did not produce a usable ordering is `unavailable`,
+     * including a request that reaches here with no model, which no HTTP contract
+     * can now produce.
+     *
+     * A call that returns without raising but hands back an empty ordering counts
+     * as `unavailable` too, and it is not the reranker "matching nothing":
+     * `rerank` asks for `top_n` over a non-empty document list, so a provider that
+     * ranked them returns one entry per document. Empty means the response carried
+     * nothing usable — no results, or only indices outside the batch, which
+     * `rerank` drops. The caller is left in vector order with no `rerankerScore`,
+     * which is exactly what `unavailable` promises, and retrying is exactly the
+     * right advice.
      */
     let rerankerStatus: RerankerStatus = !input.rerankerEnabled
       ? 'not_requested'