Merge remote-tracking branch 'origin/staging' into fix/chat-code-not-copied-inline

# Conflicts:
#	scripts/check-tool-registry-boundary.baseline.json

Author: j15z

apps/sim/executor/utils/block-data.test.ts

@@ -0,0 +1,77 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it, vi } from 'vitest'
+import { getBlockSchema } from '@/executor/utils/block-data'
+import { resolveBlockReference } from '@/executor/utils/block-reference'
+import type { SerializedBlock } from '@/serializer/types'
+
+/**
+ * These assertions are about what the real block registry publishes, so the global stub — which
+ * returns one mock block with no outputs — would make every case here pass vacuously.
+ */
+vi.unmock('@/blocks/registry')
+
+function triggerBlock(type: string, params: Record<string, unknown> = {}): SerializedBlock {
+  return {
+    id: 'trigger-1',
+    metadata: { id: type, name: 'webhook1', category: 'triggers' },
+    position: { x: 0, y: 0 },
+    config: { tool: '', params },
+    inputs: {},
+    outputs: {},
+    enabled: true,
+  } as unknown as SerializedBlock
+}
+
+function resolve(
+  pathParts: string[],
+  schema: ReturnType<typeof getBlockSchema>
+): ReturnType<typeof resolveBlockReference> {
+  return resolveBlockReference(
+    'webhook1',
+    pathParts,
+    {
+      blockNameMapping: { webhook1: 'trigger-1' },
+      blockData: { 'trigger-1': { query: { env: 'prod' } } },
+      blockOutputSchemas: schema ? { 'trigger-1': schema } : {},
+    } as never,
+    {} as never
+  )
+}
+
+describe('generic webhook output schema', () => {
+  /**
+   * A generic webhook receives whatever the caller sends, so it must publish no schema at all.
+   * `collectBlockData` registers any non-empty output declaration as exhaustive, which turns
+   * every unlisted field into a hard `InvalidFieldError` rather than an absent value.
+   */
+  it('publishes no output schema, leaving the block shape open', () => {
+    expect(getBlockSchema(triggerBlock('generic_webhook'))).toBeUndefined()
+  })
+
+  it.each([
+    [{}, 'no flags set'],
+    [{ acceptOtherMethods: true, exposeRequestHeaders: true }, 'both request-metadata flags on'],
+  ])('stays open with %o (%s)', (params) => {
+    expect(getBlockSchema(triggerBlock('generic_webhook', params))).toBeUndefined()
+  })
+
+  /**
+   * The production regression this pins: a Slack interactive payload reaching a workflow that
+   * reads `actions.0.selected_option.value`. When a delivery omits the field the reference must
+   * resolve to `undefined` so the condition simply evaluates falsy — not abort the run.
+   */
+  it('resolves an absent body field to undefined instead of throwing', () => {
+    const schema = getBlockSchema(triggerBlock('generic_webhook'))
+
+    expect(() => resolve(['actions', '0', 'selected_option', 'value'], schema)).not.toThrow()
+    expect(resolve(['actions', '0', 'selected_option', 'value'], schema)?.value).toBeUndefined()
+  })
+
+  it('still resolves request metadata the provider merges into the input', () => {
+    const schema = getBlockSchema(triggerBlock('generic_webhook'))
+
+    expect(resolve(['query', 'env'], schema)?.value).toBe('prod')
+  })
+})

apps/sim/lib/knowledge/connectors/sync-limits.ts

@@ -16,8 +16,11 @@ export const CONNECTOR_SYNC_MAX_DURATION_SECONDS = 3600
  * lock for another sync, so a TTL at or below the run ceiling would start a second
  * sync while the first is still writing, both racing the same documents.
  *
