prism branch worktree-settings-commit-hash commits 2 files 4 touched lines +75 / -0

Pre-push review: worktree-settings-commit-hash

Adds a Debug-only "Build" row in Settings → About showing the short git commit hash the build was produced from.

At a glance

  • What: Settings → About now lists a Build row with the short git commit hash, suffixed -dirty when the working tree had uncommitted changes.
  • How: A new Tools/stamp-commit-hash.sh build phase writes GitCommit into the bundled Info.plist via PlistBuddy. The script and UI are both gated to Debug builds so the hash never ships to end users.
  • Why Debug-only: The app has in-app purchase code — it's a shipped product, not a personal-use binary. Exposing internal build metadata to App Store users isn't desired.

Verdict

Ready to push

Single-concept change. Debug-only via both CONFIGURATION guard in the build script and #if DEBUG around the UI / accessor. Verified: Debug bundle Info.plist carries GitCommit = fdf0531-dirty; Release Info.plist has no such key. Lint clean. The DiagramWindowManagerTests failures observed during make test-quick reproduce on origin/main and pass in isolation on this branch — pre-existing parallel-test flakiness, unrelated to these changes.

Review findings

4 raised · 1 fixed · 3 skipped

Jump to findings →

Commits

Three-level explanation

What Changed

A new line called "Build" now shows up in the app's Settings → About screen, right under "Version". It displays a short code (the commit hash) like 8c40b9a that uniquely identifies the snapshot of source code the app was built from. If the developer had unsaved changes lying around, the code gets -dirty tacked on. This row only appears in Debug builds — the kind running on the developer's own machine. App Store builds don't show it.

Why It Matters

The version number ("1.0") changes only at official releases. But while developing, the app gets built dozens of times a day, and every build can have different code in it. With the version alone, you have no way to look at a running copy of the app and know which exact version of the code it came from. The commit hash is like a serial number for the code — it lets the developer pick up their phone running an old test build and instantly tell which build it is.

Key Concepts

  • Commit hash: Git assigns every saved snapshot of code a unique fingerprint; the short form is the first 7 characters.
  • Info.plist: A metadata file packaged inside every Mac/iOS app — the new key gets stored here.
  • Build phase: A small script Xcode runs while building. We added one that asks git for the current commit hash and writes it into Info.plist.
  • Debug vs Release: Two build modes — Debug for development, Release for shipping. The stamp only runs in Debug.

Changes Overview

Four files across two commits (fdf0531, abdecfc):

  • Tools/stamp-commit-hash.sh (new, 36 lines): Bails when ${CONFIGURATION} isn't Debug. Otherwise runs git rev-parse --short HEAD, appends -dirty if git status --porcelain is non-empty, writes the result to ${TARGET_BUILD_DIR}/${INFOPLIST_PATH} as a GitCommit key via PlistBuddy. Falls back to "unknown" outside a git checkout; bails gracefully if Info.plist is missing.
  • prism.xcodeproj/project.pbxproj: Adds a new PBXShellScriptBuildPhase to the prism target's buildPhases, after "Localisation: install compiled strings" (Info.plist already in the bundle). alwaysOutOfDate = 1, no inputPaths/outputPaths.
  • prism/Settings/SettingsView.swift: Adds Bundle.appCommitHash: String? next to Bundle.appVersion, both wrapped in #if DEBUG. Adds an #if DEBUG-gated LabeledContent("Build", value: commit) row to aboutSection, monospaced + selectable, with explicit accessibility label/value.
  • CHANGELOG.md: one bullet under ## [Unreleased] / ### Added.

Implementation Approach

