Skip to content

fix(glanceable): rate-limit native updates to stop device overheating - #6323

Open
iscekic wants to merge 6 commits into
mainfrom
kwf/req-20260918-044333-77b6
Open

iscekic wants to merge 6 commits into
mainfrom
kwf/req-20260918-044333-77b6

Conversation

@iscekic

@iscekic iscekic commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • The phone no longer heats up during long runs of 5–15 remote CLI sessions, because the glanceable surfaces stop updating on every heartbeat.
  • The widget, Live Activity, and ongoing notification still show live counts; counts-only changes may lag by up to 10 seconds.
  • Approval prompts appear and clear immediately — including when a disconnected CLI's stored permission is cleared later — so the Approve control is never delayed by the update window.
  • Only the account whose session moved into or out of an approval prompt bypasses the update window; other accounts in the same batch keep the normal cadence.
  • Renaming the newest session still redraws the surface promptly.
  • A Live Activity that failed to start is retried within seconds while the counts stay stable; a surface whose write keeps failing is retried at most once every five minutes instead of on every heartbeat.
  • A deferred counts update is retried even when a newer revision supersedes it, after a failed build, or after a failed transport; its waits grow from 10 seconds to a five-minute ceiling so a permanently failing route settles.
  • A counts update that a newer delivery already covered no longer wakes the device twice, and an ongoing notification left after the app is killed is dismissed on the next launch.

Changelog for maintainers

  • services/notifications/src/lib/glanceable-refresh.ts:128 — accepted: both re-arm sites retried a permanently failing build or transport every 10 seconds forever. The pending record now carries an attempt count, and the re-arm delay doubles from one window to a five-minute ceiling. A landed delivery or a fresh deferral resets the count.
  • services/notifications/src/lib/glanceable-refresh.ts:424 — accepted: the flush catch re-armed the consumed record with a separate get and put, so a delivery landing between them left a record that later fired a redundant wake. The catch now re-arms through the same transaction-hardened helper the trailing refresh uses, seeded with the delivery record read before the refresh started. That helper preserves the consumed deferral's write time and leaves a newer deferral alone.
  • services/notifications/src/lib/glanceable-refresh.test.ts:1193 — accepted: the seeded pending record was not due, so a successful flush also left it untouched and the assertions could not tell a failed flush from a successful one. The test now counts the rejected read and asserts the guard's content-free warning fired.
  • apps/mobile/src/lib/glanceable/publisher.ts:354 — accepted: emit advanced the published deadline before the sinks ran, so a rejected renewal suppressed the retry for a full 15-minute margin. The deadline now advances only when every sink write lands; a rejected write raises a failure count that spaces the retry with a doubling backoff capped at five minutes.
  • apps/mobile/src/lib/glanceable/publisher.ts:195 — accepted: after a process death inside the 8-second terminal window, the persisted empty snapshot made the first heartbeat a no-op, so the orphaned Android notification stayed in the shade. The first write in a process now always reaches the sinks, whose reconciliation dismisses the stale card.
  • services/session-ingest/src/dos/UserConnectionDO.ts:1950 — accepted: the comment claimed the disconnect reset writes the DB, but the clear is held for the CLI absence window and the exempt delivery fired at socket close. The comment now describes the aggregate drop, and the deferred permission-to-retry write issues its own approval-exempt refresh for a session that stays snapshot-visible. The disconnect caller also names the owning root session id, so naming a permission subagent's own id cannot drop the owning scope's exemption.
  • services/notifications/src/index.ts:354 — accepted (earlier round): one request-level flag exempted every scope in a multi-org batch, including scopes whose sessions never touched permission. The refresh RPC now takes approvalChangedSessionIds, and the service exempts only the scopes those ids resolve to.
  • apps/mobile/src/lib/glanceable/publisher.ts:201 — accepted (earlier round): an unchanged-content renewal emitted a newer frame while a coalesced timer held an older snapshot, so the timer later republished the older revision and timestamp. The renewal now cancels the pending coalesce before emitting; the publisher also skips the native write when the visible content is unchanged, raises the coalesce window from 1 s to 10 s, and re-issues the Live Activity start or update at the stale-window margin.
  • services/session-ingest/src/ingest/metadata.test.ts:620 — accepted (earlier round): the added tests moved question -> permission, which is still true through the session.status clause, so the previousStatus === 'permission' branch was unverified. A permission -> busy case now covers the clearing direction.
  • Delivery rate-limits to one aggregate device wake per account scope per 10 seconds; a change inside the window becomes a pending record the Durable Object alarm delivers as a trailing refresh. The flush runs behind a guard that contains a rejected storage read with a content-free warning, so the sweep's reclamation still runs and the pending-deadline fold reschedules the delivery. Content-free build and delivery logs make the one-build-per-window invariant auditable.
  • Review first the re-arm transaction and retry backoff in glanceable-refresh.ts, the renewal gate and first-write reconciliation in publisher.ts, and the deferred disconnect refresh in metadata.ts; each decides whether a device is woken or its deferred counts are stranded.

