Skip to content

Make Dark the default and sync icon choices with Finder - #170

Merged
LMC-Karma merged 4 commits into
mainfrom
feat/dark-default-icon
Oct 22, 2026
Merged

LMC-Karma merged 4 commits into
mainfrom
feat/dark-default-icon

Conversation

@LMC-Karma

@LMC-Karma LMC-Karma commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Summary

Use the existing Dark artwork as BetterTile's default icon and let the App icon picker update Finder as well as the Dock.

- Built-in app icon and missing/unknown preference: Classic
+ Built-in app icon and missing/unknown preference: Dark
- Picker selection: running Dock icon only
+ Picker selection: Dock icon + custom Finder icon

Preserve saved Classic, Ice Blue, and Dark choices. Use public NSWorkspace.setIcon for Finder. Selecting Dark removes the Finder override and restores the system-rendered primary icon. An explicit selection updates Finder even when the Dock icon is hidden; a saved choice is reapplied on launch. A fresh install leaves external custom icons alone. Settings reports failed Finder writes, including unwritable app locations. No privilege elevation or re-signing is used.

Evidence

  • Before: the picker changed the running Dock icon while Finder retained the primary icon.
    After: the opt-in native integration test changes a disposable signed app's effective Finder icon through the production updateFinderIcon method. Its signed Contents files remain identical; ordinary signature verification passes. While the override is active, strict verification must fail with the Finder-metadata diagnostic. Selecting Dark removes the override and restores strict verification.
  • swift test: 1,357 passed; 12 optional native/preview/benchmark tests skipped.
  • BETTERTILE_TEST_FINDER_ICON=1 swift test --filter finderIconChangesPreserveSignedContentsAndRestoreThePrimaryIcon: passed separately on macOS 27.0.1.
  • Unit tests call the real apply() method with injected preferences, Finder writes, and Dock updates. They cover fresh-install preservation, saved and unknown choices, explicit selections, Finder-before-Dock order, and failed Finder writes. Bypassing fresh-install protection made its regression test fail; restoring the protection passed. Existing tests cover default restoration, both alternate images, missing resources, failed writes, and invalid targets without modifying live apps.
  • The native test initially failed because Finder rendered the blue fixture with red = 0.261, above its absolute red < 0.1 threshold. The corrected assertion distinguishes the blue fixture by channel separation from the pale primary icon. The same native test then passed, including signed-content equality and signature checks. No retry or delay was added.
  • Debug and Release Xcode builds with CODE_SIGNING_ALLOWED=NO: passed. Their compiled Info.plists identify AppIconDarkDebug and AppIconDark as primary icons.
  • Earlier validation on ad-hoc-signed temporary copies of both compiled apps: all three actual Icon Composer assets change the effective Finder icon; ordinary signature verification passes after alternate selections; strict verification passes after restoring Dark. Release signed contents remain byte-for-byte unchanged. No installed app was changed or launched.
  • Added the missing strict-verification check while a custom icon is active. The process helper returns status and diagnostics so expected failure can be asserted. The opt-in native test passes; removing --strict from that check makes both its status and diagnostic assertions fail. Full suite and app builds above were run before this test/documentation-only follow-up; this follow-up reran the native test and mutation check.
  • git diff --check: passed.
  • Full Finder UI/cache behavior, live Dock picker interaction, a maintainer-signed update, Gatekeeper/notarization assessment, and read-only-folder behavior were not tested end to end. No release was produced.

Merge Danger

Door: two-way

Selecting Dark clears the runtime override. The default change can be reverted without migrating preferences or artwork.

Blast Radius: appearance

The new local metadata write is disclosed in SECURITY.md and the icon guide. Custom Finder icons add Icon\r resource data and Finder metadata at the app bundle root. Ordinary validation still passes with signed resources and identity unchanged. Strict verification rejects that metadata while a custom icon is active; clearing the icon restores strict verification. This is an explicit runtime limitation, not a relaxation of release checks.

