fix: address the review findings raised on the v0.8.21 release PR

Eight of the thirteen threads were real. Each was verified against source
before changing anything; three were pushed back on and are unchanged.

Knowledge and credential groups:

- The connector Access field resolved its provider through the standard-OAuth
  subset, which excludes Slack — Slack collects accounts through a custom bot.
  The field never rendered for a Slack connector, so it could not enter members
  mode and, worse, a per-member Slack connector had no way back to workspace
  mode. Resolved across all credential-group providers instead.
- The v1 document delete looked the document up with an ACL scope and then ran
  an unscoped delete. The access-aware path already existed and v2 already used
  it; v1 was the last surface on the old one. The window is small and not
  attacker-controllable, but the divergence is worth closing.
- Enrollment surfaced `CredentialGroupEnrollmentError` as a bare 500. A missing,
  disabled, or unconfigured credential group is the admin's to act on, so the
  policy now projects its 404/409 the way the credential-group routes do.
- The workspace-level member-connector listing used knowledge-base
  concealment and answered "Knowledge base not found" where its siblings return
  an authorization response. It names a workspace, not a base, so it now uses an
  unconcealed policy — the convention the policy file already documents.

Home:

- Restoring a queued Build message left the search query in the URL, and the
  rule that forces Search whenever a query is present flipped the composer
  straight back. The edit was discarded and the original message dispatched.
  Clearing the query alongside the mode restore batches into the same nuqs
  update, so the forcing rule never observes the intermediate state.

Docs and tooling:

- `redis.mdx` claimed completed work is unaffected by losing Redis. Webhook
  idempotency markers live wherever the cache does, with a 7-day TTL sized to
  the longest provider retry window, and the bundled Redis runs without
  persistence — so a redelivery after a restart can re-run a finished workflow
  with its real side effects. Billing, checkout, and Chat-send idempotency are
  pinned to PostgreSQL. Corrected the same claim in the chart's values.
- `/ship` Phase A never regenerated the docs manifest that Phase B hard-gates
  on, so adding or renaming a docs page aborted the command.
- The CLI updater prints a yarn command, but the upgrade tabs offered none.
- Documented that Ask may reach an integration for an explicitly requested
  action, not only when sources cannot answer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BwsJTEQRzWJaY4BRCkPZt

Author: waleedlatif1

apps/docs/content/docs/platform/self-hosting/redis.mdx

@@ -24,7 +24,9 @@ Sim uses Redis as a message bus and shared cache. Both deployments ship it by de
   With more than one app or realtime replica and no `REDIS_URL`, users on different pods stop seeing each other's edits and live status updates. Beyond one startup log line noting single-pod mode, nothing is logged — the app looks healthy and quietly loses events. Treat Redis as mandatory the moment `replicaCount` exceeds 1.
 </Callout>
 
-Everything Sim keeps in Redis is cache, coordination state, or an in-flight event — never committed data, which lives in PostgreSQL and object storage. Persistence is therefore not required. Losing or restarting the instance is not free, though: cancellation markers and the cross-pod half of execution streaming live here, so active runs stop streaming and a cancellation issued across the gap may not land. Completed work is unaffected.
+Everything Sim keeps in Redis is cache, coordination state, or an in-flight event — never committed data, which lives in PostgreSQL and object storage. Losing or restarting the instance costs active runs their streaming, and a cancellation issued across the gap may not land.
+
+It also costs webhook deduplication. Webhook idempotency markers live wherever the cache does, with a 7-day TTL sized to the longest provider retry window, so a redelivery arriving after a restart can re-run a workflow that already completed — with its real side effects. Billing, checkout, and Chat-send idempotency are pinned to PostgreSQL and are never at risk. The bundled Redis runs without persistence, which is fine for coordination state; if duplicate webhook side effects would be unacceptable for your deployment, point `REDIS_URL` at a managed instance with persistence enabled.
 
 ## Configuration
 

apps/sim/app/api/knowledge/member-connectors/route.ts

