Skip to content

Persist encrypted Astra conversations with bounded retention - #800

Merged
thomasluizon merged 7 commits into
redesign/mainfrom
thomasluizon/ticket-201-persist-conversations
Oct 20, 2026
Merged

thomasluizon merged 7 commits into
redesign/mainfrom
thomasluizon/ticket-201-persist-conversations

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Closes thomasluizon/orbit-tickets#201 (ORB-195).

Astra tool failures previously lost the prompt, arguments and original domain error. This adds a recent diagnostic conversation corpus without awaiting its database writes on the chat response path. Collection defaults to off pending the privacy disclosure owned by thomasluizon/orbit-tickets#104.

Change

  • ConversationTurn, OrbitDbContext, and the generated PersistConversationTurns migration store user and assistant rows sharing a turn identifier. Content and tool-call JSON use the existing encryption converters. Indexes cover user/time, retention time, and turn/role; no content index is added.
  • ProcessUserChatCommand and AstraBlockAnswerService capture ordered tool arguments and outcomes before response sanitization. Static block previews are captured and replayed answers are skipped. Full tool payloads are not copied.
  • IConversationTurnRecorder is implemented by a 64-slot channel and a background consumer with fresh scopes and contexts per batch. Full queues, oversized captures, missing encryption and storage failures drop diagnostic records with warnings. Shutdown drains within five seconds. The existing awaited mutation saves remain in place. Conversation content never becomes a Hangfire job argument.
  • ConversationRetentionJob runs hourly through the existing Hangfire registrar in both scheduler modes. Export adds an optional conversations section filtered by requesting user and retention cutoff. Account reset deletes rows and records a cutoff under the existing user lock so queued content cannot reappear; account deletion also has a cascading foreign key.
  • infra/astra-conversation-retention.md contains the disclosure handoff, capacity scenario, operational gate, kill switch and rollback behavior. The window is 30 days; expired rows are purged hourly and exports exclude expired rows immediately. The planning scenario is 100 daily active users at 10 interactions/day, yielding 60,000 rows and about 234 MiB at 4 KiB/row before indexes and PostgreSQL overhead. These are assumptions, not measured traffic.

The model, encryption, export, deletion and retention design also applies to main. Chat and static-block capture use the actual redesign/main paths required by this work order. No token telemetry or client behavior changes are included. The build regenerated both OpenAPI documents with no diff.

Retention, sizing and verbatim disclosure are also recorded in the ticket handoff comment.

Test evidence

  • Before implementation, dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~Handle_ProductionCreateHabitFailure_DoesNotExposeToolSchemaArguments' --no-restore --verbosity quiet passed the unchanged test, 1/1, despite no conversation store.
  • After adding the recorder port and strengthening that same test to require the failed tool name, arguments and original domain error, the same command failed, 0/1, exit 1, while recording was still unimplemented. With the production capture implemented it passes; the final focused Application run passed 60/60.
  • Focused Domain tests passed 7/7. Focused Infrastructure tests passed 24/24, including real encrypted writes and raw ciphertext reads using the repository's SQLite fixture, retention with seeded expired/recent rows, user isolation and reset, deletion with queued writes, full-channel rejection, fresh contexts per batch, failure recovery, bounded shutdown, and complete started/delta/final SSE output despite a recorder exception.
  • The migration was generated with dotnet ef migrations add PersistConversationTurns --project . --startup-project ../Orbit.Api --context OrbitDbContext from src/Orbit.Infrastructure, using the design-time factory. Unit tests generate PostgreSQL Up/Down SQL and check model/snapshot alignment. PostgreSQL execution and a live migration round trip were not performed because the repository forbids a real-database harness. No production latency benchmark was run; the tests establish that recording does not await storage.
  • dotnet build Orbit.slnx --verbosity quiet: exit 0, zero errors; OpenAPI regenerated without changes.
  • dotnet test tests/Orbit.Domain.Tests --no-build --verbosity quiet: exit 0, 804/804.
  • dotnet test tests/Orbit.Application.Tests --no-build --verbosity quiet: exit 0, 5,447/5,447.
  • The first full Infrastructure run exposed an older schedule-migration test that assumed its model was still current. Its assertion now permits exactly the six conversation schema/seed operations; the focused migration run passed 10/10. dotnet test tests/Orbit.Infrastructure.Tests --no-build --verbosity quiet then passed, exit 0, 3,948/3,948. Total full-suite result: 10,199 passing tests.
  • Unchanged tests passed with the defect present:
    dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~ResetAccountRetryTests|FullyQualifiedName~AccountResetRepositoryTests' --no-restore
    Exit 0, 6 passed.
  • New regression failed before the fix:
    dotnet test tests/Orbit.Infrastructure.Tests --no-restore --filter 'FullyQualifiedName~Handle_ResetWithUserConcurrencyToken_CommitsDeletionsAndCutoffWithoutRetry'
    Exit 1, DbUpdateConcurrencyException after retry exhaustion.
  • Committed regression and recorder tests passed:
    dotnet test tests/Orbit.Infrastructure.Tests --no-build --no-restore --filter 'FullyQualifiedName~Handle_ResetWithUserConcurrencyToken_CommitsDeletionsAndCutoffWithoutRetry|FullyQualifiedName~Consumer_ResetDeletesRowsAndRejectsPreviouslyQueuedTurns'
    Exit 0, 2 passed.
  • dotnet build Orbit.slnx --no-restore: exit 0, zero errors. Regenerated OpenAPI was unchanged.
  • dotnet test tests/Orbit.Domain.Tests --no-build --no-restore: exit 0, 804 passed.
  • dotnet test tests/Orbit.Application.Tests --no-build --no-restore: exit 0, 5,447 passed.
  • dotnet test tests/Orbit.Infrastructure.Tests --no-build --no-restore: exit 0, 3,949 passed.

All pre-commit checks passed.

All final commands exited 0:

  • dotnet build Orbit.slnx: zero errors.
  • dotnet test tests/Orbit.Domain.Tests --no-restore: 810 passed.
  • dotnet test tests/Orbit.Application.Tests --no-restore: 5,760 passed.
  • dotnet test tests/Orbit.Infrastructure.Tests --no-restore: 4,095 passed.

Focused checks passed: 307 handler tests and 26 conversation/migration tests, including HasPendingModelChanges() false.

Regression evidence:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~CrisisDisclosure_BypassesModerationRefusal|FullyQualifiedName~Moderation_RefusesBeforeCallingModel' --no-restore

Before strengthening, all 10 cases passed with the merge defect present. Adding recording assertions made four crisis cases fail because TryRecord was never called. After fixing the early crisis path, they passed within:

dotnet test tests/Orbit.Application.Tests --no-build --filter FullyQualifiedName~ProcessUserChatCommandHandlerTests
  • Merged head 1a4a048 (api redesign/main with orbit-api#812): build, Domain, Application and Infrastructure tests exit 0 each; OpenAPI drift 0 lines.

Live PostgreSQL deployment was not exercised.

Interface evidence

No new external response fields are read. Tool capture consumes the repository-owned AiToolCall, AiToolCallResult and AgentOperationResult producers. The recorded model comes from AI:Model, verified against AiCompletionClient's primary ChatClient construction. Channel saturation, shutdown and EF conversion behavior are exercised against the installed runtime and provider rather than invented response fixtures. The database flag mechanism and 30-second cache TTL are confirmed in FeatureFlagService, with the disabled seed registered in OrbitDbContext.

Assumptions

  • Use the repository's AppFeatureFlags and account allowlists, as the final work-order decision requires, rather than introducing a parallel AppConfigs feature flag.
  • Store two role rows per interaction with one shared turn identifier rather than storing a combined message blob; static or cached replies have a null model.
  • Keep a 30-day window and hourly deletion rather than an unbounded corpus; no archive is retained for export.
  • Bound the queue to 64 captures and each capture to 65,536 combined text characters, dropping oversized diagnostic captures rather than permitting unbounded memory use.
  • Use the established SQLite unit fixture and generated PostgreSQL SQL rather than introducing the real-Postgres integration harness prohibited by the repository contract.
  • Modeled PostgreSQL’s xmin concurrency behavior using a SQLite token and update trigger instead of a live PostgreSQL harness.
  • Followed the later handoff’s AppFeatureFlags and existing queue instructions over the earlier AppConfigs wording.
  • Updated DailyAiDollarBudgetMigrationTests to assert the six conversation additions rather than retain its obsolete newest-migration assumption.

Manual steps

  • Before collection is enabled, ORB-98 must publish the disclosure from infra/astra-conversation-retention.md in orbit-ui-mobile under privacy.* in packages/shared/src/i18n/en.json and pt-BR.json on both web and mobile. Verify the published policy in each language and surface.
  • After ORB-98 is published, turn astra_conversation_recording on through the admin Orbit MCP tool manage_feature_flags (the /flag skill), listing the flag before and after, and record the activation time on the owning ticket. The flag cache TTL is 30 seconds. Then read newly written ConversationTurns.Content and ToolCallsJson rows on the Render Postgres database (read-only query) and confirm they are ciphertext.
  • For the first month, compare the ConversationTurns row count and pg_total_relation_size on the Render Postgres database weekly with the capacity scenario (read-only queries). Shorten the retention window if growth exceeds the prediction.
  • To stop collection, turn the flag off and remove every account target for astra_conversation_recording through manage_feature_flags, then list it again to confirm both. Allowlisted accounts otherwise stay enabled. Collection stays off in this delivery.

Rollback intentionally drops all diagnostic conversation data through the migration's Down operation. It is not a lossless revert.

None added by this fix.

  • Staging: release the staging API from redesign/main (Release API, environment=staging, branch=redesign/main); it applies 20261020014306_PersistConversationTurns after 20261020005707_PriceDailyAiBudgetsInUsd. Read both ids back from __EFMigrationsHistory on the staging Render Postgres (read-only query).
  • Keep the existing Encryption__Key in the staging and production API env groups; no new variables or backfill.
  • Activation stays as the body says: after ORB-98 publishes the disclosure, turn astra_conversation_recording on through manage_feature_flags.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

The new bulk user update breaks normal PostgreSQL account resets with a persistent concurrency conflict, even while conversation recording is disabled.

Reviewed changes Reviewed the complete persistence feature, chat capture paths, retention scheduling, export compatibility, and account lifecycle changes.

  • Encrypted storage: Adds paired user/assistant conversation rows, encrypted content and tool arguments, indexes, and a disabled recording flag.
  • Chat capture: Records final replies and original tool outcomes through a bounded background queue, including static block previews while excluding replays.
  • Data lifecycle: Adds hourly retention cleanup, user-scoped exports, and reset-cutoff filtering under the existing user lock.
  • Operational rollout: Documents disclosure prerequisites, capacity assumptions, activation, shutdown, and rollback behavior.
  • Verification: All 62 focused tests passed across Application, Domain, and Infrastructure. Those tests use SQLite or mocks and do not exercise PostgreSQL xmin; no live PostgreSQL execution was performed.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread src/Orbit.Infrastructure/Persistence/AccountResetRepository.cs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No new issues found.

Reviewed changes Re-reviewed the changes since 9bed59a1, with the complete PR diff as context, and confirmed that the resolved concurrency finding is fixed.

  • Removed the conflicting update: Account cleanup no longer bulk-updates the tracked user, while the existing reset save persists the conversation cutoff atomically with deletion under the user lock.
  • Added concurrency regression coverage: Exercised the real reset handler with a simulated advancing concurrency token, checking successful cleanup, persisted cutoff, and other-user isolation.
  • Strengthened queued-write coverage: Verified that reset rejects previously queued conversation captures while allowing post-reset captures. All 17 focused reset and recorder tests passed; PostgreSQL execution and the full suite were not rerun in this review.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

…/ticket-201-persist-conversations

# Conflicts:
#	src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.Accumulator.cs
#	src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.cs
#	tests/Orbit.Application.Tests/Commands/Chat/ProcessUserChatCommandHandlerTests.cs

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes Re-reviewed the merge from redesign/main since the prior approval at ff2f0f7a, using the complete PR diff as context.

  • Preserved capture state: Retained conversation recording alongside the incoming habit-resolution accumulator fields and initialization, including block-answer recovery state.
  • Retained failure coverage: Kept the diagnostic assertions for original tool arguments and errors together with the incoming failed-operation assertion.
  • Verified merged behavior: Passed 311 focused Application tests and 20 focused Infrastructure tests covering chat, capture, streaming, exports, retention, migration shape, and account reset. The previous concurrency fix remains intact; the full suite and live PostgreSQL execution were not rerun.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

Keep input moderation, crisis replies and conversation diagnostics together. Regenerate PersistConversationTurns after the AI-budget migrations and assert the combined migration chain.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes Re-reviewed the changes since 80455eb9, using the complete PR diff as context.

  • Preserved crisis capture: Enabled diagnostic recording on the early crisis-response path according to the recording flag, without invoking moderation, tools, or the model.
  • Covered moderation boundaries: Added assertions for unrecorded normal refusals, recorded block-preparation refusals, and captured choice-answer text while retaining replay exclusion.
  • Regenerated the migration: Positioned conversation storage after the incoming OAuth and AI-budget migrations and strengthened predecessor/target/current model checks to verify the intended schema additions.
  • Verified merged behavior: Passed 355 focused Application tests and 36 focused Infrastructure tests, including recording, streaming, migration alignment, retention, and reset concurrency. The full suite and live PostgreSQL deployment were not rerun.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 4e7ddcb into redesign/main Oct 20, 2026
26 checks passed
@thomasluizon
thomasluizon deleted the thomasluizon/ticket-201-persist-conversations branch October 20, 2026 02:31
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