- * Measured against `updatedAt`, which a running sync refreshes every
- * {@link SYNC_LOCK_HEARTBEAT_INTERVAL_MS}. That is what makes the TTL mean
+ * Measured against `COALESCE(syncLockLeaseAt, updatedAt)`, the lease a running
+ * sync refreshes every {@link SYNC_LOCK_HEARTBEAT_INTERVAL_MS}. The lease is a
+ * dedicated column precisely so an unrelated write to the row — a config edit on
+ * a wedged connector — can no longer pass for a heartbeat; `updatedAt` remains
+ * only as the fallback for a row locked before that column existed. That is what makes the TTL mean
  * "nobody is working on this" rather than "this started a long time ago" — the
  * distinction the in-process fallback path needs. A Trigger.dev run is killed at
  * {@link CONNECTOR_SYNC_MAX_DURATION_SECONDS} and so is provably dead well before

apps/sim/lib/knowledge/documents/processing-queue.test.ts

@@ -254,3 +254,73 @@ describe('processDocumentsWithQueue dispatch backend', () => {
     expect(mockBatchTrigger).not.toHaveBeenCalled()
   })
 })
+
+/**
+ * The processing-attempt budget exists to stop re-billing a document that keeps
+ * failing the same way *in processing*. A dispatch that never reached a worker
+ * teaches it nothing, so the charge is given back on the one path that proves
+ * nothing was dispatched. Without the refund a Trigger.dev outage burns the
+ * allowance without a single run, and after `MAX_PROCESSING_ATTEMPTS` of them
+ * the connector sweep — which skips documents at the cap — permanently stops
+ * recovering them.
+ */
+describe('processDocumentsWithQueue attempt refund', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+    dbChainMockFns.returning.mockResolvedValue([{ id: 'document-1' }])
+    for (const key of Object.keys(env)) {
+      delete (env as Record<string, unknown>)[key]
+    }
+    Object.assign(env, { ...defaultMockEnv, TRIGGER_SECRET_KEY: 'trigger-secret' })
+    dbChainMockFns.limit.mockResolvedValue([
+      { userId: 'knowledge-owner', workspaceId: 'workspace-1' },
+    ])
+  })
+
+  it('refunds the attempt in the same write that withdraws the queue stamp', async () => {
+    mockBatchTrigger.mockRejectedValue(new Error('trigger.dev region unavailable'))
+
+    await expect(
+      processDocumentsWithQueue(
+        [DOCUMENT],
+        'knowledge-base-1',
+        {},
+        'request-1',
+        BILLING_ATTRIBUTION
+      )
+    ).rejects.toThrow('document processing dispatches failed')
+
+    const withdrawCall = dbChainMockFns.set.mock.calls.find(
+      (call) => (call[0] as Record<string, unknown> | undefined)?.processingQueuedAt === null
+    )
+    expect(withdrawCall).toBeDefined()
+
+    const values = withdrawCall?.[0] as Record<string, unknown>
+    const attempts = values.processingAttempts as { toSQL: () => { sql: string } } | undefined
+    expect(attempts).toBeDefined()
+    // Given back as a SQL decrement in the same guarded statement as the stamp,
+    // so it can only ever undo the charge this call made.
+    expect(attempts?.toSQL().sql).toContain('- 1')
+    // Floored, so a refund can never drive the count below zero.
+    expect(attempts?.toSQL().sql).toContain('GREATEST')
+  })
+
+  it('leaves the attempt spent when a dispatch did get through', async () => {
+    mockBatchTrigger.mockResolvedValue({ batchId: 'batch-1' })
+
+    await processDocumentsWithQueue(
+      [DOCUMENT],
+      'knowledge-base-1',
+      {},
+      'request-1',
+      BILLING_ATTRIBUTION
+    )
+
+    expect(
+      dbChainMockFns.set.mock.calls.some(
+        (call) => (call[0] as Record<string, unknown> | undefined)?.processingQueuedAt === null
+      )
+    ).toBe(false)
+  })
+})

apps/sim/lib/knowledge/documents/service.ts

@@ -717,6 +717,8 @@ async function markDocumentsQueued(documentIds: string[], queuedAt: Date): Promi
       // Spent here because this is the one write every dispatch passes through,
       // and it is already guarded — so the budget cannot be charged twice for a
       // single dispatch, nor skipped by a caller that dispatches another way.
