From 7ccda3b93d43b06ae396518a0aa8fa02dc8644f7 Mon Sep 17 00:00:00 2001 From: Hisku Date: Tue, 22 Sep 2026 16:53:07 +0100 Subject: [PATCH 1/6] fix(panel): refresh an extension tab when the panel comes back, and let its actions be clicked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reasons the Derby showed stale numbers until something was pressed. The panel is ordered out rather than torn down, so its SwiftUI tree survives being hidden and ExtensionTabView.onAppear — the only caller of tabAppeared — never fires again. Reopening onto a tab you were already on was therefore not an "open" as far as the host was concerned, and the pane kept whatever it last fetched. The 30s schedule usually covered that within a tick, which is why it read as intermittent; an extension declaring onOpen with no intervalSeconds would have stayed frozen indefinitely. Showing the panel now counts as the tab appearing, with a floor at the manifest's own minimum poll interval so toggling the panel cannot spawn a script faster than polling is allowed to. And the footer's action hints were not buttons. The pane is keyboard-driven by design, but it still renders each action with its key cap, which reads as a control — so it gets clicked, nothing happens, and a refresh arriving on the poll a few seconds later looks like the click having worked. That is exactly how this was reported, and a spawn probe on the installed extension confirmed it: three invocations, not one of them carrying --action. They are buttons now, disabled while a fetch is in flight, and each is paired with the row a keypress would have sent. resolve() reads a row action as belonging to the selected row and a document action as belonging to none, so the footer has to agree — otherwise a document action would spawn `--action refresh --row h1` for a row the action was never about. Co-Authored-By: Claude Opus 5 --- .../ExtensionHostTests.swift | 96 +++++++++++++++++++ panel/ExtensionHost.swift | 18 +++- panel/ExtensionTabView.swift | 24 ++++- panel/Panel.swift | 11 +++ 4 files changed, 144 insertions(+), 5 deletions(-) diff --git a/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift b/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift index b5d6802..41a5d3c 100644 --- a/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift +++ b/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift @@ -396,6 +396,26 @@ final class ExtensionHostTests: XCTestCase { visible: true, now: now)) } + // A click on a footer hint has to send what the keypress would. resolve() + // reads a row action as belonging to the selected row and a document action + // as belonging to none, so the pairing the footer renders must agree — a + // document action sent with a row id would spawn `--action refresh --row x` + // for a row the action was never about. + func testADocumentActionCarriesNoRowAndARowActionCarriesItsOwn() { + let json = """ + {"schema":1, + "rows":[{"id":"h1","title":"A","actions":[{"id":"open","label":"Open","key":"o"}]}], + "actions":[{"id":"refresh","label":"Sync now","key":"r"}]} + """ + guard case .success(let document) = ExtensionDocument.parse(Data(json.utf8)) else { + return XCTFail("fixture didn't parse") + } + XCTAssertEqual(ExtensionHost.resolve(key: "r", in: document, selectedRow: "h1")?.row, nil) + XCTAssertEqual(ExtensionHost.resolve(key: "o", in: document, selectedRow: "h1")?.row, "h1") + // With nothing selected a row action isn't reachable at all. + XCTAssertNil(ExtensionHost.resolve(key: "o", in: document, selectedRow: nil)) + } + // MARK: - Opening a tab func testOpeningATabRefreshesItUnlessTheManifestOptedOut() { @@ -407,4 +427,80 @@ final class ExtensionHostTests: XCTestCase { host.tabAppeared("radar") XCTAssertEqual(recorder.calls.map(\.id), ["derby"]) } + + // The panel is an NSPanel that is ordered out, not torn down, so its SwiftUI + // tree survives being hidden and `onAppear` — the only thing that calls + // tabAppeared — does not fire again when it comes back. Reopening onto a tab + // you were already on is therefore not an "open" as far as the host is + // concerned, and the pane shows whatever it last fetched. + // + // This asks whether the scheduled refresh covers that gap on its own. + // The floor is what makes tabAppeared safe to call from both onAppear and + // the panel becoming visible: toggling the panel must not spawn a script + // faster than polling is allowed to. + func testOpeningATabAgainImmediatelyDoesNotRespawn() { + let recorder = Recorder() + let (host, _, _) = host([manifest("derby")], recorder: recorder) + let now = Date() + host.tabAppeared("derby", now: now) + XCTAssertEqual(recorder.calls.count, 1) + + // Within the floor: a second open is the same open. + host.tabAppeared("derby", now: now.addingTimeInterval(1)) + XCTAssertEqual(recorder.calls.count, 1) + + // Past it: a real reopen, and the pane is refetched. + host.tabAppeared("derby", + now: now.addingTimeInterval( + TimeInterval(ExtensionManifest.minimumIntervalSeconds) + 1)) + XCTAssertEqual(recorder.calls.count, 2) + } + + // An extension that asks only for onOpen has no schedule to fall back on, + // so showing the panel onto its tab is the only thing that can refresh it. + func testATabWithNoScheduleStillRefreshesWhenThePanelComesBack() { + let recorder = Recorder() + let (host, _, _) = host([manifest("derby", refresh: "{\"onOpen\":true}")], + recorder: recorder) + let now = Date() + host.tabAppeared("derby", now: now) + XCTAssertEqual(recorder.calls.count, 1) + + // Hidden for ten minutes. No interval, so no tick will ever help. + var later = now + for _ in 0..<120 { + later = later.addingTimeInterval(5) + host.tick(visibleTab: nil, now: later) + } + XCTAssertEqual(recorder.calls.count, 1) + + host.tabAppeared("derby", now: later) + XCTAssertEqual(recorder.calls.count, 2, + "showing the panel is the only refresh this extension gets") + } + + func testReopeningOntoATabYouWereAlreadyOnGetsFreshData() { + let recorder = Recorder() + let (host, _, _) = host([manifest("derby", refresh: "{\"intervalSeconds\":30}")], + recorder: recorder, + result: { .transient("stub") }) + // Opened once, fetched once. + host.tabAppeared("derby") + XCTAssertEqual(recorder.calls.count, 1) + + // Hidden for five minutes: whileFocusedOnly means no polling, by design. + var now = Date() + for _ in 0..<60 { + now = now.addingTimeInterval(5) + host.tick(visibleTab: nil, now: now) + } + XCTAssertEqual(recorder.calls.count, 1, "a hidden pane must not poll") + + // Reopened onto the same tab. onAppear does not fire, so the first tick + // after it becomes visible is the only thing that can catch it up. + now = now.addingTimeInterval(5) + host.tick(visibleTab: "derby", now: now) + XCTAssertEqual(recorder.calls.count, 2, + "reopening onto a stale tab must refetch without a keypress") + } } diff --git a/panel/ExtensionHost.swift b/panel/ExtensionHost.swift index 4cc5a25..1a71018 100644 --- a/panel/ExtensionHost.swift +++ b/panel/ExtensionHost.swift @@ -114,8 +114,24 @@ final class ExtensionHost: ObservableObject { // Opening a tab refreshes it unless the manifest opted out, and unless // something is already in flight. - func tabAppeared(_ id: String) { + // + // "Opening" includes showing the panel onto a tab you were already on, + // which SwiftUI cannot tell us: the panel is ordered out rather than torn + // down, so its view tree survives being hidden and `onAppear` never fires + // again. Without that call the pane shows whatever it last fetched, and an + // extension declaring `onOpen` with no `intervalSeconds` would stay that + // way until the tab was switched away from and back. + // + // The floor is what makes it safe to call from both places. It is the + // manifest's own minimum poll interval, so toggling the panel cannot spawn + // a script faster than polling is allowed to. + func tabAppeared(_ id: String, now: Date = Date()) { guard let manifest = manifest(id), manifest.refresh.onOpen else { return } + if let attemptedAt = pane(id).attemptedAt, + now.timeIntervalSince(attemptedAt) + < TimeInterval(ExtensionManifest.minimumIntervalSeconds) { + return + } refresh(id) } diff --git a/panel/ExtensionTabView.swift b/panel/ExtensionTabView.swift index 18819d4..aa51d03 100644 --- a/panel/ExtensionTabView.swift +++ b/panel/ExtensionTabView.swift @@ -326,16 +326,32 @@ struct ExtensionTabView: View { // Only bound actions get a hint. An action whose key request was // refused has no shortcut and no button, so advertising it would be // a lie — see the note on ExtensionKey. - ForEach(hintedActions, id: \.id) { action in - FooterHint(label: action.label, keys: [Self.keyCap(action.key ?? "")]) + // Clickable, because it looks clickable. The pane was + // keyboard-only by design and the footer still advertised each + // action with its key cap — which reads as a button, so it gets + // pressed, and nothing happens. A refresh arriving on the poll + // thirty seconds later then looks like the click working. + ForEach(hintedActions, id: \.action.id) { hint in + Button { + host.perform(action: hint.action.id, row: hint.row, on: id) + } label: { + FooterHint(label: hint.action.label, + keys: [Self.keyCap(hint.action.key ?? "")]) + } + .buttonStyle(.plain) + .disabled(pane.busy) } } } - private var hintedActions: [ExtensionDocument.Action] { + // Paired with the row each one acts on, so a click sends what the keypress + // would: ExtensionHost.resolve reads a row action as belonging to the + // selected row and a document action as belonging to none. + private var hintedActions: [(action: ExtensionDocument.Action, row: String?)] { guard let document = pane.document else { return [] } let rowActions = document.rows.first { $0.id == pane.selectedRow }?.actions ?? [] - return (rowActions + document.actions).filter { $0.key != nil } + return rowActions.filter { $0.key != nil }.map { ($0, pane.selectedRow) } + + document.actions.filter { $0.key != nil }.map { ($0, nil) } } static func keyCap(_ key: String) -> String { diff --git a/panel/Panel.swift b/panel/Panel.swift index 127a479..65b070d 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -4792,12 +4792,23 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, NSApp.activate(ignoringOtherApps: true) panel.makeKeyAndOrderFront(nil) } + refreshVisibleExtensionTab() return } positionPanel() // re-resolve in case the user moved to a different display NSApp.activate(ignoringOtherApps: true) panel.makeKeyAndOrderFront(nil) usageSurfaceDidChange() + refreshVisibleExtensionTab() + } + + // Showing the panel onto an extension tab is opening that tab, to anyone + // using it. ExtensionTabView.onAppear cannot say so: the panel is ordered + // out rather than torn down, so the view survives being hidden and never + // appears again — leaving the pane on whatever it fetched before. + private func refreshVisibleExtensionTab() { + guard case .extensionTab(let id) = nav.mode else { return } + extensions.tabAppeared(id) } // NSApp.hide hides all our windows AND deactivates the app, so the system From 037e68b3b8a28af85a83b7859f50916c96d602c9 Mon Sep 17 00:00:00 2001 From: Hisku Date: Thu, 24 Sep 2026 09:52:22 +0100 Subject: [PATCH 2/6] fix(panel): tell a tab switch apart from the panel reappearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #197 found the floor I added was the wrong constant, and that applying one floor to both callers regressed something. Both come from the same mistake: treating two different events as one. Switching to a tab is someone asking for that extension. The panel reappearing over the tab they happened to leave it on is not — nobody asked, the window just came back. Only the second was ever broken. So they are separate now. `tabAppeared` has no floor, which is what it had before this branch: an explicit switch always refetches. That matters more than it sounds, because switching away and back is the only manual refresh an extension that declares no action has, and the floor had quietly taken it away. `panelBecameVisible` keeps a floor, and it is the manifest's own interval rather than ExtensionManifest.minimumIntervalSeconds. Those are different numbers and I had reached for the wrong one: the minimum is the fastest any extension is *permitted* to poll — parse clamps declared intervals up to it — not the cadence this one chose. An extension asking for 600s against a rate-limited API was spawned every 5 seconds by someone toggling the panel, 120x what it declared. Reopening sooner than the interval now leaves the pane showing data younger than the extension itself called acceptable, which is what it asked for. The reopen test was also reworked because it did not test the change: it drove only `tick` and passed identically on the base commit. It drives the new path now. All three behaviours are mutation-verified — gutting the fix, restoring the old constant, and reintroducing the floor on the explicit path each turn the suite red. Co-Authored-By: Claude Opus 5 --- .../ExtensionHostTests.swift | 81 +++++++++++++------ panel/ExtensionHost.swift | 50 ++++++++---- panel/Panel.swift | 12 ++- 3 files changed, 101 insertions(+), 42 deletions(-) diff --git a/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift b/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift index 41a5d3c..5725614 100644 --- a/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift +++ b/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift @@ -435,27 +435,58 @@ final class ExtensionHostTests: XCTestCase { // concerned, and the pane shows whatever it last fetched. // // This asks whether the scheduled refresh covers that gap on its own. - // The floor is what makes tabAppeared safe to call from both onAppear and - // the panel becoming visible: toggling the panel must not spawn a script - // faster than polling is allowed to. - func testOpeningATabAgainImmediatelyDoesNotRespawn() { + // Switching to a tab is someone asking for that extension, so it refetches + // every time. It had no floor before the panel-visible path existed and + // must not acquire one now: the tab-switch-away-and-back escape hatch is + // the only manual refresh an extension that declares no action has. + func testSwitchingToATabAlwaysRefetches() { let recorder = Recorder() - let (host, _, _) = host([manifest("derby")], recorder: recorder) + let (host, _, _) = host([manifest("derby", refresh: "{\"intervalSeconds\":600}")], + recorder: recorder) + host.tabAppeared("derby") + host.finish("derby", .transient("stub")) + host.tabAppeared("derby") + XCTAssertEqual(recorder.calls.count, 2, "an explicit switch is never suppressed") + } + + // The panel reappearing over the tab someone happened to leave it on is not + // a request for that extension, so it respects the cadence the manifest + // asked for. + // + // The floor used to be ExtensionManifest.minimumIntervalSeconds, which is a + // different number: that is the fastest any extension is *permitted* to + // poll, not what this one chose. An extension asking for 600s against a + // rate-limited API was spawned every 5s by someone toggling the panel. + func testShowingThePanelRespectsTheManifestsOwnInterval() { + let recorder = Recorder() + let (host, _, _) = host([manifest("derby", refresh: "{\"intervalSeconds\":600}")], + recorder: recorder) let now = Date() - host.tabAppeared("derby", now: now) + host.tabAppeared("derby") + host.finish("derby", .transient("stub")) XCTAssertEqual(recorder.calls.count, 1) - // Within the floor: a second open is the same open. - host.tabAppeared("derby", now: now.addingTimeInterval(1)) + // Well past the schema minimum, nowhere near what the extension asked + // for: the old floor would have spawned here. + host.panelBecameVisible("derby", now: now.addingTimeInterval(30)) XCTAssertEqual(recorder.calls.count, 1) - // Past it: a real reopen, and the pane is refetched. - host.tabAppeared("derby", - now: now.addingTimeInterval( - TimeInterval(ExtensionManifest.minimumIntervalSeconds) + 1)) + host.panelBecameVisible("derby", now: now.addingTimeInterval(601)) XCTAssertEqual(recorder.calls.count, 2) } + func testTheReopenFloorIsTheManifestsInterval() { + XCTAssertEqual(ExtensionHost.reopenFloor(manifest("a", refresh: "{\"intervalSeconds\":600}")), + 600) + // No interval to read, so the schema minimum is the only sensible value. + XCTAssertEqual(ExtensionHost.reopenFloor(manifest("b", refresh: "{\"onOpen\":true}")), + ExtensionManifest.minimumIntervalSeconds) + // A declared interval below the minimum is clamped at parse time, so it + // can never produce a floor under it. + XCTAssertEqual(ExtensionHost.reopenFloor(manifest("c", refresh: "{\"intervalSeconds\":1}")), + ExtensionManifest.minimumIntervalSeconds) + } + // An extension that asks only for onOpen has no schedule to fall back on, // so showing the panel onto its tab is the only thing that can refresh it. func testATabWithNoScheduleStillRefreshesWhenThePanelComesBack() { @@ -463,7 +494,8 @@ final class ExtensionHostTests: XCTestCase { let (host, _, _) = host([manifest("derby", refresh: "{\"onOpen\":true}")], recorder: recorder) let now = Date() - host.tabAppeared("derby", now: now) + host.tabAppeared("derby") + host.finish("derby", .transient("stub")) XCTAssertEqual(recorder.calls.count, 1) // Hidden for ten minutes. No interval, so no tick will ever help. @@ -474,18 +506,20 @@ final class ExtensionHostTests: XCTestCase { } XCTAssertEqual(recorder.calls.count, 1) - host.tabAppeared("derby", now: later) + host.panelBecameVisible("derby", now: later) XCTAssertEqual(recorder.calls.count, 2, "showing the panel is the only refresh this extension gets") } - func testReopeningOntoATabYouWereAlreadyOnGetsFreshData() { + // The reported bug, end to end. An earlier version of this test drove only + // `tick` and so passed identically without the fix — it was exercising the + // pre-existing schedule, not the new path. + func testReopeningOntoATabYouWereAlreadyOnRefetchesWithoutAKeypress() { let recorder = Recorder() let (host, _, _) = host([manifest("derby", refresh: "{\"intervalSeconds\":30}")], - recorder: recorder, - result: { .transient("stub") }) - // Opened once, fetched once. + recorder: recorder) host.tabAppeared("derby") + host.finish("derby", .transient("stub")) XCTAssertEqual(recorder.calls.count, 1) // Hidden for five minutes: whileFocusedOnly means no polling, by design. @@ -496,11 +530,10 @@ final class ExtensionHostTests: XCTestCase { } XCTAssertEqual(recorder.calls.count, 1, "a hidden pane must not poll") - // Reopened onto the same tab. onAppear does not fire, so the first tick - // after it becomes visible is the only thing that can catch it up. - now = now.addingTimeInterval(5) - host.tick(visibleTab: "derby", now: now) - XCTAssertEqual(recorder.calls.count, 2, - "reopening onto a stale tab must refetch without a keypress") + // The panel comes back. onAppear does not fire — the view never left + // the tree — so this call is the only thing standing between the user + // and stale numbers. + host.panelBecameVisible("derby", now: now.addingTimeInterval(5)) + XCTAssertEqual(recorder.calls.count, 2) } } diff --git a/panel/ExtensionHost.swift b/panel/ExtensionHost.swift index 1a71018..59c7361 100644 --- a/panel/ExtensionHost.swift +++ b/panel/ExtensionHost.swift @@ -112,29 +112,51 @@ final class ExtensionHost: ObservableObject { // MARK: - Invocation - // Opening a tab refreshes it unless the manifest opted out, and unless - // something is already in flight. + // The user switched to this tab. An explicit gesture, so it refetches; the + // only gate is a spawn already being in flight. + func tabAppeared(_ id: String) { + guard let manifest = manifest(id), manifest.refresh.onOpen else { return } + refresh(id) + } + + // The panel came back and this tab happened to be the one showing. // - // "Opening" includes showing the panel onto a tab you were already on, - // which SwiftUI cannot tell us: the panel is ordered out rather than torn - // down, so its view tree survives being hidden and `onAppear` never fires - // again. Without that call the pane shows whatever it last fetched, and an - // extension declaring `onOpen` with no `intervalSeconds` would stay that - // way until the tab was switched away from and back. + // SwiftUI cannot tell us this happened: the panel is ordered out rather + // than torn down, so its view tree survives being hidden and `onAppear` + // never fires again. Without this the pane keeps whatever it last fetched, + // and an extension declaring `onOpen` with no `intervalSeconds` would stay + // that way indefinitely. // - // The floor is what makes it safe to call from both places. It is the - // manifest's own minimum poll interval, so toggling the panel cannot spawn - // a script faster than polling is allowed to. - func tabAppeared(_ id: String, now: Date = Date()) { + // Unlike a tab switch this is incidental — nobody asked for this + // extension, the panel just reappeared over it — so it respects the cadence + // the manifest asked for rather than refetching on every toggle. + func panelBecameVisible(_ id: String, now: Date = Date()) { guard let manifest = manifest(id), manifest.refresh.onOpen else { return } if let attemptedAt = pane(id).attemptedAt, - now.timeIntervalSince(attemptedAt) - < TimeInterval(ExtensionManifest.minimumIntervalSeconds) { + now.timeIntervalSince(attemptedAt) < TimeInterval(Self.reopenFloor(manifest)) { return } refresh(id) } + // How stale a pane must be before merely showing the panel refetches it. + // + // The manifest's own interval, not the schema's minimum. Those are + // different numbers and using the minimum was wrong: it is the fastest any + // extension is *permitted* to poll (ExtensionManifest clamps declared + // intervals up to it), not the cadence this one chose. An extension asking + // for 600s against a rate-limited API would have been spawned every 5 + // seconds by someone toggling the panel — 120x what it declared. + // + // Reopening sooner than the interval leaves the pane showing data younger + // than the extension itself called acceptable, which is what it asked for. + // An onOpen-only extension has no interval to read, so it keeps the + // minimum. + static func reopenFloor(_ manifest: ExtensionManifest) -> Int { + max(ExtensionManifest.minimumIntervalSeconds, + manifest.refresh.intervalSeconds ?? ExtensionManifest.minimumIntervalSeconds) + } + func refresh(_ id: String) { invoke(id, action: nil, row: nil) } // A press while busy is ignored rather than queued: the user pressed it diff --git a/panel/Panel.swift b/panel/Panel.swift index 65b070d..2d493ce 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -4802,13 +4802,17 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, refreshVisibleExtensionTab() } - // Showing the panel onto an extension tab is opening that tab, to anyone - // using it. ExtensionTabView.onAppear cannot say so: the panel is ordered + // ExtensionTabView.onAppear cannot see the panel coming back: it is ordered // out rather than torn down, so the view survives being hidden and never - // appears again — leaving the pane on whatever it fetched before. + // appears again, leaving the pane on whatever it fetched before. + // + // Deliberately not tabAppeared. Switching to a tab is someone asking for + // that extension; the panel reappearing over the tab they happened to leave + // it on is not, so this respects the manifest's declared interval and that + // one does not. private func refreshVisibleExtensionTab() { guard case .extensionTab(let id) = nav.mode else { return } - extensions.tabAppeared(id) + extensions.panelBecameVisible(id) } // NSApp.hide hides all our windows AND deactivates the app, so the system From f6dd824d70e52031ee98a1fc8c03c9b4c3fb4b9e Mon Sep 17 00:00:00 2001 From: Hisku Date: Thu, 24 Sep 2026 10:01:45 +0100 Subject: [PATCH 3/6] fix(panel): resolve a clicked action by key, like the keypress does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #197 found two ways a click and a keypress disagreed, and one fix closes both. The button carried an action id and a row id frozen into its closure at render time. The keyboard path re-reads the selection and looks both up in the *current* document at press time, returning nil when the row is gone. So a document arriving between the render and the mouse-up — `finish` clears selectedRow when the new one drops that row — left the click spawning an action or a row the extension no longer declares. Neither `perform` nor `invoke` validates either. And `resolve` returns the *row* action when a row and a document action share a key, leaving the document one unreachable by keypress. The footer rendered both, each with the same key cap, one of which was a lie — and once they became buttons, it also offered a click for something no key could do. The button now calls `handle(key:on:)`, the same entry point the keyboard uses, so a click is not a second path that agrees with the first — it is the first. And the footer draws one hint per key, in the order resolve picks them, so what is advertised is exactly what is reachable. That also settles a latent ForEach collision the reviewer noted: action ids are deduplicated per list rather than across them, so a row and a document action could both be "refresh" and SwiftUI would see duplicate ids. Keying the ForEach by the key rather than the id makes the collection's identity match the uniqueness the footer now enforces. Mutation-verified: dropping the key dedupe fails testAShadowedDocumentActionIsNotOffered. Co-Authored-By: Claude Opus 5 --- .../ExtensionTabTests.swift | 67 +++++++++++++++++++ panel/ExtensionTabView.swift | 46 ++++++++++--- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/Tests/StackNudgePanelCoreTests/ExtensionTabTests.swift b/Tests/StackNudgePanelCoreTests/ExtensionTabTests.swift index 7b41378..c0ae5dd 100644 --- a/Tests/StackNudgePanelCoreTests/ExtensionTabTests.swift +++ b/Tests/StackNudgePanelCoreTests/ExtensionTabTests.swift @@ -123,4 +123,71 @@ final class ExtensionTabTests: XCTestCase { n.extensionTabs = [ExtensionTab(id: "derby", label: "Derby renamed")] XCTAssertEqual(n.mode, .extensionTab("derby")) } + + // MARK: - Footer hints + + private func document(_ json: String) -> ExtensionDocument { + guard case .success(let d) = ExtensionDocument.parse(Data(json.utf8)) else { + fatalError("fixture didn't parse") + } + return d + } + + // The footer's hints are now buttons, so what it renders has to be exactly + // what the keyboard can reach — otherwise a click does something no key + // can, and a key cap beside it is a lie. + func testAShadowedDocumentActionIsNotOffered() { + let d = document(""" + {"schema":1, + "rows":[{"id":"h1","title":"A","actions":[{"id":"open","label":"Open","key":"o"}]}], + "actions":[{"id":"help","label":"Help","key":"o"}, + {"id":"refresh","label":"Sync","key":"r"}]} + """) + let hints = ExtensionTabView.hintedActions(in: d, selectedRow: "h1") + // "o" belongs to the row action while that row is selected, exactly as + // ExtensionHost.resolve decides it. + XCTAssertEqual(hints.map(\.id), ["open", "refresh"]) + XCTAssertEqual(ExtensionHost.resolve(key: "o", in: d, selectedRow: "h1")?.action, "open") + } + + // With nothing selected the row action is unreachable, so the document + // action stops being shadowed and comes back. + func testTheDocumentActionReturnsWhenNoRowIsSelected() { + let d = document(""" + {"schema":1, + "rows":[{"id":"h1","title":"A","actions":[{"id":"open","label":"Open","key":"o"}]}], + "actions":[{"id":"help","label":"Help","key":"o"}]} + """) + XCTAssertEqual(ExtensionTabView.hintedActions(in: d, selectedRow: nil).map(\.id), ["help"]) + XCTAssertEqual(ExtensionHost.resolve(key: "o", in: d, selectedRow: nil)?.action, "help") + } + + // Every hint the footer draws must resolve to something, or the button is + // an affordance for nothing. + func testEveryHintResolvesToAnAction() { + let d = document(""" + {"schema":1, + "rows":[{"id":"h1","title":"A","actions":[{"id":"open","label":"Open","key":"o"}]}], + "actions":[{"id":"refresh","label":"Sync","key":"r"}, + {"id":"quiet","label":"No key","key":"!"}]} + """) + for selected in [nil, "h1"] { + for hint in ExtensionTabView.hintedActions(in: d, selectedRow: selected) { + XCTAssertNotNil(hint.key, "a keyless action must not be offered") + XCTAssertNotNil(ExtensionHost.resolve(key: hint.key ?? "", in: d, + selectedRow: selected), + "\(hint.id) is drawn but unreachable") + } + } + } + + // A refused key means no binding, so there is nothing to advertise. + func testAnActionWhoseKeyWasRefusedIsNotOffered() { + let d = document(""" + {"schema":1,"rows":[], + "actions":[{"id":"nope","label":"Nope","key":"cmd+r"}, + {"id":"fine","label":"Fine","key":"r"}]} + """) + XCTAssertEqual(ExtensionTabView.hintedActions(in: d, selectedRow: nil).map(\.id), ["fine"]) + } } diff --git a/panel/ExtensionTabView.swift b/panel/ExtensionTabView.swift index aa51d03..d0613a0 100644 --- a/panel/ExtensionTabView.swift +++ b/panel/ExtensionTabView.swift @@ -331,12 +331,20 @@ struct ExtensionTabView: View { // action with its key cap — which reads as a button, so it gets // pressed, and nothing happens. A refresh arriving on the poll // thirty seconds later then looks like the click working. - ForEach(hintedActions, id: \.action.id) { hint in + ForEach(hintedActions, id: \.key) { action in Button { - host.perform(action: hint.action.id, row: hint.row, on: id) + // By key, through the same resolver the keyboard uses, + // rather than a pre-baked (action id, row) pair frozen into + // this closure at render time. The document can swap between + // the render and the mouse-up — `finish` clears + // `selectedRow` when a new document drops that row — and a + // frozen pair would then spawn an action or a row the + // extension no longer declares. Re-resolving makes a click + // and a keypress the same call, not two calls that agree. + host.handle(key: action.key ?? "", on: id) } label: { - FooterHint(label: hint.action.label, - keys: [Self.keyCap(hint.action.key ?? "")]) + FooterHint(label: action.label, + keys: [Self.keyCap(action.key ?? "")]) } .buttonStyle(.plain) .disabled(pane.busy) @@ -344,14 +352,30 @@ struct ExtensionTabView: View { } } - // Paired with the row each one acts on, so a click sends what the keypress - // would: ExtensionHost.resolve reads a row action as belonging to the - // selected row and a document action as belonging to none. - private var hintedActions: [(action: ExtensionDocument.Action, row: String?)] { + private var hintedActions: [ExtensionDocument.Action] { guard let document = pane.document else { return [] } - let rowActions = document.rows.first { $0.id == pane.selectedRow }?.actions ?? [] - return rowActions.filter { $0.key != nil }.map { ($0, pane.selectedRow) } - + document.actions.filter { $0.key != nil }.map { ($0, nil) } + return Self.hintedActions(in: document, selectedRow: pane.selectedRow) + } + + // Exactly the actions the keyboard can reach, in the order it reaches them. + // + // One per key, because ExtensionHost.resolve checks the selected row's + // actions before the document's: a row action shadows a document action + // sharing its key, leaving that one unreachable by keypress. Rendering both + // drew two identical key caps, one of which was a lie — and now that they + // are buttons it would also offer a click for something no key can do. + // + // Keyless actions are absent for the same reason they always were: there is + // no binding to advertise, and docs/extensions.md tells extensions to give + // every action a valid key until that changes. + static func hintedActions(in document: ExtensionDocument, + selectedRow: String?) -> [ExtensionDocument.Action] { + let rowActions = document.rows.first { $0.id == selectedRow }?.actions ?? [] + var seenKeys = Set() + return (rowActions + document.actions).filter { action in + guard let key = action.key else { return false } + return seenKeys.insert(key).inserted + } } static func keyCap(_ key: String) -> String { From 1858293886a27a9a279eac7597ff06f48e2f3470 Mon Sep 17 00:00:00 2001 From: Hisku Date: Thu, 24 Sep 2026 10:11:00 +0100 Subject: [PATCH 4/6] =?UTF-8?q?fix(panel):=20one=20floor=20for=20coming=20?= =?UTF-8?q?on=20screen,=20and=20=E2=8C=98R=20to=20override=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the split from the previous commit. It was wrong, and wrong in the direction that mattered. The split exempted a "tab switch" from the floor and kept it for the panel reappearing. But in compact mode — the default, and effectively the only mode — collapsing to the pill removes the pane from the view tree entirely, so expanding it fires onAppear exactly as switching tabs does. The view cannot tell them apart, so the exemption landed on the path everyone uses and the floor on one almost nobody does: every press of the hotkey spawned a script. That is the rate-limit problem the floor exists to prevent, relocated rather than fixed. So: one floor, on every way a pane comes on screen, at the manifest's own interval. That would have taken away the switch-away-and-back that served as a manual refresh, which is the only one an extension declaring no actions has. ⌘R now refreshes the visible extension tab and ignores the floor. Every ⌘ combination was unhandled in that branch, so it costs no extension a key from its namespace, and it already means reload on the extensions browser. It still yields to a spawn in flight — it overrides the floor, not `busy`. Also corrects an assumption I had argued from: `busy` does not subsume the floor. `busy` covers "still running" and the floor covers "already finished", and a script that exits in a few milliseconds against a main-queue hop lands squarely in the second. Mutation-verified: restoring the old constant and making ⌘R honour the floor each turn the suite red. Co-Authored-By: Claude Opus 5 --- .../ExtensionHostTests.swift | 74 ++++++++++++------- panel/ExtensionHost.swift | 39 +++++----- panel/ExtensionTabView.swift | 4 + panel/Panel.swift | 17 +++-- 4 files changed, 85 insertions(+), 49 deletions(-) diff --git a/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift b/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift index 5725614..3ce5797 100644 --- a/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift +++ b/Tests/StackNudgePanelCoreTests/ExtensionHostTests.swift @@ -435,44 +435,66 @@ final class ExtensionHostTests: XCTestCase { // concerned, and the pane shows whatever it last fetched. // // This asks whether the scheduled refresh covers that gap on its own. - // Switching to a tab is someone asking for that extension, so it refetches - // every time. It had no floor before the panel-visible path existed and - // must not acquire one now: the tab-switch-away-and-back escape hatch is - // the only manual refresh an extension that declares no action has. - func testSwitchingToATabAlwaysRefetches() { + // Coming on screen respects the cadence the manifest asked for, whether it + // was a tab switch, the panel returning, or the pill expanding. The view + // cannot tell those apart — in compact mode the pane leaves the tree + // entirely when the pill collapses, so expanding fires onAppear exactly as + // switching tabs does — and an earlier version exempting "switches" put the + // exemption on the path everyone uses. + // + // The floor is the manifest's own interval, not + // ExtensionManifest.minimumIntervalSeconds. Those are different numbers: + // the minimum is the fastest any extension is *permitted* to poll, not what + // this one chose. An extension asking for 600s against a rate-limited API + // was spawned every 5s by someone pressing the hotkey. + func testComingOnScreenRespectsTheManifestsOwnInterval() { let recorder = Recorder() let (host, _, _) = host([manifest("derby", refresh: "{\"intervalSeconds\":600}")], recorder: recorder) - host.tabAppeared("derby") + let now = Date() + host.tabAppeared("derby", now: now) host.finish("derby", .transient("stub")) - host.tabAppeared("derby") - XCTAssertEqual(recorder.calls.count, 2, "an explicit switch is never suppressed") + XCTAssertEqual(recorder.calls.count, 1) + + // Well past the schema minimum, nowhere near what the extension asked + // for: the old floor spawned here, 120x the declared cadence. + host.tabAppeared("derby", now: now.addingTimeInterval(30)) + XCTAssertEqual(recorder.calls.count, 1) + + host.tabAppeared("derby", now: now.addingTimeInterval(601)) + XCTAssertEqual(recorder.calls.count, 2) } - // The panel reappearing over the tab someone happened to leave it on is not - // a request for that extension, so it respects the cadence the manifest - // asked for. - // - // The floor used to be ExtensionManifest.minimumIntervalSeconds, which is a - // different number: that is the fastest any extension is *permitted* to - // poll, not what this one chose. An extension asking for 600s against a - // rate-limited API was spawned every 5s by someone toggling the panel. - func testShowingThePanelRespectsTheManifestsOwnInterval() { + // ⌘R is the deliberate override, and the only refresh an extension that + // declares no actions of its own has once the floor is in place. + func testForceRefreshIgnoresTheFloor() { let recorder = Recorder() let (host, _, _) = host([manifest("derby", refresh: "{\"intervalSeconds\":600}")], recorder: recorder) - let now = Date() host.tabAppeared("derby") host.finish("derby", .transient("stub")) XCTAssertEqual(recorder.calls.count, 1) - // Well past the schema minimum, nowhere near what the extension asked - // for: the old floor would have spawned here. - host.panelBecameVisible("derby", now: now.addingTimeInterval(30)) - XCTAssertEqual(recorder.calls.count, 1) + host.forceRefresh("derby") + XCTAssertEqual(recorder.calls.count, 2, "⌘R must not be floored") + } - host.panelBecameVisible("derby", now: now.addingTimeInterval(601)) - XCTAssertEqual(recorder.calls.count, 2) + // It is an override of the floor, not of everything: a spawn already in + // flight still wins, because two at once is what busy exists to prevent. + // + // The pane is made busy directly because this harness runs the runner + // synchronously — `tabAppeared` would complete the fetch inline and clear + // busy before the second call, which is the one case the real async path + // has and this one does not. + func testForceRefreshStillYieldsToAFetchInFlight() { + let recorder = Recorder() + let (host, _, _) = host([manifest("derby")], recorder: recorder) + var busy = ExtensionHost.Pane() + busy.busy = true + host.replacePaneForTesting(busy, on: "derby") + + host.forceRefresh("derby") + XCTAssertTrue(recorder.calls.isEmpty, "a spawn in flight still wins") } func testTheReopenFloorIsTheManifestsInterval() { @@ -506,7 +528,7 @@ final class ExtensionHostTests: XCTestCase { } XCTAssertEqual(recorder.calls.count, 1) - host.panelBecameVisible("derby", now: later) + host.tabAppeared("derby", now: later) XCTAssertEqual(recorder.calls.count, 2, "showing the panel is the only refresh this extension gets") } @@ -533,7 +555,7 @@ final class ExtensionHostTests: XCTestCase { // The panel comes back. onAppear does not fire — the view never left // the tree — so this call is the only thing standing between the user // and stale numbers. - host.panelBecameVisible("derby", now: now.addingTimeInterval(5)) + host.tabAppeared("derby", now: now.addingTimeInterval(5)) XCTAssertEqual(recorder.calls.count, 2) } } diff --git a/panel/ExtensionHost.swift b/panel/ExtensionHost.swift index 59c7361..6de818c 100644 --- a/panel/ExtensionHost.swift +++ b/panel/ExtensionHost.swift @@ -112,25 +112,19 @@ final class ExtensionHost: ObservableObject { // MARK: - Invocation - // The user switched to this tab. An explicit gesture, so it refetches; the - // only gate is a spawn already being in flight. - func tabAppeared(_ id: String) { - guard let manifest = manifest(id), manifest.refresh.onOpen else { return } - refresh(id) - } - - // The panel came back and this tab happened to be the one showing. + // The pane came on screen: a tab switch, the panel returning, or the pill + // expanding onto the tab someone left it on. // - // SwiftUI cannot tell us this happened: the panel is ordered out rather - // than torn down, so its view tree survives being hidden and `onAppear` - // never fires again. Without this the pane keeps whatever it last fetched, - // and an extension declaring `onOpen` with no `intervalSeconds` would stay - // that way indefinitely. + // One floor for all of them, because the view cannot tell them apart. In + // compact mode — the default, and effectively the only mode — collapsing to + // the pill removes this pane from the view tree entirely, so expanding it + // again fires `onAppear` exactly as switching tabs does. An earlier version + // of this tried to treat a switch as explicit and exempt it from the floor; + // that put the exemption on the path everyone actually uses and the floor + // on one almost nobody does, so every press of the hotkey spawned a script. // - // Unlike a tab switch this is incidental — nobody asked for this - // extension, the panel just reappeared over it — so it respects the cadence - // the manifest asked for rather than refetching on every toggle. - func panelBecameVisible(_ id: String, now: Date = Date()) { + // ⌘R is the deliberate override. See `forceRefresh`. + func tabAppeared(_ id: String, now: Date = Date()) { guard let manifest = manifest(id), manifest.refresh.onOpen else { return } if let attemptedAt = pane(id).attemptedAt, now.timeIntervalSince(attemptedAt) < TimeInterval(Self.reopenFloor(manifest)) { @@ -139,7 +133,16 @@ final class ExtensionHost: ObservableObject { refresh(id) } - // How stale a pane must be before merely showing the panel refetches it. + // What ⌘R does: refetch now, whatever the floor says. + // + // The floor exists so that showing a window cannot spawn a script, which is + // a thing that happens to a user rather than something they ask for. This + // is the opposite, and without it an extension declaring no actions of its + // own has no way to refresh at all — the floor would otherwise have taken + // away the switch-away-and-back that used to serve as one. + func forceRefresh(_ id: String) { refresh(id) } + + // How stale a pane must be before coming on screen refetches it. // // The manifest's own interval, not the schema's minimum. Those are // different numbers and using the minimum was wrong: it is the fastest any diff --git a/panel/ExtensionTabView.swift b/panel/ExtensionTabView.swift index d0613a0..081c26a 100644 --- a/panel/ExtensionTabView.swift +++ b/panel/ExtensionTabView.swift @@ -320,6 +320,10 @@ struct ExtensionTabView: View { private var footer: some View { PageFooter { FooterHint(label: "Hide", keys: ["Esc"]) + // Advertised because it is the only refresh an extension that + // declares no actions has, now that coming on screen is floored to + // the manifest's own interval. + FooterHint(label: "Refresh", keys: ["⌘R"]) if let document = pane.document, !document.rows.isEmpty { FooterHint(label: "Select", keys: ["↑", "↓"]) } diff --git a/panel/Panel.swift b/panel/Panel.swift index 2d493ce..0ac4b82 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -4164,6 +4164,17 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, if case .extensionTab(let id) = nav.mode { let plain = mods.intersection([.command, .control, .option, .shift]).isEmpty guard plain else { return false } + // ⌘R before the plain guard. Coming on screen is floored to the + // manifest's own interval, so this is the only way to say "now" — + // and it matters most for an extension that declares no actions, + // which would otherwise have no refresh of its own at all. Every ⌘ + // combination was unhandled here, so it costs no extension a key, + // and it already means reload on the extensions browser. + if mods.intersection([.command, .control, .option, .shift]) == [.command], + event.keyCode == KeyCode.rKey, !event.isARepeat { + extensions.forceRefresh(id) + return true + } switch event.keyCode { case KeyCode.escape: hidePanel() @@ -4806,13 +4817,9 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, // out rather than torn down, so the view survives being hidden and never // appears again, leaving the pane on whatever it fetched before. // - // Deliberately not tabAppeared. Switching to a tab is someone asking for - // that extension; the panel reappearing over the tab they happened to leave - // it on is not, so this respects the manifest's declared interval and that - // one does not. private func refreshVisibleExtensionTab() { guard case .extensionTab(let id) = nav.mode else { return } - extensions.panelBecameVisible(id) + extensions.tabAppeared(id) } // NSApp.hide hides all our windows AND deactivates the app, so the system From f77ceeaf5e599bad193e150328ec845cbec1687a Mon Sep 17 00:00:00 2001 From: Hisku Date: Thu, 24 Sep 2026 10:13:27 +0100 Subject: [PATCH 5/6] docs(panel): compact mode is a default, not a forcing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment said compactMode was "always-on now" and "forced to true in loadFromConfig". It isn't: :1405 reads STACKNUDGE_COMPACT_MODE from config with a default of true, and nothing overrides it. Setting that key to false really does give you the full panel. The difference is not academic, and this comment cost real time during review of this PR. The non-compact panel is ordered out rather than torn down, so its view tree survives hiding — which is the entire subject of this branch. "Always-on" reads as "that path is unreachable", and both I and a reviewer took it that way before checking the code under it. Co-Authored-By: Claude Opus 5 --- panel/PanelNav.swift | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/panel/PanelNav.swift b/panel/PanelNav.swift index c1dc3b7..604f8d9 100644 --- a/panel/PanelNav.swift +++ b/panel/PanelNav.swift @@ -1072,9 +1072,13 @@ final class PanelNav: ObservableObject { static let eventsPerSessionOptions: [Int] = [3, 5, 10, 20, 50] // Compact widget mode. Shrinks the panel to a glance-only widget pinned // to a screen corner; clicking it expands back to the full panel. - // Compact mode is always-on now. The compactMode field is kept (and - // forced to true in loadFromConfig) to avoid threading the rest of - // the controller's compact-aware code; just don't expose a toggle. + // Defaults to on, and there is no UI toggle — but it is not forced. This + // comment used to say it was, and loadFromConfig reads + // STACKNUDGE_COMPACT_MODE with a default of true (see :1405), so setting + // that to false really does give you the full panel. The difference + // matters: the non-compact panel is ordered out rather than torn down, + // which is a whole code path — and a bug in it — that "always-on" reads + // as unreachable. @Published var compactMode: Bool = true @Published var compactCorner: CompactCorner = .topRight // When true (default), releasing a drag snaps the pill to the nearest From e54130d4783ce5950ba68f2a0dffaac0f9b8d823 Mon Sep 17 00:00:00 2001 From: Hisku Date: Thu, 24 Sep 2026 10:15:51 +0100 Subject: [PATCH 6/6] docs(panel): say what the compact-mode flag actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third attempt at this comment, which is the point of writing it out. It said compact mode was "always-on" and "forced to true in loadFromConfig". I replaced that with "there is no UI toggle". Both are false: loadFromConfig reads STACKNUDGE_COMPACT_MODE with a default, and Settings → Appearance → Widget flips it and persists it. Anyone who has turned the Widget off carries false durably. I got the second version wrong the same way I got the first one wrong — by taking a description of the code instead of reading it. The reviewer's correction named the config read; I wrote it up without checking whether the other half of the sentence still held, and it did not. The distinction is load-bearing. The non-compact panel is ordered out rather than torn down, so its view tree survives hiding and onAppear never fires again. That is the path this PR's refresh call exists for, and "always-on" invites you to treat it as dead code. Co-Authored-By: Claude Opus 5 --- panel/PanelNav.swift | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/panel/PanelNav.swift b/panel/PanelNav.swift index 604f8d9..2458068 100644 --- a/panel/PanelNav.swift +++ b/panel/PanelNav.swift @@ -1072,13 +1072,20 @@ final class PanelNav: ObservableObject { static let eventsPerSessionOptions: [Int] = [3, 5, 10, 20, 50] // Compact widget mode. Shrinks the panel to a glance-only widget pinned // to a screen corner; clicking it expands back to the full panel. - // Defaults to on, and there is no UI toggle — but it is not forced. This - // comment used to say it was, and loadFromConfig reads - // STACKNUDGE_COMPACT_MODE with a default of true (see :1405), so setting - // that to false really does give you the full panel. The difference - // matters: the non-compact panel is ordered out rather than torn down, - // which is a whole code path — and a bug in it — that "always-on" reads - // as unreachable. + // On by default, and switched off by Settings → Appearance → Widget, which + // persists STACKNUDGE_COMPACT_MODE (see the .widget case in applyCycle). + // loadFromConfig reads it back with a default of true. + // + // Spelled out because two previous versions of this comment were wrong in + // the same direction, and both cost review time on the PR that added the + // extension refresh. It said compact mode was "always-on" and "forced to + // true in loadFromConfig", then that there was no toggle. Neither holds: + // anyone who has turned the Widget off carries false durably. + // + // The distinction is load-bearing rather than pedantic. The non-compact + // panel is ordered out rather than torn down, so its view tree survives + // hiding and onAppear never fires again — a real code path with real users + // that "always-on" invites you to treat as dead. @Published var compactMode: Bool = true @Published var compactCorner: CompactCorner = .topRight // When true (default), releasing a drag snaps the pill to the nearest