Skip to content

fix(relay): keep startup notification catch-up silent - #11914

Open
Gigioxx wants to merge 3 commits into
pingdotgg:mainfrom
Gigioxx:t3code/fix-issue-11886
Open

Gigioxx wants to merge 3 commits into
pingdotgg:mainfrom
Gigioxx:t3code/fix-issue-11886

Conversation

@Gigioxx

@Gigioxx Gigioxx commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Restarting an environment republishes saved agent state and can flood iOS with completion and input notifications.

What Changed

Mark startup catch-up as signed silent replay and pass it through the existing APNs/FCM replay handling. Use the existing worker to serialize catch-up with live updates, and keep replay silent only while the captured state still matches. Deferred confirmations retain that identity. Ignore settlement-only events and apply the existing two-minute notification freshness window to input and approval pushes.

Why

Saved state must restore remote activity without being treated as new work. Fresh transitions still alert. This complements #9081's terminal timestamp fix and does not duplicate #11073's concurrent-thread notification selection.

Verification

  • Reproduced missing startup replay markers and stale input/approval alert eligibility before the fix.
  • 143 focused server and relay tests pass, including live changes during catch-up, transient missing snapshots, deferred completion, signed flag tampering, and APNs notification-only/unarmed/armed devices.
  • Server and relay typechecks, changed-file lint, formatting, and diff checks pass.
  • Isolated browser reconnects and shows a 100-thread fixture. Two local server restarts each publish all 100 saved states with replay enabled. Queue tests verify silence; real iPhone banners and sounds were not tested.

Requires server and relay updates. No mobile UI change or production deployment is included.

Closes #11886

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • No visual UI or animation changes; screenshots/video are not applicable

Implemented with GPT-6 in the Codex harness. Reviewed with Claude Sonnet 5 before opening the PR; its Android replay finding was reproduced and fixed with two regression cases.

Summary by CodeRabbit

  • New Features

    • Activity state replays restore current agent activity across devices and platforms.
    • Replay updates are delivered silently without duplicate alerts or notifications.
    • Replay requests are validated to prevent inconsistent activity updates.
  • Bug Fixes

    • Improved alert freshness handling for waiting-for-input and waiting-for-approval states.
    • Settled activity events no longer produce publishable updates.
    • Reduced duplicate alerts during activity transitions and startup recovery.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 15, 2026
alertAllowedForPhase(input.preferences, input.phase) &&
((input.phase !== "completed" && input.phase !== "failed") ||
isFreshTerminalNotification(input.updatedAt, input.nowMs))
isFreshNotification(input.updatedAt, input.nowMs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium agentActivity/agentActivityAlerts.ts:147

Stale waiting_for_input or waiting_for_approval updates still produce APNs alerts for armed iOS Live Activities, bypassing the intended two-minute limit. Those paths call alertForAttentionTransition directly in ApnsDeliveries.ts (including delivery-job revalidation) without checking updatedAt against nowMs; apply the same freshness filtering there.

🤖 Copy this AI Prompt to have your agent fix this:
In file @infra/relay/src/agentActivity/agentActivityAlerts.ts around line 147:

Stale `waiting_for_input` or `waiting_for_approval` updates still produce APNs alerts for armed iOS Live Activities, bypassing the intended two-minute limit. Those paths call `alertForAttentionTransition` directly in `ApnsDeliveries.ts` (including delivery-job revalidation) without checking `updatedAt` against `nowMs`; apply the same freshness filtering there.

@macroscopeapp

macroscopeapp Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This change modifies existing startup and live notification behavior across signed relay payloads, event scheduling, APNs, and FCM delivery paths rather than making a small isolated correction. An unresolved Medium-severity finding also identifies an APNs path where stale attention alerts can still be emitted.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 7eb19d30-8608-45f5-9977-dcfc813843e5

📥 Commits

Reviewing files that changed from the base of the PR and between 2938857 and 258685f.

📒 Files selected for processing (4)
  • infra/relay/src/agentActivity/ApnsDeliveries.test.ts
  • infra/relay/src/agentActivity/ApnsDeliveries.ts
  • infra/relay/src/agentActivity/agentActivityAlerts.ts
  • infra/relay/src/agentActivity/agentActivityPolicy.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The relay now marks startup snapshot publications as replays. The replay marker travels through request signing, API forwarding, and APNs/FCM delivery. Replay deliveries suppress alerts. Notification freshness now applies to attention states.

Changes

Replay-aware agent activity publishing

Layer / File(s) Summary
Replay contracts and validation
packages/contracts/src/relay.ts, infra/relay/src/environments/..., infra/relay/src/http/Api.ts
Publish requests and proofs support replay. Signature verification rejects mismatched replay values. The API forwards the flag.
Relay snapshot and event flow
apps/server/src/relay/AgentAwarenessRelay.ts, apps/server/src/relay/AgentAwarenessRelay.test.ts
Startup catch-up captures activity states and publishes matching states as replays. Live events remain non-replay publications. Settled events are suppressed.
Silent mobile replay delivery
infra/relay/src/agentActivity/...
Replay deliveries bypass alert generation in APNs and FCM. Live input still produces alerts.
Notification freshness policy
infra/relay/src/agentActivity/agentActivityAlerts.ts, infra/relay/src/agentActivity/agentActivityPolicy.test.ts
Freshness checks apply to all activity phases, including waiting states. Tests cover the 120-second boundary.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant AgentAwarenessRelay
  participant PublishAPI
  participant AgentActivityPublisher
  participant APNs
  participant FCM
  AgentAwarenessRelay->>PublishAPI: submit replay-marked activity request
  PublishAPI->>AgentActivityPublisher: forward replay flag
  AgentActivityPublisher->>APNs: enqueue silent replay delivery
  AgentActivityPublisher->>FCM: enqueue silent replay delivery
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 25868

The replay marker is carried through signing and mobile delivery, and attention notifications use the intended freshness window. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 13 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: keeping startup notification catch-up silent.
Description check ✅ Passed The description explains what changed, why it changed, verification results, scope, and checklist status. It also states that there are no UI changes, which addresses the optional UI section.
Linked Issues check ✅ Passed Issue #11886 requires startup restoration to avoid replaying historical events as alerts and to provide only relevant notifications. The relay captures activity states, serializes catch-up with live u…
Out of Scope Changes check ✅ Passed The changes stay within Issue #11886. Relay, publisher, APNs, FCM, signature, contract, and alert-policy changes support silent startup replay and correct notification selection. The tests validate th…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Notification spam on IOS

1 participant