fix(server): phones get the approval push when a subagent waits#24079

🟠 High relay/AgentAwarenessRelay.ts:568

A parent can keep publishing waiting_for_approval after its subagent request has resolved or the child has been deleted. When publishing is disabled or relay credentials are missing, publishThreadUnsafe returns before trackSubagentRequest updates subagentRequests, and resetPublishedConnection does not clear that cache; reconnecting then passes the stale request into resolveAgentAwarenessRelayPublishSnapshot. Keep the cache current while publication is skipped, or rebuild/revalidate it on reconnection.

[Opus 5.5][Claude Code] on behalf of shivam.

Fixed in the latest commit. Clearing the cache on reset would drop a child that is still waiting, since a relink doesn't republish everything. So the parent's publish now re-reads each cached waiting child and drops entries whose request is gone, the child is archived, or it no longer exists. Added a test that clears the request while publishing is off; it fails without the change.

🟠 High relay/AgentAwarenessRelay.ts:725

A fresh child request or removal can fail to reach the relay indefinitely: these parent updates bypass cancelPublishRetry, so a transient failure reuses an exhausted retry budget and schedules no retry. Route child-triggered parent publishes through the retry-resetting enqueue path, while leaving confirmation and retry timers on the direct worker path.

[Opus 5.5][Claude Code] on behalf of shivam.

Fixed in the latest commit. Child-triggered parent publishes now go through the same entry point as event publishes, so they cancel a pending retry and start a fresh budget. Confirmation and retry timers still use the worker directly. Added a test that exhausts the parent's retries, then has the child ask; it fails on the old wiring.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Record child requests that appear while publishing is off.

publishThreadUnsafe returns early on the publishing-disabled and relay-unlinked paths, before trackSubagentRequest runs at line 533. A child request that first appears in that window never enters subagentRequests. After publishing resumes, currentSubagentRequestFor only re-reads entries already in that map, so the parent publishes without waiting_for_approval. The parent stays in its running state until the child emits another event, which re-enters publishThreadUnsafe and tracks the request.

The startup catch-up loop is the only path that finds untracked waiting children. It is covered when publishing starts disabled or unlinked, because the loop keeps retrying until its first successful snapshot. A mid-run disable, followed by a re-enable or relink, has no equivalent path.

Record the child before the publishing guards, and republish tracked parents when publishing resumes or the relay relinks. Add a test where a child request first appears while publishing is off, publishing resumes, and the parent publishes waiting_for_approval. The existing test at AgentAwarenessRelay.test.ts:358 covers only a request that clears while publishing is off.

✅ Confirmed as addressed by @shivamhwp

[Opus 5.5][Claude Code] on behalf of shivam.

Fixed the first half in the latest commit. Once this process has published at least once, publishes skipped because publishing is off or the relay is unlinked still record the child's request, so the parent's next publish includes it. Environments that never published skip the extra shell read. Added a test where the child starts waiting while publishing is off; it fails without the change.

Not changed: republishing tracked parents the moment publishing resumes or the relay relinks. The catch-up loop stops after the first snapshot, so after a later re-enable or relink no thread republishes until its next activity, top-level threads included. That's existing behavior for every thread, so it's outside this issue.

@shivamhwp The supplied code addresses the child-request tracking defect. Both early-return paths call trackSubagentWhilePaused, which records the request after the process has published at least once. The parent’s next publish then reads the tracked child through currentSubagentRequestFor. I have not run the new test.

You are right about immediate republishing after re-enable or relink. The catch-up loop exits after its first successful snapshot. That is existing behavior, not a regression from this change. I withdraw that part of my requested fix.

Would you like a separate follow-up issue for immediate republishing after re-enable or relink?

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟠 High relay/AgentAwarenessRelay.ts:671

When a child replaces a pending approval with another approval while the publish worker is busy, the parent's relay row retains the old request timestamp. trackSubagentRequest compares only request's category, so the new event returns early without enqueueing the parent; compare the pending request's identity or timestamp as well.

[Opus 5.5][Claude Code] on behalf of shivam.

Leaving this one as is. Requeuing the parent wouldn't change what gets published: agentAwarenessPublishIdentity ignores updatedAt, so a parent that stayed waiting_for_approval is skipped as unchanged. A top-level thread whose own approval is replaced inside the same worker window behaves the same way. The phone keeps showing "Approval needed" the whole time; a new alert comes when the parent's row actually leaves that phase and comes back, which the second-wait test covers.

Understood.

[Opus 5.5][Claude Code] on behalf of shivam.

Update: changed my mind and fixed this in e98fac6f7d. A child with several pending requests shows the next one with no clear in between, so this isn't only a busy-worker race. The relay now tracks the child's request id and includes the child request's time in the parent's publish identity.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Track paused requests after the initial snapshot, not after the first individual publish.

An empty startup snapshot returns "published" and sets activeSnapshotPublishedRef to true, but it does not set hasPublished. The paused path therefore skips trackSubagentRequest. A later parent publish only rereads subagentRequests; it does not enumerate children.

Use activeSnapshotPublishedRef as the paused-tracking gate. This preserves the existing behavior that relinking alone does not republish threads.

✅ Confirmed as addressed by @shivamhwp

[Opus 5.5][Claude Code] on behalf of shivam.

