Label macOS worktree apps in the Dock - #113
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed head 0397db5d33d3f103a8cd3228e964492dd6e9f00f against base/merge-base 9053fa9d199e7e5ea5f3a5cb1765f78f842d5770 (brain/dev-command-ports, #109).
No merge-blocking code defects found. This is a comment review, not approval or native acceptance.
P3, non-blocking: keep the display label out of the temporary directory name
scripts/generate-dev-icon.swift:159–165 creates staged-dev-<sanitized label>-<UUID> as one filesystem component. A valid 220-character branch suffix becomes a 268-byte component, exceeding the local macOS 255-byte limit. I reproduced the actual Swift process trapping at line 165 with POSIX error 63 (File name too long), producing no icon. A sufficiently long detached-worktree directory name has the same problem. The Node helper catches the generator failure, warns, and preserves startup with the ordinary icon, so this is a narrow label failure rather than a launch blocker.
Reproduction from the checked-out head:
label=$(printf 'a%.0s' {1..220})
git check-ref-format "refs/heads/person/$label" # succeeds
swift scripts/generate-dev-icon.swift src-tauri/icons/icon.icns /tmp/buzz-long-label.icns "$label"Smallest remedy: use a fixed prefix plus UUID for the iconset parent; remove the now-unneeded label sanitizer. Validate with one real-renderer long-label case, since the existing helper tests stub Swift and cannot catch this. No new naming abstraction is needed.
Proportionality
The implementation is appropriately bounded: it extends #109’s single launcher, keeps artwork in an optional macOS helper, and uses Tauri’s existing configuration support. Unique staging plus a content-derived final path has a concrete job: protect in-flight bytes and invalidate the warm Cargo/Tauri cache after a branch rename. This is not speculative caching machinery. Notification policy, app identity, production configuration and dependencies are unchanged in the PR diff.
The restored Swift renderer still carries optional PNG output/full-size PNG encoding that this launcher does not use. Removing that is a small non-blocking simplification; a new runtime Dock owner or replacement renderer would be unnecessary scope.
Evidence and remaining gates
- At the exact head with a clean tracked tree: all 18 affected Node integration tests passed on macOS, including the launcher subprocess/config/runner test, branch rename at the same commit, detached labels and generation-failure cleanup.
git diff --checkpassed. - Ran real Swift/AppKit/iconutil generation: repeated identical input gave identical bytes/content paths; a changed label gave different bytes. Inspected a generated preview. Source-traced the locked Tauri codegen/cache and Cargo
TAURI_CONFIGinvalidation path; installed CLI help confirms explicit configs merge later and win. The author's warm-Cargo probe was not independently repeated. The combined icon/config test runs on macOS but is skipped on the current Linux CI lane; the local 18-test run exercised it without skips. - This remains stacked on #109: merge its launcher/port work first, retarget to
main, then recheck the diff. CI was partly complete and partly running when inspected. Actual Dock appearance, branch-rename/relaunch acceptance and native Windows launcher execution remain deferred. No app was launched, no source was edited, and no merge or approval was performed.
|
Fixed the long-label issue in 6642fab. The Swift temporary directory now uses a fixed prefix plus UUID; the display-label sanitizer is removed. Badge rendering and the content-keyed final icon path are unchanged. Added one macOS regression that invokes the real Swift/AppKit/iconutil pipeline with a 220-character label and decodes the resulting ICNS. It reproduced the original trap, then passed with the fix. All 82 Node integration tests passed on the final pre-commit tree; independent review cleared the two-file delta. The macOS test is explicitly skipped on Linux CI, not claimed as hosted renderer coverage. Two earlier local runs hit the existing 10-second stub-process timeout under concurrent machine load; the cause was not established. A process-observed run and the full suite then passed unchanged. No timeout increase or retry policy was introduced. The optional PNG mode is retained from the original renderer rather than coupling a separate interface cleanup to this path-naming fix. New-head CI is running. This remains draft and stacked on #109; no merge or native visual acceptance is implied. |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Re-review: no blocking code findings; integration is still pending
Reviewed head 6642faba557e4df6823e6b4bac0bedf3f3f2930b, including the change since 0397db5d33d3f103a8cd3228e964492dd6e9f00f. GitHub reports target main at base 16273b2031b249272c67269efa7fdd681c940641; I also checked the integration boundary against the newer live main b49b6e3a03de6100cb7127c2ec785f276e480c65.
- Previous P3 fixed. The renderer now uses a UUID-only temporary component and removes the unused sanitizer. Independently ran
bin/node --test tests/integration/dev-commands.test.mjs tests/integration/worktree-icon.test.mjson macOS at the clean exact head: 19 passed, 0 skipped. This includes actual Swift/AppKit generation with the 220-character label and successful ICNS decoding, plus combined launcher/config/fallback coverage. The follow-up changes only the renderer and its regression test; it is the small fix requested. - Mergeability is a separate gate, not a code-defect finding. #109 has merged, but #113 still carries its old stack. GitHub reports
CONFLICTING/DIRTY; read-only three-way merge probes against both bases report conflicts inscripts/desktop-dev.mjs,tests/integration/dev-commands.test.mjs, anddocs/contributing.md. Direct launcher comparison preserves the inherited port/runner behavior and adds only the intended icon integration. Rebase the two icon commits onto current main (or equivalently resolve the integration), preserve current-main documentation, update the stale stacking paragraph in this description, and recheck the resulting diff/head. Existing green checks do not validate a conflict resolution that has not happened. - Evidence and limits. All reported hosted checks are green, including DCO; all five commits currently listed against the PR base carry matching author sign-offs. The independent launcher re-review found no additional code regression. No app was launched or restarted for this review. Visual Dock/branch-rename acceptance and Windows launcher execution remain unverified; the macOS-specific tests are skipped in Linux CI. Earlier reported local stub-process timeouts remain unexplained, not disproved by this passing run.
This is a comment review, not approval or authorization to merge. After integration, only the resolved paths and updated head need re-review unless new evidence changes the scope.
|
Comment-readiness audit at
This pass changed only PR text; it did not resolve conflicts, rewrite commits, launch the app, approve, or merge. |
Signed-off-by: Carl <3e3d196dd9859e7da50eb419bfc7e219beb702c8730adc23dfec69f30d5064df@buzz.block.builderlab.xyz>
Signed-off-by: Carl <3e3d196dd9859e7da50eb419bfc7e219beb702c8730adc23dfec69f30d5064df@buzz.block.builderlab.xyz>
6642fab to
871e669
Compare
|
Resolved integration in
Description updated. Native visual acceptance remains unverified; no app was launched, and nothing was merged. |
What this does
Restore branch labels on the macOS Dock icon when running Buzz from a linked Git worktree. A detached worktree uses its folder name; ordinary checkouts and non-macOS launches keep their normal icons.
This is the separate Dock-label PR split from #111 (notification muting). #109 has merged, and this PR now targets
main. Integration resolved: the two Dock-label commits were rebased onto main atb49b6e3a03de6100cb7127c2ec785f276e480c65, dropping the old launcher stack. Both feature patches are unchanged by range-diff; current-main behavior and documentation are preserved.Why it matters
Parallel development copies should be distinguishable in the Dock. Renaming or switching a branch should update that label on the next launch, without cleaning the native build cache.
How it works
Builds on Brain/Wes's shared desktop launcher from #109 and restores Morgan's existing Swift badge artwork from the original #111 implementation. No competing startup wrapper or changes to the just recipes.
The optional macOS helper generates the icon in ignored staging output and names the final file from its contents. Changed artwork therefore changes Tauri's configuration and refreshes the embedded icon even with a warm Cargo build. Failed generation warns and falls back to the ordinary icon. Explicit user configuration still takes precedence; port forwarding, runner arguments, and the inherited package-manager/exit handling are preserved.
No notification, app identity, credential, profile, dependency, or production icon changes. Non-macOS startup does not invoke the generator, and this does not reintroduce the removed Bash-only package-manager path.
Verification
Integration head
871e6691edeb3c44ea0a4ad50379be506bcdd577: all 83 Node integration tests passed on macOS at this clean exact head (0 skipped), including the real Swift/AppKit/iconutil long-label case and combined launcher/config/fallback coverage. Independent integration review cleared this exact head. Main-relative diff remains the expected six Dock-label files; both commits retain valid DCO sign-offs. Normal pre-push hooks passed (no applicable unit/design inputs). All automatic hosted checks passed at this head, including DCO, security, JavaScript, Rust/tool integration, browser measurements, both Chromium/WebKit shards, and CI required (run). Windows validation was skipped as expected under current main’s manual-only policy; it was not newly exercised. GitHub reportsMERGEABLE/CLEANand an approval from wesbillman at this exact head. No app was launched or restarted during conflict resolution.Review follow-up at
6642faba557e4df6823e6b4bac0bedf3f3f2930b: long display labels no longer enter the Swift temporary-directory name; removed the now-unused sanitizer. One real-renderer regression reproduces the original trap and passes with the fix, including ICNS decoding. All 82 Node integration tests passed on the final pre-commit tree; all four icon tests passed again at the committed head. Independent review cleared the exact committed head; normal hooks and all hosted checks passed, including DCO, Rust/tool integration, JavaScript, Windows native notifications, browser measurements/journeys, and security checks. Two earlier local stub-process runs timed out during concurrent machine load; a diagnostic run and the complete suite passed without code or timeout changes. The timeout cause was not established.Initial implementation at
0397db5d33d3f103a8cd3228e964492dd6e9f00f:The initial head passed all hosted checks. At Morgan’s request,
just desktopsubsequently completed a real native build and launched the app from this worktree; Vite returned HTTP 200. This establishes launch, not visual acceptance. No app was restarted for the review fix.Deferred: visual Dock/branch-rename acceptance and Windows launcher execution. The inherited Windows subprocess path is unchanged, not newly certified here. The PR is currently marked ready for review; this agent did not change that state. Passing checks and approval do not substitute for the remaining native acceptance. No merge was performed. The macOS launcher and real-renderer tests run locally but are skipped on the Linux CI lane.
Originating Buzz conversation: channel
5e739a1f-b4cd-4c73-bb83-400fc3f28477, thread90bf281982d462b315d7b8ab254d11bdf55d51f928d56b5bd6ce9e559860c578.