') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(desktop): keep the composer + menu still when toggling Plan by MatrixA · Pull Request #3469 · apache/maka · GitHub
Skip to content

fix(desktop): keep the composer + menu still when toggling Plan - #3469

Merged
Astro-Han merged 4 commits into
apache:mainfrom
MatrixA:MatrixA/fix-plan-click-flicker
Aug 22, 2026
Merged

fix(desktop): keep the composer + menu still when toggling Plan#3469
Astro-Han merged 4 commits into
apache:mainfrom
MatrixA:MatrixA/fix-plan-click-flicker

Conversation

@MatrixA

@MatrixAMatrixA commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Clicking Plan in the composer's + menu made the open menu visibly blink: the toggle re-fetches the invocable-Skill projection, the fail-closed clear leaves mentionSkills empty for the length of the IPC round trip, and the Skills row read that transient [] as "no skills available" — graying out and growing a description line, then snapping back. useComposerMentions now also returns a settled mentionSkillsUnavailable verdict that only moves when a request resolves; the Skills row renders from it, while the / popup keeps reading the fail-closed list. A second commit removes the pending-keyed disable that dimmed the Plan/Swarm/Graph rows and the footer mode marks to half opacity during a toggle's commit round trip — the pending registries already swallow re-entrant toggles, so the disable protected nothing and only blinked.

Root cause

use-composer-mentions.ts clears the list while re-fetching on purpose (a visible / popup must never advertise a stale Skill), and newSessionCollaborationMode is a dependency of that effect, so length === 0 could not tell "re-fetching" from "nothing to offer". The same blip fired in sessions via the mode-change event, and on model or turn changes.

Verification

Before — the menu jumps on the Plan clickAfter — the menu holds still
Clicking Plan makes the open + menu grow a "no skills" line and jumpClicking Plan toggles the checkbox with the menu geometry unchanged
  • packages/ui: 192/192 unit tests pass, including three new tests pinning the Skills row's presentation (refreshing ≠ empty; settled-empty keeps its caveat; populated stays quiet).
  • apps/desktop: 1060/1060 main-process tests pass; typecheck and biome lint clean on every touched file.
  • e2e: composer-plus-menu-stability.spec.ts now carries three specs. The stability spec arms an in-page MutationObserver before the click and asserts the menu stays open, keeps one height, and never shows the transient "no skills" line or a grayed row. Two review-round specs hold IPC in-flight windows open via a preload latch (isolated-E2E gate only): a Skills click mid-refresh does nothing and works once settled, and two rapid Plan toggles land on the last requested state. All three fail without their fix; adjacent composer specs (skill invocation, slash-command menu, send-message) still pass, 9/9 total.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Claude Fable 5) diagnosed the flicker and authored the fix, unit tests and e2e spec; both commits carry Generated-by: Claude Code trailers.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

🤖 Generated with Claude Code

MatrixAand others added 2 commits August 22, 2026 15:12
Toggling Plan (or changing the model or mode) re-fetches the
invocable-Skill projection, and the fail-closed clear made the + menu's
Skills row read the in-flight `[]` as "no skills available": it grayed
out and grew a description line for the length of the IPC round trip, so
the open menu visibly jumped on every Plan click — in a new chat and in a
session alike.
The hook now also returns a settled verdict, `mentionSkillsUnavailable`,
that only moves when a request resolves. The Skills row renders from that
verdict, so its presentation holds until the catalog's emptiness actually
changed, while the `/` popup keeps reading the fail-closed list. Hosts
that never clear the list mid-flight can omit the flag; the row then
falls back to `length === 0` as before.
The new e2e spec arms an in-page MutationObserver before the click and
fails on the pre-fix build with exactly this signal.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generated-by: Claude Code
The pending registries already swallow re-entrant Plan/orchestration
toggles, so disabling the + menu rows and the footer mode marks for the
length of the commit round trip protected nothing — it dimmed them to
half opacity and back on every click, a gray blink in the very menu the
user was looking at (three sequential IPCs when a session is active).
Drop the pending-keyed disable and the `modeChanging` reason; the rows
repaint once the write lands. The pending state stays as a write-only
store because the shared pending-action helpers need one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generated-by: Claude Code

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing the visible menu jump and removing the unnecessary pending repaint. The direction is good, and both findings below are non-blocking P2s. They cover two short in-flight windows where the controls now look actionable but the underlying authority cannot honor the click.

A clean final state would give the Skill catalog one explicit loading/empty/populated representation, and either expose a real pending mode state without changing menu geometry or preserve the user’s latest toggle intent.

AI-assisted review disclosure: Codex coordinated independent state-authority and UI/test review passes against exact head 1beb6d2b399856bc39ac73e60efafc1d32dee2e1; I reviewed the consolidated evidence for this comment.

