Consent prompt window, Activity feed, tray attention (desktop supervisor) - #504
Merged
alexeyzimarev merged 29 commits intoAug 10, 2026
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aved/requester_display DTO fields, hoisted decision record
…uester_display threading
…re, consent/2 capability
…fail-closed validation
…ache, lifetime tombstones, prune hygiene Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in-flight TOCTOU Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oordinator-owned raise The window renders ONE pinned request: the sorted queue head, released only on an advance, so an arrival, a prune or a replay can never swap the display out from under a click. Three honesty rules carry the design: * hint expiry is not a verdict — the countdown says so and the buttons stay active, so a click after zero either applies (backward clock step) or runs the "Already decided" path on the daemon's Ok=false; * an in-flight resolve outranks the clock — "Expiring…", the ack governs; * a transport failure discloses nothing about the rule, whose outcome on that path describes a save that was never sent. The advance also skips the identity it just concluded: the service's eviction and the ack's continuation are two independently posted jobs, so the queue view may not have caught up yet. The coordinator owns the single window (open-or-activate, close = defer, reopen re-creates) and is what filters the service's unconditional entry-added signal by visibility and marshals it to the UI thread.
…rtup and shutdown ConsentService is created beside the daemon client over the shared ops/ticker/notifier, and the prompt coordinator builds a fresh window (and ViewModel) per raise. Teardown stays reverse-creation order and now runs coordinator before service before daemon client (spec §5), on both the shutdown and startup-failure paths, so no click can reach a disposed service; a resolve already in flight was cancelled by the shutdown token and settles on the ViewModel's silent-abort path.
…der it On the last pending request — the common single-prompt case — the warning toast was notified and then discarded: the advance emptied the queue and the window closed on the same beat, before the posted overlay could render, leaving the disclosure on stderr only. Exactly what "never a silent success" exists to prevent. Settle now asks what the advance would land on. With something queued it advances as before (multi-entry behaviour unchanged, and the toast lands over the next request); with nothing queued it takes the existing 2-tick terminal hold, showing the warning in the window as well as over it, and closes after. Applied with no warning still advances and closes immediately. The hold now serves two conclusions, so the phase is named Concluded rather than AlreadyDecided and both entry points share one Hold helper. Also drops an issue number from a comment (scripts/check-linear-ids.sh).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ing the prompt window EntryAdded keyed on the cache KEY, so it was wrong in both directions. A successor B under A's RequestId — a relaunch, the likeliest second prompt there is — replaced A's slot in silence and never raised a window for the retry. And a resubscribe's clear+replay made every replayed entry look new, so a window the user had explicitly closed (a defer) came straight back on any reconnect blip, while an OPEN window saw the intermediate empty changeset, closed itself, and was rebuilt a moment later with a fresh ViewModel, a reset pin and stolen focus. The signal now fires on the FIRST SURFACING of a PromptId: a service-lifetime `_surfaced` set with the same never-reused-GUID argument the tombstones rest on (tombstones are a subset of it, kept separate because a tombstone DROPS a frame while `_surfaced` only keeps it quiet). A successor carries a fresh identity and raises; a replayed one does not. The window half is a one-beat close deferral: only a DECISION — an ack, or the end of its terminal hold — closes on the spot. A queue the cache emptied waits one ticker beat, and a replay landing inside it disarms the close. The pin still releases immediately, so nothing that left the cache stays on screen. Also moves OnStatus's down-level `_cache.Clear()` inside `_lock`, closing the window where an Upsert that had already passed its tombstone test lands its insert after the clear and resurrects a previous incarnation's entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sistency items ActivityViewModel's ticker subscription was constructor-scoped and dropped on the floor. The shared ticker is Publish().RefCount(), so an undisposed subscriber keeps its Interval — and the ViewModel — alive past teardown, including the startup-failure path where the app lingers on an error window. It becomes IDisposable, holds the subscription, and joins App's two disposal lists in reverse creation order (after the consent service, before the pause controller). Also, from the same review pass: * the DateTimeOffset parse styles now agree — RoundtripKind in both the feed and ConsentService, over the same daemon-written ISO stamps; * LaunchConsentBroker's class doc names all four instance-scoped removal sites, not two (the timeout claim and TryResolve's own claim were missing); * UiTicker's pipeline note says "this ticker", which is what it constructs; * the §5/§6 refinements this wave discovered are recorded in the spec the way the §6 disclosure amendment was. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Summary by QodoDesktop supervisor consent prompt + activity feed + tray attention (consent/2)
AI Description
Diagram
High-Level Assessment
Files changed (57)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94da3db84e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| internal static string ActivityStatKey(string daemonName) { | ||
| try { | ||
| var path = ConsentDecisionLogReader.PathFor(daemonName); | ||
| return $"{StatOf(path + ".1")}|{StatOf(path)}"; |
There was a problem hiding this comment.
On a normal installation, consent-decisions.jsonl.1 does not exist until the live log first exceeds the rotation threshold. StatOf(path + ".1") therefore throws and collapses the entire pair to the constant key "absent", so creating or appending to the current log never looks like a stat change. This can leave the Activity tab empty or stale indefinitely—especially when the ack-triggered refresh runs before the daemon appends its record—until the user hides and reopens the tab or the first 1 MB rotation occurs. Represent each missing file independently so changes to the live file still alter the polling key.
Useful? React with 👍 / 👎.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+14
to
+17
| /// Four guards carry the reviewed reasoning: | ||
| /// | ||
| /// * <b>EntryAdded is the FIRST SURFACING of a PromptId, never a new cache key</b> — the signal | ||
| /// is the raise trigger (spec §6), so it has to mean "a request the user has not been offered |
There was a problem hiding this comment.
1. consentservice comment block too long 📘 Rule violation ⚙ Maintainability
New multi-paragraph doc/comments were added that restate design rationale in-line, which reduces readability and increases maintenance overhead. The compliance checklist requires keeping comments minimal and favoring self-explanatory code.
Agent Prompt
## Issue description
Verbose multi-paragraph comments were introduced (especially in `ConsentService` and `ActivityViewModel`) where clearer naming/structure (and shorter, targeted comments) would better convey intent.
## Issue Context
PR Compliance requires comments to be brief and necessary, avoiding long narrative blocks.
## Fix Focus Areas
- src/Capacitor.App/Services/ConsentService.cs[10-40]
- src/Capacitor.App/ViewModels/ActivityViewModel.cs[17-32]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The V1CodecReject script only read the 5-byte frame header before closing the connection, leaving ConsentResolveV2's JSON payload unread in the kernel socket buffer. Closing a socket with unread data queued sends RST on Linux (not macOS), so ubuntu-latest CI observed ECONNRESET (daemon_unreachable) instead of the intended clean EOF (unexpected_reply). The real v1 FrameCodec.ReadAsync always reads header AND payload before Decode ever throws on the unknown type byte, so the fake now does the same. ConsentSubscriptionTests' own v1 fake is left as-is (its subscribe frame has an empty payload, so header-only already is the full frame) with a comment noting the caveat for non-empty frames. Spec §10 updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ActivityStatKey wrapped both consent-decisions.jsonl.1 and the live file in one try/catch, so a missing .1 (every fresh install until the first 1MB rotation) threw and collapsed the whole key to the "absent" constant — appends to the live file never changed it, and the Activity tab went stale until the tab was reselected. Each file now gets its own try/catch, joined into one key. ActivityStatKey also gained a two-path overload so tests can point it at a temp directory without touching the real daemon-dir resolution. ActivityViewModel.SafeRefresh also called the injected read Func synchronously from UiTicker ticks on the UI thread. The stat check and log read now run off the UI thread via Task.Run, with a minimal single-flight guard (a trigger arriving while a read is in flight is dropped, not queued — the next tick re-checks) and an explicit Dispatcher.UIThread hop back for the state mutation and Apply. Tests updated to run under AvaloniaSession and await the new PendingRefreshForTesting seam instead of assuming synchronous completion; added a test for the single-flight drop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ggers_immediate_refresh The async rewrite dropped the reader.ReadCalls == 1 assertion after the awaited refresh, leaving only readCallsBefore==0 and Rows.Count==1 — an accidental double-read would still leave Rows.Count at 1 and pass unnoticed. Also adds a one-line comment on the _refreshInFlight disarm noting its thread affinity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alexeyzimarev
deleted the
alexeyzimarev/ai-1652-consent-prompt-window-activity-feed
branch
August 10, 2026 10:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #503. AI-1652.
The consent UX for the desktop supervisor app — the piece that answers the original "daemon launches without asking" complaint on machines running the app.
What's in here
App (
src/Capacitor.App)ConsentService: status-driven subscription gated on the newconsent/2capability, identity-guarded pending cache, service-lifetime tombstones (ghost-replay defense), prune hygiene, one-resolve-at-a-time lane. The shared 1 Hz ticker is hoisted into an app-lifetimeUiTicker.Wire/daemon hardening (all additive)
prompt_idrequest identity on pendings, echoed on resolves, atomically claimed by the broker — a stale resolve can never decide a different launch that reused the agent id (the daemon makes no id-non-reuse guarantee).ConsentSubscribeV2 = 17,ConsentResolveV2 = 18): a pre-upgrade daemon's codec rejects them before routing, so mixed-version consent fails closed on the wire — no capability-check TOCTOU across daemon restarts.consent/2is advertised for discovery only.rule_savedon consent acks: the handler deliberately persists an "Allow & remember" rule before resolving (durable trust), so an already-decided outcome now discloses the installed rule instead of hiding it. The button says "Allow & remember" — not "Always allow" — because earlier deny rules (including Pause) shadow the appended allow.requester_displaythreaded through the consent pipeline (gate input → prompt request → pending DTO → decision record) so prompts and the feed show names, notgithub:…ids.Process
Spec survived a 7-round hosted-Codex review (
docs/superpowers/specs/2026-08-08-ai1652-consent-prompt-activity-feed-design.md); implemented as 11 SDD tasks with per-task reviews, two task-level fix rounds, and a whole-branch final review whose one must-fix (identity-aware raise signal) landed in the final fix wave. App suite 307/307; consent-focused CLI suites green; AOT publish clean.🤖 Generated with Claude Code