@@ -13,7 +13,7 @@ export const GET = defineInternalJsonRoute({
   auth: internalSessionAuth,
   operation: knowledgeOperations.listWorkspaceMemberConnectors,
   rateLimit: internalRateLimits.none({ reason: 'One small read per visit to the Search tab' }),
-  errorPolicy: internalKnowledgeErrorPolicies.connectors,
+  errorPolicy: internalKnowledgeErrorPolicies.memberConnectors,
   mapInput: ({ query }) => ({ workspaceId: query.workspaceId }),
   useCase: listWorkspaceMemberConnectors,
   present: ({ connectors }) => ({ success: true as const, data: connectors }),

apps/sim/app/api/v1/knowledge/[id]/documents/[documentId]/route.ts

@@ -113,11 +113,12 @@ export const DELETE = withRouteHandler(
       )
       if (result instanceof NextResponse) return result
 
-      const doc = await getKnowledgeDocument(
-        knowledgeBaseId,
-        documentId,
-        await resolveV1KnowledgeAccessScope(userId, rateLimit, parsed.data.query.workspaceId)
+      const access = await resolveV1KnowledgeAccessScope(
+        userId,
+        rateLimit,
+        parsed.data.query.workspaceId
       )
+      const doc = await getKnowledgeDocument(knowledgeBaseId, documentId, access)
 
       if (!doc) {
         return NextResponse.json({ error: 'Document not found' }, { status: 404 })
@@ -130,6 +131,7 @@ export const DELETE = withRouteHandler(
           workspaceId: parsed.data.query.workspaceId,
         },
         document: { id: documentId, filename: doc.filename },
+        access,
         userId,
         source: 'api',
         requestId,

apps/sim/app/workspace/[workspaceId]/home/home.tsx

@@ -550,9 +550,10 @@ export function Home({ chatId, userName, userId }: HomeProps) {
    */
   const restoreQueuedMode = useCallback(
     (requestMode: QueuedMessage['requestMode']) => {
+      setSearchQuery('')
       void setComposerMode(requestMode === 'ask' ? 'assistant' : 'build')
     },
-    [setComposerMode]
+    [setComposerMode, setSearchQuery]
   )
 
   /** An emptied search box returns to the sources; a send in any other mode has no search to clear. */

apps/sim/app/workspace/[workspaceId]/home/types.ts

@@ -26,7 +26,8 @@ export interface FileAttachmentForApi {
 /**
  * A request mode a send asks the agent for beyond the default. `ask` is an
  * Assistant turn: an answer drawn from the attached knowledge bases first,
- * with a connected integration reached only when those cannot answer.
+ * with a connected integration reached only when those cannot answer — live or
+ * very recent data, or an action the person asked for outright.
  */
 export type ChatRequestMode = 'ask'
 

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-member-group-options.ts

@@ -3,9 +3,9 @@
 import { useMemo } from 'react'
 import type { ComboboxOption } from '@sim/emcn'
 import {
-  type CredentialGroupStandardOAuthProvider,
+  type CredentialGroupProvider,
+  getCredentialGroupProviderFromProviderId,
   getCredentialGroupProviderId,
-  getCredentialGroupStandardOAuthProviderFromProviderId,
   isCredentialGroupProvider,
 } from '@/lib/credential-groups/providers'
 import type { ConnectorMeta } from '@/connectors/types'
@@ -30,13 +30,18 @@ export function decodeConnectorMemberGroupOption(
   }
 }
 
-/** The credential-group provider that collects accounts for this connector, if any. */
+/**
+ * The credential-group provider that collects accounts for this connector, if any.
+ * Resolves across every credential-group provider, not just the standard-OAuth
+ * subset — Slack collects accounts through a custom bot and would otherwise
+ * resolve to none, hiding the Access field.
+ */
 function connectorMemberGroupProvider(
   connectorConfig: ConnectorMeta
-): CredentialGroupStandardOAuthProvider | null {
+): CredentialGroupProvider | null {
   if (connectorConfig.auth.mode !== 'oauth' || !connectorConfig.permissionScopedListing) return null
   try {
-    return getCredentialGroupStandardOAuthProviderFromProviderId(connectorConfig.auth.provider)
+    return getCredentialGroupProviderFromProviderId(connectorConfig.auth.provider)
   } catch {
     return null
   }

apps/sim/lib/knowledge/api/route-policies.ts

@@ -10,6 +10,7 @@ import {
   v2OrchestrationErrorPolicy,
 } from '@/lib/api/server/routes'
 import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
+import { CredentialGroupEnrollmentError } from '@/lib/credential-groups/enrollments'
 import { KNOWLEDGE_DELEGATION_AUDIENCE } from '@/lib/knowledge/application/authorization'
 import { KnowledgeUsageLimitExceededError } from '@/lib/knowledge/application/billing'
 import { KnowledgeDocumentNotReadyError } from '@/lib/knowledge/application/chunk-errors'
@@ -109,7 +110,23 @@ export const internalKnowledgeErrorPolicies = {
   tags: concealKnowledgeBase(
     internalKnowledgeErrorPolicy('Failed to process knowledge tag request')
   ),
-  connectors: concealKnowledgeBase(internalKnowledgeErrorPolicy('Internal server error')),
+  /**
+   * Enrollment reaches the credential-group helpers, whose failures are the
+   * admin's to act on — a missing group, a disabled one, or one with no active
+   * account option — rather than a bare 500.
+   */
+  connectors: concealKnowledgeBase(
+    extendInternalErrorPolicy(internalKnowledgeErrorPolicy('Internal server error'), (error) =>
+      error instanceof CredentialGroupEnrollmentError
+        ? internalErrorResponse(error.status, { error: error.message })
+        : null
+    )
+  ),
+  /**
+   * Workspace-scoped, like the bulk routes: the request names a workspace, not
+   * one knowledge base, so there is no resource whose existence a 403 betrays.
+   */
+  memberConnectors: internalKnowledgeErrorPolicy('Failed to fetch member connectors'),
   uploads: concealKnowledgeBase(internalKnowledgeUploadErrorPolicy),
 } as const
 

apps/sim/lib/knowledge/orchestration/documents.test.ts

@@ -8,7 +8,7 @@ const {
   mockCaptureServerEvent,
   mockCreateDocumentRecords,
   mockCreateSingleDocument,
-  mockDeleteDocument,
+  mockDeleteKnowledgeDocumentInKnowledgeBase,
   mockGetDocumentByUploadId,
   mockMarkDocumentAsFailedTimeout,
   mockProcessDocumentAsync,
@@ -21,7 +21,7 @@ const {
   mockCaptureServerEvent: vi.fn(),
   mockCreateDocumentRecords: vi.fn(),
   mockCreateSingleDocument: vi.fn(),
-  mockDeleteDocument: vi.fn(),
+  mockDeleteKnowledgeDocumentInKnowledgeBase: vi.fn(),
   mockGetDocumentByUploadId: vi.fn(),
   mockMarkDocumentAsFailedTimeout: vi.fn(),
   mockProcessDocumentAsync: vi.fn(),
@@ -47,7 +47,7 @@ vi.mock('@/lib/core/telemetry', () => ({
 vi.mock('@/lib/knowledge/documents/service', () => ({
   createDocumentRecords: mockCreateDocumentRecords,
   createSingleDocument: mockCreateSingleDocument,
-  deleteDocument: mockDeleteDocument,
+  deleteKnowledgeDocumentInKnowledgeBase: mockDeleteKnowledgeDocumentInKnowledgeBase,
   getDocumentByUploadId: mockGetDocumentByUploadId,
   markDocumentAsFailedTimeout: mockMarkDocumentAsFailedTimeout,
   processDocumentAsync: mockProcessDocumentAsync,
@@ -58,6 +58,7 @@ vi.mock('@/lib/knowledge/documents/service', () => ({
 vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mockCaptureServerEvent }))
 
 import { OrchestrationError } from '@/lib/core/orchestration/types'
+import { type KnowledgeAccessScope, WORKSPACE_ACCESS_TOKENS } from '@/lib/knowledge/access/types'
 import {
   performDeleteKnowledgeDocument,
   performMarkKnowledgeDocumentTimedOut,
@@ -75,6 +76,7 @@ const FILE = {
   mimeType: 'application/pdf',
 }
 const ACTOR = { userId: 'user-1', source: 'agent' as const, requestId: 'req-1' }
+const ACCESS: KnowledgeAccessScope = { kind: 'workspace', tokens: WORKSPACE_ACCESS_TOKENS }
 
 /**
  * Lets the fire-and-forget dispatch settle. Both upload paths queue indexing
@@ -403,31 +405,64 @@ describe('performUpdateKnowledgeDocument', () => {
 describe('performDeleteKnowledgeDocument', () => {
   beforeEach(() => {
     vi.clearAllMocks()
-    mockDeleteDocument.mockResolvedValue({ success: true, message: 'ok' })
+    mockDeleteKnowledgeDocumentInKnowledgeBase.mockResolvedValue(undefined)
   })
 
   it('audits the deletion against the acting user', async () => {
     const outcome = await performDeleteKnowledgeDocument({
       ...ACTOR,
       knowledgeBase: KB,
       document: { id: 'doc-1', filename: 'report.pdf', fileSize: 10, mimeType: 'application/pdf' },
+      access: ACCESS,
     })
 
     expect(outcome).toMatchObject({ success: true })
-    expect(mockDeleteDocument).toHaveBeenCalledWith('doc-1', 'req-1')
     expect(mockRecordAudit).toHaveBeenCalledWith(
       expect.objectContaining({ actorId: 'user-1', resourceId: 'doc-1' })
     )
     expect(mockCaptureServerEvent).toHaveBeenCalled()
   })
 
+  it("re-applies the caller's access at the delete itself", async () => {
+    await performDeleteKnowledgeDocument({
+      ...ACTOR,
+      knowledgeBase: KB,
+      document: { id: 'doc-1', filename: 'report.pdf' },
+      access: ACCESS,
+    })
+
+    expect(mockDeleteKnowledgeDocumentInKnowledgeBase).toHaveBeenCalledWith(
+      'kb-1',
+      'doc-1',
+      'req-1',
+      ACCESS
+    )
+  })
+
+  it('reports not_found when the scoped delete finds nothing to delete', async () => {
+    mockDeleteKnowledgeDocumentInKnowledgeBase.mockRejectedValue(
+      new OrchestrationError('not_found', 'Document not found')
+    )
+
+    const outcome = await performDeleteKnowledgeDocument({
+      ...ACTOR,
+      knowledgeBase: KB,
+      document: { id: 'doc-1', filename: 'report.pdf' },
+      access: ACCESS,
+    })
+
+    expect(outcome).toMatchObject({ success: false, errorCode: 'not_found' })
+    expect(mockRecordAudit).not.toHaveBeenCalled()
+  })
+
   it('emits no telemetry when the delete fails', async () => {
-    mockDeleteDocument.mockRejectedValue(new Error('deadlock detected'))
+    mockDeleteKnowledgeDocumentInKnowledgeBase.mockRejectedValue(new Error('deadlock detected'))
 
     const outcome = await performDeleteKnowledgeDocument({
       ...ACTOR,
       knowledgeBase: KB,
       document: { id: 'doc-1', filename: 'report.pdf' },
+      access: ACCESS,
     })
 
     expect(outcome).toMatchObject({ success: false, errorCode: 'internal' })

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

@@ -5,12 +5,13 @@ import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attr
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { PlatformEvents } from '@/lib/core/telemetry'
 import { generateRequestId } from '@/lib/core/utils/request'
+import type { KnowledgeAccessScope } from '@/lib/knowledge/access/types'
 import { dispatchDocumentProcessing } from '@/lib/knowledge/documents/processing-dispatch'
 import {
   createDocumentRecords,
   createSingleDocument,
   type DocumentData,
-  deleteDocument,
+  deleteKnowledgeDocumentInKnowledgeBase,
   getDocumentByUploadId,
   markDocumentAsFailedTimeout,
   type ProcessingOptions,
@@ -440,6 +441,11 @@ export async function performUpdateKnowledgeDocument(
 export interface PerformDeleteKnowledgeDocumentParams extends KnowledgeOperationContext {
   knowledgeBase: KnowledgeBaseTarget
   document: { id: string; filename: string; fileSize?: number; mimeType?: string }
+  /**
+   * Re-applied at the delete itself, so an access change landing between the
+   * caller's lookup and this write cannot still delete the document.
+   */
+  access: KnowledgeAccessScope
 }
 
 export type PerformDeleteKnowledgeDocumentResult = KnowledgeOrchestrationResult
@@ -448,11 +454,11 @@ export type PerformDeleteKnowledgeDocumentResult = KnowledgeOrchestrationResult
 export async function performDeleteKnowledgeDocument(
   params: PerformDeleteKnowledgeDocumentParams
 ): Promise<PerformDeleteKnowledgeDocumentResult> {
-  const { knowledgeBase, document, request, source } = params
+  const { knowledgeBase, document, request, source, access } = params
   const requestId = params.requestId ?? generateRequestId()
 
   try {
-    await deleteDocument(document.id, requestId)
+    await deleteKnowledgeDocumentInKnowledgeBase(knowledgeBase.id, document.id, requestId, access)
   } catch (error) {
     return classifyKnowledgeFailure(error, requestId, `Delete document ${document.id}`)
   }

helm/sim/values.yaml

@@ -597,7 +597,10 @@ redis:
     pullPolicy: IfNotPresent
 
   # No persistence is configured: Redis holds coordination state and short-lived
-  # keys, so a restart costs in-flight live updates, not committed data.
+  # keys, so a restart costs in-flight live updates, not committed data. It also
+  # drops webhook idempotency markers, so a provider redelivery after a
+  # restart can re-run an already-completed workflow — use a persistent managed
+  # instance if that matters. Billing and checkout idempotency is on PostgreSQL.
   maxmemory: "512mb"
   maxmemoryPolicy: "noeviction"