Skip to content

fix(notifications): hold the push-to-start fence across token rotation - #6689

Merged
iscekic merged 22 commits into
mainfrom
kwf/ios-live-activity-expand-vibrate-05d1
Sep 28, 2026
Merged

iscekic merged 22 commits into
mainfrom
kwf/ios-live-activity-expand-vibrate-05d1

Conversation

@iscekic

@iscekic iscekic commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • A Live Activity card stays collapsed and silent when the device registers a fresh push-to-start token while a card is already on screen.
  • The duplicate push-to-start that lit the screen and expanded the card is no longer sent.

Changelog for maintainers

  • dropFencedStarts now treats the push-to-start fence as scope-wide: while any fence under iosStartPrefix is live, every ios_push_to_start token is dropped from the send list.
  • Previously the fence was keyed to the exact token that wrote it, so a rotated token bypassed it and raised a second card beside the first.
  • Trigger found: a duplicate start. The device rotates its push-to-start token while the first card is still on screen and unadopted; APNs requires an alert on every push-to-start, and that alert is what lights the screen and expands the card.
  • An adopted card (an ios_activity row) still releases every fence in the scope and returns the full token list, so ordinary starts resume once the app owns the card.
  • Lapsed fences are still deleted on read, including those of tokens the device has rotated away.
  • Risk: while a fence is live and no card has been adopted, no push-to-start starts a new card; the fence lapses after the snapshot expiry window.
  • Not proven in this change: a push, a local refresh, a reconnect, and an app foreground. Those paths carry no server change here; they are owned by the client adoption and sweep on the base branch.
  • Test: holds the push-to-start fence across a rotated push-to-start token in the notifications glanceable delivery tests.

E2E proof

Owner request

Surface: the mobile app (apps/mobile), the iOS Live Activity.

An iOS Live Activity becomes expanded, and vibrates, with no user action. This must never happen. A Live Activity is subtle: it sits collapsed and it stays silent.

Build on the in-flight client fix, #6483 (branch kwf/owner-live-activity-strays-c7). That branch already touches apps/mobile/src/glanceable-ios/adopt-activity.ts, ios-sink.ts, register.ts and lib/glanceable/persist.ts. Reproduce this defect on top of it.

Required behaviour:

  • The card stays collapsed until the user taps it.
  • No haptic and no vibration from a Live Activity update, whatever the trigger.
  • No alert and no sound.
  • An update changes the numbers only. It never changes the presentation state on its own.

Audit every path that can expand the card or ask for a haptic: a push, a local refresh, a reconnect, an app foreground, and a duplicate start. Name the trigger you found in the pull request body. If one path cannot be proven, say so and name it.

Proof: the card over several update cycles, staying collapsed and silent, with the decisive log lines or a screen recording from a real iOS simulator.

E2E proof

  • not proved live: no live proof was captured this round

Open findings (not fixed here)

  • not fully verified: some optional checks did not run
  • the '## E2E proof' section carries no log excerpt, so nothing shows the change was driven end to end

Surface: the mobile app (apps/mobile), the iOS Live Activity.

An iOS Live Activity becomes expanded, and vibrates, with no user action. This must never happen. A Live Activity is subtle: it sits collapsed and it stays silent.

Build on the in-flight client fix, #6483 (branch `kwf/owner-live-activity-strays-c7`). That branch already touches `apps/mobile/src/glanceable-ios/adopt-activity.ts`, `ios-sink.ts`, `register.ts` and `lib/glanceable/persist.ts`. Reproduce this defect on top of it.

Required behaviour:
- The card stays collapsed until the user taps it.
- No haptic and no vibration from a Live Activity update, whatever the trigger.
- No alert and no sound.
- An update changes the numbers only. It never changes the presentation state on its own.

Audit every path that can expand the card or ask for a haptic: a push, a local refresh, a reconnect, an app foreground, and a duplicate start. Name the trigger you found in the pull request body. If one path cannot be proven, say so and name it.

Proof: the card over several update cycles, staying collapsed and silent, with the decisive log lines or a screen recording from a real iOS simulator.
@iscekic
iscekic marked this pull request as draft September 24, 2026 13:20
@iscekic
iscekic added this pull request to stack #6690 September 24, 2026 13:20
@kilo-code-bot

kilo-code-bot Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review since a73411ab: commit 8ffb37a0 drops only the unrelated gastown and session-ingest changes, reverting those files to the base tree (e16445366). The PR's net diff stays the notification push-to-start fence change; no new findings.

