[jgluck96]

No description provided.

[rickchen007]

Posted by mini on Rick's behalf — Rick is on a one-week substitution experiment and I'm holding his review seat for in-team PRs. Full review artifact (with what-I-did-not-check section) at ~/.mini/artifacts/peer-results/review-stando-pr-9/review.md; happy to share if useful.

Recommendation: request changes. Swift auth core is well-reasoned and the React atomic-design split is mostly clean. Three blockers below; the rest is should-fix / nit / lgtm. Two open questions for you at the bottom.

---

Blockers

1. src/detect-learned-skills.py:10 — runtime orphan. The docstring says "Run from the proactive loop", but git grep "detect-learned-skills" finds zero invokers anywhere — no proactive-loop/SKILL.md entry, no crons.example.json row, no startup wiring. Meanwhile SkillsViewController.swift:144-150 exposes the toggle. Result: users can flip the checkbox but nothing ever runs the detector. Either wire the runtime in this PR, or split the toggle into a follow-up and land the executor first.

2. src/detect-learned-skills.py:217-238 — code-injection seam in generated tools.ts. The generator emits const prompt = `<prompt_template with ${details} substitution>`; where prompt_template is a longest-common-prefix of recent user task strings. If any task contains `, ${...}, or \, the unescaped substring becomes executable JS in the inline-tools runtime once the user enables the generated skill. Same hazard in the description field at line 222. Owner-only doesn't make it safe — owners paste shell snippets and code into tasks. Fix: f'const prompt = {json.dumps(prompt_template)};' and do the ${details} substitution at runtime via String.prototype.replace, not at code-gen time.

3. src/session-handoff.sh:9 — hardcoded contributor-specific path. DEV_REPO="$HOME/Development/core-prod/stando" is your local clone. Downstream git -C "$DEV_REPO" log (:29), the Python sys.path.insert (:40-44), and QUOTA_FILE (:57) all silently fail for anyone else because 2>/dev/null swallows the errors — "Recent Work", "Pending Questions", and "Quota" come up blank. Fix: DEV_REPO="$(cd "$(dirname "$0")/.." && pwd)" or take from an env var with a loud error when missing.

Bonus blocker on the Swift side that's worth surfacing even if it's intentional: src/Sutando/UnifiedMainWindow.swift:96-105, 376-394 introduces an undocumented .accessory ↔ .regular activation-policy promotion (Dock icon + menu bar appear when main window opens, drop back when it closes). That's a real product behavior change not reflected in the PR title/description. Worth calling out in the description. Also: the anyOtherUserFacingWindowVisible check at :391-397 filters on canBecomeKey, which includes transient alerts and save panels — verify the demote doesn't strand the menu bar while a modal is up.

---

Should-fix

ClaudeCodeAuth.swift + SettingsWindow.swift

• No subprocess timeout on probeState / runInstaller. proc.waitUntilExit() at ClaudeCodeAuth.swift:84 and :143 blocks indefinitely. If the CLI or installer hangs, Settings sits on "Checking…" or "Installing…" forever with no cancel affordance. Wrap with a DispatchSourceTimer + Process.terminate() ceiling (~60s for auth status, ~180s for installer).
• ClaudeCodeAuthSession.start() leaves partial state on throw. process = proc runs at line 216 before try proc.run() at line 247. If run() throws, process stays non-nil and isRunning returns true. Practically harmless because the caller nils the session on catch, but the class invariant is wrong. Move process = proc to after try proc.run() succeeds.
• Auth-code input is a plain NSTextField (SettingsWindow.swift:1408), not NSSecureTextField. Channel tokens elsewhere in the same file use the secure variant — the precedent is there. Auth codes are short-lived but visible to screen-recording and accessibility readers in cleartext. Question below in case this is intentional.
• Double-fire race on out-of-band sign-in (SettingsWindow.swift). If the inline panel's exit handler and the 2s polling tick both detect success at the same time, both call applyClaudeCodeState(.signedIn) + showBanner — banner shows twice. In the timer callback, early-return when claudeAuthSession != nil.

React refactor

• Six legacy component dirs are orphaned. keyboard-shortcuts-bar/, legacy-header/, legacy-hero/, legacy-input-bar/, legacy-transcript/, dynamic-region/. Grep across the branch finds zero importers. rm -r them or move under _legacy/ with a comment.
• LegacyTranscript removed in text-only mode is a likely UX regression. ConversationPage.tsx:86-104 renders ConversationStream only when isLive === true. Text-mode users no longer see their submitted message + reply. Question below.
• QuickStartGrid.tsx:21 freezes time-of-day at mount. useMemo(() => pickQuickStarts(new Date().getHours(), connected), [connected]) — if the user keeps the page open across noon or 5pm the "Morning briefing" / "Recap today" cards never rotate. Drop the memo or add a 15-min ticker.
• Zero tests added. Highest-value targets: pickQuickStarts(hour, isLive) in quick-starts.ts:100-122 (pure, branchy); KIND_BY_STATUS mapping in LiveIndicator.tsx:14-21 (one test per enum value); ConversationComposer submit / Enter / initialValue consumption.

detect-learned-skills.py

• No top-level try/except in main(). A corrupted pending-questions.md or permissions error on read_text() kills the whole pass. Wrap and exit-0 so a single bad task file doesn't poison the proactive loop.
• No tests for the clustering math. TF-IDF + cosine + greedy single-linkage drifts silently. Minimal fixture: 4 near-dup tasks that should cluster, 1 outlier that shouldn't, assert cluster count + slug.

Misc

• client/src/index.css:4-8 re-cascades legacy.css into @layer(components) for every page. Correct for the new pill-composer (Tailwind utilities now win), but it changes the cascade for Settings / CoreCLI / Dashboard too. Smoke-test those before merge.
• SkillsViewController.swift:512 and detect-learned-skills.py:30 only agree on the sentinel path when $SUTANDO_HOME equals the repo root. Bundled .app matches; custom-rooted dev setups don't. One-line assert or comment in the Python script pinning the assumption.

---

Nits (optional)

• ClaudeCodeAuth.swift:271-282 — URL regex accepts https://claude.attacker.example/.... Tighten to (claude\.com|claude\.ai|anthropic\.com). Defense-in-depth only.
• ClaudeCodeAuth.swift:227 — stdoutBuffer += chunk never truncates after capturedURL != nil.
• ConversationPage.tsx:109 — onConsumeInitial={() => setPendingChip(null)} isn't memoised but is in ConversationComposer.tsx:66-71's effect deps. useCallback it or drop it from the deps.
• ConversationStream copy button is hover-only — group-focus-within:inline-flex closes the keyboard/SR gap.
• DASHBOARD_ORIGIN = 'http://localhost:7844' hardcoded at ConversationPage.tsx:40. Pre-dates this PR but the frontend convention work is the time to move into const-values/.
• session-handoff.sh:55 — task display went from "first 5 task filenames" to "N pending". Next session has less context. One-line bash change to restore.
• detect-learned-skills.py:151 — MD5 4-char slug = ~65k buckets. Fine now; widen to 6+ chars if N grows.
• SettingsWindow.swift:1257-1258 — two claudeStatusLabel? lines back-to-back; small helper opportunity.

---

LGTMs worth naming

• Token storage: zero Sutando-owned copies. OAuth lives in the claude CLI's keychain; Swift queries metadata only with explicit "do NOT set kSecReturnData". Good defensive code.
• Keychain-ACL false-negative workaround at ClaudeCodeAuth.swift:51-66 is the right escape hatch and well-commented.
• No logging of token material anywhere — grepped print/NSLog/os_log/debugPrint across both files.
• [weak self] discipline in SettingsWindow.swift closures is correct throughout the new code.
• windowWillClose tears down subprocess + polling timer — prevents in-flight claude auth login leak.
• React atomic-design discipline is clean — no atom-from-molecule/organism imports. The one molecule-from-organism reverse import (AppShell.tsx:3) pre-dates this PR.
• quick-starts.ts and app-copy.ts extractions are sensible — not premature.
• Branding asset swap is clean — menubar-source.png replaces the deleted SVG/PNG set; build-app.sh:113-117 and main.swift:716, 1380 updated; no stale references.

---

Two questions

1. Was the tmuxSendKeys "/login" path in the old claudeCodeAction retired intentionally? Users who had the core-CLI pane open and used "send /login to the existing claude session" lose that workflow silently. If intentional — fine, please call out in the PR description.
2. Is removing LegacyTranscript from idle/text-only mode intentional? If yes, where do text-mode replies surface now, and is that surface obvious enough? The Activity panel is the obvious answer but I want to make sure I'm not missing a third surface.

Once the blockers land and you've answered the two questions, this is approve-able with should-fix items either fixed or punted to a follow-up issue.

[bassilkhilo-ag2]

Second opinion — read mini's review above before diving into the diff.

I cross-checked the three blockers against the files and agree with each one. Adding a note on priority ordering since some of the should-fix items compound the blockers:

Fix in this order:

1. Code-injection seam in detect-learned-skills.py (blocker #2) — this is the safety issue; JSON-encode the prompt template before emitting JS
2. session-handoff.sh hardcoded path (blocker #3) — one-liner: DEV_REPO="$(cd "$(dirname "$0")/.." && pwd)"
3. Wire the runtime invoker for detect-learned-skills.py (blocker #1) — crons.example.json entry + SKILL.md section

The should-fix item I'd elevate to near-blocker: LegacyTranscript removal — text-mode users losing their reply surface is a visible regression. Worth a quick answer to mini's Q2 before merge.

The LGTMs in mini's review are solid — token-storage and subprocess-teardown work is particularly clean.

[bassilkhilo-ag2]

Second opinion — read mini's review above before diving into the diff.

I cross-checked the three blockers against the files and agree with each one. Adding a note on priority ordering since some of the should-fix items compound the blockers:

Fix in this order:

1. Code-injection seam in detect-learned-skills.py (blocker Merge/upstream main 2026 05 12 #2) — safety issue; JSON-encode the prompt template before emitting JS
2. session-handoff.sh hardcoded path (blocker fix(distribution): bundle ffmpeg + install skill deps on Background S… #3) — one-liner: DEV_REPO="$(cd "$(dirname "$0")/.." && pwd)"
3. Wire the runtime invoker for detect-learned-skills.py (blocker Merge/upstream main #1) — crons.example.json entry + SKILL.md section

The should-fix I'd elevate near-blocker: LegacyTranscript removal — text-mode reply surface is a visible regression worth clarifying (mini's Q2) before merge.

The token-storage and subprocess-teardown work is particularly clean.

[jgluck96]

Posted by mini on Rick's behalf — Rick is on a one-week substitution experiment and I'm holding his review seat for in-team PRs. Full review artifact (with what-I-did-not-check section) at ~/.mini/artifacts/peer-results/review-stando-pr-9/review.md; happy to share if useful.

Recommendation: request changes. Swift auth core is well-reasoned and the React atomic-design split is mostly clean. Three blockers below; the rest is should-fix / nit / lgtm. Two open questions for you at the bottom.

Blockers

1. src/detect-learned-skills.py:10 — runtime orphan. The docstring says "Run from the proactive loop", but git grep "detect-learned-skills" finds zero invokers anywhere — no proactive-loop/SKILL.md entry, no crons.example.json row, no startup wiring. Meanwhile SkillsViewController.swift:144-150 exposes the toggle. Result: users can flip the checkbox but nothing ever runs the detector. Either wire the runtime in this PR, or split the toggle into a follow-up and land the executor first.
2. src/detect-learned-skills.py:217-238 — code-injection seam in generated tools.ts. The generator emits const prompt = `<prompt_template with ${details} substitution>`; where prompt_template is a longest-common-prefix of recent user task strings. If any task contains `, ${...}, or \, the unescaped substring becomes executable JS in the inline-tools runtime once the user enables the generated skill. Same hazard in the description field at line 222. Owner-only doesn't make it safe — owners paste shell snippets and code into tasks. Fix: f'const prompt = {json.dumps(prompt_template)};' and do the ${details} substitution at runtime via String.prototype.replace, not at code-gen time.
3. src/session-handoff.sh:9 — hardcoded contributor-specific path. DEV_REPO="$HOME/Development/core-prod/stando" is your local clone. Downstream git -C "$DEV_REPO" log (:29), the Python sys.path.insert (:40-44), and QUOTA_FILE (:57) all silently fail for anyone else because 2>/dev/null swallows the errors — "Recent Work", "Pending Questions", and "Quota" come up blank. Fix: DEV_REPO="$(cd "$(dirname "$0")/.." && pwd)" or take from an env var with a loud error when missing.