Comment threadapps/desktop/src/renderer/use-composer-mentions.ts Outdated
Comment threadapps/desktop/src/renderer/app-shell.tsx
…st mode ask
Review follow-up on both P2s:
The catalog is now one explicit value — loading, settled empty, or
settled populated — held in a single piece of state so a refresh cannot
tear its facets apart. `mentionSkillsUnavailable` still paints the +
menu's Skills row from the last settled verdict; the new
`mentionSkillsLoading` gates what a click DOES. Mid-refresh (including
the very first fetch, where nothing has settled and the row previously
looked actionable) the row ignores clicks and stops closing the menu, so
nothing writes a stray `/` against the fail-closed list. The held state
is observable as `maka-composer-skills-loading`.
A mode toggle landing while a commit is in flight is no longer silently
dropped: each session keeps the LATEST asked value, and the in-flight
commit's finally block applies it when the settled state does not
already satisfy it — a quick "on, then off" now finishes off, for Plan
and orchestration alike.
A new preload affordance under the isolated-E2E gate latches single
bridge calls, so the two new e2e specs hold these in-flight windows open
deterministically; both fail without their fix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generated-by: Claude Code

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing the two earlier behavior regressions — the menu now keeps its geometry and the latest toggle intent wins on the main interaction path. Two independent passes rechecked this exact head. I kept one accessibility P2 and a few small lifecycle/simplification edges inline; the clean final state is one keyed lifecycle representation for both the catalog and queued mode intent, with loading exposed semantically rather than only visually.

AI-assisted review disclosure: OpenAI Codex coordinated two independent review passes. I verified the retained findings against this exact head and the current live PR state, and I made the final review decision.

Comment threadpackages/ui/src/composer.tsx
Comment threadapps/desktop/src/renderer/use-composer-mentions.ts Outdated
Comment threadapps/desktop/src/renderer/app-shell.tsx
Comment threadapps/desktop/src/renderer/app-shell.tsx Outdated
…holds
Second review round — one P2 and three P3s:
The geometry-stable Skills row now carries `aria-busy` while the catalog
refreshes, via a new vendor seam: `DropdownMenuItem` forwards the
attribute to its row (patches/README.md). A row that held its look but
announced plain "available" would silently swallow the activation it
defers. The activation gate moved inside one always-present handler —
swapping `onClick` between undefined and a function remounts Item's
internals and drops keyboard focus mid-refresh.
The catalog value now names its context: a session/project/model switch
renders once before the passive effect clears the list, and deriving
through the key makes that very render fail closed instead of showing
the old context's Skills for a frame. A same-context refresh still holds
its settled verdict; a context switch has nothing settled to hold.
Queued mode intents die with the Session's renderer lifecycle:
`clearSessionRendererState` drops them alongside the pending
registries, so an in-flight commit's finally cannot replay an old ask
against a removed Session.
The write-only pending states are gone — the registry ref is the one
re-entrancy authority, and the pending-action helpers take the setter
only where something renders from it (permission, model, retry).
New coverage: keyboard activation is a no-op mid-refresh and works once
settled; a context switch re-enters loading instead of holding the old
catalog; deleting the session while a toggle is pending settles clean;
SSR pins `aria-busy` on and off.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generated-by: Claude Code
@MatrixA
MatrixA requested a review from Astro-HanAugust 22, 2026 09:57

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks — the approach here is the part worth calling out: this fixes the jitter by removing state rather than adding a correction for it.

What this solves / how: toggling Plan made the composer's + menu shift, and the Skills row could briefly show the previous context's skills or swallow the loading state entirely. Rather than compensating for the movement, this removes what caused the re-render — the write-only pending mode state — and makes the skill catalog fail closed on a context-key mismatch so a stale list can't paint for a frame.

Review notes:

  • Keying the skill catalog on contextKey (session/project/model/mode/target) and returning an empty list on mismatch is the right default: showing nothing briefly is honest, showing another context's skills is not.
  • Dropping setPendingCollaborationModeBySession and friends is the substantive change. Pending state that only ever gets written and never read still costs a render, and that render was the blink. Holding it in the registry ref instead means the value is still available where it's needed without driving the view.
  • aria-busy with the gate inside the handler rather than a remount — keeps focus where the user left it.
  • Net effect is two concepts gone (the write-only pending states) plus the write-implies-rerender coupling, with the responsibility moving to the registry ref. Nothing was added to compensate.

Worth noting for anyone reading this later: the tempting fix for "menu jumps" is a position offset, which would have left both the stale-render and the focus-loss problems in place while adding a third thing to maintain. This went after the cause instead.

LGTM.


AI-assisted review. Verified against this exact head with CI green.

@Astro-Han
Astro-Han merged commit 404759b into apache:mainAug 22, 2026
1 check passed
Sign up for freeto 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.

2 participants

@MatrixA@Astro-Han