No executable, Info.plist, compiled icon resource, signing identity, release validation, or updater policy is changed. Sparkle 2.9.5's verifier uses ordinary all-architecture signature validation, and its delta hash excludes custom-icon resource data. These source checks do not establish full update compatibility; a signed update remains a manual check. Finder overrides are static images rather than live Icon Composer layers. No dependency, network request, permission, or private API is added.

@claude

claude Bot commented Oct 21, 2026

Copy link
Copy Markdown

Review summary

Checked the diff against the PR body's stated goal/evidence and against AGENTS.md, docs/ARCHITECTURE.md, SECURITY.md, and CONTEXT.md.

  • AppIconStyle.defaultStyle (.dark) is threaded consistently through selected(in:), apply(_:), and both SettingsView fallbacks — no leftover .classic default.
  • project.pbxproj correctly swaps primary/alternate icon names for both Debug and Release with no duplicates or omissions.
  • AppIconStyleTests.swift covers the needed reverse transitions (default → ice → dark → unknown-falls-back-to-dark → explicit classic preserved), matching the "preserve saved choices" claim.
  • No window mutation, Core/MacOS/App layering, or coordinate-conversion rule is touched.
  • The NSWorkspace.setIcon probe is disclosed as an unshipped experiment (broke code signing), not an adopted private API, so SECURITY.md's private-API design-review process doesn't apply; no networking/permissions/dependency/update changes trigger the other disclosure rules.

No confirmed code fault found — the change matches its stated goal and the repository's architecture, security, and test rules. The PR body's own disclosed gaps (live Finder cache refresh, Dock picker interaction, update installation, no signed release) are manual checks explicitly called out as not run, not code faults.

@LMC-Karma LMC-Karma changed the title Make Dark the default app icon Make Dark the default and sync icon choices with Finder Oct 21, 2026
Comment thread Sources/BetterTileApp/BetterTileApp.swift Outdated
@claude

claude Bot commented Oct 21, 2026

Copy link
Copy Markdown

Review summary

Checked this against AGENTS.md, docs/ARCHITECTURE.md, SECURITY.md, and CONTEXT.md.

