Skip to content

Clear the attention badge once a Claude question is answered - #1063

Merged
alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/question-attention-badge
Sep 21, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/question-attention-badge

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

Closes #1061 — AI-3017

What & why

For a Claude AskUserQuestion the hook's request id is broadcast but never written to the session stream, and the transcript uuid is written but never broadcast; the stream also trails the pings by the watcher's one-second poll. SessionAttentionTracker reads the stream after the response ping, finds the question recorded and unresolved, and holds the rail's ! with nothing left to remove it. A response naming an id the set does not hold now settles the session's transcript questions — held, or listed by the read it triggers — and a settled id stays settled.

Where to look

Same rule and same cost as the web UI: a parallel subagent's permission, answered before the tracker read it, clears an open question's mark early. A lost connection or a later pending ping withdraws the claim on the next read, so a question asked meanwhile still lights — a failing read's retries keep that window open far longer than the debounce.

Verification

SessionAttentionTrackerTests: 5 of the 6 new tests fail without the tracker change; 14/14 pass with it. Capacitor.App.Tests.Unit: 2648 passed, 0 failed. Not exercised in the running app.

🤖 Generated with Claude Code

No ping names a transcript question, and the stream resolves it only after the response ping, so the tracker can settle it neither by id nor by re-reading. As in the web UI, a parallel subagent's quickly answered permission can clear an open question's mark early.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Clear attention after Claude questions are answered

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Clear session attention when responses cannot match Claude transcript question IDs.
• Preserve settled questions across stale snapshots while resetting assumptions after disconnects.
• Add race, persistence, reconnection, and mixed-permission regression coverage.
Diagram

sequenceDiagram
    participant Hook as Claude Hook
    participant Lane as Server Lane
    participant Tracker as Attention Tracker
    participant Stream as Session Stream
    participant Badge as Attention Badge
    Hook->>Lane: Response ping
    Lane->>Tracker: Unmatched request ID
    Tracker->>Tracker: Settle known questions
    Tracker->>Badge: Clear question attention
    Tracker->>Stream: Reconcile session
    Stream-->>Tracker: Pending transcript UUID
    Tracker->>Tracker: Suppress settled UUID
    Tracker->>Badge: Keep badge clear
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Correlate hook and transcript IDs upstream
  • ➕ Enables exact question removal by identifier.
  • ➕ Eliminates tracker-specific inference and early clearing from unrelated subagent responses.
  • ➖ Requires broader protocol and persistence changes across hook, broadcast, and stream pipelines.
  • ➖ May require compatibility handling for existing clients and stored sessions.
2. Wait for transcript resolution
  • ➕ Keeps the tracker dependent only on authoritative stream state.
  • ➕ Avoids treating unmatched responses as question answers.
  • ➖ Retains the visible badge delay and can leave attention stuck if resolution is never recorded.
  • ➖ Polling timing still permits stale snapshots after responses.

Recommendation: Use the PR's tracker-level settlement as the focused fix because it matches existing web behavior and handles stale or permanently unresolved stream records. Upstream ID correlation would be the cleaner long-term model, but its substantially broader protocol impact is not justified for this targeted bug fix.

Files changed (3) +166 / -7

Bug fix (1) +38 / -3
SessionAttentionTracker.csSettle transcript questions on unmatched responses +38/-3

Settle transcript questions on unmatched responses

• Tracks transcript questions separately and removes them when a response cannot be matched to a known request ID. Remembers settled question UUIDs across stale snapshots, clears that state on identity changes, and withdraws pending settlement assumptions after disconnects.

src/Capacitor.App/Services/SessionAttentionTracker.cs

Tests (1) +107 / -4
SessionAttentionTrackerTests.csCover Claude question settlement and stale snapshots +107/-4

Cover Claude question settlement and stale snapshots

• Adds transcript-question fixtures and five regression tests covering delayed stream records, persistent stale questions, later distinct questions, and mixed question-permission sessions. Refactors snapshot helpers to represent permission and question interrupt kinds explicitly.

test/Capacitor.App.Tests.Unit/SessionAttentionTrackerTests.cs

Documentation (1) +21 / -0
CHANGES.mdDocument unmatched-response question settlement +21/-0

Document unmatched-response question settlement

• Explains why Claude question ping IDs and transcript UUIDs cannot be matched directly. Documents settlement persistence, reconnect behavior, and the accepted parallel-subagent tradeoff.

docs/CHANGES.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f1ed230bb

ℹ️ 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".

return;
}
SettleQuestions(s);
s.SettlesQuestions = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop settlement at a newer pending ping