Bonus blocker on the Swift side that's worth surfacing even if it's intentional: src/Sutando/UnifiedMainWindow.swift:96-105, 376-394 introduces an undocumented .accessory ↔ .regular activation-policy promotion (Dock icon + menu bar appear when main window opens, drop back when it closes). That's a real product behavior change not reflected in the PR title/description. Worth calling out in the description. Also: the anyOtherUserFacingWindowVisible check at :391-397 filters on canBecomeKey, which includes transient alerts and save panels — verify the demote doesn't strand the menu bar while a modal is up.

Should-fix

ClaudeCodeAuth.swift + SettingsWindow.swift

• No subprocess timeout on probeState / runInstaller. proc.waitUntilExit() at ClaudeCodeAuth.swift:84 and :143 blocks indefinitely. If the CLI or installer hangs, Settings sits on "Checking…" or "Installing…" forever with no cancel affordance. Wrap with a DispatchSourceTimer + Process.terminate() ceiling (~60s for auth status, ~180s for installer).
• ClaudeCodeAuthSession.start() leaves partial state on throw. process = proc runs at line 216 before try proc.run() at line 247. If run() throws, process stays non-nil and isRunning returns true. Practically harmless because the caller nils the session on catch, but the class invariant is wrong. Move process = proc to after try proc.run() succeeds.
• Auth-code input is a plain NSTextField (SettingsWindow.swift:1408), not NSSecureTextField. Channel tokens elsewhere in the same file use the secure variant — the precedent is there. Auth codes are short-lived but visible to screen-recording and accessibility readers in cleartext. Question below in case this is intentional.
• Double-fire race on out-of-band sign-in (SettingsWindow.swift). If the inline panel's exit handler and the 2s polling tick both detect success at the same time, both call applyClaudeCodeState(.signedIn) + showBanner — banner shows twice. In the timer callback, early-return when claudeAuthSession != nil.

React refactor

• Six legacy component dirs are orphaned. keyboard-shortcuts-bar/, legacy-header/, legacy-hero/, legacy-input-bar/, legacy-transcript/, dynamic-region/. Grep across the branch finds zero importers. rm -r them or move under _legacy/ with a comment.
• LegacyTranscript removed in text-only mode is a likely UX regression. ConversationPage.tsx:86-104 renders ConversationStream only when isLive === true. Text-mode users no longer see their submitted message + reply. Question below.
• QuickStartGrid.tsx:21 freezes time-of-day at mount. useMemo(() => pickQuickStarts(new Date().getHours(), connected), [connected]) — if the user keeps the page open across noon or 5pm the "Morning briefing" / "Recap today" cards never rotate. Drop the memo or add a 15-min ticker.
• Zero tests added. Highest-value targets: pickQuickStarts(hour, isLive) in quick-starts.ts:100-122 (pure, branchy); KIND_BY_STATUS mapping in LiveIndicator.tsx:14-21 (one test per enum value); ConversationComposer submit / Enter / initialValue consumption.

detect-learned-skills.py

• No top-level try/except in main(). A corrupted pending-questions.md or permissions error on read_text() kills the whole pass. Wrap and exit-0 so a single bad task file doesn't poison the proactive loop.
• No tests for the clustering math. TF-IDF + cosine + greedy single-linkage drifts silently. Minimal fixture: 4 near-dup tasks that should cluster, 1 outlier that shouldn't, assert cluster count + slug.

