improvement(knowledge): read stored ACL expiry without a flag, and keep it current on renewal

Scope renewal extends an observation without changing who observes a document, so
it now carries that into the document's stored expiry instead of leaving it behind.
The read predicate falls back to proving freshness from the evidence when a row
carries no expiry, which is what an in-flight deploy or an unfinished backfill
leaves, so the change needs no rollout flag: `knowledge-acl-expiry` and its scope
plumbing are gone.

The backfill re-checks the row is still unset inside its update, so a writer that
fills one between the page read and the lock keeps its own value.

Author: waleedlatif1

apps/sim/lib/core/config/env.ts

@@ -630,7 +630,6 @@ export const env = createEnv({
     CREDENTIAL_GROUPS:                    z.boolean().optional(),                 // Enable enterprise Credential Groups globally
     KNOWLEDGE_MEMBER_ACCESS:              z.boolean().optional(),                 // Enable per-member knowledge connectors and hybrid-by-default retrieval globally
     KNOWLEDGE_TIN_KEYWORD:                z.boolean().optional(),                 // Rank large-scope keyword retrieval through the Tin text index where it exists
-    KNOWLEDGE_ACL_EXPIRY:                 z.boolean().optional(),                 // Read mirrored-permission freshness from document.acl_valid_until instead of per-candidate evidence
 
     // Organizations - for self-hosted deployments
     ORGANIZATIONS_ENABLED:                 z.boolean().optional(),                 // Enable organizations on self-hosted (bypasses plan requirements)

apps/sim/lib/core/config/feature-flags.test.ts

@@ -177,19 +177,6 @@ describe('isFeatureEnabled', () => {
     })
   })
 