Architecture/security: No violations found. NSWorkspace.setIcon is public API, stays in BetterTileApp (no AppKit leaks into Core, no window mutation involved, so WindowCoordinator doesn't apply), adds no permission/dependency/network call, and the SECURITY.md disclosure accurately describes the signature-verification tradeoff (ordinary verification passes, strict verification rejects the Finder metadata, clearing the icon restores it).

Entry paths and reverse transitions traced: fresh install (no Finder touch), launch with a saved non-default choice (Finder reapplied), explicit picker selection with Dock icon hidden (Finder still updated), and selecting Dark to clear an override (restores primary icon) — all behave as the PR body describes, and are backed by either the unit tests or the opt-in native integration test.

Confirmed issue (1 inline comment): AppIconStyle.apply() contains the actual fresh-install-vs-reapply decision (the behavior behind the PR's "a fresh install leaves external custom icons alone" safety claim), but it hardcodes UserDefaults.standard and non-injectable closures/Bundle.main.bundleURL, so it can't be unit tested and isn't called from any test. This leaves the core state-transition logic unverified, which AGENTS.md calls out explicitly as required test coverage.

Not flagged (no code fault, for visibility only): the pre-existing applyDockPolicy() in BetterTileModel.swift (untouched by this diff) now has a new side effect — reapplying the Finder icon whenever "Show Dock icon" is turned back on — because apply()'s behavior changed under it. This is redundant-but-harmless, not a functional bug, so no inline comment was posted for it.

Comment thread Tests/BetterTileAppTests/AppIconFinderIntegrationTests.swift Outdated
@claude

claude Bot commented Oct 22, 2026

Copy link
Copy Markdown

Review summary

Checked the diff against the stated goal (Dark default, Finder-syncing picker, fresh-install preservation, failure reporting, no elevation/re-signing) and against the architecture, security, and test rules in AGENTS.md/SECURITY.md. The apply()/updateFinderIcon() logic, the pbxproj primary/alternate icon swaps for Debug and Release, the fresh-install and saved-choice entry paths, the Finder-before-Dock ordering, and the forward/reverse (non-default → Dark) transitions all match the documented behavior, and no AppKit import, private API, SIP workaround, or updater-API change is introduced.

One confirmed issue, left as an inline comment: the new AppIconFinderIntegrationTests.swift integration test never actually asserts that codesign --verify --deep --strict rejects the Finder metadata while a custom icon is active — the central safety claim repeated in SECURITY.md and the icon-composer doc. It only checks ordinary --deep verification after applying the custom icon, then jumps to restoring Dark and re-checking --strict. That claim is therefore currently unverified by any automated test and a future regression would pass silently.

No other confirmed faults found. I didn't attempt to verify the PR's claims about Sparkle 2.9.5's delta-hash/signature-verifier source behavior (external dependency source, not in this diff).

@claude

claude Bot commented Oct 22, 2026

Copy link
Copy Markdown

Review summary

No confirmed code faults. The diff matches the stated goal and the repository rules I checked it against.

Goal vs. diff

  • Default changed to Dark in both the pbxproj (ASSETCATALOG_COMPILER_APPICON_NAME/ALTERNATE_APPICON_NAMES for Debug and Release) and AppIconStyle.defaultStyle/selected(in:), matching the stated before/after.
  • AppIconStyle.apply writes Finder before Dock (matches the "Finder-before-Dock order" claim and avoids the Dock picking up a stale Finder-rendered icon when applicationIconImage is cleared).
  • Fresh-install preservation, saved-choice reapply on launch, explicit-selection-bypasses-fresh-install-guard, and Finder-write-failure reporting are each covered by a dedicated unit test (AppIconStyleTests.swift), and the reverse transition (selecting Dark clears the Finder override and restores strict codesign verification) is covered by the opt-in native test in AppIconFinderIntegrationTests.swift.
  • SettingsView's failure banner and select() behavior match the "saved choice remains available for Dock/next launch after a failed write" claim: selectedRawValue is persisted before apply() runs, so a failed Finder write doesn't roll back the preference.

Repository rules

  • Layering: everything is in BetterTileApp (AppKit-only); no BetterTileCore/BetterTileMacOS touched, no AppKit/Accessibility leaked into Core. Icon customization isn't a "window mutation" per CONTEXT.md, so the WindowCoordinator rule doesn't apply.
  • Security: only public NSWorkspace.setIcon / NSApplication.applicationIconImage are used; no private API, so the design-discussion/approval path in SECURITY.md isn't triggered. The new local Finder-metadata write and its strict-signature-verification trade-off are disclosed in SECURITY.md ("App icon customization") and cross-linked from docs/icon-composer/README.md, consistent with the change-disclosure rule in AGENTS.md.
  • Tests: geometry/state-transition/failure-path coverage is present for the new logic (apply, updateFinderIcon), for both forward and reverse transitions.

Not treated as a confirmed fault (noted for visibility, not flagged inline)

  • The PR body's claim about Sparkle 2.9.5 internals (SUCodeSigningVerifier validation scope, SUBinaryDeltaCommon delta-hash exclusion of custom-icon resource data) could not be independently checked from this checkout — Sparkle is a remote SwiftPM dependency, not vendored here. This is an unverified third-party claim, not a defect in the diff.
  • gh pr checks was blocked by this environment's permission settings, so CI status was not independently confirmed here; that is a tooling limitation, not a code issue.

@LMC-Karma
LMC-Karma merged commit f659944 into main Oct 22, 2026
4 checks passed
@LMC-Karma
LMC-Karma deleted the feat/dark-default-icon branch October 22, 2026 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant