Skip to content

feat(activity): #200 · background pending poll + everywhere waiting cue#213

Merged
hellno merged 4 commits into
mainfrom
hellno/pending-approvals-badge
Jul 8, 2026
Merged

feat(activity): #200 · background pending poll + everywhere waiting cue#213
hellno merged 4 commits into
mainfrom
hellno/pending-approvals-badge

Conversation

@hellno

@hellno hellno commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Closes #200. Part of #179.

Why

The approval/activity poller only ran while the operator was on the Activity surface, so a dapp or agent request raised from Home/Settings/a wallet view silently expired (approval TTL ~120s). The "waiting on you" promise now holds from any surface.

What

  • Background pending poll (start_pending_poll, 5s cadence — well inside the 120s TTL): a whole-session loop that mirrors start_balance_poll (task handle dropped on lock, auth_epoch-fenced against fast re-unlock). It fetches only PendingList (a wire call the app already uses) and folds the still-Pending count into polled_pending. First fetch fires immediately on unlock so a pre-parked request lights the cue the moment the shell renders.
  • No double-fetch: while Activity is open the background tick Skips (the existing ~2s feed poller owns freshness there — that poller is untouched), and refresh_activity reconciles polled_pending from the fresh feed so stepping off Activity can never show a count a refresh just settled.
  • ONE shared source, no new notification pattern: Shell::waiting_count() now feeds BOTH the E6 home waiting strip (welcome.rs) and the sidebar Activity count badge (shell_chrome.rs). On the Activity surface the feed count wins (the badge must equal the NEEDS YOU band beside it); everywhere else the background snapshot wins, with the last feed snapshot as fallback until the first poll lands.
  • Passive by design (DESIGN trust rules): the poll updates a count and nothing else — no auto-navigation, no focus steal, no approve control. Navigation stays user-initiated (strip's "Review →", badge row click, existing ⌘⇧A / palette open-activity).
  • Pure, unit-tested state machine at the bottom of shell.rs: pending_poll_tick (Stop/Skip/Fetch — encodes the two-pollers contract), pending_waiting (only ApprovalStatus::Pending counts — Allowed/Denied/Expired never cry wolf), waiting_count (source dispatch). 5 new tests in pending_poll_tests.

Non-goals honored

No OS notifications, no approval-semantics/expiry changes, no daemon/wire changes, no new dependencies. Diff kept to the poll loop + the two render sites (coordination with #198, which touches activity_view.rs — this PR does not).

Definition of done

  1. cargo fmt --all --check — clean
  2. just check — green (default AND --features tray)
  3. TMPDIR=/tmp cargo test --workspace436 passed, 2 ignored (39 suites)
  4. Pure unit tests on the poll/badge state machine (5 new) + no changes to start_activity_poller (existing feed-poll tests untouched and green)
  5. Screenshots: being captured (see PR comments) — app on Home, bridge personal_sign raises the strip + badge within one poll interval without navigation
  6. Adversarial multi-lens review incl. codex: running; P0/P1 will be fixed before merge
  7. CI: pending

hellno added 3 commits July 7, 2026 21:59
The approval poller only ran while the operator was ON the Activity surface,
so a dapp/agent request raised from Home/Settings/a wallet view silently
expired (~120s TTL). Add a low-cadence (5s) whole-session PendingList poll
(mirrors the balance poller: task handle dropped on lock, auth-epoch fenced)
that feeds ONE shared Shell::waiting_count() behind the existing E6 home
waiting strip + sidebar Activity badge — no new notification pattern, no
navigation, no focus steal (passive cue per DESIGN trust rules).

The two pollers never double-fetch: on the Activity surface the background
tick SKIPs (the ~2s feed poller owns freshness) and refresh_activity
reconciles the snapshot so stepping off the feed can't show a settled count.
Pure state machine (pending_poll_tick / pending_waiting / waiting_count)
unit-tested; only status==Pending rows count as waiting.
…oles

Adversarial review (self-verified after the workflow's verify phase hit a
session limit) surfaced four real defects in the first cut; the hung-round-
trip guard-wedge it also raised is a pre-existing shared property of the
blocking client (balance + activity pollers behave identically) and a wedged
daemon can't act on approvals anyway — documented as a known limitation, not
fixed here.

Fixed:
- P1: the home strip's 'Review →' did a raw open(Surface::Activity) — landing
  on a stale feed with the feed poller stopped AND the background poll SKIPping
  (it skips while Activity is open). The cue's own CTA buried the request it
  announced, all polling halted, until it expired. Now routes through
  open_activity (refresh + restart the feed poller), like every other feed path.
- P3: lock() cleared polled_pending but not the in-flight ActivityFeed fetch —
  a reply landing post-lock re-installed a dead session's feed AND its
  polled_pending reconcile. Bump activity_epoch in lock() to fence it.
- P3: two unsequenced writers of polled_pending — a background reply that raced
  a surface switch carried an older (pre-approve) daemon snapshot and could
  revive a just-settled count. Drop the background write while surface==Activity
  (the feed reconcile owns that state there).
- P3: lock() left self.activity populated, so off-Activity waiting_count fell
  back to a previous session's feed count and flashed a phantom amber
  'N waiting' after re-unlock until the first poll. Clear the feed snapshot on
  lock; the fallback now reads 0 (caught up) — the safe direction.
Live QA evidence (just qa vault, ask-me-everything preset, chain 31337). App
sitting on Home, never navigated: a bridge personal_sign parks a Pending, and
within one 5s background-poll interval the home waiting strip flips green
'Nothing waiting for you.' → amber '1 waiting for you · Review →' AND the
sidebar Activity row gains its amber '1' badge — no navigation, no focus
steal, no approve control (passive cue, DESIGN trust rules).
@hellno

hellno commented Jul 8, 2026

Copy link
Copy Markdown
Owner Author

Screenshot evidence (acceptance #1) ✅

Live QA (just qa vault, ask-me-everything preset, chain 31337). App sitting on Home, never navigated — a bridge personal_sign parks a Pending, and within one 5s background-poll interval the cue lights up on its own.

Before — Home, caught up (green "Nothing waiting for you.", no sidebar badge):

before

After — same Home surface, one poll interval later: amber "1 waiting for you · Review →" strip + amber "1" badge on the sidebar Activity row. No navigation, no focus steal, no approve control:

after

(The personal_sign was still blocking/awaiting approval at capture time, so the cue reflects a genuinely-live Pending, not a phantom.)

Adversarial review — findings fixed

Ran a multi-lens review (3 Claude lenses: races / trust / regression). The verify + codex passes hit an upstream session limit, so I self-verified the raw findings against the code. Four were real and are fixed in d9f0aff; one was refuted as pre-existing:

# Sev Finding Resolution
1 P1 Home strip's Review → did a raw open(Surface::Activity) — landing on a stale feed with the feed poller stopped AND the background poll SKIPping (it skips while Activity is open). The cue's own CTA buried the request it announced, all polling halted, until it expired. Route through open_activity (refresh + restart feed poller), like every other feed path.
2 P3 lock() cleared polled_pending but not an in-flight ActivityFeed fetch — a reply landing post-lock re-installed a dead session's feed + reconcile. Bump activity_epoch in lock() to fence it.
3 P3 Two unsequenced writers of polled_pending: a background reply racing a surface switch carried an older (pre-approve) snapshot and could revive a just-settled count. Drop the background write while surface == Activity (the feed reconcile owns that state there).
4 P3 lock() left self.activity populated, so off-Activity waiting_count fell back to a previous session's feed count → phantom amber "N waiting" flash after re-unlock. Clear the feed snapshot on lock; fallback now reads 0 (safe direction).
Hung PendingList round-trip wedges the in-flight guard Refuted — pre-existing shared property of the blocking client (balance + activity pollers behave identically), and a wedged daemon can't act on approvals anyway. Documented as a known limitation, not introduced here.

DoD status

  1. cargo fmt --all --check — clean ✅
  2. just check — green, both default and --features tray
  3. TMPDIR=/tmp cargo test --workspace436 passed, 2 ignored
  4. Pure unit tests on the poll/badge state machine (pending_poll_tests, 5) + existing Activity-poll behavior unchanged ✅
  5. Before/after screenshots ✅ (above)
  6. Adversarial review, P0/P1 fixed ✅
  7. CI — see checks below

…le source for the cue

Codex (GPT-5, cross-model) found the P0/P1 my Claude-lens review missed: the
Activity feed is daemon-capped at the newest 200 rows while PendingList is
uncapped, so reconciling polled_pending from the feed (refresh_activity) could
undercount to a phantom 0 and hide a real waiting request once >200 newer
records exist — a wrong-direction cue. The same reconcile was also a second,
unordered writer of polled_pending, racing the background reply.

Fix: make the uncapped PendingList the SOLE source of polled_pending. Remove
the feed-derived reconcile entirely (kills the cap-divergence AND the dual-
writer race — kick_pending_refresh is now the single, in-flight-guarded
writer). Preserve the 'leaving the feed is fresh' benefit by kicking one
uncapped kick_pending_refresh on the Activity→elsewhere nav transition
(should_kick_pending_on_nav, pure + unit-tested) in open()/select(). On the
Activity surface waiting_count still reads the feed, so badge == the NEEDS YOU
band beside it (both inherit the daemon cap — the underlying feed-cap-hides-
pending-rows issue is a pre-existing daemon bug, out of scope for this app-only
PR; worth a follow-up against the daemon).

DoD: fmt clean; just check green (both configs); cargo test --workspace 437
passed (+1 nav-transition test).
@hellno

hellno commented Jul 8, 2026

Copy link
Copy Markdown
Owner Author

Codex cross-model review (re-run after the session limit reset) ✅ + fix

Ran the codex review (GPT-5, high reasoning, read-only) against the full branch diff. It cleared the concurrency concern it was asked to break (the in-flight-guard wedge — verified safe: auth_epoch mutates only in lock(), which resets the guard before bumping) and found one real class of bug my earlier Claude-lens review missed:

P0/P1 — feed-cap divergence (now fixed in f0a426b)

The daemon caps ActivityFeed at the newest 200 rows; PendingList is uncapped. My refresh_activity reconcile derived polled_pending from the capped feed — so once >200 newer records exist, a still-Pending request drops out of the feed, the reconcile writes polled_pending = 0, and the cue can show 0 while a real request is waiting (wrong-direction — it could expire unseen). Codex also flagged the reconcile as a second, unordered writer of polled_pending, racing the background reply (a stale reply could clobber a fresh reconcile, or revive a settled count, for a poll cycle).

Fix — one uncapped source, no dual writer:

  • Removed the feed-derived reconcile entirely. polled_pending now has a single source: the uncapped PendingList in kick_pending_refresh (in-flight-guarded → only one writer ever outstanding, so the write-ordering race is gone too).
  • Preserved the "leaving the feed refreshes the cue immediately" benefit by firing one uncapped kick_pending_refresh on the Activity→elsewhere nav transition (should_kick_pending_on_nav, pure + unit-tested) in open()/select() — so an approve/deny on the feed reflects on the home/rail cue within a round-trip, from the correct source.
  • On the Activity surface waiting_count still reads the feed, so badge == the NEEDS YOU band beside it (both inherit the daemon cap).

Known limitation / follow-up (not this PR)

The root cause — the daemon's activity_feed 200-cap can hide Pending rows from the on-Activity NEEDS YOU band itself — is a pre-existing daemon bug, independent of #200 and out of scope for this app-only branch. After this fix, #200's off-Activity cue is correct (uncapped); only the on-Activity band inherits the cap (badge==band). Worth a standalone follow-up issue against the daemon (raise the cap, or send a "truncated" flag). Happy to file it on your go-ahead.

Codex P3 — test gap

Closed as far as is practical without a gpui async harness: added kicks_pending_only_when_leaving_activity covering the new pure transition rule; the async invariant it guards (never source polled_pending from the capped feed) is now structural — there is no feed→polled_pending path left to regress.

DoD after the fix: cargo fmt --all --check clean · just check green (both configs) · TMPDIR=/tmp cargo test --workspace 437 passed, 2 ignored (+1 nav test). No false positives from codex; full transcript retained locally.

@hellno
hellno merged commit 16813be into main Jul 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Surface pending approvals from any surface (background poll + badge)

1 participant