Skip to content

fix(docs): update deployment documentation and conversation ui#7

Merged
Israeltheminer merged 1 commit into
mainfrom
update-ui
Dec 7, 2025
Merged

fix(docs): update deployment documentation and conversation ui#7
Israeltheminer merged 1 commit into
mainfrom
update-ui

Conversation

@Israeltheminer

@Israeltheminer Israeltheminer commented Dec 7, 2025

Copy link
Copy Markdown
Collaborator
  • Change internal network name from poc2_internal to tale_internal
  • Update Docker image references from ghcr.io/your-org/poc2/tale-rag:latest to ghcr.io/your-org/tale/tale-rag:latest
  • Modify GitHub issues link to point to the correct repository
  • Enhance Conversations component to show an empty state when no conversations and no email providers are configured
  • Adjust SignUpForm to conditionally display password requirements

Summary by CodeRabbit

Release Notes

  • Documentation

    • Updated deployment guides with current container image references and Docker network naming conventions.
  • New Features

    • Added empty-state UI for conversations that prompts users to enable email integration.
  • Improvements

    • Password requirements tips in the sign-up form now display only when entering a password, improving clarity.

✏️ Tip: You can customize this high-level summary in your review settings.

…rnal networking and GitHub issues

- Change internal network name from `poc2_internal` to `tale_internal`
- Update Docker image references from `ghcr.io/your-org/poc2/tale-rag:latest` to `ghcr.io/your-org/tale/tale-rag:latest`
- Modify GitHub issues link to point to the correct repository
- Enhance Conversations component to show an empty state when no conversations and no email providers are configured
- Adjust SignUpForm to conditionally display password requirements
@coderabbitai

coderabbitai Bot commented Dec 7, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This pull request introduces updates to support email-based conversations functionality and documentation refactoring. Changes include renaming Docker references and network names from poc2 to tale across deployment documentation; adding a new ActivateConversationsEmptyState React component to prompt users to configure email integrations; modifying the conversations component to fetch email provider data and conditionally render an empty state when no conversations exist and no email provider is configured; and updating the sign-up form to conditionally display password requirements only when a password value is present.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Documentation changes in docs/ are straightforward naming/reference updates (homogeneous refactoring pattern)
  • New activate-conversations-empty-state.tsx is a simple, presentational component with minimal logic
  • conversations.tsx requires careful review of the emailProviders data fetch logic and the conditional rendering path to ensure the early return for the empty state doesn't inadvertently skip other initialization or side effects
  • sign-up-form.tsx conditional rendering change is localized but should be verified to ensure password guidance visibility doesn't negatively impact UX during form interaction
  • Verify the routing link in the new component correctly constructs the integrations settings URL with the email tab parameter

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between a492a00 and c9ed0a9.

📒 Files selected for processing (5)
  • docs/deployment-modes.md (1 hunks)
  • docs/tale-rag-deployment.md (4 hunks)
  • services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx (1 hunks)
  • services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx (4 hunks)
  • services/platform/components/auth/sign-up-form.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*

📄 CodeRabbit inference engine (.cursor/rules/workspace_rules.mdc)

Use English only for ALL user-facing content including UI components, labels, buttons, dialogs, forms, toast messages, error messages, success messages, comments, documentation, README files, variable names, function names, and type names

Files:

  • docs/deployment-modes.md
  • services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx
  • docs/tale-rag-deployment.md
  • services/platform/components/auth/sign-up-form.tsx
  • services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/figma_rules.mdc)

**/*.{tsx,jsx}: Avoid specifying font-['Inter:Regular',_sans-serif] as it should be the default
Only specify font-family when using non-default fonts like font-['Inter:Medium',_sans-serif]
Ensure font-family matches font-weight (Inter:Regular with font-normal, Inter:Medium with font-medium)
Use leading-normal instead of leading-[normal] in Tailwind classes
Use standard font size classes instead of arbitrary values: text-[12px]text-xs, text-[14px]text-sm, text-[16px]text-base, text-[18px]text-lg, text-[20px]text-xl, text-[24px]text-2xl
Use semantic spacing classes: p-[4px]p-1, p-[8px]p-2, m-[4px]m-1, m-[8px]m-2
Convert pixel values to rem using the 16px base for width and height measurements: w-[278px]w-[17.375rem], h-[48px]h-[3rem], min-w-[120px]min-w-[7.5rem], max-w-[400px]max-w-[25rem]
NEVER use hardcoded colors like text-gray-500, bg-gray-100, border-gray-200; ALWAYS use design system semantic colors: text-foreground for primary text, text-muted-foreground for secondary text and icons, bg-background for main backgrounds, bg-muted for subtle backgrounds and hover states, border-border for borders
ALWAYS use the Table component instead of custom flex layouts; use Table, TableHeader, TableBody, TableRow, TableHead, TableCell components with proper column widths using rem units and semantic colors

Files:

  • services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx
  • services/platform/components/auth/sign-up-form.tsx
  • services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/workspace_rules.mdc)

**/*.{ts,tsx,js,jsx}: Use Vercel AI SDK with OpenAI - import from 'ai' and '@ai-sdk/openai', never use raw OpenAI SDK or OpenRouter
Never hallucinate API keys - always use environment variables and existing .env configuration
Use camelCase for function names (e.g., getUserData)
Use SCREAMING_SNAKE_CASE for constants (e.g., API_BASE_URL, MAX_RETRIES)
Use feature flags with enums (TypeScript) or const objects (JavaScript) with UPPERCASE_WITH_UNDERSCORE naming
Implement error handling with try-catch pattern: check for result.error and display descriptive toast messages using result.error as title

