Libra CodeHub

CedarCopilot/cedar-mail

Branch: staging

fix(settings): close admin write bypass + false Slack-delivery summaries

merged#2739CedarCopilot

CedarCopilot wants to merge 1 commit into staging from fix/settings-merge-hardening

Live on prod, no production signal yetTimeline and evidence
  1. Opened
    Sep 9, 2026, 8:44 AM
  2. Sep 9, 2026, 8:52 AM
  3. Merged
    Sep 9, 2026, 1:06 PM
  4. Live on prod
    Sep 9, 2026, 1:06 PM
  5. Observed 2 days
    Sep 9, 2026, 1:06 PM
  6. Pipelines steady after this deploy
    Sep 9, 2026, 1:06 PM
  7. Unobserved

    Live on prod, no production signal yet

    Sep 11, 2026, 9:44 PM

Behaviors Libra is checking

Partial admin notification updates change only the specified Slack or email sub-keys while preserving all existing sibling settings and independently applying patches to both notification groups.Inconclusivelow confidence

Verification reached its budget before a verdict.

prod, checked Sep 11, 2026, 8:41 PM
Admin updates to a user's Slack or email notification settings are written through the shared user-settings writer to the canonical notification settings column that notification resolution reads.Inconclusivelow confidence

Verification reached its budget before a verdict.

prod, checked Sep 11, 2026, 8:42 PM
Meeting-prep agents report that a preparation was posted to Slack only when notify returns deliveredOn=slack, and explicitly report email fallback and a Slack-connection issue when deliveredOn=email despite success=true.Inconclusivehigh confidence

No production telemetry tied to the MEETING_PREP notify behavior was found since 2026-09-09T20:06:19Z: zero meeting-prep notify spans, zero matching CloudWatch logs, and zero notify arguments containing the meeting-prep headline. The notify-user surface had only 4 unrelated.

prod, checked Sep 11, 2026, 9:42 PM
Every successful user-settings update emits a structured audit record identifying the target user, caller, and changed settings groups and keys without logging setting values.Inconclusivelow confidence

Verification reached its 120s wall-clock budget before a tied verdict. Libra is keeping this intent verifying until the next check.

prod, checked Sep 11, 2026, 9:44 PM

Libra has verdicts on 0 of 4 tracked behaviors on prod; 4 are still being checked. Libra checks hourly for 3 days after each deploy.

Summary

Prevention follow-up from a live investigation into why Kendall Keahey's (and two other real accounts', at Anysphere and Strac) notification_settings.slackNotifications lost its enabled key around 2026-08-24/25 , every notify-user call since then silently fell back to email, with nothing thrown, nothing logged, and agent_executions.summary confidently narrating "posted to #cedar-channel" the whole time.

The core merge bug (jsonb || jsonb replacing a whole nested group on a one-sub-key patch) was already fixed and covered by user-settings-merge.test.ts on 2026-08-25 (878a3e77b). This PR closes a remaining bypass of that fix, adds an audit trail so a recurrence is traceable, and fixes a structural gap that let the incident go unnoticed for two weeks.

  • adminNotificationsRouter.updateUserNotificationSettings wrote the legacy settings blob directly instead of the dedicated notification_settings column that resolveUserSettings actually prefers once populated (every user since the 2026-08-20 dual-write). A write here reported {success: true} while having zero effect on resolved behavior , an admin "fixing" a user's Slack/email settings through this panel could not actually fix anything. Now routes through the shared updateUserSettings writer, passing only the caller-specified sub-keys (spreading defaults back in would itself risk clobbering an untouched sibling value , the same failure shape as the original bug).
  • updateUserSettings had zero logging on write, so this incident required a multi-hour SQL forensic reconstruction instead of an Axiom query. Added one minimal structured log (userSettings.updated) recording which keys were patched per column, per user , never values.
  • meeting-prep.ts's delivery instructions told the agent to check only notify's success field. success: true is also what a silent email fallback returns, which is exactly why execution summaries kept claiming Slack delivery that never happened. Now checks deliveredOn and narrates the fallback plainly when it occurs. (daily-agenda.ts already did this correctly; crm-updater.ts/pipeline-review.ts/drafter.ts don't have a success-checking instruction to begin with, so no fix needed there.)

Out of scope (flagged, not fixed here)

db/migrations/scripts/reconcile-categorized-settings.ts exists specifically to close the underlying migration gap (users still on the legacy blob after the 2026-08-20 dual-write reading empty on the dedicated column) but is a manual CLI script, not wired to any cron , the two real corrupted accounts besides Kendall's (Anysphere, Strac) still need a human to run it. Wiring it into an automatic schedule would remove the human judgment gate the script's author deliberately built in for activation keys (agentExecutionEnabled etc.), so that's a separate decision, not something to fold into a "prevention" PR silently.

Test plan

  • user-settings-merge.test.ts (existing, 8 tests) + user-settings.test.ts (existing) + new admin-notifications-update-settings.test.ts (4 tests) , all pass (35/35)
  • agent-defaults.test.ts , pass (unaffected by the meeting-prep prose change)
  • pnpm --filter @zero/server run types , clean

šŸ¤– Generated with Claude Code

https://claude.ai/code/session_01YZB5RgfxHLmuKtWof2wP4g

Greptile Summary

This PR routes administrative notification-setting changes through the dedicated categorized-settings writer, adds structured write auditing, and updates meeting-prep delivery instructions to distinguish Slack delivery from fallback delivery.

  • Prevents the admin route from writing only the obsolete legacy settings blob.
  • Preserves unspecified nested notification settings through the shared deep-merge writer.
  • Adds caller-attributed, value-free settings-write logs.
  • Improves delivery summaries by consulting `delivered
Show production surfaces and changed-file mapping

Production surfaces

Libra has not measured any production surfaces for this change yet.

Changed files → surfaces

  • apps/server/src/services/playbook/agent-defaults/meeting-prep.tsno production surface mapped
  • apps/server/src/services/users/user-settings.tsno production surface mapped
  • apps/server/src/trpc/routes/__tests__/admin-notifications-update-settings.test.tsno production surface mapped
  • apps/server/src/trpc/routes/admin-notifications.tsno production surface mapped