Build-time metadata injection via post-bundling plist mutation. Xcode already auto-generates Info.plist keys from build settings; we piggyback by mutating the same plist after Xcode produces it — avoiding the alternative of code-generating a Swift source file (which fights Xcode's source-input discovery for generated files). PlistBuddy's Delete-then-Add idiom (with the delete silenced) makes re-runs idempotent.

Trade-offs

  • alwaysOutOfDate over input-tracked: .git is a file (not a directory) inside worktrees and index churns on every git add, so input-tracking would either be wrong or barely save any work.
  • Build-time stamp over runtime read: shipping git into the app would be absurd; committing the hash to a generated file loses the "is this build clean" signal.
  • Debug-only over always-on: User flagged that this is a publicly-distributed app (has IAP code) and shouldn't leak internal build state to end users.

Technical Deep Dive

  • The phase runs after Resources and Localisation: install, so by the time PlistBuddy opens the file, Xcode's Process Info.plist file step has already merged INFOPLIST_KEY_* build settings into the source prism/Info.plist and copied the result into the bundle. Code-signing then re-runs against the mutated plist — the build log shows the ordering.
  • git status --porcelain is intentionally chosen over git diff --quiet HEAD: porcelain also flags untracked files, which is the spirit of "did the developer have stuff they hadn't committed yet?"
  • cd "${SRCROOT}" is necessary because the build-phase CWD is undefined and porcelain/rev-parse need a worktree-relative cwd. Inside a worktree, SRCROOT is the worktree path; git resolves .git as a file pointing to <repo>/.git/worktrees/<name>, which both commands understand.
  • The Delete :GitCommit ... 2>/dev/null || true idiom is defensive: PlistBuddy returns non-zero when the key is absent (first build for a given DerivedData), but the script must be first-run and re-run safe.
  • Failure mode if Info.plist is missing: stderr log + exit 0. 0 over non-zero is deliberate — missing plist usually means misconfigured target, and failing the build here would create a confusing diagnostic.

Architecture Impact

  • Adds a Debug build-time dependency on git on PATH. Standard on dev machines.
  • Couples the build to the host filesystem (cwd into SRCROOT). Sandbox-mode builds would need read access to .git. Project has ENABLE_USER_SCRIPT_SANDBOXING = NO, matching the existing localisation phases.
  • Bundle.appCommitHash joins Bundle.appVersion as a small surface for build metadata; future stamps (branch name, build timestamp, CI build number) can follow the same recipe.

Potential Issues

  • TestFlight / App Store builds: resolved by the Debug gate. Verified Release Info.plist has no GitCommit key.
  • Build determinism: archive builds for the same commit produce non-identical Debug bundles when the working tree's -dirty state differs. Release builds are unaffected.
  • Code-sign overhead: Debug incremental builds re-sign the bundle because Info.plist changed. ~1s overhead per build. Negligible for a personal workflow.
  • Submodules: git status --porcelain flags submodule pointer changes too — currently no submodules, so a non-issue, but worth remembering.
  • Multi-target binaries (iOS + macOS): same phase runs for both; different bundle layouts (Contents/Info.plist vs root-level Info.plist) handled transparently via INFOPLIST_PATH.

Important changes — detailed

stamp-commit-hash.sh: Debug-only git hash injection into Info.plist

Tools/stamp-commit-hash.sh

Why it matters. This is the entire mechanism: it reads git state, formats the hash, and writes it into the embedded Info.plist. Correctness, Debug-gate behaviour, and the dirty-detection rule all live here.

What to look at. Tools/stamp-commit-hash.sh:1-36

Takeaway. Use a build-phase script + PlistBuddy to inject build-time metadata into the bundled Info.plist after Xcode's own plist-processing has finished. Cleaner than code-generating a Swift source file (which fights Xcode's source-input discovery).
Rationale. User explicitly asked for the stamp to be Debug-only after noticing the app has in-app purchase code and ships publicly. The CONFIGURATION guard is the primary gate; the #if DEBUG in Swift is defence in depth.

project.pbxproj: new PBXShellScriptBuildPhase

prism.xcodeproj/project.pbxproj

Why it matters. Defines when the script runs (after Localisation: install) and how Xcode tracks it (alwaysOutOfDate=1, no input/output dependencies). Wrong placement or wrong dependency model would silently produce stale or missing stamps.

What to look at. prism.xcodeproj/project.pbxproj:132, 328-342

Takeaway. alwaysOutOfDate=1 is the honest choice when the dependency is git state — input-tracking .git/HEAD/index in a worktree leads to either wrong skips (worktree .git is a file pointer) or no real savings (index churns constantly).
Rationale. Worktree-friendly: .git is a file inside a worktree, not a directory, so declaring .git/HEAD as an input either fails or behaves unpredictably. Cost of running every build (~50ms git + Info.plist re-sign) is acceptable for a Debug-only verification aid. (inferred — not stated by the author)

SettingsView: Debug-only Build row + Bundle.appCommitHash accessor

prism/Settings/SettingsView.swift

Why it matters. User-visible surface. The #if DEBUG block guarantees the hash UI never reaches Release users even if a GitCommit key somehow ended up in a Release plist via a future bug.