E2E proof

Captured 2026-09-18. The backend run appended below replaces the earlier log excerpts and is the replacement evidence for the retry and delivery changes; it ran on an unrecorded platform. The request asked for Android and iOS. The CLI check did not start and is not counted. The captures are Android; no iOS capture ran. The iOS capture is skipped and pending owner verification.

[e3] No remote CLI sessions (empty tray): Agents tab shows the empty state and no live count — prior/e3.png

[e8] ux-check: move a session between question and permission; the Approve control must appear/disappear within ~1s, not the 10s window — prior/e8-shade-approve.png

[e8] ux-check: move a session between question and permission; the Approve control must appear/disappear within ~1s, not the 10s window — prior/e8-shade-question.png

[e3] No remote CLI sessions (empty tray): Agents tab shows the empty state and no live count — scripted-shard1/e3.png

Owner request

The phone gets unbearably hot during normal use, with 5 to 15 remote CLI sessions running. Confirm the cause through debug logs or profiling, then fix.

E2E proof — log excerpts

INTERNAL_API_SECRET missing; skipping glanceable aggregate delivery
 ✓ src/lib/glanceable-refresh.test.ts (7 tests) 117ms
Using secrets defined in .dev.vars
 ✓ src/lib/scheduled-action-push.test.ts (14 tests) 89ms
Using secrets defined in .dev.vars
 ✓ src/lib/notifications-service-cloud-agent.test.ts (35 tests) 62ms
Using secrets defined in .dev.vars
 ✓ src/__tests__/routes-dispatch.test.ts (7 tests) 93ms
Using secrets defined in .dev.vars
 ✓ src/lib/apns-live-activity.test.ts (11 tests) 39ms
Using secrets defined in .dev.vars
 ✓ src/lib/agent-session-notification-push.test.ts (16 tests) 61ms
Using secrets defined in .dev.vars
 ✓ src/lib/notifications-service.test.ts (15 tests) 31ms
Using secrets defined in .dev.vars
 ✓ src/__tests__/send-push-for-conversation.test.ts (6 tests) 74ms
Using secrets defined in .dev.vars
 ✓ src/lib/push-sink.test.ts (5 tests) 11ms
 ✓ src/lib/internal-dispatch-push.test.ts (24 tests) 25ms
 ✓ src/queue-consumer.test.ts (1 test) 11ms
 Test Files  17 passed (17)
      Tests  319 passed (319)
   Start at  05:31:33
   Duration  46.32s (transform 15.24s, setup 693ms, import 95.97s, tests 12.54s, environment 7ms)

@iscekic
iscekic marked this pull request as draft September 18, 2026 18:26
@iscekic
iscekic force-pushed the kwf/req-20260918-044333-77b6 branch from 5debca9 to 62202ed Compare September 18, 2026 19:31
@iscekic
iscekic marked this pull request as ready for review September 18, 2026 19:52
Comment thread apps/mobile/src/lib/glanceable/publisher.ts Outdated
Comment thread services/notifications/src/lib/glanceable-refresh.ts
Comment thread services/notifications/src/lib/glanceable-refresh.ts
Comment thread services/notifications/src/dos/NotificationChannelDO.ts
@kilo-code-bot

kilo-code-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • apps/mobile/src/lib/glanceable/publisher.ts
  • apps/mobile/src/lib/glanceable/publisher.test.ts