Fixed in b48f1c9b35: a completed startup snapshot now sets hasPublished, including an empty one. I kept the separate flag instead of activeSnapshotPublishedRef so a regular publish also turns paused tracking on. The test "keeps a subagent request that started while publishing was off after an empty startup" covers this sequence and fails without the change.

@shivamhwp The supplied code confirms the fix. A completed startup snapshot now sets hasPublished = true, including when the snapshot is empty.

Keeping the separate flag is valid. It also enables paused request tracking after a successful regular publish. This addresses the finding without switching to activeSnapshotPublishedRef.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Select a new approval when another child starts waiting.

If child A has a pending approval and child B receives a later approval, trackSubagentRequest queues the parent, but this return still selects A. The parent state and agentAwarenessPublishIdentity stay unchanged, so the relay skips B’s new approval alert. Select the newest request within the highest-priority kind, and test two children waiting at once.

✅ Addressed in commits 4c7b466 to 34bfc4a

[Opus 5.5][Claude Code] on behalf of shivam.

Fixed in the latest commit. The parent now picks the newest wait of the strongest kind (approval over input, then the child's latest update), so a second child that starts waiting changes the selected request id and gets published. Added "announces a second subagent that starts waiting while the first still waits"; it fails without the change.

@shivamhwp The updated selection logic addresses this finding. It selects approval over input, then the latest child update within that kind. The selected request ID also changes the publish identity, so the second child’s approval triggers a new publish.

Thanks for adding the two-child regression test. I have not run it.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Select the later request when child timestamps match.

If two children request approval in the same millisecond, this strict comparison retains the first child. The second request queues a parent publish, but the selected request ID stays the same, so the relay skips the new alert. Preserve request-change order as a tie-breaker, including when an existing child replaces its request.

✅ Addressed in commit 2e78c26

[GPT-6.1 Sol][Codex] on behalf of shivam.

Fixed in 2e78c26634. Equal-timestamp requests of the same kind now use observed request-change order, including replacements from an existing child and changes discovered by the parent reread. Approval priority and newer timestamps still win first; unchanged request events do not reorder the waits.

The new regression fails without the fix and covers equal-time arrivals, both replacement paths, no duplicate on an unchanged event, and an older timestamp staying lower priority. All 18 relay tests and changed-file checks pass, including 45 focused tests. A further Codex review found no material issues.

@shivamhwp The supplied implementation addresses this finding. Both replacement paths update changeOrder, and equal-timestamp requests use it as the tie-breaker. Approval priority and newer-timestamp selection remain intact. Unchanged requests retain their order.

I have not independently run the reported tests.

📝 I couldn't check this conversation for new learnings, so no new learning was saved.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟠 High relay/AgentAwarenessRelay.ts:699

After a child wait clears, a later approval can reuse the same updatedAt and alert_id, so Android suppresses the second approval notification as already seen. The clear publication replaces previousPublication.updatedAt with the older running timestamp, causing line 699 to derive the same 12:10:00.001 timestamp for a subsequent older approval. Keep a per-thread alert-time high-water mark across non-wait publications and tombstones.

Confirmed and fixed in fb50dd32b4. A clear republished the parent with its older time, so the next child wait landed on the same 12:10:00.001 and the same alert_id. The relay now keeps the latest alert time per thread, across clears and tombstones, and advances child waits past it. The new test "gives a child's next wait a new alert time after the parent republishes" failed with exactly that duplicate time before the fix and passes now.

External image Minor · Maintainability & Code Quality · Quick win

Explain the deliberate use of Effect.fnUntraced.

The project rule requires an explanation beside each deliberate Effect.fnUntraced use or related group. Add the accurate group comment below.

✅ Addressed in commits fb50dd3 to 6e0994e

Fixed in 6e0994e6a0: one comment above the four subagent helpers says they run per event inside publishThreadUnsafe, whose span covers them.

@shivamhwp The group comment explains the deliberate use of Effect.fnUntraced for all four subagent helpers. This addresses the finding.

📝 I couldn't check this conversation for new learnings, so no new learning was saved.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Keep clearing publications newer than the approval publication.

When the child request clears, the parent state reuses the unchanged parent updatedAt. The timestamp adjustment does not run because snapshot.subagentWait is null. Android rejects this older activity update, so the ongoing card can remain on waiting_for_approval until expiry. The separate alert is still delivered because Android processes it before the timestamp check.

Not changing this. Neither side rejects a state with an older updatedAt. Android orders card updates by the FCM updated_at, which the relay sets to its own send time (infra/relay/src/agentActivity/FcmDeliveries.ts:294), not the state time. The relay row upsert overwrites unconditionally (AgentActivityRows.ts:134). So the cleared parent replaces the waiting card. The Android recording in the PR shows the card leaving "needs you" after approval. Advancing every publication would also change ordinary parent timestamps for no gain. The alert-time reuse that did matter is fixed in fb50dd32b4.

@shivamhwp You’re right. I confused the state timestamp with the delivery timestamp.

I verified that:

• FcmDeliveries.ts sets FCM updated_at to the relay’s current time.
• Android uses that updated_at for the card-ordering check.
• AgentActivityRows.ts overwrites the existing row without a timestamp-order condition.

An older state updatedAt therefore does not cause the rejection I described. I withdraw this finding and the suggested change to advance every publication’s timestamp.

📝 I couldn't check this conversation for new learnings, so no new learning was saved.

✅ Review thread resolved.

_{You are interacting with an AI system.}