When a new PermissionPending for the same session arrives before this response-triggered reconciliation completes (within the debounce window or during a retry), OnPending reuses the session without clearing SettlesQuestions. The next snapshot therefore treats every transcript question it contains as answered, including a question issued after this response, so that question never lights the attention badge. A later pending ping must end or supersede this blanket settlement claim.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 0e6bccb: OnPending now withdraws the claim. The debounce alone made this nearly unreachable for one agent, but a failing read keeps the claim armed through its retries (up to 30 s apiece), so a question asked meanwhile was settled unseen and never lit. A_question_asked_while_the_answers_read_is_retrying_still_lights reproduces it and failed before the change. The trade, recorded in docs/CHANGES.md: a question the set did not yet hold stays lit when a parallel prompt lands inside that window, which is the cheaper of the two errors.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-20T13:53:38.944429Z 8f1ed23 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

qodo-code-review Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. A field comment repeats its meaning ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
Questions is documented only as “The members of Ids that are transcript questions,” which
restates the field’s name and its collection relationship. Because the declaration already expresses
that role and the comment records no invariant or trap, later readers gain no behavior-critical
guidance.
Code

src/Capacitor.App/Services/SessionAttentionTracker.cs[R23-24]

+        /// The members of Ids that are transcript questions.
+        public readonly HashSet<string> Questions = new(StringComparer.Ordinal);
Relevance

●●● Strong

Recent comment-minimality precedents accept removing redundant field comments under rule 2762993.

PR-#904
PR-#1052

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2762993 permits comments only when they document non-obvious, behavior-critical
constraints. The added comment at lines 23-24 merely paraphrases the Questions field and its
relationship to Ids.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.App/Services/SessionAttentionTracker.cs[23-24]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The XML comment on `Questions` merely restates the field's evident purpose without documenting a non-obvious behavioral constraint.

## Fix Focus Areas
- src/Capacitor.App/Services/SessionAttentionTracker.cs[23-24]

## Recommended Fix
Remove the redundant XML comment, or replace it only if there is a non-obvious invariant about how `Questions` must remain synchronized with `Ids` that maintainers need to preserve.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Question history grows without bound ✗ Dismissed 🐞 Bug ➹ Performance
Description
_settledQuestions permanently retains every answered transcript-question identifier and
reconciliation only reads or adds entries without removing them. A long-lived desktop process that
answers questions across many sessions accumulates this historical state until the account changes
or the application exits.
Code

src/Capacitor.App/Services/SessionAttentionTracker.cs[R37-39]

+    /// Outlives its session's entry: the stream may never record the resolution, and the ids are
+    /// never reused.
+    readonly HashSet<string> _settledQuestions = new(StringComparer.Ordinal);
Relevance

●●● Strong

Same-day precedent accepted fixing process-lifetime retention of unbounded identifier-derived state.

PR-#1055

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Settlement unions identifiers into the global set, while reconciliation only checks or adds them and
never retires resolved or ended-session entries. The tracker is created once during application
setup, and the only runtime clearing path is an authenticated-subject change, so ordinary session
completion does not release this history.

src/Capacitor.App/Services/SessionAttentionTracker.cs[37-39]
src/Capacitor.App/Services/SessionAttentionTracker.cs[98-103]
src/Capacitor.App/Services/SessionAttentionTracker.cs[141-146]
src/Capacitor.App/Services/SessionAttentionTracker.cs[213-224]
src/Capacitor.App/App.axaml.cs[581-589]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The global `_settledQuestions` set grows for the tracker’s entire lifetime because answered question IDs are never retired after their session ends or disappears.

## Fix Focus Areas
- src/Capacitor.App/Services/SessionAttentionTracker.cs[37-39]
- src/Capacitor.App/Services/SessionAttentionTracker.cs[98-103]
- src/Capacitor.App/Services/SessionAttentionTracker.cs[213-227]

## Recommended Fix
Scope settled-question tombstones by session and remove a session’s tombstones when reconciliation confirms that the session ended or was not found. Preserve tombstones while a live session can still return its unresolved transcript entry, and add tests proving both stale-question suppression and cleanup after session termination.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: f47dc3dc)
Review mode: ⚖️ Balanced: This modifies stateful session-attention reconciliation and connection behavior with meaningful edge cases, but remains a focused single-path change rather than a broadly bug-dense multi-path diff.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.App/Services/SessionAttentionTracker.cs Outdated
Comment thread src/Capacitor.App/Services/SessionAttentionTracker.cs
A failing read keeps the claim armed through its retries, so a question asked meanwhile would be settled unseen and never light on a session only the tracker reports.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit 06a33b9 into main Sep 21, 2026
8 checks passed
@alexeyzimarev
alexeyzimarev deleted the alexeyzimarev/question-attention-badge branch September 21, 2026 07:29
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.

Clear the rail's attention badge once a Claude question is answered

1 participant