Files:

  • services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx
  • services/platform/components/auth/sign-up-form.tsx
  • services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx
**/app/**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/workspace_rules.mdc)

In Next.js App Router, use page.tsx as server components by default; use 'use client' only for interactions and state

Files:

  • services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx
  • services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/workspace_rules.mdc)

**/*.{ts,tsx}: Use kebab-case for file names (e.g., user-profile.tsx)
Use PascalCase for component names (e.g., UserProfile)
Use descriptive messages as toast title (never generic 'Error'), with optional description for additional context only
Follow component structure: 'use client' directive, imports, interface Props, hooks, effects, event handlers, then render
Prioritize data fetching methods in order: Server Actions (preferred), Route Handlers (when needed), Client-side (minimal use)
Use React.memo for expensive components to optimize performance
Use Next.js Image component for all images instead of native img tags
Use dynamic imports for code splitting

Files:

  • services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx
  • services/platform/components/auth/sign-up-form.tsx
  • services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx
🧬 Code graph analysis (1)
services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx (1)
services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx (1)
  • ActivateConversationsEmptyState (9-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build services/platform
🔇 Additional comments (5)
docs/deployment-modes.md (1)

164-164: LGTM! Documentation update aligns with infrastructure rename.

The network name update from poc2_internal to tale_internal is consistent with the broader PR objectives to rename the project infrastructure.

services/platform/components/auth/sign-up-form.tsx (1)

166-184: Good UX improvement! Conditionally showing password requirements reduces visual clutter.

Showing password requirements only when the user starts typing provides a cleaner initial state while still offering guidance when needed. The validation schema (lines 20-29) still enforces all requirements.

docs/tale-rag-deployment.md (1)

174-174: LGTM! Comprehensive documentation updates for the infrastructure rename.

All references have been consistently updated from poc2 to tale:

  • Container image paths updated to ghcr.io/your-org/tale/tale-rag:latest
  • Network name changed to tale_internal
  • Volume name changed to tale_rag-data
  • GitHub repository URL updated to tale/issues

These changes align with the PR's objectives and maintain documentation consistency.

Also applies to: 180-180, 287-287, 310-310, 346-346

services/platform/app/(app)/dashboard/[id]/conversations/components/conversations.tsx (1)

75-79: Good implementation of email providers check.

The email providers query follows the same conditional pattern as the conversations query and correctly handles the loading state with optional chaining (emailProviders?.length ?? 0).

Also applies to: 85-85

services/platform/app/(app)/dashboard/[id]/conversations/components/activate-conversations-empty-state.tsx (1)

1-33: LGTM! Well-structured empty state component following all coding guidelines.

The component correctly:

  • Uses semantic design system colors (text-muted-foreground, text-foreground, ring-border)
  • Uses standard size classes (size-6, size-4, text-lg, text-sm)
  • Uses semantic spacing tokens (mb-5, mb-2, my-6, mx-4, px-4, mr-2)
  • Implements the Button with asChild pattern for proper Next.js Link integration
  • Routes to the integrations page with the ?tab=email query parameter for better UX

@Israeltheminer Israeltheminer merged commit d799d36 into main Dec 7, 2025
2 checks passed
@Israeltheminer Israeltheminer deleted the update-ui branch December 7, 2025 08:54
larryro added a commit that referenced this pull request Dec 30, 2025
…lidate

- repairObject now recursively processes array elements to repair corrupted
  keys inside nested objects within arrays
- validateObject now recursively validates array elements to catch control
  characters in nested object keys
- Added biome-ignore comments for intentional control character regex patterns
- Added camelCase normalization for repaired field names (e.g., userprompt -> userPrompt)

Addresses CodeRabbit review comments #5, #6, and #7.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
larryro added a commit that referenced this pull request Dec 30, 2025
…lidate

- repairObject now recursively processes array elements to repair corrupted
  keys inside nested objects within arrays
- validateObject now recursively validates array elements to catch control
  characters in nested object keys
- Added biome-ignore comments for intentional control character regex patterns
- Added camelCase normalization for repaired field names (e.g., userprompt -> userPrompt)

Addresses CodeRabbit review comments #5, #6, and #7.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
larryro added a commit that referenced this pull request May 7, 2026
`restoreChatThread` and `deleteChatThread` (the user-initiated trash
helper, also called from arena cleanup and the REST API DELETE) ignored
`legalHolds`. The retention runner already gates Pass-B physical
deletion on holds, but the soft layer was open: a user could trash a
held thread (preventing them from producing its content during
discovery) or restore one (re-enabling edits/deletes on a frozen
target). Both break the eDiscovery contract even though physical data
survives.

`restore_chat_thread.ts`
- After auth/permission checks, call `loadActiveHolds(ctx, orgId)` and
  refuse with `LEGAL_HOLD_BLOCKS_RESTORE` when `orgHeld` or the thread
  id is in `holds.threadIds`.
- The deferred `Bundle 3` comment is removed; Bundle 3 has shipped on
  this branch.

`delete_chat_thread.ts`
- Lift the `threadMetadata` fetch above the destructive ops (agent-side
  archive, status flip, webhook-mapping cascade) so we can short-circuit
  on hold before any side effect.
- Throw `LEGAL_HOLD_BLOCKS_DELETE` when held. All callers — public
  `deleteChatThread` mutation, REST `DELETE /threads/:id`, and arena
  cleanup helper — inherit the gate.

Out-of-scope here:
- Splitting internal-cascade vs user-trash modes for `cleanupArenaBranch`
  (W1 #7) — arena cleanup currently raises a hold-error if the arena
  thread is held; a future refactor will hard-delete in that case.
- Audit-log emission on the refusal (W3 follow-ups will route the
  refusal through createAuditLog like erasure does).
larryro added a commit that referenced this pull request May 7, 2026
Two reviewer-confirmed thread-state issues.

deleteChatThread helper (W1 #7)
- New `mode: 'user-trash' | 'internal-cascade'` parameter (default
  'user-trash' preserves existing public-mutation semantics).
- internal-cascade hard-deletes immediately via
  cascadeDeleteThreadChildren and bypasses Trash. Used by
  cleanupArenaBranch — arena Thread B is an ephemeral internal artifact
  the user never saw as a separate entity, so soft-trashing it would
  pollute the user's Trash with rows they cannot reason about.
- Status flip in user-trash mode no longer resets statusChangedAt when
  the row is already 'trashed'/'expired' — defeats the grace-extension
  attack where a user re-trashes to keep a row alive past its window.

archive_chat_thread::unarchiveChatThread (W1 #8)
- Refuses to flip 'trashed'/'expired'/'deleted' rows directly to
  'active' (round-2 backdoor finding). Only 'archived' is reversible
  via this entry point; trashed/expired threads must use
  restoreChatThread, which enforces auth + legal-hold checks + audit.
- The metadata row check fires BEFORE the agent-component flip, so a
  guard failure leaves the agent-side state untouched.
larryro added a commit that referenced this pull request May 7, 2026
Round-2 W8 batch (six independent UI / docs items, all small).

i18n keys (W8 #1)
- Add governance.retentionPolicy.group.deletionBehavior.{title,description}
  to en/de/fr — previously missing in all three locales, retention editor
  fell back to the inline English string in every locale.
- Add governance.retentionPolicy.chatFilterEvents.{title,description,
  placeholder,helper} to en/de/fr — previously the row title rendered
  the literal id "chatFilterEvents".

useTranslation namespace (W8 #2)
- use-data-classification-notice.ts now binds useTranslation('dataNotice')
  so the dataNotice.default fallback string is reachable; the prior
  default-namespace bind meant the DE/FR fallbacks were unreachable.

UI delete copy (W8 #7)
- en/de/fr.json deletePermanentMessage now matches the actual
  user-visible behavior (Trash + grace + retention) instead of saying
  "permanently deleted" while the code soft-trashes.

Stale test (W8 #10)
- delete_chat_thread.test.ts was asserting the legacy
  `{ status: 'deleted' }` patch shape; the helper now writes
  `{ status: 'trashed', statusChangedAt: <ms> }` in user-trash mode.

Docs cron + maker-checker (W8 #8)
- docs/{en,de,fr}/self-hosted/configuration/retention.md: cron 03:00
  UTC → 04:00 UTC (matches crons.ts), plus a sibling note about the
  new 01:00 UTC effectReleasesOnly cron.
- en docs: replace the non-existent `releaseLegalHold` reference with
  the correct dual-control flow names plus the new
  RELEASE_APPROVAL_MIN_DELAY_MS, ORG_HOLD_REQUIRES_DUAL_CONTROL, and
  closeLegalMatter fan-out behavior.
- retention_cleanup.ts stale "03:00 UTC" comment fixed.
larryro added a commit that referenced this pull request May 8, 2026
`restoreChatThread` and `deleteChatThread` (the user-initiated trash
helper, also called from arena cleanup and the REST API DELETE) ignored
`legalHolds`. The retention runner already gates Pass-B physical
deletion on holds, but the soft layer was open: a user could trash a
held thread (preventing them from producing its content during
discovery) or restore one (re-enabling edits/deletes on a frozen
target). Both break the eDiscovery contract even though physical data
survives.

`restore_chat_thread.ts`
- After auth/permission checks, call `loadActiveHolds(ctx, orgId)` and
  refuse with `LEGAL_HOLD_BLOCKS_RESTORE` when `orgHeld` or the thread
  id is in `holds.threadIds`.
- The deferred `Bundle 3` comment is removed; Bundle 3 has shipped on
  this branch.

`delete_chat_thread.ts`
- Lift the `threadMetadata` fetch above the destructive ops (agent-side
  archive, status flip, webhook-mapping cascade) so we can short-circuit
  on hold before any side effect.
- Throw `LEGAL_HOLD_BLOCKS_DELETE` when held. All callers — public
  `deleteChatThread` mutation, REST `DELETE /threads/:id`, and arena
  cleanup helper — inherit the gate.

Out-of-scope here:
- Splitting internal-cascade vs user-trash modes for `cleanupArenaBranch`
  (W1 #7) — arena cleanup currently raises a hold-error if the arena
  thread is held; a future refactor will hard-delete in that case.
- Audit-log emission on the refusal (W3 follow-ups will route the
  refusal through createAuditLog like erasure does).
larryro added a commit that referenced this pull request May 8, 2026
Two reviewer-confirmed thread-state issues.

deleteChatThread helper (W1 #7)
- New `mode: 'user-trash' | 'internal-cascade'` parameter (default
  'user-trash' preserves existing public-mutation semantics).
- internal-cascade hard-deletes immediately via
  cascadeDeleteThreadChildren and bypasses Trash. Used by
  cleanupArenaBranch — arena Thread B is an ephemeral internal artifact
  the user never saw as a separate entity, so soft-trashing it would
  pollute the user's Trash with rows they cannot reason about.
- Status flip in user-trash mode no longer resets statusChangedAt when
  the row is already 'trashed'/'expired' — defeats the grace-extension
  attack where a user re-trashes to keep a row alive past its window.

archive_chat_thread::unarchiveChatThread (W1 #8)
- Refuses to flip 'trashed'/'expired'/'deleted' rows directly to
  'active' (round-2 backdoor finding). Only 'archived' is reversible
  via this entry point; trashed/expired threads must use
  restoreChatThread, which enforces auth + legal-hold checks + audit.
- The metadata row check fires BEFORE the agent-component flip, so a
  guard failure leaves the agent-side state untouched.
larryro added a commit that referenced this pull request May 8, 2026
Round-2 W8 batch (six independent UI / docs items, all small).

i18n keys (W8 #1)
- Add governance.retentionPolicy.group.deletionBehavior.{title,description}
  to en/de/fr — previously missing in all three locales, retention editor
  fell back to the inline English string in every locale.
- Add governance.retentionPolicy.chatFilterEvents.{title,description,
  placeholder,helper} to en/de/fr — previously the row title rendered
  the literal id "chatFilterEvents".

useTranslation namespace (W8 #2)
- use-data-classification-notice.ts now binds useTranslation('dataNotice')
  so the dataNotice.default fallback string is reachable; the prior
  default-namespace bind meant the DE/FR fallbacks were unreachable.

UI delete copy (W8 #7)
- en/de/fr.json deletePermanentMessage now matches the actual
  user-visible behavior (Trash + grace + retention) instead of saying
  "permanently deleted" while the code soft-trashes.

Stale test (W8 #10)
- delete_chat_thread.test.ts was asserting the legacy
  `{ status: 'deleted' }` patch shape; the helper now writes
  `{ status: 'trashed', statusChangedAt: <ms> }` in user-trash mode.

Docs cron + maker-checker (W8 #8)
- docs/{en,de,fr}/self-hosted/configuration/retention.md: cron 03:00
  UTC → 04:00 UTC (matches crons.ts), plus a sibling note about the
  new 01:00 UTC effectReleasesOnly cron.
- en docs: replace the non-existent `releaseLegalHold` reference with
  the correct dual-control flow names plus the new
  RELEASE_APPROVAL_MIN_DELAY_MS, ORG_HOLD_REQUIRES_DUAL_CONTROL, and
  closeLegalMatter fan-out behavior.
- retention_cleanup.ts stale "03:00 UTC" comment fixed.
larryro added a commit that referenced this pull request May 8, 2026
`restoreChatThread` and `deleteChatThread` (the user-initiated trash
helper, also called from arena cleanup and the REST API DELETE) ignored
`legalHolds`. The retention runner already gates Pass-B physical
deletion on holds, but the soft layer was open: a user could trash a
held thread (preventing them from producing its content during
discovery) or restore one (re-enabling edits/deletes on a frozen
target). Both break the eDiscovery contract even though physical data
survives.

`restore_chat_thread.ts`
- After auth/permission checks, call `loadActiveHolds(ctx, orgId)` and
  refuse with `LEGAL_HOLD_BLOCKS_RESTORE` when `orgHeld` or the thread
  id is in `holds.threadIds`.
- The deferred `Bundle 3` comment is removed; Bundle 3 has shipped on
  this branch.

`delete_chat_thread.ts`
- Lift the `threadMetadata` fetch above the destructive ops (agent-side
  archive, status flip, webhook-mapping cascade) so we can short-circuit
  on hold before any side effect.
- Throw `LEGAL_HOLD_BLOCKS_DELETE` when held. All callers — public
  `deleteChatThread` mutation, REST `DELETE /threads/:id`, and arena
  cleanup helper — inherit the gate.

Out-of-scope here:
- Splitting internal-cascade vs user-trash modes for `cleanupArenaBranch`
  (W1 #7) — arena cleanup currently raises a hold-error if the arena
  thread is held; a future refactor will hard-delete in that case.
- Audit-log emission on the refusal (W3 follow-ups will route the
  refusal through createAuditLog like erasure does).
larryro added a commit that referenced this pull request May 8, 2026
Two reviewer-confirmed thread-state issues.

deleteChatThread helper (W1 #7)
- New `mode: 'user-trash' | 'internal-cascade'` parameter (default
  'user-trash' preserves existing public-mutation semantics).
- internal-cascade hard-deletes immediately via
  cascadeDeleteThreadChildren and bypasses Trash. Used by
  cleanupArenaBranch — arena Thread B is an ephemeral internal artifact
  the user never saw as a separate entity, so soft-trashing it would
  pollute the user's Trash with rows they cannot reason about.
- Status flip in user-trash mode no longer resets statusChangedAt when
  the row is already 'trashed'/'expired' — defeats the grace-extension
  attack where a user re-trashes to keep a row alive past its window.

archive_chat_thread::unarchiveChatThread (W1 #8)
- Refuses to flip 'trashed'/'expired'/'deleted' rows directly to
  'active' (round-2 backdoor finding). Only 'archived' is reversible
  via this entry point; trashed/expired threads must use
  restoreChatThread, which enforces auth + legal-hold checks + audit.
- The metadata row check fires BEFORE the agent-component flip, so a
  guard failure leaves the agent-side state untouched.
larryro added a commit that referenced this pull request May 8, 2026
Round-2 W8 batch (six independent UI / docs items, all small).

i18n keys (W8 #1)
- Add governance.retentionPolicy.group.deletionBehavior.{title,description}
  to en/de/fr — previously missing in all three locales, retention editor
  fell back to the inline English string in every locale.
- Add governance.retentionPolicy.chatFilterEvents.{title,description,
  placeholder,helper} to en/de/fr — previously the row title rendered
  the literal id "chatFilterEvents".

useTranslation namespace (W8 #2)
- use-data-classification-notice.ts now binds useTranslation('dataNotice')
  so the dataNotice.default fallback string is reachable; the prior
  default-namespace bind meant the DE/FR fallbacks were unreachable.

UI delete copy (W8 #7)
- en/de/fr.json deletePermanentMessage now matches the actual
  user-visible behavior (Trash + grace + retention) instead of saying
  "permanently deleted" while the code soft-trashes.

Stale test (W8 #10)
- delete_chat_thread.test.ts was asserting the legacy
  `{ status: 'deleted' }` patch shape; the helper now writes
  `{ status: 'trashed', statusChangedAt: <ms> }` in user-trash mode.

Docs cron + maker-checker (W8 #8)
- docs/{en,de,fr}/self-hosted/configuration/retention.md: cron 03:00
  UTC → 04:00 UTC (matches crons.ts), plus a sibling note about the
  new 01:00 UTC effectReleasesOnly cron.
- en docs: replace the non-existent `releaseLegalHold` reference with
  the correct dual-control flow names plus the new
  RELEASE_APPROVAL_MIN_DELAY_MS, ORG_HOLD_REQUIRES_DUAL_CONTROL, and
  closeLegalMatter fan-out behavior.
- retention_cleanup.ts stale "03:00 UTC" comment fixed.
larryro added a commit that referenced this pull request May 9, 2026
Addresses every CRITICAL Phase-B finding from the multi-agent review of
this branch.

- workflow rag_action: get_chunks/search now resolve _variables.organizationId
  and call verifyStorageIdsBelongToOrg before forwarding fileIds to the RAG
  service, mirroring document_action's `compare` branch. The RAG service has
  no per-org namespace; without this gate a workflow in OrgA could fetch any
  org's chunks by guessing or upstreaming a fileId.

- documents/rest_api retry-indexing: pre-fetch via getDocumentByIdRaw with
  callerOrgId so the action only runs for the caller's own org.

- threads/mutations updateBranchSelections: now calls assertThreadAccess.
  Previous gap let any authenticated user overwrite branchSelections on any
  thread by guessing the threadId.

- threads/internal_mutations createChatThreadInternal + REST createThread:
  thread the organizationId arg through so the new threadMetadata row carries
  org context. Without this, POST /api/v1/threads created an org-less row
  that GET /api/v1/threads/:id immediately 404'd because callerOrgId !== row.org.

- threads/internal_queries listThreadsInternal + listArchivedThreadsInternal +
  REST listThreads: filter by organizationId so multi-org users don't get
  threads from every org they belong to leaking into a single API response.

- threads/get_thread_ancestor_chain: per-hop org check via getThreadMetadata
  with callerOrgId; chain truncates at any cross-org parent so a tampered
  summary.parentThreadId can't leak another tenant's threadId into
  accessibleThreadIds. rag_search_tool callers updated to pass orgId.

- governance/legal_hold requestLegalHoldRelease / approveLegalHoldRelease /
  rejectLegalHoldRelease: now require explicit organizationId arg, run
  membership check BEFORE db.get(holdId|requestId), and verify the resolved
  row's organizationId matches. Previously, a guessed holdId leaked
  reason/targetType/targetId via the resulting error, and let id-enumeration
  distinguish "exists but you can't see" from "doesn't exist". The frontend
  dialogs (request/approve/reject) now pass organizationId via
  useOrganizationId().

- governance/legal_hold_queries shapeReleaseRequests: defense-in-depth — skip
  enrichment when a fetched hold's organizationId doesn't match the request
  rows' org. Today no write path crosses orgs, but the invariant should be
  enforced explicitly.

- governance/legal_hold requestLegalHoldRelease: extend the existing-request
  filter to include 'approved' (mirroring closeLegalMatter:1102-1110), so a
  second pending request can't be filed while an approved-cooldown-pending
  one is outstanding.

- documents/internal_mutations updateDocument: throw ConvexError({code:
  'not_found'}) on cross-tenant write instead of silently returning. REST
  PATCH used to 204 on cross-org writes, masking the rejection.

- lib/rest/helpers withRestAuth: map ConvexError codes (unauthenticated,
  forbidden, not_found, validation) to proper HTTP statuses (401/403/404/400)
  so typed errors thrown by mutations surface as actionable 4xx rather than
  opaque 500s with the message in the body.

CRITICAL #7 (can_access_thread.ts:60-61) marked SKIP in plan after
verification: the legacy fallback lives inside the owner branch, so a
no-org thread is only readable by its owner — no IDOR. False-positive
finding from round-1.

Lint + typecheck clean.
larryro added a commit that referenced this pull request May 9, 2026
`restoreChatThread` and `deleteChatThread` (the user-initiated trash
helper, also called from arena cleanup and the REST API DELETE) ignored
`legalHolds`. The retention runner already gates Pass-B physical
deletion on holds, but the soft layer was open: a user could trash a
held thread (preventing them from producing its content during
discovery) or restore one (re-enabling edits/deletes on a frozen
target). Both break the eDiscovery contract even though physical data
survives.

`restore_chat_thread.ts`
- After auth/permission checks, call `loadActiveHolds(ctx, orgId)` and
  refuse with `LEGAL_HOLD_BLOCKS_RESTORE` when `orgHeld` or the thread
  id is in `holds.threadIds`.
- The deferred `Bundle 3` comment is removed; Bundle 3 has shipped on
  this branch.

`delete_chat_thread.ts`
- Lift the `threadMetadata` fetch above the destructive ops (agent-side
  archive, status flip, webhook-mapping cascade) so we can short-circuit
  on hold before any side effect.
- Throw `LEGAL_HOLD_BLOCKS_DELETE` when held. All callers — public
  `deleteChatThread` mutation, REST `DELETE /threads/:id`, and arena
  cleanup helper — inherit the gate.

Out-of-scope here:
- Splitting internal-cascade vs user-trash modes for `cleanupArenaBranch`
  (W1 #7) — arena cleanup currently raises a hold-error if the arena
  thread is held; a future refactor will hard-delete in that case.
- Audit-log emission on the refusal (W3 follow-ups will route the
  refusal through createAuditLog like erasure does).
larryro added a commit that referenced this pull request May 9, 2026
Two reviewer-confirmed thread-state issues.

deleteChatThread helper (W1 #7)
- New `mode: 'user-trash' | 'internal-cascade'` parameter (default
  'user-trash' preserves existing public-mutation semantics).
- internal-cascade hard-deletes immediately via
  cascadeDeleteThreadChildren and bypasses Trash. Used by
  cleanupArenaBranch — arena Thread B is an ephemeral internal artifact
  the user never saw as a separate entity, so soft-trashing it would
  pollute the user's Trash with rows they cannot reason about.
- Status flip in user-trash mode no longer resets statusChangedAt when
  the row is already 'trashed'/'expired' — defeats the grace-extension
  attack where a user re-trashes to keep a row alive past its window.

archive_chat_thread::unarchiveChatThread (W1 #8)
- Refuses to flip 'trashed'/'expired'/'deleted' rows directly to
  'active' (round-2 backdoor finding). Only 'archived' is reversible
  via this entry point; trashed/expired threads must use
  restoreChatThread, which enforces auth + legal-hold checks + audit.
- The metadata row check fires BEFORE the agent-component flip, so a
  guard failure leaves the agent-side state untouched.
larryro added a commit that referenced this pull request May 9, 2026
Round-2 W8 batch (six independent UI / docs items, all small).

i18n keys (W8 #1)
- Add governance.retentionPolicy.group.deletionBehavior.{title,description}
  to en/de/fr — previously missing in all three locales, retention editor
  fell back to the inline English string in every locale.
- Add governance.retentionPolicy.chatFilterEvents.{title,description,
  placeholder,helper} to en/de/fr — previously the row title rendered
  the literal id "chatFilterEvents".

useTranslation namespace (W8 #2)
- use-data-classification-notice.ts now binds useTranslation('dataNotice')
  so the dataNotice.default fallback string is reachable; the prior
  default-namespace bind meant the DE/FR fallbacks were unreachable.

UI delete copy (W8 #7)
- en/de/fr.json deletePermanentMessage now matches the actual
  user-visible behavior (Trash + grace + retention) instead of saying
  "permanently deleted" while the code soft-trashes.

Stale test (W8 #10)
- delete_chat_thread.test.ts was asserting the legacy
  `{ status: 'deleted' }` patch shape; the helper now writes
  `{ status: 'trashed', statusChangedAt: <ms> }` in user-trash mode.

Docs cron + maker-checker (W8 #8)
- docs/{en,de,fr}/self-hosted/configuration/retention.md: cron 03:00
  UTC → 04:00 UTC (matches crons.ts), plus a sibling note about the
  new 01:00 UTC effectReleasesOnly cron.
- en docs: replace the non-existent `releaseLegalHold` reference with
  the correct dual-control flow names plus the new
  RELEASE_APPROVAL_MIN_DELAY_MS, ORG_HOLD_REQUIRES_DUAL_CONTROL, and
  closeLegalMatter fan-out behavior.
- retention_cleanup.ts stale "03:00 UTC" comment fixed.
larryro added a commit that referenced this pull request May 9, 2026
Addresses every CRITICAL Phase-B finding from the multi-agent review of
this branch.

- workflow rag_action: get_chunks/search now resolve _variables.organizationId
  and call verifyStorageIdsBelongToOrg before forwarding fileIds to the RAG
  service, mirroring document_action's `compare` branch. The RAG service has
  no per-org namespace; without this gate a workflow in OrgA could fetch any
  org's chunks by guessing or upstreaming a fileId.

- documents/rest_api retry-indexing: pre-fetch via getDocumentByIdRaw with
  callerOrgId so the action only runs for the caller's own org.

- threads/mutations updateBranchSelections: now calls assertThreadAccess.
  Previous gap let any authenticated user overwrite branchSelections on any
  thread by guessing the threadId.

- threads/internal_mutations createChatThreadInternal + REST createThread:
  thread the organizationId arg through so the new threadMetadata row carries
  org context. Without this, POST /api/v1/threads created an org-less row
  that GET /api/v1/threads/:id immediately 404'd because callerOrgId !== row.org.

- threads/internal_queries listThreadsInternal + listArchivedThreadsInternal +
  REST listThreads: filter by organizationId so multi-org users don't get
  threads from every org they belong to leaking into a single API response.

- threads/get_thread_ancestor_chain: per-hop org check via getThreadMetadata
  with callerOrgId; chain truncates at any cross-org parent so a tampered
  summary.parentThreadId can't leak another tenant's threadId into
  accessibleThreadIds. rag_search_tool callers updated to pass orgId.

- governance/legal_hold requestLegalHoldRelease / approveLegalHoldRelease /
  rejectLegalHoldRelease: now require explicit organizationId arg, run
  membership check BEFORE db.get(holdId|requestId), and verify the resolved
  row's organizationId matches. Previously, a guessed holdId leaked
  reason/targetType/targetId via the resulting error, and let id-enumeration
  distinguish "exists but you can't see" from "doesn't exist". The frontend
  dialogs (request/approve/reject) now pass organizationId via
  useOrganizationId().

- governance/legal_hold_queries shapeReleaseRequests: defense-in-depth — skip
  enrichment when a fetched hold's organizationId doesn't match the request
  rows' org. Today no write path crosses orgs, but the invariant should be
  enforced explicitly.

- governance/legal_hold requestLegalHoldRelease: extend the existing-request
  filter to include 'approved' (mirroring closeLegalMatter:1102-1110), so a
  second pending request can't be filed while an approved-cooldown-pending
  one is outstanding.

- documents/internal_mutations updateDocument: throw ConvexError({code:
  'not_found'}) on cross-tenant write instead of silently returning. REST
  PATCH used to 204 on cross-org writes, masking the rejection.

- lib/rest/helpers withRestAuth: map ConvexError codes (unauthenticated,
  forbidden, not_found, validation) to proper HTTP statuses (401/403/404/400)
  so typed errors thrown by mutations surface as actionable 4xx rather than
  opaque 500s with the message in the body.

CRITICAL #7 (can_access_thread.ts:60-61) marked SKIP in plan after
verification: the legacy fallback lives inside the owner branch, so a
no-org thread is only readable by its owner — no IDOR. False-positive
finding from round-1.

Lint + typecheck clean.
larryro added a commit that referenced this pull request May 17, 2026
Closes #5, #6, #7, #8, #40 — backend billing / ledger correctness.

- `checkRuleAgainstUsage` and `collectWarnings` gain a symmetric
  `prospectiveRequests` parameter. The post-ledger TTS call site
  (`reserveChunk` → `checkBudget`) passes `1` so an admin who set
  `maxRequests` for the period sees parallel chunks of a single
  message honour the cap the same way `maxCostCents` already did.
  LLM call sites default to 0 — their ledger write is synchronous
  so retrospective checks stay accurate.
- `reserveChunk` overwrite branch: `agentSlug` now falls back to
  `existing.agentSlug` when the thread temporarily reports no agent
  on a retry (agent detached between attempts). Without the fallback,
  ledger writes for the retry landed under the TTS_SLUG sentinel and
  Top Agents analytics drifted across retries.
- `markChunkReadyAndRecordUsage` moves the `!row.userId` check ABOVE
  the `status: 'ready'` patch. A pending row missing userId now flips
  to `'failed'` with a `PROVIDER_ERROR` code instead of silently
  becoming playable while skipping the ledger write. The
  `synthesize.ts` compensating block handles the resulting throw,
  deleting the just-uploaded blob and reporting the failure to the
  client.
- `markChunkFailed` now schedules `maybeCleanupChunks` on `index === 0`
  too, matching the success path. A message whose chunk 0 always fails
  used to leave the daily cron as the only backstop; the `cleanup:tts`
  limiter still gates the schedule so a burst of failures can't flood
  the dispatcher.
- `recordTtsUsageInline` adds an in-memory `provider` filter on the
  upsert lookup. A TTS row with `provider: 'openai'` no longer merges
  into a sibling LLM row that happens to share (org, user, period,
  team, agent, model) under a different provider. Latent today on
  single-TTS-provider configs; load-bearing once a second TTS
  provider ships. A structural fix (extend the index to include
  `provider`) is tracked as a follow-up.
larryro added a commit that referenced this pull request May 17, 2026
Closes #5, #6, #7, #8, #40 — backend billing / ledger correctness.

- `checkRuleAgainstUsage` and `collectWarnings` gain a symmetric
  `prospectiveRequests` parameter. The post-ledger TTS call site
  (`reserveChunk` → `checkBudget`) passes `1` so an admin who set
  `maxRequests` for the period sees parallel chunks of a single
  message honour the cap the same way `maxCostCents` already did.
  LLM call sites default to 0 — their ledger write is synchronous
  so retrospective checks stay accurate.
- `reserveChunk` overwrite branch: `agentSlug` now falls back to
  `existing.agentSlug` when the thread temporarily reports no agent
  on a retry (agent detached between attempts). Without the fallback,
  ledger writes for the retry landed under the TTS_SLUG sentinel and
  Top Agents analytics drifted across retries.
- `markChunkReadyAndRecordUsage` moves the `!row.userId` check ABOVE
  the `status: 'ready'` patch. A pending row missing userId now flips
  to `'failed'` with a `PROVIDER_ERROR` code instead of silently
  becoming playable while skipping the ledger write. The
  `synthesize.ts` compensating block handles the resulting throw,
  deleting the just-uploaded blob and reporting the failure to the
  client.
- `markChunkFailed` now schedules `maybeCleanupChunks` on `index === 0`
  too, matching the success path. A message whose chunk 0 always fails
  used to leave the daily cron as the only backstop; the `cleanup:tts`
  limiter still gates the schedule so a burst of failures can't flood
  the dispatcher.
- `recordTtsUsageInline` adds an in-memory `provider` filter on the
  upsert lookup. A TTS row with `provider: 'openai'` no longer merges
  into a sibling LLM row that happens to share (org, user, period,
  team, agent, model) under a different provider. Latent today on
  single-TTS-provider configs; load-bearing once a second TTS
  provider ships. A structural fix (extend the index to include
  `provider`) is tracked as a follow-up.
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