feat: add CLI tools for notification filters and channel preferences

[nooblemon-eth]

Summary

Third PR in Phase C notification system rewrite. Adds CLI tools for managing the new notification preference system introduced in lemonade-backend#2069.

New tools

2 consolidated wrappers (always-loaded, visible in AI tool list):

• manage_notification_filters (actions: list, set)
• manage_notification_channel_preferences (actions: list, set)

Delete actions intentionally routed through granular tools to preserve destructive confirmation and dry-run gates.

6 granular tools (for fine-grained control and destructive operations):

• notification_filters_list (deferred, query)
• notification_filters_set (mutation, create/update)
• notification_filters_delete (mutation, destructive)
• notification_channel_preferences_list (deferred, query)
• notification_channel_preferences_set (mutation, create/update)
• notification_channel_preferences_delete (mutation, destructive)

Enhanced notifications_list

• Switched from aiGetNotifications → getNotifications (required for the new category parameter)
• Now supports category param for UI tab filtering (event/social/messaging/payment/space/store/system)
• Queries expanded fields (from_expanded, ref_event_expanded, ref_space_expanded) for human-readable names
• Returns mapped shape with from_user_name, ref_event_title, ref_space_title

Natural language usage

> mute all social notifications
> only show me event notifications
> hide notifications from the Warehouse Party event
> show me my notification settings

The AI chains tool calls to search for event/space IDs when scoping filters to specific entities.

Test plan

• yarn build passes
• yarn lint passes
• yarn test passes (1416 tests, 41 new)
• Consolidated tools reject delete action (must use granular)
• Consolidated tools reject set without required params (mode for filters, enabled_channels for preferences)
• Delete operations marked destructive and backendType mutation
• notifications_list returns expanded field names

[nooblemon-eth]

Karen's Review — lemonade-cli PR #205

Verdict: APPROVE

Risk Level: LOW-MEDIUM

This is the third PR in the Phase C notification rewrite, adding 8 CLI tools targeting the merged backend PR #2069. I went deep on the cross-repo contract since this PR rewrites a core notification query and adds 6 brand-new resolvers, all of which depend on backend types that landed in the same release cycle. The contract holds end-to-end and I have no blocking concerns.

Stats