Misc

• client/src/index.css:4-8 re-cascades legacy.css into @layer(components) for every page. Correct for the new pill-composer (Tailwind utilities now win), but it changes the cascade for Settings / CoreCLI / Dashboard too. Smoke-test those before merge.
• SkillsViewController.swift:512 and detect-learned-skills.py:30 only agree on the sentinel path when $SUTANDO_HOME equals the repo root. Bundled .app matches; custom-rooted dev setups don't. One-line assert or comment in the Python script pinning the assumption.

Nits (optional)

• ClaudeCodeAuth.swift:271-282 — URL regex accepts https://claude.attacker.example/.... Tighten to (claude\.com|claude\.ai|anthropic\.com). Defense-in-depth only.
• ClaudeCodeAuth.swift:227 — stdoutBuffer += chunk never truncates after capturedURL != nil.
• ConversationPage.tsx:109 — onConsumeInitial={() => setPendingChip(null)} isn't memoised but is in ConversationComposer.tsx:66-71's effect deps. useCallback it or drop it from the deps.
• ConversationStream copy button is hover-only — group-focus-within:inline-flex closes the keyboard/SR gap.
• DASHBOARD_ORIGIN = 'http://localhost:7844' hardcoded at ConversationPage.tsx:40. Pre-dates this PR but the frontend convention work is the time to move into const-values/.
• session-handoff.sh:55 — task display went from "first 5 task filenames" to "N pending". Next session has less context. One-line bash change to restore.
• detect-learned-skills.py:151 — MD5 4-char slug = ~65k buckets. Fine now; widen to 6+ chars if N grows.
• SettingsWindow.swift:1257-1258 — two claudeStatusLabel? lines back-to-back; small helper opportunity.

LGTMs worth naming