Previous Review Summaries (8 snapshots, latest commit be9678c)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit be9678c)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The new write-failure backoff only gates the renewal branch, so a rejected write on a content-change heartbeat still waits for the 15-minute renewal margin; all six findings from the previous round are verified fixed at be9678c.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/mobile/src/lib/glanceable/publisher.ts 407 isRenewalDue's margin short-circuit runs before the writeFailures backoff, so a rejected content-change emit is not retried for up to ~15 minutes
Files Reviewed (8 files)
  • apps/mobile/src/lib/glanceable/publisher.ts - 1 issue
  • apps/mobile/src/lib/glanceable/publisher.test.ts
  • apps/mobile/src/lib/glanceable/sink-registry.ts
  • services/notifications/src/lib/glanceable-refresh.ts
  • services/notifications/src/lib/glanceable-refresh.test.ts
  • services/session-ingest/src/dos/UserConnectionDO.ts
  • services/session-ingest/src/ingest/metadata.ts
  • services/session-ingest/src/ingest/metadata.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 52a8712)

Status: 6 Issues Found | Recommendation: Address before merge

Executive Summary

The new per-scope delivery-window machinery has no terminal condition: a permanently failing snapshot build or transport re-arms a trailing refresh every 10 seconds indefinitely (notifications service), and the mobile renewal path advances its publish timestamp even when every sink call fails; five lower-severity edge cases and one weak failure-path test round out the review. All four findings from the previous round were accepted and verified fixed at HEAD.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 5
Issue Details (click to expand)

WARNING

File Line Issue
services/notifications/src/lib/glanceable-refresh.ts 128 Trailing-refresh re-arm has no retry cap, backoff, or TTL — a permanently failing build/transport loops a DO alarm + build every 10s per scope indefinitely

SUGGESTION

File Line Issue
services/notifications/src/lib/glanceable-refresh.ts 424 Flush catch re-arm is a non-transactional get→put, the interleave rearmTrailingRefresh hardened against — can resurrect a covered deferral for one redundant wake
apps/mobile/src/lib/glanceable/publisher.ts 354 emit advances lastPublishedAt before sinks run, so a failed renewal emit delays the retry by a full 15-minute margin and can lapse the stale date
apps/mobile/src/lib/glanceable/publisher.ts 195 After process death inside the 8s terminal window, the restarted publisher's unchanged-empty gate skips publish, so the orphaned Android ongoing notification is never dismissed
services/session-ingest/src/dos/UserConnectionDO.ts 1950 Disconnect exemption comment misdescribes the mechanism; for a DB-visible session the exempt delivery fires while permission is still held, and the actual clear 10 min later triggers no refresh
services/notifications/src/lib/glanceable-refresh.test.ts 1193 DO-level GC-failure test cannot detect a removed guard: the seeded record is not due and the sweep-failure warn is never asserted
Files Reviewed (15 files)
  • services/notifications/src/lib/glanceable-refresh.ts - 2 issues
  • apps/mobile/src/lib/glanceable/publisher.ts - 2 issues
  • services/session-ingest/src/dos/UserConnectionDO.ts - 1 issue
  • services/notifications/src/lib/glanceable-refresh.test.ts - 1 issue
  • apps/mobile/src/lib/glanceable/publisher.test.ts
  • apps/mobile/src/lib/glanceable/snapshot-transforms.ts
  • packages/app-shared/src/glanceable-agents-snapshot.ts
  • packages/notifications/src/rpc-schemas.ts
  • packages/notifications/src/rpc-schemas.test.ts
  • services/notifications/src/dos/NotificationChannelDO.ts
  • services/notifications/src/index.ts
  • services/notifications/src/lib/glanceable-delivery.test.ts
  • services/session-ingest/src/dos/UserConnectionDO.test.ts
  • services/session-ingest/src/ingest/metadata.test.ts
  • services/session-ingest/src/ingest/metadata.ts

Fix these issues in Kilo Cloud

Previous review

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The approval-exemption hint is a single request-level flag applied to every account scope, so a permission move on one session bypasses the rate-limit window on unrelated scopes; three lower-severity ordering and coverage gaps round out the review.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 3
Issue Details (click to expand)

WARNING

File Line Issue
services/notifications/src/index.ts 354 approvalChanged is forwarded to every resolved scope, so a permission move on one session exempts unrelated scopes from the per-scope delivery window