• Files reviewed: 6
• Findings: 0 BLOCKER, 0 CRITICAL, 0 MAJOR, 1 MINOR, 3 NIT
• Build: PASS (yarn build clean, no TS errors)
• Lint: PASS (yarn lint clean)
• Tests: PASS (1416/1416 on re-run; one flaky test in tests/unit/codegen/loader.test.ts on first full-suite run that passed in isolation and on retry — pre-existing test isolation issue, NOT caused by this PR)
• CI: GREEN (lint, build, test all SUCCESS on the latest commit)
• Cross-repo impact: NONE (the old aiGetNotifications resolver is still present in backend for any other consumer; web-new generated types still reference it but no live consumers in this PR's scope)

What I verified

1. Backend contract (lemonade-backend master, PR #2069 merged)

I read the actual resolver and type definitions and cross-checked every CLI query/mutation against them:

• getNotifications(skip, limit, category) — category: NotificationCategory arg confirmed at lemonade-backend/src/graphql/types/notification.ts:23-24. CLI's $category: NotificationCategory matches.
• from_expanded.name — User.name is a non-null @Field at lemonade-backend/src/app/models/user.ts:298-300. Safe.
• ref_event_expanded.title — Event.title exists. Safe.
• ref_space_expanded.title — Space extends Whitelabel, which has title!: string at lemonade-backend/src/app/models/partials/whitelabel.ts:8. Safe.
• getNotificationFilters / setNotificationFilter / deleteNotificationFilter — resolver signatures at lemonade-backend/src/graphql/resolvers/notification.ts:183-256 match the CLI's GraphQL strings exactly (input type NotificationFilterInput, scalar MongoID! for filterId).
• getNotificationChannelPreferences / setNotificationChannelPreference / deleteNotificationChannelPreference — same: signatures at notification.ts:259-337 match.
• Field set on NotificationFilter and NotificationChannelPreference partials matches the queried fields exactly: _id, mode, notification_type, notification_category, ref_type, ref_id, space_scoped and enabled_channels respectively. Verified at lemonade-backend/src/app/models/partials/notification-filter.ts:25-55 and notification-channel-preference.ts:10-40.
• Enum values (NotificationCategory, NotificationFilterMode, NotificationRefType) verified against backend enums — CLI const arrays match exactly.

2. IDOR / ownership

deleteNotificationFilter and deleteNotificationChannelPreference are scoped via UserModel.updateOne({ _id: userId }, { \$pull: ... }), and setNotificationFilter with a client _id uses { '_id': userId, 'notification_filters._id': clientId } then asserts matchedCount > 0. A user cannot mutate or delete another user's filter/preference. The backend even has thoughtful comments explaining the create-vs-update separation to prevent stale-cache duplicate minting (notification.ts:204-228, 287-309). No IDOR.

3. Breaking change to notifications_list shape

Old shape: { id, type, message, from_user_name, ref_event_title, read, created_at }
New shape: { _id, type, title, message, created_at, is_seen, from_user_name, ref_event_title, ref_space_title }

Field renames: id → _id, read → is_seen. New fields: title, ref_space_title. Removed: none of value (old from_user_name and ref_event_title are preserved via the new mapping in notifications.ts:90-100).

I grep'd the entire CLI repo for consumers of these field names. The only consumers of notifications_list output are:

• The LLM (consumes JSON, no schema lock-in)
• tests/unit/chat/tools/notifications-category.test.ts (asserts the new shape — test was added in this PR)

The other notification consumers — src/chat/notifications/poller.ts and src/api/subscriptions.ts — use their OWN independent GraphQL queries against getNotifications and notificationCreated subscription with their OWN field selections (image_url, from, ref_event, ref_space, ref_user, is_seen). They do NOT depend on the notifications_list tool's mapped shape and were NOT touched by this PR. Their field selections still resolve correctly against the backend Notification type. No regression.

4. Tool registration

All 8 new tools are reachable through getAllCapabilities() via notificationsTools and consolidatedTools arrays imported in src/chat/tools/registry.ts:13,8. The 6 granular tools are exported from src/chat/tools/domains/notifications.ts and the 2 consolidated wrappers from src/chat/tools/domains/consolidated.ts. The dedicated test file (tests/unit/chat/tools/notification-preferences.test.ts:34-49) verifies registry presence with the correct category, defer flag, and space/event scoping for all 6 granular tools.

5. Destructive handling

• Granular notification_filters_delete (notifications.ts:227-257) and notification_channel_preferences_delete (notifications.ts:355-386) both have destructive: true AND backendType: 'mutation'. Verified by tests at notification-preferences.test.ts:51-59.
• The executor (src/chat/tools/executor.ts:287, 134-143) gates BOTH the destructive confirmation prompt AND the dry-run interception on the OUTER tool's flags. So calling the granular delete directly always trips both gates.
• Consolidated wrappers (backendType: 'none') intentionally exclude delete from their actionMap (consolidated.ts:301-303, 347-350) to force destructive operations through the granular tools where the gates fire. The PR author documents this rationale clearly at consolidated.ts:57-61. This matches the existing pattern used by manage_newsletters, manage_subscription, etc.
• Tests at notification-preferences.test.ts:234-244 verify that action: 'delete' is rejected by the consolidated wrappers (expect(...).rejects.toThrow('Unknown action') AND expect(mockedRequest).not.toHaveBeenCalled()).

6. Test quality

46 new tests across 2 files exercise the important paths:

• Conditional input building (only-provided fields, empty-string omission)
• _id vs no-_id create-vs-update behavior
• Delegation paths (consolidated → granular)
• Required-param validation in consolidated wrappers (mode for filters, enabled_channels for preferences) — including the empty-array case
• Delete rejection in consolidated wrappers (no underlying GraphQL call made)
• Category filter passthrough, expanded-field handling, missing-expansion graceful fallback

Tests are not just "the mock works" — they assert against the actual variable shape sent to graphqlRequest, which catches input-construction bugs. The conditional-omission tests are especially valuable because Mongoose enum validation would 422 on empty-string notification_category="" etc., and these tests prove the CLI strips empty strings before sending.

7. Schema validation

The auto-discovery test tests/unit/chat/schema-validation.test.ts was updated (lines 74-81) to add BACKEND_SCHEMA entries for all 6 new operations plus the new fields from_expanded, ref_event_expanded, ref_space_expanded on getNotifications. All 161 schema-validation tests pass — every queried field is in the allowlist.

---

Findings

[MINOR] [BE-AGENT-B] notification_type parameter has no enum constraint

[FILE] src/chat/tools/domains/notifications.ts:174, 303 (and consolidated.ts:284, 331)
[ISSUE] Backend NotificationFilter.notification_type and NotificationChannelPreference.notification_type are typed as the strict NotificationType enum (~50 values like event_announce, event_invite, payment_succeeded, etc., defined at lemonade-backend/src/app/models/notification.ts:15-72). The CLI exposes this parameter as a free-form string with no enum array, so the LLM gets no schema-level guidance about valid values.
[IMPACT] The LLM may waste tool calls passing invalid notification_type values (e.g., "event_published" instead of "event_announce"). The backend will reject with a Type-GraphQL enum validation error, which is safe but burns tokens and creates a poor UX. By contrast, notification_category IS enum-constrained on the CLI side, and mode, ref_type likewise — notification_type is the only free-form one.
[FIX] Either (a) export a NOTIFICATION_TYPES const array mirroring the backend enum and add enum: [...NOTIFICATION_TYPES] to all four parameter declarations, or (b) document in the parameter description that valid values match the backend NotificationType enum and link to it. Option (a) is preferred since the rest of this file uses const arrays for the same pattern. If the enum is too large to maintain manually, the consolidated wrappers' whenToUse strings could at least mention "use list_notifications first to see actual type values your account uses" so the LLM has a fallback path.
[CONTEXT] This is not a security or correctness issue — backend validation is the source of truth — it's a tool-schema usability issue. Filing as MINOR because the feature works without it. Not blocking merge.

[NIT] [BE-AGENT-B] CLI does not pre-validate limit > 1000

[FILE] src/chat/tools/domains/notifications.ts:60-62
[ISSUE] Backend PaginationArgs has @Max(1000) on limit (lemonade-backend/src/graphql/types/pagination.ts:13). CLI does no client-side validation, so an LLM passing limit: 5000 will get a backend validation error rather than a clear local message.
[IMPACT] Token waste on a round-trip; user-facing error could be cryptic. This is consistent with how other CLI tools handle it though, so it's a NIT and a follow-up improvement, not a finding specific to this PR.
[FIX] Optional: clamp limit to Math.min(args.limit ?? 25, 1000) before sending. Or skip — pre-existing pattern.

[NIT] [BE-AGENT-B] notification_filters_set and _set do not include formatResult for the create case where the response has no _id

[FILE] src/chat/tools/domains/notifications.ts:219-223, 348-352
[ISSUE] The fallback path returns JSON.stringify(result) if !f._id. Since the backend always returns the full record (including the freshly minted _id), this branch should be unreachable in practice. Not wrong — just defensive code that will never fire. If you want, you could replace it with a clearer error since "no _id in response" indicates a backend regression.
[FIX] Optional: if (!f || !f._id) return 'Filter saved (response shape unexpected — check backend version).';

[NIT] [BE-AGENT-B] Process tracking

[ISSUE] I checked claude/IMPLEMENTATION-STATUS.md for an entry covering this PR / Phase C notification rewrite and didn't find one. Per the protocol, feature PRs should be tracked. This is the third PR in the phase, so the tracking may be on a parent feature row rather than per-PR — and the PR description does cite the cross-repo backend PR (#2069) clearly, which mitigates the concern.
[FIX] Lead Agent: confirm that Phase C notification rewrite is captured on the dashboard. If not, add a row noting "Phase C: notification rewrite — be#2069 merged, cli#205 in review, fe TBD".

---

Residual risks

1. Consolidated wrapper update bypass — The consolidated manage_notification_filters (action: set) and manage_notification_channel_preferences (action: set) can update an existing filter/preference (when filter_id / preference_id is provided) without firing the destructive confirmation gate, because the wrapper itself is destructive: false and the underlying granular set tools are also destructive: false. This is consistent with the existing pattern in manage_newsletters, manage_subscription, etc., and the PR author's comment at consolidated.ts:57-61 documents the dry-run limitation. Not blocking — but worth noting that "update an existing notification filter via the consolidated wrapper" is one of the few mutating operations in the system that bypasses both confirmation AND dry-run. If this becomes a footgun for users in practice, consider promoting filter UPDATES (where filter_id is set) to destructive: true on the granular _set tool, which would propagate the gate up automatically.

2. Auto-generated skill markdown — src/chat/skills/generated/notifications.md is .gitignore'd and regenerated at build. It still describes only 2 tools and the OLD notifications_list description on a stale local copy. Not a code issue (the file is ignored) but worth a quick yarn build:skills (or whatever the regen command is) before merge to verify the generator picks up all 8 new tools without errors. Test files don't exercise the generator, so a regression in the generator would not be caught here.

3. Schema validation field allowlist is flat — The schema-validation test extracts ALL field names (top-level + nested) into a single flat list and checks them against a flat allowlist per resolver. This means nested-field correctness is approximate — from_expanded { _id name } would still pass even if User.name were renamed, as long as name exists ANYWHERE in the allowlist. Not introduced by this PR; pre-existing limitation. Just be aware that the schema test is a coarse safety net, not a strict type checker.

---

Testing recommendations for FE Test Engineer / Agent D

None — the test suite added in this PR is thorough and exercises the important paths. If you want belt-and-suspenders coverage, consider:

• An E2E test that calls manage_notification_filters with action: set, mode: 'mute', notification_category: 'event' against a real backend and verifies the filter appears in getNotificationFilters results. (Not in scope here — would belong in an integration test track if one exists.)

Deploy notes

• Backend PR #2069 is already merged to lemonade-backend master, so this CLI PR can merge any time without coordination concerns.
• No new env vars.
• No migrations.
• No infrastructure changes.
• Backwards-compatible at the backend layer: aiGetNotifications is still present, so any other consumer (web-new types, MCP, CLI fork, etc.) is unaffected.

---

Verdict: APPROVE

Clean PR with thorough test coverage, correct backend contract, IDOR-safe ownership scoping, and a thoughtful destructive-handling architecture. The single MINOR finding (notification_type enum constraint) is a usability nit and explicitly does not block merge. The author did the right thing by routing deletes through granular tools to preserve the destructive + dry-run gates, and the conditional input-building correctly handles the create-vs-update distinction the backend expects.

Recommend Lead Agent merges after addressing the MINOR (or accepting it as a follow-up). I'll formally APPROVE via gh pr review --approve momentarily.

---

Karen, Staff Engineer, Release Quality Lead

[nooblemon-eth]

Karen formal review marker: APPROVED. (Cannot use --approve API path because the GitHub account that runs Karen happens to also be the PR author for this repo's commit history; the detailed review with verdict APPROVE has been posted as a PR comment. Treat this as the authoritative Karen verdict — APPROVED — for dashboard and Lead Agent purposes.)

[nooblemon-eth]

Karen Final Re-Review — PR #205

Commit reviewed: b3650be (fix: address Karen review findings for notification preference tools)

Verification (Phase 2)

Step	Result
yarn build	PASS (clean)
yarn tsc --noEmit	PASS
yarn lint	PASS
yarn test	PASS — 1416/1416 tests, 93 files
gh pr checks 205	PASS — build/lint/test all green
Branch state	OPEN, MERGEABLE, mergeStateStatus=CLEAN
Diff scope	consolidated.ts, notifications.ts, plus prior PR files only — no unrelated drift

Findings Verification

MINOR (notification_type lacks enum constraint) — FIXED

• NOTIFICATION_TYPES const exported in src/chat/tools/domains/notifications.ts:15 with 54 values, JSDoc references the backend source.
• Cross-reference against lemonade-backend/src/app/models/notification.ts (commit fd94d88d): backend NotificationType enum has 54 values. Programmatic set diff:
  • Backend → CLI missing: 0
  • CLI → Backend extras: 0
  • Duplicates in either side: 0
  • Exact match.
• Enum constraint applied to all 4 locations:
  1. src/chat/tools/domains/notifications.ts:245 — notification_filters_set
  2. src/chat/tools/domains/notifications.ts:373 — notification_channel_preferences_set
  3. src/chat/tools/domains/consolidated.ts:285 — manage_notification_filters
  4. src/chat/tools/domains/consolidated.ts:332 — manage_notification_channel_preferences
• Type compatibility verified: NOTIFICATION_TYPES is as const (readonly tuple), but [...NOTIFICATION_TYPES] produces a mutable string[] matching ToolParam.enum?: string[] in src/chat/providers/interface.ts:69. tsc --noEmit clean.

NIT-A (no client-side limit clamp) — FIXED

• src/chat/tools/domains/notifications.ts:129 — const limit = Math.min((args.limit as number) ?? 25, 1000);
• Description updated at line 116: 'Max results (default 25, cap 1000)'.
• Edge case spot-check: undefined→25, null→25, 1001→1000, 5000→1000. Negative/zero values pass through unclamped, but the original finding was about the upper bound only and backend handles invalid lower bounds — out of scope for this NIT.

NIT-B (unreachable formatResult fallback) — FIXED

• src/chat/tools/domains/notifications.ts:290-293 — notification_filters_set formatResult: defensive null/!_id branch removed; cast simplified to NotificationFilterRecord.
• src/chat/tools/domains/notifications.ts:418-421 — notification_channel_preferences_set formatResult: same fix.
• execute() returns result.setNotificationFilter / result.setNotificationChannelPreference directly from a typed graphql call, so the prior fallback was indeed unreachable.

NIT-C (no IMPLEMENTATION-STATUS.md entry) — SKIPPED PER INSTRUCTIONS

Acknowledged. Not blocking.

New Findings From This Remediation

None. No new lint/type issues, no unused imports, no test regressions, no unintended scope creep.

Code Quality Observations

• The NOTIFICATION_TYPES const is grouped with the other as const enums at the top of notifications.ts and includes a JSDoc comment naming the backend file as the source of truth. Good practice for future drift detection.
• Order in CLI list does not match backend enum order, but this is a presentation difference only — set equivalence is what matters for an enum constraint, and it is exact.
• Maintenance note (informational, not a finding): if the backend NotificationType enum changes, this list must be updated by hand. A future improvement would be a generated file or a CI check that diffs the two, but a JSDoc pointer is acceptable for now.

Verdict

APPROVE

All previously raised findings (MINOR, NIT-A, NIT-B) are fixed correctly. NIT-C skipped per the documented instructions. Build/lint/type-check/tests all pass on the branch and in CI. No new findings introduced. PR is mergeable.

[nooblemon-eth]

Karen formal review submission note: Attempted to submit a formal GitHub review via gh pr review 205 --approve, but GitHub returned: Review Can not approve your own pull request. This is because the gh CLI on this machine is authenticated as nooblemon-eth, the same account that opened the PR. GitHub does not permit self-approval via the API.

The Karen verdict is APPROVE (see prior comment). To satisfy branch protection's "requires approval" rule, a different reviewer account must click Approve in the GitHub UI, or the gh CLI must be re-authenticated to a non-author account. The verdict itself stands and is fully documented.