+      // Refunded by `clearDocumentsQueued` when the dispatch provably never
+      // happened, so only attempts a worker could have seen are ever spent.
       processingAttempts: sql`${document.processingAttempts} + 1`,
     })
     .where(and(inArray(document.id, documentIds), eq(document.processingStatus, 'pending')))
@@ -733,6 +735,18 @@ async function markDocumentsQueued(documentIds: string[], queuedAt: Date): Promi
  * failure is the one case where nothing was dispatched, so the stamp can be
  * taken back and the next sweep is free to reclaim them immediately.
  *
+ * The attempt {@link markDocumentsQueued} charged is refunded in the same
+ * statement. The budget exists to stop re-billing a document that keeps failing
+ * the same way *in processing*; an attempt that never reached a worker teaches
+ * it nothing. Leaving it spent let an infrastructure outage — a Trigger.dev
+ * region error, an exhausted quota — burn the allowance without a single run,
+ * and {@link MAX_PROCESSING_ATTEMPTS} such outages dead-letter a document the
+ * connector sweep then permanently excludes (`processingAttempts <
+ * MAX_PROCESSING_ATTEMPTS`), stranding it with no automatic recovery left.
+ * Floored at zero so a refund can never drive the count negative, and scoped by
+ * the same guard as the stamp, so it can only ever give back the charge this
+ * call made.
+ *
  * Scoped three ways so it can only ever undo its own write: to the ids in this
  * batch, to rows still `pending` (a worker that has since claimed one keeps its
  * timestamps — see {@link markDocumentsQueued}), and to the exact stamp this