SUGGESTION

File Line Issue
apps/mobile/src/lib/glanceable/publisher.ts 201 Renewal emit can race a pending coalesced emit and republish an older revision/updatedAt
services/notifications/src/lib/glanceable-refresh.ts 146 Trailing null-build re-arm ignores supersession, allowing a redundant device wake after a newer delivery
services/session-ingest/src/ingest/metadata.test.ts 620 New tests do not cover the permission-clearing half of the approvalChanged expression
Files Reviewed (15 files)
  • apps/mobile/src/lib/glanceable/publisher.ts - 1 issue
  • apps/mobile/src/lib/glanceable/publisher.test.ts
  • apps/mobile/src/lib/glanceable/snapshot-transforms.ts
  • packages/app-shared/src/glanceable-agents-snapshot.ts
  • packages/notifications/src/rpc-schemas.ts
  • packages/notifications/src/rpc-schemas.test.ts
  • services/notifications/src/lib/glanceable-refresh.ts - 1 issue
  • services/notifications/src/lib/glanceable-refresh.test.ts
  • services/notifications/src/lib/glanceable-delivery.test.ts
  • services/notifications/src/dos/NotificationChannelDO.ts
  • services/notifications/src/index.ts - 1 issue
  • services/session-ingest/src/dos/UserConnectionDO.ts
  • services/session-ingest/src/dos/UserConnectionDO.test.ts
  • services/session-ingest/src/ingest/metadata.ts
  • services/session-ingest/src/ingest/metadata.test.ts - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit fe39011)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental changes re-arm a failed trailing glanceable refresh so deferred counts are not lost, and make the UserConnectionDO storage fake model its alarm; the two previously reported issues are addressed on the changed lines and no new issues were found.

Files Reviewed (3 files)
  • services/notifications/src/lib/glanceable-refresh.ts
  • services/notifications/src/lib/glanceable-refresh.test.ts
  • services/session-ingest/src/dos/UserConnectionDO.test.ts

Previous review (commit c09f32f)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/notifications/src/lib/glanceable-refresh.ts 319 A trailing delivery that throws drops the already-consumed pending deferral with no re-arm, so the deferred final counts can be lost.

SUGGESTION

File Line Issue
services/session-ingest/src/dos/UserConnectionDO.test.ts 655 The fake DO storage omits getAlarm, so the new deferral test throws internally and passes without verifying the alarm is armed.
Files Reviewed (15 files)
  • apps/mobile/src/lib/glanceable/publisher.ts
  • apps/mobile/src/lib/glanceable/publisher.test.ts
  • apps/mobile/src/lib/glanceable/snapshot-transforms.ts
  • packages/app-shared/src/glanceable-agents-snapshot.ts
  • packages/notifications/src/rpc-schemas.ts
  • packages/notifications/src/rpc-schemas.test.ts
  • services/notifications/src/index.ts
  • services/notifications/src/dos/NotificationChannelDO.ts
  • services/notifications/src/lib/glanceable-refresh.ts - 1 issue
  • services/notifications/src/lib/glanceable-refresh.test.ts
  • services/notifications/src/lib/glanceable-delivery.test.ts
  • services/session-ingest/src/dos/UserConnectionDO.ts
  • services/session-ingest/src/dos/UserConnectionDO.test.ts - 1 issue
  • services/session-ingest/src/ingest/metadata.ts
  • services/session-ingest/src/ingest/metadata.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 5a4d8db)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • services/notifications/src/lib/glanceable-refresh.ts
  • services/notifications/src/lib/glanceable-refresh.test.ts