Files Reviewed (18 files)
  • services/gastown/src/dos/Agent.do.ts
  • services/gastown/src/dos/Town.do.ts
  • services/gastown/src/dos/town/agents.ts
  • services/gastown/src/gastown.worker.ts
  • services/gastown/src/handlers/town-container.handler.ts
  • services/gastown/test/integration/awaiting-approval.test.ts
  • services/gastown/test/integration/convoy-dag.test.ts
  • services/gastown/test/integration/http-api.test.ts
  • services/gastown/test/integration/mayor-idle.test.ts
  • services/gastown/test/integration/pr-poll-errors.test.ts
  • services/gastown/test/integration/reconciler.test.ts
  • services/gastown/test/integration/review-failure.test.ts
  • services/gastown/test/integration/rig-alarm.test.ts
  • services/gastown/test/integration/rig-do.test.ts
  • services/gastown/test/integration/town-container.test.ts
  • services/gastown/test/integration/town-deletion.test.ts
  • services/session-ingest/src/ingest/validate-oversized.test.ts
  • services/session-ingest/src/ingest/validate.test.ts
Previous Review Summaries (2 snapshots, latest commit a73411a)

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

Previous review (commit a73411a)

Status: No Issues Found | Recommendation: Merge

Incremental review since 0eb54ffd: the only changed lines are formatting-only (oxfmt) edits; no new findings.

Files Reviewed (3 files)
  • services/gastown/src/gastown.worker.ts
  • services/gastown/test/integration/http-api.test.ts
  • services/gastown/test/integration/town-container.test.ts

Previous review (commit 0eb54ff)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (20 files)
  • services/gastown/src/dos/Agent.do.ts
  • services/gastown/src/dos/Town.do.ts
  • services/gastown/src/dos/town/agents.ts
  • services/gastown/src/gastown.worker.ts
  • services/gastown/src/handlers/town-container.handler.ts
  • services/gastown/test/integration/awaiting-approval.test.ts
  • services/gastown/test/integration/convoy-dag.test.ts
  • services/gastown/test/integration/http-api.test.ts
  • services/gastown/test/integration/mayor-idle.test.ts
  • services/gastown/test/integration/pr-poll-errors.test.ts
  • services/gastown/test/integration/reconciler.test.ts
  • services/gastown/test/integration/review-failure.test.ts
  • services/gastown/test/integration/rig-alarm.test.ts
  • services/gastown/test/integration/rig-do.test.ts
  • services/gastown/test/integration/town-container.test.ts
  • services/gastown/test/integration/town-deletion.test.ts
  • services/notifications/src/lib/glanceable-delivery.test.ts
  • services/notifications/src/lib/glanceable-refresh.ts
  • services/session-ingest/src/ingest/validate-oversized.test.ts
  • services/session-ingest/src/ingest/validate.test.ts

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

Review guidance: REVIEW.md from base branch kwf/owner-live-activity-strays-c7

…-expand-vibrate-05d1

# Conflicts:
#	apps/mobile/src/lib/glanceable/persist.ts
…ty-strays-c7

# Conflicts:
#	apps/mobile/src/lib/glanceable/persist.ts
The fake SecureStore in this suite is a plain object, not a vi.fn, so the
mock helpers were not on it: typecheck rejected them and the async
implementation tripped the no-await rule. Inject a rejecting store, then a
gated one, through _setSecureStoreForTests, the way the neighbouring
first-restore case does.
…7' into kwf/ios-live-activity-expand-vibrate-05d1

# Conflicts:
#	apps/mobile/src/lib/glanceable/persist.ts
…e mock

The new test installed a one-shot rejection and a one-shot pending read on
`secureStoreMock.getItemAsync`, but that field was a plain async function, so
both `mockRejectedValueOnce` and `mockImplementationOnce` were invalid calls.
oxlint failed the PR on the second one (promise-function-async,
prefer-await-to-then). Wrap the field in `vi.fn` and await the gate inside the
one-shot implementation.
…7' into kwf/ios-live-activity-expand-vibrate-05d1

# Conflicts:
#	apps/mobile/src/glanceable-ios/ios-sink.test.ts
oxlint rejects an async store read whose body is only a throw (require-await),
so the case no longer needs a rejecting store: a first restore against the
empty mirror already settles with a null snapshot, which is the state the
later in-flight read must not be confused with.
…7' into kwf/ios-live-activity-expand-vibrate-05d1
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 25, 2026
@iscekic
iscekic marked this pull request as ready for review September 25, 2026 13:49
iscekic added a commit that referenced this pull request Sep 25, 2026
The gastown auth and Durable Object fixes, the session-ingest test
repair, and the security-auto-analysis integration config came from an
unrelated backend gate repair. They do not belong to a cloud-agent-sdk
capability change.

Reverts those trees to the branch merge base (8e59fe6). The same
gastown fix is present in #6689 and #6580.
The gastown auth, Durable Object lifecycle, gastown integration
tests, and session-ingest validation changes came from a backend gate
repair, not from this change. Restored to the branch merge base
(f1f708e).
A sweep that met an in-flight persisted-state read deferred and never ran
again, so when that read settled with an empty mirror the card the sweep kept
was unowned and stayed on the Lock Screen until the next foreground or
publisher update.

