Skip to content

Give each build lane its own bundle file name - #233

Merged
delano merged 3 commits into
mainfrom
feature/lane-bundle-names
Oct 2, 2026
Merged

delano merged 3 commits into
mainfrom
feature/lane-bundle-names

Conversation

@delano

@delano delano commented Oct 2, 2026

Copy link
Copy Markdown
Member

The local lane installed to /Applications/OnetimePad.app, the same
path a TestFlight install takes, so the two replaced each other and
TestFlight fell back to "OnetimePad 2.app". Only the App Store lane
keeps the plain name now: debug builds are "OnetimePad Debug.app" and
local installs are "OnetimePad Local.app". The names sit in
build-lanes.sh beside the bundle ids.

delano added 3 commits October 1, 2026 23:37
The local lane installed to /Applications/OnetimePad.app, the same
path a TestFlight install takes, so the two replaced each other and
TestFlight fell back to "OnetimePad 2.app". Only the App Store lane
keeps the plain name now: debug builds are "OnetimePad Debug.app" and
local installs are "OnetimePad Local.app". The names sit in
build-lanes.sh beside the bundle ids.
Earlier install.sh runs left the local lane at OnetimePad.app. That
bundle is removed only when it carries the local bundle id, so a
TestFlight copy at the same path stays.
@greptile-apps

greptile-apps Bot commented Oct 2, 2026

Copy link
Copy Markdown

Required label not found on this PR.

@claude

claude Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Claude finished @delano's task —— View job


Code review: lane-specific bundle names

  • Read the diff and the three scripts end to end
  • Check every remaining OnetimePad.app reference in the tree
  • Check CI, tests, and the docs sweep
  • Post review

The split is the right call and the constants land in the right place (scripts/build-lanes.sh:24-29, beside the ids). The CI quoting sweep looks complete, the digest file now derives from the bundle name (package-app.sh:488), quit-app.sh is keyed on the executable name so it is unaffected, LaunchAtLogin.pathMayRegister still passes on /Applications/OnetimePad Local.app (SettingsSections.swift:18-19), and docs/development/testflight-distribution.md was correctly left pointing at OnetimePad.app. Four things below I'd want changed before merge, then some follow-ups.

Worth fixing

1. migrate_legacy_bundle can delete the bundle it just installed. scripts/install.sh:156 passes $BUILD_BUNDLE_ID, which is only safe because install.sh hard-codes select_build_lane local (line 29). If this script ever selects the app-store lane, BUILD_APP_NAME becomes OnetimePad and $BUILD_BUNDLE_ID becomes the production id, so the call matches the freshly installed bundle on both the name and the id check and rm -rfs it. A one-line guard makes that structural rather than incidental:

migrate_legacy_bundle() { # <legacy app name> [required bundle id]
  local name="$1" id="${2:-}"
  [[ "$name" != "$BUILD_APP_NAME" ]] || return 0

or pass $LOCAL_BUNDLE_ID explicitly, which says "this lane's old name" rather than "whatever lane we are".

2. The migration runs after the install, so the ordinary upgrade path takes the risk. install.sh:153-156 is now quit-new, install, quit-old, remove-old. For an existing dogfooder the live copy is the one at /Applications/OnetimePad.app, so the new bundle is written first and only then is the running old copy asked to quit. If it refuses, quit_installed takes the exit 1 at line 103 and leaves two bundles sharing dev.onetimesecret.pad registered with LaunchServices, with nothing launched. Moving migrate_legacy_bundle "$PRODUCTION_APP_NAME" ... above install_bundle makes that refusal cost nothing, which is the same reasoning the APP_DEST guard states for itself at lines 51-53 ("refuse before any build work starts so a bad invocation costs nothing").

3. The id check cannot distinguish a mismatch from an unreadable plist. install.sh:144-147 sends plutil stderr to /dev/null, so a bundle with a missing or corrupt Info.plist silently takes the "leave it alone" path with no output at all. That is the safe direction, but the dogfooder is left with two OnetimePads in Launchpad and no explanation. One echo on the skip path ("leaving $legacy, its bundle id is not $id") would pay for itself.

4. Nothing cleans up the pre-rename artifacts. package-app.sh:324 only rm -rfs its own lane's path, so every existing checkout keeps dist/OnetimePad.app (carrying a dev or local id, still registered with LaunchServices) and a stale dist/OnetimePad.presig.sha256 indefinitely — and that stale digest is exactly the file SECURITY.md used to tell people to cat. Either remove the other lanes' paths when assembling, or add a line to the CHANGELOG entry telling people to rm -rf dist once.

Follow-ups

5. "Debug" and "Dev" now disagree. The debug bundle is OnetimePad Debug.app on disk, but package-app.sh:471-481 still writes CFBundleName/CFBundleDisplayName as OnetimePad Dev, so Finder, ⌘Tab, the Dock and the About panel say "Dev" while the file says "Debug". The comment at lines 476-480 argues precisely that a bundle whose names disagree "calls itself two things in one session"; this adds a third. Pick one word.

6. The local install is still indistinguishable where it matters most. The local lane keeps the plain CFBundleName, so in ⌘Tab, the Dock and the menu bar a local install and a TestFlight install both read "OnetimePad"; only the file name tells them apart. If the point is that the two copies coexist, a Local display name for that lane (or a note in build-lanes.sh on why the file name alone is enough) would finish the job.

7. The rename breaks an existing login item, silently. SMAppService.mainApp registered the old /Applications/OnetimePad.app; removing that bundle leaves the registration pointing at a path that no longer exists, and the new bundle shares the id but not the path. Worth a line in docs/dogfood/DOGFOOD.md to re-toggle Launch at Login after this upgrade, alongside the expectation of fresh Accessibility / Screen Recording prompts for the moved bundle.

8. dev.sh sources a lane library without selecting a lane. scripts/dev.sh:39 sources build-lanes.sh purely to read DEV_APP_NAME at line 91. It works because the names are top-level, and set -u would catch a regression loudly, but it quietly depends on DEV_APP_NAME never moving into the case block the way BUILD_APP_NAME did. Either select_build_lane dev (it is side-effect-free here beyond unsetting inherited signing values, which package-app.sh redoes anyway) or a comment saying the source is deliberately partial.

9. One stale copy of the updated command. crates/ffi/src/diagnostics.rs:9 still says /Applications/OnetimePad.app/Contents/MacOS/OnetimePad — the same invocation DOGFOOD.md:328-332 just moved to OnetimePad Local.app. (docs/adr/* and sandbox-file-access.md:240 are correctly untouched: the first is a record, the second is a TestFlight procedure.)

10. Prose wrapping. The insertions left orphan half-lines rather than reflowed paragraphs: install.sh:3-5 ("…rather than / # over it. The installed copy / # runs from…"), dev.sh:2-4 ("# takes its own bundle id"), and DOGFOOD.md:17-20, where the line ending "Running the installed" runs long and the migration note now sits between "installs it to /Applications" and "copy matters for two reasons", so the antecedent is three sentences away.

11. SECURITY.md:23-25 names only the local and App Store digests, though the debug lane writes one too and CI publishes it. Fine if deliberate.

Verification

I could not run scripts/tests/test_build_lanes.py or shellcheck here (python3 and shellcheck are outside my allowed tools), so the updated SELECTION expectations are reviewed by reading only; the seven-field printf and the three expected rows line up with select_build_lane.
· feature/lane-bundle-names

@delano
delano merged commit aa672a8 into main Oct 2, 2026
7 checks passed
@delano
delano deleted the feature/lane-bundle-names branch October 2, 2026 06:43
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