feat(chat/runs): alert-only zombie-owner check on scheduled runs - #783
feat(chat/runs): alert-only zombie-owner check on scheduled runs#783sweetmantech wants to merge 1 commit into
Conversation
`handleStartChatRun` starts scheduled runs with no check that a human still uses the account, so an abandoned account keeps generating (and billing) long after the owner left (recoupable/chat#1885). Fire a DEDUPED Telegram alert when the owner's last `role='user'` message is older than 45 days (or they've never sent one). The run is NOT blocked — this is alert-only. - `getLatestUserMessageAt` (`lib/supabase/chat_messages/`) — reads the owner's most recent `role='user'` message via `chat_messages → chats → sessions` inner embeds filtered on `sessions.account_id`. - `isOwnerInactive` — pure 45-day threshold predicate (null → inactive). - `markZombieOwnerAlerted` — per-owner Redis `SET NX EX` dedup marker (30-day window) so daily runs don't spam; fails open on Redis error. - `alertZombieOwner` — orchestrates the three + the Telegram send; never throws. - `handleStartChatRun` — schedules the check via `after()` so it never blocks the 202 or the run. TDD: RED→GREEN for the >45d detection, the dedup marker, the orchestrator, and the handler wiring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (5)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Confidence score: 3/5
- In
lib/chat/runs/alertZombieOwner.ts, claiming the dedup marker beforesendMessagemeans a transient Telegram error can block owner alerts for the full dedup window, so real zombie runs may go unreported until the window expires — release/rollback the claimed marker when send fails (or only claim after a successful send). - In
lib/chat/runs/__tests__/handleStartChatRun.test.ts, the handler’safter()path callsalertZombieOwner(...)without awaiting or handling its Promise, so a rejection can surface as an unhandled promise rejection and create flaky behavior or process-level warnings/failures — await it or attach explicit error handling in the callback.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/chat/runs/alertZombieOwner.ts">
<violation number="1" location="lib/chat/runs/alertZombieOwner.ts:36">
P2: A transient Telegram failure suppresses this owner's alert for the full dedup window: the marker is claimed before the send, then retained when `sendMessage` rejects. Consider releasing the claimed marker on send failure (or otherwise marking only successful delivery) so a later scheduled run can retry.</violation>
</file>
<file name="lib/chat/runs/__tests__/handleStartChatRun.test.ts">
<violation number="1" location="lib/chat/runs/__tests__/handleStartChatRun.test.ts:120">
P2: The `after()` callback in the handler fires `alertZombieOwner(...)` and discards its Promise. When the `alertZombieOwner` mock rejects (as the second test does), the rejection becomes an unhandled promise rejection in the test environment, which can cause test flakiness or warnings in stricter runners. Add `.catch(() => {})` to the `alertZombieOwner` call inside `after()` so both production and test environments are safe regardless of the implementation contract.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Scheduler as External Scheduler
participant API as handleStartChatRun
participant After as after() Hook
participant Alert as alertZombieOwner
participant DB as Supabase
participant Redis as Redis
participant Telegram as Telegram Bot API
Note over Scheduler,Telegram: Zombie-Owner Alert Flow for Scheduled Runs
Scheduler->>API: POST /api/chat/runs
API->>API: validateChatRunRequest()
API->>API: provisionRunSession()
API->>API: start workflow with accountId, chatId, sessionId
API-->>Scheduler: 202 Accepted (run started)
Note over API,After: Post-response (non-blocking via after())
API->>After: schedule alertZombieOwner(accountId, chatId, sessionId)
After->>Alert: alertZombieOwner(params)
Alert->>DB: getLatestUserMessageAt(accountId)
Note over Alert,DB: chat_messages → chats → sessions via PostgREST embeds
DB-->>Alert: latest user message timestamp or null
alt Owner never messaged OR message > 45 days old
Alert->>Alert: isOwnerInactive() → true
Alert->>Redis: markZombieOwnerAlerted(accountId)
Note over Alert,Redis: SET zombie-owner-alert:{id} 1 EX 2592000 NX
alt First alert in dedup window (Redis returns OK)
Redis-->>Alert: true
Alert->>Telegram: sendMessage(zombie alert text)
Note over Alert,Telegram: Deduped markdown message with account/chat/session details
Telegram-->>Alert: sent
else Marker already exists (duplicate)
Redis-->>Alert: null
Note over Alert,Alert: Skip - already alerted this window
end
else Owner active (message within 45 days)
Note over Alert,Alert: Skip - owner still active
end
opt Redis error (infra blip)
Redis-->>Alert: throws
Note over Alert: Fails open - returns true to avoid missing real alert
Alert->>Telegram: sendMessage(zombie alert text)
end
opt DB error
DB-->>Alert: error
Note over Alert: Logs error, returns null (safe default)
Note over Alert,Alert: getLatestUserMessageAt returns null → treats as never messaged
end
opt Any dependency throws
Note over Alert: Surrounding try/catch catches and logs
Note over Alert,Alert: Never throws - run continues unaffected
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (!isOwnerInactive(lastUserMessageAt, new Date())) return; | ||
|
|
||
| // Dedup before sending so repeated scheduled runs don't spam. | ||
| const shouldSend = await markZombieOwnerAlerted(accountId); |
There was a problem hiding this comment.
P2: A transient Telegram failure suppresses this owner's alert for the full dedup window: the marker is claimed before the send, then retained when sendMessage rejects. Consider releasing the claimed marker on send failure (or otherwise marking only successful delivery) so a later scheduled run can retry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/runs/alertZombieOwner.ts, line 36:
<comment>A transient Telegram failure suppresses this owner's alert for the full dedup window: the marker is claimed before the send, then retained when `sendMessage` rejects. Consider releasing the claimed marker on send failure (or otherwise marking only successful delivery) so a later scheduled run can retry.</comment>
<file context>
@@ -0,0 +1,56 @@
+ if (!isOwnerInactive(lastUserMessageAt, new Date())) return;
+
+ // Dedup before sending so repeated scheduled runs don't spam.
+ const shouldSend = await markZombieOwnerAlerted(accountId);
+ if (!shouldSend) return;
+
</file context>
| }); | ||
|
|
||
| it("does not block the run when the zombie-owner alert throws", async () => { | ||
| vi.mocked(alertZombieOwner).mockRejectedValueOnce(new Error("alert boom")); |
There was a problem hiding this comment.
P2: The after() callback in the handler fires alertZombieOwner(...) and discards its Promise. When the alertZombieOwner mock rejects (as the second test does), the rejection becomes an unhandled promise rejection in the test environment, which can cause test flakiness or warnings in stricter runners. Add .catch(() => {}) to the alertZombieOwner call inside after() so both production and test environments are safe regardless of the implementation contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/runs/__tests__/handleStartChatRun.test.ts, line 120:
<comment>The `after()` callback in the handler fires `alertZombieOwner(...)` and discards its Promise. When the `alertZombieOwner` mock rejects (as the second test does), the rejection becomes an unhandled promise rejection in the test environment, which can cause test flakiness or warnings in stricter runners. Add `.catch(() => {})` to the `alertZombieOwner` call inside `after()` so both production and test environments are safe regardless of the implementation contract.</comment>
<file context>
@@ -98,6 +107,23 @@ describe("handleStartChatRun", () => {
+ });
+
+ it("does not block the run when the zombie-owner alert throws", async () => {
+ vi.mocked(alertZombieOwner).mockRejectedValueOnce(new Error("alert boom"));
+
+ const res = await handleStartChatRun(req());
</file context>
1 participant
What
handleStartChatRunstarts scheduled runs with no check that a human still uses the account — an abandoned account keeps generating (and billing) long after the owner left (recoupable/chat#1885, "Split out — billing & engagement integrity").This fires a deduped Telegram alert when the owner's last
role='user'message is older than 45 days (or they've never sent one). The run is NOT blocked — alert-only, per the issue spec.Changes
getLatestUserMessageAt(lib/supabase/chat_messages/) — reads the owner's most recentrole='user'message, walkingchat_messages → chats → sessionsvia PostgREST inner embeds filtered onsessions.account_id.isOwnerInactive— pure 45-day threshold predicate;null(never messaged) counts as inactive; exactly-at-threshold is still active.markZombieOwnerAlerted— per-owner RedisSET NX EXdedup marker (30-day window) so daily scheduled runs don't spam; fails open on a Redis error so an infra blip never silences a real alert.alertZombieOwner— orchestrates the read → detect → dedup → Telegram send; never throws.handleStartChatRun— schedules the check viaafter()so it never blocks the 202 or the run.Scope note
Detection uses
chat_messages.role(the durable workflow chat surface the scheduled-run journey writes to) — the table with an explicitrolecolumn the issue references.Verification
isOwnerInactive— null→inactive, >45d→inactive, <45d/at-threshold/today→active, threshold is 45.markZombieOwnerAlerted— SET NX+EX returns true first time, false when marker exists, fails open on Redis error.alertZombieOwner— alerts on >45d and on never-messaged, silent when active, deduped when marker claimed, never throws on dependency failure.handleStartChatRun— fires the alert with the run's owner/chat/session after start; a throwing alert does not block the 202.lib/chat/runsdomain suite green — 35 tests passing.tsc --noEmit: no errors in changed files.eslint+prettierclean on all changed files.Targets main.
Relates to recoupable/chat#1885 (split-out billing & engagement integrity).
🤖 Generated with Claude Code
Summary by cubic
Adds an alert-only zombie-owner check for scheduled chat runs. If the owner hasn’t sent a user message in 45+ days (or never), we send a deduped Telegram alert after the run starts without blocking it.
getLatestUserMessageAt: reads the owner’s latestrole='user'message via Supabase embeds (chat_messages → chats → sessionsfiltered bysessions.account_id).isOwnerInactive: pure 45-day predicate;nullcounts inactive; exactly 45 days is still active.markZombieOwnerAlerted: per-owner Redis dedup (SET NX EX, 30-day TTL); fails open on Redis errors to avoid missing real alerts.alertZombieOwner: orchestrates read → detect → dedup → Telegram send; never throws. Wired intohandleStartChatRunvia Nextafter()so it never delays the 202 or the run.Written for commit 5117ce8. Summary will update on new commits.