-  describe('knowledge-acl-expiry flag', () => {
-    it('is a global switch', async () => {
-      expect(await isFeatureEnabled('knowledge-acl-expiry')).toBe(false)
-      envRef.KNOWLEDGE_ACL_EXPIRY = true
-      expect(await isFeatureEnabled('knowledge-acl-expiry')).toBe(true)
-    })
-
-    it('follows an AppConfig global rule', async () => {
-      withAppConfig({ 'knowledge-acl-expiry': { enabled: true } })
-      expect(await isFeatureEnabled('knowledge-acl-expiry')).toBe(true)
-    })
-  })
-
   describe('knowledge-member-access flag', () => {
     it('uses a global fallback switch off AppConfig', async () => {
       expect(await isFeatureEnabled('knowledge-member-access')).toBe(false)

apps/sim/lib/core/config/feature-flags.ts

@@ -93,15 +93,6 @@ const FEATURE_FLAGS = {
       'KNOWLEDGE_MEMBER_ACCESS.',
     fallback: 'KNOWLEDGE_MEMBER_ACCESS',
   },
-  'knowledge-acl-expiry': {
-    description:
-      'Read mirrored-permission freshness from document.acl_valid_until, written by every ACL ' +
-      'writer, instead of re-deriving it per candidate from verification and observation ' +
-      'timestamps. Strictly no wider than the per-candidate proof: a stored expiry is never ' +
-      'later than the evidence behind it. Requires the 0020 backfill. Off-AppConfig falls back ' +
-      'to KNOWLEDGE_ACL_EXPIRY.',
-    fallback: 'KNOWLEDGE_ACL_EXPIRY',
-  },
   'knowledge-tin-keyword': {
     description:
       'Rank keyword retrieval for members whose permitted set is too large to enumerate through ' +

apps/sim/lib/knowledge/access/predicate.postgres.test.ts

@@ -21,7 +21,9 @@ vi.mock('@/connectors/registry.server', () => ({ CONNECTOR_REGISTRY: {} }))
 const { drizzle } = await import('drizzle-orm/postgres-js')
 const schema = await import('@sim/db/schema')
 const { persistDocumentAcls } = await import('@/lib/knowledge/connectors/sync-persistence')
-const { materializeDocumentAcls } = await import('@/lib/knowledge/connectors/member-observations')
+const { materializeDocumentAcls, refreshObservedAclExpiry } = await import(
+  '@/lib/knowledge/connectors/member-observations'
+)
 const { mergeMirroredAcls, hideUnlistedDocuments } = await import(
   '@/lib/knowledge/connectors/mirrored-acls'
 )
@@ -88,8 +90,7 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
     join = false,
     githubInstallationGrants?: GitHubInstallationReadGrant[],
     userId = 'reader',
-    confluenceSiteGrants?: ConfluenceSiteReadGrant[],
-    storedAclFreshness = false
+    confluenceSiteGrants?: ConfluenceSiteReadGrant[]
   ): Promise<boolean> {
     const query = new PgDialect().sqlToQuery(
       knowledgeAccessCondition({
@@ -98,7 +99,6 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
         tokens,
         githubInstallationGrants,
         confluenceSiteGrants,
-        storedAclFreshness,
       })
     )
     const values = query.params.map((value: unknown) => {
@@ -504,10 +504,8 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
    * with the earliest observation it names, so one stale observer hides the document from readers
    * whose own observation is still current, until the sweep drops it and re-materializes.
    */
-  it('decides mirrored freshness from the row, never admitting what per-candidate evidence refuses', async () => {
-    const proof = (tokens: string[], id: string) => readable(tokens, id)
-    const stored = (tokens: string[], id: string) =>
-      readable(tokens, id, false, undefined, 'reader', undefined, true)
+  it('decides mirrored freshness from the row, falling back to evidence only where none is stored', async () => {
+    const stored = (tokens: string[], id: string) => readable(tokens, id)
 
     await putDocument('admin-fresh', ['u:alice@corp.com'])
     await connection.unsafe(
@@ -550,20 +548,32 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
       ['members-fresh', [alice], true],
       ['members-stale', [alice], false],
     ] as const) {
-      expect(await proof([...tokens], id)).toBe(expected)
       expect(await stored([...tokens], id)).toBe(expected)
     }
-    /** Stricter, never wider: the fresh observer waits for the sweep. */
-    expect(await proof([alice], 'members-mixed')).toBe(true)
+    /** Stricter than the evidence, never wider: the fresh observer waits for the sweep. */
     expect(await stored([alice], 'members-mixed')).toBe(false)
-    expect(await proof([bob], 'members-mixed')).toBe(false)
     expect(await stored([bob], 'members-mixed')).toBe(false)
-    /** A row the backfill has not reached yet is unreadable rather than assumed current. */
-    expect(await proof(['u:alice@corp.com'], 'admin-unbackfilled')).toBe(true)
+    /**
+     * Scope renewal extends an observation without changing who observes the document; carrying
+     * that into the stored expiry is what makes the document readable again.
+     */
+    await connection.unsafe(
+      `UPDATE knowledge_document_observation SET last_seen_at = statement_timestamp()
+       WHERE document_id = 'members-mixed' AND member_id = 'bob'`
+    )
+    expect(await stored([alice], 'members-mixed')).toBe(false)
+    await refreshObservedAclExpiry('members', ['members-mixed'], drizzle(connection, { schema }))
+    expect(await stored([alice], 'members-mixed')).toBe(true)
+    expect(await stored([bob], 'members-mixed')).toBe(true)
+
+    /** A row no current writer has touched still proves freshness from its evidence. */
+    expect(await stored(['u:alice@corp.com'], 'admin-unbackfilled')).toBe(true)
+    await connection.unsafe(
+      "UPDATE document SET acl_verified_at = statement_timestamp() - interval '25 hours' WHERE id = 'admin-unbackfilled'"
+    )
     expect(await stored(['u:alice@corp.com'], 'admin-unbackfilled')).toBe(false)
     /** Uploads and workspace-mode documents carry no expiry and are unaffected. */
     await connection.unsafe("INSERT INTO document(id) VALUES ('upload')")
-    expect(await proof(['ws'], 'upload')).toBe(true)
     expect(await stored(['ws'], 'upload')).toBe(true)
   })
 
@@ -608,7 +618,19 @@ describe.runIf(Boolean(databaseUrl))('knowledge ACLs in PostgreSQL', () => {
     expect(await readable([page], 'persisted')).toBe(false)
     expect(await readable([space, page], 'persisted')).toBe(true)
     await connection.unsafe(
-      "UPDATE document SET acl_verified_at = statement_timestamp() - interval '25 hours'"
+      `UPDATE document SET acl_verified_at = statement_timestamp() - interval '25 hours',
+       acl_valid_until = statement_timestamp() - interval '1 hour'`
+    )
+    expect(await readable([space, page], 'persisted')).toBe(false)
+    await persistDocumentAcls('admin', input, executor)
+    expect(await readable([space, page], 'persisted')).toBe(true)
+    /**
+     * A rolling-deploy writer that knows only the evidence column leaves the stored expiry behind,
+     * and the document waits for a current writer instead of being read from stale state.
+     */
+    await connection.unsafe(
+      `UPDATE document SET acl_verified_at = statement_timestamp(),
+       acl_valid_until = statement_timestamp() - interval '1 hour'`
     )
     expect(await readable([space, page], 'persisted')).toBe(false)
     await persistDocumentAcls('admin', input, executor)

apps/sim/lib/knowledge/access/predicate.ts

@@ -223,18 +223,21 @@ function storedKnowledgeAccessCondition(
   const cutoff = aclFreshnessCutoff()
   /**
    * Mirrored permissions are current while the row says so. Every writer of an ACL records when
-   * its evidence expires, so a candidate is checked with one comparison instead of a lookup per
-   * document into its connector's members and observations. The stored expiry is never later than
-   * the evidence it was written from: a members-mode document expires with the earliest
-   * observation its ACL names, so this can hide a document the per-candidate proof would still
-   * admit, and can never admit one it would refuse.
+   * its evidence expires, so a candidate costs one comparison instead of a lookup per document
+   * into its connector's members and observations. The stored expiry is never later than the
+   * evidence it was written from: a members-mode document expires with the earliest observation
+   * its ACL names, so this can hide a document the evidence would still admit, and can never
+   * admit one it would refuse.
+   *
+   * A row without an expiry is one no current writer has touched — the rollout's backfill has not
+   * reached it, or an older app version wrote its ACL during a deploy — so it falls back to
+   * proving freshness from the evidence itself. That branch is dead once every row carries an
+   * expiry.
    */
-  const mirroredIsCurrent = (mode: 'admin' | 'members'): SQL =>
-    scope.storedAclFreshness
-      ? sql`${knowledgeConnector.accessMode} = ${mode} AND ${document.aclValidUntil} > statement_timestamp()`
-      : mode === 'admin'
-        ? sql`${knowledgeConnector.accessMode} = 'admin' AND ${document.aclVerifiedAt} > ${cutoff}`
-        : sql`${knowledgeConnector.accessMode} = 'members' AND ${memberObservationCondition(tokens, cutoff)}`
+  const isCurrent = (evidence: SQL): SQL => sql`(
+      ${document.aclValidUntil} > statement_timestamp()
+      OR (${document.aclValidUntil} IS NULL AND ${evidence})
+    )`
   return sql`(
     ${aclOverlap(tokens)}
     AND NOT EXISTS (
@@ -254,8 +257,8 @@ function storedKnowledgeAccessCondition(
           AND (
             (${knowledgeConnector.accessMode} = 'workspace' AND ${document.acl} = ARRAY['ws']::text[])
             OR (${document.acl} <> ARRAY['ws']::text[] AND (
-            (${mirroredIsCurrent('admin')})
-            OR (${mirroredIsCurrent('members')})
+            (${knowledgeConnector.accessMode} = 'admin' AND ${isCurrent(sql`${document.aclVerifiedAt} > ${cutoff}`)})
+            OR (${knowledgeConnector.accessMode} = 'members' AND ${isCurrent(memberObservationCondition(tokens, cutoff))})
             ))
           )
       )

apps/sim/lib/knowledge/access/scope.ts

@@ -16,7 +16,6 @@ import {
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { and, eq, gte, inArray, isNull, or, sql } from 'drizzle-orm'
-import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { type ResourceScope, resourceScopeFromOwner } from '@/lib/core/resource-scope'
 import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
@@ -356,46 +355,25 @@ async function resolveKnowledgeIdentity(
   if (subject?.kind !== 'sim_user') {
     if (context.organizationId)
       throw new OrchestrationError('forbidden', 'Organization search requires a user subject')
-    const storedAclFreshness = await storedAclFreshnessEnabled()
-    return {
-      /** The shared constant while the rollout has not reached this request. */
-      access: storedAclFreshness.storedAclFreshness
-        ? { ...WORKSPACE_ACCESS_SCOPE, ...storedAclFreshness }
-        : WORKSPACE_ACCESS_SCOPE,
-      githubReaders: [],
-      confluenceReaders: [],
-    }
+    return { access: WORKSPACE_ACCESS_SCOPE, githubReaders: [], confluenceReaders: [] }
   }
   return resolveUserKnowledgeIdentity(subject.userId, context)
 }
 
 async function resolveUserKnowledgeIdentity(userId: string, context: KnowledgeAccessScopeContext) {
   resourceScopeFromOwner(context)
-  const [{ tokens, githubReaders = [], confluenceReaders = [] }, storedAclFreshness] =
-    await Promise.all([loadUserAccess(userId, context), storedAclFreshnessEnabled()])
+  const {
+    tokens,
+    githubReaders = [],
+    confluenceReaders = [],
+  } = await loadUserAccess(userId, context)
   return {
-    access: { kind: 'user' as const, userId, tokens, ...storedAclFreshness },
+    access: { kind: 'user' as const, userId, tokens },
     githubReaders,
     confluenceReaders,
   }
 }
 
-/**
- * Whether this request reads mirrored-permission freshness from the document row. Resolved once
- * per scope so the predicate stays synchronous; a configuration read that fails keeps the
- * per-candidate proof, which is correct and only slower.
- */
-async function storedAclFreshnessEnabled(): Promise<{ storedAclFreshness?: true }> {
-  try {
-    return (await isFeatureEnabled('knowledge-acl-expiry')) ? { storedAclFreshness: true } : {}
-  } catch (error) {
-    logger.warn('Feature flag read failed; deriving ACL freshness per candidate', {
-      error: getErrorMessage(error),
-    })
-    return {}
-  }
-}
-
 /**
  * The scope of a person identified only by user id — the shape session-backed
  * routes outside the application layer have in hand. Never call this with a
@@ -406,11 +384,7 @@ export async function resolveUserKnowledgeAccessScope(
   userId: string,
   workspaceId: string | undefined
 ): Promise<KnowledgeAccessScope> {
-  const [access, storedAclFreshness] = await Promise.all([
-    loadUserAccess(userId, { workspaceId }),
-    storedAclFreshnessEnabled(),
-  ])
-  return { kind: 'user', userId, tokens: access.tokens, ...storedAclFreshness }
+  return { kind: 'user', userId, tokens: (await loadUserAccess(userId, { workspaceId })).tokens }
 }
 
 /** Memoises {@link resolveKnowledgeAccessScope} for one operation; a failed lookup is retried on the next call. */

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

@@ -30,8 +30,6 @@ export const WORKSPACE_ACCESS_TOKENS = [PUBLIC_ACCESS_TOKEN, WORKSPACE_ACCESS_TO
 export interface WorkspaceAccessScope {
   kind: 'workspace'
   tokens: typeof WORKSPACE_ACCESS_TOKENS
-  /** See {@link UserAccessScope.storedAclFreshness}. */
-  storedAclFreshness?: true
 }
 
 export interface UserAccessScope {
@@ -47,14 +45,6 @@ export interface UserAccessScope {
   githubInstallationGrants?: readonly GitHubInstallationReadGrant[]
   /** Current reader access to the immutable Confluence site behind a central crawl. */
   confluenceSiteGrants?: readonly ConfluenceSiteReadGrant[]
-  /**
-   * Whether mirrored-permission freshness is read from `document.acl_valid_until` instead of
-   * being re-derived per candidate from observation and verification timestamps. Resolved once
-   * per scope from the `knowledge-acl-expiry` rollout flag, so a predicate stays synchronous and
-   * every surface that carries a scope agrees on one answer for the whole request. Absent while
-   * the flag is off, so a scope is unchanged until the rollout reaches it.
-   */
-  storedAclFreshness?: true
 }
 
 export interface ConfluenceSiteReadGrant {

apps/sim/lib/knowledge/connectors/member-observations.test.ts

@@ -91,7 +91,11 @@ describe('renewMemberObservationsInScopes', () => {
       renewed: 2,
       finished: true,
     })
-    expect(renewedIds()).toEqual([['d-kept', 'd-dm']])
+    /** The renewed documents are re-materialized too, so their stored expiry follows. */
+    expect(renewedIds()).toEqual([
+      ['d-kept', 'd-dm'],
+      ['d-kept', 'd-dm'],
+    ])
   })
 
   it('matches every id under a broader scope that covers a narrower one', async () => {
@@ -101,7 +105,11 @@ describe('renewMemberObservationsInScopes', () => {
     ])
     dbChainMockFns.returning.mockResolvedValueOnce([{ documentId: 'd-1' }, { documentId: 'd-2' }])
     await renew(['slack:v4:T1:C2:', 'slack:v4:T1:'])
-    expect(renewedIds()).toEqual([['d-1', 'd-2']])
+    /** The renewed documents are re-materialized too, so their stored expiry follows. */
+    expect(renewedIds()).toEqual([
+      ['d-1', 'd-2'],
+      ['d-1', 'd-2'],
+    ])
   })
 
   it('writes nothing when no scope is granted and stops at its deadline', async () => {

apps/sim/lib/knowledge/connectors/member-observations.ts

@@ -215,8 +215,8 @@ export async function renewMemberObservationsInScopes(input: {
       .map((row) => row.documentId)
     if (renewable.length > 0) {
       const now = new Date()
-      const rows = await input.withLease((tx) =>
-        tx
+      const rows = await input.withLease(async (tx) => {
+        const updated = await tx
           .update(knowledgeDocumentObservation)
           .set({ lastSeenAt: now })
           .where(
@@ -227,7 +227,20 @@ export async function renewMemberObservationsInScopes(input: {
             )
           )
           .returning({ documentId: knowledgeDocumentObservation.documentId })
-      )
+        /**
+         * Renewed evidence has to reach the document too: the read path takes a document's
+         * freshness from its stored expiry, which is only as current as its last write. Renewal
+         * cannot change who observes a document, so the ACL itself stays as it is.
+         */
+        if (updated.length > 0) {
+          await refreshObservedAclExpiry(
+            input.connectorId,
+            updated.map((row) => row.documentId),
+            tx
+          )
+        }
+        return updated
+      })
       renewed += rows.length
     }
     if (page.length < RENEWAL_PAGE_SIZE) return { renewed, finished: true }
@@ -349,6 +362,31 @@ export async function rewriteConnectorAcls(
   }
 }
 
+/**
+ * Carries renewed observation freshness into the documents it covers, leaving their ACLs alone.
+ * Renewal only extends how long existing evidence stands, so the expiry is the one thing that
+ * moves.
+ */
+export async function refreshObservedAclExpiry(
+  connectorId: string,
+  documentIds: readonly string[],
+  executor: DbOrTx = db
+): Promise<void> {
+  for (let offset = 0; offset < documentIds.length; offset += MATERIALIZE_BATCH_SIZE) {
+    const batch = documentIds.slice(offset, offset + MATERIALIZE_BATCH_SIZE)
+    await executor
+      .update(document)
+      .set({ aclValidUntil: observedAclValidUntil() })
+      .where(
+        and(
+          inArray(document.id, batch),
+          eq(document.connectorId, connectorId),
+          sql`${document.aclValidUntil} IS DISTINCT FROM ${observedAclValidUntil()}`
+        )
+      )
+  }
+}
+
 export async function materializeDocumentAcls(
   connectorId: string,
   documentIds: Iterable<string>,

packages/db/script-migrations/0016_backfill_search_vectors.postgres.test.ts

@@ -391,6 +391,7 @@ describe.runIf(Boolean(databaseUrl))('search projection upgrade in PostgreSQL',
       { name: '0017_index_search_documents' },
       { name: '0018_repair_workspace_file_content_revision' },
       { name: '0019_tin_keyword_projection' },
+      { name: '0020_document_acl_valid_until' },
     ])
     const [{ complete }] = await sql`SELECT count(*)::int AS complete FROM embedding e
       JOIN embedding_search s ON s.id = e.id JOIN embedding_keyword_search k ON k.id = e.id