feat(platform): configurable upload and retention policies#1393
Merged
Conversation
📝 WalkthroughWalkthroughThis PR implements configurable governance policies for upload and retention management. It introduces UI components ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This was referenced Apr 11, 2026
Closed
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
larryro
force-pushed
the
feat/issue-1363-upload-retention
branch
from
46dbcfe to
8658b72
Compare
Wire the existing governance data model for upload and retention policies into a complete feature: admin UI editors, server-side enforcement, per-user volume tracking, client-side dynamic limits, cron-based retention cleanup, and audit logging. Upload policies (#1363): - Add uploadedBy field and by_org_user index to fileMetadata schema - Enforce upload policy in saveFileMetadata and createDocumentFromUpload - Fix per-user volume check to filter by uploadedBy instead of all org - Add UploadPolicyEditor component with extension, MIME, size, volume - Add useUploadPolicy hook for client-side dynamic limit awareness - Override hardcoded limits in chat upload hook and document upload dialog - Validate upload_policy config with Zod in upsertPolicy mutation Retention policies (#1169): - Register runRetentionCleanup as daily cron at 4 AM UTC - Fix listExpiredDocuments to filter by scope/sourceProvider - Add fileMetadata cleanup to deleteExpiredDocument - Add audit logging for automated retention deletions - Add per-document error handling with summary logging - Add RetentionPolicyEditor component with scope, days, batch size - Validate retention_policy config with Zod in upsertPolicy mutation Closes #1363 Closes #1169
larryro
force-pushed
the
feat/issue-1363-upload-retention
branch
from
8658b72 to
7779e3d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
services/platform/convex/governance/retention_cleanup.ts (1)
57-60:⚠️ Potential issue | 🔴 CriticalRetention
scopeis not applied when listing expired documents.At Line 59,
listExpiredDocumentsis called withoutscope, so upload/agent scoping from policy config is not enforced in this path.Suggested change
const expiredDocs = await ctx.runQuery( internal.governance.internal_queries.listExpiredDocuments, - { organizationId, cutoffMs, batchSize }, + { + organizationId, + cutoffMs, + batchSize, + scope: config.scope ?? 'all', + }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/governance/retention_cleanup.ts` around lines 57 - 60, The expired document query call in retention_cleanup.ts omits the retention scope, so upload/agent scoping from the policy isn't enforced; update the ctx.runQuery call to pass the policy's scope (e.g., include scope: policy.scope or scope variable derived from the retention policy) alongside organizationId, cutoffMs, and batchSize when invoking internal.governance.internal_queries.listExpiredDocuments (the call that assigns expiredDocs) so the query filters by the correct scope. Ensure the variable name you use matches how the retention policy/scope is named in this file.services/platform/convex/governance/mutations.ts (1)
60-92:⚠️ Potential issue | 🟠 MajorPersist the validated config, not the raw input.
In
upsertPolicy, validation checks occur at lines 42–92, but lines 107 and 117 persistargs.configdirectly instead of usingparsed.data. Since Zod'sz.object({...}).safeParse()strips unknown keys by default, any extra keys in the client input will survive validation and be stored in the database, defeating the schema contract. Use the validated data fromparsed.datafor persistence.The
upsertPiiConfigmutation (lines 174–182) demonstrates the correct pattern: validate the input, then use a constructed config object (lines 193–197) for storage.Also, move the validation schemas from
lib/shared/schemas/governancetolib/shared/validators/governance.tsper the coding guideline: validators should be organized per domain inlib/shared/validators/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/governance/mutations.ts` around lines 60 - 92, In upsertPolicy, the code validates args.config using safeParse but then persists the raw args.config; change the persistence to use the validated result (parsed.data) for each branch (upload_policy, pii_config, feature_flags, retention_policy) so only the sanitized, typed config is stored (mirror the approach used in upsertPiiConfig). Also relocate the Zod schemas from lib/shared/schemas/governance to lib/shared/validators/governance.ts and update imports in this file to import the validators instead of the old schemas.services/platform/convex/file_metadata/mutations.ts (3)
48-54:⚠️ Potential issue | 🟠 MajorPreserve the original
uploadedByon updates.The upsert path now rewrites
uploadedByevery time the samestorageIdis saved. That retroactively moves ownership, quota usage, and audit attribution to the latest caller instead of the user who actually uploaded the file. Only setuploadedByon insert, or fill it on patch only when the existing row is missing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/file_metadata/mutations.ts` around lines 48 - 54, The current upsert path always sets uploadedBy in the patchData (when existing is truthy), overwriting original ownership; change the logic in the mutation handling (the block that builds patchData for existing rows, and related upsert code) so uploadedBy is only included on insert or included in patchData only if the existing row has no uploadedBy. Concretely, when constructing patchData (the variable named patchData in this mutation), omit setting uploadedBy unless you are in the insert branch or you first check existing.uploadedBy is null/undefined and only then set uploadedBy from args.uploadedBy/userId.
31-46:⚠️ Potential issue | 🟠 MajorCheck for an existing
storageIdbefore enforcing per-user quota.This mutation enforces upload policy before the
by_storageIdlookup, so retryingsaveFileMetadatafor an already-recorded file can count that same file twice. Users at or near quota will start getting false rejections on idempotent retries even though no new storage is being added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/file_metadata/mutations.ts` around lines 31 - 46, The code calls checkUploadPolicy before checking for an existing file entry, which can double-count retries; move the ctx.db.query('fileMetadata').withIndex('by_storageId', (q) => q.eq('storageId', args.storageId)).first() lookup to run before invoking checkUploadPolicy (or call it first and short‑circuit if a matching record exists), and if an existing record is found return or reuse that metadata instead of enforcing per-user quota; update the flow around checkUploadPolicy and the mutation (saveFileMetadata / this mutation handler) so idempotent retries don't trigger quota checks for already-stored storageId.
23-41:⚠️ Potential issue | 🔴 CriticalEnforce org membership before this public mutation touches policy or metadata.
Right now any authenticated user can call
saveFileMetadatawith an arbitraryorganizationId, which lets them write metadata into another org and probe that org’s upload-policy behavior. This handler should follow the normal mutation boundary flow and callgetOrganizationMember(...)before continuing.🔐 Suggested fix
+import { getOrganizationMember } from '../lib/rls'; @@ async handler(ctx, args) { const authUser = await authComponent.getAuthUser(ctx); if (!authUser) { throw new Error('Unauthenticated'); } + + await getOrganizationMember(ctx, args.organizationId, { + userId: String(authUser._id), + email: authUser.email, + name: authUser.name, + }); const userId = String(authUser._id);Based on learnings: Enforce the established authorization flow in mutation handlers: authenticate → getOrganizationMember → perform operation, and do not introduce authorizeRls() into mutation handlers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/file_metadata/mutations.ts` around lines 23 - 41, The handler for saveFileMetadata currently authenticates the user then immediately calls checkUploadPolicy and proceeds; instead, after obtaining authUser via authComponent.getAuthUser and computing userId, call getOrganizationMember(ctx, args.organizationId, userId) and verify membership/roles before any call to checkUploadPolicy or writing metadata. Move the getOrganizationMember check to occur between authComponent.getAuthUser(...) and checkUploadPolicy(...), aborting with an error if the user is not a member, and then continue with extractExtension, checkUploadPolicy, and the rest; do not add authorizeRls() in this mutation.services/platform/convex/governance/internal_mutations_retention.ts (1)
14-18:⚠️ Potential issue | 🟠 MajorUse
doc.organizationIdfor the audit log, not the caller-provided org id.This mutation already loaded the document, so trusting a second
organizationIdinput can write the retention audit record into the wrong org if the caller passes stale or incorrect data. Either validate the match immediately afterctx.db.get(...)or derive the audit org directly fromdoc.organizationId.Based on learnings: For conversation write paths, validate
args.organizationIdmatches the loaded record immediately after fetching it and before any writes to prevent cross-org mutations if an ID leaks.Also applies to: 48-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/governance/internal_mutations_retention.ts` around lines 14 - 18, After loading the document with ctx.db.get(args.documentId), do not trust args.organizationId for the retention audit; immediately validate that args.organizationId === doc.organizationId (and return an error/null if it doesn't match) or, better, derive the audit organization from doc.organizationId when creating the retention audit record. Update the retention-audit creation path (the code that writes the audit record later in this mutation) to use doc.organizationId instead of args.organizationId and add the immediate match-check right after retrieving doc to prevent cross-org writes.services/platform/messages/de.json (1)
3690-3887:⚠️ Potential issue | 🔴 CriticalResolve the merge conflict markers in this locale file.
The
<<<<<<<,=======, and>>>>>>>markers makede.jsoninvalid and are already breaking lint/build. When resolving this, merge the newuploadPolicy/retentionPolicyentries into the existinggovernanceobject instead of picking one side wholesale, otherwise you'll either drop translations or keep duplicatetabs/featureFlagskeys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/messages/de.json` around lines 3690 - 3887, Remove the Git conflict markers and merge the two conflicting blocks by consolidating the uploadPolicy and retentionPolicy keys into the existing governance/localization object rather than choosing one side; keep all unique fields from both variants (e.g., retain mbUnit/gbUnit/extensionPlaceholder/mimeTypePlaceholder/blockedExtensions/allowedMimeTypes/enabled/saveFailed and the retention presets/batchSize/warning/scopeAll/scopeUpload/scopeAgent from the other side) while preserving the original budgets, defaultModels, featureFlags and tabs entries (ensure no duplicate "tabs" or "featureFlags" keys remain); delete the <<<<<<<, =======, >>>>>>> markers and verify the final JSON contains a single uploadPolicy and a single retentionPolicy object with the merged properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/features/chat/hooks/use-convex-file-upload.ts`:
- Around line 51-62: The mergedConfig currently injects
policyLimits.allowedTypes but downstream code still allows text files via an
isTextBasedFile(...) fallback causing client-side acceptance of files the server
may reject; update the logic that computes mergedConfig (and/or the downstream
upload validation that uses isTextBasedFile) so that when
policyLimits.policyEnabled is true the text-file fallback is disabled — e.g., if
policyLimits.policyEnabled then ensure mergedConfig.allowedTypes is
authoritative and do not call or honor isTextBasedFile, or fold text allowances
into policyLimits.allowedTypes; target the useUploadPolicy/mergedConfig creation
and the downstream isTextBasedFile check to enforce this gating.
In
`@services/platform/app/features/documents/components/__tests__/document-upload-dialog.test.tsx`:
- Around line 50-57: The current mock of useUploadPolicy in
document-upload-dialog.test.tsx forces policyEnabled: false and so never
exercises the new policy-driven upload path; update the tests to add new cases
that set useUploadPolicy.policyEnabled to true (and vary allowedTypes,
maxFileSize, documentMaxFileSize) and assert DocumentUploadDialog shows the
correct UI feedback and validation for type and size violations as well as the
happy path when policy allows the file; keep existing legacy tests too so you
cover both code paths and assert the specific error messages/components rendered
when policy rejects an upload.
In
`@services/platform/app/features/documents/components/document-upload-dialog.tsx`:
- Around line 60-64: The component only applies policyLimits.documentMaxFileSize
(via effectiveMaxFileSize) but still uses hardcoded DOCUMENT_UPLOAD_ACCEPT and
isAllowedDocumentUpload for the file picker and pre-upload validation; update
the file type filtering to use the upload policy from useUploadPolicy
(policyLimits) instead of hardcoded constants so picker accept attribute and
validation reflect admin-configured extensions/MIME types. Concretely, read
allowed file types/extensions and/or mime types from policyLimits (fallback to
DOCUMENT_UPLOAD_ACCEPT / existing lists if policy not enabled), replace usage of
DOCUMENT_UPLOAD_ACCEPT in the file input accept prop, and update
isAllowedDocumentUpload (or its call sites) to consult
policyLimits.allowedExtensions/allowedMimeTypes (with the same fallback)
wherever file-type validation occurs (including the other two blocks mentioned)
so both UI and pre-upload checks mirror backend governance.
In
`@services/platform/app/features/settings/governance/components/__tests__/retention-policy-editor.test.tsx`:
- Around line 57-65: The test never exercises the enabled path; update the
'renders warning banner when enabled' test to remock the useGovernancePolicy
hook (not vi.fn()) to return a policy object with enabled: true (or the shape
the component expects) before rendering RetentionPolicyEditor, then assert that
an alert role exists (e.g.,
expect(screen.getByRole('alert')).toBeInTheDocument()). Specifically target
useGovernancePolicy via vi.mocked(useGovernancePolicy).mockReturnValue(...) (or
mockResolvedValue if it’s async) and change the final assertion from
queryByRole(...).toBeNull() to checking presence of the alert.
In
`@services/platform/app/features/settings/governance/components/__tests__/upload-policy-editor.test.tsx`:
- Around line 32-61: Add interaction tests for UploadPolicyEditor: mock the
save/mutation function (e.g., the update/saveUploadPolicy mutation or API client
used by the component) and assert it is called with the correct payload when the
user toggles the enable switch, edits "allowed file extensions", "maximum file
size" and "maximum total volume", and clicks the save button; also add a
failing-input test that types invalid values (e.g., non-numeric size or empty
extensions), asserts validation error messages appear and that the save/mutation
mock is NOT called. Locate tests around UploadPolicyEditor in
upload-policy-editor.test.tsx and use userEvent to simulate interactions, jest
mocks for the mutation, and screen queries to assert payload and validation
behavior.
In
`@services/platform/app/features/settings/governance/components/retention-policy-editor.tsx`:
- Around line 79-82: The save handler (handleSave) currently returns silently
when retentionDays is NaN or negative; update it to validate and surface an
accessible error instead: add a validation state (e.g., retentionError) set when
Number(retentionDays) is NaN or < 1 (or change input from min={0} to min={1} to
match intent), prevent save when invalid, and ensure the input uses
aria-describedby pointing to the error message so screen readers can read it;
update the UI to render a clear message naming the field and how to fix it
(e.g., "Retention days must be a whole number >= 1") and ensure handleSave
checks this same validation before proceeding.
- Around line 83-92: The config object in retention-policy-editor.tsx
incorrectly includes a non-existent "scope" field and overwrites temp-retention
settings; remove "scope" from the constructed RetentionPolicyConfig and ensure
you merge/preserve existing temp-retention fields (userTempEnabled,
userTempRetentionHours, agentTempEnabled, agentTempRetentionHours) when building
the config before saving (e.g., start from the existing config object and set
enabled, retentionDays and batchSize as needed, keeping any existing
temp-related properties intact); also keep the batchSize handling (batch
variable / Number(batchSize)) but assign it only when valid.
In
`@services/platform/app/features/settings/governance/components/upload-policy-editor.tsx`:
- Around line 103-110: The buildConfig() logic currently omits numeric fields
when they parse to 0/NaN which causes saved policies to lose constraints; update
buildConfig() to validate populated numeric inputs (maxFileSizeMB and
maxVolumeGB) and reject the save by setting a field error instead of dropping
the field: if the input is empty allow omission, but if it's non-empty and
Number.isNaN(value) or value <= 0, call the form's field error setter (or add to
the component's validationErrors) for the corresponding property and
return/throw to block the save; only when the parsed value is a positive number
set config.maxFileSizeBytes = sizeMB * 1024 * 1024 and
config.maxTotalVolumeBytesPerUser = volGB * 1024 * 1024 * 1024.
In
`@services/platform/app/features/settings/governance/hooks/use-upload-policy.ts`:
- Around line 49-57: The hook useUploadPolicy currently only exposes
allowedMimeTypes via allowedTypes, so client pickers won't reflect server-side
allowedExtensions/blockedExtensions; update the returned object from
use-upload-policy (e.g., the function producing maxFileSize, allowedTypes,
documentMaxFileSize, policyEnabled) to also include extension constraints
(return allowedExtensions and blockedExtensions fields) and/or compute a
combined accept list that merges config.allowedMimeTypes and
config.allowedExtensions (deduplicated) so the client UI can derive the file
input accept string from both MIME types and extensions; ensure you reference
config.allowedExtensions and config.blockedExtensions when building the policy
returned to the client.
In
`@services/platform/convex/documents/__tests__/create_document_from_upload.test.ts`:
- Around line 76-78: Add a test that exercises the policy-denial branch by
stubbing checkUploadPolicy to resolve { allowed: false, reason: 'reason text' }
and asserting that calling the createDocumentFromUpload mutation rejects with
that reason and does not create a document; specifically, in the
create_document_from_upload.test.ts test suite replace or add a mock for
checkUploadPolicy returning allowed:false with a reason, invoke
createDocumentFromUpload (the same helper or mutation used in existing tests),
expect the call to throw the provided reason, and verify no document creation
logic (e.g., createDocument/DB insert or appropriate mutation) was invoked.
In `@services/platform/convex/documents/mutations.ts`:
- Around line 128-141: The policy check is bypassable when args.fileSize is
omitted; ensure fileSize is required before calling checkUploadPolicy and before
skipping the fileMetadata insert: validate that args.fileSize is present (e.g.,
coerce/throw if undefined) and pass the numeric value into checkUploadPolicy
(instead of undefined), and ensure the subsequent logic that inserts
fileMetadata always runs when fileSize is present; update the call sites around
checkUploadPolicy (the block using userId, ext, resolvedContentType) and the
analogous block at lines 166-175 to enforce and use args.fileSize so max-size
limits and per-user volume tracking cannot be skipped.
In `@services/platform/convex/file_metadata/__tests__/mutations.test.ts`:
- Around line 48-50: Add a test that simulates a denied policy by mocking
checkUploadPolicy to resolve { allowed: false } and asserting saveFileMetadata
does not perform any DB writes; specifically, in the mutations.test.ts suite
replace or add a case that sets vi.mock('../../governance/upload_enforcement',
...) to return { allowed: false } and then call the mutation (saveFileMetadata)
and assert that the DB write helpers used by the implementation (e.g.,
ctx.db.insert and ctx.db.patch or whatever write functions saveFileMetadata
calls) were not invoked and that the mutation either throws/rejects or returns
the appropriate denial response. Ensure the test covers both the mock setup for
checkUploadPolicy and explicit expect(...not.toHaveBeenCalled()) checks against
the concrete DB write spies.
In `@services/platform/convex/file_metadata/queries.ts`:
- Around line 12-15: The handler's call to authComponent.getAuthUser(ctx) can
throw and must be treated like other public queries: wrap the call in a
try/catch, on success use authUser as before, but on any thrown error swallow it
(empty catch) and treat as unauthenticated by returning { totalBytes: 0 } (or
proceeding with authUser === null flow); update the async handler in queries.ts
to catch errors from authComponent.getAuthUser(ctx) and return the
unauthenticated fallback instead of letting the exception bubble.
In `@services/platform/convex/governance/__tests__/upload_enforcement.test.ts`:
- Around line 17-35: The mock QueryBuilder returned by createMockQueryBuilder
currently ignores the index predicate and yields every row; update
withIndex(indexName, fn) to apply the provided predicate fn to each element of
mockFileMetadataRows and only yield rows where fn(row) is truthy (so filtering
by uploadedBy / organization works), and then add at least one non-matching
entry to mockFileMetadataRows so tests actually exercise the filtering behavior;
locate createMockQueryBuilder and mockFileMetadataRows in the test and implement
the predicate-based filtering there.
In `@services/platform/convex/governance/internal_queries.ts`:
- Around line 99-102: The code in listExpiredDocuments reads args.scope (lines
referencing args.scope) but the args validator for listExpiredDocuments doesn't
declare scope and callers (e.g., retention_cleanup.ts) don't pass it, causing a
type/contract mismatch and dead filtering; fix by either adding a scoped
enum/string property to the listExpiredDocuments args validator and update
callers (e.g., retention_cleanup.ts) to pass scope values like 'upload' or
'agent' and then keep the two if checks, or remove the two scope-based if checks
entirely from listExpiredDocuments if you don't intend to filter by
sourceProvider—update the validator accordingly so TypeScript strict mode and
the query contract are consistent with the chosen approach.
In `@services/platform/convex/governance/retention_cleanup.ts`:
- Around line 77-82: The current await of
ctx.runMutation(internal.governance.internal_mutations_retention.deleteExpiredDocument,
{ documentId: doc._id, organizationId }) can throw and abort the whole cleanup
loop; wrap that call in a try/catch inside the loop (around the call to
deleteExpiredDocument) so failures for one doc are caught, log or record the
error (include doc._id and organizationId) and then continue processing
remaining docs; optionally aggregate errors to report at the end but do not
rethrow inside the per-document catch so the cleanup run proceeds.
In `@services/platform/lib/shared/schemas/__tests__/governance.test.ts`:
- Around line 93-100: The test fails because retentionPolicyConfigSchema
currently allows arbitrary strings for the scope field; update the schema
definition for retentionPolicyConfigSchema to validate scope as an enum/union of
the allowed scope strings (e.g., the project's supported values such as
"global", "organization", "user" — or whatever the codebase uses) instead of a
free string so that safeParse rejects values like 'invalid'; locate and change
the scope property in the retentionPolicyConfigSchema to use z.enum or z.union
of literal strings (or equivalent validator) so the test expecting
result.success to be false for 'invalid' passes.
In `@services/platform/messages/en.json`:
- Line 1455: The localization key settings.providers.description currently
contains the literal string "Description", causing the providers settings page
to render a label instead of the intended subtitle; update
settings.providers.description in the messages file to the correct subtitle text
(restore the original descriptive sentence) so the UI shows the proper
subtitle—if unsure, retrieve the prior value from git history or ask the i18n/UX
owner and replace the literal "Description" with that restored subtitle.
- Around line 3778-3797: Remove the Git conflict markers (<<<<<<<, =======,
>>>>>>>) and merge the two JSON variants into a single consistent object that
includes the needed localization keys referenced by the UI: keep or reconcile
"tempFileRetentionHours" if still used, and ensure "scopeAll", "scopeUpload",
"scopeAgent", "batchSize", "batchSizeHint", "warning", "presets", "saved", and
"saveFailed" are all present with appropriate string values; validate the
resulting JSON is syntactically correct so the linter error TS1185 goes away and
the retention-policy-editor’s key lookups
("scopeAll"/"scopeUpload"/"scopeAgent") resolve.
---
Outside diff comments:
In `@services/platform/convex/file_metadata/mutations.ts`:
- Around line 48-54: The current upsert path always sets uploadedBy in the
patchData (when existing is truthy), overwriting original ownership; change the
logic in the mutation handling (the block that builds patchData for existing
rows, and related upsert code) so uploadedBy is only included on insert or
included in patchData only if the existing row has no uploadedBy. Concretely,
when constructing patchData (the variable named patchData in this mutation),
omit setting uploadedBy unless you are in the insert branch or you first check
existing.uploadedBy is null/undefined and only then set uploadedBy from
args.uploadedBy/userId.
- Around line 31-46: The code calls checkUploadPolicy before checking for an
existing file entry, which can double-count retries; move the
ctx.db.query('fileMetadata').withIndex('by_storageId', (q) => q.eq('storageId',
args.storageId)).first() lookup to run before invoking checkUploadPolicy (or
call it first and short‑circuit if a matching record exists), and if an existing
record is found return or reuse that metadata instead of enforcing per-user
quota; update the flow around checkUploadPolicy and the mutation
(saveFileMetadata / this mutation handler) so idempotent retries don't trigger
quota checks for already-stored storageId.
- Around line 23-41: The handler for saveFileMetadata currently authenticates
the user then immediately calls checkUploadPolicy and proceeds; instead, after
obtaining authUser via authComponent.getAuthUser and computing userId, call
getOrganizationMember(ctx, args.organizationId, userId) and verify
membership/roles before any call to checkUploadPolicy or writing metadata. Move
the getOrganizationMember check to occur between authComponent.getAuthUser(...)
and checkUploadPolicy(...), aborting with an error if the user is not a member,
and then continue with extractExtension, checkUploadPolicy, and the rest; do not
add authorizeRls() in this mutation.
In `@services/platform/convex/governance/internal_mutations_retention.ts`:
- Around line 14-18: After loading the document with
ctx.db.get(args.documentId), do not trust args.organizationId for the retention
audit; immediately validate that args.organizationId === doc.organizationId (and
return an error/null if it doesn't match) or, better, derive the audit
organization from doc.organizationId when creating the retention audit record.
Update the retention-audit creation path (the code that writes the audit record
later in this mutation) to use doc.organizationId instead of args.organizationId
and add the immediate match-check right after retrieving doc to prevent
cross-org writes.
In `@services/platform/convex/governance/mutations.ts`:
- Around line 60-92: In upsertPolicy, the code validates args.config using
safeParse but then persists the raw args.config; change the persistence to use
the validated result (parsed.data) for each branch (upload_policy, pii_config,
feature_flags, retention_policy) so only the sanitized, typed config is stored
(mirror the approach used in upsertPiiConfig). Also relocate the Zod schemas
from lib/shared/schemas/governance to lib/shared/validators/governance.ts and
update imports in this file to import the validators instead of the old schemas.
In `@services/platform/convex/governance/retention_cleanup.ts`:
- Around line 57-60: The expired document query call in retention_cleanup.ts
omits the retention scope, so upload/agent scoping from the policy isn't
enforced; update the ctx.runQuery call to pass the policy's scope (e.g., include
scope: policy.scope or scope variable derived from the retention policy)
alongside organizationId, cutoffMs, and batchSize when invoking
internal.governance.internal_queries.listExpiredDocuments (the call that assigns
expiredDocs) so the query filters by the correct scope. Ensure the variable name
you use matches how the retention policy/scope is named in this file.
In `@services/platform/messages/de.json`:
- Around line 3690-3887: Remove the Git conflict markers and merge the two
conflicting blocks by consolidating the uploadPolicy and retentionPolicy keys
into the existing governance/localization object rather than choosing one side;
keep all unique fields from both variants (e.g., retain
mbUnit/gbUnit/extensionPlaceholder/mimeTypePlaceholder/blockedExtensions/allowedMimeTypes/enabled/saveFailed
and the retention presets/batchSize/warning/scopeAll/scopeUpload/scopeAgent from
the other side) while preserving the original budgets, defaultModels,
featureFlags and tabs entries (ensure no duplicate "tabs" or "featureFlags" keys
remain); delete the <<<<<<<, =======, >>>>>>> markers and verify the final JSON
contains a single uploadPolicy and a single retentionPolicy object with the
merged properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b07918f-94e3-4957-aad5-3ddff62793b6
📒 Files selected for processing (28)
services/platform/app/features/chat/hooks/use-convex-file-upload.tsservices/platform/app/features/documents/components/__tests__/document-upload-dialog.test.tsxservices/platform/app/features/documents/components/document-upload-dialog.tsxservices/platform/app/features/settings/governance/components/__tests__/retention-policy-editor.test.tsxservices/platform/app/features/settings/governance/components/__tests__/upload-policy-editor.test.tsxservices/platform/app/features/settings/governance/components/retention-policy-editor.tsxservices/platform/app/features/settings/governance/components/upload-policy-editor.tsxservices/platform/app/features/settings/governance/hooks/use-upload-policy.tsservices/platform/app/routes/dashboard/$id/settings/governance.tsxservices/platform/convex/crons.tsservices/platform/convex/documents/__tests__/create_document_from_upload.test.tsservices/platform/convex/documents/mutations.tsservices/platform/convex/file_metadata/__tests__/mutations.test.tsservices/platform/convex/file_metadata/internal_mutations.tsservices/platform/convex/file_metadata/mutations.tsservices/platform/convex/file_metadata/queries.tsservices/platform/convex/file_metadata/schema.tsservices/platform/convex/governance/__tests__/upload_enforcement.test.tsservices/platform/convex/governance/internal_mutations_retention.tsservices/platform/convex/governance/internal_queries.tsservices/platform/convex/governance/mutations.tsservices/platform/convex/governance/retention_cleanup.tsservices/platform/convex/governance/upload_enforcement.tsservices/platform/lib/shared/schemas/__tests__/governance.test.tsservices/platform/messages/de-AT.jsonservices/platform/messages/de-CH.jsonservices/platform/messages/de.jsonservices/platform/messages/en.json
Comment on lines
+51
to
+62
| const policyLimits = useUploadPolicy(config.organizationId); | ||
|
|
||
| const mergedConfig = useMemo( | ||
| () => ({ ...DEFAULT_UPLOAD_CONFIG, ...config }), | ||
| [config], | ||
| () => ({ | ||
| ...DEFAULT_UPLOAD_CONFIG, | ||
| ...config, | ||
| ...(policyLimits.policyEnabled && { | ||
| maxFileSize: policyLimits.maxFileSize, | ||
| allowedTypes: policyLimits.allowedTypes, | ||
| }), | ||
| }), | ||
| [config, policyLimits], |
There was a problem hiding this comment.
Policy-enabled uploads still bypass admin type restrictions for text files.
This merges policyLimits.allowedTypes into mergedConfig, but the downstream || isTextBasedFile(...) fallback still accepts text files even when the admin policy disables them. The result is a misleading client check: users can upload files the server will reject. Gate that fallback off when policyLimits.policyEnabled is true, or fold those allowances into the policy itself.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/hooks/use-convex-file-upload.ts` around
lines 51 - 62, The mergedConfig currently injects policyLimits.allowedTypes but
downstream code still allows text files via an isTextBasedFile(...) fallback
causing client-side acceptance of files the server may reject; update the logic
that computes mergedConfig (and/or the downstream upload validation that uses
isTextBasedFile) so that when policyLimits.policyEnabled is true the text-file
fallback is disabled — e.g., if policyLimits.policyEnabled then ensure
mergedConfig.allowedTypes is authoritative and do not call or honor
isTextBasedFile, or fold text allowances into policyLimits.allowedTypes; target
the useUploadPolicy/mergedConfig creation and the downstream isTextBasedFile
check to enforce this gating.
Comment on lines
+50
to
+57
| vi.mock('@/app/features/settings/governance/hooks/use-upload-policy', () => ({ | ||
| useUploadPolicy: () => ({ | ||
| maxFileSize: 10 * 1024 * 1024, | ||
| allowedTypes: ['application/pdf', 'image/png'], | ||
| documentMaxFileSize: 50 * 1024 * 1024, | ||
| policyEnabled: false, | ||
| }), | ||
| })); |
There was a problem hiding this comment.
Current mock bypasses the new policy-driven upload behavior.
At Line 55, policyEnabled: false keeps tests on legacy behavior only. Add cases with policy enabled (size/type violations and UI feedback) to validate the new governance path.
As per coding guidelines: "ALWAYS write tests for new features and bug fixes. Tests should cover happy paths, edge cases, and error conditions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/documents/components/__tests__/document-upload-dialog.test.tsx`
around lines 50 - 57, The current mock of useUploadPolicy in
document-upload-dialog.test.tsx forces policyEnabled: false and so never
exercises the new policy-driven upload path; update the tests to add new cases
that set useUploadPolicy.policyEnabled to true (and vary allowedTypes,
maxFileSize, documentMaxFileSize) and assert DocumentUploadDialog shows the
correct UI feedback and validation for type and size violations as well as the
happy path when policy allows the file; keep existing legacy tests too so you
cover both code paths and assert the specific error messages/components rendered
when policy rejects an upload.
Comment on lines
+60
to
+64
| const policyLimits = useUploadPolicy(organizationId); | ||
|
|
||
| const effectiveMaxFileSize = policyLimits.policyEnabled | ||
| ? policyLimits.documentMaxFileSize | ||
| : DOCUMENT_MAX_FILE_SIZE; |
There was a problem hiding this comment.
This dialog only applies half of the upload policy.
effectiveMaxFileSize is policy-driven now, but file type filtering is still hardcoded through DOCUMENT_UPLOAD_ACCEPT and isAllowedDocumentUpload. Admin-configured extension/MIME restrictions therefore are not reflected in the picker or pre-upload validation, so users can still attempt uploads that governance will reject on the backend.
Also applies to: 113-150, 195-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/documents/components/document-upload-dialog.tsx`
around lines 60 - 64, The component only applies
policyLimits.documentMaxFileSize (via effectiveMaxFileSize) but still uses
hardcoded DOCUMENT_UPLOAD_ACCEPT and isAllowedDocumentUpload for the file picker
and pre-upload validation; update the file type filtering to use the upload
policy from useUploadPolicy (policyLimits) instead of hardcoded constants so
picker accept attribute and validation reflect admin-configured extensions/MIME
types. Concretely, read allowed file types/extensions and/or mime types from
policyLimits (fallback to DOCUMENT_UPLOAD_ACCEPT / existing lists if policy not
enabled), replace usage of DOCUMENT_UPLOAD_ACCEPT in the file input accept prop,
and update isAllowedDocumentUpload (or its call sites) to consult
policyLimits.allowedExtensions/allowedMimeTypes (with the same fallback)
wherever file-type validation occurs (including the other two blocks mentioned)
so both UI and pre-upload checks mirror backend governance.
Comment on lines
+57
to
+65
| it('renders warning banner when enabled', () => { | ||
| vi.mocked( | ||
| // Re-mock to return enabled policy | ||
| vi.fn(), | ||
| ); | ||
| render(<RetentionPolicyEditor organizationId="org-1" />); | ||
| // Warning should not be visible when policy is not enabled (default state) | ||
| expect(screen.queryByRole('alert')).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
This test never exercises the enabled warning path.
vi.mocked(vi.fn()) does not remock useGovernancePolicy, and the assertion still checks the default disabled state (queryByRole('alert') === null). As written, this passes even if the warning banner never renders when the policy is enabled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/__tests__/retention-policy-editor.test.tsx`
around lines 57 - 65, The test never exercises the enabled path; update the
'renders warning banner when enabled' test to remock the useGovernancePolicy
hook (not vi.fn()) to return a policy object with enabled: true (or the shape
the component expects) before rendering RetentionPolicyEditor, then assert that
an alert role exists (e.g.,
expect(screen.getByRole('alert')).toBeInTheDocument()). Specifically target
useGovernancePolicy via vi.mocked(useGovernancePolicy).mockReturnValue(...) (or
mockResolvedValue if it’s async) and change the final assertion from
queryByRole(...).toBeNull() to checking presence of the alert.
Comment on lines
+32
to
+61
| describe('UploadPolicyEditor', () => { | ||
| describe('accessibility', () => { | ||
| it('passes axe audit', async () => { | ||
| const { container } = render( | ||
| <UploadPolicyEditor organizationId="org-1" />, | ||
| ); | ||
| await checkAccessibility(container); | ||
| }); | ||
| }); | ||
|
|
||
| it('renders the enable toggle', () => { | ||
| render(<UploadPolicyEditor organizationId="org-1" />); | ||
| expect(screen.getByRole('switch')).toBeDefined(); | ||
| }); | ||
|
|
||
| it('renders file extension input', () => { | ||
| render(<UploadPolicyEditor organizationId="org-1" />); | ||
| expect(screen.getByLabelText(/allowed file extensions/i)).toBeDefined(); | ||
| }); | ||
|
|
||
| it('renders max file size input', () => { | ||
| render(<UploadPolicyEditor organizationId="org-1" />); | ||
| expect(screen.getByLabelText(/maximum file size/i)).toBeDefined(); | ||
| }); | ||
|
|
||
| it('renders max volume input', () => { | ||
| render(<UploadPolicyEditor organizationId="org-1" />); | ||
| expect(screen.getByLabelText(/maximum total volume/i)).toBeDefined(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This new editor needs interaction coverage, not just smoke tests.
The suite proves the form renders, but it never verifies that changing values and saving produces the expected mutation payload, nor that invalid input is handled correctly. For a new admin policy editor, that leaves the critical path untested.
As per coding guidelines, "Tests should cover happy paths, edge cases, and error conditions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/__tests__/upload-policy-editor.test.tsx`
around lines 32 - 61, Add interaction tests for UploadPolicyEditor: mock the
save/mutation function (e.g., the update/saveUploadPolicy mutation or API client
used by the component) and assert it is called with the correct payload when the
user toggles the enable switch, edits "allowed file extensions", "maximum file
size" and "maximum total volume", and clicks the save button; also add a
failing-input test that types invalid values (e.g., non-numeric size or empty
extensions), asserts validation error messages appear and that the save/mutation
mock is NOT called. Locate tests around UploadPolicyEditor in
upload-policy-editor.test.tsx and use userEvent to simulate interactions, jest
mocks for the mutation, and screen queries to assert payload and validation
behavior.
Comment on lines
+99
to
+102
| if (doc._creationTime >= args.cutoffMs) continue; | ||
|
|
||
| if (args.scope === 'upload' && doc.sourceProvider !== 'upload') continue; | ||
| if (args.scope === 'agent' && doc.sourceProvider !== 'agent') continue; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '85,105p' services/platform/convex/governance/internal_queries.tsRepository: tale-project/tale
Length of output: 738
🏁 Script executed:
#!/bin/bash
# Read the full function implementation to understand context
sed -n '85,130p' services/platform/convex/governance/internal_queries.tsRepository: tale-project/tale
Length of output: 1164
🏁 Script executed:
#!/bin/bash
# Search for callers of listExpiredDocuments to see if this function is actually used
rg 'listExpiredDocuments' --type tsRepository: tale-project/tale
Length of output: 294
🏁 Script executed:
#!/bin/bash
# Check how listExpiredDocuments is called in retention_cleanup.ts
rg -B 5 -A 5 'listExpiredDocuments' services/platform/convex/governance/retention_cleanup.tsRepository: tale-project/tale
Length of output: 430
🏁 Script executed:
#!/bin/bash
# Check if there are any other callers or if scope is used anywhere in governance module
rg 'listExpiredDocuments|scope.*sourceProvider|sourceProvider.*upload|sourceProvider.*agent' services/platform/convex/governance/ --type ts -B 2 -A 2Repository: tale-project/tale
Length of output: 1484
🏁 Script executed:
#!/bin/bash
# Check tsconfig to see type checking strictness
cat tsconfig.json | head -40Repository: tale-project/tale
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Find tsconfig files in the repo
find . -name 'tsconfig*.json' -type f | head -10Repository: tale-project/tale
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Check what happens when args.scope is undefined - verify it's truly dead code
# Look at the document structure to understand sourceProvider field
rg 'sourceProvider' services/platform/convex/ --type ts -B 1 -A 1 | head -40Repository: tale-project/tale
Length of output: 3571
🏁 Script executed:
#!/bin/bash
# Check tsconfig for strictness settings
cat services/platform/convex/tsconfig.json | grep -A 30 '"compilerOptions"'Repository: tale-project/tale
Length of output: 615
🏁 Script executed:
#!/bin/bash
# See full retention_cleanup.ts context to understand if filtering by scope is needed
cat services/platform/convex/governance/retention_cleanup.tsRepository: tale-project/tale
Length of output: 4951
🏁 Script executed:
#!/bin/bash
# Check the document schema to understand what sourceProvider values are valid
rg 'sourceProvider' services/platform/convex/ --type ts -B 2 -A 2 | grep -A 5 -B 5 "'upload'\|'agent'" | head -50Repository: tale-project/tale
Length of output: 3472
🏁 Script executed:
#!/bin/bash
# Check if scope or similar filtering is used elsewhere in the governance module
rg 'scope.*sourceProvider|sourceProvider.*scope' services/platform/convex/ --type tsRepository: tale-project/tale
Length of output: 330
Add scope to the query args validator or remove the unused filtering.
The handler reads args.scope at lines 99–101, but listExpiredDocuments never declares scope in its args validator. With TypeScript strict mode enabled, this violates the query boundary contract. The caller in retention_cleanup.ts does not pass scope, so both filtering conditions are always false, making this dead code. Either add scope to the validator if filtering by source provider is needed, or remove these two lines if the filtering is not required.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/governance/internal_queries.ts` around lines 99 -
102, The code in listExpiredDocuments reads args.scope (lines referencing
args.scope) but the args validator for listExpiredDocuments doesn't declare
scope and callers (e.g., retention_cleanup.ts) don't pass it, causing a
type/contract mismatch and dead filtering; fix by either adding a scoped
enum/string property to the listExpiredDocuments args validator and update
callers (e.g., retention_cleanup.ts) to pass scope values like 'upload' or
'agent' and then keep the two if checks, or remove the two scope-based if checks
entirely from listExpiredDocuments if you don't intend to filter by
sourceProvider—update the validator accordingly so TypeScript strict mode and
the query contract are consistent with the chosen approach.
Comment on lines
77
to
82
| await ctx.runMutation( | ||
| internal.governance.internal_mutations_retention | ||
| .deleteExpiredDocument, | ||
| { documentId: doc._id }, | ||
| { documentId: doc._id, organizationId }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
One deletion failure can abort the entire cleanup run for the organization.
At Line 77, deleteExpiredDocument is awaited without local error handling. A single mutation throw will stop processing remaining expired docs for that org.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/governance/retention_cleanup.ts` around lines 77 -
82, The current await of
ctx.runMutation(internal.governance.internal_mutations_retention.deleteExpiredDocument,
{ documentId: doc._id, organizationId }) can throw and abort the whole cleanup
loop; wrap that call in a try/catch inside the loop (around the call to
deleteExpiredDocument) so failures for one doc are caught, log or record the
error (include doc._id and organizationId) and then continue processing
remaining docs; optionally aggregate errors to report at the end but do not
rethrow inside the per-document catch so the cleanup run proceeds.
Comment on lines
+93
to
+100
| it('rejects invalid scope value', () => { | ||
| const result = retentionPolicyConfigSchema.safeParse({ | ||
| enabled: true, | ||
| retentionDays: 30, | ||
| scope: 'invalid', | ||
| }); | ||
|
|
||
| expect(result.success).toBe(false); |
There was a problem hiding this comment.
Schema accepts invalid retention scope values (blocking CI).
At Line 100, this assertion fails because retentionPolicyConfigSchema.safeParse is returning success for scope: 'invalid'. That allows unsupported scope values into policy configs.
Suggested fix (in schema definition file)
- scope: z.string().optional(),
+ scope: z.enum(['all', 'upload', 'agent']).optional(),🧰 Tools
🪛 GitHub Check: Test
[failure] 100-100: [server] lib/shared/schemas/tests/governance.test.ts > retentionPolicyConfigSchema > rejects invalid scope value
AssertionError: expected true to be false // Object.is equality
- Expected
- Received
- false
- true
❯ lib/shared/schemas/tests/governance.test.ts:100:28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/lib/shared/schemas/__tests__/governance.test.ts` around
lines 93 - 100, The test fails because retentionPolicyConfigSchema currently
allows arbitrary strings for the scope field; update the schema definition for
retentionPolicyConfigSchema to validate scope as an enum/union of the allowed
scope strings (e.g., the project's supported values such as "global",
"organization", "user" — or whatever the codebase uses) instead of a free string
so that safeParse rejects values like 'invalid'; locate and change the scope
property in the retentionPolicyConfigSchema to use z.enum or z.union of literal
strings (or equivalent validator) so the test expecting result.success to be
false for 'invalid' passes.
| "providers": { | ||
| "title": "Providers", | ||
| "description": "Manage LLM providers and their models", | ||
| "description": "Description", |
There was a problem hiding this comment.
Restore the provider page description.
settings.providers.description now resolves to the literal text "Description", so the providers settings page will render a label instead of an actual subtitle.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` at line 1455, The localization key
settings.providers.description currently contains the literal string
"Description", causing the providers settings page to render a label instead of
the intended subtitle; update settings.providers.description in the messages
file to the correct subtitle text (restore the original descriptive sentence) so
the UI shows the proper subtitle—if unsure, retrieve the prior value from git
history or ask the i18n/UX owner and replace the literal "Description" with that
restored subtitle.
Comment on lines
+3778
to
+3797
| <<<<<<< HEAD | ||
| "tempFileRetentionHours": "Temporary file retention (hours)", | ||
| "saved": "Retention policy saved" | ||
| ======= | ||
| "scopeAll": "All documents", | ||
| "scopeUpload": "Uploaded files only", | ||
| "scopeAgent": "Agent-generated only", | ||
| "batchSize": "Batch size per cleanup run", | ||
| "batchSizeHint": "Number of documents to delete per scheduled run. Higher values process faster but use more resources.", | ||
| "warning": "Enabling this policy will permanently delete documents that exceed the retention period. This action cannot be undone.", | ||
| "presets": { | ||
| "30": "30 days", | ||
| "60": "60 days", | ||
| "90": "90 days", | ||
| "180": "6 months", | ||
| "365": "1 year" | ||
| }, | ||
| "saved": "Retention policy saved", | ||
| "saveFailed": "Failed to save retention policy" | ||
| >>>>>>> 46dbcfec0 (feat(platform): configurable upload and retention policies) |
There was a problem hiding this comment.
Resolve the retained merge conflict before merge.
This file still contains <<<<<<<, =======, and >>>>>>>, and lint is already failing with TS1185 on these lines. It also leaves retentionPolicy.scopeAll/scopeUpload/scopeAgent unresolved even though services/platform/app/features/settings/governance/components/retention-policy-editor.tsx reads those keys on Lines 72-74.
🧰 Tools
🪛 GitHub Actions: Lint
[error] 3778-3778: TypeScript error TS1185: Merge conflict marker encountered.
[error] 3781-3781: TypeScript error TS1185: Merge conflict marker encountered.
[error] 3797-3797: TypeScript error TS1185: Merge conflict marker encountered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 3778 - 3797, Remove the Git
conflict markers (<<<<<<<, =======, >>>>>>>) and merge the two JSON variants
into a single consistent object that includes the needed localization keys
referenced by the UI: keep or reconcile "tempFileRetentionHours" if still used,
and ensure "scopeAll", "scopeUpload", "scopeAgent", "batchSize",
"batchSizeHint", "warning", "presets", "saved", and "saveFailed" are all present
with appropriate string values; validate the resulting JSON is syntactically
correct so the linter error TS1185 goes away and the retention-policy-editor’s
key lookups ("scopeAll"/"scopeUpload"/"scopeAgent") resolve.
…or messages Client-side validation now checks allowed/blocked extensions before uploading, and the file picker filters by policy-allowed extensions. Replaces generic error messages with specific rejection reasons.
- Remove dead retention-policy-editor component (superseded by main's RetentionEditor) - Remove stale scope references in internal_queries.ts - Add missing policyLimits dependency in useCallback
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
uploadedByfield tofileMetadata, enforce policies in both upload paths (saveFileMetadataandcreateDocumentFromUpload), fix per-user volume check, addUploadPolicyEditoradmin UI, integrateuseUploadPolicyhook into chat/document upload for dynamic client-side limit awarenesslistExpiredDocuments, addfileMetadatacleanup and audit logging todeleteExpiredDocument, add per-document error handling, addRetentionPolicyEditoradmin UI with scope/days/batch-size controlsupload_policyandretention_policyinupsertPolicymutationCloses #1363
Closes #1169
Test plan
UploadPolicyEditorrenders and allows configuring allowed/blocked extensions, MIME types, max file size, and volume per userRetentionPolicyEditorrenders with enable toggle, retention days, scope selector, batch size, and warning banneruploadedBy+by_org_userindexfileMetadatarows, and creates audit log entriesuploadedBycontinue working (not counted toward quota)upsertPolicySummary by CodeRabbit