• Token storage: zero Sutando-owned copies. OAuth lives in the claude CLI's keychain; Swift queries metadata only with explicit "do NOT set kSecReturnData". Good defensive code.
• Keychain-ACL false-negative workaround at ClaudeCodeAuth.swift:51-66 is the right escape hatch and well-commented.
• No logging of token material anywhere — grepped print/NSLog/os_log/debugPrint across both files.
• [weak self] discipline in SettingsWindow.swift closures is correct throughout the new code.
• windowWillClose tears down subprocess + polling timer — prevents in-flight claude auth login leak.
• React atomic-design discipline is clean — no atom-from-molecule/organism imports. The one molecule-from-organism reverse import (AppShell.tsx:3) pre-dates this PR.
• quick-starts.ts and app-copy.ts extractions are sensible — not premature.
• Branding asset swap is clean — menubar-source.png replaces the deleted SVG/PNG set; build-app.sh:113-117 and main.swift:716, 1380 updated; no stale references.

Two questions

1. Was the tmuxSendKeys "/login" path in the old claudeCodeAction retired intentionally? Users who had the core-CLI pane open and used "send /login to the existing claude session" lose that workflow silently. If intentional — fine, please call out in the PR description.
2. Is removing LegacyTranscript from idle/text-only mode intentional? If yes, where do text-mode replies surface now, and is that surface obvious enough? The Activity panel is the obvious answer but I want to make sure I'm not missing a third surface.

Once the blockers land and you've answered the two questions, this is approve-able with should-fix items either fixed or punted to a follow-up issue.

Q1 — tmuxSendKeys "/login" retirement

Yes, intentional. The new ClaudeCodeAuthSession (SettingsWindow.swift:1442) spawns claude auth login in-process, opens the OAuth URL in the browser, and surfaces a paste-the-code panel inline — and crucially it gets auth completion back to the UI via subprocess exit status, which the tmux-send-keys path could never do (Settings had no signal for when /login finished inside the pane).

Trade-off acknowledged: users who relied on "Settings → Sign in forwards /login to my running sutando-core pane so it re-auths in place" now have to run /login in that pane themselves. I'll add this to the PR description:

Settings → Sign in no longer forwards /login to the sutando-core tmux pane. The new flow runs its own claude auth login subprocess and reports completion to the UI. If you had a core-CLI pane open, run /login there directly to keep its history.

Happy to add a "Send /login to sutando-core" follow-up surface (menu item, only shown when pgrep -f sutando-core finds the session) if it's worth keeping the workflow alive — my read is no, the tmux path was fragile and produced no completion signal, but say the word.

Q2 — LegacyTranscript removal in text-only mode

Not intentional — collateral damage from gating ConversationStream on isLive alone. Fixed in [next commit]: stream now renders whenever isLive || entries.length > 0, so a text submit in idle mode immediately swaps the hero out for the chat surface (useTextSubmit already appends to conversationStore synchronously, so there's no flicker between submit and the user bubble appearing).

And no, the Activity panel isn't a viable third surface — it fetches /activity once on tab open and renders commits + completed-task previews from disk history. It doesn't subscribe to the conversation store and doesn't even know about an in-flight text submit. The chat-bubble stream is the only surface, and it's wired correctly now.

Repro path for re-test: idle the page (no voice), type into the composer, hit Send → user bubble appears immediately, assistant bubble lands when the bridge poll returns.

[rickchen007]

Posted by mini on Rick's behalf — Rick is on a one-week substitution experiment and I'm holding his review seat. Re-review artifact at ~/.mini/artifacts/peer-results/review-stando-pr-9/re-review-1.md; happy to share verbatim.

Re-review against 1cdde00. CI is green (tsc + tests 47s) and the three blockers from the prior review are properly resolved — two of them more cleanly than what I asked for. Moving from request-changes toward approve but with three asks before I flip the button.

Blockers — all three resolved

• Merge/upstream main #1 runtime orphan. crons.example.json learned-skills-scan at 07:30 + proactive-loop/SKILL.md step 3a fallback, both gated on the same sentinel. ✅
• Merge/upstream main 2026 05 12 #2 code-injection seam. json.dumps on all three injected strings + {details} substitution moved to runtime via split().join(). Better than the String.prototype.replace I suggested — no regex-escape pitfalls. ✅
• fix(distribution): bundle ffmpeg + install skill deps on Background S… #3 hardcoded path. DEV_REPO="${SUTANDO_DEV_REPO:-$(cd "$SCRIPT_DIR/.." && pwd)}" + a stderr warning when $DEV_REPO/src is missing. ✅ Small follow-up only: the sanity check warns but doesn't bail — if src/ is absent the downstream sections still come up empty (just preceded by a warning). Future hardening question, not a blocker.

Q1 and Q2

• Q1 (tmux /login retirement) — confirmed intentional with good rationale. ✅ The "Settings → Sign in no longer forwards /login" blurb you proposed for the PR description is the right shape. I agree with you on the follow-up: no, don't keep the tmux path alive.
• Q2 (LegacyTranscript text-only regression) — fix verified. showStream = isLive || hasTranscript matches what ConversationStream actually consumes (same useConversation().entries). ✅

Two things to clean up before approve

1. The PR description and title don't reflect what's actually in here

This branch now contains three distinct workstreams:

• Settings ↔ claude parity (the original title) — ClaudeCodeAuth.swift, SettingsWindow.swift, detect-learned-skills.py, atomic-design React refactor. Reviewed.
• ConversationPanels panel-dock rewrite (commit 492658e "ui change") — panel-tab atom deleted, panel-tab-bar molecule deleted, ConversationPanels rewritten from tabbed-inside-<main> to sticky-dock-alongside-the-top-bar with a drop-down drawer. +99 net in that file alone, plus the ConversationPage layout rewire.
• macOS 26 first-launch crash fix (commit 1cdde00 "fix bug for grant opening") — shouldSkipScreenRecordingPrompt gate, runLiveScreenRecordingProbe disabled across versions, canReadForeignWindowNames removed for false-positive reasons, screencapture TCC-registration, new app/dev-cert.sh. About 481 lines of Permissions/Onboarding/main churn. Comments are great — they name the hazard, the version, why each removal happened, and the re-enable conditions.

Could you update the PR title + body to cover all three workstreams (and the activation-policy promotion in UnifiedMainWindow.swift from the first round)? I'm not asking you to split this into three PRs — at this stage of the project the bundling is fine — but the title/description being accurate matters because:

• Reviewers shouldn't have to read commit messages + diff stats to discover that a first-launch crash fix is bundled with a settings refactor.
• When this lands, the commit log is the durable record; "ui changes + bug fixes" against 60 files won't tell anyone six months from now that they got a macOS 26 wizard fix in this merge.

2. The carry-over should-fix list

ClaudeCodeAuth.swift, SettingsWindow.swift, and UnifiedMainWindow.swift are byte-identical to the version I reviewed. That's fine if you're consciously punting, but I want it explicit. Which of these are punt-to-follow-up vs pick-up-in-this-PR?

• Subprocess timeouts on probeState / runInstaller (ClaudeCodeAuth.swift:84, :143).
• ClaudeCodeAuthSession.start() partial state on throw (move process = proc after try proc.run()).
• Auth-code input → NSSecureTextField (SettingsWindow.swift:1408).
• Double-fire race in the 2s polling tick (SettingsWindow.swift:1559-1573).
• Six orphan legacy component dirs (legacy-header/, legacy-hero/, legacy-input-bar/, legacy-transcript/, keyboard-shortcuts-bar/, dynamic-region/).
• QuickStartGrid frozen-time useMemo.
• Activation-policy .accessory ↔ .regular promotion — at minimum, doc it in the PR description; ideally also verify the anyOtherUserFacingWindowVisible check doesn't strand the menu bar while a transient modal is up.

The mechanical Swift ones (timeout, secure field, partial state, double-fire) are five-minute fixes if you want them in this PR. The rest are reasonable follow-ups — happy to file the issues myself if that's the path.

Two things I did not review

• The ConversationPanels panel-dock rewrite. Looks consistent on skim — atomic-design still clean, deleted atoms have no remaining importers — but a UX surface change of that size deserves its own pass and I'd rather flag the gap than rubber-stamp it.
• The macOS 26 fix in Permissions.swift / OnboardingWindow.swift / screen-capture-server.py / app/dev-cert.sh. First-launch flow on a major macOS = customer impact — same caveat. The code reads responsibly but I want eyes on it.

I can take Workstream A (panel-dock) in a follow-up pass if you want. For B (macOS 26 wizard), might be worth asking @bassilkhilo-ag2 — he cross-checked the original blockers and would be a good second set of eyes on a first-launch flow.

---

Net: blocker fixes look great, code quality on the new comments is high, hold for description cleanup + the should-fix punt list. Once that's settled and somebody's read the macOS 26 + panel-dock workstreams, this is approve-able.