What to look at. prism/Settings/SettingsView.swift:492-501, 575-587

Takeaway. Layer compile-time gates (#if DEBUG) on top of build-system gates (CONFIGURATION check) for defence in depth on features that should never reach production users.
Rationale. Single-line guards both at the script level (CONFIGURATION) and the Swift level (#if DEBUG). Wrapped Bundle.appCommitHash in #if DEBUG too, so it's dead code stripped from Release rather than always-compiled-but-never-used.

Key decisions

Debug-only gating via both CONFIGURATION check and #if DEBUG

The build script bails on ${CONFIGURATION} != Debug, which is sufficient on its own — Release builds produce no GitCommit key and the conditional UI if let commit = ... hides the row. The #if DEBUG wrapper around the UI and accessor is defence in depth, so a future regression that accidentally lets the key into a Release plist still can't reach end users.

alwaysOutOfDate over input-tracked build phase

Inside a git worktree, .git is a file (not a directory) that points to the real worktree state, so naive input-paths like $(SRCROOT)/.git/HEAD may not exist on disk where Xcode expects them. The index file also changes on every git add, even when the commit hash hasn't changed, so input-tracking it would barely save any work. Running ~50ms of git + a plist re-sign every Debug build is a reasonable trade-off.

(inferred — not stated by the author.)
PlistBuddy mutation over code-generated Swift

Generated Swift sources need to be added to the Sources build phase; Xcode's source-input discovery doesn't auto-pick-up generated files cleanly. Mutating the already-emitted Info.plist sidesteps that entirely — only Xcode's existing signing step needs to re-run.

(inferred — not stated by the author.)
git status --porcelain over git diff --quiet HEAD for dirty detection

Porcelain also flags untracked files, which matches the developer intent of "did I have stuff I hadn't committed yet?" git diff --quiet HEAD would miss new files that aren't yet git add-ed.

(inferred — not stated by the author.)

Review findings

SeverityAreaFindingResolution
minorRelease-build hash leakInitial commit (fdf0531) stamped GitCommit into every build, including Release/TestFlight/App Store archives, exposing internal dev metadata to end users.Second commit (abdecfc) added a CONFIGURATION=Debug guard to the script and wrapped the UI / Bundle accessor in #if DEBUG. Verified Release plist has no GitCommit key.
nitStringly-typed Info.plist keyThe literal "GitCommit" appears in both the shell script and the Swift accessor.Skipped — can't share a constant across the shell/Swift language boundary anyway, and the surface is two call-sites.
nitgit status --porcelain cost on every buildEfficiency agent noted alwaysOutOfDate + porcelain costs ~50ms + bundle re-sign on every incremental build.Skipped — input-tracking .git state is worse (worktree .git is a file pointer, index churns) and the cost is Debug-only.
nitFalse positive: "Build" literal flagged as localisation violationCode-quality agent flagged LabeledContent("Build", value:) as missing a LocalizedStringKey wrapper.Skipped — CLAUDE.md explicitly allows literals ("Xcode auto-extracts these as catalog keys"); the existing LabeledContent("Version", value:) on the line above uses the same pattern.

Per-file diffs

Click to expand.

CHANGELOG.md Modified +1 / -0
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8dd085a..85aabb6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `prism-notes-js` spec for a self-contained single-file JavaScript library that adds basic note-taking and copy-as-markdown to any HTML page (personal-use companion artifact; not part of the Prism Swift app). Smolspec, decision log, and 8 implementation tasks live under `specs/prism-notes-js/`.
 - `prism-notes-js/prism-notes.js` implementation (~1500 lines, single ES2022 IIFE, no dependencies). Selecting text shows a "+ Add note" pill; saving creates a note anchored via a text-quote selector (prefix/exact/suffix + position hint) and a SHA-256 document-text hash. Notes persist in `localStorage` keyed by `pathname+search`, re-highlight via the CSS Custom Highlight API on reload, and surface in a side panel with a "Copy as Markdown" footer button (clipboard write with a textarea-fallback modal). Includes `prism-notes-js/README.md` documenting the three install paths (script tag, bookmarklet for permissive CSP, DevTools paste for strict CSP) and `prism-notes-js/test-page.html` as a manual-test fixture. Personal-use scope: no sync, no sharing, no import, no Shadow DOM, no accessibility commitments.
 - `footnote-badge-a11y` smolspec (T-1036) for VoiceOver discoverability and operability of inline footnote badges, covering paragraph-level accessibility-label rebuild and per-paragraph accessibility actions in `HighlightedInlineText`. Decision log captures the pivot away from a Textual fork change to a single-file view-layer approach. Five linearly-dependent tasks live under `specs/footnote-badge-a11y/`.
+- **Debug-only:** Settings → About now shows a "Build" row beneath "Version" with the short git commit hash the build was produced from, suffixed `-dirty` when the working tree had uncommitted changes (and `unknown` when built outside a git checkout). A new `Tools/stamp-commit-hash.sh` build phase writes `GitCommit` into the bundled `Info.plist` using `PlistBuddy`; the script bails early when `CONFIGURATION` isn't `Debug`, so Release/TestFlight/App Store builds never carry the key. The Settings UI and `Bundle.appCommitHash` accessor are wrapped in `#if DEBUG` as a second line of defence. The phase is marked always-out-of-date so the Debug value refreshes on every build.
 - Footnote badges are now discoverable and operable for VoiceOver (T-1036). New `FootnoteAccessibility` helper (`prism/Services/FootnoteAccessibility.swift`) extracts unique footnote references in first-occurrence order and rebuilds a paragraph-level accessibility label that substitutes each resolvable `[^id]` with the localised inline phrase "footnote N". `HighlightedInlineText` applies a private `FootnoteAccessibilityModifier` that overrides the paragraph's accessibility label and exposes one "Show footnote N" action per unique reference via `.accessibilityActions { … }`. Each action opens the existing `prism://footnote/{id}` URL through `@Environment(\.openURL)`, reusing the tap-routing flow already wired in `DocumentReaderView`. The modifier is a no-op when no resolvable references are present, so paragraphs without footnotes keep their auto-derived narration. New localised keys `a11y.footnote.inline %lld` and `a11y.footnote.action %lld` added to `prism/Localizable.xcstrings`. Helpers covered by new unit tests in `prismTests/FootnoteAccessibilityTests.swift`.

 ### Fixed
Tools/stamp-commit-hash.sh Added +36 / -0
diff --git a/Tools/stamp-commit-hash.sh b/Tools/stamp-commit-hash.sh
new file mode 100755
index 0000000..dcb5131
--- /dev/null
+++ b/Tools/stamp-commit-hash.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+# Stamps the current git commit hash into the bundled Info.plist as the
+# `GitCommit` key, so the running app can show which commit it was built from.
+# Falls back to "unknown" outside a git working tree.
+
+set -eu
+
+# Debug builds only — we don't want to leak a git commit hash to end users
+# in TestFlight / App Store archives. Release builds skip the stamp; the
+# Settings UI is `#if DEBUG`-gated as a second layer of defence.
+if [ "${CONFIGURATION:-}" != "Debug" ]; then
+    echo "stamp-commit-hash: skipping (CONFIGURATION=${CONFIGURATION:-unset})"
+    exit 0
+fi
+
+PLIST_PATH="${TARGET_BUILD_DIR}/${INFOPLIST_PATH}"
+
+if [ ! -f "${PLIST_PATH}" ]; then
+    echo "stamp-commit-hash: Info.plist not found at ${PLIST_PATH}" >&2
+    exit 0
+fi
+
+cd "${SRCROOT}"
+
+if COMMIT=$(git rev-parse --short HEAD 2>/dev/null); then
+    if [ -n "$(git status --porcelain 2>/dev/null)" ]; then
+        COMMIT="${COMMIT}-dirty"
+    fi
+else
+    COMMIT="unknown"
+fi
+
+/usr/libexec/PlistBuddy -c "Delete :GitCommit" "${PLIST_PATH}" 2>/dev/null || true
+/usr/libexec/PlistBuddy -c "Add :GitCommit string ${COMMIT}" "${PLIST_PATH}"
+
+echo "stamp-commit-hash: GitCommit=${COMMIT}"
prism.xcodeproj/project.pbxproj Modified +16 / -0
diff --git a/prism.xcodeproj/project.pbxproj b/prism.xcodeproj/project.pbxproj
index 80fee3e..7cf486c 100644
--- a/prism.xcodeproj/project.pbxproj
+++ b/prism.xcodeproj/project.pbxproj
@@ -129,6 +129,7 @@
 				D49C76612F0CC12F00CE740B /* Frameworks */,
 				D49C76622F0CC12F00CE740B /* Resources */,
 				D4A678812FA80000005E8A40 /* Localisation: install compiled strings */,
+				D4A678822FA80000005E8A40 /* Stamp git commit hash */,
 			);
 			buildRules = (
 			);
@@ -324,6 +325,21 @@
 			shellPath = /bin/sh;
 			shellScript = "set -euo pipefail\nINSTALL_DIR=\"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}\"\nmkdir -p \"${INSTALL_DIR}\"\nfor locale in en en-AU en-GB en-US; do\n    mkdir -p \"${INSTALL_DIR}/${locale}.lproj\"\n    for ext in strings stringsdict; do\n        src=\"${DERIVED_FILE_DIR}/CompiledStrings/${locale}.lproj/Localizable.${ext}\"\n        if [ -f \"${src}\" ]; then\n            /bin/cp \"${src}\" \"${INSTALL_DIR}/${locale}.lproj/Localizable.${ext}\"\n        fi\n    done\ndone\n";
 		};