@@ -742,7 +756,10 @@ async function markDocumentsQueued(documentIds: string[], queuedAt: Date): Promi
 async function clearDocumentsQueued(documentIds: string[], queuedAt: Date): Promise<void> {
   await db
     .update(document)
-    .set({ processingQueuedAt: null })
+    .set({
+      processingQueuedAt: null,
+      processingAttempts: sql`GREATEST(${document.processingAttempts} - 1, 0)`,
+    })
     .where(
       and(
         inArray(document.id, documentIds),

apps/sim/lib/knowledge/documents/types.ts

@@ -7,9 +7,12 @@
  * that fails deterministically (a corrupt file, an unsupported encoding) was
  * billed once per sync indefinitely. Five is chosen against the unit that is
  * actually consumed: one attempt per *dispatch*, not per Trigger.dev retry, so
- * a short-interval connector can burn several inside one transient outage.
- * Three left too little room for that; five still bounds the spend well inside
- * `RETRY_WINDOW_DAYS`.
+ * a short-interval connector can still burn several inside one transient
+ * outage. A dispatch that provably reached nothing is refunded — see
+ * `clearDocumentsQueued` — which covers the total-failure shape, but a partial
+ * batch failure and an accepted dispatch whose run never starts both stay
+ * charged. Three left too little room for those; five still bounds the spend
+ * well inside `RETRY_WINDOW_DAYS`.
  *
  * Reaching it is a dead letter, not a deletion: the document keeps its `failed`
  * status and stays user-retryable, it simply stops being swept automatically.

apps/sim/triggers/generic/webhook.test.ts

@@ -13,11 +13,13 @@ function setupInstructions(): string {
 }
 
 describe('genericWebhookTrigger', () => {
-  it('declares the request metadata so it can be referenced from later blocks', () => {
-    expect(Object.keys(genericWebhookTrigger.outputs)).toEqual(['method', 'query', 'headers'])
-    expect(genericWebhookTrigger.outputs.method.type).toBe('string')
-    expect(genericWebhookTrigger.outputs.query.type).toBe('object')
-    expect(genericWebhookTrigger.outputs.headers.type).toBe('object')
+  /**
+   * Declaring outputs here does not add editor completions — the executor reads the same list as
+   * an exhaustive schema and rejects every field outside it. See
+   * `executor/utils/block-data.test.ts` for the behavior this protects.
+   */
+  it('declares no outputs, because the caller decides the payload shape', () => {
+    expect(genericWebhookTrigger.outputs).toEqual({})
   })
 
   /**
@@ -40,40 +42,21 @@ describe('genericWebhookTrigger', () => {
     expect(instructions).toContain('GET, PUT, PATCH and DELETE')
   })
 
-  it('names every reserved key the input can carry', () => {
-    const instructions = setupInstructions()
-
-    for (const key of Object.keys(genericWebhookTrigger.outputs)) {
-      expect(instructions).toContain(`"${key}"`)
+  /**
+   * Named explicitly rather than derived from `outputs`, which is intentionally empty — deriving
+   * it would make this assertion vacuous.
+   */
+  it.each(['method', 'query', 'headers'])(
+    'names the reserved "%s" key the input can carry',
+    (key) => {
+      expect(setupInstructions()).toContain(`"${key}"`)
     }
-  })
+  )
 
   it('names the switch that exposes headers rather than promising them', () => {
     expect(setupInstructions()).toContain('"Expose Request Headers"')
   })
 
-  /**
-   * Two of the three outputs only exist once a switch is on, so they are conditioned on it: the
-   * reference dropdown must not offer a field the running webhook will not send.
-   */
-  it.each([
-    ['method', 'acceptOtherMethods'],
-    ['headers', 'exposeRequestHeaders'],
-  ])('gates the %s output on the switch that produces it', (key, field) => {
-    expect(genericWebhookTrigger.outputs[key].condition).toEqual({
-      field,
-      value: [true, 'true'],
-    })
-  })
-
-  /**
-   * Query parameters are the one key that is not opt-in, so offering them unconditionally is
-   * correct — gating them on a switch that does not exist would hide them entirely.
-   */
-  it('offers query unconditionally', () => {
-    expect(genericWebhookTrigger.outputs.query.condition).toBeUndefined()
-  })
-
   /**
    * Auth is header-based, so a plain link cannot carry it. Saying so is the difference between a
    * user disabling auth knowingly and discovering it after publishing an open trigger URL.

apps/sim/triggers/generic/webhook.ts

@@ -152,33 +152,21 @@ export const genericWebhookTrigger: TriggerConfig = {
   ],
 
   /**
-   * Body fields stay undeclared because a generic webhook receives whatever JSON the caller
-   * sends. The request metadata below is known ahead of time, so it can be offered for reference.
+   * Deliberately empty, and it must stay that way.
    *
-   * `method` and `headers` are conditioned on the switch that produces them, so the reference
-   * dropdown never offers a field the running webhook will not send. Both truthy forms are
-   * matched because a YAML- or Copilot-authored workflow can write the string rather than the
-   * boolean — the same tolerance `isProviderConfigFlagEnabled` applies at delivery time.
+   * A generic webhook receives whatever the caller sends, so its output shape is unknowable. The
+   * executor treats any non-empty output declaration as an exhaustive schema: `collectBlockData`
+   * registers it, and `resolveBlockReference` then throws `InvalidFieldError` for any reference
+   * outside it that resolves to `undefined`. Declaring `method`, `query` and `headers` here
+   * therefore did not add three completions — it made those three the *only* legal fields, and
+   * every workflow reading a body field failed the moment a delivery omitted it.
+   *
+   * The metadata is still merged into the input at delivery time by the generic provider's
+   * `formatInput`; it is only undeclared, which is what keeps the block's shape open. Offering
+   * these as editor completions needs a way to mark outputs as hints rather than a closed schema,
+   * which is a change to `getRegistrySchema`, not to this list.
    */
-  outputs: {
-    method: {
-      type: 'string',
-      description:
-        'HTTP method of the request. Yields to a body field of the same name if the caller sends one.',
-      condition: { field: 'acceptOtherMethods', value: [true, 'true'] },
-    },
-    query: {
-      type: 'object',
-      description:
-        'Query parameters from the request URL, when it has any. Yields to a body field of the same name if the caller sends one.',
-    },
-    headers: {
-      type: 'object',
-      description:
-        'Request headers, excluding the ones that carry credentials. Yields to a body field of the same name if the caller sends one.',
-      condition: { field: 'exposeRequestHeaders', value: [true, 'true'] },
-    },
-  },
+  outputs: {},
 
   webhook: {
     method: 'POST',