Add an opt-in setting for suppressing surfaces while the session host is focused - #282
Add an opt-in setting for suppressing surfaces while the session host is focused#282weilinzung wants to merge 2 commits into
Conversation
561d93f to
6aa7276
Compare
f72796b to
6d98e6e
Compare
6d98e6e to
fda75aa
Compare
783f215 to
3c59c5d
Compare
|
@weilinzung 这里的实现存在问题,VS Code 可见当前还不代表终端可见含义,可以单独开个设置项作为这个逻辑的开关 |
3c59c5d to
6e30e49
Compare
|
@erha19 感谢 review,已按建议改好:
另外两点想请你确认:
已 rebase 到 |
|
@weilinzung CI 报错了,修复一下 |
… is focused Reviewer feedback on the previous revision: VS Code being frontmost does not carry the meaning of "terminal visible", so this logic should not ride on smartSuppression. smartSuppression is left exactly as it was — same key, same subtitle, same localisations, same gate on the pending-session auto-open, still asking only whether a terminal window is visible on the current space. Host focus is a different question with a wider blast radius, since it also governs approvals, questions and sounds, so it gets its own setting, suppressWhenSessionHostFocused, defaulting off. An install that never touches it behaves as before, which a persistence test pins. With the setting on, and the session's own terminal or IDE frontmost, the Island withholds attention presentation, both completion-notification queues, and notification sounds. Suppression stops short of prompts the Island exclusively owns: with routePromptsToTerminal off the bridge blocks and the client renders nothing, so islandOwnsBlockingPrompt vetoes suppression there rather than stall the agent behind a closed-notch indicator. Resolving the host needed two fixes. Sessions do not always carry a pid — an agent running as an IDE extension reports none in its hook metadata — so the host is matched from SessionClientInfo.hostBundleIdentifier, which reads terminalBundleIdentifier and falls back to bundleIdentifier. Helper bundles fold onto their host, and only registry-recognised bundles count, which keeps the Island's own bundle from suppressing its approvals. Where a pid does exist, isSessionFocused compared the frontmost application pid against a terminal pid resolved by process name, which cannot work when the agent binary lives inside the IDE's own extension directory; ownership now comes from the session's parent chain, with name matching kept as a fallback. Matching is bundle-level, so two windows of one IDE cannot be told apart. The ownership veto bounds that: a blocking approval always surfaces regardless.
The nine-term `+` chain building `keys` made Swift give up with "unable to type-check this expression in reasonable time", which failed the Xcode test build on every PR that merged against main. `+` is overloaded across String, Array, RangeReplaceableCollection and the numeric types, so a nine-term chain of differently-shaped flatMap/map results leaves the solver a large space to search with no annotation to anchor it. Annotating `keys` as [String] and appending each group in its own statement gives every statement a known result type, so each one type-checks on its own. Same keys, same order, same assertions.
6e30e49 to
9f9eb5d
Compare
done. |
erha19
left a comment
There was a problem hiding this comment.
这版整体方向没问题,设置持久化、宿主识别和新增策略测试也都通过了;但目前还有两个会影响实际行为的阻塞项:
-
[HIGH] detached 模式没有接入审批/提问抑制。
PingIsland/UI/Views/NotchView.swift:1077-1100只覆盖 docked 路径;PingIsland/UI/Window/DetachedIslandWindowController.swift:1334-1351的presentExistingAttentionIfNeeded()和:1354-1372的handleManualAttentionChange()仍会直接展开 bubble,既不读取suppressWhenSessionHostFocused,也不调用shouldSuppressManualAttentionPresentation/islandOwnsBlockingPrompt。而 detached 时NotchView会被shouldSuppressAutomaticPresentation提前挡住,所以没有第二层兜底。结果是开启新开关、会话宿主位于前台时,悬浮模式的审批/提问仍自动弹出,与设置文案和本 PR 写入 AGENTS.md 的行为约定不一致。请让 detached 的“已有 attention”和“新 attention”两条入口复用同一 suppression policy,并补上 setting on/off 及 Island 独占 prompt 的回归测试。 -
[HIGH] 被抑制的 attention 会提前吞掉全部完成通知。
PingIsland/UI/Views/NotchView.swift:1071在异步宿主判定之前调用clearCompletionNotifications(keepPanelOpen: true);如果随后在:1089-1097命中 host-focus suppression,函数直接返回,但队列中的通知已经由markCompletionNotificationConsumed永久消费。这样,正在前台查看 session A 时,一个被抑制的审批/提问可能把 session B 的完成提醒也静默吃掉。请把清理动作移到确认确实会展示 manual attention 之后,或至少只处理同一 session 的通知,并补一个多会话队列回归测试。
验证:PR head 9f9eb5d 的 GitHub Validate 为绿色;本地定向 Xcode tests(ManualAttentionSuppression、ProcessTreeBuilder、AppSettingsPersistence、SettingsWindowController)和 Prototype tests 均通过。上述问题属于当前测试没有覆盖到的跨 presentation/queue 接线缺口。
Revised after review: this no longer touches

smartSuppression.What changed
New opt-in setting
suppressWhenSessionHostFocused— 「查看会话应用时抑制」 — default off. While the session's own terminal or IDE is frontmost, the Island withholds its automatic surfaces.smartSuppressionpending auto-opensmartSuppressionkeeps its key, subtitle, localisations and gate — still terminal-visibility only. You were right that VS Code being frontmost does not carry that meaning, and it also should not have pulled approvals and sounds under a toggle promising neither.Two things worth reviewing
Suppression stops at prompt ownership. With
routePromptsToTerminaloff the bridge blocks and the client renders nothing, so the Island holds the only copy of an approval.SessionState.islandOwnsBlockingPromptvetoes suppression there — otherwise enabling the setting could strand an agent behind a notch indicator. This pairs naturally with 「保留终端中的提问与审批」.One change is not behind the toggle.
isSessionFocusedcompared the frontmost app pid against a terminal pid resolved by process name, which can never match an IDE: an agent binary under~/.vscode/extensions/…/claudematchesisTerminalitself, so resolution stopped on the agent. Ownership now comes from the parent chain (ProcessTreeBuilder.isAncestor). That repair applies unconditionally, so it slightly changes existing sound behaviour for IDE-hosted sessions. Happy to gate it too if you would rather nothing moves without the opt-in.Sessions also may carry no pid at all — an IDE-extension agent reports none — so the host is matched from
SessionClientInfo.hostBundleIdentifier(terminalBundleIdentifier, falling back tobundleIdentifier; neverterminalProgram, which can be stale). Helper bundles fold onto their host, and only registry-recognised bundles count, keeping Island's own bundle out.Limitations
await, so they use a synchronous check and skip the tmux pane refinement. tmux otherwise keeps pane-level resolution.Verification
Rebased on
415cc8c, clean over the locale-consistency commit. One settings key added, none removed, nothing renamed, no localisation lines removed.swift test --package-path Prototypexcodebuild -only-testing:PingIslandTestsAppSettingsPersistenceTestsDetachedIslandWindowControllerTests.testNewAttentionSessionAutoOpensBubbleInFloatingModeis timing-bound (asserts insideasyncAfter(0.05)) and fails intermittently here under suite load. It is structurally outside this change: its session carries an intervention, soisCompletedReadySessionrejects it and the gate added here is never reached.