feat(web): per-provider AI consent gate + coverage expansion (A-01/A-02/A-07/A-08) - #381
Merged
Merged
Conversation
…aise coverage floors - docs: SECURITY.md now lists v1.9.x as supported (was stale at 1.8.x) - deps: add pnpm.overrides @babel/core >=7.29.6 (GHSA-4x5r-pxfx-6jf8 — arbitrary file read via sourceMappingURL comment; devDep path via Stryker instrumenter) pnpm audit now reports 0 vulnerabilities across all severity levels - test: raise vitest coverage thresholds to measured values — lines 43%, functions 41% (Stufe C; all 4 critical-path files confirmed >=80%: safetyPipeline 100%, syncEncryptionService 96.77%, plantSimulationService 98.5%, diagnosisService 96.1%) - docs: AUDIT-REPORT-2026-07-02-FULL.md — full-scale deep audit and perfection report (2936 tests, all quality gates green, maturity score 8.9/10, backlog A-01 to A-12) - docs: CHANGELOG unreleased section updated with post-v1.9.0 splits and ci improvements Gates verified: typecheck pass, lint:changed pass, file-budget pass, pnpm audit 0 vulns
Enforce data-transmission consent (GDPR Art. 6/7) before the first cloud AI call to each provider. Consent is stored per-provider in localStorage; once granted it is not re-prompted. - services/localRoutingService.ts: consent check in withLocalFallback — dynamically imports aiConsentService + aiProviderService; requests consent via UIStore promise; denial falls back to local AI - stores/useUIStore.ts: add providerConsentRequest state + requestProviderConsent(provider) promise-based action + resolveProviderConsent(granted) resolver - components/common/ProviderConsentModal.tsx: new modal subscribed to UIStore providerConsentRequest; renders provider display name + DPA link from aiConsentService; all 5 locales via settings namespace - components/views/plants/App.tsx: mount ProviderConsentModal at app root - locales/*/settings.ts: add ai.providerConsentDeny key (all 5 locales) Tests: - services/aiConsentEnforcement.test.ts: 5 new tests — deny falls back to local, grant calls cloud + persists consent, already-granted skips dialog, active-provider used (not hardcoded), cloud error after consent falls back - services/localRoutingService.test.ts: add consent/provider/UIStore mocks so existing routing tests continue to pass unaffected Gates: typecheck pass, lint:changed pass (10 files), 16/16 tests pass
…(A-02/A-07/A-08) analyticsService.test.ts (157 -> 363 LOC, +21 tests): - empty state, gardenScore weighting, riskFactors health/environment, milestone estimation harvest/transplant/finished, journalActivityTrend 14-day buckets and entry counting, nutrientConsistency entries, growDurationStats, multi-plant strain ranking, stage distribution voiceCommandRegistry.test.ts (+25 tests): - levenshtein: identity, empty, substitution, insertion, deletion, multi-char edit distance, symmetry, unicode cannabis terms - matchVoiceCommand pass 1 (exact alias), pass 2 (fuzzy <= 2), pass 3 (keyword tokens >= 2), edge cases, requiresConfirmation passthrough A-09 CRDT conflict UI verified complete: SyncConflictModal wired in CloudSyncPanel with setSyncConflict -> syncState.conflictInfo -> modal (3 resolution paths: merge, keep local, keep remote). No code changes needed. Gates: typecheck pass, 46/46 tests pass
…d-file check These are translation data files and strain catalog content — the 700 LOC architecture budget applies to component/service/hook modules, not locale dictionaries or strain data files which are inherently large by design. The SCAN_GLOBS (used for the advisory full scan) already excluded these paths; the diff-based check (used in pre-push and CI changed-files gate) now matches.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
qnbs
marked this pull request as ready for review
July 14, 2026 12:18
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a global per-provider AI consent flow with local fallback enforcement, provider-specific modal content, and translations. It also expands analytics service tests and refocuses voice command tests on pure matching utilities. ChangesProvider consent flow
Analytics service test coverage
Voice command matcher tests
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CloudRouting as withLocalFallback
participant ProviderService as aiProviderService
participant ConsentService as aiConsentService
participant UIStore as useUIStore
participant ConsentModal as ProviderConsentModal
CloudRouting->>ProviderService: getActiveProvider()
CloudRouting->>ConsentService: hasConsent(provider)
CloudRouting->>UIStore: requestProviderConsent(provider)
UIStore->>ConsentModal: expose consent request
ConsentModal->>UIStore: resolveProviderConsent(granted)
UIStore-->>CloudRouting: return consent result
CloudRouting->>ConsentService: grantConsent(provider)
CloudRouting->>ProviderService: execute cloud call
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This branch predates the pnpm 11 migration (#413) by 37 commits, so it still carried package.json's `pnpm.overrides` block -- which pnpm 11 no longer reads. Its only addition there, `@babel/core: >=7.29.6`, already lives on main in pnpm-workspace.yaml, so package.json and the lockfile take main's version. check-file-budget.mjs also takes main's version. Both branches hit the same problem -- touching a locale file failed the budget gate -- and solved it differently: this branch blacklisted `locales/` and `data/strains/`, main scoped the changed-file path to SCAN_GLOBS. The whitelist subsumes the blacklist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Docker |  |
Jul 14, 2026 2:47p.m. | Review ↗ |
| JavaScript |  |
Jul 14, 2026 2:47p.m. | Review ↗ |
| Python | |
Jul 14, 2026 2:47p.m. | Review ↗ |
| Rust | |
Jul 14, 2026 2:47p.m. | Review ↗ |
| Shell | |
Jul 14, 2026 2:47p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
…aller requestProviderConsent() stored the pending resolver in the store and returned a fresh promise each time. Two AI calls reaching the gate before the user answers -- two analyses in flight, or a StrictMode double-invoke -- meant the second call overwrote the first one's resolver. resolveProviderConsent() then settled only the second. The first promise stayed pending forever and its AI call sat in a loading state that nothing could ever clear. A caller arriving while a prompt is open now joins it instead of replacing it, and both settle on the single answer the user gives. Covered by two tests, which hang against the previous implementation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Rebased onto main (37 commits behind) and reviewed. MergeThree conflicts, all from this branch predating the pnpm 11 migration (#413):
Both new gates on main pass against this branch: Review of the consent gateThe placement is right. One bug found and fixed (ac374d9)
requestProviderConsent: (provider) =>
new Promise<boolean>((resolve) => {
set({ providerConsentRequest: { provider, resolve } })
}),Two AI calls reaching the gate before the user answers — two analyses in flight, or a StrictMode double-invoke — meant the second overwrote the first one's resolver. A caller arriving while a prompt is open now joins it rather than replacing it, and both settle on the single answer the user gives. Two tests cover it; they hang against the previous implementation. 🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/components/common/ProviderConsentModal.tsx`:
- Around line 17-23: Update the translation lookups in ProviderConsentModal to
use the security.* keys instead of ai.* for the provider consent title, deny
action, prompt, and DPA link. Keep the existing settings namespace and
interpolation behavior unchanged while switching each affected t call to the
matching security locale entries.
In `@apps/web/locales/es/settings.ts`:
- Line 79: Update ProviderConsentModal.tsx to read all consent modal strings
from the security namespace: security.providerConsent,
security.providerConsentDeny, security.providerConsentPrompt, and
security.providerDpaLink. Apply the corresponding locale-key correction in
apps/web/locales/es/settings.ts (79-79), apps/web/locales/fr/settings.ts
(79-79), and apps/web/locales/nl/settings.ts (77-77), ensuring the title,
prompt, and buttons resolve consistently.
In `@apps/web/services/analyticsService.test.ts`:
- Around line 267-301: The getNutrientConsistency test currently places nutrient
values in journal entries, but getNutrientConsistency reads plant.medium.ph and
plant.medium.ec. Update the plantWithReadings fixture to provide the intended pH
and EC values through its medium field, while preserving the assertions that
validate the resulting consistency entry.
In `@apps/web/services/localRoutingService.ts`:
- Around line 114-127: Wrap the entire per-provider consent gate in
withLocalFallback’s existing error-handling path, including getActiveProviderId,
the useUIStore import, requestProviderConsent, and grantProviderConsent, so any
thrown error invokes localFallback() rather than escaping. Preserve the current
denied-consent fallback and granted/already-granted behavior.
- Around line 115-119: Update the consent flow in withLocalFallback around
aiProviderService.getActiveProviderId and useUIStore.requestProviderConsent to
deduplicate in-flight requests per provider. Ensure overlapping calls for the
same activeProvider await the same promise or queued resolver without
overwriting an earlier caller, while preserving independent consent handling for
different providers.
In `@apps/web/services/voiceCommandRegistry.test.ts`:
- Around line 2-7: Update the module docstring in the voiceCommandRegistry test
file to remove the inaccurate claim that buildVoiceCommands is tested via a mock
fixture, leaving only coverage statements for the utilities actually imported
and tested, such as levenshtein and matchVoiceCommand.
In `@apps/web/stores/useUIStore.ts`:
- Around line 65-69: Implement a FIFO queue for provider-consent requests
instead of storing only one pending request. Update the providerConsentRequest
state and requestProviderConsent, plus the resolution logic around the affected
consent handlers, so concurrent callers retain each resolve callback, are
presented one at a time, and each promise settles exactly once before advancing
to the next queued request. Preserve the existing behavior for a single request
and update related state typing or documentation to reflect the queue.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85163e61-5598-4dd2-9fce-3950b122d582
📒 Files selected for processing (13)
apps/web/components/common/ProviderConsentModal.tsxapps/web/components/views/plants/App.tsxapps/web/locales/de/settings.tsapps/web/locales/en/settings.tsapps/web/locales/es/settings.tsapps/web/locales/fr/settings.tsapps/web/locales/nl/settings.tsapps/web/services/aiConsentEnforcement.test.tsapps/web/services/analyticsService.test.tsapps/web/services/localRoutingService.test.tsapps/web/services/localRoutingService.tsapps/web/services/voiceCommandRegistry.test.tsapps/web/stores/useUIStore.ts
…nsent-enforcement-5c7a
My previous commit reached for `useUIStore.getState()` inside the store's own creator and *returned* a value derived from it. That makes the store's type circular: TypeScript gives up inferring UIState, and every `useUIStore((s) => ...)` selector in the app collapses to `any` -- 40+ TS7006 and TS18046 errors across components that never touched consent. Zustand hands the creator a `get` for exactly this. Signature is now `(set, get)`, and both consent actions read through it. The four remaining `useUIStore.getState()` calls in this file predate this branch and are fine: they return void, so nothing circular flows back into the type. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Branch coverage came in at 24.99% against a 25% threshold -- and the branches that dragged it under were mine: the race fix added a queue path for a second provider that no test exercised. Covered now. The test also pins the behaviour down: the queued request opens its own prompt once the first is answered, and each caller gets the answer given to its own prompt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t gate
Two of these were production bugs.
**The consent gate escaped its own fallback contract.** The try/catch wrapped
only cloudFn(), but the gate ahead of it does I/O: two dynamic imports, a
provider lookup and an awaited prompt. Any of those throwing propagated straight
out of withLocalFallback -- breaking the guarantee the function documents, that
the user always gets a response. The gate now sits inside the try, so a failing
import or a rejected prompt lands in the local fallback like any other failure.
Two tests cover it; both fail against the previous code.
**ProviderConsentModal rendered raw keys.** It looked the strings up as
`t('ai.providerConsent', { ns: 'settings' })`, but they live under `security.*`
in the settings locale. The modal would have shown the literal key strings to
every user it prompted. All four lookups corrected.
**The nutrient-consistency test asserted nothing.** getNutrientConsistency()
reads plant.medium.ph / .ec; the fixture only set journal entries, which the
function never consults. The test therefore passed or failed on basePlant.medium,
not on the values it appeared to be testing. Rewritten against `medium`, with
exact assertions on avgPh/avgEc/variance/rating, plus an unstable-drift case.
Also:
- withLocalFallback: complexity 10 (DeepSource JS-R1005). The consent gate is
extracted into ensureProviderConsent(), which is what pushed it over.
- Function declarations in global scope (JS-0067): withLocalFallback, makeCmd and
importWithLocalFallback are now const arrows.
- async without await (JS-0116): five mock factories no longer claim to be async.
- voiceCommandRegistry.test.ts claimed to cover buildVoiceCommands, which it never
imports. Docstring now says what the file actually tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…udget DeepSource JS-R1005: still 9 after extracting the consent gate. The mode check and the rate-limit notification are both self-contained and neither is about falling back, so they move out too -- isCloudDisabled() and notifyIfRateLimited(). What is left reads as the contract it implements: skip the cloud in local modes, ask for consent, call the cloud, and fall back to local on any failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'Analyze (python)' job died in 'Initialize CodeQL' with 'Error: read ECONNRESET' -- a network failure on GitHub's side, not a finding. The run sits on the current HEAD and GitHub refuses to retry it, so this empty commit is the only way to get a fresh one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
Works through backlog items A-01, A-02, A-07, A-08, and A-09 from the 2026-07-02 full audit report.
A-01: Per-Provider AI Consent Gate (P1 — security/UX)
Enforces data-transmission consent (GDPR Art. 6/7) before any cloud AI call to each provider. The consent is stored per-provider in localStorage; once granted, the user is not re-prompted.
Changes:
services/localRoutingService.ts: consent check injected intowithLocalFallback— dynamically importsaiConsentService+aiProviderService, requests user consent via UIStore promise; denial routes to local AI fallback without any network requeststores/useUIStore.ts:providerConsentRequeststate +requestProviderConsent(provider)promise action +resolveProviderConsent(granted)resolvercomponents/common/ProviderConsentModal.tsx: new modal (rendered at app root) subscribed to UIStoreproviderConsentRequest; shows provider display name, DPA link; i18n viasettingsnamespacecomponents/views/plants/App.tsx: mountProviderConsentModalat app rootlocales/*/settings.ts: addai.providerConsentDenykey (all 5 locales: EN/DE/ES/FR/NL)Tests (5 new —
aiConsentEnforcement.test.ts):A-02/A-07: analyticsService coverage expansion
analyticsService.test.tsexpanded from 157 to 363 LOC (+21 meaningful tests):A-02/A-08: voiceCommandRegistry coverage expansion
voiceCommandRegistry.test.tsexpanded (+25 tests):levenshtein: identity, empty string, substitution, insertion, deletion, multi-char (kitten→sitting=3), symmetry, unicode cannabis termsmatchVoiceCommandpass 1 (exact alias), pass 2 (fuzzy Levenshtein ≤2), pass 3 (keyword tokens ≥2), edge cases (empty registry, short tokens),requiresConfirmationpassthroughA-09: CRDT conflict UI verified
SyncConflictModalis fully wired inCloudSyncPanel:setSyncConflict(info, null)→syncState.conflictInfo→ modal renders with 3 resolution paths (merge, keep local, keep remote). No code changes needed — wiring confirmed complete.chore: file-budget locale exclusion
scripts/check-file-budget.mjsdiff-based check now excludeslocales/anddata/strains/paths — these are translation dictionaries and content files, not architecture-bounded modules. The SCAN_GLOBS (advisory full scan) already excluded these paths; the changed-file gate now matches.Security Impact
Details:
withLocalFallbacknow checksaiConsentService.hasProviderConsent(provider)before every cloud AI call. Denial routes to local AI — no data leaves the device without explicit per-provider user consent.i18n
ai.providerConsentDenyadded to all 5 locale settings filesValidation
pnpm run typecheckpnpm run lint:changedpnpm run check:file-budgetpnpm run test:runpnpm auditNotes
Summary by CodeRabbit
New Features
Tests