Repository navigation
fix(freshclaude): reliable alert bell, hidden-pane approvals, saved default model, and needs-attention marks on mobile - #862
Merged
Conversation
…bort release A chime that the browser or OS pauses without firing 'ended' held the ring queue for the life of the page, silencing every later bell. Each ring now arms a 5 s safety watchdog (logged at warn as notification_ring_watchdog_fired) that only backstops progression; an Audio 'pause' releases the queue at once and an 'abort' falls back to the built-in tone. The timer is cleared on normal finish and on unmount. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move one ring's chime, fallback, pause/abort and watchdog handling into a module-level createRingAttempt with explicit start/dispose, so the hook only owns the queue. Unmount disposes the active ring, which clears its watchdog and guarantees no late completion drains the queue. No behavior change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…thout blocking A hidden pane neither refetched nor marked itself dirty for permission and question requests or their cancellations, so after reveal the card could take up to the 3 s busy poll to appear (or a cancelled card lingered). Marking the pane transcript-dirty would be wrong: the blocking "Refreshing conversation" overlay clears only on a newer transcript revision, which an approval never produces. A hidden pane now records that it owes a surface refresh for those events and, on reveal, runs one non-blocking 'reveal-surface' snapshot refresh. A pane that is also transcript-dirty lets the blocking reveal refresh cover it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the snapshot-, transcript- and surface-invalidating event sets and their predicates out of FreshAgentView into fresh-agent-event-classes.ts, with one pure classifyHiddenPaneEvent (transcript | surface | none) that the WS handler uses for both the dirty-mark and the owed surface refresh. A table-driven unit test covers every event type. No behavior change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ault model The model saved for Freshclaude/Kilroy (model dialog or settings popover) was ignored: new panes always used the registry default opus[1m], and the resume path hardcoded the registry default for every fresh provider. One shared resolveFreshAgentPaneModelDefaults now seeds the model for new panes (PaneContainer) and resumed panes (buildResumeContent, used by the sidebar, context menu, handoff, tabs thunk and terminal-to-fresh resume): - claude/kilroy: saved fresh-pane default, then the registry default; the terminal Claude CLI model is deliberately not consulted (decision D1); - codex/opencode: saved default, then the runtime CLI model, then the registry default (unchanged for new panes). Resumed panes start on the saved default (decision D2), which also makes codex resume honor the saved codex default. A no-layout tab restored by TabContent now passes the saved provider settings too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…w panes
A saved catalog-only Freshclaude/Kilroy default (one absent from the
static opus table) started new panes with effort normalized against the
opus table, so a model with no effort levels was still sent 'high'.
The model dialog and the settings popover now save the levels they
already stamp on the pane as freshAgent.providers[type].modelEffortLevels
({modelId, levels}, null clears). The shared resolver returns them only
for claude and only when paired with the resolved model, PaneContainer
and resume content stamp them as the pane's modelEffortLevels, and
resolveFreshAgentPaneCreateEffort clamps against them. The clamp is one
extracted clampEffortToStampedLevels shared with
getEffectiveFreshAgentEffort. A Rust settings-store test pins that the
opaque providers subtree keeps and clears the field.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Browser e2e for G11 against the real Rust server and the fake Claude-SDK sidecar: the saved Freshclaude/Kilroy default is written through PATCH /api/settings, a picker-created pane's sidecar create carries it, and a pane resumed from the sidebar after the default changed carries the new default (decision D2), asserted on the exact stdin create frames. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the building blocks every tab surface will share for the needs-attention mark: memoized selectors for the style-gated union of attentionByTab and watchedCompletionByTab in tab order (tabAttentionSelectors), one tabAttentionClassName owning the emerald, darken and pulse tokens per surface, the 'N other tabs need attention' wording, and a TabAttentionIndicator that renders a polite status (or an aria-hidden dot inside a labelled control). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a phone the tab strip never showed that a tab finished or is waiting.
It now uses the same style-gated mark as the desktop tabs: the active
tab's title takes the attention pill (described to assistive tech as
'This tab needs attention'), a polite status counts the other marked
tabs ('N other tabs need attention'), and the previous/next chevrons get
a dot when a marked tab lies in that direction. tabAttentionStyle 'none'
hides every mark; existing control labels are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tab-switcher cards now carry the same style-gated needs-attention mark as the desktop tabs: the attention card treatment, a visible 'Needs attention' badge, and ', needs attention' appended to the card's accessible name. A tap goes through a new shared activateTabFromSurface thunk, so in click dismiss mode it dismisses the tab's and its panes' attention before activating it, including on the already-active card (which previously never dismissed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry tab surface
TabBar's tab click and the Stream Deck's focusTabFromDeck now dispatch
activateTabFromSurface instead of each repeating the 'dismiss in click
mode, then activate' rule, and TabItem takes its attention classes from
tabAttentionClassName('tab'). No behavior change; the existing TabItem
class tests, TabBar tests and deck tests pin it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…docs Browser e2e for G17 at 390x844: on a real Freshclaude pane (real Rust server, fake Claude-SDK sidecar), a turn-complete edge and an approval waiting edge delivered as the exact server wire frames while the user is in another tab both produce the strip's '1 other tab needs attention' status and the Previous-chevron description; the switcher card is named '..., needs attention', and tapping it activates the tab and clears the attention. docs/index.html's mobile notes describe the marks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gged row The G11 spec failed on the cloud backend at the sidebar click: once the pane closed, the conversation's row vanished. Three setup gaps, none of them the behaviour under test: - the isolated home had no ~/.claude/projects at boot, so the session directory never indexed the transcript written after boot; - a transcript with one user message is classified non-interactive and filtered out (the hideEmptySessions config entry was not the cause, and is dropped); - a brand-new Claude-runtime pane's session has no sessionType record, so the row would reopen as a Claude CLI terminal. The spec now records the flavour through POST /api/session-metadata and waits for the row's data-session-type before clicking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e announced The mobile strip's "N other tabs need attention" status mounted and unmounted with the count, and carried its meaning only in aria-label over aria-hidden content. A live region inserted already filled, or one whose only change is an attribute, is generally not announced, so screen-reader users were never told when another tab needed attention. The polite status now stays mounted (empty at zero) and holds the label as visually hidden text next to the aria-hidden dot and count, so every change is a text change inside an existing region (aria-atomic). The unit tests pin that the same element carries the text as the count moves 0 -> 1 -> 2 -> 0, and the mobile e2e finds the status by its text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The strip tests asserted only each chevron's accessible description, which hangs off the same flag as the visible dot but separate JSX, so dropping the dot left them green. They now assert the aria-hidden dot on the chevron that points at a marked tab, its absence on the other chevron and under style 'none', and that it follows the darken style. Disabling both dot renders now fails three tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Desktop tabs still derived their mark separately: TabBar passed attentionByTab as a needsAttention prop and TabItem ORed it with selectTabStripAttention, while the mobile strip and switcher read the new tabAttentionSelectors module, whose per-tab selector went unused. A change to the union rule in one place could make desktop and mobile disagree. The union rule now lives once, in tabAttentionSelectors (selectTabStripAttention moves there and folds into selectTabNeedsAttention). TabItem reads selectTabNeedsAttention itself; the redundant needsAttention prop and TabBar's attentionByTab subscription are gone. The TabItem tests seed the store instead of passing the prop, and gain a case for another tab's marks. TabAttentionIndicator's style prop is renamed attentionStyle so it no longer shadows React's style prop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AGENTS.md said the next bell starts only when the previous chime ends or the fallback tone completes. The ring queue now also releases on a paused chime, substitutes the tone for an aborted one, and has a 5 s per-ring watchdog that logs notification_ring_watchdog_fired. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mobile-attention spec said the server's attention-edge emission is pinned by fresh-agent-error-turn-attention.spec.ts, but that spec is red on origin/main (fake-sidecar sends never arrive), so nothing green covers "server edge -> mobile mark". The default-model spec tags the session's flavour through the API because new claude panes never record it, a real product gap the comment presented as a setup step. Both comments now say so and name what to change once those gaps close. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… on reveal G26 had no browser coverage. The new spec raises, then cancels, a Bash approval on a Freshclaude pane while its tab is in the background, using the client's own freshAgent.interrupt frame for that session (what the Stream Deck stop sends) as the fake sidecar's trigger, so it needs no composer send. After each reveal it asserts the card is shown, then gone, and that the blocking "Refreshing conversation" overlay is never drawn. Swapping G26's owed surface refresh for the blocking transcript-dirty mark fails the spec on the overlay check. It cannot isolate the owed refresh for Claude: a hidden Claude pane already refetches when its Redux session record changes, which the permission frames do; the header says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No textual conflicts and no files changed on both sides. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mobile attention spec injected the server's attention frames because composer sends to the fake Claude sidecar were broken on main. PR #854 fixed that lane, so the spec now drives both edges through a real turn: the composer sends, the fake sidecar ends the turn (or raises an approval) after a 3 s window in which the user moves to another tab, and the server's edge puts the mark on the mobile strip and switcher. Each step also checks the edge on the sidecar wire and on the client's received WS frames, and that the ending was not watched (a late tab move fails loudly). No injected case is kept: the real turns cover both edges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
danshapiro
added a commit
that referenced
this pull request
Oct 23, 2026
Brings PR-A (freshclaude core runtime: C01-C07 plus review fixes) up to main at fc1024c (#852-#862). Non-trivial conflict resolutions: 1. handle_send's ownership fence (main #854 vs PR-A 827d939/bdfe2658d). One implementation now serves both test suites. PR-A's `claude_owner_key` (shared by kill and send) and `send_owner_key` stay: a reported session is fenced on its durable id, and a supplied observed owner pair is always checked, even before the session is reported (F4). Main's `fence_owner_key` is dropped as a duplicate of that rule. From main's version, handle_send keeps the `self.ownership.is_some()` gate (a lane without a coordinator, the session host's, has nothing to fence and never logs `send_before_session_report`) and the `owner_key` field on the refusal warning. Main's three checkpointed_send tests and PR-A's a_fenced_send_before_the_session_report_is_checked_not_skipped all stay and pass. 2. Sidecar spawn (main #859 ClaudeNodeCommand/SidecarTreeGuard vs PR-A C04 with_sidecar_environment and C07 stderr logging). spawn_sidecar now builds through main's `sidecar_node_command` + `spawn_guarded`, and `SpawnedSidecar` carries main's `tree_guard` next to PR-A's `log_labels`. PR-A's test-support environment moves into the launch recipe as `ClaudeNodeCommand::with_complete_environment`: the configured environment replaces the inherited one, and the ownership tag, OneCLI key flag and managed secrets still layer over it, so each rig sidecar still gets exactly its own environment (new claude_spawn unit tests cover the layering and that nothing is inherited). PR-A's stderr logger replaces main's drain_reader. Two PR-A spawn tests now hold the tree guard until the child is reaped (dropping the guard kills the tree), and main's teardown test uses the SpawnedSidecar struct. 3. Model-catalog probe (main #859 tree guard + timeout field vs PR-A F7 structured logs). The probe spawns through main's catalog_node_command + spawn_guarded and kills the whole tree on timeout; its stderr goes through PR-A's sidecar logger. The timeout's tree confirmation is folded into the single `freshagent.claude.model_catalog_unavailable` warning instead of a second warning. Tests: one staging helper (`stage_catalog_sidecar`, copies every sidecar module, so sdk-env.mjs, log.mjs and hosted.mjs are all present) serves PR-A's F7 tests and main's OneCLI-key and timeout tests. 4. Sidecar JS (main #859 sdk-env.mjs + stdout-error shutdown vs PR-A log.mjs/hosted.mjs). index.mjs and model-catalog.mjs import all three modules; both use main's createClaudeSdkCleanEnv (so the probe now keeps the managed OneCLI key like the session sidecar). Main's stdout-error handler logs through PR-A's structured log as `stdout.closed` (warn, as before) since `logerr` no longer exists. 5. AGENTS.md Agent Status Indicators: main's ring-queue watchdog text and PR-A's Stop-gate (aborted results only) text are both kept. 6. claude_sidecar_interrupt_dispatch.rs: PR-A's stage-every-module helper is kept; it already covers main's added sdk-env.mjs. Main's new streaming-query-module.mjs fixture (stdout-loss test) is kept as is: it runs the real index.mjs through the SDK seam; moving it onto the CLI simulator belongs with C30. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
danshapiro
added a commit
that referenced
this pull request
Oct 23, 2026
…ycle Brings in main's merged work (#852-#862) as PR-A reconciled it: the send fence fix, hosted configure, G28 TranscriptLocation, graceful teardown with SidecarTreeGuard and ClaudeNodeCommand (claude_spawn.rs), sdk-env.mjs, the sidecar's stdout-error shutdown, the durable transcript projection, client alerts and the managed session-host startup. Conflicts and how they were resolved: - crates/freshell-claude-sidecar/index.mjs: both import sets kept (stream-end, boot, terminal-commands, sdk-env). The stdin-close handler keeps PR-B's `exiting` guard. The new stdout-error handler (#859) does the same: a sidecar already exiting on its own (refused boot, unrequested stream end) has closed its query and keeps its own exit code (3 or 2), so the handler only logs; otherwise it shuts down and exits 0 as before. - crates/freshell-freshagent/src/claude.rs: the three session inserts (create, rollback respawn, attach resume) take PR-B's record (no result_idle_pair_pending, the dead-session forget under the lock) plus PR-A's tree_guard. All three start sites already share spawn_sidecar_process (ClaudeNodeCommand + spawn_guarded) and write PR-B's create_line() v2 request. - One reap/teardown path: PR-B's end-of-output path reaped an ended sidecar with its own claude_sidecar_exit::reap (a bounded child.wait), while #859's requested teardown stops through graceful_then_kill and the tree guard. The process stop is now one function, SidecarProcesses::stop in claude.rs (graceful_then_kill, the /proc ownership reap, the tree-death confirmation), used by teardown_removed_session_within (requested: consumer aborted first, map entry removed first, so it stays silent and logs freshagent.claude.teardown) and by the consumer's unrequested-end path (which still records the dead session before the stop, and now reports the exit only once the old tree is confirmed gone; an unconfirmed tree escalates detached through the shared escalate_unconfirmed_teardown). claude_sidecar_exit::reap and its budget are gone; the exit status comes from the stop, and a node the stop had to force reports no status as the cause. freshagent.claude.sidecar_exit now also logs stop and confirmation. - crates/freshell-freshagent/src/lib.rs: both module lists kept. - crates/freshell-session-host/src/providers/fresh_agent.rs: both test sets kept (PR-B's simulated-CLI resume starts, PR-A's hosted configure). PR-B's HostedTransport::from_parts now also takes the broadcast sender PR-A's configure subscribes to (provider_frames). - test/e2e-browser/fixtures/fake-claude-sidecar.mjs: protocol-2 `created` and the boot.resume field (PR-B) with PR-A's FAKE_CLAUDE_SIDECAR_INIT_AT knob. Protocol v2 follow-through for tests PR-A brought in: fakes answer `created` with protocol 2 (checkpointed_send, claude_sidecar_teardown, safe11 shutdown reaping, the hosted configure fake); the rollback G28 test reads the fork from create.boot; the stdout-loss unit test and the pinned CLI shutdown contract send a v2 create (protocol 2, boot fresh), and the streaming query fixture reports initializationResult(). The teardown unit fixture drops result_idle_pair_pending, which PR-B removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Summary
This covers the first two parts (PR-A and PR-B) of the Freshclaude client alerts plan:
What changed
notification_ring_watchdog_firedand releases the queue (src/hooks/useNotificationSound.ts). AGENTS.md lists every way a ring releases the queue.fresh-agent-event-classes.ts, with a table-driven unit test.resolveFreshAgentPaneModelDefaultspicks the model for new panes (PaneContainer) and for resumed panes (buildResumeContent, which the sidebar, context menu, handoff, tabs and terminal-to-fresh resume all use). For Claude/Kilroy the order is: saved fresh-pane default, then the registry default. Codex and OpenCode keep their existing order. The model dialog and the settings popover also save the model's effort levels (freshAgent.providers[type].modelEffortLevels), so a model that has no effort levels is not senthigh. A Rust settings-store test checks that the server keeps this field and clears it.tabAttentionSelectors), one class helper (tabAttentionClassName) and oneTabAttentionIndicator. On the mobile strip, the active tab's title gets the attention pill and the chevrons get a dot when a marked tab is in that direction. A status line that stays mounted reads "N other tabs need attention", so screen readers announce changes. Tab-switcher cards get a "Needs attention" badge and ", needs attention" at the end of their accessible name. Desktop tabs, the switcher and the Stream Deck all use oneactivateTabFromSurfacethunk to dismiss the mark and activate the tab.docs/index.htmldescribes the marks.origin/main. There were no conflicts and no file was changed on both sides.Tests
All runs are from the clean, committed branch head after the merge.
cargo fmt --all --check: pass.cargo clippy -p freshell-server --all-targets -- -D warnings: pass.cargo test -p freshell-server --bin freshell-server settings_store: 74 passed, including the new effort-levels round-trip test.freshell-ws(claude_restore_unavailable,claude_session_rebind,freshagent_claude_attach,freshagent_claude_checkpointed_send,freshagent_claude_kill_after_restart,freshagent_claude_kill_interrupt,freshagent_claude_rollback,rest_claude_identity): 36 passed.pnpm run typecheck: pass.src/files: no new errors. The 5 errors inFreshAgentView.tsxare also onorigin/main.--project=chromium): 25 specs, 151 passed, 0 failed, 0 skipped, 0 flaky. None of these specs is inCLOUD_SKIP_SPECS:fresh-agent-mobile-attention(1),fresh-agent-hidden-approval-reveal(1),freshagent-default-model-rust(2).fresh-agent-error-turn-attention(1).fresh-agent(25),fresh-agent-control-rust(25),harness-03-provider-fixtures(26),tab-management(11),fresh-agent-rollback-rust(9),fresh-agent-rest-resume-rust(7),multirow-tabs(6),freshagent-settings-resume-rust(5),idle-gate-semantics-rust(5),unified-agent-names-freshclaude(5),freshclaude-durable-transcript-rust(4),freshclaude-identity-persistence-rust(4),fresh-agent-mobile(2),fresh-agent-model-dialog-parity(2),freshagent-live-model-convergence-rust(2),freshagent-extras-rust(2),freshclaude-restart-parity-rust(2),freshclaude-first-send-rust(1),freshagent-click-focus(1),fresh-agent-pty-notification-display(1),turn-complete-restart-resume-rust(1).Known follow-ups
fresh-agent-hidden-approval-reveal.spec.tsstill triggers its approval with the client's ownfreshAgent.interruptframe rather than a composer send. It cannot tell G26's owed refresh apart from the refetch a hidden Claude pane already does when its session record changes. The unit tests check the owed refresh directly. The spec header explains this.sessionType), sofreshagent-default-model-rust.spec.tstags it throughPOST /api/session-metadatabefore it resumes from the sidebar. That is a real product gap, not just test setup.🤖 Generated with Claude Code