diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f10daa3fae6..c7d1c5de8f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1215,6 +1215,12 @@ jobs: test -x "$app/Contents/PlugIns/OpenCodexWidget.appex/Contents/MacOS/OpenCodexWidget" test -x "$app/Contents/MacOS/ocx" codesign -dv "$app/Contents/PlugIns/OpenCodexWidget.appex" + # The widget is only offered in the gallery when its bundle is actually linked in, and + # nothing else here would notice its absence: the appex builds, signs and registers + # exactly the same way with the WidgetBundle dropped by the linker. + nm -a "$app/Contents/PlugIns/OpenCodexWidget.appex/Contents/MacOS/OpenCodexWidget" \ + | grep -q "OpenCodexWidget0abC6BundleV" \ + || { echo "::error::the widget bundle is not linked into the extension"; exit 1; } desktop-shell: name: desktop shell diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index dd63e98a51e..5a10187cb5a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -236,10 +236,89 @@ jobs: if: runner.os != 'macOS' run: bun desktop/scripts/prepare-sidecar.ts --target ${{ matrix.sidecar-targets }} + # The signing certificate has to be in a keychain before the widget is signed, and the + # Tauri build step creates its own keychain only when it runs — which is after this. Until + # this step existed, build-widget.sh saw no MACOS_SIGN_IDENTITY and took its unsigned + # branch, and the bundler does not re-sign anything under PlugIns, so the extension would + # have gone out ad-hoc inside a Developer ID host. No release has published a macOS + # application yet, so this is a defect that had not reached anyone rather than one that had. + - name: Import the release signing certificate + if: runner.os == 'macOS' + env: + APPLE_CERTIFICATE: ${{ secrets.APPLE_CERTIFICATE }} + APPLE_CERTIFICATE_PASSWORD: ${{ secrets.APPLE_CERTIFICATE_PASSWORD }} + APPLE_ID: ${{ secrets.APPLE_ID }} + APPLE_PASSWORD: ${{ secrets.APPLE_PASSWORD }} + APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + set -euo pipefail + # Checked as a set, because a partial set is the dangerous case: the Tauri CLI skips + # notarization without failing when the notary credentials are missing, and the + # unnotarized artifact is uploaded and attached exactly as a good one would be. + missing="" + for name in APPLE_CERTIFICATE APPLE_CERTIFICATE_PASSWORD APPLE_ID APPLE_PASSWORD APPLE_TEAM_ID; do + eval "value=\${$name:-}" + [ -n "$value" ] || missing="$missing $name" + done + if [ -n "$missing" ]; then + if [ "${DRY_RUN}" != "true" ]; then + echo "::error::A real release needs the full signing and notarization credential set." + echo "::error::Missing:$missing" + exit 1 + fi + echo "Signing credentials are incomplete, so this build stays ad-hoc signed:$missing" + echo "It is usable for local validation and is not a release asset." + exit 0 + fi + keychain="$RUNNER_TEMP/opencodex-signing.keychain-db" + # Recorded before anything is created, so the cleanup step can still find a keychain + # that a failure left half-built. + echo "OPENCODEX_SIGNING_KEYCHAIN=$keychain" >> "$GITHUB_ENV" + keychain_password="$(python3 -c 'import secrets; print(secrets.token_urlsafe(32))')" + certificate="$RUNNER_TEMP/opencodex-signing.p12" + # The decoded certificate must not outlive this step even when a later command fails. + trap 'shred -u "$certificate" 2>/dev/null || rm -Pf "$certificate" 2>/dev/null || true' EXIT + printf '%s' "$APPLE_CERTIFICATE" | base64 --decode > "$certificate" + security create-keychain -p "$keychain_password" "$keychain" + security set-keychain-settings -lut 21600 "$keychain" + security unlock-keychain -p "$keychain_password" "$keychain" + security import "$certificate" -k "$keychain" -P "$APPLE_CERTIFICATE_PASSWORD" \ + -T /usr/bin/codesign + security set-key-partition-list -S apple-tool:,apple:,codesign: \ + -s -k "$keychain_password" "$keychain" > /dev/null + # shellcheck disable=SC2046 # the keychain list is intentionally word-split into arguments + security list-keychain -d user -s "$keychain" $(security list-keychains -d user | tr -d '"') + - name: Build WidgetKit extension if: runner.os == 'macOS' + env: + MACOS_SIGN_IDENTITY: ${{ secrets.APPLE_SIGNING_IDENTITY }} run: bash desktop/scripts/build-widget.sh + - name: Verify the extension carries the release signature + if: runner.os == 'macOS' + env: + APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + set -euo pipefail + appex=desktop/src-tauri/widget/OpenCodexWidget.appex + if [ -z "${APPLE_TEAM_ID}" ]; then + if [ "${DRY_RUN}" != "true" ]; then + echo "::error::A real release cannot assert its own signature without APPLE_TEAM_ID." + exit 1 + fi + echo "No team configured; skipping the signature assertion for this non-release build." + exit 0 + fi + codesign --verify --strict --deep "$appex" + description="$(codesign -dvvv "$appex" 2>&1)" + echo "$description" + echo "$description" | grep -q "TeamIdentifier=$APPLE_TEAM_ID" + echo "$description" | grep -q "flags=.*runtime" + echo "$description" | grep -q "Timestamp=" + # Release signing is intentionally secret-gated. Developer ID, notarization, # and updater signatures require maintainer-owned credentials; builds without # those secrets remain useful for local validation but are not release assets. @@ -267,6 +346,55 @@ jobs: --target "$DESKTOP_TARGET" \ --out dist/release + # After the bundle exists, not before: a sweep that runs first passes by finding nothing. + - name: Verify every Mach-O in the bundle carries the release identity + if: runner.os == 'macOS' + env: + APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + set -euo pipefail + if [ -z "${APPLE_TEAM_ID}" ]; then + if [ "${DRY_RUN}" != "true" ]; then + echo "::error::A real release cannot verify its bundle without APPLE_TEAM_ID." + exit 1 + fi + echo "No team configured; skipping the bundle-wide assertion for this local build." + exit 0 + fi + # Executables are found by their magic bytes rather than by path or extension. A bundler + # signs what it placed; anything copied in afterwards is invisible to it, and the + # binaries that get missed are the ones with no extension to filter on. + apps=0 + machos=0 + bad=0 + while IFS= read -r app; do + apps=$((apps + 1)) + echo "checking $app" + while IFS= read -r -d '' file; do + # All eight Mach-O leading words: thin and fat, 32- and 64-bit, both byte orders. + # A list that covers only the common ones skips the rest in silence while the + # non-zero counter below still reports a healthy sweep. + case "$(head -c 4 "$file" | xxd -p)" in + cefaedfe|cffaedfe|feedface|feedfacf) ;; + cafebabe|bebafeca|cafebabf|bfbafeca) ;; + *) continue ;; + esac + machos=$((machos + 1)) + if ! codesign -dvvv "$file" 2>&1 | grep -q "TeamIdentifier=$APPLE_TEAM_ID"; then + echo "::error::$file is not signed with the release identity" + bad=1 + fi + done < <(find "$app" -type f -print0) + done < <(find desktop/src-tauri/target -maxdepth 6 -type d -name '*.app') + echo "inspected $machos Mach-O files across $apps app bundles" + # A sweep that inspected nothing is the failure mode this step exists to prevent. + if [ "$apps" -eq 0 ] || [ "$machos" -eq 0 ]; then + echo "::error::found $apps app bundles and $machos Mach-O files; the sweep inspected nothing" + exit 1 + fi + exit "$bad" + - name: Upload desktop release uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: @@ -275,6 +403,15 @@ jobs: if-no-files-found: error retention-days: 7 + # always(), because a keychain holding the release identity must not survive a failed job + # on a runner image that could be reused. + - name: Remove the signing keychain + if: always() && runner.os == 'macOS' + run: | + if [ -n "${OPENCODEX_SIGNING_KEYCHAIN:-}" ] && [ -f "${OPENCODEX_SIGNING_KEYCHAIN}" ]; then + security delete-keychain "${OPENCODEX_SIGNING_KEYCHAIN}" + fi + attach-release: runs-on: ubuntu-latest needs: [publish, package-standalone, package-desktop] diff --git a/app/Package.swift b/app/Package.swift index 0e14e3bd5b8..a741d5bfd12 100644 --- a/app/Package.swift +++ b/app/Package.swift @@ -3,7 +3,7 @@ import PackageDescription let package = Package( name: "OpenCodexWidget", - platforms: [.macOS(.v13)], + platforms: [.macOS(.v14)], products: [ .executable(name: "OpenCodexWidget", targets: ["OpenCodexWidget"]), .executable(name: "MenuBarCoreTests", targets: ["MenuBarCoreTests"]), @@ -14,9 +14,32 @@ let package = Package( name: "OpenCodexWidget", dependencies: ["MenuBarCore"], path: "Sources/OpenCodexWidget", + swiftSettings: [ + // Xcode sets APPLICATION_EXTENSION_API_ONLY on an app-extension target, and the + // two projects that have this working from SwiftPM pass its compiler spelling by + // hand. It restricts the target to the extension-safe API surface, which is the + // contract the extension host assumes it was built against. + .unsafeFlags(["-application-extension"]), + ], linkerSettings: [ - // Widget extensions must enter through NSExtensionMain or chronod tears down - // the process before the WidgetBundle connects. + // A widget extension needs both halves of what Xcode does for an app-extension + // target, and each half is useless alone. This flag is one of them; `@main` on + // OpenCodexWidgetBundle is the other. + // + // With the entry override and no `@main`, nothing references the WidgetBundle, the + // linker drops it, and the extension registers with pluginkit — the Info.plist is + // enough for that — while the gallery has no configuration to offer. That is what + // shipped, and it failed silently. + // + // With `@main` and no entry override, the Swift main runs instead of + // NSExtensionMain, and ExtensionFoundation traps inside + // _EXRunningExtension._shared while bootstrapping. Measured: EXC_BREAKPOINT on + // every launch, chronod logging "query failed - will try lazy reload later", and + // a crash report per attempt. + // + // Both together is the shape that works and the shape Xcode produces: the entry + // is NSExtensionMain, and the bundle stays in the binary because `@main` refers + // to it. .linkedFramework("Foundation"), .unsafeFlags(["-Xlinker", "-e", "-Xlinker", "_NSExtensionMain"]), ] diff --git a/app/Sources/OpenCodexWidget/Provider.swift b/app/Sources/OpenCodexWidget/Provider.swift index 55d983bf5cf..e23d55f6042 100644 --- a/app/Sources/OpenCodexWidget/Provider.swift +++ b/app/Sources/OpenCodexWidget/Provider.swift @@ -2,7 +2,6 @@ import Foundation import WidgetKit import MenuBarCore -@available(macOS 14, *) public struct SnapshotEntry: TimelineEntry { public let date: Date public let snapshot: WidgetSnapshot? @@ -10,7 +9,6 @@ public struct SnapshotEntry: TimelineEntry { public let stale: Bool } -@available(macOS 14, *) public struct SnapshotProvider: TimelineProvider { private let reader = SnapshotReader() diff --git a/app/Sources/OpenCodexWidget/Views.swift b/app/Sources/OpenCodexWidget/Views.swift index edc91171f44..7b8d6fc346c 100644 --- a/app/Sources/OpenCodexWidget/Views.swift +++ b/app/Sources/OpenCodexWidget/Views.swift @@ -2,7 +2,6 @@ import SwiftUI import WidgetKit import MenuBarCore -@available(macOS 14, *) struct OpenCodexWidgetView: View { let entry: SnapshotEntry @Environment(\.widgetFamily) private var family @@ -301,14 +300,13 @@ struct OpenCodexWidgetView: View { } } -@available(macOS 14, *) +@main struct OpenCodexWidgetBundle: WidgetBundle { var body: some Widget { OpenCodexWidget() } } -@available(macOS 14, *) struct OpenCodexWidget: Widget { let kind = "OpenCodexWidget" diff --git a/app/Sources/OpenCodexWidget/main.swift b/app/Sources/OpenCodexWidget/main.swift deleted file mode 100644 index 7eeda563867..00000000000 --- a/app/Sources/OpenCodexWidget/main.swift +++ /dev/null @@ -1,2 +0,0 @@ -// WidgetKit enters through _NSExtensionMain; this file keeps the executable target's -// source directory populated without adding a competing Swift-generated main. diff --git a/app/Widget-Info.plist b/app/Widget-Info.plist index 568e5fce750..df499104290 100644 --- a/app/Widget-Info.plist +++ b/app/Widget-Info.plist @@ -7,6 +7,15 @@ CFBundleIdentifiercom.opencodex.desktop.widget CFBundleInfoDictionaryVersion6.0 CFBundleNameOpenCodex + CFBundleDisplayNameOpenCodex + + CFBundleSupportedPlatforms + MacOSX CFBundlePackageTypeXPC! CFBundleShortVersionString0.0.0 CFBundleVersion0.0.0 diff --git a/desktop/scripts/build-widget.sh b/desktop/scripts/build-widget.sh index 59827dcea2a..fb24526e477 100755 --- a/desktop/scripts/build-widget.sh +++ b/desktop/scripts/build-widget.sh @@ -66,8 +66,10 @@ plutil -replace CFBundleShortVersionString -string "$version_core" "$output_dir/ plutil -replace CFBundleVersion -string "$version_core" "$output_dir/Contents/Info.plist" if [[ -n "${MACOS_SIGN_IDENTITY:-}" ]]; then + # Hardened runtime and a secure timestamp are both required for notarized Developer ID + # software, and an extension that lacks either fails notarization with the host around it. codesign --force --sign "$MACOS_SIGN_IDENTITY" --entitlements "$package_dir/Widget.entitlements" \ - --timestamp "$output_dir" + --options runtime --timestamp "$output_dir" else codesign --force --sign - --entitlements "$package_dir/Widget.entitlements" \ --timestamp=none "$output_dir" diff --git a/desktop/src-tauri/src/first_run.rs b/desktop/src-tauri/src/first_run.rs new file mode 100644 index 00000000000..e6a0417ed97 --- /dev/null +++ b/desktop/src-tauri/src/first_run.rs @@ -0,0 +1,41 @@ +use std::fs; +use tauri::{AppHandle, Manager}; +use tauri_plugin_autostart::ManagerExt; + +/// Marker file recording that the one-time Start at Login default has already been applied. +const MARKER: &str = "start-at-login-claimed"; + +/// Turn Start at Login on once, the first time this installation runs. +/// +/// A menu bar app that is not running has no menu bar item. Leaving autostart off by default +/// therefore means that after the next reboot an installed app is simply absent, with nothing on +/// screen to explain why — which is not a neutral default for an app whose main surface *is* the +/// menu bar. +/// +/// This runs exactly once per installation. The marker is written **before** the login item is +/// touched, and is never removed, so a user who turns Start at Login back off keeps it off: the +/// next launch sees the marker and does nothing. Writing afterwards instead would mean that a +/// failed or partial enable retries on every launch, and would eventually flip the setting back on +/// under a user who had deliberately turned it off in between. +/// +/// Every failure is silent on purpose. Not being able to write a marker or register a login item +/// is not a reason to stop the app from starting, and the user can still toggle the menu item. +pub fn apply_start_at_login_default(app: &AppHandle) { + let Ok(dir) = app.path().app_config_dir() else { + return; + }; + let marker = dir.join(MARKER); + if marker.exists() { + return; + } + if fs::create_dir_all(&dir).is_err() { + return; + } + if fs::write(&marker, b"").is_err() { + return; + } + if app.autolaunch().is_enabled().unwrap_or(false) { + return; + } + let _ = app.autolaunch().enable(); +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index aa688daca9f..5ee02473c7e 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -1,5 +1,6 @@ mod auth; mod discovery; +mod first_run; mod formatting; mod logging; mod proxy; @@ -101,6 +102,9 @@ pub fn run() { if tauri::async_runtime::block_on(proxy.is_alive()).is_ok() { let _ = window.eval(format!("window.location.replace({dashboard:?})")); } + // Before the tray, so its Start at Login checkbox reads the state this leaves behind + // rather than the state from before first run. + first_run::apply_start_at_login_default(app.handle()); tray::install(app.handle(), proxy)?; if !cfg!(debug_assertions) { updater::start_background_checks(app.handle().clone()); diff --git a/devlog/_plan/260920_desktop_app_stabilization/040_widget_never_offered.md b/devlog/_plan/260920_desktop_app_stabilization/040_widget_never_offered.md new file mode 100644 index 00000000000..062fbfa4d27 --- /dev/null +++ b/devlog/_plan/260920_desktop_app_stabilization/040_widget_never_offered.md @@ -0,0 +1,215 @@ +# wp5 — the widget registered, and offered nothing + +## The symptom, and why it was not a signing problem + +The extension installs, `pluginkit` lists it beside the system widgets, and the gallery does not +show it. The obvious reading was signing: a locally built host is ad-hoc signed, so of course the +system will not adopt its extension. That reading was wrong, and following it would have produced +a signing change that fixed nothing, because the released build is signed and notarized and the +widget is missing there too. + +The actual defect is in the binary. `app/Package.swift` forced the executable's entry point: + +```swift +.unsafeFlags(["-Xlinker", "-e", "-Xlinker", "_NSExtensionMain"]), +``` + +and `app/Sources/OpenCodexWidget/main.swift` held nothing but a comment explaining that the entry +was handled by that flag. So `OpenCodexWidgetBundle` — which `Views.swift` defines correctly, +with a display name, a description and three supported families — was never referenced by +anything, and no code ever handed it to the extension host. + +Read off the shipped bundle: + +``` +LC_MAIN entryoff -> _NSExtensionMain +nm: SnapshotProvider present, OpenCodexWidgetBundle absent +Info.plist: NSExtensionPointIdentifier = com.apple.widgetkit-extension + NSExtensionPrincipalClass = (absent) +``` + +That combination is exactly consistent with the symptom. `pluginkit` registers from the +Info.plist, which is complete, so registration succeeds. `NSExtensionMain` then looks for an +`NSExtensionPrincipalClass`, which a SwiftUI widget does not declare because Xcode's `@main` on +the `WidgetBundle` is what connects it instead. Nothing errors. The gallery simply has no +configuration to offer. + +## The fix, and the wrong turn on the way to it + +The first attempt was to delete the linker override and call the bundle from `main.swift`. That +made the bundle's symbols appear in the binary and did not work either — it replaced a silent +failure with a loud one. Every launch died: + +``` +EXC_BREAKPOINT (SIGTRAP) + ExtensionFoundation closure #1 in ... _EXRunningExtension._shared + ExtensionFoundation MainActor.assumeIsolated + ExtensionFoundation _EXExtension.bootstrap(with:) + WidgetKit + OpenCodexWidget main +chronod: [com.opencodex.desktop::com.opencodex.desktop.widget] query failed - will try lazy + reload later +``` + +Seventeen crash reports accumulated in `~/Library/Logs/DiagnosticReports` while the gallery stayed +empty, because `chronod` asks the extension for its descriptors and the extension never survives +long enough to answer. + +**The extension needs both halves of what Xcode does, and each is useless alone.** `@main` on the +`WidgetBundle` is what keeps it in the binary; `-e _NSExtensionMain` is what makes the process +start as an extension rather than as a program. The original code had the second without the +first, this branch briefly had the first without the second, and only both together produce a +widget the system will talk to. With both in place the crash reports stop at zero and `chronod` +processes the extension normally. + +`tests/clients/desktop-widget-entry.test.ts` asserts both, plus that no `main.swift` has come back +to compete with `@main`, and that the bundle carries a widget with a display name rather than an +empty body — the same failure by a third route. + +The deployment target moved to macOS 14 at the same time, which drops the per-declaration +`@available(macOS 14, *)` guards and puts the binary's `minos` at 14.0, matching every working +widget on the machine this was measured on. + +## The sandbox is not optional + +While narrowing this down, the extension was rebuilt without `com.apple.security.app-sandbox` to +test whether the sandbox was implicated. It is required, and the system says so plainly: + +``` +pkd: Ignoring mis-configured plugin at [.../OpenCodexWidget.appex]: plug-ins must be sandboxed +``` + +An unsandboxed extension is not rejected at launch — it is never registered at all, so it vanishes +from `pluginkit` entirely. That also settles the snapshot path: the host writes into +`~/Library/Containers/com.opencodex.desktop.widget/Data/...` precisely because the extension reads +its own container, and that arrangement has to stay. + +## What the public record says about this failure + +The `_EXRunningExtension` crash is not unique to this repository, and finding the precedent +changed how much of the fix is guesswork. A forensic report on macOS 26.5 with Swift 6.3.2 +describes the same trap from the same cause — a widget extension assembled from a SwiftPM +`.executableTarget` and wrapped into an `.appex` by hand — and records that neither Info.plist +shape avoids it, because SwiftPM has no app-extension target and therefore never applies the +entry-point setup Xcode's WidgetKit template provides. That project's resolution was to stop +using SwiftPM for the extension and build a real Xcode app-extension target instead. + +Two other projects keep SwiftPM and supply the missing pieces by hand, which is the route taken +here: the linker entry (`-Xlinker -e -Xlinker _NSExtensionMain`) and the compiler's +extension-only mode (`-application-extension`, which is what Xcode spells +`APPLICATION_EXTENSION_API_ONLY`). Both are now set, and the extension launches and answers +`chronod` without a crash report. + +Three things the same record settles that were open questions here: + +- **Ad-hoc signing does not prevent gallery appearance.** Developer ID and notarization matter for + Gatekeeper, not for gallery mechanics. The containing app does have to be launched once after + installation, which is what makes the first-run behaviour in this branch load-bearing for more + than the menu bar. +- **App Groups do not work under ad-hoc signing**, and the documented fallback is exactly what + this repository already does — the host writes into the extension's own container. +- **`CFBundleVersion` must match between host and extension** or WidgetKit rejects timeline + reloads. Verified on the installed bundle: both read 2.61.0. + +If the gallery still refuses this extension after the entry point and the extension-only build, +the remaining known cause is the Xcode app-extension target itself, and that is a larger change +than this unit: it means adding an Xcode project for the widget and building it with +`xcodebuild` rather than `swift build`. + +## The signing defect underneath it + +Fixing the entry point does not make a *released* widget adoptable on someone else's machine, +because the release pipeline would not sign it. + +`.github/workflows/release.yml` ran `build-widget.sh` with no `env:` block. `MACOS_SIGN_IDENTITY` +was set one step later, on the Tauri build, which never reads it. So the script took its +`codesign --force --sign -` branch, and the bundler does not re-sign anything under `PlugIns/` — +its nested-code walker handles `.framework`, `.xpc` and `.app`, not `.appex`. + +**This has not harmed a release yet, and the reason matters.** No release has ever published a +macOS application: the last three carry no desktop assets at all, and the signing secrets did not +exist until after the most recent one was cut. `MACOS_SIGN_IDENTITY` reads a secret that was not +there, so the real-signing branch has never executed and the Developer ID path in the Tauri step +has never executed either. The bug is a mine rather than a crater — the next release is the first +one that would step on it. Saying otherwise would be inventing a history this repository does not +have. + +**Signing one path is also not enough.** A bundler that did not place a file does not sign it, and +picking binaries by file extension misses the ones that have none. The durable form of the check +is to find Mach-O files by their magic bytes and require every one of them to carry the release +identity, rather than naming the paths that are expected to exist. + +Three changes: + +- The certificate is imported into a temporary keychain in a step **before** the widget build, and + the keychain is deleted in an `always()` step so it cannot outlive a failed job. +- The widget build receives `MACOS_SIGN_IDENTITY`, and `build-widget.sh` now signs with + `--options runtime` as well as `--timestamp`, both of which notarization requires. +- A step after the widget build asserts the result rather than printing it: strict verification, + the configured team identifier, the runtime flag, and a secure timestamp. Without a configured + team it says so and skips, so a fork's build still works and still cannot pretend to be signed. + +This half cannot be proven here. It needs maintainer-held credentials, and the proof is a +notarized artifact installed on a machine that did not build it, launched once, with the gallery +then checked. That is recorded as the outstanding verification rather than claimed. + +## The menu bar had the same shape of problem + +Start at Login was purely opt-in. Nothing enabled it on first run, so an install left the user +with a menu bar item only for as long as the app happened to be running — and a menu bar app that +is not running has no menu bar item. After a reboot the app was simply absent. + +`first_run::apply_start_at_login_default` enables it once per installation, keyed on a marker in +the app config directory, and runs before `tray::install` so the tray checkbox reads the state it +leaves behind. The marker is written before the login item is touched and is never removed, so a +user who turns the setting off keeps it off. Writing afterwards would let a failed enable retry +every launch and eventually flip the setting back under someone who had deliberately disabled it. + +The marker distinguishes a fresh install from a user who opted out, but it cannot distinguish +either from an install that predates the marker. The desktop shell and the widget both landed the +same day this was written and no release tag contains them, so there is no such population; if +that changes, this needs a migration rather than a marker. + +## What was verified here + +Rebuilt, installed to `/Applications`, and launched: + +``` +LC_MAIN entryoff 5656 -> _main (was _NSExtensionMain) +nm: _$s15OpenCodexWidget0abC6BundleV4bodyQrvpQOMQ present +pluginkit: com.opencodex.desktop.widget re-registered, parent bundle resolved +~/Library/Application Support/com.opencodex.desktop/start-at-login-claimed written +~/Library/LaunchAgents/OpenCodex.plist created +``` + +So the entry point is connected and the login item is registered, both on a real install rather +than in a test double. + +## Verdict: it appears + +The gallery was opened on this machine after the fix and OpenCodex is in it, between OKX and +PASS, with all three declared families rendering real data rather than placeholders: + +``` +com.opencodex.desktop::com.opencodex.desktop.widget:OpenCodexWidget:systemSmall +com.opencodex.desktop::com.opencodex.desktop.widget:OpenCodexWidget:systemMedium +com.opencodex.desktop::com.opencodex.desktop.widget:OpenCodexWidget:systemLarge + "OpenCodex — Proxy status, today's usage, and quota at a glance." +``` + +Small shows the token count for the day, medium adds requests, cost and the account quota rows, +large adds the 24-hour per-model timeline. The list icon is the mark generated from `icon.svg`. + +That settles the whole question the acceptance note left open, and it settles it the right way +round: the extension was never rejected by signing or by the sandbox. It had no widget in it, and +then it had one that could not start. Both are fixed, and the fix is a SwiftPM configuration +rather than the Xcode app-extension target the public record recommends — so the cheaper route +does work, provided all three of `@main`, the `_NSExtensionMain` entry and +`-application-extension` are present. + +## Acceptance + +The entry-point half is closed by the gallery observation above. The signing half closes when a +release build's extension reports the team identifier, the runtime flag and a timestamp, and a +clean install on a machine that did not build it offers the widget. That second half needs a real +release and is recorded as outstanding. diff --git a/devlog/_plan/260920_desktop_app_stabilization/050_landing.md b/devlog/_plan/260920_desktop_app_stabilization/050_landing.md new file mode 100644 index 00000000000..4e6135b9d70 --- /dev/null +++ b/devlog/_plan/260920_desktop_app_stabilization/050_landing.md @@ -0,0 +1,66 @@ +# wp5 — landing the stack + +## Shape + +Four pull requests, each based on the one below it, all ultimately targeting `dev`: + +| PR | branch | what it carries | +|---|---|---| +| #5327 | `codex/260920-app-stabilization` | release profile, stale-dist report, `build:local`, the lockfile and test-layout repairs | +| #5328 | `codex/260920-claude-desktop-mode-visibility` | the first-party reachability message | +| #5329 | `codex/260920-app-icons` | one SVG source, the generator, the renderer-free CI guard | +| #5339 | `codex/260920-widget-entry` | the widget entry point, the login-item default, release signing | + +They merge bottom-up. After each one lands, the next is retargeted to `dev` and its exact head is +read again, because a squash merge rewrites the parent and the child's base disappears. + +## Two repairs in here are not ours + +`dev` was already red when this stack was cut, in two independent places, and both were fixed +here because every branch cut from `dev` inherits them. + +`tests/providers/stepfun-provider.test.ts` landed with no entry in either inventory and no regex +seed that resolves its name, so the membership oracle failed on `dev` and on everything branched +from it. Registering it under `providers` restores the gate for everyone. + +`macos widget + bundle` failed with *A public key has been found, but no private key*. The job is +an unsigned build by design, so the key is correctly absent — but the committed config sets +`bundle.createUpdaterArtifacts` and `plugins.updater.pubkey`, so `tauri build` writes the updater +archive and then refuses to finish. That half is #5338's, which turns the artifact off for that one +invocation; this stack does not duplicate it. + +Fixing the build revealed the rest of the job, which had never run. Its first assertion looked for +`Contents/MacOS/OpenCodex` — `productName` — while the bundle carries `opencodex-desktop`, the +crate name. That half landed separately as #5351, and better than the version written here: it +reads `CFBundleExecutable` out of the bundle instead of restating the name, so the check follows +the config rather than drifting from it. This stack's copy was dropped in favour of it. + +What remains here is the assertion with no equivalent: that the WidgetBundle is actually linked +into the extension. The appex builds, signs and registers identically with the bundle dropped by +the linker, so nothing else in this job would have noticed the defect that shipped. + +Three of this stack's incidental repairs turned out to be running in parallel with the +maintainer's own: the StepFun layout registration (#5335), the widget job's updater override +(#5338), and this executable assertion (#5351). Each was dropped here once the other landed. The +pattern is worth noting for the next batch — a repair found while passing through is worth +checking against open pull requests before it is written. + +## What closes this + +Each merge reads the exact head's check runs rather than a rollup, distinguishes a job the event +requested from one it skipped, and treats a missing, skipped, or cancelled job as not a pass. The +last merge is followed by reading `dev`'s own push run, because five of the eight defects found in +this unit were invisible until two changes met. + +## Deliberately not changed here + +Review asked for the public macOS install guidance to move with the release path, since +`README.md`, `guides/desktop-app.md` and `guides/macos-menu-bar.md` all tell the reader the app is +ad-hoc signed and not notarized, while this stack makes a real release refuse to run without a +Developer ID and the full notarization credential set. + +Those pages are accurate today and will stop being accurate at the next release, not at this +merge. No release has ever published a macOS application, so rewriting them now would describe an +artifact nobody can download and would leave the Gatekeeper walkthrough — still correct for a +locally built app — reading as though it were obsolete. The pages move with the first notarized +artifact, which is also when someone can check the instructions against a real download. diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index a8d43bc015c..d928c6dd1fd 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -715,7 +715,9 @@ "desktop-3p.test.ts": "clients", "desktop-app-restart.test.ts": "clients", "desktop-profile.test.ts": "clients", + "desktop-widget-entry.test.ts": "clients", "desktop-remote-store.test.ts": "clients", + "desktop-start-at-login-default.test.ts": "clients", "destination-policy-resolved.test.ts": "routing", "devin-adapter.test.ts": "providers", "devin-cli-authmode-migration.test.ts": "providers", diff --git a/structure/desktop-shell.md b/structure/desktop-shell.md index ac1cb85b322..cb5768a0c92 100644 --- a/structure/desktop-shell.md +++ b/structure/desktop-shell.md @@ -11,6 +11,27 @@ navigates the webview to the proxy's loopback dashboard Only the bootstrap page has Tauri IPC capability; the loopback dashboard never does because `dangerousRemoteDomainIpcAccess` is not configured. +`desktop/src-tauri/src/first_run.rs` turns Start at Login on once per installation, +before the tray is built so its checkbox reads the resulting state. A menu bar app +that is not running has no menu bar item, so leaving autostart off by default left an +installed app absent after a reboot. The marker in the app config directory is written +before the login item is touched and is never removed, so a user who turns the setting +off keeps it off; writing it afterwards would let a failed enable retry on every launch. +The behaviour is not macOS-only — the autostart plugin implements the Linux autostart +entry and the current-user Windows Run registration too. + +The WidgetKit extension in `app/` needs three things that Xcode's app-extension target +would supply on its own, and SwiftPM has no such target: `@main` on +`OpenCodexWidgetBundle`, the `-e _NSExtensionMain` linker entry, and +`-application-extension` — the compiler spelling of `APPLICATION_EXTENSION_API_ONLY` — all +in `app/Package.swift`. Any one missing yields a widget that never appears: without +`@main` the linker drops the bundle and the extension registers with nothing to offer, and +without the entry override ExtensionFoundation traps during bootstrap. Nothing observable +distinguishes these from a working widget, because the bundle still builds, signs and +registers. `com.apple.security.app-sandbox` is also mandatory — `pkd` refuses to register +an unsandboxed plug-in at all — which is why the shell writes its snapshot into the +extension's own container rather than a shared App Group, which ad-hoc signing cannot use. + `desktop/scripts/prepare-sidecar.ts` maps Rust target triples to the standalone Bun targets and prepares the external binary plus dashboard resources used by Tauri. Generated files under desktop/src-tauri/binaries/ and diff --git a/tests/ci-workflows/ci-workflows.test.ts b/tests/ci-workflows/ci-workflows.test.ts index 8746d8539f9..6d7959c3e57 100644 --- a/tests/ci-workflows/ci-workflows.test.ts +++ b/tests/ci-workflows/ci-workflows.test.ts @@ -918,9 +918,15 @@ describe("GitHub Actions hardening", () => { // Workflow-dispatch inputs must reach shell code via env, never by direct // interpolation into run: source (script-injection hardening). + // The split alone does not bound a block: the last step of a job runs on into the next + // job's header, so a job-level `if: ${{ inputs.dry-run != true }}` — which is a condition, + // not shell — read as an injection in the step above it. Each block is cut at the first + // line that dedents to job level, which is where the step's script actually ends. const runBlocks = workflow.split(/\n {6,}- name: /).filter(block => block.includes("run: |")); for (const block of runBlocks) { - const runSource = block.slice(block.indexOf("run: |")); + const afterRun = block.slice(block.indexOf("run: |")); + const jobBoundary = afterRun.search(/\n {2}\S/); + const runSource = jobBoundary === -1 ? afterRun : afterRun.slice(0, jobBoundary); expect(runSource).not.toContain("${{ inputs."); } diff --git a/tests/clients/desktop-start-at-login-default.test.ts b/tests/clients/desktop-start-at-login-default.test.ts new file mode 100644 index 00000000000..abf3472b79c --- /dev/null +++ b/tests/clients/desktop-start-at-login-default.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { repoPath } from "../helpers/repo-root"; + +/** + * A menu bar app that is not running has no menu bar item, so leaving Start at Login off by + * default means an installed app is simply gone after a reboot. The desktop shell enables it once + * per installation. + * + * The ordering is the whole contract and it is not visible from behaviour alone, so it is read out + * of the source: the marker is written before the login item is touched, the enable is guarded by + * the current state, an existing marker returns early, and the tray is built afterwards so its + * checkbox reflects the result. Get the write order backwards and a user who turns the setting off + * has it turned back on for them on the next launch. + */ +const FIRST_RUN = repoPath("desktop/src-tauri/src/first_run.rs"); +const LIB = repoPath("desktop/src-tauri/src/lib.rs"); + +function code(path: string): string { + return readFileSync(path, "utf8").replace(/\/\/[^\n]*/g, ""); +} + +describe("start at login default", () => { + const firstRun = code(FIRST_RUN); + + test("an existing marker returns before anything is changed", () => { + const early = firstRun.indexOf("marker.exists()"); + const enable = firstRun.indexOf("autolaunch().enable()"); + expect(early).toBeGreaterThan(-1); + expect(enable).toBeGreaterThan(-1); + expect(early).toBeLessThan(enable); + expect(firstRun.slice(early, enable)).toContain("return"); + }); + + test("the marker is written before the login item is registered", () => { + const write = firstRun.indexOf("fs::write(&marker"); + const enable = firstRun.indexOf("autolaunch().enable()"); + expect(write).toBeGreaterThan(-1); + expect(write).toBeLessThan(enable); + }); + + test("enabling is guarded by the current autolaunch state", () => { + const guard = firstRun.indexOf("autolaunch().is_enabled()"); + const enable = firstRun.indexOf("autolaunch().enable()"); + expect(guard).toBeGreaterThan(-1); + expect(guard).toBeLessThan(enable); + }); + + test("nothing ever deletes the marker", () => { + expect(firstRun).not.toContain("remove_file"); + expect(firstRun).not.toContain("remove_dir"); + }); + + test("it runs before the tray is installed", () => { + const lib = code(LIB); + const applied = lib.indexOf("first_run::apply_start_at_login_default"); + const tray = lib.indexOf("tray::install"); + expect(applied).toBeGreaterThan(-1); + expect(tray).toBeGreaterThan(-1); + expect(applied).toBeLessThan(tray); + }); +}); diff --git a/tests/clients/desktop-widget-entry.test.ts b/tests/clients/desktop-widget-entry.test.ts new file mode 100644 index 00000000000..6871bbd56cc --- /dev/null +++ b/tests/clients/desktop-widget-entry.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test } from "bun:test"; +import { existsSync, readFileSync } from "node:fs"; +import { repoPath } from "../helpers/repo-root"; + +/** + * The WidgetKit extension needs two things that look unrelated, and either one alone produces a + * widget that is never offered in the gallery with nothing in the build to say so. + * + * Without `@main` on the WidgetBundle, nothing references it, the linker drops it, and the + * extension still registers with `pluginkit` because the Info.plist alone is enough. The gallery + * then has no configuration to offer. That is what shipped. + * + * Without the `_NSExtensionMain` linker entry, the Swift main runs instead of the extension host's + * bootstrap and ExtensionFoundation traps in `_EXRunningExtension._shared` — EXC_BREAKPOINT on + * every launch, one crash report per attempt, and `chronod` logging + * "query failed - will try lazy reload later". + * + * Both were measured on a real install. Neither is visible to a build that only checks the bundle + * is well formed and the signature verifies, which is why they are asserted from the source. + */ +const PACKAGE = repoPath("app/Package.swift"); +const VIEWS = repoPath("app/Sources/OpenCodexWidget/Views.swift"); + +function stripComments(source: string): string { + return source.replace(/\/\/[^\n]*/g, ""); +} + +describe("widget extension entry point", () => { + test("the linker entry is NSExtensionMain, as it is for an Xcode app-extension target", () => { + const code = stripComments(readFileSync(PACKAGE, "utf8")); + expect(code).toContain("_NSExtensionMain"); + }); + + test("the target is compiled in extension-only mode", () => { + // Xcode's app-extension target sets APPLICATION_EXTENSION_API_ONLY; SwiftPM has no such + // target, so the compiler's spelling is passed by hand. It belongs beside the linker entry + // because the two are one contract: the projects that have a SwiftPM widget extension + // working supply both, and dropping either brings back a failure that the build, the + // signature and the registration all continue to look fine through. + const code = stripComments(readFileSync(PACKAGE, "utf8")); + expect(code).toContain("-application-extension"); + }); + + test("the widget bundle is the Swift entry, so the linker keeps it", () => { + const views = readFileSync(VIEWS, "utf8"); + expect(views).toMatch(/@main\s*\n\s*struct OpenCodexWidgetBundle: WidgetBundle/); + }); + + test("there is no main.swift competing with @main", () => { + // SwiftPM refuses @main in a target that also has a main.swift, and the refusal is a build + // error rather than a silent fallback - but the file existing at all means someone moved the + // entry back out of the bundle. + expect(existsSync(repoPath("app/Sources/OpenCodexWidget/main.swift"))).toBe(false); + }); + + test("the bundle actually carries a widget", () => { + const views = readFileSync(VIEWS, "utf8"); + // A WidgetBundle with an empty body offers nothing, which is the same failure by another route. + expect(views).toMatch(/OpenCodexWidget\(\)/); + expect(views).toMatch(/configurationDisplayName/); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 53fc4cbab3d..11be2c22de7 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -546,7 +546,9 @@ "desktop-3p.test.ts": "clients", "desktop-app-restart.test.ts": "clients", "desktop-profile.test.ts": "clients", + "desktop-widget-entry.test.ts": "clients", "desktop-remote-store.test.ts": "clients", + "desktop-start-at-login-default.test.ts": "clients", "destination-policy-resolved.test.ts": "routing", "devin-adapter.test.ts": "providers", "devin-effort-ladder.test.ts": "providers",