Skip to content

feat(notifications): excerpts, mute, MCP dismiss + since_timestamp#39

Closed
artyomsv wants to merge 4 commits into
masterfrom
feat/notifications-informativeness
Closed

feat(notifications): excerpts, mute, MCP dismiss + since_timestamp#39
artyomsv wants to merge 4 commits into
masterfrom
feat/notifications-informativeness

Conversation

@artyomsv

@artyomsv artyomsv commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

Make the notification sidebar genuinely informative (the user's main concern was "the side notification window is not very informative") and close the gaps in the MCP API surface. Eight planned items from the notifications evaluation done end-to-end.

Sidebar informativeness (the user's main concern)

  • Every event now carries the last few stripped output lines that triggered it, populated at all four emit sites (idle, bell, OSC 133;D, process_exit).
  • The sidebar renders the excerpt as a 4th line per card (dim grey, blank when no excerpt). Each event takes 4 lines so pagination math stays predictable.
  • Helpers: paneOutputExcerpt(pane, n) reads the trailing 4 KiB of the ring buffer, ANSI-strips, returns the last n non-empty lines. withExcerpt(event, excerpt) populates Message + Data["excerpt"] idempotently.
  • MCP get_notifications returns the same excerpt in data.excerpt so agents skip a follow-up read_pane_output round-trip.

Per-pane mute (Alt+M)

  • Pane.Muted (persisted in workspace.json, restored on daemon start) suppresses every event from a pane at the source — not just in the UI. Solves npm test --watch flooding the sidebar.
  • MsgUpdatePane.Muted *bool (pointer tristate so unset is distinguishable from explicit false).
  • Pane border shows [muted] chip when active.

MCP API

  • dismiss_notifications (new tool): pass event_id to ack one, omit to clear all. Closes the read-only-queue gap where MCP-only sessions accumulated forever.
  • watch_notifications gains since_timestamp: pass the Unix ms of the last event you handled; the daemon scans the queue oldest-to-newest and returns the first newer event immediately, only registering a blocking watcher if none qualifies. Closes the race between agent action and watcher registration.

Cleanups

  • Default notification.max_events raised from 50 to 200 (~60 KB at full).
  • Dead [[notification_handlers]] code path is documented as deprecated in internal/plugin/scraper.go:50 and never evaluated — now emits a one-shot warning per stale plugin at load time. TOML is still accepted for back-compat.
  • Active-pane output_idle events are suppressed in the TUI dispatcher (Model.isActivePane). Other event types still queue on the active pane as a session audit trail.
  • docs/mcp.md corrected: idle matches come from [[idle_handlers]], not [[notification_handlers]]. Tool count bumped 17 → 18.
  • Defensive nil-guards on Daemon.broadcastState and emitEvent let unit tests construct a bare daemon without spinning up the IPC server.

Test plan

  • go vet ./... clean
  • go test ./... — all 17 packages pass
  • go test -race ./... — all packages pass under race detector
  • New tests:
    • event_excerpt_test.go — 10 cases for paneOutputExcerpt + withExcerpt
    • event_findsince_test.go — 5 cases for eventQueue.FindSince (oldest-first, strict inequality, filter, no-match, empty queue)
    • event_mute_test.go — 4 cases (muted drops, unknown-pane still emits, empty-pane-id still emits, MsgUpdatePane toggles muted)
    • notification_excerpt_test.gofirstNonEmptyLine table + 3 sidebar render cases
    • notification_active_pane_test.go — 6 cases (idle suppressed on active, idle queued on background, process_exit + bell always queue, isActivePane edge cases)
  • Manual: build & dev-mode smoke (./scripts/dev.sh build then ./quil-dev.exe), verify:
    • Alt+M shows [muted] chip and suppresses events for the active pane
    • Sidebar event cards show one excerpt line under the title
    • Two excerpt scenarios: regex-matched idle (claude-code "Needs your approval") + bell-from-vim

artyomsv added 4 commits June 5, 2026 19:11
Make the notification sidebar genuinely informative and close gaps in
the MCP API. Notification events now carry the last few stripped output
lines that triggered them, so the sidebar can render context (4-line
cards: separator + name/time + title + excerpt preview) and MCP agents
no longer need a follow-up read_pane_output per event. Per-pane mute
(Alt+M, persisted in workspace.json) drops events at the source for
chatty processes like `npm test --watch`. Active-pane output_idle
events are suppressed TUI-side as redundant. The MCP surface gains
dismiss_notifications (ack from the agent side) and watch_notifications
since_timestamp (closes the race between kicking off a task and
registering the watcher). Defaults bumped to 200 max events. Dead
notification_handlers code path now emits a deprecation warning at
plugin load instead of failing silently; docs corrected.
Address three signal-quality issues spotted while testing the sidebar:

1. ANSI sequence fragments leaked into excerpts. The trailing 4 KiB
   slice of the ring buffer could begin mid-CSI — the `\x1b[` ended up
   in the discarded prefix but parameters like `2;30;30;30m` or `;18H`
   survived into the window and ansi.Strip rendered them as plain text.
   New `trimToNewlineSafe` helper advances the slice start to the next
   newline before stripping, applied in both `paneOutputExcerpt` and
   `analyzeIdleTitle`.

2. Output_idle events whose excerpt collapses to a single shell prompt
   rune (%, $, >, ❯, #, ➜, λ, ») are now suppressed when the default
   "Output idle" title fired. Shells idling at a prompt are not a state
   change worth notifying — the user can see the prompt by looking at
   the pane. Plugin idle handlers (Claude "Needs your approval" etc.)
   still fire because their non-default title means the regex saw
   something meaningful.

3. eventQueue.Push aggregates repeat (PaneID, Title) events: the new
   event reuses the prior event's ID, bumps Data["count"], and replaces
   it at the front of the queue. The TUI sidebar's AddEvent updates the
   matching card in place and bubbles it to position 0. The title line
   now renders a ×N badge when count > 1. Replaces N near-duplicate
   cards saying the same thing with one bumping card.
Field test surfaced two issues my first round of follow-ups missed:

1. ANSI fragments STILL leaked in claude-code panes (`2;30;30;30m|
   Thinking: ...`, `0mConfig/Build files:- ...`). Root cause: the
   newline-only seek in trimToNewlineSafe fell through on TUIs that
   emit one screen paint with no newline inside the trailing 4 KiB
   window. The bounded scan now also accepts an ESC byte (0x1b) as a
   clean boundary — ansi.Strip handles a full sequence starting at
   ESC, even if the next newline is far away. Scan is bounded to 512
   bytes so it stays cheap.

2. Output-idle events with `%` excerpts kept firing — both panes had
   their cards collapsed to `×2` even though my isPromptOnlyExcerpt
   covers the bare `%` case. The screenshot shows what the sidebar
   renders (first non-empty line), not what the daemon's idle checker
   sees (the full 5-line excerpt). Add a debug-level log line at every
   idle decision (pane id, type, title, suppress verdict, excerpt
   prefix) so we can see why the check returns false on real-world
   shells. With the dev build's ldflags-forced debug level, this
   surfaces in the next test run's quild.log.
Field diagnosis with debug logs confirmed the real shape of the bug.
The terminal-pane idle excerpts logged in quild.log look like:

  "%   [padding]   \r \r\rArtjoms_Stukans@EPCHZURW03..."

That's: prompt rune, padding spaces, CR-return-to-start, more CRs, then
OSC 0 window-title content that leaks through ansi.Strip (the strip
state machine appears to bail on an embedded CR inside the OSC payload).
The sidebar then renders just "%" because that's the first non-empty
line — but the *actual* terminal-visible content is the hostname leak,
not the prompt that was immediately overwritten by the CRs.

Two coordinated changes:

1. lastNLines now applies per-line CR-reset semantics — splitting a line
   at the last `\r` and keeping only the trailing segment. This matches
   what the terminal would actually display, so excerpts no longer
   capture text the user can never see (the pre-CR prompt) and miss
   the text they DO see (the post-CR window title).

2. isPromptOnlyExcerpt is now suffix-based and recognises hostname-like
   patterns. A line is "prompt-like" when, after trimming, it ends with
   a recognised prompt rune (covers `%`, `user@host % `, `~/repo $ `,
   etc) OR matches a user@host pattern (the OSC 0 leak signature). Long
   lines (> 200 chars) are presumed to be command output regardless of
   trailing characters — real prompts are short.

The two changes interact: after CR-reset the excerpt's visible content
is the hostname leak; the new prompt classifier recognises that pattern
and suppresses correctly. `ls` output still survives suppression because
its line endings don't match prompt patterns. Verified against the
field-logged excerpts.
@artyomsv

artyomsv commented Jun 8, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #41 — combined with PR #40 (broadcast hardening) into a single PR with clean linear history (foundation-first).

@artyomsv artyomsv closed this Jun 8, 2026
artyomsv added a commit that referenced this pull request Jun 8, 2026
 + #40) (#41)

## Summary

Combines PR #39 (notifications informativeness) and PR #40 (broadcast
hardening) into a single PR with clean linear history. Broadcast wedge
defense lands first as the foundation; notification UX work builds on
top.

Closes #39, closes #40.

## What's in here

### Phase A — broadcast hardening (foundation)
- Per-conn 64-slot buffered send goroutine; `Broadcast` is non-blocking
- Marshal once, share frame across N conns with `slices.Clone` defensive
copy
- CAS-guarded overflow: one log, one async close per slow conn, zero
broadcast stall
- 4 KiB per-event Message cap + **1 KiB per-Data-value cap** (raised
from 128 B to fit multi-line `data.excerpt`)
- Reserved `_quil_truncated` namespace flag with compile-time invariant
guard
- 30 s `SetWriteDeadline` belt-and-suspenders
- `Server.ConnCount()` test accessor
- `logger.Warn` / `logger.Error` for new code (project leveled logger)

### Notifications informativeness
- Events carry excerpts (`Message` + `data.excerpt`); sidebar renders
excerpt under title
- Per-pane mute (`Alt+M`, persisted in workspace.json)
- Active-pane idle suppression (TUI-side filter)
- `(PaneID, Title)` aggregation with `×N` badge; AddEvent updates in
place + moves to front
- `\r`-reset cleanup in `lastNLines` (handles OSC 0 window-title leak)
- Prompt-only excerpt suppression (recognises `%`, `$`, `❯`, hostname
patterns)
- ANSI boundary scan also seeks ESC byte (handles claude-code TUI's
no-newline regions)
- Default `notification.max_events` raised 50 → 200
- Dead `[[notification_handlers]]` emits one-shot deprecation warning on
plugin load

### MCP surface
- New `dismiss_notifications` tool — agents can ack events
- `watch_notifications` gains `since_timestamp` — closes the race
between agent action and watcher registration
- All events carry `data.excerpt` with full multi-line content (fits
within 1 KiB Data cap)

## Commit list (8 commits, in foundation-first order)

1. `54125eb` fix(ipc): non-blocking broadcast + 4 KiB per-event size cap
2. `8b177af` fix(ipc): address review findings on PR #40
3. `af85f36` feat(notifications): excerpts, mute, MCP dismiss +
since_timestamp
4. `8711c14` fix(notifications): ANSI boundary, prompt-only suppress,
aggregation
5. `01d2d70` fix(notifications): seek ESC byte in ANSI trim, log idle
decisions
6. `5340412` fix(notifications): CR-reset cleanup + OSC hostname leak as
prompt
7. `2e38d1a` fix(daemon): raise Data value cap to 1 KiB for multi-line
excerpts

## Test plan

- [x] `go vet ./...` clean
- [x] `go test ./...` — all 17 packages pass
- [x] `go test -race ./...` — all packages pass under race detector
- [x] Comprehensive code review (security-officer, code-reviewer,
rules-compliance, qa) on the broadcast hardening portion; all
Critical/High/Medium findings addressed in commit `8b177af`
- [x] 14 new tests in scope (broadcast resilience, conn internal, size
cap boundary tests, excerpt rendering, mute, aggregation, find-since,
active-pane suppression)
- [ ] Manual smoke (when reviewed): launch dev TUI, exercise `Alt+M`
mute, verify sidebar excerpts render under titles, verify `×N` badges
aggregate, kick off slow MCP client and confirm daemon stays responsive

## What's left for PR #41

- Phase B + C + D from the hook-events plan
(`~/.claude/plans/velvety-bubbling-twilight.md`): the JSONL spool
watcher, Claude/OpenCode hook script extensions registering the
opinionated event tier, config knobs. Independent of this PR.
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.

1 participant