Skip to content

fix(knowledge): count documents through the reader's access and only where counts are shown - #8329

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/knowledge-counts-reader-access
Sep 26, 2026
Merged

waleedlatif1 merged 3 commits into
stagingfrom
fix/knowledge-counts-reader-access

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • v1 knowledge base list, detail, and update returned docCount/tokenCount over every document in the base, including connector documents the caller cannot read. They now count through the caller's access (resolveV1KnowledgeReadAccess), the same way the v1 documents routes already do. v1 detail also now returns its real connectorTypes
  • The internal KB list joined document through the full access predicate on every call, but only the Knowledge page shows the totals. The list contract gains includeCounts (default false), and without it the query reads only knowledge_base columns. The service option is countsFor: access, so a count without an access filter can't be written
  • React Query: counted lists get their own key beside list(), used only by the Knowledge page and its server prefetch. Document, upload, and connector mutations refresh only the counted lists; KB create, rename, delete, restore, and move refresh both
  • Removed getKnowledgeBaseById, whose unfiltered count join ran on every context resolution. Callers use getActiveKnowledgeBaseReference, and single-base totals come only from attachKnowledgeBaseConnectors(kb, access)
  • The v2 list keeps returning totals, since its public contract requires them

Type of Change

  • Bug fix

Testing

  • New app/api/v1/knowledge/route.integration.ts (real Postgres, only auth mocked): a read-role caller sees docCount: 1 for a base holding one upload plus one admin-only connector document on v1 list and detail. All 3 tests fail on the old code
  • Search-index policy integration suite and the knowledge service, application, and context unit suites updated and passing
  • Type-check, lint, and check:audits pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 26, 2026 8:16pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 25 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors knowledge-base document counting to respect access control.

The PR appears safe to merge based on the reviewed changes and current thread states.

Summary

The PR scopes v1 knowledge-base totals to documents the caller can read and makes internal-list totals opt-in, with a separate counted-list cache for the Knowledge page.

  • Removes unfiltered count reads from base-reference and update paths.
  • Updates list contracts, prefetching, cache invalidation, and integration coverage.

Reviews (3) · Last reviewed commit: "fix(knowledge): resolve the v1 update re..."

Comment thread apps/sim/app/api/v1/knowledge/[id]/route.ts
Comment thread apps/sim/app/api/v1/knowledge/[id]/route.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 27 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 27 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit e625a4c into staging Sep 26, 2026
32 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/knowledge-counts-reader-access branch September 26, 2026 21:29

This branch was previously deployed

1 inactive deployment
Preview — 5ef72897 Deployed Sep 26, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant