From fba8650785250fca8129003f8a2038d2ded7b5ae Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 18:35:28 +0100 Subject: [PATCH 01/11] fix(GenesisHub): the hub's session list refreshes every minute while visible, so a working session no longer reads idle with its launch-time age (cherry picked from commit 90c2ea5c7a709cc2db9bfee790b797a0114d34ad) --- .../Sources/Parts/Hub/Hub/HubWindow.swift | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift index e47a9a20fe..f6d0bf8f8f 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift @@ -377,6 +377,7 @@ package func runHub(_ args: [String]) -> Never { } } model.loadSessions() + MainActor.assumeIsolated { model.startSessionRefresh() } // The hub is a live monitor: transcripts stream in while it sits behind other windows. App Nap // throttled the whole process there (main-queue work ran 0.5–1 s late and even a dedicated // userInteractive sampler thread woke only at the end of each "stall"). Only this face opts out. @@ -1223,9 +1224,35 @@ final class HubModel: ObservableObject { } } + /// Keeps the session list current while the hub is open: a refresh every minute while the window can be seen. + /// The list used to load once at launch, so a session that worked all afternoon still read "active 53m ago" + /// (Martin, 2026-10-25). A failed refresh logs and keeps the old list, with no error banner. + @MainActor + func startSessionRefresh(every seconds: Double = 60) { + sessionRefresh?.cancel() + sessionRefresh = Task { @MainActor [weak self] in + while !Task.isCancelled { + do { try await Task.sleep(for: .seconds(seconds)) } catch { return } + await HubVisibility.shared.untilVisible() + guard let self, !self.loadingSessions else { continue } + let generation = self.sessionsGeneration + do { + let rows = try await self.readSessions(Self.recentHours) + guard generation == self.sessionsGeneration else { continue } + self.applySessions(rows, flash: false) + self.writeSessionCache(rows, Self.recentHours) + } catch { + PerfLog.mark("hub.sessions.refresh failed: \(error)") + } + } + } + } + + private var sessionRefresh: Task? + /// Shows `rows`, sliding new ones in; over a list already on screen, the new and moved ones flash. @MainActor - private func applySessions(_ rows: [HubSession]) { + private func applySessions(_ rows: [HubSession], flash: Bool = true) { var fresh = rows .filter { !($0.archived ?? false) } .sorted { $0.mtime > $1.mtime } @@ -1238,6 +1265,11 @@ final class HubModel: ObservableObject { guard fresh != sessions else { return } let before = Dictionary(sessions.map { ($0.id, "\($0.mtime)") }, uniquingKeysWith: { first, _ in first }) let moved = SWR.changed(before: before, after: fresh.map { ($0.id, "\($0.mtime)") }) + // A periodic refresh moves rows without the change flash: a row whose only news is a later time is not news. + guard flash else { + withAnimation(SWR.animation) { sessions = fresh } + return + } withAnimation(SWR.animation) { sessions = fresh changedSessions = moved From 17b7bb10a064b5333442202513e361dcbf7ca284 Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 15:01:08 +0100 Subject: [PATCH 02/11] fix(GenesisHub): a Cmd+Tab back to GenesisTools that shows no window logs why: no window list, not a Cmd+Tab, or nothing to hand off (cherry picked from commit 3b0d70512016c7d74275351815f8f6e5b2e5427c) --- .../Sources/Host/App/WindowCycle.swift | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift b/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift index fe9330f1e9..b5749166ff 100644 --- a/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift +++ b/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift @@ -314,13 +314,27 @@ package enum WindowCycle { /// The tile owner came to the front. On Cmd+Tab back, show the window that was key when GenesisTools left. private static func becameActive() { - guard let left = leftFrom else { return } + guard NSApp.activationPolicy() == .regular else { return } + guard let left = leftFrom else { + HubPerf.log("windowCycle: active, no window list from leaving (another face took focus since)") + return + } leftFrom = nil - guard NSApp.activationPolicy() == .regular, Date().timeIntervalSince(lastCycleRaise) > 1.5 else { return } + guard Date().timeIntervalSince(lastCycleRaise) > 1.5 else { return } let since = { (type: CGEventType) in CGEventSource.secondsSinceLastEventType(.combinedSessionState, eventType: type) } let mouse = min(since(.leftMouseDown), since(.rightMouseDown), since(.otherMouseDown)) - guard cameByCommandTab(sinceFlags: since(.flagsChanged), sinceMouse: mouse, sinceKey: since(.keyDown)), - let target = restoreTarget(left, candidates: currentCandidates(), own: getpid()) else { return } + guard cameByCommandTab(sinceFlags: since(.flagsChanged), sinceMouse: mouse, sinceKey: since(.keyDown)) else { + HubPerf.log(String(format: "windowCycle: active, not by Cmd+Tab (flags %.2fs, mouse %.2fs, key %.2fs ago)", + since(.flagsChanged), mouse, since(.keyDown))) + return + } + let candidates = currentCandidates() + guard let target = restoreTarget(left, candidates: candidates, own: getpid()) else { + let first = left.first { item in candidates.contains { $0.number == item.number } } + HubPerf.log("windowCycle: Cmd+Tab back, nothing to hand off (left \(left.count) windows, first on screen " + + (first.map { "\($0.number) of pid \($0.pid)" } ?? "none") + ")") + return + } HubPerf.log("windowCycle: Cmd+Tab back, showing window \(target.number) of pid \(target.pid)") handOff(to: target, session: nil) } From 43f6903fa0e12af7cf61ef1cc99fd91865115bce Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 15:17:26 +0100 Subject: [PATCH 03/11] fix(GenesisHub): Cmd+Tab back to GenesisTools shows the settings, permissions or review window you left again: the switch that activates while Command is still held after Tab now counts as Cmd+Tab (cherry picked from commit 4c20105b63f2e4c0410a79fbc241962ae6a702cc) --- .../Sources/Host/App/WindowCycle.swift | 18 +++++++++++++----- .../GenesisTools/Tests/WindowCycleTests.swift | 4 ++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift b/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift index b5749166ff..2bbc26da1e 100644 --- a/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift +++ b/src/macos/GenesisTools/Sources/Host/App/WindowCycle.swift @@ -183,8 +183,14 @@ package enum WindowCycle { /// Cmd+Tab activates on the release of ⌘, so a modifier change is the newest input. A click (the Dock, /// a window, a notification, a link in a browser) or a typed key (a launcher, a terminal) is newer otherwise. - package static func cameByCommandTab(sinceFlags: Double, sinceMouse: Double, sinceKey: Double) -> Bool { - sinceFlags < 1 && sinceFlags < sinceMouse && sinceFlags < sinceKey + /// + /// The activation can also arrive while ⌘ is still down, right after the Tab press: logged 2026-10-25 15:13 as + /// "flags 1.65s, mouse 8.92s, key 0.29s ago", and the window stayed behind. A typed key with ⌘ held is that case; + /// a launcher's typed key has no ⌘ held. + package static func cameByCommandTab(sinceFlags: Double, sinceMouse: Double, sinceKey: Double, + commandHeld: Bool = false) -> Bool { + if sinceFlags < 1 && sinceFlags < sinceMouse && sinceFlags < sinceKey { return true } + return commandHeld && sinceKey < 1 && sinceKey < sinceMouse } private static func handle(_ event: NSEvent) -> Bool { @@ -323,9 +329,11 @@ package enum WindowCycle { guard Date().timeIntervalSince(lastCycleRaise) > 1.5 else { return } let since = { (type: CGEventType) in CGEventSource.secondsSinceLastEventType(.combinedSessionState, eventType: type) } let mouse = min(since(.leftMouseDown), since(.rightMouseDown), since(.otherMouseDown)) - guard cameByCommandTab(sinceFlags: since(.flagsChanged), sinceMouse: mouse, sinceKey: since(.keyDown)) else { - HubPerf.log(String(format: "windowCycle: active, not by Cmd+Tab (flags %.2fs, mouse %.2fs, key %.2fs ago)", - since(.flagsChanged), mouse, since(.keyDown))) + let commandHeld = CGEventSource.flagsState(.combinedSessionState).contains(.maskCommand) + guard cameByCommandTab(sinceFlags: since(.flagsChanged), sinceMouse: mouse, sinceKey: since(.keyDown), + commandHeld: commandHeld) else { + HubPerf.log(String(format: "windowCycle: active, not by Cmd+Tab (flags %.2fs, mouse %.2fs, key %.2fs ago, ⌘ %@)", + since(.flagsChanged), mouse, since(.keyDown), commandHeld ? "held" : "up")) return } let candidates = currentCandidates() diff --git a/src/macos/GenesisTools/Tests/WindowCycleTests.swift b/src/macos/GenesisTools/Tests/WindowCycleTests.swift index 897def581b..f6bafe00f6 100644 --- a/src/macos/GenesisTools/Tests/WindowCycleTests.swift +++ b/src/macos/GenesisTools/Tests/WindowCycleTests.swift @@ -79,6 +79,10 @@ final class WindowCycleTests: XCTestCase { XCTAssertTrue(WindowCycle.cameByCommandTab(sinceFlags: 0.05, sinceMouse: 4, sinceKey: 0.2)) XCTAssertFalse(WindowCycle.cameByCommandTab(sinceFlags: 2, sinceMouse: 4, sinceKey: 0.1), "a typed key (a launcher)") XCTAssertFalse(WindowCycle.cameByCommandTab(sinceFlags: 0.3, sinceMouse: 0.02, sinceKey: 5), "a click (Dock, link, notification)") + XCTAssertTrue(WindowCycle.cameByCommandTab(sinceFlags: 1.65, sinceMouse: 8.92, sinceKey: 0.29, commandHeld: true), + "activated while ⌘ is still down, right after Tab (logged 2026-10-25 15:13)") + XCTAssertFalse(WindowCycle.cameByCommandTab(sinceFlags: 1.65, sinceMouse: 8.92, sinceKey: 0.29, commandHeld: false), + "the same timing with ⌘ up is a typed key") } /// Any process can post the raise and focus notifications: a payload that does not fit is a plain raise or is From da2e28fff32fd8a4ee20b259321dfe20160e5092 Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 18:20:35 +0100 Subject: [PATCH 04/11] feat(GenesisKit, GenesisHub): a navigation history engine that logs every place with who went there (user, agent, link) and why, walks back and forward without ever replacing, hops to any entry, and its list view; the hub's History settings (keep 200, agents navigate) (cherry picked from commit 7295faba4f86eec0c8ac71df606d6a950e42222d) --- src/macos/GenesisKit/README.md | 1 + .../Navigation/NavigationHistory.swift | 262 ++++++++++++++++++ .../Navigation/NavigationHistoryList.swift | 166 +++++++++++ .../GenesisKit/Settings/HubPreferences.swift | 55 +++- .../Settings/HubSettingsPages.swift | 43 +++ .../GenesisKitTests/HubPreferencesTests.swift | 21 +- .../NavigationHistoryTests.swift | 257 +++++++++++++++++ 7 files changed, 803 insertions(+), 2 deletions(-) create mode 100644 src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift create mode 100644 src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistoryList.swift create mode 100644 src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift diff --git a/src/macos/GenesisKit/README.md b/src/macos/GenesisKit/README.md index 210fb66f67..30ebc89db6 100644 --- a/src/macos/GenesisKit/README.md +++ b/src/macos/GenesisKit/README.md @@ -53,6 +53,7 @@ A fixture an app test needs goes there under its own name, never into a test tar | `Paths/` | `PathOpener` (folder in Finder by bundle id, file in Cursor at a line), `PathLabel`, `PathActionsMenu`, `Clipboard` + `CopyToast`, `DefaultApp` (a file's default app by name, opened at a line through `cursor://` / `vscode://`) | | `Diff/` | git and diffs for any host: `GenesisKitGTProvider` (the data source, swappable), `GitWorkingTreeSource` + `DiffScope`, `GitDiffView` + `GitScopePicker` + `GitCommitPicker` (popover choosers built when opened, `GitPickerChip`), `GitFileDiffModel`, `GitRepos` (one status per repository, refreshed by git's own folder events), the file state badge and marks, the gutter (`GitGutterModel`, `GitHunks`, `GitHunkPopover`), history and commits (`GitHistoryModel`, `GitCommitList`, `GitRepoPanel`, `GitCommitView`), tree marks, agent blame | | `Time/` | `LiveTime`, `LiveAgo`, `LiveTimeFormat` | +| `Navigation/` | `NavigationHistory` (a log of every place a window showed, who took it there: user, agent or link, and why; Back / Forward / hop to any entry; visits never replace), `NavigationHistoryStore` (the host persists it: the hub through `tools hub nav`), `NavigationHistoryList` (the list, newest first, a click hops) | | `Cmux/` | `CmuxTree`, `CmuxTarget`, `CmuxTargetPicker` (Tree / Layout), `CmuxSessionPanel` | | `Settings/` | the settings window (`FeatureSettingsWindowController`, `NativeSettingsStore`, pages and controls, `NativeSettingsTilePicker`), the plug-in contract for a window that several parts of an app add pages to (`FeatureSettingsPlugin`, `FeatureSettingsPluginHost`, `FeatureSettingsMode`) with the two pages every such window has (`FeatureSettingsPages.general`, `.about(additionalInfo:)`), its look (`NativeSettingsAppearance`, `NativeSettingsBackdrop`, `NativePaneFill` / `NativePaneBackground` for a host that draws its panes in that look), and the hub's preferences with their Hub pages (`HubPreferences`, `HubSettingsPages`) | | `Alerts/` | Turning measured values into alert events (`AlertPolicy`: tiers, hysteresis, snooze, quiet hours), and the channels that show them (`AlertDispatcher`, `AlertChannelMatrix`) | diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift new file mode 100644 index 0000000000..64881572d6 --- /dev/null +++ b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift @@ -0,0 +1,262 @@ +import Foundation + +// A browser's history plus a log: every place a window showed, who took it there (the person, an agent, a link) and +// why, with back and forward that walk it and a hop to any entry. The host owns the target type (the hub's is its +// link query) and the storage (`NavigationHistoryStore`); this type only decides the order. + +/// Who moved the window to a place. +public enum NavigationActor: String, Codable, CaseIterable, Sendable { + /// A click, a key or a menu in the window itself. + case user + /// An agent that asked for it (`tools hub nav open --actor agent`), usually with a note. + case agent + /// A link from outside the window (a browser, a notification, `tools hub open`) that named no actor. + case link + + public var title: String { + switch self { + case .user: return "User" + case .agent: return "Agent" + case .link: return "Link" + } + } +} + +/// One place in the log. +public struct NavigationEntry: Identifiable, Codable, Hashable { + public var id: String + public var at: Date + public var target: Target + /// What the place is, for the arrows' tooltips and the history list ("PR #563 Fix the login"). + public var title: String + public var actor: NavigationActor + /// The agent session that asked for it, when an agent did. + public var agentSession: String? + /// Why the agent opened it ("the commit that broke the build"). + public var note: String? + + public init( + id: String = UUID().uuidString, at: Date = Date(), target: Target, title: String, + actor: NavigationActor = .user, agentSession: String? = nil, note: String? = nil + ) { + self.id = id + self.at = at + self.target = target + self.title = title + self.actor = actor + self.agentSession = agentSession + self.note = note + } +} + +/// Where a host keeps the log. `load` runs once at start, `record` after each new or changed entry; both may be +/// slow (a `tools` call), so the host runs them off the main thread. +public protocol NavigationHistoryStore: AnyObject { + associatedtype Target: Codable & Hashable + /// Every kept entry, in any order. + func load() throws -> [NavigationEntry] + /// Adds the entry, or replaces the one with its id, and keeps at most `keep` entries (the newest). + func record(_ entry: NavigationEntry, keep: Int) throws +} + +/// The log and the cursor in it. A value type with no clock and no I/O, so tests drive it. +/// +/// - `visit` adds an entry and moves to it; the places ahead of the cursor leave the forward stack, as in a +/// browser, but stay in the log, so a hop still reaches them. Nothing is ever replaced. +/// - `goBack` / `goForward` move the cursor; `hop(to:)` moves it to any entry. +/// - A provisional place (a mode whose list still loads) is shown but never kept: the next visit takes its +/// spot, so the history holds no stop with nothing in it. +public struct NavigationHistory: Equatable { + public typealias Entry = NavigationEntry + + public static var defaultLimit: Int { 200 } + + /// Oldest first. + public private(set) var entries: [Entry] = [] + /// Ids, oldest first; the last is the place Back goes to. + public private(set) var backIDs: [String] = [] + /// Ids, farthest first; the last is the place Forward goes to. + public private(set) var forwardIDs: [String] = [] + public private(set) var currentID: String? + /// The place on screen is not in the log (see `visit(…provisional:)`). + public private(set) var currentTarget: Target? + /// How many entries the log keeps; the oldest go first. + public private(set) var limit: Int + + public init(limit: Int = Self.defaultLimit) { + self.limit = max(1, limit) + } + + /// A log read back from the store: every entry is behind the cursor, the newest is the current place. + public init(entries: [Entry], limit: Int = Self.defaultLimit) { + self.limit = max(1, limit) + let sorted = entries.sorted { $0.at < $1.at } + self.entries = Array(sorted.suffix(self.limit)) + backIDs = self.entries.dropLast().map(\.id) + currentID = self.entries.last?.id + currentTarget = self.entries.last?.target + } + + public var canGoBack: Bool { !backIDs.isEmpty } + public var canGoForward: Bool { !forwardIDs.isEmpty } + public var current: Entry? { currentID.flatMap(entry) } + public var backEntry: Entry? { backIDs.last.flatMap(entry) } + public var forwardEntry: Entry? { forwardIDs.last.flatMap(entry) } + /// Newest first, for a list. + public var newestFirst: [Entry] { entries.reversed() } + + public func entry(_ id: String) -> Entry? { + entries.last { $0.id == id } + } + + /// Where an entry sits relative to the cursor: negative behind it (−1 is Back), positive ahead, 0 for the + /// current place, nil for an entry that only the log holds (left by a visit after going back). + public func offset(of id: String) -> Int? { + if id == currentID { return 0 } + if let index = backIDs.lastIndex(of: id) { return index - backIDs.count } + if let index = forwardIDs.lastIndex(of: id) { return forwardIDs.count - index } + return nil + } + + /// A new place on screen. Returns the entry the store must write: the new one, an existing one an agent + /// annotated, or nil when nothing changed (the same place again, a provisional place, a return). + /// + /// - The same place as the current one: a user's visit is nothing; an agent's or a link's replaces the current + /// entry's who, note and id in place, so an agent opening what is already shown still leaves its note. + /// - After a provisional place: the new place takes its spot; when it is the place right behind, the cursor + /// goes back to it rather than logging a copy ("out to an empty mode and straight back"). + @discardableResult + public mutating func visit( + _ target: Target, title: String, actor: NavigationActor = .user, agentSession: String? = nil, + note: String? = nil, id: String? = nil, at: Date = Date(), provisional: Bool = false + ) -> Entry? { + if target == currentTarget, currentID != nil || provisional { + guard !provisional, let currentID, actor != .user, let index = entries.lastIndex(where: { $0.id == currentID }) + else { return nil } + var annotated = entries[index] + annotated.actor = actor + annotated.agentSession = agentSession + annotated.note = note + annotated.at = at + if !title.isEmpty { annotated.title = title } + if let id, id != currentID { + // The id the agent's CLI already stored, so the store keeps one row for it. + annotated.id = id + self.currentID = id + } + entries.remove(at: index) + entries.append(annotated) + return annotated + } + let wasProvisional = currentID == nil && currentTarget != nil + if provisional { + if !wasProvisional, let currentID { + backIDs.append(currentID) + forwardIDs.removeAll() + } + self.currentID = nil + currentTarget = target + trim() + return nil + } + if wasProvisional, let last = backIDs.last, entry(last)?.target == target { + backIDs.removeLast() + currentID = last + currentTarget = target + return nil + } + let made = Entry(id: id ?? UUID().uuidString, at: at, target: target, title: title, actor: actor, + agentSession: agentSession, note: note) + if let currentID { + backIDs.append(currentID) + } + forwardIDs.removeAll() + entries.removeAll { $0.id == made.id } + entries.append(made) + currentID = made.id + currentTarget = target + trim() + return made + } + + /// Adds an entry to the log without moving to it: an agent's place while agents may only add to the list. + @discardableResult + public mutating func record( + _ target: Target, title: String, actor: NavigationActor, agentSession: String? = nil, note: String? = nil, + id: String? = nil, at: Date = Date() + ) -> Entry { + let made = Entry(id: id ?? UUID().uuidString, at: at, target: target, title: title, actor: actor, + agentSession: agentSession, note: note) + entries.removeAll { $0.id == made.id } + entries.append(made) + trim() + return made + } + + public mutating func goBack() -> Entry? { + guard let previous = backIDs.popLast() else { return nil } + if let currentID { + forwardIDs.append(currentID) + } + return move(to: previous) + } + + public mutating func goForward() -> Entry? { + guard let next = forwardIDs.popLast() else { return nil } + if let currentID { + backIDs.append(currentID) + } + return move(to: next) + } + + /// Moves to any entry. One behind or ahead keeps the stacks a browser would (the places passed over stay on + /// the other side); an entry only the log holds goes on top, as a visit does, without a new entry. + public mutating func hop(to id: String) -> Entry? { + guard id != currentID, entry(id) != nil else { return nil } + if let index = backIDs.lastIndex(of: id) { + let passed = Array(backIDs[(index + 1)...]) + (currentID.map { [$0] } ?? []) + forwardIDs.append(contentsOf: passed.reversed()) + backIDs.removeSubrange(index...) + } else if let index = forwardIDs.lastIndex(of: id) { + let passed = (currentID.map { [$0] } ?? []) + forwardIDs[(index + 1)...].reversed() + backIDs.append(contentsOf: passed) + forwardIDs.removeSubrange(index...) + } else { + if let currentID { + backIDs.append(currentID) + } + forwardIDs.removeAll() + } + return move(to: id) + } + + /// Changes the cap; the oldest entries beyond it go. + public mutating func setLimit(_ limit: Int) { + self.limit = max(1, limit) + trim() + } + + /// Gives an entry a better title once its place has loaded (a PR's title arrives after its number). + public mutating func retitle(_ id: String, _ title: String) -> Entry? { + guard !title.isEmpty, let index = entries.lastIndex(where: { $0.id == id }), entries[index].title != title + else { return nil } + entries[index].title = title + return entries[index] + } + + private mutating func move(to id: String) -> Entry? { + currentID = id + let landed = entry(id) + currentTarget = landed?.target + return landed + } + + private mutating func trim() { + guard entries.count > limit else { return } + let keep = currentID + let dropped = Set(entries.map(\.id).filter { $0 != keep }.prefix(entries.count - limit)) + entries.removeAll { dropped.contains($0.id) } + backIDs.removeAll { dropped.contains($0) } + forwardIDs.removeAll { dropped.contains($0) } + } +} diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistoryList.swift b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistoryList.swift new file mode 100644 index 0000000000..770923ea53 --- /dev/null +++ b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistoryList.swift @@ -0,0 +1,166 @@ +import SwiftUI + +/// The history as a list, newest first: who went there (User, Agent, Link), the place, the agent's note, when, and +/// where it sits from the cursor. A click hops there (`NavigationHistory.hop(to:)`). Rows take values only, so the +/// list costs nothing while the host's model changes. +public struct NavigationHistoryList: View { + let history: NavigationHistory + var maxHeight: CGFloat + let onHop: (String) -> Void + + public init(history: NavigationHistory, maxHeight: CGFloat = 420, onHop: @escaping (String) -> Void) { + self.history = history + self.maxHeight = maxHeight + self.onHop = onHop + } + + public var body: some View { + let rows = history.newestFirst + VStack(alignment: .leading, spacing: 0) { + HStack(spacing: 6) { + Text("History").kicker() + Spacer(minLength: 8) + Text(verbatim: Plural.count(rows.count, "place")) + .font(.system(size: 10.5).monospacedDigit()) + .foregroundColor(KitPalette.faint) + } + .padding(.horizontal, 14) + .padding(.top, 10) + .padding(.bottom, 6) + if rows.isEmpty { + EmptyState(symbol: "clock.arrow.circlepath", text: "No places yet", + detail: "Every session, worktree, PR or commit the hub shows lands here.") + .frame(maxWidth: .infinity) + .padding(.vertical, 18) + } else { + FittedScroll(maxHeight: maxHeight) { + LazyVStack(alignment: .leading, spacing: 1) { + ForEach(rows) { entry in + NavigationHistoryRow(entry: NavigationHistoryRowValue(entry, offset: history.offset(of: entry.id)), + onHop: onHop) + } + } + .padding(.horizontal, 6) + .padding(.bottom, 6) + } + } + } + } +} + +/// What one row shows; equal values draw the same. +struct NavigationHistoryRowValue: Equatable { + let id: String + let title: String + let actor: NavigationActor + let note: String? + let agentSession: String? + let at: Date + /// 0 for the place on screen, negative behind it, positive ahead, nil for a place only the log holds. + let offset: Int? + + init(_ entry: NavigationEntry, offset: Int?) { + id = entry.id + title = entry.title + actor = entry.actor + note = entry.note.flatMap { $0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty ? nil : $0 } + agentSession = entry.agentSession + at = entry.at + self.offset = offset + } + + var isCurrent: Bool { offset == 0 } + + /// "Back 2", "Forward 1", or nil for the current place and a place only the log holds. + var position: String? { + guard let offset, offset != 0 else { return nil } + return offset < 0 ? "Back \(-offset)" : "Forward \(offset)" + } + + var accessibilityText: String { + var parts = ["\(actor.title): \(title)"] + if let note { parts.append(note) } + if isCurrent { parts.append("shown now") } else if let position { parts.append(position) } + return parts.joined(separator: ", ") + } +} + +extension NavigationActor { + /// The badge colour: the person is quiet, an agent stands out, a link sits between. + var tint: Color { + switch self { + case .user: return KitPalette.dim + case .agent: return Color(red: 0.74, green: 0.58, blue: 1.0) + case .link: return KitPalette.renamed + } + } + + var symbol: String { + switch self { + case .user: return "person.fill" + case .agent: return "sparkles" + case .link: return "link" + } + } +} + +struct NavigationHistoryRow: View, Equatable { + let entry: NavigationHistoryRowValue + let onHop: (String) -> Void + + static func == (lhs: Self, rhs: Self) -> Bool { lhs.entry == rhs.entry } + + var body: some View { + HStack(alignment: .top, spacing: 8) { + Circle() + .fill(entry.isCurrent ? KitTheme.accent : Color.clear) + .overlay(Circle().stroke(entry.isCurrent ? Color.clear : KitPalette.faint, lineWidth: 1)) + .frame(width: 7, height: 7) + .padding(.top, 5) + VStack(alignment: .leading, spacing: 3) { + HStack(spacing: 6) { + Badge(entry.actor.title, color: entry.actor.tint, look: entry.actor == .user ? .tag : .tone, + symbol: entry.actor.symbol) + Text(verbatim: entry.title) + .font(.system(size: 12, weight: entry.isCurrent ? .semibold : .regular)) + .foregroundColor(KitPalette.text) + .lineLimit(1) + .truncationMode(.middle) + Spacer(minLength: 6) + LiveAgo(date: entry.at, style: .brief) + .font(.system(size: 10.5).monospacedDigit()) + .foregroundColor(KitPalette.faint) + .fixedSize() + } + if let note = entry.note { + Text(verbatim: note) + .font(.system(size: 11)) + .foregroundColor(KitPalette.dim) + .lineLimit(2) + .fixedSize(horizontal: false, vertical: true) + } + if entry.isCurrent || entry.position != nil || entry.agentSession != nil { + HStack(spacing: 6) { + if entry.isCurrent { + Text("Shown now").font(.system(size: 10, weight: .medium)).foregroundColor(KitTheme.accent) + } else if let position = entry.position { + Text(verbatim: position).font(.system(size: 10)).foregroundColor(KitPalette.faint) + } + if let session = entry.agentSession { + Text(verbatim: "session \(session.prefix(8))") + .font(.system(size: 10, design: .monospaced)) + .foregroundColor(KitPalette.faint) + } + } + } + } + } + .padding(.horizontal, 8) + .padding(.vertical, 6) + .frame(maxWidth: .infinity, alignment: .leading) + .rowButton(cornerRadius: 6) { onHop(entry.id) } + .disabled(entry.isCurrent) + .accessibilityLabel(Text(verbatim: entry.accessibilityText)) + .accessibilityHint(entry.isCurrent ? Text("") : Text("Opens this place")) + } +} diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubPreferences.swift b/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubPreferences.swift index 6deff1fd33..201b486093 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubPreferences.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubPreferences.swift @@ -15,6 +15,26 @@ public enum HubWindowStyle: String, CaseIterable, Identifiable, Sendable { } } +/// What an agent's (or a link's) request to show a place does to the hub: it navigates there, as a click would, +/// or it only adds the place to the history list and leaves the view alone. Either way Back returns. +public enum HubAgentNavigation: String, CaseIterable, Identifiable, Sendable { + case navigate, listOnly + public var id: String { rawValue } + public var title: String { + switch self { + case .navigate: return "Show it" + case .listOnly: return "Only add it to History" + } + } + + public var detail: String { + switch self { + case .navigate: return "The hub moves to what the agent opened. Back returns to where you were." + case .listOnly: return "The view stays. The place waits in History (⌘Y) until you open it." + } + } +} + /// The hub's preferences that a person sets, shared by the hub window (`--hub`) and the Hub pages of the settings /// window (`--features`). Both faces run from one bundle, so they read one defaults domain. A write posts a distributed /// notification, and every other process reloads, as `NativeSettingsAppearance` does for General. @@ -28,11 +48,16 @@ public final class HubPreferences: ObservableObject { public static let glass = "hub.glass" public static let processEnergy = "hub.procs.energy" public static let cleanupIdleDays = "hub.worktreeCleanup.olderThanDays" + public static let navigationLimit = "hub.nav.limit" + public static let agentNavigation = "hub.nav.agentNavigation" } public static let shared = HubPreferences() /// The idle days a worktree needs before cleanup counts it as removable; 0 means any idle time. public static let cleanupIdleDaysRange = 0...90 + /// How many places the navigation history keeps (`tools hub nav`); the oldest go first. + public static let navigationLimitRange = 20...2000 + public static let navigationLimitDefault = 200 @Published public var style: HubWindowStyle { didSet { if oldValue != style { persist(.style) } } } /// The Hub style only: the desktop shows through, blurred. The Native style follows General → Theme instead. @@ -44,8 +69,16 @@ public final class HubPreferences: ObservableObject { @Published public var cleanupIdleDays: Int { didSet { if oldValue != cleanupIdleDays { persist(.cleanupIdleDays) } } } + /// The places Back, Forward and History keep. + @Published public var navigationLimit: Int { + didSet { if oldValue != navigationLimit { persist(.navigationLimit) } } + } + /// What an agent's request to show a place does: navigate (the default) or only add it to History. + @Published public var agentNavigation: HubAgentNavigation { + didSet { if oldValue != agentNavigation { persist(.agentNavigation) } } + } - private enum Field { case style, glass, processEnergy, cleanupIdleDays } + private enum Field { case style, glass, processEnergy, cleanupIdleDays, navigationLimit, agentNavigation } public let notificationNamespace: String public let notificationName: Notification.Name private let defaults: UserDefaults @@ -70,6 +103,8 @@ public final class HubPreferences: ObservableObject { glass = defaults.bool(forKey: Key.glass) processEnergy = defaults.bool(forKey: Key.processEnergy) cleanupIdleDays = Self.storedIdleDays(defaults) + navigationLimit = Self.storedNavigationLimit(defaults) + agentNavigation = Self.storedAgentNavigation(defaults) if observeExternalChanges { observer = DistributedNotificationCenter.default().addObserver( forName: notificationName, object: notificationNamespace, queue: .main @@ -91,11 +126,15 @@ public final class HubPreferences: ObservableObject { let storedGlass = defaults.bool(forKey: Key.glass) let storedEnergy = defaults.bool(forKey: Key.processEnergy) let storedDays = Self.storedIdleDays(defaults) + let storedLimit = Self.storedNavigationLimit(defaults) + let storedAgent = Self.storedAgentNavigation(defaults) applyingExternal = true if style != storedStyle { style = storedStyle } if glass != storedGlass { glass = storedGlass } if processEnergy != storedEnergy { processEnergy = storedEnergy } if cleanupIdleDays != storedDays { cleanupIdleDays = storedDays } + if navigationLimit != storedLimit { navigationLimit = storedLimit } + if agentNavigation != storedAgent { agentNavigation = storedAgent } applyingExternal = false } @@ -107,6 +146,16 @@ public final class HubPreferences: ObservableObject { min(cleanupIdleDaysRange.upperBound, max(cleanupIdleDaysRange.lowerBound, defaults.integer(forKey: Key.cleanupIdleDays))) } + /// Unset reads as the default; a stored value outside the range reads as its nearest limit. + private static func storedNavigationLimit(_ defaults: UserDefaults) -> Int { + guard defaults.object(forKey: Key.navigationLimit) != nil else { return navigationLimitDefault } + return min(navigationLimitRange.upperBound, max(navigationLimitRange.lowerBound, defaults.integer(forKey: Key.navigationLimit))) + } + + private static func storedAgentNavigation(_ defaults: UserDefaults) -> HubAgentNavigation { + HubAgentNavigation(rawValue: defaults.string(forKey: Key.agentNavigation) ?? "") ?? .navigate + } + private func persist(_ field: Field) { guard !applyingExternal else { return } switch field { @@ -116,6 +165,10 @@ public final class HubPreferences: ObservableObject { case .cleanupIdleDays: let clamped = min(Self.cleanupIdleDaysRange.upperBound, max(Self.cleanupIdleDaysRange.lowerBound, cleanupIdleDays)) defaults.set(clamped, forKey: Key.cleanupIdleDays) + case .navigationLimit: + let clamped = min(Self.navigationLimitRange.upperBound, max(Self.navigationLimitRange.lowerBound, navigationLimit)) + defaults.set(clamped, forKey: Key.navigationLimit) + case .agentNavigation: defaults.set(agentNavigation.rawValue, forKey: Key.agentNavigation) } guard broadcasts else { return } if !defaults.synchronize() { log.warning("Could not flush the hub preferences") } diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift b/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift index ce4f897998..396c5e67ab 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift @@ -6,6 +6,7 @@ import SwiftUI public enum HubSettingsPages { public static let appearancePageID = "hub.appearance" public static let panesPageID = "hub.panes" + public static let historyPageID = "hub.history" public static func section(preferences: HubPreferences? = nil) -> NativeSettingsSection { let preferences = preferences ?? .shared @@ -24,6 +25,12 @@ public enum HubSettingsPages { ) { HubPaneSettings(preferences: preferences) }, + NativeSettingsPage( + id: historyPageID, title: "History", symbol: "clock.arrow.circlepath", tint: .purple, + subtitle: "Back, Forward and the list of places the hub showed, yours and the agents'." + ) { + HubHistorySettings(preferences: preferences) + }, ], order: 35) } } @@ -115,3 +122,39 @@ private struct HubPaneSettings: View { } } } + +@MainActor +private struct HubHistorySettings: View { + @ObservedObject var preferences: HubPreferences + + var body: some View { + VStack(spacing: 18) { + NativeSettingsCard( + "When an agent opens something", + subtitle: "An agent or a link names a PR, a commit, a worktree or a session (tools hub nav open)." + ) { + NativeSettingsSegmented( + "Agent navigation", identifier: "hub.nav.agentNavigation", + options: HubAgentNavigation.allCases.map { NativeSettingsOption($0, $0.title, detail: $0.detail) }, + selection: $preferences.agentNavigation, minSegmentWidth: 90) + } + NativeSettingsCard("History") { + NativeSettingsRow( + "Keep the last", detail: "Older places leave History, Back and Forward. The log is ~/.genesis-tools/hub/navigation.db." + ) { + HStack(spacing: 8) { + NativeSettingsNumberPicker( + "Places kept", + range: HubPreferences.navigationLimitRange.lowerBound + ..< (HubPreferences.navigationLimitRange.upperBound + 1), + identifier: "hub.nav.limit", value: $preferences.navigationLimit) + Text("places").font(.system(size: 12)).foregroundStyle(.secondary) + } + } + } + Text("⌘[ and ⌘] go back and forward; ⌘Y lists every place. Changes apply to a running hub at once.") + .font(.system(size: 11)).foregroundStyle(.secondary) + .frame(maxWidth: .infinity, alignment: .leading) + } + } +} diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/HubPreferencesTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/HubPreferencesTests.swift index 4533446ccb..9aef29fe59 100644 --- a/src/macos/GenesisKit/Tests/GenesisKitTests/HubPreferencesTests.swift +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/HubPreferencesTests.swift @@ -59,6 +59,25 @@ final class HubPreferencesTests: XCTestCase { XCTAssertEqual(preferences(defaults, name).cleanupIdleDays, 0) } + func testHistoryKeepsTwoHundredPlacesAndAgentsNavigateUntilChanged() { + let (defaults, name) = suite() + defer { defaults.removePersistentDomain(forName: name) } + let hub = preferences(defaults, name) + XCTAssertEqual(hub.navigationLimit, 200) + XCTAssertEqual(hub.agentNavigation, .navigate) + hub.navigationLimit = 50 + hub.agentNavigation = .listOnly + XCTAssertEqual(defaults.integer(forKey: "hub.nav.limit"), 50) + XCTAssertEqual(defaults.string(forKey: "hub.nav.agentNavigation"), "listOnly") + let reopened = preferences(defaults, name) + XCTAssertEqual(reopened.navigationLimit, 50) + XCTAssertEqual(reopened.agentNavigation, .listOnly) + defaults.set(5, forKey: HubPreferences.Key.navigationLimit) + defaults.set("steal", forKey: HubPreferences.Key.agentNavigation) + XCTAssertEqual(preferences(defaults, name).navigationLimit, HubPreferences.navigationLimitRange.lowerBound) + XCTAssertEqual(preferences(defaults, name).agentNavigation, .navigate) + } + func testAChangeInOneProcessReachesAnotherThroughTheDistributedNotification() async { let (defaults, name) = suite() defer { defaults.removePersistentDomain(forName: name) } @@ -111,7 +130,7 @@ final class HubPreferencesTests: XCTestCase { defer { defaults.removePersistentDomain(forName: name) } let section = HubSettingsPages.section(preferences: preferences(defaults, name)) XCTAssertEqual(section.title, "Hub") - XCTAssertEqual(section.pages.map(\.id), [HubSettingsPages.appearancePageID, HubSettingsPages.panesPageID]) + XCTAssertEqual(section.pages.map(\.id), [HubSettingsPages.appearancePageID, HubSettingsPages.panesPageID, HubSettingsPages.historyPageID]) let appearance = NativeSettingsAppearance(defaults: defaults, notificationNamespace: name, observeExternalChanges: false) let clicky = ClickyModel(defaults: defaults, previewOnly: true, appearance: appearance, observeSystemEvents: false) let widget = WidgetModel(binaryPath: "/fixture/no-process", defaults: defaults, appearance: appearance) diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift new file mode 100644 index 0000000000..5ca7b251f5 --- /dev/null +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift @@ -0,0 +1,257 @@ +import AppKit +import SwiftUI +import XCTest + +@testable import GenesisKitBase +@testable import GenesisKit + +/// The navigation engine (`NavigationHistory`): visits never replace, Back and Forward walk, a hop reaches any entry, +/// provisional places leave no stop, the cap drops the oldest, and an agent's visit keeps its note. +final class NavigationHistoryTests: XCTestCase { + private typealias History = NavigationHistory + + private func titles(_ history: History, _ ids: [String]) -> [String] { + ids.compactMap { history.entry($0)?.title } + } + + private func walk(_ places: [String], limit: Int = 200) -> History { + var history = History(limit: limit) + for place in places { + history.visit(place, title: place) + } + return history + } + + func testVisitsStackAndBackAndForwardWalkThem() { + var history = walk(["a", "b", "c"]) + XCTAssertEqual(history.current?.target, "c") + XCTAssertEqual(history.goBack()?.target, "b") + XCTAssertEqual(history.goBack()?.target, "a") + XCTAssertNil(history.goBack(), "nothing before the first place") + XCTAssertEqual(history.goForward()?.target, "b") + XCTAssertEqual(history.goForward()?.target, "c") + XCTAssertNil(history.goForward()) + XCTAssertEqual(history.entries.count, 3, "walking never adds entries") + } + + func testAVisitAfterGoingBackClearsForwardButTheLogKeepsEveryPlace() { + var history = walk(["a", "b", "c"]) + _ = history.goBack() + _ = history.goBack() + history.visit("d", title: "d") + XCTAssertFalse(history.canGoForward) + XCTAssertEqual(titles(history, history.backIDs), ["a"]) + XCTAssertEqual(history.newestFirst.map(\.target), ["d", "c", "b", "a"], "b and c are still in the list") + // A hop reaches a place that left the forward stack, and Back returns from it. + let c = try! XCTUnwrap(history.entries.first { $0.target == "c" }) + XCTAssertNil(history.offset(of: c.id), "only the log holds it") + XCTAssertEqual(history.hop(to: c.id)?.target, "c") + XCTAssertEqual(history.goBack()?.target, "d") + } + + func testTheSamePlaceTwiceIsOneStop() { + var history = walk(["a"]) + XCTAssertNil(history.visit("a", title: "a")) + XCTAssertFalse(history.canGoBack) + XCTAssertEqual(history.entries.count, 1) + } + + func testHopBackwardsKeepsThePassedPlacesAheadInOrder() throws { + var history = walk(["a", "b", "c", "d"]) + let a = try XCTUnwrap(history.entries.first) + XCTAssertEqual(history.offset(of: a.id), -3) + XCTAssertEqual(history.hop(to: a.id)?.target, "a") + XCTAssertFalse(history.canGoBack) + XCTAssertEqual(history.goForward()?.target, "b") + XCTAssertEqual(history.goForward()?.target, "c") + XCTAssertEqual(history.goForward()?.target, "d") + } + + func testHopForwardsKeepsThePassedPlacesBehindInOrder() throws { + var history = walk(["a", "b", "c", "d"]) + let d = try XCTUnwrap(history.entries.last) + _ = history.hop(to: try XCTUnwrap(history.entries.first).id) + XCTAssertEqual(history.offset(of: d.id), 3) + XCTAssertEqual(history.hop(to: d.id)?.target, "d") + XCTAssertFalse(history.canGoForward) + XCTAssertEqual(history.goBack()?.target, "c") + XCTAssertEqual(history.goBack()?.target, "b") + XCTAssertEqual(history.goBack()?.target, "a") + XCTAssertNil(history.hop(to: "no-such-id")) + } + + func testAProvisionalPlaceIsNeverAStop() { + var history = History() + history.visit("sessions:-", title: "Sessions", provisional: true) + history.visit("prs:-", title: "PRs", provisional: true) + XCTAssertFalse(history.canGoBack, "the launch's empty mode is no place to go back to") + XCTAssertTrue(history.entries.isEmpty) + + history.visit("pr1", title: "pr1") + history.visit("worktrees:-", title: "Worktrees", provisional: true) + history.visit("a", title: "a") + XCTAssertEqual(titles(history, history.backIDs), ["pr1"], "a mode left while its list loaded is not a stop") + + // Out to an empty mode and straight back: the place before it, not a copy of it. + history.visit("worktrees:-", title: "Worktrees", provisional: true) + history.visit("a", title: "a") + XCTAssertEqual(titles(history, history.backIDs), ["pr1"]) + XCTAssertEqual(history.current?.target, "a") + XCTAssertEqual(history.entries.count, 2) + } + + func testAnAgentVisitCarriesItsWhoAndNoteAndKeepsTheCLIsID() { + var history = walk(["a"]) + let made = history.visit("pr563", title: "PR #563", actor: .agent, agentSession: "abc", note: "the failing CI run", + id: "cli-id") + XCTAssertEqual(made?.id, "cli-id") + XCTAssertEqual(history.current?.actor, .agent) + XCTAssertEqual(history.current?.note, "the failing CI run") + XCTAssertEqual(history.backEntry?.target, "a") + } + + func testAnAgentOpeningWhatIsShownAnnotatesItInsteadOfAddingACopy() { + var history = walk(["a", "b"]) + let annotated = history.visit("b", title: "", actor: .agent, note: "look here", id: "cli-id") + XCTAssertEqual(annotated?.id, "cli-id") + XCTAssertEqual(annotated?.title, "b", "an empty title keeps the one it had") + XCTAssertEqual(history.entries.count, 2) + XCTAssertEqual(history.current?.note, "look here") + XCTAssertEqual(history.backEntry?.target, "a") + // A user's repeat of the same place is nothing at all. + XCTAssertNil(history.visit("b", title: "b")) + } + + func testRecordAddsToTheListWithoutMovingTheCursor() throws { + var history = walk(["a", "b"]) + let added = history.record("pr9", title: "PR #9", actor: .agent, note: "later") + XCTAssertEqual(history.current?.target, "b") + XCTAssertNil(history.offset(of: added.id)) + XCTAssertEqual(history.newestFirst.first?.target, "pr9") + XCTAssertEqual(history.hop(to: added.id)?.target, "pr9") + XCTAssertEqual(history.goBack()?.target, "b") + } + + func testTheCapDropsTheOldestButNeverThePlaceOnScreen() { + var history = walk((0..<30).map { "p\($0)" }, limit: 10) + XCTAssertEqual(history.entries.count, 10) + XCTAssertEqual(history.entries.first?.target, "p20") + XCTAssertEqual(history.backIDs.count, 9) + for _ in 0..<9 { _ = history.goBack() } + history.setLimit(3) + XCTAssertEqual(history.current?.target, "p20", "the place on screen survives a smaller cap") + XCTAssertTrue(history.entries.contains { $0.target == "p20" }) + XCTAssertLessThanOrEqual(history.entries.count, 3) + } + + func testALogReadBackPutsEveryEntryBehindTheNewest() { + let now = Date() + let entries = [ + NavigationEntry(id: "2", at: now, target: "b", title: "b"), + NavigationEntry(id: "1", at: now.addingTimeInterval(-60), target: "a", title: "a", actor: .agent, note: "why"), + ] + var history = History(entries: entries) + XCTAssertEqual(history.current?.target, "b") + XCTAssertEqual(history.goBack()?.note, "why") + // The launch's own place after a read-back pushes the newest entry behind it. + var launched = History(entries: entries) + launched.visit("c", title: "c") + XCTAssertEqual(launched.backEntry?.target, "b") + } + + func testRetitleChangesOnlyTheNamedEntry() throws { + var history = walk(["a", "b"]) + let a = try XCTUnwrap(history.entries.first) + XCTAssertEqual(history.retitle(a.id, "PR #1 Fix it")?.title, "PR #1 Fix it") + XCTAssertNil(history.retitle(a.id, "PR #1 Fix it"), "the same title again writes nothing") + XCTAssertEqual(history.entries.last?.title, "b") + } + + func testEntriesRoundTripThroughJSON() throws { + let entry = NavigationEntry(id: "x", at: Date(timeIntervalSince1970: 1_700_000_000), target: ["pr": "563"], + title: "PR #563", actor: .link, agentSession: nil, note: nil) + let data = try JSONEncoder().encode(entry) + XCTAssertEqual(try JSONDecoder().decode(NavigationEntry<[String: String]>.self, from: data), entry) + } + + /// The store protocol the host implements: a memory store reads back what it was given, newest kept. + func testAStoreKeepsTheNewestEntriesAndReplacesByID() throws { + final class MemoryStore: NavigationHistoryStore { + var rows: [NavigationEntry] = [] + func load() throws -> [NavigationEntry] { rows } + func record(_ entry: NavigationEntry, keep: Int) throws { + rows.removeAll { $0.id == entry.id } + rows.append(entry) + rows = Array(rows.sorted { $0.at < $1.at }.suffix(keep)) + } + } + let store = MemoryStore() + var history = History(limit: 2) + let start = Date() + for (index, place) in ["a", "b", "c"].enumerated() { + if let made = history.visit(place, title: place, at: start.addingTimeInterval(Double(index))) { + try store.record(made, keep: history.limit) + } + } + if let annotated = history.visit("c", title: "c", actor: .agent, note: "n", at: start.addingTimeInterval(9)) { + try store.record(annotated, keep: history.limit) + } + let reread = History(entries: try store.load(), limit: 2) + XCTAssertEqual(reread.entries.map(\.target), ["b", "c"]) + XCTAssertEqual(reread.current?.note, "n") + } +} + +/// The history list as the hub's popover draws it, in the dark settings look. +@MainActor +final class NavigationHistoryListShotTests: XCTestCase { + func testTheListDrawsEveryActorTheNoteAndTheCurrentPlace() throws { + let rep = try KitShots.render(NavigationHistoryGallery(), size: CGSize(width: 380, height: 430), name: "navigation-history", + settle: 0.6) + // The agent badge's violet and the link badge's blue must both be on the picture. + var violet = 0 + var blue = 0 + for y in stride(from: 0, to: rep.pixelsHigh, by: 2) { + for x in stride(from: 0, to: rep.pixelsWide / 2, by: 2) { + guard let c = rep.colorAt(x: x, y: y)?.usingColorSpace(.sRGB), c.saturationComponent > 0.3, + c.brightnessComponent > 0.5 else { continue } + let hue = c.hueComponent + if (0.70...0.80).contains(hue) { violet += 1 } + if (0.58...0.66).contains(hue) { blue += 1 } + } + } + XCTAssertGreaterThan(violet, 8, "the Agent badge") + XCTAssertGreaterThan(blue, 8, "the Link badge") + XCTAssertGreaterThan(KitShots.brightness(rep, in: CGRect(x: 40, y: 30, width: 300, height: 360), viewWidth: 380), 0.08, + "row titles must be drawn") + } +} + +/// Five places: the person's, two an agent opened with notes, one from a link, and the place on screen. +struct NavigationHistoryGallery: View { + static var sample: NavigationHistory { + let now = Date() + var history = NavigationHistory() + history.visit("s1", title: "session Fix the login redirect", at: now.addingTimeInterval(-3600)) + history.visit("pr563", title: "#563 hub: navigation history", actor: .agent, agentSession: "2f320c19-ff19", + note: "The PR with the failing Swift suite; the second commit adds the store.", + at: now.addingTimeInterval(-1500)) + history.visit("c1", title: "commit 0751d8a971 in GenesisTools", actor: .agent, agentSession: "2f320c19-ff19", + note: "The commit that moved the cache chip focus.", at: now.addingTimeInterval(-600)) + history.visit("wt", title: "worktree GenesisTools feat/2026-10-25-hub-navigation-history", actor: .link, + at: now.addingTimeInterval(-240)) + history.visit("s2", title: "session Review the hub history popover", at: now.addingTimeInterval(-30)) + _ = history.goBack() + return history + } + + var body: some View { + NavigationHistoryList(history: Self.sample) { _ in } + .frame(width: 380, alignment: .top) + .frame(maxHeight: .infinity, alignment: .top) + .background(NativeThemePalette.graphiteBase) + .environment(\.nativeSettingsTheme, .graphite) + .environment(\.colorScheme, .dark) + .preferredColorScheme(.dark) + } +} From f4949f20b535ed3b8e35e755cab2bfb76c311dce Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 18:24:54 +0100 Subject: [PATCH 05/11] feat(hub): tools hub nav keeps the hub's navigation log in SQLite (who: user, agent or link, the place, the note), nav open records and opens a place for an agent, and hub links carry actor, note, from and nav (cherry picked from commit a020147b56a98d166eaa1a63f5c755ace621d47c) --- src/hub/README.md | 15 +- src/hub/cli.ts | 2 + src/hub/commands/nav.ts | 351 ++++++++++++++++++++++ src/hub/lib/navigation/navigation.test.ts | 122 ++++++++ src/hub/lib/navigation/store.ts | 128 ++++++++ src/hub/lib/navigation/target.ts | 124 ++++++++ src/hub/lib/navigation/types.ts | 67 +++++ src/hub/lib/open.ts | 19 ++ src/utils/app-links/targets.ts | 7 + 9 files changed, 834 insertions(+), 1 deletion(-) create mode 100644 src/hub/commands/nav.ts create mode 100644 src/hub/lib/navigation/navigation.test.ts create mode 100644 src/hub/lib/navigation/store.ts create mode 100644 src/hub/lib/navigation/target.ts create mode 100644 src/hub/lib/navigation/types.ts diff --git a/src/hub/README.md b/src/hub/README.md index 8d15afdb12..1ebe9b0c0e 100644 --- a/src/hub/README.md +++ b/src/hub/README.md @@ -29,6 +29,19 @@ A missing PR commit does not trigger a project-diff lookup. To open that commit Other `open` flags: `--agent`, `--filter`, `--palette [text]`, `--find [text]`, `--session-search [text]`, `--digest`, `--rules`, `--prompts`, `--handoff`, `--no-build` (`tools hub open --help`). +## History: Back, Forward and who went where + +The hub keeps a log of every place it showed, with who took it there: `user` (a click, a key or a menu in the hub), `agent`, or `link` (a link or `tools hub open` that named nobody). An agent adds why. The top-left arrows (⌘[ and ⌘]) walk it, and the clock button beside them (⌘Y) lists it, newest first; a click hops to any entry. A new place never replaces the old one: Back always returns. + +**Agents open things with `nav open`, not `open`**, so the person sees why and can go back: + + tools hub nav open '#563' --note "the PR whose Swift suite fails" + tools hub nav open --repo /path/to/checkout --commit 4e7a3bc --note "the commit that moved the focus" + tools hub nav open 'genesis-tools://hub?pr=owner%2Frepo%2342&commit=4e7a3bc' --note "why" + tools hub nav list [--json] [--limit 20] + +`nav open` takes a hub link, a PR ref or page, a commit sha (with `--pr` or `--repo`), a checkout folder, or the `open` flags. It records the entry (`~/.genesis-tools/hub/navigation.db`), then opens the hub on it. `--actor` defaults to `agent` inside an agent session (its id goes in as `--from`), else `link`. Settings → Hub → History sets how many places are kept (200) and whether an agent's place is shown at once (the default) or only added to History. Every entry writes one `hub nav:` line to the day log. + ## Links `--print-url` prints the link for the same flags instead of opening: @@ -36,7 +49,7 @@ Other `open` flags: `--agent`, `--filter`, `--palette [text]`, `--find [text]`, tools hub open --print-url --pr 'owner/repo#42' --commit 4e7a3bc # genesis-tools://hub?pr=owner%2Frepo%2342&commit=4e7a3bc -A `genesis-tools://hub?…` link opens the hub where it points, with or without the browser router. Its keys are the `open` flags without dashes: `mode`, `session`, `agent`, `pr`, `reveal`, `commit`, `repo`, `tab`, `filter`, `worktree`, `decision`, `question` (`HubRequest.linkKeys` in `src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift`). Every other key is dropped: a link shows a place, it never runs a command. The `#` of a PR ref must be written `%23`. +A `genesis-tools://hub?…` link opens the hub where it points, with or without the browser router. Its keys are the `open` flags without dashes: `mode`, `session`, `agent`, `pr`, `reveal`, `commit`, `repo`, `tab`, `filter`, `worktree`, `decision`, `question`, and for the history `actor` (user, agent, link), `note`, `from` (the agent's session) and `nav` (an entry `nav open` stored) (`HubRequest.linkKeys` in `src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift`). Every other key is dropped: a link shows a place, it never runs a command. The `#` of a PR ref must be written `%23`. Through the browser router, the same query on the link host opens the hub as well, so a terminal or a chat that only makes `https://` clickable works too: diff --git a/src/hub/cli.ts b/src/hub/cli.ts index 78aa92716d..107e800bfd 100644 --- a/src/hub/cli.ts +++ b/src/hub/cli.ts @@ -23,6 +23,7 @@ import { registerDevCommands } from "./commands/dev"; import { registerDigestCommand } from "./commands/digest"; import { registerForecastCommand } from "./commands/forecast"; import { registerInsightsCommands } from "./commands/insights"; +import { registerNavCommands } from "./commands/nav"; import { registerNotifyCommands } from "./commands/notify"; import { registerProcsCommand } from "./commands/procs"; import { registerPromptsCommand } from "./commands/prompts"; @@ -1284,6 +1285,7 @@ registerSearchCommand(program); registerDigestCommand(program); registerForecastCommand(program); registerRulesCommand(program); +registerNavCommands(program); registerInsightsCommands(program); registerProcsCommand(program); registerPromptsCommand(program); diff --git a/src/hub/commands/nav.ts b/src/hub/commands/nav.ts new file mode 100644 index 0000000000..04b24b5146 --- /dev/null +++ b/src/hub/commands/nav.ts @@ -0,0 +1,351 @@ +import { statSync } from "node:fs"; +import { resolve } from "node:path"; +import { createBuildProgressDisplay } from "@app/macos/lib/permissions/swift-progress"; +import { agentSessionIds } from "@genesiscz/utils/agent/host"; +import { suggestEnumFlag } from "@genesiscz/utils/cli"; +import { toolCommand } from "@genesiscz/utils/cli/tool-command"; +import { env } from "@genesiscz/utils/env"; +import { formatRelativeTime } from "@genesiscz/utils/format"; +import { SafeJSON } from "@genesiscz/utils/json"; +import { logger, out } from "@genesiscz/utils/logger"; +import { createBoxTable, renderCliHeader, truncateDisplay } from "@genesiscz/utils/table"; +import { type Command, InvalidArgumentError } from "commander"; +import pc from "picocolors"; +import { listNav, navDbPath, navKeep, openNavDb, recordNav } from "../lib/navigation/store"; +import { navTargetProblem, navTitle, parseNavInput } from "../lib/navigation/target"; +import { + isNavActor, + NAV_ACTORS, + type NavActor, + type NavEntry, + type NavTarget, + normalizeTarget, +} from "../lib/navigation/types"; +import { HUB_MODES, HUB_TABS, hubUrl, openHub } from "../lib/open"; + +const { log } = logger.scoped("hub:nav"); + +function positiveInt(value: string): number { + const parsed = Number(value); + + if (!Number.isInteger(parsed) || parsed <= 0) { + throw new InvalidArgumentError("a whole number above 0"); + } + + return parsed; +} + +function isFolder(path: string): boolean { + try { + return statSync(resolve(path)).isDirectory(); + } catch (error) { + log.debug({ path, error }, "hub nav: not a folder"); + return false; + } +} + +/** The one log line per navigation, in the day log (`~/.genesis-tools/logs/.log`). */ +function logNavigation(entry: NavEntry, via: string): void { + // Debug: the day log keeps every level, and an agent's terminal does not need the block. + log.debug( + { via, actor: entry.actor, target: entry.target, note: entry.note, from: entry.from, id: entry.id }, + `hub nav: ${entry.actor} → ${entry.title}${entry.note ? ` (${entry.note})` : ""}` + ); +} + +/** The calling agent's own session, so an agent need not pass `--from`. */ +function callerSession(): string | undefined { + return agentSessionIds(env.getProcessEnv())[0]?.id; +} + +function actorFlag(value: string | undefined, fallback: NavActor): NavActor | null { + if (value === undefined) { + return fallback; + } + + if (isNavActor(value)) { + return value; + } + + out.log.error(suggestEnumFlag(toolCommand("hub nav open"), "--actor", NAV_ACTORS, { given: value })); + return null; +} + +interface NavOpenFlags { + pr?: string; + commit?: string; + repo?: string; + worktree?: string; + session?: string; + agent?: string; + mode?: string; + tab?: string; + reveal?: string; + actor?: string; + note?: string; + from?: string; + printUrl?: boolean; + activate: boolean; + build: boolean; +} + +function flagTarget(flags: NavOpenFlags): NavTarget { + return normalizeTarget({ + pr: flags.pr, + commit: flags.commit, + repo: flags.repo ? (/^\.{1,2}(?:[/\\]|$)/.test(flags.repo) ? resolve(flags.repo) : flags.repo) : undefined, + worktree: flags.worktree ? resolve(flags.worktree) : undefined, + session: flags.session, + agent: flags.agent, + mode: flags.mode, + tab: flags.tab, + reveal: flags.reveal, + }); +} + +async function navOpen(input: string | undefined, flags: NavOpenFlags): Promise { + const parsed = parseNavInput(input ?? "", isFolder); + + if ("error" in parsed) { + out.log.error(parsed.error); + process.exitCode = 1; + return; + } + + const target: NavTarget = { ...parsed.target, ...flagTarget(flags) }; + const problem = navTargetProblem(target); + + if (problem) { + out.log.error(problem); + process.exitCode = 1; + return; + } + + if (target.mode !== undefined && !(HUB_MODES as readonly string[]).includes(target.mode)) { + out.log.error(suggestEnumFlag(toolCommand("hub nav open"), "--mode", HUB_MODES, { given: target.mode })); + process.exitCode = 1; + return; + } + + if (target.tab !== undefined && !(HUB_TABS as readonly string[]).includes(target.tab)) { + out.log.error(suggestEnumFlag(toolCommand("hub nav open"), "--tab", HUB_TABS, { given: target.tab })); + process.exitCode = 1; + return; + } + + const from = flags.from ?? parsed.from ?? callerSession(); + // An agent's shell names its session in the environment; anything else that asked is a link. + const actor = actorFlag(flags.actor ?? parsed.actor, from ? "agent" : "link"); + + if (actor === null) { + process.exitCode = 1; + return; + } + + const note = flags.note ?? parsed.note; + const mode = HUB_MODES.find((value) => value === target.mode); + const tab = HUB_TABS.find((value) => value === target.tab); + const link = { + mode, + session: target.session, + agent: target.agent, + pr: target.pr, + reveal: target.reveal, + repo: target.repo, + commit: target.commit, + tab, + worktree: target.worktree, + actor, + note, + from, + }; + + if (flags.printUrl) { + out.println(hubUrl(link)); + return; + } + + const entry: NavEntry = { id: crypto.randomUUID(), at: Date.now(), actor, title: navTitle(target), target }; + + if (note) { + entry.note = note; + } + + if (from) { + entry.from = from; + } + + const db = openNavDb(); + + try { + recordNav(db, entry); + } finally { + db.close(); + } + + logNavigation(entry, "cli"); + const progress = createBuildProgressDisplay({ + step: (message) => out.log.step(message), + spinner: () => out.spinner(), + }); + + try { + const opened = await openHub({ + ...link, + nav: entry.id, + activate: flags.activate, + build: flags.build, + onStep: progress.onStep, + onProgress: progress.onProgress, + }); + progress.finish(); + out.log.success( + `Hub opened on ${entry.title}${opened.built ? " (app rebuilt first)" : ""}; recorded as ${actor}` + ); + } catch (error) { + progress.finish(); + out.log.error( + `Recorded ${entry.title}, but the hub did not open: ${error instanceof Error ? error.message : error}` + ); + process.exitCode = 1; + } +} + +interface NavRecordFlags { + id: string; + actor: string; + title: string; + target: string; + at?: string; + note?: string; + from?: string; + keep?: number; + json?: boolean; +} + +/** The app's door: one entry the hub's engine made or changed. */ +function navRecord(flags: NavRecordFlags): void { + if (!isNavActor(flags.actor)) { + out.log.error(suggestEnumFlag(toolCommand("hub nav record"), "--actor", NAV_ACTORS, { given: flags.actor })); + process.exitCode = 1; + return; + } + + const target = normalizeTarget(SafeJSON.parse(flags.target, { strict: true })); + const at = flags.at === undefined ? Date.now() : Number(flags.at); + const entry: NavEntry = { + id: flags.id, + at: Number.isFinite(at) ? at : Date.now(), + actor: flags.actor, + title: flags.title, + target, + }; + + if (flags.note) { + entry.note = flags.note; + } + + if (flags.from) { + entry.from = flags.from; + } + + const db = openNavDb(); + + try { + const result = recordNav(db, entry, flags.keep); + logNavigation(entry, "hub"); + + if (flags.json) { + out.result({ entry, ...result }); + } + } finally { + db.close(); + } +} + +function navList(flags: { json?: boolean; limit?: number }): void { + const db = openNavDb(); + + try { + const entries = listNav(db, flags.limit); + + if (flags.json) { + out.result({ entries, keep: navKeep(db), path: navDbPath() }); + return; + } + + renderCliHeader("Hub history", `${entries.length} places, newest first · ${navDbPath()}`); + const table = createBoxTable(["WHEN", "WHO", "PLACE", "NOTE"]); + const tint = { user: pc.dim, agent: pc.magenta, link: pc.cyan } as const; + + for (const entry of entries) { + table.push([ + formatRelativeTime(new Date(entry.at)), + tint[entry.actor](entry.actor), + truncateDisplay(entry.title, 60), + truncateDisplay(entry.note ?? "", 60), + ]); + } + + out.println(table.toString()); + out.println(pc.dim(`Open one: ${toolCommand("hub nav open")} --note ""`)); + } finally { + db.close(); + } +} + +export function registerNavCommands(program: Command): void { + const nav = program + .command("nav") + .description( + "The hub's navigation history: every place it showed, who took it there (user, agent, link) and why. Back, Forward and History (⌘Y) in the hub walk it" + ); + + nav.command("list") + .description("The places, newest first") + .option("--json", "machine-readable output: { entries, keep, path }") + .option("--limit ", "at most this many", positiveInt) + .action((flags: { json?: boolean; limit?: number }) => navList(flags)); + + nav.command("open") + .description( + "Open the hub on a place AND record who asked and why. Agents: use this instead of tools hub open, so the person can go Back" + ) + .argument( + "[link-or-target]", + "a hub link, a PR (#563, group/app!12, a PR page), a commit sha, or a checkout folder" + ) + .option("--pr ", "the PR/MR: 42, group/app#42, or its page URL") + .option("--commit ", "a commit (7 to 40 hex characters) or .., in --pr or in --repo") + .option("--repo ", "with --commit and no --pr: the commit's checkout or forge project") + .option("--worktree ", "open the Worktrees mode on this checkout") + .option("--session ", "select this session (id or prefix)") + .option("--agent ", "open this agent in the Agents mode") + .option("--mode ", HUB_MODES.join("|")) + .option("--tab ", HUB_TABS.join("|")) + .option("--reveal ", "with --pr: this repo-relative file in its review") + .option("--actor ", `${NAV_ACTORS.join("|")} (default: agent inside an agent session, else link)`) + .option("--note ", "why you open it; the hub's History shows it under the place") + .option("--from ", "the agent session that asks (default: this shell's own agent session)") + .option("--print-url", "print the hub link (with actor, note, from) instead of opening and recording") + .option("--no-activate", "open behind the window in front, without taking focus") + .option("--no-build", "never build; fail when GenesisTools.app is missing or stale") + .action(async (input: string | undefined, flags: NavOpenFlags) => navOpen(input, flags)); + + nav.command("record") + .description("Store one entry (GenesisTools.app's door: every place the hub shows)") + .requiredOption("--id ", "the entry id; an existing one is replaced") + .requiredOption("--actor ", NAV_ACTORS.join("|")) + .requiredOption("--title ", "what the place is") + .requiredOption("--target ", 'the place as the hub\'s link query, JSON: {"pr":"…","commit":"…"}') + .option("--at ", "when, milliseconds since the epoch (default now)") + .option("--note ", "why") + .option("--from ", "the agent session that asked") + .option( + "--keep ", + "keep the newest n entries (the hub's History setting); stored for later writers", + positiveInt + ) + .option("--json", "print the stored entry") + .action((flags: NavRecordFlags) => navRecord(flags)); +} diff --git a/src/hub/lib/navigation/navigation.test.ts b/src/hub/lib/navigation/navigation.test.ts new file mode 100644 index 0000000000..e29ef17385 --- /dev/null +++ b/src/hub/lib/navigation/navigation.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from "bun:test"; +import { hubArgs, hubUrl } from "../open"; +import { listNav, navKeep, openNavDb, recordNav } from "./store"; +import { navTargetProblem, navTitle, parseNavInput } from "./target"; +import { type NavEntry, normalizeTarget } from "./types"; + +function entry(id: string, at: number, extra: Partial = {}): NavEntry { + return { id, at, actor: "user", title: id, target: { mode: "sessions", session: id }, ...extra }; +} + +describe("the navigation log", () => { + it("lists newest first, replaces by id and keeps the agent's note and session", () => { + const db = openNavDb(":memory:"); + recordNav(db, entry("a", 1)); + recordNav( + db, + entry("b", 2, { actor: "agent", note: "the failing run", from: "session-1", target: { pr: "563" } }) + ); + recordNav(db, entry("a", 3, { title: "a again" })); + const rows = listNav(db); + expect(rows.map((row) => row.id)).toEqual(["a", "b"]); + expect(rows[0].title).toBe("a again"); + expect(rows[1]).toMatchObject({ + actor: "agent", + note: "the failing run", + from: "session-1", + target: { pr: "563" }, + }); + db.close(); + }); + + it("keeps the newest entries, and a later writer without a cap uses the one the hub stored", () => { + const db = openNavDb(":memory:"); + expect(navKeep(db)).toBe(200); + for (let index = 0; index < 5; index++) { + recordNav(db, entry(`p${index}`, index), 3); + } + + expect(listNav(db).map((row) => row.id)).toEqual(["p4", "p3", "p2"]); + expect(navKeep(db)).toBe(3); + recordNav(db, entry("agent", 10, { actor: "agent" })); + expect(listNav(db).map((row) => row.id)).toEqual(["agent", "p4", "p3"]); + expect(listNav(db, 1).map((row) => row.id)).toEqual(["agent"]); + db.close(); + }); + + it("drops unknown target keys and empty values, and reads an unknown actor as user", () => { + expect(normalizeTarget({ pr: "1", snapshot: "/tmp/x.png", commit: " ", tab: 3 })).toEqual({ pr: "1" }); + const db = openNavDb(":memory:"); + recordNav(db, entry("x", 1)); + db.run(`UPDATE nav_entries SET actor = 'robot', target = '{"pr":"1","menu":"Go/Back"}'`); + expect(listNav(db)[0]).toMatchObject({ actor: "user", target: { pr: "1" } }); + expect(Object.keys(listNav(db)[0].target)).toEqual(["pr"]); + db.close(); + }); +}); + +describe("what tools hub nav open names", () => { + const noFolder = () => false; + + it("reads a hub link in either form with its actor, note and session", () => { + expect( + parseNavInput("genesis-tools://hub?pr=owner%2Frepo%23563&commit=0751d8a971&actor=agent¬e=why", noFolder) + ).toEqual({ + target: { pr: "owner/repo#563", commit: "0751d8a971" }, + actor: "agent", + note: "why", + from: undefined, + }); + expect(parseNavInput("https://genesis.example/hub?repo=%2Fsrc%2Fapp&commit=abcdef0&from=s1", noFolder)).toEqual( + { + target: { repo: "/src/app", commit: "abcdef0" }, + actor: undefined, + note: undefined, + from: "s1", + } + ); + }); + + it("reads a PR ref, a PR page, a sha and a folder", () => { + expect(parseNavInput("#563", noFolder)).toEqual({ target: { pr: "#563" } }); + expect(parseNavInput("https://github.com/acme/app/pull/12", noFolder)).toEqual({ + target: { pr: "https://github.com/acme/app/pull/12" }, + }); + expect(parseNavInput("0751d8a971", noFolder)).toEqual({ target: { commit: "0751d8a971" } }); + expect(parseNavInput("/src/app", (path) => path === "/src/app")).toEqual({ target: { worktree: "/src/app" } }); + expect("error" in parseNavInput("not a place", noFolder)).toBe(true); + expect("error" in parseNavInput("https://example.org/other?pr=1", noFolder)).toBe(true); + }); + + it("refuses what the hub could not open, and names the place in a few words", () => { + expect(navTargetProblem({})).toContain("name a place"); + expect(navTargetProblem({ commit: "0751d8a971" })).toContain("--pr"); + expect(navTargetProblem({ pr: "nope" })).toContain("--pr takes"); + expect(navTargetProblem({ repo: "/src/app", commit: "0751d8a971" })).toBeUndefined(); + expect(navTitle({ pr: "563", commit: "0751d8a971ff" })).toBe("PR #563 · commit 0751d8a971"); + expect(navTitle({ pr: "https://github.com/acme/app/pull/12" })).toBe("PR acme/app/pull/12"); + expect(navTitle({ repo: "/src/app", commit: "aaaaaaa..bbbbbbb" })).toBe("commit aaaaaaa..bbbbbbb in app"); + expect(navTitle({ worktree: "/src/app-feature" })).toBe("worktree app-feature"); + expect(navTitle({ mode: "inbox" })).toBe("Inbox"); + }); + + it("hands the actor, note, session and entry id to the hub as flags and link keys", () => { + const args = hubArgs({ pr: "563", actor: "agent", note: "why", from: "s1", nav: "id-1" }); + expect(args).toEqual([ + "--hub", + "--pr", + "563", + "--actor", + "agent", + "--note", + "why", + "--from", + "s1", + "--nav", + "id-1", + ]); + expect(hubUrl({ pr: "563", actor: "agent", note: "a b" })).toBe( + "genesis-tools://hub?pr=563&actor=agent¬e=a%20b" + ); + }); +}); diff --git a/src/hub/lib/navigation/store.ts b/src/hub/lib/navigation/store.ts new file mode 100644 index 0000000000..0deba6e362 --- /dev/null +++ b/src/hub/lib/navigation/store.ts @@ -0,0 +1,128 @@ +import { Database } from "bun:sqlite"; +import { mkdirSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { type Migration, runMigrations } from "@genesiscz/utils/database/migrations"; +import { SafeJSON } from "@genesiscz/utils/json"; +import { logger } from "@genesiscz/utils/logger"; +import { Storage } from "@genesiscz/utils/storage"; +import { isNavActor, NAV_DEFAULT_KEEP, type NavEntry, normalizeTarget } from "./types"; + +const { log } = logger.scoped("hub:nav"); + +export const NAV_MIGRATIONS: Migration[] = [ + { + id: "001-hub-nav-entries", + description: + "one row per place the hub showed: when, who (user, agent, link), the place, its title and the note", + apply: (db) => { + db.exec(`CREATE TABLE IF NOT EXISTS nav_entries ( + id TEXT PRIMARY KEY, + at INTEGER NOT NULL, + actor TEXT NOT NULL, + title TEXT NOT NULL, + target TEXT NOT NULL, + note TEXT, + from_session TEXT + );`); + db.exec("CREATE INDEX IF NOT EXISTS idx_nav_entries_at ON nav_entries(at);"); + db.exec("CREATE TABLE IF NOT EXISTS nav_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);"); + }, + }, +]; + +/** `~/.genesis-tools/hub/navigation.db`: one log per machine, shared by every hub and agent. */ +export function navDbPath(): string { + return join(new Storage("hub").getBaseDir(), "navigation.db"); +} + +export function openNavDb(dbPath: string = navDbPath()): Database { + if (dbPath !== ":memory:") { + mkdirSync(dirname(dbPath), { recursive: true }); + } + + const db = new Database(dbPath); + db.exec("PRAGMA journal_mode = WAL;"); + db.exec("PRAGMA busy_timeout = 2000;"); + runMigrations(db, NAV_MIGRATIONS, { tableName: "hub_nav" }); + log.debug({ dbPath }, "hub nav: opened the log"); + return db; +} + +interface NavRow { + id: string; + at: number; + actor: string; + title: string; + target: string; + note: string | null; + from_session: string | null; +} + +function fromRow(row: NavRow): NavEntry { + const entry: NavEntry = { + id: row.id, + at: row.at, + actor: isNavActor(row.actor) ? row.actor : "user", + title: row.title, + target: normalizeTarget(SafeJSON.parse(row.target, { strict: true })), + }; + + if (row.note) { + entry.note = row.note; + } + + if (row.from_session) { + entry.from = row.from_session; + } + + return entry; +} + +/** The cap the hub last set (its History setting), else the default. */ +export function navKeep(db: Database): number { + const row = db.query<{ value: string }, []>("SELECT value FROM nav_meta WHERE key = 'keep'").get(); + const keep = row ? Number(row.value) : Number.NaN; + return Number.isInteger(keep) && keep > 0 ? keep : NAV_DEFAULT_KEEP; +} + +/** + * Adds the entry, or replaces the row with its id, then keeps the newest `keep` rows. A `keep` given here is stored + * as the cap for later writers that do not know it (an agent's `tools hub nav open`). + */ +export function recordNav(db: Database, entry: NavEntry, keep?: number): { kept: number; dropped: number } { + const write = db.transaction(() => { + if (keep !== undefined) { + db.run("INSERT OR REPLACE INTO nav_meta (key, value) VALUES ('keep', ?)", [String(keep)]); + } + + db.run( + `INSERT OR REPLACE INTO nav_entries (id, at, actor, title, target, note, from_session) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + [ + entry.id, + entry.at, + entry.actor, + entry.title, + SafeJSON.stringify(normalizeTarget(entry.target)), + entry.note ?? null, + entry.from ?? null, + ] + ); + const cap = keep ?? navKeep(db); + const dropped = db.run( + "DELETE FROM nav_entries WHERE id NOT IN (SELECT id FROM nav_entries ORDER BY at DESC, rowid DESC LIMIT ?)", + [cap] + ).changes; + return { kept: cap, dropped }; + }); + return write(); +} + +/** Newest first. */ +export function listNav(db: Database, limit?: number): NavEntry[] { + const rows = + limit === undefined + ? db.query("SELECT * FROM nav_entries ORDER BY at DESC, rowid DESC").all() + : db.query("SELECT * FROM nav_entries ORDER BY at DESC, rowid DESC LIMIT ?").all(limit); + return rows.map(fromRow); +} diff --git a/src/hub/lib/navigation/target.ts b/src/hub/lib/navigation/target.ts new file mode 100644 index 0000000000..4233b78431 --- /dev/null +++ b/src/hub/lib/navigation/target.ts @@ -0,0 +1,124 @@ +import { basename } from "node:path"; +import { isHubCommitSha, isHubPrRef } from "../pr-ref"; +import { type NavTarget, normalizeTarget } from "./types"; + +/** The query of a hub link, `genesis-tools://hub?…` or `https:///hub?…`; undefined for anything else. */ +function hubLinkQuery(input: string): URLSearchParams | undefined { + if (!URL.canParse(input)) { + return undefined; + } + + const url = new URL(input); + + if (url.protocol === "genesis-tools:") { + // `genesis-tools://hub?…` parses with `hub` as the host; some builders write `genesis-tools:hub?…`. + return url.host === "hub" || url.pathname.replace(/^\/+/, "") === "hub" ? url.searchParams : undefined; + } + + if ((url.protocol === "https:" || url.protocol === "http:") && /^\/hub\/?$/.test(url.pathname)) { + return url.searchParams; + } + + return undefined; +} + +export type ParsedNavInput = { target: NavTarget; actor?: string; note?: string; from?: string } | { error: string }; + +/** + * What `tools hub nav open ` names: a hub link (its `actor`, `note` and `from` come along), a PR or MR + * page, a PR ref (`#563`, `group/app!12`), a commit sha (needs `--pr` or `--repo` beside it), or a checkout folder. + */ +export function parseNavInput(input: string, isFolder: (path: string) => boolean): ParsedNavInput { + const text = input.trim(); + + if (text === "") { + return { target: {} }; + } + + const query = hubLinkQuery(text); + + if (query) { + const raw: Record = {}; + + for (const [key, value] of query) { + raw[key] = value; + } + + return { + target: normalizeTarget(raw), + actor: query.get("actor") ?? undefined, + note: query.get("note") ?? undefined, + from: query.get("from") ?? undefined, + }; + } + + if (isHubPrRef(text)) { + return { target: { pr: text } }; + } + + if (isHubCommitSha(text)) { + return { target: { commit: text } }; + } + + if (isFolder(text)) { + return { target: { worktree: text } }; + } + + return { + error: `${text} is not a hub link, a PR (#563, group/app!12, a PR page), a commit sha or a checkout folder`, + }; +} + +/** What a place is, in a few words, until the hub names it better ("PR #563 · commit 0751d8a971"). */ +export function navTitle(target: NavTarget): string { + const commit = target.commit + ? `commit ${target.commit + .split("..") + .map((sha) => sha.slice(0, 10)) + .join("..")}` + : ""; + + if (target.pr) { + const pr = /^\d+$/.test(target.pr) ? `#${target.pr}` : target.pr.replace(/^https?:\/\/[^/]+\//, ""); + return [`PR ${pr}`, commit].filter(Boolean).join(" · "); + } + + if (target.repo && commit) { + return `${commit} in ${basename(target.repo)}`; + } + + if (target.worktree) { + return target.worktree === "cleanup" ? "worktree cleanup" : `worktree ${basename(target.worktree)}`; + } + + if (target.agent) { + return `agent ${target.agent}`; + } + + if (target.session) { + return `session ${target.session.slice(0, 8)}`; + } + + return target.mode ? target.mode[0].toUpperCase() + target.mode.slice(1) : "Hub"; +} + +/** Why the place cannot open as given (the same rules as `tools hub open`); undefined when it can. */ +export function navTargetProblem(target: NavTarget): string | undefined { + if (Object.keys(target).length === 0) { + return "name a place: a hub link, --pr, --commit with --repo, --worktree, --session or --mode"; + } + + if (target.pr !== undefined && !isHubPrRef(target.pr)) { + return `--pr takes 42, #42, #42 or a PR page URL, got ${target.pr}`; + } + + if (target.commit !== undefined && !isHubCommitSha(target.commit)) { + return `--commit takes a sha of 7 to 40 hex characters, or .., got ${target.commit}`; + } + + if (target.commit !== undefined && target.pr === undefined && target.repo === undefined) { + return "--commit needs the PR it is in (--pr) or its project (--repo )"; + } + + return undefined; +} diff --git a/src/hub/lib/navigation/types.ts b/src/hub/lib/navigation/types.ts new file mode 100644 index 0000000000..a1eed3d9c1 --- /dev/null +++ b/src/hub/lib/navigation/types.ts @@ -0,0 +1,67 @@ +/** + * The hub's navigation history: every place the hub showed, who took it there and why. The log lives here, in + * `~/.genesis-tools/hub/navigation.db`; GenesisTools.app reads and writes it through `tools hub nav`, and its + * GenesisKit engine (`NavigationHistory`) walks it with Back, Forward and a hop. + */ + +/** Who moved the hub: a click in the hub, an agent (`tools hub nav open`), or a link that named nobody. */ +export const NAV_ACTORS = ["user", "agent", "link"] as const; +export type NavActor = (typeof NAV_ACTORS)[number]; + +/** + * A place, as the hub's link query names it (`HUB_LINK_KEYS`, read by `HubRequest` in the app). `inbox` is the hub's + * own: the Inbox mode's selected session, which no link names. + */ +export const NAV_TARGET_KEYS = [ + "mode", + "session", + "agent", + "pr", + "reveal", + "repo", + "commit", + "tab", + "worktree", + "inbox", +] as const; +export type NavTargetKey = (typeof NAV_TARGET_KEYS)[number]; +export type NavTarget = Partial>; + +export interface NavEntry { + id: string; + /** Milliseconds since the epoch. */ + at: number; + actor: NavActor; + title: string; + target: NavTarget; + note?: string; + /** The agent session that asked for it. */ + from?: string; +} + +export const NAV_DEFAULT_KEEP = 200; + +export function isNavActor(value: unknown): value is NavActor { + return typeof value === "string" && (NAV_ACTORS as readonly string[]).includes(value); +} + +function isNavTargetKey(value: string): value is NavTargetKey { + return (NAV_TARGET_KEYS as readonly string[]).includes(value); +} + +/** The known keys of `raw` with a non-empty string value; anything else is dropped. */ +export function normalizeTarget(raw: unknown): NavTarget { + const target: NavTarget = {}; + + if (typeof raw !== "object" || raw === null) { + return target; + } + + for (const [key, value] of Object.entries(raw)) { + if (isNavTargetKey(key) && typeof value === "string" && value.trim() !== "") { + target[key] = value.trim(); + } + } + + return target; +} diff --git a/src/hub/lib/open.ts b/src/hub/lib/open.ts index e967a88c8a..4ba1717238 100644 --- a/src/hub/lib/open.ts +++ b/src/hub/lib/open.ts @@ -52,6 +52,14 @@ export interface OpenHubOptions { prompts?: boolean; /** Opens the handoff composer for `session` (or the selected session). */ handoff?: boolean; + /** Who asked for this place, for the hub's history: `user`, `agent` or `link` (the hub's default). */ + actor?: string; + /** Why the place was opened; the history list shows it under the place. */ + note?: string; + /** The agent session that asked for it. */ + from?: string; + /** The history entry `tools hub nav open` already stored, so the hub updates it instead of adding one. */ + nav?: string; /** false keeps the window behind whatever is in front (`--no-activate`). */ activate?: boolean; /** false never builds; a missing or stale app is then an error. */ @@ -141,6 +149,17 @@ export function hubArgs(options: OpenHubOptions): string[] { args.push("--handoff"); } + for (const [flag, value] of [ + ["--actor", options.actor], + ["--note", options.note], + ["--from", options.from], + ["--nav", options.nav], + ] as const) { + if (value) { + args.push(flag, value); + } + } + if (options.activate === false) { args.push("--no-activate"); } diff --git a/src/utils/app-links/targets.ts b/src/utils/app-links/targets.ts index 55ee5412be..6c4ee5f056 100644 --- a/src/utils/app-links/targets.ts +++ b/src/utils/app-links/targets.ts @@ -20,6 +20,13 @@ export const HUB_LINK_KEYS = [ "tab", "filter", "worktree", + // Who asked for the place, for the hub's navigation history (`tools hub nav open`): `actor` is user, agent or + // link (a link without it counts as link), `note` says why, `from` is the agent's session id, `nav` the id the + // CLI already stored the entry under, so the hub updates that entry instead of adding a second one. + "actor", + "note", + "from", + "nav", ] as const; export type HubLinkQuery = Partial>; From 541a2192dd432ad23734016a23598cdea3af77b9 Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 18:37:36 +0100 Subject: [PATCH 06/11] =?UTF-8?q?feat(GenesisKit,=20GenesisHub):=20the=20h?= =?UTF-8?q?ub's=20Back=20and=20Forward=20walk=20the=20shared=20navigation?= =?UTF-8?q?=20log,=20so=20a=20link=20or=20an=20agent=20opening=20a=20PR,?= =?UTF-8?q?=20a=20commit=20or=20a=20session=20adds=20a=20place=20instead?= =?UTF-8?q?=20of=20replacing=20it;=20the=20History=20popover=20(=E2=8C=98Y?= =?UTF-8?q?)=20lists=20every=20place=20with=20User,=20Agent=20or=20Link=20?= =?UTF-8?q?and=20the=20agent's=20note,=20and=20a=20click=20hops=20there?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit 1cbefbe3110994c99c0cc5dbf25a2d4cae8ad13e) --- src/hub/lib/navigation/types.ts | 7 +- .../Navigation/NavigationHistory.swift | 42 +++ .../NavigationHistoryTests.swift | 41 +++ .../GenesisTools/Sources/Host/HubLink.swift | 2 + .../Sources/Parts/Hub/Hub/HubCommitLink.swift | 3 +- .../Sources/Parts/Hub/Hub/HubKeymap.swift | 3 + .../Sources/Parts/Hub/Hub/HubMainMenu.swift | 1 + .../Sources/Parts/Hub/Hub/HubNavigation.swift | 277 +++++++++++--- .../Sources/Parts/Hub/Hub/HubWindow.swift | 348 +++++++++++++++--- .../GenesisTools/Tests/HubKeymapTests.swift | 10 +- .../Tests/HubNavigationTests.swift | 206 +++++++---- 11 files changed, 773 insertions(+), 167 deletions(-) diff --git a/src/hub/lib/navigation/types.ts b/src/hub/lib/navigation/types.ts index a1eed3d9c1..4a005ebdb2 100644 --- a/src/hub/lib/navigation/types.ts +++ b/src/hub/lib/navigation/types.ts @@ -9,8 +9,9 @@ export const NAV_ACTORS = ["user", "agent", "link"] as const; export type NavActor = (typeof NAV_ACTORS)[number]; /** - * A place, as the hub's link query names it (`HUB_LINK_KEYS`, read by `HubRequest` in the app). `inbox` is the hub's - * own: the Inbox mode's selected session, which no link names. + * A place, as the hub's link query names it (`HUB_LINK_KEYS`, read by `HubRequest` in the app). `selection` is the + * hub's own: the row its mode had selected (a session row id, a worktree path, a PR's id), which the hub restores + * exactly; a place an agent named has the link keys only. */ export const NAV_TARGET_KEYS = [ "mode", @@ -22,7 +23,7 @@ export const NAV_TARGET_KEYS = [ "commit", "tab", "worktree", - "inbox", + "selection", ] as const; export type NavTargetKey = (typeof NAV_TARGET_KEYS)[number]; export type NavTarget = Partial>; diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift index 64881572d6..b847fcdb9e 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift @@ -49,6 +49,8 @@ public struct NavigationEntry: Identifiable, Codable } } +extension NavigationEntry: Sendable where Target: Sendable {} + /// Where a host keeps the log. `load` runs once at start, `record` after each new or changed entry; both may be /// slow (a `tools` call), so the host runs them off the main thread. public protocol NavigationHistoryStore: AnyObject { @@ -244,6 +246,46 @@ public struct NavigationHistory: Equatable { return entries[index] } + /// Gives an entry the place its target turned out to be once shown (an agent's `pr=563` becomes the hub's own + /// form of that PR), so the place on screen and its entry stay equal and the next look at it adds nothing. + public mutating func retarget(_ id: String, to target: Target, title: String? = nil) -> Entry? { + guard let index = entries.lastIndex(where: { $0.id == id }) else { return nil } + let unchanged = entries[index].target == target && (title.map { $0.isEmpty || $0 == entries[index].title } ?? true) + guard !unchanged else { return nil } + entries[index].target = target + if let title, !title.isEmpty { entries[index].title = title } + if id == currentID { currentTarget = target } + return entries[index] + } + + /// Puts a log read back from the store behind what this window already did since it opened: entries it does + /// not hold yet, oldest first, become the oldest places Back reaches. When the log ends on the place this window + /// opened on (a relaunch on the place it left), the two are one entry, the log's: its id, who and note. Returns + /// the ids of this window's entries that merged away, which the host must not store. + @discardableResult + public mutating func adoptOlder(_ older: [Entry]) -> [String] { + let known = Set(entries.map(\.id)) + var fresh = older.filter { !known.contains($0.id) }.sorted { $0.at < $1.at } + guard !fresh.isEmpty else { return [] } + var merged: [String] = [] + if let newest = fresh.last, let oldest = entries.first, newest.target == oldest.target { + fresh.removeLast() + merged.append(oldest.id) + entries[0] = newest + backIDs = backIDs.map { $0 == oldest.id ? newest.id : $0 } + forwardIDs = forwardIDs.map { $0 == oldest.id ? newest.id : $0 } + if currentID == oldest.id { currentID = newest.id } + } + entries = fresh + entries + backIDs = fresh.map(\.id) + backIDs + if currentID == nil, currentTarget == nil, let last = backIDs.popLast() { + currentID = last + currentTarget = entry(last)?.target + } + trim() + return merged + } + private mutating func move(to id: String) -> Entry? { currentID = id let landed = entry(id) diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift index 5ca7b251f5..07bf7ea8ef 100644 --- a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift @@ -159,6 +159,47 @@ final class NavigationHistoryTests: XCTestCase { XCTAssertEqual(launched.backEntry?.target, "b") } + func testRetargetMakesTheShownPlaceEqualToItsEntry() throws { + var history = walk(["a"]) + history.visit("pr=563", title: "PR #563", actor: .agent, note: "why") + let id = try XCTUnwrap(history.currentID) + XCTAssertEqual(history.retarget(id, to: "prs:github.com/acme/app/pull/563", title: "#563 Fix it")?.title, "#563 Fix it") + XCTAssertNil(history.visit("prs:github.com/acme/app/pull/563", title: "#563 Fix it"), "the shown place is the entry") + XCTAssertEqual(history.current?.note, "why") + XCTAssertNil(history.retarget(id, to: "prs:github.com/acme/app/pull/563")) + } + + func testAnOlderLogGoesBehindWhatTheWindowDidSinceItOpened() { + let now = Date() + var history = History() + history.visit("launch", title: "launch", at: now) + history.adoptOlder([ + NavigationEntry(id: "o2", at: now.addingTimeInterval(-10), target: "older2", title: "older2"), + NavigationEntry(id: "o1", at: now.addingTimeInterval(-20), target: "older1", title: "older1"), + ]) + XCTAssertEqual(history.current?.target, "launch") + XCTAssertEqual(history.goBack()?.target, "older2") + XCTAssertEqual(history.goBack()?.target, "older1") + // A relaunch on the place the log ended with: one entry, the log's, with its who and note. + var relaunched = History() + relaunched.visit("pr1", title: "pr1", id: "window-id", at: now) + relaunched.visit("pr2", title: "pr2", at: now.addingTimeInterval(1)) + let merged = relaunched.adoptOlder([ + NavigationEntry(id: "log-a", at: now.addingTimeInterval(-30), target: "a", title: "a"), + NavigationEntry(id: "log-pr1", at: now.addingTimeInterval(-20), target: "pr1", title: "pr1", actor: .agent, note: "n"), + ]) + XCTAssertEqual(merged, ["window-id"]) + XCTAssertEqual(relaunched.entries.map(\.id).prefix(2), ["log-a", "log-pr1"]) + XCTAssertEqual(relaunched.entries.count, 3) + XCTAssertEqual(relaunched.goBack()?.note, "n") + XCTAssertEqual(relaunched.goBack()?.target, "a") + // Nothing on screen yet: the newest adopted entry is the current place. + var empty = History() + empty.adoptOlder([NavigationEntry(id: "x", at: now, target: "x", title: "x")]) + XCTAssertEqual(empty.current?.target, "x") + XCTAssertFalse(empty.canGoBack) + } + func testRetitleChangesOnlyTheNamedEntry() throws { var history = walk(["a", "b"]) let a = try XCTUnwrap(history.entries.first) diff --git a/src/macos/GenesisTools/Sources/Host/HubLink.swift b/src/macos/GenesisTools/Sources/Host/HubLink.swift index 7f844d0967..d07acb70d8 100644 --- a/src/macos/GenesisTools/Sources/Host/HubLink.swift +++ b/src/macos/GenesisTools/Sources/Host/HubLink.swift @@ -7,6 +7,8 @@ package enum HubLink { /// show, not a file to write or a command to run. package static let linkKeys: Set = [ "mode", "session", "agent", "pr", "reveal", "commit", "repo", "tab", "filter", "worktree", "decision", "question", + // Who asked and why, for the hub's history (`tools hub nav open`): never a command, only words on an entry. + "actor", "note", "from", "nav", ] /// `genesis-tools://hub?session=&agent=` (any of `linkKeys`) as `--hub` arguments, the diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubCommitLink.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubCommitLink.swift index e495abe2b9..328095de45 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubCommitLink.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubCommitLink.swift @@ -272,7 +272,7 @@ extension HubModel { guard commitLinkRequest == request else { return } switch outcome { case .found(let found): - showCommit(repo: found.root, sha: found.sha, title: found.title, base: found.base) + showCommit(repo: found.root, sha: found.sha, title: found.title, base: found.base, link: commitLinkRaw) case .failed(let message): HubPerf.log("hub.commitLink \(message)") notice = message @@ -293,6 +293,7 @@ extension HubModel { notice = "Commit \(span.label) needs its PR (pr=) or its project (repo=) to open" return } + commitLinkRaw = span.to.map { "\(span.from)..\($0)" } ?? span.from cancelSessionNavigation() let known = prs.allProjects Task { diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubKeymap.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubKeymap.swift index 14fea3d5e7..310dee41e6 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubKeymap.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubKeymap.swift @@ -125,6 +125,9 @@ enum HubKeymap { // The two keys right of P: "[" and "]" on US, "ú" and ")" on Czech. The position is the shortcut. KeymapCommand("hub.go.back", "Back", group: "Go", keys: [.code(33, .command)]), KeymapCommand("hub.go.forward", "Forward", group: "Go", keys: [.code(30, .command)]), + KeymapCommand("hub.go.history", "History", group: "Go", + detail: "Every place the hub showed, who opened it (you, an agent, a link) and why.", + keys: [.character("y", .command)]), KeymapCommand("hub.go.session", "Session", group: "Go", keys: []), KeymapCommand("hub.go.worktree", "Worktree", group: "Go", keys: []), KeymapCommand("hub.go.pr", "Pull Request", group: "Go", keys: []), diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubMainMenu.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubMainMenu.swift index c3982d5bd5..12a84f5c56 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubMainMenu.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubMainMenu.swift @@ -245,6 +245,7 @@ enum HubMenuCommands { static let go: [HubMenuCommand] = [ HubMenuCommand(title: "Back", id: "hub.go.back", run: { $0.goBack() }, enabled: { $0.history.canGoBack }), HubMenuCommand(title: "Forward", id: "hub.go.forward", run: { $0.goForward() }, enabled: { $0.history.canGoForward }), + HubMenuCommand(title: "History…", id: "hub.go.history", run: { $0.historyOpen.toggle() }), HubMenuCommand(title: "Session…", id: "hub.go.session", run: { palette($0, "session ") }, separated: true), HubMenuCommand(title: "Worktree…", id: "hub.go.worktree", run: { palette($0, "worktree ") }), HubMenuCommand(title: "Pull Request…", id: "hub.go.pr", run: { palette($0, "pr ") }), diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift index a0ecbebcd2..6ef9cc5aca 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift @@ -1,65 +1,202 @@ import AppKit import SwiftUI -// Back and forward through what the hub showed: every mode switch, session, worktree and PR picked, -// and "Open" from a PR's or a worktree's session list. ⌘[ and ⌘], the arrows above the mode switch, -// the mouse's side buttons and a trackpad swipe all walk the same history (`HubModel.goBack`). +// Back and forward through what the hub showed, and the log of it: every mode switch, session, worktree, PR and +// commit, who took the hub there (a click here, an agent, a link) and why. The engine is GenesisKit's +// `NavigationHistory`; the log lives in `tools hub nav` (`~/.genesis-tools/hub/navigation.db`), so an agent's +// `tools hub nav open` and the hub write one list. ⌘[ and ⌘], the arrows above the mode switch, the mouse's side +// buttons and a trackpad swipe walk it (`HubModel.goBack`); ⌘Y and the clock button list it (`HubHistoryButton`). -/// One place in the hub: the mode and what it had selected. +/// The place a mode shows: the mode and what it had selected. struct HubNavEntry: Equatable { var mode: HubMode /// The session row id, worktree path or PR id the mode shows; nil while nothing is picked yet. var selection: String? } -/// A browser's history: `visit` pushes, `goBack` and `goForward` walk it. Pure, so tests drive it. -struct HubNavHistory: Equatable { - static let limit = 100 - - private(set) var back: [HubNavEntry] = [] - private(set) var current: HubNavEntry? - private(set) var forward: [HubNavEntry] = [] - - var canGoBack: Bool { !back.isEmpty } - var canGoForward: Bool { !forward.isEmpty } - - /// A new place. A new place after going back drops the forward entries, as a browser does. - /// - /// A place with nothing picked (the hub's first moment, a mode whose list still loads) is never - /// kept: whatever follows replaces it. Otherwise every launch began with a dead "back" to an - /// empty Sessions mode, and a quick look at a loading mode left a stop with nothing in it. - mutating func visit(_ entry: HubNavEntry) { - guard entry != current else { return } - if let current, current.selection != nil { - back.append(current) - if back.count > Self.limit { - back.removeFirst(back.count - Self.limit) - } - forward.removeAll() - } else if back.last == entry { - // Out to an empty mode and straight back: the place before it, not a copy of it. - current = back.popLast() - return - } - current = entry +/// A place in the history: the hub's link keys (`HubLink.linkKeys`) plus `selection`, the row the hub's own mode +/// had selected. A place the hub showed has `mode` and `selection` and is restored exactly; a place an agent named +/// (`tools hub nav open`) has link keys only and opens as that link would. Stored as a flat JSON object, which is +/// the `target` of `tools hub nav` (src/hub/lib/navigation/types.ts). +struct HubNavTarget: Codable, Hashable, Sendable { + private(set) var fields: [String: String] + + /// The keys a place keeps, in the order `arguments` writes them. + static let keys = ["mode", "selection", "session", "agent", "pr", "reveal", "repo", "commit", "tab", "worktree"] + + init(_ fields: [String: String]) { + self.fields = fields.filter { Self.keys.contains($0.key) && !$0.value.isEmpty } + } + + /// The place a mode shows, with the commit a link or an Activity row opened there. + init(place: HubNavEntry, commit: String? = nil, repo: String? = nil) { + var fields = ["mode": place.mode.rawValue] + fields["selection"] = place.selection + fields["commit"] = commit + fields["repo"] = repo + self.init(fields) + } + + init(from decoder: Decoder) throws { + self.init(try decoder.singleValueContainer().decode([String: String].self)) + } + + func encode(to encoder: Encoder) throws { + var container = encoder.singleValueContainer() + try container.encode(fields) + } + + subscript(_ key: String) -> String? { fields[key] } + + var mode: HubMode? { fields["mode"].flatMap(HubMode.init(rawValue:)) } + + /// The hub's own place, when it has one. + var place: HubNavEntry? { + guard let mode, let selection = fields["selection"] else { return nil } + return HubNavEntry(mode: mode, selection: selection) + } + + /// The `--hub` flags of a place an agent or a link named. + var arguments: [String] { + Self.keys.filter { $0 != "selection" }.flatMap { key in fields[key].map { ["--\(key)", $0] } ?? [] } + } +} + +typealias HubNavHistory = NavigationHistory + +/// The commit a link or an Activity row shows in a checkout's Worktrees diff (`HubModel.showCommit`). +struct HubShownCommit: Equatable { + /// The checkout, as the Worktrees mode selects it. + let repo: String + /// One sha or `..`, as the link wrote it. + let commit: String +} + +/// A place an agent or a link asked for, with who and why, until the hub shows it. Matched against what the hub +/// shows (`HubModel.navRequestMatches`), so a PR that loads for a few seconds still gets its note. +struct HubNavRequest { + var actor: NavigationActor + var note: String? + var from: String? + /// The entry `tools hub nav open` stored; the hub's record replaces it. + var id: String? + var pr: HubPRRef? + var commit: String? + var repo: String? + var session: String? + var agent: String? + var worktree: String? + var mode: HubMode? + /// A history entry being shown again: its own entry takes the shown place's form instead of a new entry. + var restoring = false + let asked: Date + + /// Nil for a request that names no place (an overlay, `--palette`, a bare `tools hub`). + init?(_ request: HubRequest, now: Date = Date()) { + guard request.session != nil || request.mode != nil || request.agent != nil || request.worktree != nil + || request.pr != nil || request.commit != nil + else { return nil } + actor = request.actor.flatMap(NavigationActor.init(rawValue:)) ?? .link + note = request.note + from = request.from + id = request.nav + pr = request.pr + commit = request.commit + repo = request.repo + session = request.session + agent = request.agent + worktree = request.worktree + mode = request.mode + asked = now } - mutating func goBack() -> HubNavEntry? { - guard let previous = back.popLast() else { return nil } - if let current, current.selection != nil { - forward.append(current) + /// How long the hub waits for the place before it forgets who asked. + static let patience: TimeInterval = 45 + /// How long, after the request, the hub's own steps on the way (the mode switch, the old row still selected) + /// are not taken for the person's clicks. + static let settle: TimeInterval = 3 + + /// The link keys, for an entry the hub adds without showing it (agents only add to History). + var target: HubNavTarget { + var fields: [String: String] = [:] + fields["mode"] = mode?.rawValue + fields["session"] = session + fields["agent"] = agent + fields["pr"] = pr.map { $0.pageURL ?? $0.label } + fields["commit"] = commit + fields["repo"] = repo + fields["worktree"] = worktree + return HubNavTarget(fields) + } + + /// "PR #563 · commit 0751d8a971", until the hub can name it better. + var title: String { + let commitLabel = commit.map { "commit " + $0.split(separator: ".").filter { !$0.isEmpty }.map { String($0.prefix(10)) }.joined(separator: "..") } + if let pr { + return (["PR \(pr.label)"] + (commitLabel.map { [$0] } ?? [])).joined(separator: " · ") + } + if let commitLabel, let repo { + return "\(commitLabel) in \((repo as NSString).lastPathComponent)" } - current = previous - return previous + if let worktree { return "worktree " + (worktree as NSString).lastPathComponent } + if let agent { return "agent \(agent)" } + if let session { return "session \(session.prefix(8))" } + return mode?.title ?? "Hub" } +} + +/// The log, through `tools hub nav` (the hub's resident server answers it without a new process). Every call runs +/// on `queue`, never on the main thread. +final class HubNavStore: NavigationHistoryStore, Sendable { + typealias Target = HubNavTarget + + static let queue = DispatchQueue(label: "hub.nav.store", qos: .utility) + let limit: Int - mutating func goForward() -> HubNavEntry? { - guard let next = forward.popLast() else { return nil } - if let current, current.selection != nil { - back.append(current) + init(limit: Int) { + self.limit = limit + } + + private struct Row: Decodable { + let id: String + let at: Double + let actor: String + let title: String + let target: HubNavTarget + let note: String? + let from: String? + } + + private struct Listing: Decodable { + let entries: [Row] + } + + func load() throws -> [NavigationEntry] { + try Self.decode(ToolsCLIRunner.run(["hub", "nav", "list", "--json", "--limit", String(limit)])) + } + + /// `tools hub nav list --json`: `{ entries: [{ id, at (ms), actor, title, target, note?, from? }], keep, path }`. + static func decode(_ data: Data) throws -> [NavigationEntry] { + try JSONDecoder().decode(Listing.self, from: data).entries.map { row in + NavigationEntry(id: row.id, at: Date(timeIntervalSince1970: row.at / 1000), target: row.target, title: row.title, + actor: NavigationActor(rawValue: row.actor) ?? .user, agentSession: row.from, note: row.note) } - current = next - return next + } + + func record(_ entry: NavigationEntry, keep: Int) throws { + _ = try ToolsCLIRunner.run(try Self.recordArguments(entry, keep: keep)) + } + + static func recordArguments(_ entry: NavigationEntry, keep: Int) throws -> [String] { + let target = String(decoding: try JSONEncoder().encode(entry.target), as: UTF8.self) + // Free text rides as `--flag=value`, so a title or a note that starts with "-" is not read as a flag. + var args = [ + "hub", "nav", "record", "--id=\(entry.id)", "--at=\(Int64(entry.at.timeIntervalSince1970 * 1000))", + "--actor=\(entry.actor.rawValue)", "--title=\(entry.title)", "--target=\(target)", "--keep=\(keep)", + ] + if let note = entry.note { args.append("--note=\(note)") } + if let from = entry.agentSession { args.append("--from=\(from)") } + return args } } @@ -97,7 +234,8 @@ enum HubNavInput { // MARK: - Buttons -/// The back and forward arrows above the mode switch. The tooltip names where each one goes. +/// The back and forward arrows above the mode switch, and the history list beside them. The tooltips name where +/// each arrow goes and who took the hub there. struct HubNavButtons: View { @ObservedObject var model: HubModel @@ -105,15 +243,25 @@ struct HubNavButtons: View { let history = model.history HStack(spacing: 4) { arrow("chevron.left", enabled: history.canGoBack, - tooltip: history.back.last.map { "Back to \(model.navTitle($0)) (\(HubKeymap.label("hub.go.back")))" } + tooltip: history.backEntry.map { "Back to \(Self.describe($0)) (\(HubKeymap.label("hub.go.back")))" } ?? "\(HubKeymap.hint("Back", "hub.go.back")): nothing earlier") { model.goBack() } arrow("chevron.right", enabled: history.canGoForward, - tooltip: history.forward.last.map { "Forward to \(model.navTitle($0)) (\(HubKeymap.label("hub.go.forward")))" } + tooltip: history.forwardEntry.map { "Forward to \(Self.describe($0)) (\(HubKeymap.label("hub.go.forward")))" } ?? "\(HubKeymap.hint("Forward", "hub.go.forward")): nothing later") { model.goForward() } + HubHistoryButton(model: model) + } + } + + /// "PR #563 Fix it", or "PR #563 Fix it, opened by an agent" for a place the person did not pick. + static func describe(_ entry: NavigationEntry) -> String { + switch entry.actor { + case .user: return entry.title + case .agent: return "\(entry.title), opened by an agent" + case .link: return "\(entry.title), opened by a link" } } @@ -123,3 +271,34 @@ struct HubNavButtons: View { .disabled(!enabled) } } + +/// The clock beside the arrows: every place, newest first, with who went there and why; a click hops there. +struct HubHistoryButton: View { + @ObservedObject var model: HubModel + + var body: some View { + IconButton(systemName: "clock.arrow.circlepath", tooltip: HubKeymap.hint("History", "hub.go.history"), size: 12) { + model.historyOpen.toggle() + } + .foregroundColor(model.history.entries.isEmpty ? ReviewPalette.dim.opacity(0.6) : ReviewPalette.dim) + .popover(isPresented: $model.historyOpen, arrowEdge: .bottom) { + HubHistoryPopover(history: model.history) { id in + model.historyOpen = false + model.hop(to: id) + } + } + } +} + +struct HubHistoryPopover: View { + let history: HubNavHistory + let onHop: (String) -> Void + + var body: some View { + NavigationHistoryList(history: history, maxHeight: 460, onHop: onHop) + .frame(width: 400) + .padding(.bottom, 4) + .background(Color(nsColor: KitPalette.background)) + .environment(\.colorScheme, .dark) + } +} diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift index f6d0bf8f8f..a7a5e2abe7 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift @@ -92,6 +92,12 @@ struct HubRequest { /// (`--menu "Go/Pull Request…"` opens the palette with "pr " typed). Verifies the menu in a snapshot. var menu: String? var activate = true + /// Who asked for the place, for the history (`--actor user|agent|link`; a request without it counts as a link), + /// why (`--note`), the agent's session (`--from`) and the entry `tools hub nav open` stored (`--nav`). + var actor: String? + var note: String? + var from: String? + var nav: String? /// The file `--reveal` names, in the PR `--pr` names. var prReveal: PRReveal? { @@ -162,6 +168,10 @@ struct HubRequest { } index += 1 case "--menu": menu = value; index += 1 + case "--actor": actor = value; index += 1 + case "--note": note = value; index += 1 + case "--from": from = value; index += 1 + case "--nav": nav = value; index += 1 case "--no-activate": activate = false default: break } @@ -187,6 +197,8 @@ struct HubRequest { package func runHub(_ args: [String]) -> Never { PerfLog.phase("hub.launch") + // A relaunch's place is the one the person left, not a link's. + let resuming = args.contains(HubPlace.resumeFlag) // `--resume` (a rebuild's relaunch) opens the place the hub last showed (Hub/HubPlace.swift). let args = HubPlace.expand(args) let request = HubRequest(args) @@ -229,6 +241,11 @@ package func runHub(_ args: [String]) -> Never { AppMainMenu.install(hub: model) } model.initialMode = mode + if !request.isScripted { + // Who asked for the launch's place: the first time the hub shows it, the history entry carries it. + model.pendingNav = resuming ? nil : HubNavRequest(request) + MainActor.assumeIsolated { model.connectNavLog() } + } if let panes = request.panes { model.panes = panes } if let worktree = request.worktree { model.selectedWorktree = WorktreeCleanup.selection(for: worktree) @@ -339,7 +356,8 @@ package func runHub(_ args: [String]) -> Never { MainActor.assumeIsolated { HubVisibility.shared.watch(window) } HubSingleInstance.serve { args in let later = HubRequest(args) - model.apply(later) + // An agent's place while agents only add to History: the view and the focus stay where they are. + guard model.apply(later) else { return } if let path = later.menu { MainActor.assumeIsolated { AppMainMenu.perform(path) } } @@ -682,9 +700,24 @@ final class HubModel: ObservableObject { /// PRs mode state (`tools hub pr list/show`). Main-actor: created on the main thread in `runHub`. let prs: PRsModel - /// Back and forward (Hub/HubNavigation.swift). Taken from the published selection itself, so - /// every path that changes it (rows, the palette, a handed-over `--session`, "Open") is recorded. + /// Back, forward and the History list (Hub/HubNavigation.swift). Taken from the published selection itself, so + /// every path that changes it (rows, the palette, a handed-over `--session`, "Open", a link) is recorded. @Published private(set) var history = HubNavHistory() + /// The History popover (⌘Y, the clock beside the arrows). + @Published var historyOpen = false + /// The commit a link or an Activity row shows in the Worktrees diff: part of that place in the history. + @Published private(set) var shownCommit: HubShownCommit? + /// The commit a link selected in a PR's review: part of that PR's place while it stays selected. + private var prLinkedCommit: (prID: String, commit: String)? + /// The place an agent or a link asked for, with who and why, until the hub shows it. + var pendingNav: HubNavRequest? + /// A `repo=…&commit=…` link's commit as written (one sha or `..`), for the place `showCommit` shows. + var commitLinkRaw: String? + /// `tools hub nav`; nil in a scripted run, which never reads or writes the log. + private var navStore: HubNavStore? + /// Entries made before the log was read back: written once it is, unless they merged into its last entry. + private var navUnsaved: [String]? + private var navLimitWatch: AnyCancellable? private var navRecorder: AnyCancellable? private var navRecordPending = false private var navRestoring = false @@ -706,7 +739,10 @@ final class HubModel: ObservableObject { if !panes.contains(tab) { panes = HubTab.allCases.filter { panes.contains($0) || $0 == tab } } - MainActor.assumeIsolated { startNavRecorder() } + MainActor.assumeIsolated { + startNavRecorder() + startNavLog() + } // A scripted run with the transcript pane settles on that pane's first page (see `select`). firstPageObserver = NotificationCenter.default.addObserver(forName: HubSessionDetailHost.firstPageDone, object: nil, queue: .main) { [weak self] _ in MainActor.assumeIsolated { @@ -727,6 +763,7 @@ final class HubModel: ObservableObject { prs.$selectedID.map { _ in () }.eraseToAnyPublisher(), inbox.$selectedID.map { _ in () }.eraseToAnyPublisher(), agents.$selectedID.map { _ in () }.eraseToAnyPublisher(), + $shownCommit.map { _ in () }.eraseToAnyPublisher(), ] navRecorder = Publishers.MergeMany(changes).sink { [weak self] in if self?.settlingLaunch == false { @@ -744,9 +781,164 @@ final class HubModel: ObservableObject { DispatchQueue.main.async { [weak self] in guard let self else { return } self.navRecordPending = false - MainActor.assumeIsolated { HubPlace.record(self.navEntry, tab: self.tab) } - guard !self.navRestoring else { return } - MainActor.assumeIsolated { self.history.visit(self.navEntry) } + MainActor.assumeIsolated { self.recordPlace() } + } + } + + /// The History setting's cap, followed at once. + @MainActor + private func startNavLog() { + let preferences = HubDefaults.preferences + history.setLimit(preferences.navigationLimit) + navLimitWatch = preferences.$navigationLimit.dropFirst().sink { [weak self] limit in + self?.history.setLimit(limit) + } + } + + /// The log (`tools hub nav`): read back once, off the main thread, and put behind what this window did since it + /// opened; every later entry is written to it. Only the live hub connects: a scripted run and a test model + /// neither read nor write the person's log. + @MainActor + func connectNavLog() { + guard navStore == nil, !HubDefaults.isolated else { return } + let store = HubNavStore(limit: HubDefaults.preferences.navigationLimit) + navStore = store + navUnsaved = [] + HubNavStore.queue.async { [weak self] in + let span = HubPerf.begin("nav.load") + var older: [NavigationEntry] = [] + do { + older = try store.load() + span.end("\(older.count) entries") + } catch { + span.end("failed") + PerfLog.mark("hub.nav load failed: \(error)") + } + DispatchQueue.main.async { + MainActor.assumeIsolated { self?.adoptNavLog(older) } + } + } + } + + @MainActor + private func adoptNavLog(_ older: [NavigationEntry]) { + let merged = Set(history.adoptOlder(older)) + let unsaved = navUnsaved ?? [] + navUnsaved = nil + for id in unsaved where !merged.contains(id) { + if let entry = history.entry(id) { + writeNav(entry) + } + } + } + + /// Takes the place on screen into the history: a place with nothing picked is provisional, a place an agent or a + /// link asked for carries who and why, anything else is the person's. + @MainActor + private func recordPlace() { + let place = navEntry + HubPlace.record(place, tab: tab) + guard !navRestoring else { return } + if let linked = prLinkedCommit, place.mode == .prs, place.selection != linked.prID { + prLinkedCommit = nil + } + guard place.selection != nil else { + history.visit(navTarget(place), title: navTitle(place), provisional: true) + return + } + if let asked = pendingNav { + let age = Date().timeIntervalSince(asked.asked) + if navRequestMatches(asked) { + pendingNav = nil + if asked.pr != nil, let commit = asked.commit, place.mode == .prs, let id = place.selection { + prLinkedCommit = (id, commit) + } + let target = navTarget(place) + let title = navTitle(target) + if asked.restoring, let id = asked.id { + persistNav(history.retarget(id, to: target, title: title)) + } else { + persistNav(history.visit(target, title: title, actor: asked.actor, agentSession: asked.from, + note: asked.note, id: asked.id)) + } + return + } + if age > HubNavRequest.patience { + pendingNav = nil + HubPerf.log("nav.request expired: \(asked.title) never showed") + } else if age < HubNavRequest.settle { + // The hub's own steps toward the asked place (a mode switch, the old row still selected). + return + } + } + persistNav(history.visit(navTarget(place), title: navTitle(navTarget(place)))) + } + + /// The place on screen with the commit shown in it. + @MainActor + func navTarget(_ place: HubNavEntry) -> HubNavTarget { + switch place.mode { + case .prs: + if let linked = prLinkedCommit, linked.prID == place.selection { + return HubNavTarget(place: place, commit: linked.commit) + } + case .worktrees: + if let shown = shownCommit, shown.repo == place.selection { + return HubNavTarget(place: place, commit: shown.commit, repo: shown.repo) + } + default: + break + } + return HubNavTarget(place: place) + } + + /// Whether the hub now shows the place a request asked for. + @MainActor + func navRequestMatches(_ asked: HubNavRequest) -> Bool { + if let pr = asked.pr { + return mode == .prs && prs.selected.map(pr.matches) == true + } + if let commit = asked.commit { + let from = HubCommitSpan(commit)?.from ?? commit.lowercased() + return mode == .worktrees && shownCommit.map { $0.commit.lowercased().hasPrefix(from) && selectedWorktree == $0.repo } == true + } + if let worktree = asked.worktree { + return mode == .worktrees && selectedWorktree == WorktreeCleanup.selection(for: worktree) + } + if asked.agent != nil || asked.mode == .agents, asked.session != nil || asked.agent != nil { + return mode == .agents && agents.selectedID != nil + } + if let session = asked.session { + return mode == .sessions && (selectedID == session || selected?.sessionId.hasPrefix(session) == true) + } + if let wanted = asked.mode { + return mode == wanted + } + return false + } + + /// Writes a new or changed entry to the log, off the main thread, and one line to app-perf.log. + @MainActor + private func persistNav(_ entry: NavigationEntry?) { + guard let entry else { return } + HubPerf.log("nav \(entry.actor.rawValue) \(entry.title)\(entry.note.map { " (\($0))" } ?? "")") + writeNav(entry) + } + + @MainActor + private func writeNav(_ entry: NavigationEntry) { + guard let store = navStore else { return } + if navUnsaved != nil { + if navUnsaved?.contains(entry.id) == false { navUnsaved?.append(entry.id) } + return + } + let keep = history.limit + HubNavStore.queue.async { + do { + try store.record(entry, keep: keep) + } catch { + PerfLog.mark("hub.nav record failed: \(error)") + } } } @@ -774,55 +966,100 @@ final class HubModel: ObservableObject { @MainActor func goBack() { if let entry = history.goBack() { - restore(entry) + restore(entry, how: "back") } } @MainActor func goForward() { if let entry = history.goForward() { - restore(entry) + restore(entry, how: "forward") } } - /// Shows a place from the history with the same methods the rows use; the changes it makes are - /// not recorded again. + /// Any entry of the History list. @MainActor - private func restore(_ entry: HubNavEntry) { + func hop(to id: String) { + if let entry = history.hop(to: id) { + restore(entry, how: "hop") + } + } + + /// Shows a place from the history with the same methods the rows use; the changes it makes are not recorded + /// again. A place an agent named (link keys only) opens as its link would, and its entry takes the form of + /// the place the hub shows once it is there. + @MainActor + private func restore(_ entry: NavigationEntry, how: String) { navRestoring = true - HubPerf.log("nav.restore \(entry.mode.rawValue) \(entry.selection ?? "-")") + let target = entry.target + HubPerf.log("nav.\(how) \(entry.actor.rawValue) \(entry.title)") + if let place = target.place { + restorePlace(place, commit: target["commit"], repo: target["repo"]) + } else { + var asked = HubNavRequest(HubRequest(target.arguments)) + asked?.actor = entry.actor + asked?.id = entry.id + asked?.restoring = true + pendingNav = asked + navigate(HubRequest(target.arguments)) + } + // The recorder's turn for these changes is already queued, so it runs first and skips them. + DispatchQueue.main.async { [weak self] in self?.navRestoring = false } + } + + @MainActor + private func restorePlace(_ entry: HubNavEntry, commit: String?, repo: String?) { setMode(entry.mode) - if let id = entry.selection { - switch entry.mode { - case .sessions: - select(id) - case .worktrees: - if let worktree = worktrees.first(where: { $0.path == id }) { - selectWorktree(worktree) + guard let id = entry.selection else { return } + switch entry.mode { + case .sessions: + select(id) + case .worktrees: + if let worktree = worktrees.first(where: { $0.path == id }) { + selectWorktree(worktree) + } else { + selectedWorktree = id + shownCommit = nil + } + if let commit, let repo { + // Said at once, so the place on screen is the entry while the commit is looked up again. + shownCommit = HubShownCommit(repo: id, commit: commit) + openCommitLink(commit, repo: repo) + } + case .prs: + if let commit { + prLinkedCommit = (id, commit) + } + if let pr = prs.prs.first(where: { $0.id == id }) { + if let commit, let ref = HubPRRef(id) { + prs.request(ref, commit: commit) } else { - selectedWorktree = id - } - case .prs: - if let pr = prs.prs.first(where: { $0.id == id }) { prs.select(pr) - } else { - prs.showUnavailable(id) - post(prs.state == "all" - ? "That PR is not in the PR list any more." - : "That PR is not in the \(prs.state) list any more; pick All to find it.", .warning) - } - case .inbox: - inbox.selectedID = id == Self.wholeList ? nil : id - case .timeline: - break - case .agents: - if id != Self.wholeList, let key = AgentTree.split(id) { - agents.select(parent: key.parent, child: key.child) } + } else { + prs.showUnavailable(id) + post(prs.state == "all" + ? "That PR is not in the PR list any more." + : "That PR is not in the \(prs.state) list any more; pick All to find it.", .warning) + } + case .inbox: + inbox.selectedID = id == Self.wholeList ? nil : id + case .timeline: + break + case .agents: + if id != Self.wholeList, let key = AgentTree.split(id) { + agents.select(parent: key.parent, child: key.child) } } - // The recorder's turn for these changes is already queued, so it runs first and skips them. - DispatchQueue.main.async { [weak self] in self?.navRestoring = false } + } + + /// A place's title with the commit shown in it: "PR #563 Fix it · commit 0751d8a971". + @MainActor + func navTitle(_ target: HubNavTarget) -> String { + guard let place = target.place else { return HubNavRequest(HubRequest(target.arguments))?.title ?? "Hub" } + let name = navTitle(place) + guard let commit = target["commit"] else { return name } + return "\(name) · commit \(commit.split(separator: ".").filter { !$0.isEmpty }.map { String($0.prefix(10)) }.joined(separator: ".."))" } /// What a history entry shows, for the arrows' tooltips. @@ -980,6 +1217,7 @@ final class HubModel: ObservableObject { func selectWorktree(_ worktree: HubWorktree) { selectedWorktree = worktree.path + shownCommit = nil notice = nil if review?.home.path != worktree.path { let next = ReviewModel(repo: URL(fileURLWithPath: worktree.path), options: DiffViewOptions.remembered(), session: sessions(for: worktree).first?.sessionId) @@ -994,10 +1232,11 @@ final class HubModel: ObservableObject { /// `--repo` + `--commit` link. The checkout is selected before the mode opens, so a first walk of /// the worktrees includes it; a launch that has not settled yet opens the mode when it settles. @MainActor - func showCommit(repo: String, sha: String, title: String, base: String? = nil) { + func showCommit(repo: String, sha: String, title: String, base: String? = nil, link: String? = nil) { cancelSessionNavigation() commitLinkRequest = UUID() selectedWorktree = repo + shownCommit = HubShownCommit(repo: repo, commit: link ?? String(sha.prefix(12))) notice = nil if didSettleLaunch || initialMode != .worktrees { setMode(.worktrees) @@ -1565,8 +1804,33 @@ final class HubModel: ObservableObject { } } - /// A later `GenesisTools --hub …` handed to this hub: the same flags as a first launch. - func apply(_ request: HubRequest) { + /// A later `GenesisTools --hub …` handed to this hub: the same flags as a first launch. A place an agent or a + /// link asked for is recorded with who and why; while agents only add to History (Settings → Hub → History), + /// it goes into the list and the view stays. Returns false when the hub did not move, so the window stays back. + @discardableResult + func apply(_ request: HubRequest) -> Bool { + guard let asked = HubNavRequest(request) else { + navigate(request) + return true + } + if asked.actor != .user, MainActor.assumeIsolated({ HubDefaults.preferences.agentNavigation }) == .listOnly { + MainActor.assumeIsolated { + let entry = history.record(asked.target, title: asked.title, actor: asked.actor, agentSession: asked.from, + note: asked.note, id: asked.id) + persistNav(entry) + post("\(asked.actor == .agent ? "An agent" : "A link") added \(entry.title) to History (\(HubKeymap.label("hub.go.history")))", .success) + } + return false + } + pendingNav = asked + navigate(request) + // The same place again (another commit of the PR on screen) changes no selection: record it anyway. + DispatchQueue.main.async { [weak self] in self?.scheduleNavRecord() } + return true + } + + /// Shows the place a request names. + func navigate(_ request: HubRequest) { if request.session != nil || request.mode != nil || request.agent != nil || request.worktree != nil || request.pr != nil || request.commit != nil { commitLinkRequest = UUID() diff --git a/src/macos/GenesisTools/Tests/HubKeymapTests.swift b/src/macos/GenesisTools/Tests/HubKeymapTests.swift index af6a764efa..62b3d269ab 100644 --- a/src/macos/GenesisTools/Tests/HubKeymapTests.swift +++ b/src/macos/GenesisTools/Tests/HubKeymapTests.swift @@ -84,8 +84,10 @@ final class HubKeymapTests: XCTestCase { func seed() { var history = HubNavHistory() - history.visit(HubNavEntry(mode: .sessions, selection: "fixture-a")) - history.visit(HubNavEntry(mode: .sessions, selection: "fixture-b")) + for id in ["fixture-a", "fixture-b"] { + let target = HubNavTarget(place: HubNavEntry(mode: .sessions, selection: id)) + history.visit(target, title: id) + } model.replaceHistory(history) } func press(_ code: UInt16, _ character: String, _ flags: NSEvent.ModifierFlags = .command) -> Bool { @@ -98,11 +100,11 @@ final class HubKeymapTests: XCTestCase { for character in ["ú", "[", "x", ")"] { seed() XCTAssertTrue(press(33, character), "⌘ on key 33 types \(character)") - XCTAssertEqual(model.history.current?.selection, "fixture-a", character) + XCTAssertEqual(model.history.current?.target.place?.selection, "fixture-a", character) } seed() XCTAssertFalse(press(33, "ú", [.command, .option]), "another modifier set is not Back") - XCTAssertEqual(model.history.current?.selection, "fixture-b") + XCTAssertEqual(model.history.current?.target.place?.selection, "fixture-b") XCTAssertFalse(press(0, "a"), "⌘A is Select All, not a hub-window command") } diff --git a/src/macos/GenesisTools/Tests/HubNavigationTests.swift b/src/macos/GenesisTools/Tests/HubNavigationTests.swift index 4f11dc806b..9e1e996d09 100644 --- a/src/macos/GenesisTools/Tests/HubNavigationTests.swift +++ b/src/macos/GenesisTools/Tests/HubNavigationTests.swift @@ -163,75 +163,63 @@ final class HubNavigationTests: XCTestCase { } } - private func place(_ mode: HubMode, _ selection: String?) -> HubNavEntry { - HubNavEntry(mode: mode, selection: selection) + private func place(_ mode: HubMode, _ selection: String?) -> HubNavTarget { + HubNavTarget(place: HubNavEntry(mode: mode, selection: selection)) } - // MARK: History - - func testBackAndForwardWalkThePlacesInOrder() { - var history = HubNavHistory() - history.visit(place(.sessions, "a")) - history.visit(place(.prs, "pr1")) - history.visit(place(.sessions, "b")) - XCTAssertTrue(history.canGoBack) - XCTAssertFalse(history.canGoForward) - - XCTAssertEqual(history.goBack(), place(.prs, "pr1")) - XCTAssertEqual(history.goBack(), place(.sessions, "a")) - XCTAssertNil(history.goBack(), "nothing before the first place") - XCTAssertEqual(history.current, place(.sessions, "a")) - - XCTAssertEqual(history.goForward(), place(.prs, "pr1")) - XCTAssertEqual(history.goForward(), place(.sessions, "b")) - XCTAssertNil(history.goForward()) - } - - func testANewPlaceAfterGoingBackDropsTheForwardOnes() { - var history = HubNavHistory() - history.visit(place(.sessions, "a")) - history.visit(place(.sessions, "b")) - _ = history.goBack() - history.visit(place(.worktrees, "/repo")) - XCTAssertFalse(history.canGoForward) - XCTAssertEqual(history.back, [place(.sessions, "a")]) + // MARK: History (the engine's own rules are GenesisKit's NavigationHistoryTests) + + func testAPlaceKeepsItsSelectionAndCommitAndALinkPlaceOpensAsItsFlags() throws { + let shown = HubNavTarget(place: HubNavEntry(mode: .worktrees, selection: "/src/app"), commit: "0751d8a971", repo: "/src/app") + XCTAssertEqual(shown.place, HubNavEntry(mode: .worktrees, selection: "/src/app")) + XCTAssertEqual(shown["commit"], "0751d8a971") + let data = try JSONEncoder().encode(shown) + XCTAssertEqual(try JSONSerialization.jsonObject(with: data) as? [String: String], + ["mode": "worktrees", "selection": "/src/app", "commit": "0751d8a971", "repo": "/src/app"], + "a flat object, the target of tools hub nav") + XCTAssertEqual(try JSONDecoder().decode(HubNavTarget.self, from: data), shown) + + let named = try JSONDecoder().decode(HubNavTarget.self, from: Data(##"{"pr":"#563","commit":"abcdef0","snapshot":"/tmp/x.png"}"##.utf8)) + XCTAssertNil(named.place, "an agent's place has no selection of the hub's own") + XCTAssertEqual(named.arguments, ["--pr", "#563", "--commit", "abcdef0"], "unknown keys never become flags") } - func testTheSamePlaceTwiceIsOneEntry() { - var history = HubNavHistory() - history.visit(place(.sessions, "a")) - history.visit(place(.sessions, "a")) - XCTAssertFalse(history.canGoBack) + /// The wire to `tools hub nav` (src/hub/commands/nav.ts): what `list --json` prints, and `record`'s flags. + func testTheStoreReadsTheListAndWritesFreeTextAsFlagEqualsValue() throws { + let listed = Data(##"{"entries":[{"id":"x2","at":1792949067131,"actor":"agent","title":"PR #563","target":{"pr":"563"},"note":"why","from":"s-1"},{"id":"x1","at":1792949066674,"actor":"robot","title":"s","target":{"mode":"sessions","selection":"claude:a"}}],"keep":50,"path":"/x/navigation.db"}"##.utf8) + let entries = try HubNavStore.decode(listed) + XCTAssertEqual(entries.map(\.id), ["x2", "x1"]) + XCTAssertEqual(entries[0].actor, .agent) + XCTAssertEqual(entries[0].note, "why") + XCTAssertEqual(entries[0].agentSession, "s-1") + XCTAssertEqual(entries[0].at.timeIntervalSince1970, 1_792_949_067.131, accuracy: 0.001) + XCTAssertEqual(entries[1].actor, .user, "an unknown actor reads as the person's") + XCTAssertEqual(entries[1].target.place, HubNavEntry(mode: .sessions, selection: "claude:a")) + + let args = try HubNavStore.recordArguments( + NavigationEntry(id: "e", at: Date(timeIntervalSince1970: 2), target: place(.prs, "p"), title: "-x title", + actor: .agent, agentSession: "s", note: "--why"), keep: 20) + XCTAssertEqual(Array(args.prefix(3)), ["hub", "nav", "record"]) + XCTAssertTrue(args.contains("--title=-x title")) + XCTAssertTrue(args.contains("--note=--why")) + XCTAssertTrue(args.contains("--at=2000")) + XCTAssertTrue(args.contains("--keep=20")) + XCTAssertTrue(args.contains("--from=s")) } - func testAPlaceWithNothingPickedIsReplacedByWhatFollows() { - var history = HubNavHistory() - history.visit(place(.sessions, nil)) - history.visit(place(.prs, nil)) - XCTAssertFalse(history.canGoBack, "the launch's empty Sessions mode is no place to go back to") - - history.visit(place(.prs, "pr1")) - history.visit(place(.worktrees, nil)) - history.visit(place(.sessions, "a")) - XCTAssertEqual(history.back, [place(.prs, "pr1")], "a mode left while its list loaded is not a stop") - - // Out to a loading mode and straight back is the place before it, not a second copy of it. - history.visit(place(.worktrees, nil)) - history.visit(place(.sessions, "a")) - XCTAssertEqual(history.back, [place(.prs, "pr1")]) - XCTAssertEqual(history.current, place(.sessions, "a")) - - _ = history.goBack() - XCTAssertEqual(history.forward, [place(.sessions, "a")]) - } - - func testTheHistoryKeepsOnlyTheNewestPlaces() { - var history = HubNavHistory() - for index in 0...(HubNavHistory.limit + 20) { - history.visit(place(.sessions, "s\(index)")) - } - XCTAssertEqual(history.back.count, HubNavHistory.limit) - XCTAssertEqual(history.back.first, place(.sessions, "s20")) + func testARequestCarriesWhoAndWhyAndALinkWithoutAnActorIsALink() throws { + let request = HubRequest(["--pr", "acme/app#563", "--commit", "0751d8a971", "--actor", "agent", "--note", "why", + "--from", "session-1", "--nav", "entry-1"]) + let asked = try XCTUnwrap(HubNavRequest(request)) + XCTAssertEqual(asked.actor, .agent) + XCTAssertEqual(asked.note, "why") + XCTAssertEqual(asked.from, "session-1") + XCTAssertEqual(asked.id, "entry-1") + XCTAssertEqual(asked.title, "PR acme/app#563 · commit 0751d8a971") + XCTAssertEqual(HubNavRequest(HubRequest(["--session", "abc"]))?.actor, .link) + XCTAssertNil(HubNavRequest(HubRequest(["--palette", "gt pr"])), "an overlay is no place") + XCTAssertEqual(HubLink.arguments(fromLink: "genesis-tools://hub?pr=1&actor=agent¬e=a%20b&from=s&nav=n&menu=Go"), + ["--pr", "1", "--actor", "agent", "--note", "a b", "--from", "s", "--nav", "n"]) } func testTheModelRecordsEachPickAndWalksBackAndForward() { @@ -241,13 +229,15 @@ final class HubNavigationTests: XCTestCase { XCTAssertFalse(model.history.canGoBack, "the launch's empty selection is not a place") model.selectedID = "claude:two" drainMainQueue() - XCTAssertEqual(model.history.back, [place(.sessions, "claude:one")]) + XCTAssertEqual(model.history.backEntry?.target, place(.sessions, "claude:one")) + XCTAssertEqual(model.history.current?.actor, .user) MainActor.assumeIsolated { model.goBack() } drainMainQueue() XCTAssertEqual(model.selectedID, "claude:one") - XCTAssertEqual(model.history.forward, [place(.sessions, "claude:two")]) - XCTAssertEqual(model.history.current, place(.sessions, "claude:one"), "the restore itself is not recorded") + XCTAssertEqual(model.history.forwardEntry?.target, place(.sessions, "claude:two")) + XCTAssertEqual(model.history.current?.target, place(.sessions, "claude:one"), "the restore itself is not recorded") + XCTAssertEqual(model.history.entries.count, 2) MainActor.assumeIsolated { model.goForward() } drainMainQueue() @@ -255,6 +245,86 @@ final class HubNavigationTests: XCTestCase { XCTAssertFalse(model.history.canGoForward) } + /// Martin, 2026-10-25: what an agent opened replaced the place he was on, and he could not get back. + @MainActor + func testAnAgentsPlaceIsAddedWithItsNoteAndBackReturns() async throws { + let a = HubSession(sessionId: "fixture-a"), b = HubSession(sessionId: "fixture-b"), c = HubSession(sessionId: "fixture-c") + let model = isolatedModel(wanted: a.sessionId) + let initial = expectation(description: "initial list") + model.readSessions = { _ in [a, b, c] } + model.writeSessionCache = { _, _ in initial.fulfill() } + model.loadSessions() + await fulfillment(of: [initial], timeout: 2) + await drain() + model.select(b.id) + await drain() + XCTAssertEqual(model.history.current?.target, place(.sessions, b.id)) + + XCTAssertTrue(model.apply(HubRequest(["--session", "fixture-c", "--actor", "agent", "--note", "the failing run", + "--from", "agent-session", "--nav", "cli-id"]))) + await drain() + XCTAssertEqual(model.selectedID, c.id) + let current = try XCTUnwrap(model.history.current) + XCTAssertEqual(current.id, "cli-id", "the entry tools hub nav open stored, not a second one") + XCTAssertEqual(current.target, place(.sessions, c.id)) + XCTAssertEqual(current.actor, .agent) + XCTAssertEqual(current.note, "the failing run") + XCTAssertEqual(current.agentSession, "agent-session") + XCTAssertEqual(model.history.entries.map(\.target), [place(.sessions, a.id), place(.sessions, b.id), place(.sessions, c.id)], + "nothing was replaced") + + model.goBack() + await drain() + XCTAssertEqual(model.selectedID, b.id) + // The same place again from an agent: its note, no new entry. + XCTAssertTrue(model.apply(HubRequest(["--session", "fixture-b", "--actor", "agent", "--note", "look again"]))) + await drain() + XCTAssertEqual(model.history.current?.note, "look again") + XCTAssertEqual(model.history.entries.count, 3) + } + + private func drain() async { + for _ in 0..<3 { + await withCheckedContinuation { continuation in DispatchQueue.main.async { continuation.resume() } } + } + } + + func testWhileAgentsOnlyAddToHistoryTheViewStays() throws { + let preferences = MainActor.assumeIsolated { HubDefaults.preferences } + MainActor.assumeIsolated { preferences.agentNavigation = .listOnly } + defer { MainActor.assumeIsolated { preferences.agentNavigation = .navigate } } + let model = HubModel(wantedSession: nil, tab: .transcript) + model.selectedID = "claude:one" + drainMainQueue() + let shown = model.apply(HubRequest(["--pr", "acme/app#9", "--actor", "agent", "--note", "later", "--nav", "cli-id"])) + drainMainQueue() + XCTAssertFalse(shown) + XCTAssertEqual(model.mode, .sessions) + XCTAssertEqual(model.selectedID, "claude:one") + XCTAssertEqual(model.history.current?.target, place(.sessions, "claude:one")) + let added = try XCTUnwrap(model.history.entry("cli-id")) + XCTAssertEqual(added.note, "later") + XCTAssertEqual(added.target["pr"], "acme/app#9") + XCTAssertNil(model.history.offset(of: "cli-id"), "in the list, not on the back or forward stack") + // A person's own request still navigates. + XCTAssertTrue(model.apply(HubRequest(["--mode", "prs", "--actor", "user"]))) + XCTAssertEqual(model.mode, .prs) + } + + func testAHopShowsAnyPlaceAndBackReturns() throws { + let model = HubModel(wantedSession: nil, tab: .transcript) + for id in ["claude:one", "claude:two", "claude:three"] { + model.selectedID = id + drainMainQueue() + } + let first = try XCTUnwrap(model.history.entries.first) + MainActor.assumeIsolated { model.hop(to: first.id) } + drainMainQueue() + XCTAssertEqual(model.selectedID, "claude:one") + XCTAssertEqual(model.history.forwardEntry?.target, place(.sessions, "claude:two")) + XCTAssertEqual(model.history.entries.count, 3) + } + // Martin, 2026-10-06: Agents at parent X, child Y; a banner opens PR 123; back must land on X / Y again. func testBackFromAPrOpenedByABannerReturnsToTheAgentChild() { let model = HubModel(wantedSession: nil, tab: .transcript) @@ -269,14 +339,14 @@ final class HubNavigationTests: XCTestCase { model.prs.selectedID = "github.com/acme/app#123" } drainMainQueue() - XCTAssertEqual(model.history.back.last, place(.agents, child)) + XCTAssertEqual(model.history.backEntry?.target, place(.agents, child)) MainActor.assumeIsolated { model.goBack() } drainMainQueue() XCTAssertEqual(model.mode, .agents) let selected = MainActor.assumeIsolated { model.agents.selectedID } XCTAssertEqual(selected, child) - XCTAssertEqual(model.history.forward, [place(.prs, "github.com/acme/app#123")]) + XCTAssertEqual(model.history.forwardEntry?.target, place(.prs, "github.com/acme/app#123")) } private func drainMainQueue() { From a137da26fc58a46d7e1a970258ba80f3cce30246 Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 19:04:37 +0100 Subject: [PATCH 07/11] refactor(GenesisHub, hub): the hub keeps its rows in one database, hub/data.db, where each feature adds its migrations; the navigation log moves in from navigation.db once (cherry picked from commit 3fcf4f4eed10232885d8cac12bb1723dea22a262) --- src/hub/README.md | 2 +- src/hub/commands/nav.ts | 13 ++-- src/hub/lib/data-db.ts | 40 +++++++++++++ src/hub/lib/navigation/navigation.test.ts | 29 +++++++-- src/hub/lib/navigation/schema.ts | 60 +++++++++++++++++++ src/hub/lib/navigation/store.ts | 48 +-------------- src/hub/lib/navigation/types.ts | 2 +- .../Settings/HubSettingsPages.swift | 2 +- .../Sources/Parts/Hub/Hub/HubNavigation.swift | 2 +- 9 files changed, 137 insertions(+), 61 deletions(-) create mode 100644 src/hub/lib/data-db.ts create mode 100644 src/hub/lib/navigation/schema.ts diff --git a/src/hub/README.md b/src/hub/README.md index 1ebe9b0c0e..895c3f310c 100644 --- a/src/hub/README.md +++ b/src/hub/README.md @@ -40,7 +40,7 @@ The hub keeps a log of every place it showed, with who took it there: `user` (a tools hub nav open 'genesis-tools://hub?pr=owner%2Frepo%2342&commit=4e7a3bc' --note "why" tools hub nav list [--json] [--limit 20] -`nav open` takes a hub link, a PR ref or page, a commit sha (with `--pr` or `--repo`), a checkout folder, or the `open` flags. It records the entry (`~/.genesis-tools/hub/navigation.db`), then opens the hub on it. `--actor` defaults to `agent` inside an agent session (its id goes in as `--from`), else `link`. Settings → Hub → History sets how many places are kept (200) and whether an agent's place is shown at once (the default) or only added to History. Every entry writes one `hub nav:` line to the day log. +`nav open` takes a hub link, a PR ref or page, a commit sha (with `--pr` or `--repo`), a checkout folder, or the `open` flags. It records the entry in the hub's own database (`~/.genesis-tools/hub/data.db`, one file for every hub feature: a new one adds its migrations to `src/hub/lib/data-db.ts`), then opens the hub on it. `--actor` defaults to `agent` inside an agent session (its id goes in as `--from`), else `link`. Settings → Hub → History sets how many places are kept (200) and whether an agent's place is shown at once (the default) or only added to History. Every entry writes one `hub nav:` line to the day log. ## Links diff --git a/src/hub/commands/nav.ts b/src/hub/commands/nav.ts index 04b24b5146..1d0e16d081 100644 --- a/src/hub/commands/nav.ts +++ b/src/hub/commands/nav.ts @@ -11,7 +11,8 @@ import { logger, out } from "@genesiscz/utils/logger"; import { createBoxTable, renderCliHeader, truncateDisplay } from "@genesiscz/utils/table"; import { type Command, InvalidArgumentError } from "commander"; import pc from "picocolors"; -import { listNav, navDbPath, navKeep, openNavDb, recordNav } from "../lib/navigation/store"; +import { hubDbPath, openHubDb } from "../lib/data-db"; +import { listNav, navKeep, recordNav } from "../lib/navigation/store"; import { navTargetProblem, navTitle, parseNavInput } from "../lib/navigation/target"; import { isNavActor, @@ -175,7 +176,7 @@ async function navOpen(input: string | undefined, flags: NavOpenFlags): Promise< entry.from = from; } - const db = openNavDb(); + const db = openHubDb(); try { recordNav(db, entry); @@ -249,7 +250,7 @@ function navRecord(flags: NavRecordFlags): void { entry.from = flags.from; } - const db = openNavDb(); + const db = openHubDb(); try { const result = recordNav(db, entry, flags.keep); @@ -264,17 +265,17 @@ function navRecord(flags: NavRecordFlags): void { } function navList(flags: { json?: boolean; limit?: number }): void { - const db = openNavDb(); + const db = openHubDb(); try { const entries = listNav(db, flags.limit); if (flags.json) { - out.result({ entries, keep: navKeep(db), path: navDbPath() }); + out.result({ entries, keep: navKeep(db), path: hubDbPath() }); return; } - renderCliHeader("Hub history", `${entries.length} places, newest first · ${navDbPath()}`); + renderCliHeader("Hub history", `${entries.length} places, newest first · ${hubDbPath()}`); const table = createBoxTable(["WHEN", "WHO", "PLACE", "NOTE"]); const tint = { user: pc.dim, agent: pc.magenta, link: pc.cyan } as const; diff --git a/src/hub/lib/data-db.ts b/src/hub/lib/data-db.ts new file mode 100644 index 0000000000..217f4ae6b6 --- /dev/null +++ b/src/hub/lib/data-db.ts @@ -0,0 +1,40 @@ +import { Database } from "bun:sqlite"; +import { mkdirSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { type Migration, runMigrations } from "@genesiscz/utils/database/migrations"; +import { logger } from "@genesiscz/utils/logger"; +import { Storage } from "@genesiscz/utils/storage"; +import { importLegacyNavDb, NAV_MIGRATIONS } from "./navigation/schema"; + +const { log } = logger.scoped("hub:data"); + +/** + * Every hub feature that keeps rows: one scope each, all in one file. A new feature adds its migrations here instead + * of opening another SQLite file (Martin, 2026-10-25: "no need to create sqlite db for every thing"). + */ +const HUB_SCHEMAS: ReadonlyArray<{ scope: string; migrations: Migration[] }> = [ + { scope: "hub_nav", migrations: NAV_MIGRATIONS }, +]; + +/** `~/.genesis-tools/hub/data.db`: the hub's own data, one file per machine, shared by every hub and agent. */ +export function hubDbPath(): string { + return join(new Storage("hub").getBaseDir(), "data.db"); +} + +/** Opens data.db (or `:memory:` in a test) with every hub schema applied. */ +export function openHubDb(dbPath: string = hubDbPath()): Database { + if (dbPath !== ":memory:") { + mkdirSync(dirname(dbPath), { recursive: true }); + } + + const db = new Database(dbPath); + db.exec("PRAGMA journal_mode = WAL;"); + db.exec("PRAGMA busy_timeout = 2000;"); + for (const { scope, migrations } of HUB_SCHEMAS) { + runMigrations(db, migrations, { tableName: scope }); + } + + importLegacyNavDb(db); + log.debug({ dbPath }, "hub data: opened"); + return db; +} diff --git a/src/hub/lib/navigation/navigation.test.ts b/src/hub/lib/navigation/navigation.test.ts index e29ef17385..afb0b14af8 100644 --- a/src/hub/lib/navigation/navigation.test.ts +++ b/src/hub/lib/navigation/navigation.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "bun:test"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { openHubDb } from "../data-db"; import { hubArgs, hubUrl } from "../open"; -import { listNav, navKeep, openNavDb, recordNav } from "./store"; +import { listNav, navKeep, recordNav } from "./store"; import { navTargetProblem, navTitle, parseNavInput } from "./target"; import { type NavEntry, normalizeTarget } from "./types"; @@ -10,7 +14,7 @@ function entry(id: string, at: number, extra: Partial = {}): NavEntry describe("the navigation log", () => { it("lists newest first, replaces by id and keeps the agent's note and session", () => { - const db = openNavDb(":memory:"); + const db = openHubDb(":memory:"); recordNav(db, entry("a", 1)); recordNav( db, @@ -30,7 +34,7 @@ describe("the navigation log", () => { }); it("keeps the newest entries, and a later writer without a cap uses the one the hub stored", () => { - const db = openNavDb(":memory:"); + const db = openHubDb(":memory:"); expect(navKeep(db)).toBe(200); for (let index = 0; index < 5; index++) { recordNav(db, entry(`p${index}`, index), 3); @@ -46,7 +50,7 @@ describe("the navigation log", () => { it("drops unknown target keys and empty values, and reads an unknown actor as user", () => { expect(normalizeTarget({ pr: "1", snapshot: "/tmp/x.png", commit: " ", tab: 3 })).toEqual({ pr: "1" }); - const db = openNavDb(":memory:"); + const db = openHubDb(":memory:"); recordNav(db, entry("x", 1)); db.run(`UPDATE nav_entries SET actor = 'robot', target = '{"pr":"1","menu":"Go/Back"}'`); expect(listNav(db)[0]).toMatchObject({ actor: "user", target: { pr: "1" } }); @@ -120,3 +124,20 @@ describe("what tools hub nav open names", () => { ); }); }); + +describe("hub data.db", () => { + it("the first open copies an old navigation.db in and renames it aside", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-hub-data-")); + try { + const old = openHubDb(join(dir, "navigation.db")); + recordNav(old, { id: "n1", at: 1, actor: "agent", title: "PR #1", target: { pr: "1" }, note: "why" }); + old.close(); + const db = openHubDb(join(dir, "data.db")); + expect(listNav(db).map((entry) => entry.id)).toEqual(["n1"]); + expect(existsSync(join(dir, "navigation.db"))).toBe(false); + db.close(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/hub/lib/navigation/schema.ts b/src/hub/lib/navigation/schema.ts new file mode 100644 index 0000000000..9493be1671 --- /dev/null +++ b/src/hub/lib/navigation/schema.ts @@ -0,0 +1,60 @@ +import type { Database } from "bun:sqlite"; +import { existsSync, renameSync } from "node:fs"; +import { dirname, join } from "node:path"; +import type { Migration } from "@genesiscz/utils/database/migrations"; +import { logger } from "@genesiscz/utils/logger"; + +const { log } = logger.scoped("hub:nav"); + +/** The navigation log's tables in the hub's data.db (`openHubDb`). */ +export const NAV_MIGRATIONS: Migration[] = [ + { + id: "001-hub-nav-entries", + description: + "one row per place the hub showed: when, who (user, agent, link), the place, its title and the note", + apply: (db) => { + db.exec(`CREATE TABLE IF NOT EXISTS nav_entries ( + id TEXT PRIMARY KEY, + at INTEGER NOT NULL, + actor TEXT NOT NULL, + title TEXT NOT NULL, + target TEXT NOT NULL, + note TEXT, + from_session TEXT + );`); + db.exec("CREATE INDEX IF NOT EXISTS idx_nav_entries_at ON nav_entries(at);"); + db.exec("CREATE TABLE IF NOT EXISTS nav_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);"); + }, + }, +]; + +/** + * The log's first home was hub/navigation.db (2026-10-25): copied into data.db once, then renamed aside. Outside the + * migrations because SQLite refuses ATTACH inside their transaction; INSERT OR IGNORE makes a second process harmless. + */ +export function importLegacyNavDb(db: Database): void { + if (!db.filename || db.filename === ":memory:") { + return; + } + + const legacy = join(dirname(db.filename), "navigation.db"); + if (legacy === db.filename || !existsSync(legacy)) { + return; + } + + try { + db.exec(`ATTACH DATABASE '${legacy.replaceAll("'", "''")}' AS legacy`); + try { + db.exec("INSERT OR IGNORE INTO nav_entries SELECT * FROM legacy.nav_entries"); + db.exec("INSERT OR IGNORE INTO nav_meta SELECT * FROM legacy.nav_meta"); + } finally { + db.exec("DETACH DATABASE legacy"); + } + + const aside = `${legacy}.migrated-${new Date().toISOString().slice(0, 10)}`; + renameSync(legacy, aside); + log.info({ from: legacy, aside }, "hub nav: moved the log into data.db"); + } catch (error) { + log.warn({ error, legacy }, "hub nav: could not move the old log into data.db; it stays where it is"); + } +} diff --git a/src/hub/lib/navigation/store.ts b/src/hub/lib/navigation/store.ts index 0deba6e362..64cb4e11ba 100644 --- a/src/hub/lib/navigation/store.ts +++ b/src/hub/lib/navigation/store.ts @@ -1,53 +1,7 @@ -import { Database } from "bun:sqlite"; -import { mkdirSync } from "node:fs"; -import { dirname, join } from "node:path"; -import { type Migration, runMigrations } from "@genesiscz/utils/database/migrations"; +import type { Database } from "bun:sqlite"; import { SafeJSON } from "@genesiscz/utils/json"; -import { logger } from "@genesiscz/utils/logger"; -import { Storage } from "@genesiscz/utils/storage"; import { isNavActor, NAV_DEFAULT_KEEP, type NavEntry, normalizeTarget } from "./types"; -const { log } = logger.scoped("hub:nav"); - -export const NAV_MIGRATIONS: Migration[] = [ - { - id: "001-hub-nav-entries", - description: - "one row per place the hub showed: when, who (user, agent, link), the place, its title and the note", - apply: (db) => { - db.exec(`CREATE TABLE IF NOT EXISTS nav_entries ( - id TEXT PRIMARY KEY, - at INTEGER NOT NULL, - actor TEXT NOT NULL, - title TEXT NOT NULL, - target TEXT NOT NULL, - note TEXT, - from_session TEXT - );`); - db.exec("CREATE INDEX IF NOT EXISTS idx_nav_entries_at ON nav_entries(at);"); - db.exec("CREATE TABLE IF NOT EXISTS nav_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);"); - }, - }, -]; - -/** `~/.genesis-tools/hub/navigation.db`: one log per machine, shared by every hub and agent. */ -export function navDbPath(): string { - return join(new Storage("hub").getBaseDir(), "navigation.db"); -} - -export function openNavDb(dbPath: string = navDbPath()): Database { - if (dbPath !== ":memory:") { - mkdirSync(dirname(dbPath), { recursive: true }); - } - - const db = new Database(dbPath); - db.exec("PRAGMA journal_mode = WAL;"); - db.exec("PRAGMA busy_timeout = 2000;"); - runMigrations(db, NAV_MIGRATIONS, { tableName: "hub_nav" }); - log.debug({ dbPath }, "hub nav: opened the log"); - return db; -} - interface NavRow { id: string; at: number; diff --git a/src/hub/lib/navigation/types.ts b/src/hub/lib/navigation/types.ts index 4a005ebdb2..f7d16b6a7e 100644 --- a/src/hub/lib/navigation/types.ts +++ b/src/hub/lib/navigation/types.ts @@ -1,6 +1,6 @@ /** * The hub's navigation history: every place the hub showed, who took it there and why. The log lives here, in - * `~/.genesis-tools/hub/navigation.db`; GenesisTools.app reads and writes it through `tools hub nav`, and its + * the `nav_entries` table of `~/.genesis-tools/hub/data.db` (`openHubDb`); GenesisTools.app reads and writes it through `tools hub nav`, and its * GenesisKit engine (`NavigationHistory`) walks it with Back, Forward and a hop. */ diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift b/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift index 396c5e67ab..9b8f1fd35d 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Settings/HubSettingsPages.swift @@ -140,7 +140,7 @@ private struct HubHistorySettings: View { } NativeSettingsCard("History") { NativeSettingsRow( - "Keep the last", detail: "Older places leave History, Back and Forward. The log is ~/.genesis-tools/hub/navigation.db." + "Keep the last", detail: "Older places leave History, Back and Forward. The log is in ~/.genesis-tools/hub/data.db." ) { HStack(spacing: 8) { NativeSettingsNumberPicker( diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift index 6ef9cc5aca..9d6b213501 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift @@ -3,7 +3,7 @@ import SwiftUI // Back and forward through what the hub showed, and the log of it: every mode switch, session, worktree, PR and // commit, who took the hub there (a click here, an agent, a link) and why. The engine is GenesisKit's -// `NavigationHistory`; the log lives in `tools hub nav` (`~/.genesis-tools/hub/navigation.db`), so an agent's +// `NavigationHistory`; the log lives in `tools hub nav` (`~/.genesis-tools/hub/data.db`), so an agent's // `tools hub nav open` and the hub write one list. ⌘[ and ⌘], the arrows above the mode switch, the mouse's side // buttons and a trackpad swipe walk it (`HubModel.goBack`); ⌘Y and the clock button list it (`HubHistoryButton`). From 0c3816d130aa3e4024da61c75d03440d450ff05f Mon Sep 17 00:00:00 2001 From: Martin Date: Sun, 25 Oct 2026 23:35:22 +0100 Subject: [PATCH 08/11] fix(hub): the hub nav help names its tools command through toolCommand, so check-tool-commands passes (the nav.ts part of 8b3d0f7132) --- src/hub/commands/nav.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hub/commands/nav.ts b/src/hub/commands/nav.ts index 1d0e16d081..727bc288e4 100644 --- a/src/hub/commands/nav.ts +++ b/src/hub/commands/nav.ts @@ -310,7 +310,7 @@ export function registerNavCommands(program: Command): void { nav.command("open") .description( - "Open the hub on a place AND record who asked and why. Agents: use this instead of tools hub open, so the person can go Back" + `Open the hub on a place AND record who asked and why. Agents: use this instead of ${toolCommand("hub open")}, so the person can go Back` ) .argument( "[link-or-target]", From 691410b6eee9511c79ff9073e8393cd21941054c Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 26 Oct 2026 01:40:07 +0100 Subject: [PATCH 09/11] fix(GenesisKit, GenesisHub, hub): #586 followup - restore navigation targets and shared history --- src/hub/commands/nav.ts | 18 ++++- src/hub/lib/navigation/navigation.test.ts | 34 +++++++- src/hub/lib/navigation/store.ts | 36 +++++++-- src/hub/lib/navigation/target.ts | 4 +- .../Navigation/NavigationHistory.swift | 5 +- .../NavigationHistoryTests.swift | 9 ++- .../Sources/Parts/Hub/Hub/HubAgents.swift | 9 +++ .../Sources/Parts/Hub/Hub/HubNavigation.swift | 15 ++++ .../Sources/Parts/Hub/Hub/HubWindow.swift | 24 ++++-- .../Tests/HubNavigationTests.swift | 79 +++++++++++++++++++ 10 files changed, 210 insertions(+), 23 deletions(-) diff --git a/src/hub/commands/nav.ts b/src/hub/commands/nav.ts index 727bc288e4..c40f3712ee 100644 --- a/src/hub/commands/nav.ts +++ b/src/hub/commands/nav.ts @@ -12,7 +12,7 @@ import { createBoxTable, renderCliHeader, truncateDisplay } from "@genesiscz/uti import { type Command, InvalidArgumentError } from "commander"; import pc from "picocolors"; import { hubDbPath, openHubDb } from "../lib/data-db"; -import { listNav, navKeep, recordNav } from "../lib/navigation/store"; +import { listNav, navKeep, recordNav, setNavKeep } from "../lib/navigation/store"; import { navTargetProblem, navTitle, parseNavInput } from "../lib/navigation/target"; import { isNavActor, @@ -221,6 +221,7 @@ interface NavRecordFlags { note?: string; from?: string; keep?: number; + replacing?: string[]; json?: boolean; } @@ -253,7 +254,7 @@ function navRecord(flags: NavRecordFlags): void { const db = openHubDb(); try { - const result = recordNav(db, entry, flags.keep); + const result = recordNav(db, entry, { keep: flags.keep, replacing: flags.replacing }); logNavigation(entry, "hub"); if (flags.json) { @@ -333,9 +334,22 @@ export function registerNavCommands(program: Command): void { .option("--no-build", "never build; fail when GenesisTools.app is missing or stale") .action(async (input: string | undefined, flags: NavOpenFlags) => navOpen(input, flags)); + nav.command("configure") + .description("Persist the History limit and prune the log immediately") + .requiredOption("--keep ", "keep the newest n entries", positiveInt) + .action((flags: { keep: number }) => { + const db = openHubDb(); + try { + setNavKeep(db, flags.keep); + } finally { + db.close(); + } + }); + nav.command("record") .description("Store one entry (GenesisTools.app's door: every place the hub shows)") .requiredOption("--id ", "the entry id; an existing one is replaced") + .option("--replacing ", "remove these superseded entries in the same transaction") .requiredOption("--actor ", NAV_ACTORS.join("|")) .requiredOption("--title ", "what the place is") .requiredOption("--target ", 'the place as the hub\'s link query, JSON: {"pr":"…","commit":"…"}') diff --git a/src/hub/lib/navigation/navigation.test.ts b/src/hub/lib/navigation/navigation.test.ts index afb0b14af8..797077a1da 100644 --- a/src/hub/lib/navigation/navigation.test.ts +++ b/src/hub/lib/navigation/navigation.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from "bun:test"; import { existsSync, mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import { openHubDb } from "../data-db"; import { hubArgs, hubUrl } from "../open"; -import { listNav, navKeep, recordNav } from "./store"; +import { listNav, navKeep, recordNav, setNavKeep } from "./store"; import { navTargetProblem, navTitle, parseNavInput } from "./target"; import { type NavEntry, normalizeTarget } from "./types"; @@ -37,7 +37,7 @@ describe("the navigation log", () => { const db = openHubDb(":memory:"); expect(navKeep(db)).toBe(200); for (let index = 0; index < 5; index++) { - recordNav(db, entry(`p${index}`, index), 3); + recordNav(db, entry(`p${index}`, index), { keep: 3 }); } expect(listNav(db).map((row) => row.id)).toEqual(["p4", "p3", "p2"]); @@ -48,6 +48,31 @@ describe("the navigation log", () => { db.close(); }); + it("reconciles an annotated current place with the CLI row before reloading", () => { + const db = openHubDb(":memory:"); + recordNav(db, entry("older", 1)); + recordNav(db, entry("shown", 2)); + recordNav(db, entry("cli", 3, { target: { session: "shown" } })); + recordNav(db, entry("cli", 4, { target: { session: "shown" }, note: "look here" }), { replacing: ["shown"] }); + expect(listNav(db).map((row) => row.id)).toEqual(["cli", "older"]); + expect(listNav(db)[0].note).toBe("look here"); + db.close(); + }); + + it("prunes on a preference change before any new visit and later writers use the cap", () => { + const db = openHubDb(":memory:"); + for (let index = 0; index < 5; index++) { + recordNav(db, entry(`p${index}`, index)); + } + + expect(setNavKeep(db, 2)).toEqual({ kept: 2, dropped: 3 }); + expect(navKeep(db)).toBe(2); + expect(listNav(db).map((row) => row.id)).toEqual(["p4", "p3"]); + recordNav(db, entry("next", 10)); + expect(listNav(db).map((row) => row.id)).toEqual(["next", "p4"]); + db.close(); + }); + it("drops unknown target keys and empty values, and reads an unknown actor as user", () => { expect(normalizeTarget({ pr: "1", snapshot: "/tmp/x.png", commit: " ", tab: 3 })).toEqual({ pr: "1" }); const db = openHubDb(":memory:"); @@ -88,6 +113,9 @@ describe("what tools hub nav open names", () => { }); expect(parseNavInput("0751d8a971", noFolder)).toEqual({ target: { commit: "0751d8a971" } }); expect(parseNavInput("/src/app", (path) => path === "/src/app")).toEqual({ target: { worktree: "/src/app" } }); + expect(parseNavInput("./checkout", (path) => path === "./checkout")).toEqual({ + target: { worktree: resolve("./checkout") }, + }); expect("error" in parseNavInput("not a place", noFolder)).toBe(true); expect("error" in parseNavInput("https://example.org/other?pr=1", noFolder)).toBe(true); }); diff --git a/src/hub/lib/navigation/store.ts b/src/hub/lib/navigation/store.ts index 64cb4e11ba..0029b7e0fd 100644 --- a/src/hub/lib/navigation/store.ts +++ b/src/hub/lib/navigation/store.ts @@ -39,16 +39,43 @@ export function navKeep(db: Database): number { return Number.isInteger(keep) && keep > 0 ? keep : NAV_DEFAULT_KEEP; } +/** Persists a changed History setting and prunes immediately, even without a new visit. */ +export function setNavKeep(db: Database, keep: number): { kept: number; dropped: number } { + return db.transaction(() => { + db.run("INSERT OR REPLACE INTO nav_meta (key, value) VALUES ('keep', ?)", [String(keep)]); + return pruneNav(db, keep); + })(); +} + +function pruneNav(db: Database, keep: number): { kept: number; dropped: number } { + const dropped = db.run( + "DELETE FROM nav_entries WHERE id NOT IN (SELECT id FROM nav_entries ORDER BY at DESC, rowid DESC LIMIT ?)", + [keep] + ).changes; + return { kept: keep, dropped }; +} + /** * Adds the entry, or replaces the row with its id, then keeps the newest `keep` rows. A `keep` given here is stored * as the cap for later writers that do not know it (an agent's `tools hub nav open`). */ -export function recordNav(db: Database, entry: NavEntry, keep?: number): { kept: number; dropped: number } { +export function recordNav( + db: Database, + entry: NavEntry, + options: { keep?: number; replacing?: string[] } = {} +): { kept: number; dropped: number } { + const { keep, replacing } = options; const write = db.transaction(() => { if (keep !== undefined) { db.run("INSERT OR REPLACE INTO nav_meta (key, value) VALUES ('keep', ?)", [String(keep)]); } + for (const id of replacing ?? []) { + if (id !== entry.id) { + db.run("DELETE FROM nav_entries WHERE id = ?", [id]); + } + } + db.run( `INSERT OR REPLACE INTO nav_entries (id, at, actor, title, target, note, from_session) VALUES (?, ?, ?, ?, ?, ?, ?)`, @@ -62,12 +89,7 @@ export function recordNav(db: Database, entry: NavEntry, keep?: number): { kept: entry.from ?? null, ] ); - const cap = keep ?? navKeep(db); - const dropped = db.run( - "DELETE FROM nav_entries WHERE id NOT IN (SELECT id FROM nav_entries ORDER BY at DESC, rowid DESC LIMIT ?)", - [cap] - ).changes; - return { kept: cap, dropped }; + return pruneNav(db, keep ?? navKeep(db)); }); return write(); } diff --git a/src/hub/lib/navigation/target.ts b/src/hub/lib/navigation/target.ts index 4233b78431..1fdd5a4e0c 100644 --- a/src/hub/lib/navigation/target.ts +++ b/src/hub/lib/navigation/target.ts @@ -1,4 +1,4 @@ -import { basename } from "node:path"; +import { basename, resolve } from "node:path"; import { isHubCommitSha, isHubPrRef } from "../pr-ref"; import { type NavTarget, normalizeTarget } from "./types"; @@ -61,7 +61,7 @@ export function parseNavInput(input: string, isFolder: (path: string) => boolean } if (isFolder(text)) { - return { target: { worktree: text } }; + return { target: { worktree: resolve(text) } }; } return { diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift index b847fcdb9e..52aa86b786 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift @@ -34,6 +34,8 @@ public struct NavigationEntry: Identifiable, Codable public var agentSession: String? /// Why the agent opened it ("the commit that broke the build"). public var note: String? + /// When annotations adopt external ids, the store removes these old rows in the same write. + public var replacedIDs: [String]? public init( id: String = UUID().uuidString, at: Date = Date(), target: Target, title: String, @@ -57,7 +59,7 @@ public protocol NavigationHistoryStore: AnyObject { associatedtype Target: Codable & Hashable /// Every kept entry, in any order. func load() throws -> [NavigationEntry] - /// Adds the entry, or replaces the one with its id, and keeps at most `keep` entries (the newest). + /// Adds or replaces by id, removes `entry.replacedIDs` transactionally, and keeps the newest `keep` entries. func record(_ entry: NavigationEntry, keep: Int) throws } @@ -143,6 +145,7 @@ public struct NavigationHistory: Equatable { if !title.isEmpty { annotated.title = title } if let id, id != currentID { // The id the agent's CLI already stored, so the store keeps one row for it. + annotated.replacedIDs = (annotated.replacedIDs ?? []) + [currentID] annotated.id = id self.currentID = id } diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift index 07bf7ea8ef..6780b37e1f 100644 --- a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift @@ -221,7 +221,7 @@ final class NavigationHistoryTests: XCTestCase { var rows: [NavigationEntry] = [] func load() throws -> [NavigationEntry] { rows } func record(_ entry: NavigationEntry, keep: Int) throws { - rows.removeAll { $0.id == entry.id } + rows.removeAll { $0.id == entry.id || entry.replacedIDs?.contains($0.id) == true } rows.append(entry) rows = Array(rows.sorted { $0.at < $1.at }.suffix(keep)) } @@ -234,12 +234,17 @@ final class NavigationHistoryTests: XCTestCase { try store.record(made, keep: history.limit) } } - if let annotated = history.visit("c", title: "c", actor: .agent, note: "n", at: start.addingTimeInterval(9)) { + let originalID = try XCTUnwrap(history.currentID) + try store.record(NavigationEntry(id: "cli-id", at: start.addingTimeInterval(8), target: "c", title: "c"), keep: 200) + if let annotated = history.visit("c", title: "c", actor: .agent, note: "n", id: "cli-id", at: start.addingTimeInterval(9)) { + XCTAssertEqual(annotated.replacedIDs, [originalID]) try store.record(annotated, keep: history.limit) } + XCTAssertEqual(store.rows.count, 2, "the original id and the CLI id become one persisted stop") let reread = History(entries: try store.load(), limit: 2) XCTAssertEqual(reread.entries.map(\.target), ["b", "c"]) XCTAssertEqual(reread.current?.note, "n") + XCTAssertEqual(reread.currentID, "cli-id") } } diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubAgents.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubAgents.swift index 913d08f9e9..47111f785d 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubAgents.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubAgents.swift @@ -944,6 +944,15 @@ final class HubAgentsModel: ObservableObject { resolvePendingRequest(final: false) } + func matchesRequest(parent: String?, child: String) -> Bool { + if child == AgentTree.mainChild { + guard let wanted = parents.first(where: { parent.map($0.sessionId.hasPrefix) ?? false }) else { return false } + return selectedID == AgentTree.mainKey(wanted.sessionId) + } + guard let wanted = Self.requested(child: child, parent: parent, in: index) else { return false } + return selectedID == wanted.key + } + private func resolvePendingRequest(final: Bool) { guard let wanted = pendingRequest, loaded || !parents.isEmpty else { return } // `--session

` alone: the lead session's Main row. diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift index 9d6b213501..d675997640 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubNavigation.swift @@ -87,6 +87,8 @@ struct HubNavRequest { var agent: String? var worktree: String? var mode: HubMode? + var reveal: String? + var tab: HubTab? /// A history entry being shown again: its own entry takes the shown place's form instead of a new entry. var restoring = false let asked: Date @@ -107,6 +109,8 @@ struct HubNavRequest { agent = request.agent worktree = request.worktree mode = request.mode + reveal = request.reveal + tab = request.tab asked = now } @@ -126,6 +130,8 @@ struct HubNavRequest { fields["commit"] = commit fields["repo"] = repo fields["worktree"] = worktree + fields["reveal"] = reveal + fields["tab"] = tab?.rawValue return HubNavTarget(fields) } @@ -187,6 +193,14 @@ final class HubNavStore: NavigationHistoryStore, Sendable { _ = try ToolsCLIRunner.run(try Self.recordArguments(entry, keep: keep)) } + func setLimit(_ keep: Int) throws { + _ = try ToolsCLIRunner.run(Self.limitArguments(keep)) + } + + static func limitArguments(_ keep: Int) -> [String] { + ["hub", "nav", "configure", "--keep=\(keep)"] + } + static func recordArguments(_ entry: NavigationEntry, keep: Int) throws -> [String] { let target = String(decoding: try JSONEncoder().encode(entry.target), as: UTF8.self) // Free text rides as `--flag=value`, so a title or a note that starts with "-" is not read as a flag. @@ -196,6 +210,7 @@ final class HubNavStore: NavigationHistoryStore, Sendable { ] if let note = entry.note { args.append("--note=\(note)") } if let from = entry.agentSession { args.append("--from=\(from)") } + if let replacedIDs = entry.replacedIDs, !replacedIDs.isEmpty { args += ["--replacing"] + replacedIDs } return args } } diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift index a7a5e2abe7..4144bf6d01 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift @@ -792,6 +792,15 @@ final class HubModel: ObservableObject { history.setLimit(preferences.navigationLimit) navLimitWatch = preferences.$navigationLimit.dropFirst().sink { [weak self] limit in self?.history.setLimit(limit) + self?.persistNavLimit(limit) + } + } + + private func persistNavLimit(_ limit: Int) { + guard let store = navStore else { return } + HubNavStore.queue.async { + do { try store.setLimit(limit) } + catch { PerfLog.mark("hub.nav configure failed: \(error)") } } } @@ -803,6 +812,7 @@ final class HubModel: ObservableObject { guard navStore == nil, !HubDefaults.isolated else { return } let store = HubNavStore(limit: HubDefaults.preferences.navigationLimit) navStore = store + persistNavLimit(history.limit) navUnsaved = [] HubNavStore.queue.async { [weak self] in let span = HubPerf.begin("nav.load") @@ -906,7 +916,7 @@ final class HubModel: ObservableObject { return mode == .worktrees && selectedWorktree == WorktreeCleanup.selection(for: worktree) } if asked.agent != nil || asked.mode == .agents, asked.session != nil || asked.agent != nil { - return mode == .agents && agents.selectedID != nil + return mode == .agents && agents.matchesRequest(parent: asked.session, child: asked.agent ?? AgentTree.mainChild) } if let session = asked.session { return mode == .sessions && (selectedID == session || selected?.sessionId.hasPrefix(session) == true) @@ -991,6 +1001,7 @@ final class HubModel: ObservableObject { @MainActor private func restore(_ entry: NavigationEntry, how: String) { navRestoring = true + pendingNav = nil let target = entry.target HubPerf.log("nav.\(how) \(entry.actor.rawValue) \(entry.title)") if let place = target.place { @@ -1027,14 +1038,13 @@ final class HubModel: ObservableObject { openCommitLink(commit, repo: repo) } case .prs: - if let commit { - prLinkedCommit = (id, commit) - } + prLinkedCommit = commit.map { (id, $0) } if let pr = prs.prs.first(where: { $0.id == id }) { if let commit, let ref = HubPRRef(id) { prs.request(ref, commit: commit) } else { - prs.select(pr) + if prs.selectedID != pr.id { prs.select(pr) } + prs.showWholePR() } } else { prs.showUnavailable(id) @@ -1047,7 +1057,9 @@ final class HubModel: ObservableObject { case .timeline: break case .agents: - if id != Self.wholeList, let key = AgentTree.split(id) { + if id == Self.wholeList { + agents.selectedID = nil + } else if let key = AgentTree.split(id) { agents.select(parent: key.parent, child: key.child) } } diff --git a/src/macos/GenesisTools/Tests/HubNavigationTests.swift b/src/macos/GenesisTools/Tests/HubNavigationTests.swift index 9e1e996d09..98373577b7 100644 --- a/src/macos/GenesisTools/Tests/HubNavigationTests.swift +++ b/src/macos/GenesisTools/Tests/HubNavigationTests.swift @@ -205,6 +205,11 @@ final class HubNavigationTests: XCTestCase { XCTAssertTrue(args.contains("--at=2000")) XCTAssertTrue(args.contains("--keep=20")) XCTAssertTrue(args.contains("--from=s")) + var annotated = NavigationEntry(target: place(.prs, "p"), title: "p") + annotated.replacedIDs = ["old-id", "cli-1"] + let replacement = try HubNavStore.recordArguments(annotated, keep: 20) + XCTAssertEqual(Array(replacement.suffix(3)), ["--replacing", "old-id", "cli-1"]) + XCTAssertEqual(HubNavStore.limitArguments(20), ["hub", "nav", "configure", "--keep=20"]) } func testARequestCarriesWhoAndWhyAndALinkWithoutAnActorIsALink() throws { @@ -311,6 +316,80 @@ final class HubNavigationTests: XCTestCase { XCTAssertEqual(model.mode, .prs) } + @MainActor + func testListOnlyKeepsRevealAndTabThroughAHop() async throws { + let preferences = HubDefaults.preferences + preferences.agentNavigation = .listOnly + defer { preferences.agentNavigation = .navigate } + let model = isolatedModel() + let request = HubRequest(["--pr", "acme/app#9", "--reveal", "Sources/File.swift", "--tab", "changes", + "--actor", "agent", "--nav", "cli-reveal"]) + XCTAssertFalse(model.apply(request)) + let entry = try XCTUnwrap(model.history.entry("cli-reveal")) + XCTAssertEqual(entry.target["reveal"], "Sources/File.swift") + XCTAssertEqual(entry.target["tab"], "changes") + model.hop(to: entry.id) + XCTAssertEqual(model.tab, .changes) + XCTAssertEqual(model.prs.pendingReveal?.path, "Sources/File.swift") + await drain() + let session = try XCTUnwrap(HubNavRequest(HubRequest(["--session", "fixture", "--tab", "changes"]))) + XCTAssertEqual(HubRequest(session.target.arguments).tab, .changes) + } + + @MainActor + func testAnUnresolvedAgentRequestDoesNotMatchThePreviouslySelectedAgent() throws { + let model = isolatedModel() + model.mode = .agents + model.agents.select(parent: "old-parent", child: "old-agent") + let asked = try XCTUnwrap(HubNavRequest(HubRequest(["--session", "new-parent", "--agent", "new-agent"]))) + XCTAssertFalse(model.navRequestMatches(asked)) + let main = try XCTUnwrap(HubNavRequest(HubRequest(["--mode", "agents", "--session", "new-parent"]))) + XCTAssertFalse(model.navRequestMatches(main)) + } + + @MainActor + func testBackToTheWholePRClearsTheLinkedCommitAndResetsTheScope() async throws { + let json = #"{"prs":[{"repo":"app","number":9,"title":"fixture","state":"OPEN","draft":false,"headBranch":"feature","baseBranch":"main","url":"https://github.com/acme/app/pull/9","labels":[],"reviewers":[]}],"repos":[]}"# + let list = try JSONDecoder().decode(HubPRList.self, from: Data(json.utf8)) + let pr = try XCTUnwrap(list.prs.first) + let model = isolatedModel() + model.mode = .prs + model.prs.apply(list, key: "fixture", flash: false) + model.pendingNav = HubNavRequest(HubRequest(["--pr", pr.url, "--commit", "abcdef0"])) + model.prs.selectedID = pr.id + await drain() + let review = ReviewModel(repo: URL(fileURLWithPath: "/fixture/navigation"), options: DiffViewOptions()) + review.scope = .commit(sha: "abcdef0", title: "linked") + model.prs.review = review + var history = HubNavHistory() + history.visit(place(.prs, pr.id), title: "whole PR") + history.visit(HubNavTarget(place: HubNavEntry(mode: .prs, selection: pr.id), commit: "abcdef0"), title: "commit") + model.replaceHistory(history) + model.goBack() + XCTAssertNil(model.navTarget(model.navEntry)["commit"]) + XCTAssertEqual(review.scope, PRsModel.scope(pr, detail: nil)) + XCTAssertNil(model.prs.pendingCommit) + await drain() + } + + @MainActor + func testBackCancelsAnUnfulfilledRequestAndRestoresTheAgentsListWithoutAChild() async throws { + let model = isolatedModel() + model.mode = .agents + model.agents.select(parent: "parent", child: "child") + await drain() + var history = HubNavHistory() + history.visit(place(.agents, HubModel.wholeList), title: "Agents") + history.visit(place(.agents, AgentTree.key(parent: "parent", child: "child")), title: "child") + model.replaceHistory(history) + model.pendingNav = HubNavRequest(HubRequest(["--session", "missing", "--agent", "missing"])) + model.goBack() + XCTAssertNil(model.pendingNav, "Back supersedes an older external request") + XCTAssertNil(model.agents.selectedID, "the list entry has no child selected") + await drain() + XCTAssertEqual(model.history.current?.target, place(.agents, HubModel.wholeList)) + } + func testAHopShowsAnyPlaceAndBackReturns() throws { let model = HubModel(wantedSession: nil, tab: .transcript) for id in ["claude:one", "claude:two", "claude:three"] { From 04f9943d11468a23a1e23546f80f530f95fecc30 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 26 Oct 2026 03:35:44 +0100 Subject: [PATCH 10/11] fix(GenesisKit, GenesisHub, hub): #586 followup - navigation history and the nav command follow the round-2 review --- src/hub/commands/nav.ts | 46 +++++++--------- src/hub/lib/navigation/navigation.test.ts | 55 ++++++++++++++++++- src/hub/lib/navigation/store.ts | 26 +++++++++ .../Navigation/NavigationHistory.swift | 8 ++- .../NavigationHistoryTests.swift | 38 +++++++++++++ 5 files changed, 143 insertions(+), 30 deletions(-) diff --git a/src/hub/commands/nav.ts b/src/hub/commands/nav.ts index c40f3712ee..81208a7ec7 100644 --- a/src/hub/commands/nav.ts +++ b/src/hub/commands/nav.ts @@ -12,7 +12,7 @@ import { createBoxTable, renderCliHeader, truncateDisplay } from "@genesiscz/uti import { type Command, InvalidArgumentError } from "commander"; import pc from "picocolors"; import { hubDbPath, openHubDb } from "../lib/data-db"; -import { listNav, navKeep, recordNav, setNavKeep } from "../lib/navigation/store"; +import { readNav, recordNav, setNavKeep } from "../lib/navigation/store"; import { navTargetProblem, navTitle, parseNavInput } from "../lib/navigation/target"; import { isNavActor, @@ -266,34 +266,28 @@ function navRecord(flags: NavRecordFlags): void { } function navList(flags: { json?: boolean; limit?: number }): void { - const db = openHubDb(); - - try { - const entries = listNav(db, flags.limit); - - if (flags.json) { - out.result({ entries, keep: navKeep(db), path: hubDbPath() }); - return; - } + const { entries, keep } = readNav({ limit: flags.limit }); - renderCliHeader("Hub history", `${entries.length} places, newest first · ${hubDbPath()}`); - const table = createBoxTable(["WHEN", "WHO", "PLACE", "NOTE"]); - const tint = { user: pc.dim, agent: pc.magenta, link: pc.cyan } as const; - - for (const entry of entries) { - table.push([ - formatRelativeTime(new Date(entry.at)), - tint[entry.actor](entry.actor), - truncateDisplay(entry.title, 60), - truncateDisplay(entry.note ?? "", 60), - ]); - } + if (flags.json) { + out.result({ entries, keep, path: hubDbPath() }); + return; + } - out.println(table.toString()); - out.println(pc.dim(`Open one: ${toolCommand("hub nav open")} --note ""`)); - } finally { - db.close(); + renderCliHeader("Hub history", `${entries.length} places, newest first · ${hubDbPath()}`); + const table = createBoxTable(["WHEN", "WHO", "PLACE", "NOTE"]); + const tint = { user: pc.dim, agent: pc.magenta, link: pc.cyan } as const; + + for (const entry of entries) { + table.push([ + formatRelativeTime(new Date(entry.at)), + tint[entry.actor](entry.actor), + truncateDisplay(entry.title, 60), + truncateDisplay(entry.note ?? "", 60), + ]); } + + out.println(table.toString()); + out.println(pc.dim(`Open one: ${toolCommand("hub nav open")} --note ""`)); } export function registerNavCommands(program: Command): void { diff --git a/src/hub/lib/navigation/navigation.test.ts b/src/hub/lib/navigation/navigation.test.ts index 797077a1da..c0ca4a366a 100644 --- a/src/hub/lib/navigation/navigation.test.ts +++ b/src/hub/lib/navigation/navigation.test.ts @@ -1,10 +1,12 @@ +import { Database } from "bun:sqlite"; import { describe, expect, it } from "bun:test"; -import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { clearDatabaseReadSnapshots } from "@genesiscz/utils/database/read-snapshot"; import { openHubDb } from "../data-db"; import { hubArgs, hubUrl } from "../open"; -import { listNav, navKeep, recordNav, setNavKeep } from "./store"; +import { listNav, navKeep, readNav, recordNav, setNavKeep } from "./store"; import { navTargetProblem, navTitle, parseNavInput } from "./target"; import { type NavEntry, normalizeTarget } from "./types"; @@ -154,6 +156,55 @@ describe("what tools hub nav open names", () => { }); describe("hub data.db", () => { + it("lists an absent store without creating directories or importing the legacy database", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-hub-read-")); + try { + const absent = join(dir, "missing", "data.db"); + expect(readNav({ dbPath: absent })).toEqual({ entries: [], keep: 200 }); + expect(existsSync(join(dir, "missing"))).toBe(false); + const legacy = join(dir, "navigation.db"); + const old = openHubDb(legacy); + recordNav(old, entry("legacy", 1)); + old.close(); + const before = readFileSync(legacy); + const names = readdirSync(dir).sort(); + expect(readNav({ dbPath: join(dir, "data.db") })).toEqual({ entries: [], keep: 200 }); + expect(readFileSync(legacy)).toEqual(before); + expect(readdirSync(dir).sort()).toEqual(names); + } finally { + clearDatabaseReadSnapshots(); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("reads initialized history and an unmigrated database without modifying either", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-hub-read-")); + try { + const dbPath = join(dir, "data.db"); + const db = openHubDb(dbPath); + recordNav(db, entry("a", 1), { keep: 20 }); + recordNav(db, entry("b", 2)); + db.close(); + const bytes = readFileSync(dbPath); + const names = readdirSync(dir).sort(); + expect(readNav({ dbPath, limit: 1 })).toEqual({ entries: [entry("b", 2)], keep: 20 }); + expect(readFileSync(dbPath)).toEqual(bytes); + expect(readdirSync(dir).sort()).toEqual(names); + const emptyPath = join(dir, "unmigrated.db"); + const unmigrated = new Database(emptyPath); + unmigrated.run("CREATE TABLE unrelated (id INTEGER)"); + unmigrated.close(); + const emptyBytes = readFileSync(emptyPath); + const emptyNames = readdirSync(dir).sort(); + expect(readNav({ dbPath: emptyPath })).toEqual({ entries: [], keep: 200 }); + expect(readFileSync(emptyPath)).toEqual(emptyBytes); + expect(readdirSync(dir).sort()).toEqual(emptyNames); + } finally { + clearDatabaseReadSnapshots(); + rmSync(dir, { recursive: true, force: true }); + } + }); + it("the first open copies an old navigation.db in and renames it aside", () => { const dir = mkdtempSync(join(tmpdir(), "gt-hub-data-")); try { diff --git a/src/hub/lib/navigation/store.ts b/src/hub/lib/navigation/store.ts index 0029b7e0fd..628118822f 100644 --- a/src/hub/lib/navigation/store.ts +++ b/src/hub/lib/navigation/store.ts @@ -1,5 +1,7 @@ import type { Database } from "bun:sqlite"; +import { withDatabaseReadSnapshot } from "@genesiscz/utils/database/read-snapshot"; import { SafeJSON } from "@genesiscz/utils/json"; +import { hubDbPath } from "../data-db"; import { isNavActor, NAV_DEFAULT_KEEP, type NavEntry, normalizeTarget } from "./types"; interface NavRow { @@ -32,6 +34,30 @@ function fromRow(row: NavRow): NavEntry { return entry; } +/** Reads a transient snapshot without creating, migrating or importing the durable store. */ +export function readNav({ dbPath = hubDbPath(), limit }: { dbPath?: string; limit?: number } = {}): { + entries: NavEntry[]; + keep: number; +} { + return withDatabaseReadSnapshot({ + path: dbPath, + initialize: () => undefined, + read: (db) => { + const tables = db + .query<{ count: number }, []>( + "SELECT COUNT(*) AS count FROM sqlite_master WHERE type = 'table' AND name IN ('nav_entries', 'nav_meta')" + ) + .get(); + + if (tables?.count !== 2) { + return { entries: [], keep: NAV_DEFAULT_KEEP }; + } + + return { entries: listNav(db, limit), keep: navKeep(db) }; + }, + }); +} + /** The cap the hub last set (its History setting), else the default. */ export function navKeep(db: Database): number { const row = db.query<{ value: string }, []>("SELECT value FROM nav_meta WHERE key = 'keep'").get(); diff --git a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift index 52aa86b786..06788b8167 100644 --- a/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift +++ b/src/macos/GenesisKit/Sources/GenesisKit/Navigation/NavigationHistory.swift @@ -144,12 +144,14 @@ public struct NavigationHistory: Equatable { annotated.at = at if !title.isEmpty { annotated.title = title } if let id, id != currentID { + backIDs.removeAll { $0 == id } + forwardIDs.removeAll { $0 == id } // The id the agent's CLI already stored, so the store keeps one row for it. annotated.replacedIDs = (annotated.replacedIDs ?? []) + [currentID] annotated.id = id self.currentID = id } - entries.remove(at: index) + entries.removeAll { $0.id == currentID || $0.id == annotated.id } entries.append(annotated) return annotated } @@ -172,7 +174,9 @@ public struct NavigationHistory: Equatable { } let made = Entry(id: id ?? UUID().uuidString, at: at, target: target, title: title, actor: actor, agentSession: agentSession, note: note) - if let currentID { + backIDs.removeAll { $0 == made.id } + forwardIDs.removeAll { $0 == made.id } + if let currentID, currentID != made.id { backIDs.append(currentID) } forwardIDs.removeAll() diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift index 6780b37e1f..4a88159fdf 100644 --- a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift @@ -110,6 +110,44 @@ final class NavigationHistoryTests: XCTestCase { XCTAssertEqual(history.backEntry?.target, "a") } + func testResolvingAnAdoptedCLIEntryNeverPutsItBehindItself() { + let start = Date(timeIntervalSince1970: 100) + for hasLaunchPlace in [false, true] { + var history = History() + if hasLaunchPlace { history.visit("launch", title: "launch", id: "launch", at: start.addingTimeInterval(2)) } + history.adoptOlder([ + NavigationEntry(id: "previous", at: start, target: "previous", title: "previous"), + NavigationEntry(id: "cli", at: start.addingTimeInterval(1), target: "raw-request", title: "requested", actor: .agent), + ]) + history.visit("resolved", title: "resolved", actor: .agent, note: "why", id: "cli", at: start.addingTimeInterval(3)) + XCTAssertEqual(history.currentID, "cli") + XCTAssertFalse(history.backIDs.contains("cli")) + XCTAssertFalse(history.forwardIDs.contains("cli")) + XCTAssertEqual(history.entries.filter { $0.id == "cli" }.count, 1) + if hasLaunchPlace { XCTAssertEqual(history.goBack()?.target, "launch") } + XCTAssertEqual(history.goBack()?.target, "previous") + XCTAssertNil(history.goBack()) + if hasLaunchPlace { XCTAssertEqual(history.goForward()?.target, "launch") } + XCTAssertEqual(history.goForward()?.target, "resolved") + XCTAssertNil(history.goForward()) + } + } + + func testAnnotatingWithAnAdoptedCLIIdRemovesItsOldRowAndStackReference() { + var history = History() + history.visit("shown", title: "shown", id: "shown") + history.adoptOlder([ + NavigationEntry(id: "previous", at: Date(timeIntervalSince1970: 1), target: "previous", title: "previous"), + NavigationEntry(id: "cli", at: Date(timeIntervalSince1970: 2), target: "raw-request", title: "requested"), + ]) + history.visit("shown", title: "shown", actor: .agent, id: "cli") + XCTAssertEqual(history.entries.filter { $0.id == "cli" }.count, 1) + XCTAssertFalse(history.backIDs.contains("cli")) + XCTAssertEqual(history.goBack()?.target, "previous") + XCTAssertEqual(history.goForward()?.target, "shown") + XCTAssertNil(history.goForward()) + } + func testAnAgentOpeningWhatIsShownAnnotatesItInsteadOfAddingACopy() { var history = walk(["a", "b"]) let annotated = history.visit("b", title: "", actor: .agent, note: "look here", id: "cli-id") From 0047241a96ccd51e9d79ae3748ac175fd8e213b1 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 26 Oct 2026 04:14:29 +0100 Subject: [PATCH 11/11] fix(GenesisHub, hub): #586 followup - the legacy navigation database is imported read-only and once, reserved cleanup destinations are kept, and a worktree pick after a linked commit restores its ordinary scope --- src/hub/commands/nav.ts | 4 +- src/hub/lib/data-db.ts | 7 +- src/hub/lib/navigation/navigation.test.ts | 83 ++++++++++++++++++- src/hub/lib/navigation/schema.ts | 42 ++++++++-- src/hub/lib/navigation/target.ts | 5 ++ .../NavigationHistoryTests.swift | 4 +- .../Sources/Parts/Hub/Hub/HubWindow.swift | 2 + .../Tests/HubNavigationTests.swift | 23 +++++ 8 files changed, 156 insertions(+), 14 deletions(-) diff --git a/src/hub/commands/nav.ts b/src/hub/commands/nav.ts index 81208a7ec7..09b4c6d013 100644 --- a/src/hub/commands/nav.ts +++ b/src/hub/commands/nav.ts @@ -13,7 +13,7 @@ import { type Command, InvalidArgumentError } from "commander"; import pc from "picocolors"; import { hubDbPath, openHubDb } from "../lib/data-db"; import { readNav, recordNav, setNavKeep } from "../lib/navigation/store"; -import { navTargetProblem, navTitle, parseNavInput } from "../lib/navigation/target"; +import { navTargetProblem, navTitle, parseNavInput, resolveNavWorktree } from "../lib/navigation/target"; import { isNavActor, NAV_ACTORS, @@ -95,7 +95,7 @@ function flagTarget(flags: NavOpenFlags): NavTarget { pr: flags.pr, commit: flags.commit, repo: flags.repo ? (/^\.{1,2}(?:[/\\]|$)/.test(flags.repo) ? resolve(flags.repo) : flags.repo) : undefined, - worktree: flags.worktree ? resolve(flags.worktree) : undefined, + worktree: flags.worktree ? resolveNavWorktree(flags.worktree) : undefined, session: flags.session, agent: flags.agent, mode: flags.mode, diff --git a/src/hub/lib/data-db.ts b/src/hub/lib/data-db.ts index 217f4ae6b6..914e9e18d4 100644 --- a/src/hub/lib/data-db.ts +++ b/src/hub/lib/data-db.ts @@ -1,4 +1,4 @@ -import { Database } from "bun:sqlite"; +import { constants, Database } from "bun:sqlite"; import { mkdirSync } from "node:fs"; import { dirname, join } from "node:path"; import { type Migration, runMigrations } from "@genesiscz/utils/database/migrations"; @@ -27,7 +27,10 @@ export function openHubDb(dbPath: string = hubDbPath()): Database { mkdirSync(dirname(dbPath), { recursive: true }); } - const db = new Database(dbPath); + const db = new Database( + dbPath, + constants.SQLITE_OPEN_READWRITE | constants.SQLITE_OPEN_CREATE | constants.SQLITE_OPEN_URI + ); db.exec("PRAGMA journal_mode = WAL;"); db.exec("PRAGMA busy_timeout = 2000;"); for (const { scope, migrations } of HUB_SCHEMAS) { diff --git a/src/hub/lib/navigation/navigation.test.ts b/src/hub/lib/navigation/navigation.test.ts index c0ca4a366a..7a8857afea 100644 --- a/src/hub/lib/navigation/navigation.test.ts +++ b/src/hub/lib/navigation/navigation.test.ts @@ -1,13 +1,15 @@ import { Database } from "bun:sqlite"; -import { describe, expect, it } from "bun:test"; -import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; +import { describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { clearDatabaseReadSnapshots } from "@genesiscz/utils/database/read-snapshot"; import { openHubDb } from "../data-db"; import { hubArgs, hubUrl } from "../open"; +import { importLegacyNavDb } from "./schema"; import { listNav, navKeep, readNav, recordNav, setNavKeep } from "./store"; -import { navTargetProblem, navTitle, parseNavInput } from "./target"; +import { navTargetProblem, navTitle, parseNavInput, resolveNavWorktree } from "./target"; import { type NavEntry, normalizeTarget } from "./types"; function entry(id: string, at: number, extra: Partial = {}): NavEntry { @@ -122,6 +124,17 @@ describe("what tools hub nav open names", () => { expect("error" in parseNavInput("https://example.org/other?pr=1", noFolder)).toBe(true); }); + it("keeps cleanup panel flags and resolves only checkout paths", () => { + expect(resolveNavWorktree("cleanup")).toBe("cleanup"); + expect(resolveNavWorktree("cleanup-blocked")).toBe("cleanup-blocked"); + expect(resolveNavWorktree("./checkout")).toBe(resolve("./checkout")); + expect(hubArgs({ worktree: resolveNavWorktree("cleanup-blocked") })).toEqual([ + "--hub", + "--worktree", + "cleanup-blocked", + ]); + }); + it("refuses what the hub could not open, and names the place in a few words", () => { expect(navTargetProblem({})).toContain("name a place"); expect(navTargetProblem({ commit: "0751d8a971" })).toContain("--pr"); @@ -205,6 +218,70 @@ describe("hub data.db", () => { } }); + it("does not create a legacy database that vanished after its existence check", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-hub-import-")); + const db = openHubDb(join(dir, "data.db")); + const legacy = join(dir, "navigation.db"); + const originalExists = fs.existsSync; + const exists = spyOn(fs, "existsSync").mockImplementation((path) => path === legacy || originalExists(path)); + try { + importLegacyNavDb(db); + } finally { + exists.mockRestore(); + db.close(); + } + + try { + expect(existsSync(legacy)).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("does not partially import a legacy database whose navigation schema is incomplete", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-hub-import-")); + try { + const legacy = join(dir, "navigation.db"); + const old = openHubDb(legacy); + recordNav(old, entry("legacy", 1)); + old.run("DROP TABLE nav_meta"); + old.close(); + const bytes = readFileSync(legacy); + const db = openHubDb(join(dir, "data.db")); + expect(listNav(db)).toEqual([]); + expect(db.query("SELECT 1 FROM nav_meta WHERE key = 'legacy-navigation-imported'").get()).toBeNull(); + db.close(); + expect(existsSync(legacy)).toBe(true); + expect(readFileSync(legacy)).toEqual(bytes); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("never resurrects pruned rows when the legacy rename fails after a committed import", () => { + const dir = mkdtempSync(join(tmpdir(), "gt-hub-import-")); + try { + const legacy = join(dir, "navigation.db"); + const old = openHubDb(legacy); + recordNav(old, entry("old", 1)); + recordNav(old, entry("new", 2)); + old.close(); + mkdirSync(`${legacy}.migrated-${new Date().toISOString().slice(0, 10)}`); + const dbPath = join(dir, "data.db"); + const db = openHubDb(dbPath); + expect(listNav(db).map((row) => row.id)).toEqual(["new", "old"]); + expect(existsSync(legacy)).toBe(true); + setNavKeep(db, 1); + db.close(); + const reopened = openHubDb(dbPath); + expect(listNav(reopened).map((row) => row.id)).toEqual(["new"]); + expect(navKeep(reopened)).toBe(1); + reopened.close(); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("the first open copies an old navigation.db in and renames it aside", () => { const dir = mkdtempSync(join(tmpdir(), "gt-hub-data-")); try { diff --git a/src/hub/lib/navigation/schema.ts b/src/hub/lib/navigation/schema.ts index 9493be1671..5f7c23c22d 100644 --- a/src/hub/lib/navigation/schema.ts +++ b/src/hub/lib/navigation/schema.ts @@ -1,6 +1,7 @@ import type { Database } from "bun:sqlite"; import { existsSync, renameSync } from "node:fs"; import { dirname, join } from "node:path"; +import { pathToFileURL } from "node:url"; import type { Migration } from "@genesiscz/utils/database/migrations"; import { logger } from "@genesiscz/utils/logger"; @@ -30,7 +31,8 @@ export const NAV_MIGRATIONS: Migration[] = [ /** * The log's first home was hub/navigation.db (2026-10-25): copied into data.db once, then renamed aside. Outside the - * migrations because SQLite refuses ATTACH inside their transaction; INSERT OR IGNORE makes a second process harmless. + * migrations so ATTACH and DETACH surround the import transaction. A committed marker prevents another import even + * when the final rename fails; concurrent importers recheck it under the write lock. */ export function importLegacyNavDb(db: Database): void { if (!db.filename || db.filename === ":memory:") { @@ -38,15 +40,45 @@ export function importLegacyNavDb(db: Database): void { } const legacy = join(dirname(db.filename), "navigation.db"); - if (legacy === db.filename || !existsSync(legacy)) { + if ( + legacy === db.filename || + db.query("SELECT 1 FROM nav_meta WHERE key = 'legacy-navigation-imported'").get() || + !existsSync(legacy) + ) { return; } try { - db.exec(`ATTACH DATABASE '${legacy.replaceAll("'", "''")}' AS legacy`); + const uri = `${pathToFileURL(legacy).href}?mode=ro`; + db.run("ATTACH DATABASE ? AS legacy", [uri]); try { - db.exec("INSERT OR IGNORE INTO nav_entries SELECT * FROM legacy.nav_entries"); - db.exec("INSERT OR IGNORE INTO nav_meta SELECT * FROM legacy.nav_meta"); + const tables = db + .query<{ count: number }, []>( + "SELECT COUNT(*) AS count FROM legacy.sqlite_master WHERE type = 'table' AND name IN ('nav_entries', 'nav_meta')" + ) + .get(); + + if (tables?.count !== 2) { + return; + } + + const imported = db.transaction(() => { + const claimed = db.run( + "INSERT OR IGNORE INTO nav_meta (key, value) VALUES ('legacy-navigation-imported', '1')" + ).changes; + + if (claimed === 0) { + return false; + } + + db.exec("INSERT OR IGNORE INTO nav_entries SELECT * FROM legacy.nav_entries"); + db.exec("INSERT OR IGNORE INTO nav_meta SELECT * FROM legacy.nav_meta"); + return true; + })(); + + if (!imported) { + return; + } } finally { db.exec("DETACH DATABASE legacy"); } diff --git a/src/hub/lib/navigation/target.ts b/src/hub/lib/navigation/target.ts index 1fdd5a4e0c..accdcf1387 100644 --- a/src/hub/lib/navigation/target.ts +++ b/src/hub/lib/navigation/target.ts @@ -69,6 +69,11 @@ export function parseNavInput(input: string, isFolder: (path: string) => boolean }; } +/** Reserved cleanup destinations are panel targets; other values are checkout paths. */ +export function resolveNavWorktree(worktree: string): string { + return worktree === "cleanup" || worktree === "cleanup-blocked" ? worktree : resolve(worktree); +} + /** What a place is, in a few words, until the hub names it better ("PR #563 · commit 0751d8a971"). */ export function navTitle(target: NavTarget): string { const commit = target.commit diff --git a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift index 4a88159fdf..2e4153a88f 100644 --- a/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift +++ b/src/macos/GenesisKit/Tests/GenesisKitTests/NavigationHistoryTests.swift @@ -34,7 +34,7 @@ final class NavigationHistoryTests: XCTestCase { XCTAssertEqual(history.entries.count, 3, "walking never adds entries") } - func testAVisitAfterGoingBackClearsForwardButTheLogKeepsEveryPlace() { + func testAVisitAfterGoingBackClearsForwardButTheLogKeepsEveryPlace() throws { var history = walk(["a", "b", "c"]) _ = history.goBack() _ = history.goBack() @@ -43,7 +43,7 @@ final class NavigationHistoryTests: XCTestCase { XCTAssertEqual(titles(history, history.backIDs), ["a"]) XCTAssertEqual(history.newestFirst.map(\.target), ["d", "c", "b", "a"], "b and c are still in the list") // A hop reaches a place that left the forward stack, and Back returns from it. - let c = try! XCTUnwrap(history.entries.first { $0.target == "c" }) + let c = try XCTUnwrap(history.entries.first { $0.target == "c" }) XCTAssertNil(history.offset(of: c.id), "only the log holds it") XCTAssertEqual(history.hop(to: c.id)?.target, "c") XCTAssertEqual(history.goBack()?.target, "d") diff --git a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift index 4144bf6d01..d76bb97ffe 100644 --- a/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift +++ b/src/macos/GenesisTools/Sources/Parts/Hub/Hub/HubWindow.swift @@ -1237,6 +1237,8 @@ final class HubModel: ObservableObject { next.scope = worktree.isMain ? .uncommitted : .branch next.worktreeChosen = true review = next + } else { + review?.setScope(worktree.isMain ? .uncommitted : .branch) } } diff --git a/src/macos/GenesisTools/Tests/HubNavigationTests.swift b/src/macos/GenesisTools/Tests/HubNavigationTests.swift index 98373577b7..bfe64a5b2c 100644 --- a/src/macos/GenesisTools/Tests/HubNavigationTests.swift +++ b/src/macos/GenesisTools/Tests/HubNavigationTests.swift @@ -1,3 +1,4 @@ +import Combine import XCTest @testable import GenesisToolsHost @testable import GenesisToolsHub @@ -372,6 +373,28 @@ final class HubNavigationTests: XCTestCase { await drain() } + @MainActor + func testBackFromALinkedWorktreeCommitRestoresTheOrdinaryScope() async throws { + for main in [false, true] { + let model = isolatedModel() + model.mode = .worktrees + model.loadingWorktrees = true + let worktree = HubWorktree(path: "/fixture/worktree-scope", repo: "app", commonDir: "/fixture/common", branch: "feature", isMain: main) + model.worktrees = [worktree] + model.selectWorktree(worktree) + await drain() + model.showCommit(repo: worktree.path, sha: "abcdef0123", title: "linked") + await drain() + XCTAssertEqual(model.review?.scope, .commit(sha: "abcdef0123", title: "linked")) + XCTAssertEqual(model.history.backEntry?.target, place(.worktrees, worktree.path)) + model.goBack() + await drain() + XCTAssertEqual(model.review?.scope, main ? .uncommitted : .branch) + XCTAssertNil(model.shownCommit) + XCTAssertNil(model.history.current?.target["commit"]) + } + } + @MainActor func testBackCancelsAnUnfulfilledRequestAndRestoresTheAgentsListWithoutAChild() async throws { let model = isolatedModel()