+		D4A678822FA80000005E8A40 /* Stamp git commit hash */ = {
+			isa = PBXShellScriptBuildPhase;
+			alwaysOutOfDate = 1;
+			buildActionMask = 2147483647;
+			files = (
+			);
+			inputPaths = (
+			);
+			name = "Stamp git commit hash";
+			outputPaths = (
+			);
+			runOnlyForDeploymentPostprocessing = 0;
+			shellPath = /bin/sh;
+			shellScript = "set -eu\n/bin/sh \"${SRCROOT}/Tools/stamp-commit-hash.sh\"\n";
+		};
 /* End PBXShellScriptBuildPhase section */

 /* Begin PBXSourcesBuildPhase section */
prism/Settings/SettingsView.swift Modified +22 / -0
diff --git a/prism/Settings/SettingsView.swift b/prism/Settings/SettingsView.swift
index 202f356..4de8b24 100644
--- a/prism/Settings/SettingsView.swift
+++ b/prism/Settings/SettingsView.swift
@@ -490,6 +490,15 @@ struct SettingsView: View {
     private var aboutSection: some View {
         Section {
             LabeledContent("Version", value: Bundle.main.appVersion)
+            #if DEBUG
+            if let commit = Bundle.main.appCommitHash {
+                LabeledContent("Build", value: commit)
+                    .accessibilityLabel(LocalizedStringKey("Build commit hash"))
+                    .accessibilityValue(commit)
+                    .textSelection(.enabled)
+                    .monospaced()
+            }
+            #endif
             Link(destination: URL(string: "https://github.com/ArjenSchwarz/prism")!) {
                 HStack {
                     Text("Source Code")
@@ -565,6 +574,19 @@ extension Bundle {
     var appVersion: String {
         (infoDictionary?["CFBundleShortVersionString"] as? String) ?? "1.0"
     }
+
+    #if DEBUG
+    /// Short git commit hash stamped into the Debug bundle at build time by
+    /// `Tools/stamp-commit-hash.sh`. Returns `nil` when the build wasn't
+    /// stamped (e.g. the build phase didn't run) and "unknown" when the build
+    /// happened outside a git working tree. Debug-only: Release builds skip
+    /// the stamp so the hash never ships to end users.
+    var appCommitHash: String? {
+        guard let value = infoDictionary?["GitCommit"] as? String,
+              !value.isEmpty else { return nil }
+        return value
+    }
+    #endif
 }

 #Preview {

Things to double-check

DiagramWindowManagerTests parallel flakiness

During make test-quick, ~15 tests in DiagramWindowManagerTests failed (e.g. deinitRemovesCloseObserver, closingMultipleWindowsViaNotificationsWorksCorrectly). Running the same tests in isolation passes on both this branch and on a clean origin/main checkout. Failure pattern (random subsets across parallel test runners) indicates shared-state pollution between parallel test instances. Not caused by this change — the diff touches Settings, a build phase, and a shell script; nothing touches NSWindow or notification observers.

Future build metadata extensions

If you ever want a branch name, build timestamp, or CI build number to show up similarly, the recipe is established: extend stamp-commit-hash.sh with extra PlistBuddy writes, add matching Bundle.app* accessors. Keep the Debug gate.