Previous review (commit f990f3d)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
services/notifications/src/lib/glanceable-refresh.ts 64 An approval-exempt in-window delivery can delete a counts-only deferral written while it is in flight, dropping the final counts until the next status change.
Files Reviewed (12 files)
  • apps/mobile/src/lib/glanceable/publisher.ts
  • apps/mobile/src/lib/glanceable/publisher.test.ts
  • packages/notifications/src/rpc-schemas.ts
  • packages/notifications/src/rpc-schemas.test.ts
  • services/notifications/src/index.ts
  • services/notifications/src/dos/NotificationChannelDO.ts
  • services/notifications/src/lib/glanceable-refresh.ts - 1 issue
  • services/notifications/src/lib/glanceable-refresh.test.ts
  • services/session-ingest/src/dos/UserConnectionDO.ts
  • services/session-ingest/src/dos/UserConnectionDO.test.ts
  • services/session-ingest/src/ingest/metadata.ts
  • services/session-ingest/src/ingest/metadata.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 62202ed)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The new same-content early return in the mobile publisher permanently stops retrying startOrUpdate (a Live Activity can stay absent), and the backend trailing-refresh drops its pending record on the production null-build path so deferred counts can be lost.

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/glanceable/publisher.ts 153 Unchanged-content early return only publishes, never startOrUpdate, so a failed/deferred Live Activity start is never retried while counts are stable.
services/notifications/src/lib/glanceable-refresh.ts 252 Pending record is deleted before the trailing refresh; production buildSnapshot returns null (no throw), so the final counts are dropped with no re-arm.
services/notifications/src/lib/glanceable-refresh.ts 60 The server-side 10 s window has no exemption for needsApproval, so on the locked/background path the Approve control can lag up to a full window.

SUGGESTION

File Line Issue
services/notifications/src/dos/NotificationChannelDO.ts 455 The glanceable deadline is captured once; a concurrent deferral during alarm()'s awaits can be overwritten by the final setAlarm, delaying the trailing delivery.
Files Reviewed (9 files)
  • apps/mobile/src/lib/glanceable/publisher.ts - 1 issue
  • apps/mobile/src/lib/glanceable/publisher.test.ts
  • apps/mobile/src/lib/glanceable/snapshot-transforms.ts
  • packages/app-shared/src/glanceable-agents-snapshot.ts
  • services/notifications/src/dos/NotificationChannelDO.ts - 1 issue
  • services/notifications/src/lib/glanceable-delivery.test.ts
  • services/notifications/src/lib/glanceable-refresh.test.ts
  • services/notifications/src/lib/glanceable-refresh.ts - 2 issues
  • services/session-ingest/src/dos/UserConnectionDO.test.ts

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic marked this pull request as draft September 18, 2026 20:16
@iscekic
iscekic force-pushed the kwf/req-20260918-044333-77b6 branch from ca843bd to f990f3d Compare September 18, 2026 21:36
@iscekic
iscekic marked this pull request as ready for review September 18, 2026 21:56
Comment thread services/notifications/src/lib/glanceable-refresh.ts
@iscekic
iscekic marked this pull request as draft September 18, 2026 22:04
@iscekic
iscekic force-pushed the kwf/req-20260918-044333-77b6 branch from f990f3d to 7a1c91e Compare September 18, 2026 22:44
@iscekic
iscekic force-pushed the kwf/req-20260918-044333-77b6 branch from 7a1c91e to 5a4d8db Compare September 18, 2026 22:57
@iscekic
iscekic marked this pull request as ready for review September 19, 2026 00:06
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 19, 2026
@iscekic iscekic self-assigned this Sep 19, 2026
@iscekic
iscekic force-pushed the kwf/req-20260918-044333-77b6 branch from 5a4d8db to c09f32f Compare September 19, 2026 23:26
Comment thread services/notifications/src/lib/glanceable-refresh.ts
Comment thread services/session-ingest/src/dos/UserConnectionDO.test.ts
Comment thread services/notifications/src/index.ts Outdated
Comment thread apps/mobile/src/lib/glanceable/publisher.ts
Comment thread services/notifications/src/lib/glanceable-refresh.ts Outdated
Comment thread services/session-ingest/src/ingest/metadata.test.ts
@iscekic
iscekic force-pushed the kwf/req-20260918-044333-77b6 branch from b48f111 to 0edfab8 Compare September 20, 2026 17:11
Comment thread services/notifications/src/lib/glanceable-refresh.ts Outdated
Comment thread services/notifications/src/lib/glanceable-refresh.ts Outdated
Comment thread apps/mobile/src/lib/glanceable/publisher.ts Outdated
Comment thread apps/mobile/src/lib/glanceable/publisher.ts
Comment thread services/session-ingest/src/dos/UserConnectionDO.ts Outdated
Comment thread services/notifications/src/lib/glanceable-refresh.test.ts
Comment thread apps/mobile/src/lib/glanceable/publisher.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant