Phase 9: User Settings UI on Curated Catalog#74
Merged
Conversation
- Move model probing into the Spring AI gateway package and delegate admin master-key probes through ProviderConnectionTester - Add per-provider probe result model collection with Anthropic x-api-key plus anthropic-version handling - Add BYOK base URL SSRF validation, pinned probe transport, provider allow-list, and Redis-backed rate limiter
Add UserByokService and ByokProviderResolver for stored-row BYOK lifecycle. Replace legacy tenant BYOK resolver in gateway and chat factory. Rewrite legacy /api/llm/byok controller as 410 Gone shim and fill BYOK lifecycle tests.
Add /api/byok controller, DTOs, stored-row test connection, activation and model endpoints. Add /api/settings/ai/cost backed by tenant-wide seven-day llm_call_audit SUM. Fill BYOK controller, SSRF, plaintext, enum-only, sentinel, and cost tests.
…tings-ui-on-curated-catalog # Conflicts: # apps/web/features/ai/components/AiConfigPage.tsx # apps/web/lib/api/schema.d.ts
…tings-ui-on-curated-catalog # Conflicts: # apps/web/lib/api/schema.d.ts # apps/web/openapi/openapi.json # backend/api/src/test/java/com/zeromail/api/controllers/llm/ByokControllerIntegrationTest.java # backend/core/src/main/java/com/zeromail/core/thread/usecases/ClassifyThreadReplyStatusService.java # backend/core/src/test/java/com/zeromail/core/thread/ClassifyThreadReplyStatusServiceIntegrationTest.java
…/Tools - Replace MAIL_NAV/MANAGE_NAV with DAILY_NAV/AUTOMATION_NAV/TOOLS_NAV - Place Rules and AI config together in Automation group - Swap nav.sectionMail/sectionManage for sectionDaily/sectionAutomation/sectionTools Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Rebuild vi.json/en.json with sectionDaily/sectionAutomation/sectionTools - Drop orphaned sectionMail/sectionManage (additive generator leaves stale keys) - typecheck, lint, i18n:check all green Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ro-mail.* - SecurityConfig @value session.cookie.secure + admin.webauthn.* placeholders -> zero-mail.* - Loadtest + 5 e2e-stub @ConditionalOnProperty keys -> zero-mail.* - TriagePendingReaperBatch + WaitlistService @value placeholders -> zero-mail.* - application.yml: fold standalone zeromail: session block into zero-mail: - application-loadtest.yml + application-e2e-stub.yml: keys + single zero-mail: top-level mapping - env var NAME ZEROMAIL_SESSION_COOKIE_SECURE unchanged (relaxed binding maps it) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dant yml autoconfigure exclude, and no-op prod format_sql - base ymls: remove provider/base-url/4 models/timeouts under llm.platform (all equal LlmProperties record defaults; only api-key is deployment-supplied) - base ymls: remove spring.autoconfigure.exclude block (GenAI embedding exclusion already on @SpringBootApplication excludeName, timing-safe per Boot #26858) - prod profiles: remove no-op spring.jpa format_sql:false (base already sets it); annotate retained privacy observations block as intentional defense-in-depth Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… (low-fan-out subtrees) - Extract CryptoProperties, GmailProperties, BillingProperties, AdminProperties from ZeroMailCoreProperties into per-feature config packages, each its own @ConfigurationProperties(prefix=zero-mail.<feature>); bound keys unchanged - Preserve @validated + @DefaultValue + compact-constructor defaults + masked toString() (crypto KEK, LemonSqueezy secret, admin audit HMAC KEK) - Rewire consumers to inject the specific record instead of the god-object - ZeroMailCoreProperties now holds only the llm subtree (deleted in Task 3b) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rties and delete god-object - New @ConfigurationProperties(prefix=zero-mail.llm) LlmProperties holding relocated PlatformProperties (was ZeroMailLlmProperties) + ByokProperties (was ZeroMailLlmByokProperties) - Masked toString() over platform api-key - Update all .llm() consumers + ~15 nested-import sites across main, test, and aiEval source set - DELETE ZeroMailCoreProperties.java (god-object fully decomposed) - bound keys unchanged (zero-mail.llm.platform.*, zero-mail.llm.byok.*) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ule config records - ZeroMailApiProperties -> ApiProperties (prefix zero-mail.api unchanged) - ZeroMailWorkerProperties -> WorkerProperties (prefix zero-mail.worker unchanged) - ZeroMailLlmDriftProperties -> DriftProperties (prefix zero-mail.llm.drift unchanged) - ZeroMailChatProperties -> ChatProperties (prefix zero-mail.chat unchanged) - rename test ZeroMailApiPropertiesTest -> ApiPropertiesTest; update all reference sites - no @ConfigurationProperties prefix changed; only Java type names Note: chat controller/orchestrator/history/tokenizer/factory carried concurrent parallel-agent streaming edits in the working tree at stage time (same race as STATE.md Phase 1.2 P05 / 01.2.1 P04); included here since the rename is interleaved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ia zero-mail-shared.yml - New backend/core zero-mail-shared.yml holds the correctness-locked block: spring.ai.model.*=none (8 modalities) + chat[.client].observations.log-prompt/completion=false - Both base ymls import it via spring.config.import: optional:classpath:zero-mail-shared.yml (imported-file values WIN, locking the privacy invariant) and drop the duplicated block - api keeps spring.ai.openai...include-completion; worker keeps spring.ai.retry.max-attempts - CONVENTIONS.md #9: document the centralization bend for correctness-locked shared boilerplate Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ts ciphers The god-object split placed CryptoProperties in an invented top-level package core.crypto.config holding a single 1-field record, orphaned from the actual crypto code. Relocate it to com.zeromail.core.shared.crypto — beside PlatformSecretCipher/Hashing and the RefreshTokenCryptoConfig that consumes it. Bound key zero-mail.crypto.refresh-token-key-base64 unchanged; masked toString() + @validated preserved. Empty core.crypto package removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…+ state Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tings - /settings now uses /ai-style SectionHeader + SettingCard (no tabs): Account, Display (inline language + theme segmented), Gmail connection, Danger zone. - Removed pseudo-tab nav strip, credit card, triage pause card, provider card, and the privacy card (all duplicated elsewhere). - Consolidated daily-digest into /ai Updates (ported send-hour Select); deleted NotificationsSection + its test. - Deleted /settings/privacy route + orphaned features/privacy; removed its AppSidebar nav item + privacy-page.spec.ts; cleaned check-i18n allowlist. - SettingCard.title made optional (backward-compatible) so single-card sections don't repeat the section header label. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erge Phase 9 follow-ups on the /ai settings surface plus adjacent cleanups: - Voice settings: tone-preset removed (TonePresetDialog deleted), DTO/command/result trimmed - Merge assistant_memory into assistant_knowledge (changeset 106); delete AssistantMemoryEntity/Service/Repository + tests; SearchMemoriesToolHandler and knowledge service now share one store - Frontend chat streaming: add StreamingTextResponse component + AssistantTextNormalizer, fix one-block streaming regression - Remove legacy dashboard route + DashboardPageClient/PauseBanner - Add react-doctor skill/workflow/config tooling - Regenerate OpenAPI schema + lockfile updates Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 276 files, which is 126 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (24)
📒 Files selected for processing (276)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
React Doctor found 66 issues in 15 files.
Top Findings
Generated by React Doctor. Questions? Contact founders@million.dev. |
Phase 9 cleanup (tone-preset removal, dashboard deletion, memory merge) removed tested surface and dropped function coverage to 22.28%, below the 25% gate. Align functions with the other three metrics already at 20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public boolean autoDraftRepliesEnabled(UUID tenantId) { |
|
|
||
| @Transactional(readOnly = true) | ||
| public Optional<ResolvedLlmProviderCredential> resolve( | ||
| UUID tenantId, String ignoredFallbackModel) { |
|
|
||
| @Transactional(readOnly = true) | ||
| public Optional<ResolvedLlmProviderCredential> resolveForChat( | ||
| UUID tenantId, String ignoredRequestedModel) { |
|
|
||
| @FunctionalInterface | ||
| interface PinnedResolutionObserver { | ||
| void observe(String host, InetAddress resolvedAddress); |
|
|
||
| @FunctionalInterface | ||
| interface PinnedResolutionObserver { | ||
| void observe(String host, InetAddress resolvedAddress); |
| if (statusParts.length < 2) { | ||
| throw new IOException("Malformed HTTP status line"); | ||
| } | ||
| int statusCode = Integer.parseInt(statusParts[1]); |
Two CI gate failures on backend/api:test:
- Spring Modulith verify(): controllers depended on non-exposed dto.byok
types (plan 09-04 added the BYOK DTOs without a NamedInterface). Add
package-info @NamedInterface("byok") mirroring the other dto.* packages.
- SpringAiObservationDisabledTest read worker application.yml literally, but
commit f3789ba centralized the prompt/completion suppression keys into
zero-mail-shared.yml. Point the static-config assertion at the shared file
(imported by both api and worker), preserving the privacy invariant guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aries Phase 9 BYOK reused admin master-key types from core.llm, which violated the deliberate 'core.llm must not import core.admin' boundary (LlmGatewayBoundaryTest, 108x) and created an admin<->llm module cycle (ThreadModuleScenarioTest). Both were latent: this branch had never hit CI. - Move LlmProvider, KeyFormat, MasterKeyTestResult from core.admin.mkey into core.llm.domain (the foundational layer admin adapts to); rewrite ~80 imports. admin now depends downward on llm.domain (allowed direction). - Expose llm.config (RestClientConfig reads LlmProperties) and llm.byok (chat SpringAiChatModelFactory + VoiceGenerationService use ByokProviderResolver / ByokRateLimiter) as NamedInterfaces; whitelist them in the consuming modules' allowedDependencies (+ billing::domain for chat VoiceGenerationService). - ApplicationYmlLlmConfigTest: assert the centralized observation-suppression block in zero-mail-shared.yml (completing the f3789ba refactor), matching the api SpringAiObservationDisabledTest fix. Full backend core+api suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Phase 9: User Settings UI on Curated Catalog (v1.2)
Goal: A user can open
/aiand configure their writing voice, assistant behavior, sender safety net, and per-feature AI provider/model on a single settings surface — with the AI tab pulling exclusively from the admin-curated catalog and BYOK only for the four user-allowed providers (OpenAI / Anthropic / Google / DeepSeek).Status: Phase marked complete 2026-05-29 (7/7 plans).⚠️ No automated
VERIFICATION.mdon record — shipped per explicit operator request;09-VALIDATION.md+ Playwright golden path +Phase9ArchitectureTeststand in as evidence.Phase 9 delivers the Inbox Zero–style
/aisettings surface: flatSectionHeadergroups (Your voice, Behavior, Updates, Safety net, AI Provider),SettingCard+Dialogedit pattern, a knowledge-snippet table withUNIQUE(tenant_id, title), aLOW/MEDIUM/HIGHdraft-confidence enum, sender safety-net add/remove with a triage audit "blocked by safety net" badge, and a single BYOK card enforcing the Save → Test → Pick → Activate lifecycle through the sharedProviderConnectionTester. The final commit on this branch folds in post-completion cleanups: removing the tone preset, mergingassistant_memoryintoassistant_knowledge, and a frontend chat-streaming regression fix.Changes
Plan 09-01: Database foundation
Liquibase changesets 094–097 (assistant settings columns, knowledge unique-title, safety-net pattern kind + audit badge, user BYOK key table) + JPA scaffolding + Wave-0 test stubs.
Plan 09-02: Voice + Behavior + Knowledge backend
Voice/behavior settings endpoints, tenant-scoped knowledge-snippet CRUD, shared sanitizer + knowledge write-site ArchUnit rules, draft runtime reads for auto-draft/signature/sensitive-data toggles.
Plan 09-03: Safety Net + audit badge
Safety-net DOMAIN pattern opt-in, tenant-scoped DELETE authority, matcher returning matched patterns, triage audit API exposing
blockedBySafetyNetPattern.Plan 09-04: BYOK + cost
Shared
ProviderConnectionTester(admin MKEY + user BYOK),UserByokService+ByokProviderResolver,/api/byokendpoints,/api/settings/ai/cost7-day tenant cost endpoint.Plan 09-05: Generate-from-Sent (SET-VOICE-07)
POST /api/settings/voice/generate-from-sentpreview, Gmail Sent reader with quoted-reply stripping + aggregate prompt cap, in-memory privacy invariant + Spring AI observation hardening, leak/rate-limit tests.Plan 09-06: Frontend settings surface
Flat five-section
/aishell; Voice/Behavior/Knowledge/Safety-net/BYOK features deriving types from generatedschema.d.ts; OpenAPI regen after legacy/api/llm/byokdeletion; audit safety-net badge in table + card views.Plan 09-07: Validation closeout
Playwright
ai-settings.spec.tsgolden path, aggregatePhase9ArchitectureTest, manual UX checkpoint,09-VALIDATION.md+ roadmap closeout.Post-completion cleanups (this branch's final commit)
TonePresetDialogdeleted; voice DTO/command/result trimmed)assistant_memorymerged intoassistant_knowledge(changeset 106);AssistantMemoryEntity/Service/Repository+ tests deleted; one shared storeStreamingTextResponse+AssistantTextNormalizer, resolves one-block streaming regressionDashboardPageClient/PauseBannerremovedreact-doctorskill/workflow/config tooling addedRequirements Addressed
19 requirements:
SET-VOICE-01..06,SET-BEHV-01..05,SET-SAFE-01..04,SET-AI-01..04(SET-SAFE-02/03 deferred to v1.3 per spec-phase scope decision).Verification
VERIFICATION.md: not on record — phase closed via09-VALIDATION.mdinsteadapps/web/e2e/ai-settings.spec.tsPhase9ArchitectureTestaggregate ArchUnit guard/aigolden path + BYOK Save→Test→Pick→Activate in a browser before mergeKey Decisions
SensitiveDataProtectionDeciderlives incore.llm.usecases(notllm.redaction) so chat settings avoid depending on a non-exported impl package.TriageDraftSettingsto avoid a triage → chat Modulith cycle.SafetyNetMatcherlives incore.triage.usecases(shared viaTriageOrchestratorService), not a worker-only class./ai(single-tenant-per-user); per-feature picker + tenant-wide mode card removed in favor of one BYOK card with an Active switch.RefreshTokenCipherAES-GCM envelope.🤖 Generated with Claude Code