`persist` now exposes `whenGlanceableRestoresSettle`, which resumes a caller
when the last in-flight read lands, and the sweep awaits it before it runs
again. A read that fails still settles, so the sweep re-reads the unreadable
flag instead of waiting forever.
…7' into kwf/ios-live-activity-expand-vibrate-05d1
The mobile lint enables the promise rules, so `then` callbacks fail the lint
job: the sweep resumes through an async IIFE, the tests await the waiter
through the same shape, and `whenGlanceableRestoresSettle` is async.
…7' into kwf/ios-live-activity-expand-vibrate-05d1
Base automatically changed from kwf/owner-live-activity-strays-c7 to main September 26, 2026 00:51
iscekic added a commit that referenced this pull request Sep 26, 2026
)

* fix(cloud-agent-sdk): treat unknown CLI capabilities as supported

Surface: the mobile app (apps/mobile) and the cloud-agent SDK (packages/cloud-agent-sdk).

A capability gate that depends on CLI support must default to YES. Today it defaults to NO, so a feature disappears until the CLI advertises it.

Evidence:
- `packages/cloud-agent-sdk/src/session-manager.ts:880` creates `supportsAttachmentsAtom` as `atom(false)`.
- `recomputeSupportsAttachments` (`:1468`-`:1485`) sets `true` for `cloud-agent`, and `currentCapabilities?.attachments === true` for `remote`. Every other remote state (absent, false, mid-reconnect) sets `false`.
- `apps/mobile/src/components/agents/session-detail-content.tsx:2298` passes that atom to `attachmentsEnabled`, so the paperclip is absent until the CLI reports the capability.

Requirements:
- Optimistic default: while the CLI capability is unknown, the gate reports supported.
- Downgrade only on an explicit negative. A heartbeat or `sessions.list` row that says `attachments === false` sets the gate to false.
- A `read-only` session stays unsupported.
- Apply the same rule to every gate in this file that reads a CLI capability. Attachments is one example, not the whole set.
- The downgrade must still take effect as soon as the CLI reports it. Do not lose the reconciliation.

Proof: unit tests for unknown -> true, explicit false -> false, `cloud-agent` -> true, `read-only` -> false. Then one live proof on the platform you choose: open a remote session whose CLI has not yet reported capabilities, and show the attachmen

* fix: kwf-fix-review-af0e patch delivery

* fix: fix the failed backend verification gate (second attempt, different model) (kwf kwf-fix-review-af0e/gr2)

* style: apply the repo formatter

* fix(gastown): keep per-town auth on container control-plane routes

The /container/ entry in the /api/towns/:townId/* skip list let every
Town Container control-plane route (agents/start, agents/:id/stop,
agents/:id/message, agents/:id/status, agents/:id/stream-ticket, health,
pty) bypass kiloAuthMiddleware, adminAuditMiddleware and
townAuthMiddleware. The handlers proxy straight to the container control
server and check no authorization of their own, so any principal that
clears Cloudflare Access could drive another tenant's container by
supplying its townId. CF Access authenticates the caller but does not
enforce town ownership.

Drop the skip and return the middleware response instead of awaiting it,
so an unauthenticated caller gets the middleware 401 instead of a dropped
response. Update the container route comment and the two integration
tests that asserted an unauthenticated request reached the body validator.

* chore(scope): keep the PR to the CLI capability change

The gastown auth and Durable Object fixes, the session-ingest test
repair, and the security-auto-analysis integration config came from an
unrelated backend gate repair. They do not belong to a cloud-agent-sdk
capability change.

Reverts those trees to the branch merge base (8e59fe6). The same
gastown fix is present in #6689 and #6580.

* fix(mobile): recheck spawn admission against the refreshed instance

The file and clone checks ran against the press-time row before the
refetch resolved the live row. A rebooted host can come back on a new
connectionId and report an explicit refusal the press-time row did not,
so the spawn could use a row that now refuses the file payload or the
clone source. Both checks now run again against the live row before the
spawn commits; the attempt was already admitted, so a refusal fails it
and re-arms the abandon guard.

* fix(cloud-agent-sdk): refuse remote parts without the consumer path

The send guard checked the session type and the CLI capability, but not
the consumer's declaration that it can deliver remote attachment parts.
The UI gate disables the attachment control without that declaration, so
a caller that supplied attachmentParts could still have them forwarded.
The guard now requires config.supportsRemoteAttachmentParts, the same
condition the gate uses.

* test(mobile): make the refreshed-instance refetch stubs async

The two new stubs returned Promise.resolve from a plain arrow, which the
repo's promise rules reject (promise-function-async and
prefer-await-to-then). An async arrow satisfies both without the disable
comment the older stubs needed.

* test(mobile): use the file's refetch stub pattern for the new cases

An async arrow trips require-await in the mobile lint config, and a bare
Promise.resolve arrow trips promise-function-async and
prefer-await-to-then. The file's established single-line stub with the
disable comment satisfies all three; the refreshed list is hoisted to a
const so the stub stays on one line.
@iscekic iscekic added the merge-by-human the merge bot routed this PR to a human label Sep 26, 2026
@iscekic iscekic removed the human-ready The PR is ready for human review. label Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-by-human the merge bot routed this PR to a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants