From 3f6f5473eeb286cd02ffeab476681a047c3ab665 Mon Sep 17 00:00:00 2001 From: StuBehan Date: Wed, 29 Jul 2026 19:01:25 +0100 Subject: [PATCH] fix(panel): refresh a stale hook script so handoffs record --- .release-please-config.json | 4 + README.md | 9 + .../BootstrapTests.swift | 170 ++++++++++++++++++ .../OutcomesViewTests.swift | 46 +++++ install.sh | 15 +- notify.sh | 44 ++++- panel/Bootstrap.swift | 139 +++++++++++++- panel/Handoff.swift | 39 ++++ panel/OutcomesView.swift | 69 ++++++- panel/Panel.swift | 36 +++- panel/PanelNav.swift | 28 ++- panel/Settings.swift | 20 +++ scripts/xctest-shim.swift | 12 ++ 13 files changed, 606 insertions(+), 25 deletions(-) create mode 100644 Tests/StackNudgePanelCoreTests/BootstrapTests.swift diff --git a/.release-please-config.json b/.release-please-config.json index f680e67..3bc8ae7 100644 --- a/.release-please-config.json +++ b/.release-please-config.json @@ -11,6 +11,10 @@ { "type": "generic", "path": "panel/Info.plist" + }, + { + "type": "generic", + "path": "notify.sh" } ] } diff --git a/README.md b/README.md index f7b0fd9..8a44141 100644 --- a/README.md +++ b/README.md @@ -233,6 +233,13 @@ Opt-in GitHub linking adds real **PR + CI status** (so even squash-merged work r | `STACKNUDGE_TICKET_URL` | Deep-link template for ticket rows, e.g. `https://linear.app/acme/issue/{key}` — `{key}` is replaced with the ticket | | `STACKNUDGE_HIDE_SHIPPED` | `true` to drop groups once their PR reads merged, keeping the tab on in-flight work | +**Nothing showing up?** A session is recorded when an agent's turn ends *inside a git repo*, and only if the hook payload carries a session id. The tab's empty state names which of those failed; `~/.stack-nudge/app.log` has a line per dropped turn. The usual cause is an installed hook script older than the app, since updates swap the `.app` alone: the app repairs that on launch, and Settings warns in the footer if the rewrite couldn't be applied. To fix it by hand: + +```bash +grep -c stack-nudge-version ~/.stack-nudge/notify.sh # 0 means the script predates v1.26 +./install.sh # or reinstall from the app +``` + ### Menu bar (macOS) When the panel daemon is running, a bell icon appears in your menu bar. The same items you can reach from the in-panel Settings tab are mirrored here for one-click access without summoning the panel: @@ -283,6 +290,8 @@ stack-nudge polls GitHub Releases on launch and every 6 hours. When a newer rele 5. Runs `launchctl kickstart -k` — the current process dies, launchd brings up the new bundle 6. The new bundle's first launch shows a welcome-style "Updated to vX.Y.Z" screen with the release notes +Because the swap replaces only the bundle, the hook script the agents invoke (`~/.stack-nudge/notify.sh`) would otherwise stay at whatever version first installed it. Each launch compares its `stack-nudge-version` stamp against the bundled script's and rewrites it when they differ, so payload fields added by a release reach the panel without a reinstall. + No source clone, no swiftc rebuild on the user's machine — the new bundle is the already-signed-and-notarized artifact from CI. Updates are fast and don't disturb the user's Xcode CLT or Python install (or lack thereof). While the StackOne stack-nudge repo is private the auto-updater falls back to your local `gh` CLI auth (`gh api`) to read the release metadata. Org members with `gh` configured see no friction; the actual artifact download uses the release's signed asset URL. diff --git a/Tests/StackNudgePanelCoreTests/BootstrapTests.swift b/Tests/StackNudgePanelCoreTests/BootstrapTests.swift new file mode 100644 index 0000000..d347d34 --- /dev/null +++ b/Tests/StackNudgePanelCoreTests/BootstrapTests.swift @@ -0,0 +1,170 @@ +import XCTest + +@testable import StackNudgePanelCore + +final class BootstrapTests: XCTestCase { + + // MARK: - notifyVersion(inScript:) + + func test_notifyVersion_readsTheStampFromTheHeader() { + let script = """ + #!/usr/bin/env bash + # stack-nudge: Cross-platform notifications for AI coding agent hooks + # stack-nudge-version: 1.26.0 # x-release-please-version + AGENT="${1:-agent}" + """ + XCTAssertEqual(Bootstrap.notifyVersion(inScript: script), "1.26.0") + } + + func test_notifyVersion_isNilForAScriptWithoutAStamp() { + let script = """ + #!/usr/bin/env bash + # stack-nudge: Cross-platform notifications for AI coding agent hooks + AGENT="${1:-agent}" + """ + XCTAssertNil(Bootstrap.notifyVersion(inScript: script)) + } + + func test_notifyVersion_ignoresAMalformedStamp() { + let script = "#!/usr/bin/env bash\n# stack-nudge-version: v1.26\n" + XCTAssertNil(Bootstrap.notifyVersion(inScript: script)) + } + + // The stamp is a header comment; a mention further down (a heredoc, a log + // line, a doc block) must not be mistaken for it. + func test_notifyVersion_ignoresMatchesBelowTheHeader() { + let filler = Array(repeating: "# padding", count: 60).joined(separator: "\n") + let script = "#!/usr/bin/env bash\n\(filler)\n# stack-nudge-version: 9.9.9\n" + XCTAssertNil(Bootstrap.notifyVersion(inScript: script)) + } + + func test_notifyVersion_toleratesNoSpaceAfterTheHash() { + XCTAssertEqual(Bootstrap.notifyVersion(inScript: "#stack-nudge-version:1.2.3"), "1.2.3") + } + + // MARK: - needsNotifyRefresh + + func test_needsNotifyRefresh_isTrueWhenTheInstalledScriptIsUnstamped() { + // Every install predating the stamp: the drift this repairs. + XCTAssertTrue(Bootstrap.needsNotifyRefresh(installed: nil, bundled: "1.26.0")) + } + + func test_needsNotifyRefresh_isTrueWhenTheVersionsDiffer() { + XCTAssertTrue(Bootstrap.needsNotifyRefresh(installed: "1.12.0", bundled: "1.26.0")) + } + + // Not just "older": a downgrade to an earlier bundle should also restore the + // script that bundle expects to be talking to. + func test_needsNotifyRefresh_isTrueWhenTheInstalledScriptIsNewer() { + XCTAssertTrue(Bootstrap.needsNotifyRefresh(installed: "1.30.0", bundled: "1.26.0")) + } + + func test_needsNotifyRefresh_isFalseWhenTheVersionsMatch() { + XCTAssertFalse(Bootstrap.needsNotifyRefresh(installed: "1.26.0", bundled: "1.26.0")) + } + + // An unstamped bundle is a local swiftc build, where the developer's own + // script is the one under test. Leave it alone rather than guess. + func test_needsNotifyRefresh_isFalseWhenTheBundleIsUnstamped() { + XCTAssertFalse(Bootstrap.needsNotifyRefresh(installed: "1.12.0", bundled: nil)) + XCTAssertFalse(Bootstrap.needsNotifyRefresh(installed: nil, bundled: nil)) + } + + // MARK: - writeNotifyScript + + func test_writeNotifyScript_writesExecutableContent() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + + try Bootstrap.writeNotifyScript("#!/usr/bin/env bash\necho new\n", to: path) + + XCTAssertEqual(try String(contentsOfFile: path, encoding: .utf8), + "#!/usr/bin/env bash\necho new\n") + XCTAssertTrue(FileManager.default.isExecutableFile(atPath: path)) + } + + func test_writeNotifyScript_replacesAnExistingScript() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + try "old".write(toFile: path, atomically: true, encoding: .utf8) + + try Bootstrap.writeNotifyScript("new", to: path) + + XCTAssertEqual(try String(contentsOfFile: path, encoding: .utf8), "new") + XCTAssertTrue(FileManager.default.isExecutableFile(atPath: path)) + } + + // The swap must never leave the destination absent, since a hook firing + // mid-write would otherwise find no script to run. + func test_writeNotifyScript_leavesNoTempFilesBehind() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + try "old".write(toFile: path, atomically: true, encoding: .utf8) + + try Bootstrap.writeNotifyScript("new", to: path) + + let entries = try FileManager.default.contentsOfDirectory(atPath: dir.path) + XCTAssertEqual(entries, ["notify.sh"]) + } + + func test_writeNotifyScript_throwsWhenTheDirectoryIsMissing() { + let path = "/nonexistent-\(UUID().uuidString)/notify.sh" + XCTAssertThrowsError(try Bootstrap.writeNotifyScript("new", to: path)) + } + + // MARK: - refreshNotifyScript + + private static let stamped = "#!/usr/bin/env bash\n# stack-nudge-version: 1.26.0\necho new\n" + + func test_refreshNotifyScript_replacesAnUnstampedInstall() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + try "#!/usr/bin/env bash\necho old\n".write(toFile: path, atomically: true, encoding: .utf8) + + let written = Bootstrap.refreshNotifyScript(bundled: Self.stamped, installedPath: path) + + XCTAssertEqual(written, "1.26.0") + XCTAssertEqual(try String(contentsOfFile: path, encoding: .utf8), Self.stamped) + XCTAssertTrue(FileManager.default.isExecutableFile(atPath: path)) + } + + func test_refreshNotifyScript_leavesAMatchingInstallUntouched() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + let installed = "#!/usr/bin/env bash\n# stack-nudge-version: 1.26.0\necho mine\n" + try installed.write(toFile: path, atomically: true, encoding: .utf8) + + XCTAssertNil(Bootstrap.refreshNotifyScript(bundled: Self.stamped, installedPath: path)) + // Same stamp means same protocol, so a locally tweaked script survives. + XCTAssertEqual(try String(contentsOfFile: path, encoding: .utf8), installed) + } + + func test_refreshNotifyScript_doesNothingWithoutABundledScript() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + try "old".write(toFile: path, atomically: true, encoding: .utf8) + + XCTAssertNil(Bootstrap.refreshNotifyScript(bundled: nil, installedPath: path)) + XCTAssertEqual(try String(contentsOfFile: path, encoding: .utf8), "old") + } + + // No installed script means no install to repair; writing one would leave a + // file nothing is wired to invoke. + func test_refreshNotifyScript_doesNotCreateAMissingScript() throws { + let dir = try temporaryDirectory() + let path = dir.appendingPathComponent("notify.sh").path + + XCTAssertNil(Bootstrap.refreshNotifyScript(bundled: Self.stamped, installedPath: path)) + XCTAssertFalse(FileManager.default.fileExists(atPath: path)) + } + + // MARK: - Helpers + + private func temporaryDirectory() throws -> URL { + let dir = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("bootstrap-tests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + addTeardownBlock { try? FileManager.default.removeItem(at: dir) } + return dir + } +} diff --git a/Tests/StackNudgePanelCoreTests/OutcomesViewTests.swift b/Tests/StackNudgePanelCoreTests/OutcomesViewTests.swift index 5ece159..9d33529 100644 --- a/Tests/StackNudgePanelCoreTests/OutcomesViewTests.swift +++ b/Tests/StackNudgePanelCoreTests/OutcomesViewTests.swift @@ -261,4 +261,50 @@ final class OutcomesViewTests: XCTestCase { func test_selectedRow_nilWhenNothingToSelect() { XCTAssertNil(OutcomesView.selectedRow([], index: 0)) } + + // MARK: - emptyReason + + func test_emptyReason_isNoSessionsOnAnUntouchedLedger() { + XCTAssertEqual(OutcomesView.emptyReason(totalGroups: 0, drops: [:]), .noSessions) + } + + func test_emptyReason_reportsFilteredGroupsBeforeDrops() { + // Groups exist but none are visible, so hide-shipped is the explanation + // even when unrelated drops have also been counted. + XCTAssertEqual( + OutcomesView.emptyReason(totalGroups: 3, drops: [.notAGitRepo: 2]), + .allShipped(hidden: 3)) + } + + func test_emptyReason_reportsTheDominantDropReasonAndTotalCount() { + let reason = OutcomesView.emptyReason( + totalGroups: 0, drops: [.notAGitRepo: 2, .missingSessionID: 7]) + XCTAssertEqual(reason, .allDropped(count: 9, reason: .missingSessionID)) + } + + func test_emptyReason_ignoresZeroedDropCounts() { + XCTAssertEqual(OutcomesView.emptyReason(totalGroups: 0, drops: [.missingSessionID: 0]), + .noSessions) + } + + // The actionable case names the remedy; the expected one (no repo) doesn't + // pretend something is broken. + func test_emptyReasonDetail_includesTheRemedyOnlyWhenThereIsOne() { + let broken = OutcomesView.EmptyReason.allDropped(count: 1, reason: .missingSessionID) + XCTAssertTrue(broken.detail.contains("notify.sh")) + let expected = OutcomesView.EmptyReason.allDropped(count: 1, reason: .notAGitRepo) + XCTAssertTrue(expected.detail.contains("git repo")) + XCTAssertFalse(expected.detail.contains("notify.sh")) + } + + func test_emptyReasonTitle_singularAndPlural() { + XCTAssertEqual(OutcomesView.EmptyReason.allDropped(count: 1, reason: .notAGitRepo).title, + "1 turn wasn't recorded") + XCTAssertEqual(OutcomesView.EmptyReason.allDropped(count: 4, reason: .notAGitRepo).title, + "4 turns weren't recorded") + XCTAssertEqual(OutcomesView.EmptyReason.allShipped(hidden: 1).title, + "1 shipped ticket hidden") + XCTAssertEqual(OutcomesView.EmptyReason.allShipped(hidden: 2).title, + "2 shipped tickets hidden") + } } diff --git a/install.sh b/install.sh index b5ce397..9d1eb52 100755 --- a/install.sh +++ b/install.sh @@ -221,10 +221,21 @@ fi # Copy notify.sh and the phrase pools (sourced by notify.sh at runtime # based on the configured voice's language) to the shared install dir. -cp "$SCRIPT_DIR/notify.sh" "$INSTALL_DIR/notify.sh" -chmod +x "$INSTALL_DIR/notify.sh" +# +# Written via a temp file + mv so the swap is atomic: on a reinstall the agents +# are usually live, and a hook firing between rm and cp would find no script. +cp "$SCRIPT_DIR/notify.sh" "$INSTALL_DIR/.notify.sh.new" +chmod +x "$INSTALL_DIR/.notify.sh.new" +mv -f "$INSTALL_DIR/.notify.sh.new" "$INSTALL_DIR/notify.sh" echo " Installed notify.sh -> $INSTALL_DIR/notify.sh" +# jq is optional: the socket payload is parsed by python3, but the permission +# banner's "what needs approval" detail still reads the hook JSON with jq. +if ! command -v jq >/dev/null 2>&1; then + echo " ! jq not found; permission banners will omit the tool/command detail." + echo " Install it with: brew install jq" +fi + if [[ -d "$SCRIPT_DIR/phrases" ]]; then rm -rf "$INSTALL_DIR/phrases" cp -R "$SCRIPT_DIR/phrases" "$INSTALL_DIR/phrases" diff --git a/notify.sh b/notify.sh index a37cd68..74fae8b 100755 --- a/notify.sh +++ b/notify.sh @@ -1,5 +1,13 @@ #!/usr/bin/env bash # stack-nudge: Cross-platform notifications for AI coding agent hooks +# +# The installed copy at ~/.stack-nudge/notify.sh is what agent hooks actually +# invoke, and it is only written by install.sh / the first-launch wizard; app +# updates swap the .app bundle alone. The stamp below is how the app spots a copy +# older than itself and replaces it (Bootstrap.refreshNotifyScriptIfNeeded); +# release-please rewrites it in step with panel/Info.plist. Don't hand-edit it. +# stack-nudge-version: 1.26.0 # x-release-please-version +# # Usage: notify.sh # agent: claude-code | cursor | gemini | codex | # event: stop | permission @@ -445,6 +453,14 @@ post_to_panel() { walk_session_chain detect_iterm_tab_name + # Cap what the exec carries: env and argv share ARG_MAX (1 MiB on macOS), and a + # PermissionRequest payload can hold a whole file's contents in tool_input. + # Overflowing it fails the python3 exec, which would lose the event entirely. + # The fields read from it are a few hundred bytes deep in a Stop payload, so a + # 32 KiB cap never bites for the events that need them. + local hook_json="$HOOK_JSON" + (( ${#hook_json} > 32768 )) && hook_json="" + NUDGE_AGENT="$AGENT" \ NUDGE_EVENT="$EVENT" \ NUDGE_TITLE="$1" \ @@ -466,12 +482,32 @@ post_to_panel() { NUDGE_TERM_PROGRAM="${TERM_PROGRAM:-}" \ NUDGE_SESSION_ID="${TERM_SESSION_ID:-${ITERM_SESSION_ID:-}}" \ NUDGE_ITERM_TAB_NAME="${ITERM_TAB_NAME:-}" \ - NUDGE_CLAUDE_SESSION_ID="$(command -v jq &>/dev/null && [[ -n "$HOOK_JSON" ]] && printf '%s' "$HOOK_JSON" | jq -r '.session_id // empty' 2>/dev/null || true)" \ - NUDGE_TRANSCRIPT_PATH="$(command -v jq &>/dev/null && [[ -n "$HOOK_JSON" ]] && printf '%s' "$HOOK_JSON" | jq -r '.transcript_path // empty' 2>/dev/null || true)" \ + NUDGE_HOOK_JSON="$hook_json" \ python3 - <<'PY' 2>/dev/null import json, os, socket, time env = os.environ + +# session_id / transcript_path come straight out of the hook JSON. Parsed here +# rather than with jq upstream: only macOS 15+ ships /usr/bin/jq, and the old +# `command -v jq && … || true` form degraded to an empty string when it was +# absent, which the panel can't distinguish from "no session". Missing +# claude_session_id silently drops the whole handoff record +# (PanelController.captureHandoff), leaving the Tickets tab empty. python3 is +# already required to reach the socket at all, so parsing here adds no dependency. +def hook_field(name): + raw = env.get("NUDGE_HOOK_JSON") or "" + if not raw: + return None + try: + parsed = json.loads(raw) + except ValueError: + return None + if not isinstance(parsed, dict): + return None + value = parsed.get(name) + return value if isinstance(value, str) and value else None + out = { "agent": env["NUDGE_AGENT"], "event": env["NUDGE_EVENT"], @@ -496,8 +532,8 @@ optional = { "term_program": env.get("NUDGE_TERM_PROGRAM"), "session_id": env.get("NUDGE_SESSION_ID"), "iterm_tab_name": env.get("NUDGE_ITERM_TAB_NAME"), - "claude_session_id": env.get("NUDGE_CLAUDE_SESSION_ID"), - "transcript_path": env.get("NUDGE_TRANSCRIPT_PATH"), + "claude_session_id": hook_field("session_id"), + "transcript_path": hook_field("transcript_path"), "voice_message": env.get("NUDGE_VOICE_MESSAGE"), "sound_name": env.get("NUDGE_SOUND"), } diff --git a/panel/Bootstrap.swift b/panel/Bootstrap.swift index 70f67d9..cef56a2 100644 --- a/panel/Bootstrap.swift +++ b/panel/Bootstrap.swift @@ -91,6 +91,139 @@ enum Bootstrap { pattern: #"(?:^|/|")\.?(?:tinynudge|stack-nudge)/notify\.sh"# ) + // MARK: Managed-script freshness + + // notify.sh carries `# stack-nudge-version: `, bumped by + // release-please alongside panel/Info.plist. + static let notifyVersionPattern = #"^#\s*stack-nudge-version:\s*([0-9]+\.[0-9]+\.[0-9]+)"# + + // Read the stamp out of a script's text. Pure so the matrix of shapes + // (stamped, unstamped, malformed) is testable without touching disk. + // Scans only the header: the stamp is a top-of-file comment, and a later + // line mentioning the key in prose must not win. + static func notifyVersion(inScript script: String) -> String? { + guard let regex = try? NSRegularExpression(pattern: notifyVersionPattern, + options: [.anchorsMatchLines]) + else { return nil } + let header = script.split(separator: "\n", maxSplits: 40, + omittingEmptySubsequences: false) + .prefix(40).joined(separator: "\n") + let range = NSRange(header.startIndex.. String? { + guard let script = try? String(contentsOfFile: notifyPath, encoding: .utf8) + else { return nil } + return notifyVersion(inScript: script) + } + + // The hook script this bundle ships, nil on a build that didn't bundle it. + static func bundledNotifyScript() -> String? { + guard let url = Bundle.main.url(forResource: "notify.sh", withExtension: nil) + else { return nil } + return try? String(contentsOf: url, encoding: .utf8) + } + + // Stamp of the script this bundle ships, nil on an unstamped local build. + static func bundledNotifyVersion() -> String? { + bundledNotifyScript().flatMap { notifyVersion(inScript: $0) } + } + + // Whether the installed script should be replaced by the bundled one. + // + // Compares the two scripts' own stamps rather than the installed stamp + // against CFBundleShortVersionString: same source of truth on both sides, so + // a dev build whose Info.plist and script disagree can't trigger a pointless + // rewrite. nil `installed` means a copy from before stamping existed (every + // install that predates v1.26.x), which is exactly the drift to repair. nil + // `bundled` means we can't tell (an unstamped local build), so leave the + // user's file alone. + static func needsNotifyRefresh(installed: String?, bundled: String?) -> Bool { + guard let bundled else { return false } + return installed != bundled + } + + // Replace ~/.stack-nudge/notify.sh when the bundle ships a different version. + // + // Exists because the hook script is half of the wire protocol with the panel + // (it builds the socket payload) but is not part of the .app that updates + // swap, so a self-updating install pins whatever version first installed it. + // A stale script silently omits newer payload fields: a pre-1.12 copy sends + // no claude_session_id, which drops every handoff record and leaves the + // Tickets tab permanently empty. + // + // Returns the version it wrote, or nil when nothing needed doing. Cheap + // enough for every launch: one small read, and a write only on drift. + @discardableResult + static func refreshNotifyScriptIfNeeded() -> String? { + refreshNotifyScript(bundled: bundledNotifyScript(), installedPath: notifyPath) + } + + // The decision plus the write, with both sides injected so the whole path is + // testable (Bundle.main in a test process ships no Resources/notify.sh). + // Skips a machine with no installed script at all: that's a pre-install or + // mid-uninstall state, and planting one there would wire nothing up anyway. + @discardableResult + static func refreshNotifyScript(bundled bundledScript: String?, + installedPath: String) -> String? { + guard FileManager.default.fileExists(atPath: installedPath), + let bundledScript + else { return nil } + let bundled = notifyVersion(inScript: bundledScript) + let installed = (try? String(contentsOfFile: installedPath, encoding: .utf8)) + .flatMap { notifyVersion(inScript: $0) } + guard needsNotifyRefresh(installed: installed, bundled: bundled) else { return nil } + do { + try writeNotifyScript(bundledScript, to: installedPath) + log("refreshed notify.sh: \(installed ?? "unstamped") -> \(bundled ?? "unstamped")") + return bundled + } catch { + log("failed to refresh notify.sh: \(error.localizedDescription)") + return nil + } + } + + // Write the hook script and make it executable, swapping it in atomically: + // agents are live while the app runs, and a hook firing during a + // remove-then-copy would find no script at all. Written to a sibling temp + // path so the replace is a same-volume rename. + // `to:` is injectable for tests only; every caller writes the real path. + static func writeNotifyScript(_ script: String, to path: String = notifyPath) throws { + let fm = FileManager.default + let dest = URL(fileURLWithPath: path) + let temp = dest.deletingLastPathComponent() + .appendingPathComponent(".notify.sh.\(UUID().uuidString)") + do { + try script.write(to: temp, atomically: false, encoding: .utf8) + if fm.fileExists(atPath: dest.path) { + _ = try fm.replaceItemAt(dest, withItemAt: temp) + } else { + try fm.moveItem(at: temp, to: dest) + } + // After the swap, not before: replaceItemAt deliberately carries the + // *original* file's attributes over to the replacement, so a mode set + // on the temp file is discarded. A copy that arrived non-executable + // (0644 from a hand-edit or an odd umask) would otherwise stay that + // way and every hook would fail with "permission denied". + _ = chmod(dest.path, 0o755) + } catch { + try? fm.removeItem(at: temp) + throw BootstrapError.writeFailed(path, underlying: error) + } + } + + // Diagnostics go to stderr, which launchd redirects to + // ~/.stack-nudge/app.log (see writePanelPlist). + private static func log(_ message: String) { + FileHandle.standardError.write(Data("stack-nudge: \(message)\n".utf8)) + } + // MARK: Detection // First-launch detection. Returns true when any of the install @@ -302,8 +435,10 @@ enum Bootstrap { withIntermediateDirectories: true) progress("Copying notify.sh…") - try copyBundledResource(named: "notify.sh", to: notifyPath) - _ = chmod(notifyPath, 0o755) + guard let notifyURL = Bundle.main.url(forResource: "notify.sh", withExtension: nil), + let notifyScript = try? String(contentsOf: notifyURL, encoding: .utf8) + else { throw BootstrapError.bundleResourceMissing("notify.sh") } + try writeNotifyScript(notifyScript) progress("Copying phrase pools…") // Wipe then recopy so reinstalls pick up new phrases. diff --git a/panel/Handoff.swift b/panel/Handoff.swift index 3d47e11..669fe54 100644 --- a/panel/Handoff.swift +++ b/panel/Handoff.swift @@ -29,3 +29,42 @@ struct HandoffRecord: Codable, Identifiable, Equatable { let createdAt: Date var updatedAt: Date } + +// Why a Stop event produced no record. Every case is a legitimate skip *or* a +// broken install, and the two look identical from the Tickets tab, so the reason +// is counted and surfaced there rather than dropped on the floor. +enum HandoffDropReason: String, Equatable { + // The hook payload carried no session id. Means the installed notify.sh + // predates the field (see Bootstrap.refreshNotifyScriptIfNeeded) or couldn't + // parse the hook JSON. Nothing can be recorded without it. + case missingSessionID + // No cwd in the payload, so there's no repo to attribute the work to. + case missingProjectPath + // Agent ran outside a git repo. Expected, not a fault: there's no branch or + // ticket to roll up. + case notAGitRepo + + // Shown in the Tickets empty state, in reason-priority order. + var summary: String { + switch self { + case .missingSessionID: + return "the hook payload had no session id" + case .missingProjectPath: + return "the hook payload had no working directory" + case .notAGitRepo: + return "the agent wasn't running inside a git repo" + } + } + + // The user-actionable next step, or nil when the drop is expected. Both + // payload gaps point at the same culprit: a hook script the app couldn't + // bring up to date. + var remedy: String? { + switch self { + case .missingSessionID, .missingProjectPath: + return "Reinstall from Settings to refresh ~/.stack-nudge/notify.sh." + case .notAGitRepo: + return nil + } + } +} diff --git a/panel/OutcomesView.swift b/panel/OutcomesView.swift index 63dcfd9..4d5d505 100644 --- a/panel/OutcomesView.swift +++ b/panel/OutcomesView.swift @@ -674,14 +674,16 @@ struct OutcomesView: View { } private var emptyState: some View { - VStack(spacing: 10) { - Image(systemName: "tag") + let reason = Self.emptyReason(totalGroups: nav.totalOutcomeGroupCount(), + drops: nav.handoffDrops) + return VStack(spacing: 10) { + Image(systemName: reason.symbol) .font(.title2) .foregroundStyle(.secondary) - Text("No tracked sessions yet") + Text(reason.title) .font(.subheadline) .foregroundStyle(.secondary) - Text("When an agent finishes a turn inside a git repo, stack-nudge records the session here — grouped by its Linear/Jira ticket, or the branch when there's no ticket.") + Text(reason.detail) .font(.caption) .foregroundStyle(.tertiary) .multilineTextAlignment(.center) @@ -691,4 +693,63 @@ struct OutcomesView: View { .frame(maxWidth: .infinity, maxHeight: .infinity) .padding(.vertical, 24) } + + // MARK: - Empty state (pure, testable) + + // Why the list is empty. Three states that used to render one string, which + // made a filtered-away list and a broken capture path indistinguishable from + // a machine that simply hasn't finished a turn yet. + enum EmptyReason: Equatable { + case noSessions + // Ledger has groups, but the hide-shipped filter removed all of them. + case allShipped(hidden: Int) + // Stops arrived and every one was dropped before it reached the ledger. + case allDropped(count: Int, reason: HandoffDropReason) + + var symbol: String { + switch self { + case .noSessions: return "tag" + case .allShipped: return "checkmark.seal" + case .allDropped: return "exclamationmark.triangle" + } + } + + var title: String { + switch self { + case .noSessions: + return "No tracked sessions yet" + case .allShipped(let hidden): + return hidden == 1 ? "1 shipped ticket hidden" : "\(hidden) shipped tickets hidden" + case .allDropped(let count, _): + return count == 1 ? "1 turn wasn't recorded" : "\(count) turns weren't recorded" + } + } + + var detail: String { + switch self { + case .noSessions: + return "When an agent finishes a turn inside a git repo, stack-nudge records the session here, grouped by its Linear/Jira ticket, or the branch when there's no ticket." + case .allShipped: + return "Everything tracked has merged. Turn off Hide shipped in Settings to see it." + case .allDropped(_, let reason): + let remedy = reason.remedy.map { " \($0)" } ?? "" + return "Turns ended but \(reason.summary), so there was nothing to attribute.\(remedy)" + } + } + } + + // Priority: an all-filtered list is the least alarming explanation and takes + // precedence, then drops (the actionable fault), then a genuinely idle ledger. + // Reports the highest-count drop reason, since a mixed bag is dominated by + // whichever link is actually broken. + static func emptyReason(totalGroups: Int, + drops: [HandoffDropReason: Int]) -> EmptyReason { + if totalGroups > 0 { return .allShipped(hidden: totalGroups) } + let ranked = drops.filter { $0.value > 0 } + .sorted { ($0.value, $0.key.rawValue) > ($1.value, $1.key.rawValue) } + if let worst = ranked.first { + return .allDropped(count: ranked.reduce(0) { $0 + $1.value }, reason: worst.key) + } + return .noSessions + } } diff --git a/panel/Panel.swift b/panel/Panel.swift index f1de567..e84a23c 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -658,6 +658,11 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, // the marker so notify.sh will relaunch us on the next event after // the *next* Quit. Bootstrap.clearUserQuitMarker() + // Updates swap the .app but never the installed hook script, so a + // self-updating machine can run a current panel against a notify.sh from + // any earlier release, one that omits payload fields we now depend on. + // Repair it here, while we know the bundle we shipped with. + Bootstrap.refreshNotifyScriptIfNeeded() let size = Self.loadSavedPanelSize() let frame = NSRect(origin: .zero, size: size) @@ -1626,14 +1631,20 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, // for the per-ticket usage rollup. Non-git directories are skipped — there's // nothing to attribute to a repo/ticket. private func captureHandoff(for event: NudgeEvent) { - guard event.kind == .stop, - let sessionID = event.claudeSessionID, - let cwd = event.projectPath, !cwd.isEmpty - else { return } + guard event.kind == .stop else { return } + guard let sessionID = event.claudeSessionID else { + return dropHandoff(.missingSessionID, agent: event.agent) + } + guard let cwd = event.projectPath, !cwd.isEmpty else { + return dropHandoff(.missingProjectPath, agent: event.agent) + } let agent = Agent.canonical(event.agent) let transcriptPath = event.transcriptPath - DispatchQueue.global(qos: .utility).async { - guard let repoRoot = Self.gitValue(cwd, ["rev-parse", "--show-toplevel"]) else { return } + DispatchQueue.global(qos: .utility).async { [weak self] in + guard let repoRoot = Self.gitValue(cwd, ["rev-parse", "--show-toplevel"]) else { + DispatchQueue.main.async { self?.dropHandoff(.notAGitRepo, agent: agent) } + return + } let branch = Self.gitValue(cwd, ["rev-parse", "--abbrev-ref", "HEAD"]) // Only consider the subject of a commit *unique to this branch* // (base..HEAD). The absolute last commit may already be on main — @@ -1682,6 +1693,19 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, } } + // Account for a Stop that produced no ledger row: one stderr line (launchd + // redirects it to ~/.stack-nudge/app.log) plus a counter the Tickets empty + // state reads, so a broken payload is visible in the UI instead of looking + // like an idle machine. Main-thread only, like the ledger itself. + private func dropHandoff(_ reason: HandoffDropReason, agent: String) { + nav.handoffDrops[reason, default: 0] += 1 + // Working outside a repo is the expected skip, so it stays quiet rather + // than filling app.log for anyone running an agent in a scratch directory. + guard reason != .notAGitRepo else { return } + FileHandle.standardError.write(Data( + "stack-nudge: dropped \(agent) handoff: \(reason.summary)\n".utf8)) + } + // Recompute "did it ship?" for every distinct repo+branch in the ledger, // off-main (git is slow), then publish to nav for the Tickets tab. Uses the // newest record per branch (ledger is newest-first) for the headCommit / diff --git a/panel/PanelNav.swift b/panel/PanelNav.swift index 780d64f..1f887b4 100644 --- a/panel/PanelNav.swift +++ b/panel/PanelNav.swift @@ -334,6 +334,12 @@ final class PanelNav: ObservableObject { // Wired by PanelController — start/cancel the device-flow sign-in. var startGithubSignIn: (() -> Void)? var cancelGithubSignIn: (() -> Void)? + // Stop events that reached us but produced no ledger row, counted by reason + // (PanelController.captureHandoff). Lets the Tickets empty state say which + // link in the chain broke instead of implying no work has happened: the + // difference between "you haven't finished a turn yet" and "every turn was + // dropped because the hook payload is missing a session id". + @Published var handoffDrops: [HandoffDropReason: Int] = [:] static func outcomeKey(_ repoRoot: String?, _ branch: String?) -> String { "\(repoRoot ?? "")\n\(branch ?? "")" @@ -347,17 +353,25 @@ final class PanelNav: ObservableObject { // The groups the Tickets tab renders, after the hide-shipped filter. Single // source of truth so the view and the keyboard indexing never disagree. func visibleOutcomeGroups() -> [TicketGroup] { - let groups: [TicketGroup] - if let cached = cachedOutcomeGroups { - groups = cached - } else { - groups = OutcomesView.groups(from: HandoffLedger.shared.all()) - cachedOutcomeGroups = groups - } + let groups = allOutcomeGroups() guard hideShippedTickets else { return groups } return groups.filter { !isShipped($0) } } + // Every group in the ledger, filter aside, as against the tab-strip badge's + // `visibleOutcomeGroups().count`. The empty state reads this to tell "nothing + // was ever recorded" from "hide-shipped removed them all". + func totalOutcomeGroupCount() -> Int { + allOutcomeGroups().count + } + + private func allOutcomeGroups() -> [TicketGroup] { + if let cached = cachedOutcomeGroups { return cached } + let groups = OutcomesView.groups(from: HandoffLedger.shared.all()) + cachedOutcomeGroups = groups + return groups + } + // "Shipped" = every branch in the group reads merged (PR state preferred, // else the local outcome). Empty groups aren't shipped. func isShipped(_ group: TicketGroup) -> Bool { diff --git a/panel/Settings.swift b/panel/Settings.swift index eb154de..0e60a40 100644 --- a/panel/Settings.swift +++ b/panel/Settings.swift @@ -14,6 +14,11 @@ struct SettingsView: View { @ObservedObject var nav: PanelNav + // Hook-script freshness, sampled on appear (two small file reads) rather than + // recomputed every render. Surfaced in aboutFooter. + @State private var installedHookVersion: String? + @State private var hookScriptStale = false + var body: some View { VStack(alignment: .leading, spacing: 0) { ScrollViewReader { proxy in @@ -186,6 +191,10 @@ struct SettingsView: View { // after the user grants a permission in System Settings and // returns to the panel. nav.refreshPermissions() + installedHookVersion = Bootstrap.installedNotifyVersion() + hookScriptStale = Bootstrap.needsNotifyRefresh( + installed: installedHookVersion, + bundled: Bootstrap.bundledNotifyVersion()) } } @@ -520,6 +529,17 @@ struct SettingsView: View { Text("StackNudge v\(version)") .font(.caption2.monospacedDigit()) .foregroundStyle(.tertiary) + // The app rewrites a stale hook script at launch, so a mismatch that + // survives to here means the rewrite failed (read-only dotdir, wrong + // owner) and hook payloads may be missing fields the panel needs. + // Read on appear, not per render, to keep this off the render path. + if hookScriptStale { + Text("Hook script \(installedHookVersion.map { "v\($0)" } ?? "unstamped") is out of date; reinstall to refresh") + .font(.caption2) + .foregroundStyle(.orange) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + } Button { if let url = URL(string: "https://github.com/StackOneHQ/stack-nudge") { NSWorkspace.shared.open(url) diff --git a/scripts/xctest-shim.swift b/scripts/xctest-shim.swift index e0dde60..3fdc642 100644 --- a/scripts/xctest-shim.swift +++ b/scripts/xctest-shim.swift @@ -21,6 +21,9 @@ class XCTestCase { @MainActor func tearDown() {} @MainActor func setUpWithError() throws {} @MainActor func tearDownWithError() throws {} + // Real XCTest runs these after the test; nothing to run here, and the block + // is deliberately discarded rather than invoked so the type-check stays inert. + func addTeardownBlock(_ block: @escaping () throws -> Void) {} } func XCTAssertEqual(_ a: @autoclosure () throws -> T, @@ -61,6 +64,15 @@ func XCTAssertLessThanOrEqual(_ a: @autoclosure () throws -> T, file: StaticString = #filePath, line: UInt = #line) {} func XCTFail(_ message: @autoclosure () -> String = "", file: StaticString = #filePath, line: UInt = #line) {} +// The expression is `throws`, so the stand-in has to accept a throwing closure +// (and the trailing error handler) to type-check the same call sites XCTest does. +func XCTAssertThrowsError(_ e: @autoclosure () throws -> T, + _ message: @autoclosure () -> String = "", + file: StaticString = #filePath, line: UInt = #line, + _ errorHandler: (Error) -> Void = { _ in }) {} +func XCTAssertNoThrow(_ e: @autoclosure () throws -> T, + _ message: @autoclosure () -> String = "", + file: StaticString = #filePath, line: UInt = #line) {} func XCTUnwrap(_ e: @autoclosure () throws -> T?, _ message: @autoclosure () -> String = "", file: StaticString = #filePath, line: UInt = #line) throws -> T {