chore(notifications): cleanup script for 7 grandfathered free-tier alertRules rows#3486
Merged
Merged
Conversation
…ertRules rows Companion to PR #3483 (server-side mutation gate, layer 2) and PR #3485 (relay fail-closed, layer 3). Disables notifications for the 7 free-tier users that have grandfathered alertRules rows from before the layer-2 gate existed. Mechanism: calls internal.alertRules.setAlertRulesForUser (the INTENTIONALLY-ungated operator path — see PR #3483's contract test) with enabled=false. Preserves sensitivity/channels/eventTypes; only the on/off toggle flips. Affected userIds (verified via 2026-04-28 dry-run): user_3BwtmadXVR2XLnGfIUndMvCNn3S variant=full daily/high planKey=free user_3BybzNDxo0OIm1Q0wseyVPyZZI6 variant=full daily/high planKey=free user_3Bzq0tV0AqS1NGkGfAwkGiloBUW variant=full daily/all planKey=free user_3C0GSJz7gpTcqWdujingRCxxmMs variant=full daily/high planKey=free user_3C2rpobAryOk10hR2BKopVwaupC variant=full daily/all planKey=free user_3C4Y8NV7cVZlysodXrfOQufFBRE variant=full daily/all planKey=free user_3C6K7V9lEzPIH81TdSgthOmf2ZQ variant=full daily/all planKey=free DO NOT MERGE / RUN UNTIL #3483 + #3485 ARE DEPLOYED. Otherwise the same users could re-enable through the still-open mutation surface tomorrow. The user explicitly stated: "close it first, before doing anything to free users." Tier breakdown at time of script authoring: tier 0 (free): 7 users (25%) tier 1 (pro): 18 users tier 2 (pro+): 3 users Dry-run by default. --apply required to mutate. Per-row failures logged + counted; exit 1 on any failure. Idempotent: re-running after apply finds 0 free-tier rows in the enabled set. Audit trail kept committed (matches PR #3463 / #3468 / #3481 pattern).
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
… lookup errors (Greptile P1+P1) Two P1 findings on PR #3486 round 1 — both real, both fixed. P1 #1 (line 142): apply mode wiped eventTypes + channels. setAlertRulesForUser PATCHES (does NOT preserve) eventTypes and channels when they're supplied in args. The previous version sent `eventTypes: [], channels: []` — which would have wiped any user's saved subscriptions/channels, even though the script claimed to "only flip enabled". A user later upgrading to PRO would have to reconfigure from scratch. Concrete impact in the current population: user_3Bzq0tV0AqS1NGkGfAwkGiloBUW has channels=[1] (one configured channel). Old version would have wiped it. Fix: pass through `row.eventTypes ?? []` and `row.channels ?? []` from the existing row (which we already have in scope from getByEnabled). P1 #2 (line 87): silent failure on entitlement lookup. Previous version did `tier: tierMatch ? parseInt(tierMatch[1]) : -1` and then `filter(r => r.tier === 0)` — so a failed `npx convex run`, timeout, or output-format change would silently mark every user as tier=-1, get filtered out as "not free", and exit 0 with "no free users to disable." A regression in the lookup path could mask actual free users behind a green-status script run. Fix: fail-closed on three signals: - result.status !== 0 → FATAL exit code 4 - tier regex no match → FATAL exit code 4 - planKey regex no match → FATAL exit code 4 Includes the last few lines of stdout/stderr in the error so the operator can see the actual failure without re-running. Latent bug also fixed: the previous dedupe-by-userId-during-iteration would have caused multi-variant free users to only have ONE variant disabled. New version: dedupe entitlement LOOKUPS by userId (per-user data, no need to look up twice), but iterate ALL rows for the cleanup target list. Multi-variant users get every variant disabled in one pass. Dry-run on prod still shows the expected 7 rows. New diagnostic surface prints `eventTypes=[N] channels=[N]` per row so the operator sees what's preserved.
koala73
marked this pull request as ready for review
April 28, 2026 06:57
Contributor
Greptile SummaryThis PR adds a one-shot Node.js cleanup script that disables Confidence Score: 4/5Safe to merge as a draft — sequencing gate in PR description is the operative concern, not code correctness Only a P2 style finding (template-literal JSON vs JSON.stringify in the entitlement lookup). Core logic is verified correct against the alertRules.ts implementation: sensitivity is preserved by Convex's partial-patch semantics, eventTypes/channels are correctly passed through, the fail-closed abort path works, and the internalMutation path is reachable via the deploy key. No files require special attention beyond the single P2 comment on line 103 of the script. Important Files Changed
Sequence DiagramsequenceDiagram
participant Script as disable-free-user-notifications.mjs
participant Convex as Convex HTTP (getByEnabled)
participant CLI as npx convex run
participant DB as Convex DB (internalMutation)
Script->>Convex: query alertRules.getByEnabled({enabled:true})
Convex-->>Script: allEnabled rows (28)
loop for each unique userId (deduped)
Script->>CLI: entitlements:getEntitlementsByUserId
CLI-->>Script: {tier, planKey}
alt non-zero exit or unparseable
Script->>Script: process.exit(4) — fail-closed
end
end
Script->>Script: filter freeRows where tier===0 (7 rows)
alt dry-run (no --apply)
Script->>Script: print report, process.exit(0)
else --apply mode
loop for each free row
Script->>CLI: alertRules:setAlertRulesForUser {enabled:false, eventTypes, channels}
CLI->>DB: internalMutation patch (preserves sensitivity, aiDigestEnabled)
DB-->>CLI: ok
CLI-->>Script: exit 0 or exit 1
end
Script->>Script: exit 0 (all ok) or exit 1 (failures logged)
end
Reviews (1): Last reviewed commit: "fix(disable-free-notif): preserve eventT..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (const userId of uniqueUserIds) { | ||
| const result = spawnSync( | ||
| "npx", | ||
| ["convex", "run", "entitlements:getEntitlementsByUserId", `{"userId":"${userId}"}`], |
Contributor
There was a problem hiding this comment.
The entitlement lookup constructs JSON via string interpolation ({"userId":"${userId}"}), while the apply step (line 177) correctly uses JSON.stringify. If a userId ever contains a " or \ character, the template-literal form produces malformed JSON and the Convex CLI will exit non-zero, triggering the fail-closed abort for all users — with a misleading "FATAL entitlement lookup failure" message that hides the real cause.
Suggested change
| ["convex", "run", "entitlements:getEntitlementsByUserId", `{"userId":"${userId}"}`], | |
| ["convex", "run", "entitlements:getEntitlementsByUserId", JSON.stringify({ userId })], |
fuleinist
pushed a commit
to fuleinist/worldmonitor
that referenced
this pull request
May 9, 2026
…ertRules rows (koala73#3486) * chore(notifications): cleanup script for 7 grandfathered free-tier alertRules rows Companion to PR koala73#3483 (server-side mutation gate, layer 2) and PR koala73#3485 (relay fail-closed, layer 3). Disables notifications for the 7 free-tier users that have grandfathered alertRules rows from before the layer-2 gate existed. Mechanism: calls internal.alertRules.setAlertRulesForUser (the INTENTIONALLY-ungated operator path — see PR koala73#3483's contract test) with enabled=false. Preserves sensitivity/channels/eventTypes; only the on/off toggle flips. Affected userIds (verified via 2026-04-28 dry-run): user_3BwtmadXVR2XLnGfIUndMvCNn3S variant=full daily/high planKey=free user_3BybzNDxo0OIm1Q0wseyVPyZZI6 variant=full daily/high planKey=free user_3Bzq0tV0AqS1NGkGfAwkGiloBUW variant=full daily/all planKey=free user_3C0GSJz7gpTcqWdujingRCxxmMs variant=full daily/high planKey=free user_3C2rpobAryOk10hR2BKopVwaupC variant=full daily/all planKey=free user_3C4Y8NV7cVZlysodXrfOQufFBRE variant=full daily/all planKey=free user_3C6K7V9lEzPIH81TdSgthOmf2ZQ variant=full daily/all planKey=free DO NOT MERGE / RUN UNTIL koala73#3483 + koala73#3485 ARE DEPLOYED. Otherwise the same users could re-enable through the still-open mutation surface tomorrow. The user explicitly stated: "close it first, before doing anything to free users." Tier breakdown at time of script authoring: tier 0 (free): 7 users (25%) tier 1 (pro): 18 users tier 2 (pro+): 3 users Dry-run by default. --apply required to mutate. Per-row failures logged + counted; exit 1 on any failure. Idempotent: re-running after apply finds 0 free-tier rows in the enabled set. Audit trail kept committed (matches PR koala73#3463 / koala73#3468 / koala73#3481 pattern). * fix(disable-free-notif): preserve eventTypes+channels, fail-closed on lookup errors (Greptile P1+P1) Two P1 findings on PR koala73#3486 round 1 — both real, both fixed. P1 #1 (line 142): apply mode wiped eventTypes + channels. setAlertRulesForUser PATCHES (does NOT preserve) eventTypes and channels when they're supplied in args. The previous version sent `eventTypes: [], channels: []` — which would have wiped any user's saved subscriptions/channels, even though the script claimed to "only flip enabled". A user later upgrading to PRO would have to reconfigure from scratch. Concrete impact in the current population: user_3Bzq0tV0AqS1NGkGfAwkGiloBUW has channels=[1] (one configured channel). Old version would have wiped it. Fix: pass through `row.eventTypes ?? []` and `row.channels ?? []` from the existing row (which we already have in scope from getByEnabled). P1 #2 (line 87): silent failure on entitlement lookup. Previous version did `tier: tierMatch ? parseInt(tierMatch[1]) : -1` and then `filter(r => r.tier === 0)` — so a failed `npx convex run`, timeout, or output-format change would silently mark every user as tier=-1, get filtered out as "not free", and exit 0 with "no free users to disable." A regression in the lookup path could mask actual free users behind a green-status script run. Fix: fail-closed on three signals: - result.status !== 0 → FATAL exit code 4 - tier regex no match → FATAL exit code 4 - planKey regex no match → FATAL exit code 4 Includes the last few lines of stdout/stderr in the error so the operator can see the actual failure without re-running. Latent bug also fixed: the previous dedupe-by-userId-during-iteration would have caused multi-variant free users to only have ONE variant disabled. New version: dedupe entitlement LOOKUPS by userId (per-user data, no need to look up twice), but iterate ALL rows for the cleanup target list. Multi-variant users get every variant disabled in one pass. Dry-run on prod still shows the expected 7 rows. New diagnostic surface prints `eventTypes=[N] channels=[N]` per row so the operator sees what's preserved.
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
1 participant
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.
Sequencing matters. The user explicitly stated: "close it first, before doing anything to free users."
Running this cleanup before the gap closes lets the same 7 users re-enable through the still-open mutation surface tomorrow. The two gating PRs must merge AND deploy before this script runs.
Summary
Companion to:
Disables notifications for the 7 free-tier users that have grandfathered
alertRulesrows from before the layer-2 gate existed (audit on 2026-04-28).Affected userIds (verified via dry-run)
Tier breakdown at audit time: tier 0 = 7 users (25%) · tier 1 = 18 users · tier 2 = 3 users.
Mechanism
Calls
internal.alertRules.setAlertRulesForUser— the INTENTIONALLY-ungated operator path pinned by a contract test in #3483. Patchesenabled: false; preservessensitivity/channels/eventTypesso the row's structural shape is untouched. If a user later upgrades to PRO and wants notifications back, they can re-enable from the UI.Run order (post-merge of #3483 + #3485)
Test plan
node -csyntax check cleanWhy a committed script + draft PR vs running the mutation manually
Audit trail. Committed script + commit message + PR description = operator-facing record of exactly which userIds were touched and why. Pattern matches PR #3463 / #3468 / #3481.