From 8ba28e6f5afc73a274198b5a0c5cc49ee1413002 Mon Sep 17 00:00:00 2001 From: Raj D <25481060+radroid@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:40:35 -0400 Subject: [PATCH] fix(t3x): auto-build fixes found running it for real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects surfaced by actually setting up the LaunchAgent end to end. 1. --print-launchd emitted a plist that could never run. macOS TCC gates ~/Documents, ~/Desktop, ~/Downloads and iCloud Drive. launchd jobs get no TCC grant and never trigger a consent prompt, so a repo in one of those folders yields EPERM: launchd cannot chdir to WorkingDirectory and /bin/bash cannot read the script. The failure is near-invisible — bootstrap succeeds, 'launchctl print' shows the job loaded, and the only evidence is 'last exit code = 126'. Now detected up front: refuse to emit (exit 2) with the three remedies, or --force past it. 2. --install --dry-run named the wrong app. It hardcoded "${T3X_AUTOBUILD_APP_NAME:-T3 Code}.app" and never looked at the dmg, so it always claimed 'T3 Code.app' while a real install replaced 'T3 Code (Alpha).app' — wrong about the one fact the preview exists to establish. It also bailed before printing anything when no dmg existed yet, i.e. for every first-time user. Now mounts the dmg read-only for ground truth, falls back to a mirror of resolveDesktopProductName(), and warns when the target app is absent (the 'creates a silent third app' footgun). 3. log() wrote every line twice under launchd. It pipes through 'tee -a "$LOG_FILE" >&2' while the plist points BOTH StandardOutPath and StandardErrorPath at that same file, so one watcher's output reads exactly like two racing. The plist now declares T3X_AUTOBUILD_STDERR_IS_LOG=1. Detection via 'stat -f %d:%i /dev/fd/2' is deliberately NOT used: on macOS that stats the devfs node, never the redirect target, so it can never match — noted in-line so it doesn't get re-attempted. Terminal behaviour is unchanged throughout; only the launchd path differs. Co-Authored-By: Claude Opus 4.8 --- docs/t3x/auto-build-runbook.md | 76 +++++++++++-- scripts/t3x/auto-build-desktop.sh | 178 +++++++++++++++++++++++++----- 2 files changed, 216 insertions(+), 38 deletions(-) diff --git a/docs/t3x/auto-build-runbook.md b/docs/t3x/auto-build-runbook.md index 402abea8e14..1eb4dcafba9 100644 --- a/docs/t3x/auto-build-runbook.md +++ b/docs/t3x/auto-build-runbook.md @@ -43,20 +43,34 @@ The `latest` name comes from `"productName": "T3 Code (Alpha)"` in applies if that field is ever removed upstream. So a default local build replaces **`T3 Code (Alpha).app`**, the app most fork users already run. -Confirm for yourself before the first `--install` — this reads the real name out -of the built dmg rather than trusting the table above: +Don't trust the table — ask the script. `--dry-run` mounts the built `.dmg` +read-only and reports the **real** name, falling back to the prediction above +only when nothing has been built yet: ```bash -ls -d "/Applications/"*T3* # what you have installed +ls -d "/Applications/"*T3* # what you have installed +scripts/t3x/auto-build-desktop.sh --install --dry-run --force # what would be replaced +``` + +(`--force` is needed only because a dry run otherwise stops at "no change since +last build" before it reaches the install step.) + +It prints one of: -MP=$(mktemp -d) && hdiutil attach -nobrowse -readonly -mountpoint "$MP" \ - release/*.dmg >/dev/null && ls -d "$MP"/*.app && hdiutil detach "$MP" >/dev/null +``` +DRY-RUN install: read 'T3 Code (Alpha).app' from …/release/T3-Code-0.0.28-arm64.dmg +DRY-RUN: '/Applications/T3 Code (Alpha).app' exists and WOULD be replaced ``` -If the built name doesn't match the app you actually launch, `--install` creates -a **new, separate** app and it will look like "nothing updated." In that case -either switch to running the app this build produces, or redirect the install: -`T3X_AUTOBUILD_APPLICATIONS_DIR=/some/dir scripts/t3x/auto-build-desktop.sh --install`. +``` +DRY-RUN WARNING: '/Applications/T3 Code.app' does NOT exist — a real --install +would create a NEW app and leave whatever you currently run untouched +``` + +That second form is the failure you care about: `--install` would create a +**new, separate** app and it would look like "nothing updated." Either switch to +running the app this build produces, or redirect the install with +`T3X_AUTOBUILD_APPLICATIONS_DIR=/some/dir`. **This also assumes you run the _installed_ app, not `pnpm start:desktop`.** Those are different processes; auto-install updates the installed one only. @@ -122,22 +136,60 @@ resolve against the repo root, matching `build-desktop-artifact.ts`), positive integer). `--print-launchd` copies all of these into the emitted plist, so a plist generated from a shell where you overrode them keeps those overrides. -`T3X_AUTOBUILD_APP_NAME` only names the `.app` assumed during `--dry-run` (a real install -reads the actual name out of the mounted dmg); it does not redirect where you install. +`T3X_AUTOBUILD_APP_NAME` overrides the `.app` name during `--dry-run`, short-circuiting +the dmg read; it does not redirect where you install. +`T3X_AUTOBUILD_STDERR_IS_LOG=1` stops `log()` echoing to stderr — set automatically in the +generated plist, where stderr already points at the log file. **Exit codes:** `0` success · `3` nothing to do (HEAD unchanged, or another instance holds -the lock) · `1` build or install failed · `2` bad flag or invalid env value. +the lock) · `1` build or install failed · `2` bad flag, invalid env value, or a +TCC-protected repo path. ## Running it hands-off ### Option 1 — LaunchAgent (starts at login) +> **🔴 Your repo must NOT live in `~/Documents`, `~/Desktop`, `~/Downloads`, or iCloud +> Drive.** Those are TCC-protected. LaunchAgents get no TCC grant and — unlike apps — +> never trigger a consent prompt, so macOS just returns `EPERM`. The agent loads, +> `launchctl print` cheerfully reports it, and it dies instantly with +> `last exit code = 126`, having built nothing: +> +> ``` +> shell-init: error retrieving current directory: getcwd: … Operation not permitted +> /bin/bash: …/scripts/t3x/auto-build-desktop.sh: Operation not permitted +> ``` +> +> `--print-launchd` detects this and refuses to emit a plist (exit `2`) rather than hand +> you one that silently never works. Move the repo somewhere unprotected — `~/Developer` +> is the conventional spot: +> +> ```bash +> mkdir -p ~/Developer +> mv ~/Documents/t3code ~/Developer/t3code +> git -C ~/Developer/t3code worktree repair # plus each worktree, if you use them +> ``` +> +> The alternative — granting Full Disk Access to `/bin/bash` — works, but grants FDA to +> *every* bash script on the machine. Not worth it for this. Option 3 needs no move at +> all, since a terminal already has the grant. + ```bash scripts/t3x/auto-build-desktop.sh --print-launchd --install --interval 120 \ > ~/Library/LaunchAgents/dev.t3x.autobuild.plist launchctl bootstrap "gui/$UID" ~/Library/LaunchAgents/dev.t3x.autobuild.plist ``` +**Verify it actually runs** — a loaded agent is not a working one: + +```bash +launchctl print "gui/$UID/dev.t3x.autobuild" | grep -E "state|pid|last exit code" +tail -f ~/.t3/userdata/logs/t3x-autobuild.log +``` + +Want `state = running` with a pid and `last exit code = (never exited)`. A +`last exit code = 126` means the TCC problem above. + Stop / remove it: ```bash diff --git a/scripts/t3x/auto-build-desktop.sh b/scripts/t3x/auto-build-desktop.sh index 35c8039c9ee..5b9fc04605f 100755 --- a/scripts/t3x/auto-build-desktop.sh +++ b/scripts/t3x/auto-build-desktop.sh @@ -101,7 +101,25 @@ mkdir -p "$STATE_DIR" "$LOG_DIR" # --- logging ----------------------------------------------------------------- now_iso() { date '+%Y-%m-%dT%H:%M:%S%z'; } -log() { printf '%s %s\n' "$(now_iso)" "$*" | tee -a "$LOG_FILE" >&2; } + +# `tee -a "$LOG_FILE" >&2` writes the line to the log AND echoes it to stderr, which is +# what you want in a terminal. Under launchd it is not: the generated plist points BOTH +# StandardOutPath and StandardErrorPath at $LOG_FILE, so the echo lands straight back in +# the same file and every line appears twice — which reads exactly like two watchers +# racing when only one is running. +# +# print_launchd sets this in the plist it generates, because that plist is precisely +# where stderr gets aimed at the log. Do NOT try to detect it by comparing +# `stat -f '%d:%i' /dev/fd/2` against the log file: on macOS that stats the devfs node +# (e.g. 2540177495:339), never the redirect target, so the comparison can never match. +STDERR_IS_LOG="${T3X_AUTOBUILD_STDERR_IS_LOG:-0}" +log() { + if [[ "$STDERR_IS_LOG" == "1" ]]; then + printf '%s %s\n' "$(now_iso)" "$*" >>"$LOG_FILE" + else + printf '%s %s\n' "$(now_iso)" "$*" | tee -a "$LOG_FILE" >&2 + fi +} # --- json status (no jq) ----------------------------------------------------- # NOTE: `printf '%s'`, not a `<<<` here-string. A here-string appends a trailing @@ -190,8 +208,81 @@ acquire_lock() { } # --- install ----------------------------------------------------------------- +# Answer "which .app would be installed?" WITHOUT changing anything. +# +# LOGIC MIRROR of scripts/build-desktop-artifact.ts (resolveDesktopUpdateChannel / +# resolveDesktopProductName). Only used when no .dmg exists yet to read the name out of; +# if that upstream logic changes, this prediction goes stale (the dmg peek below does not). +predicted_app_name() { + python3 - "$REPO/apps/desktop/package.json" <<'PY' 2>/dev/null || printf 'T3 Code' +import json, re, sys +try: + pkg = json.load(open(sys.argv[1])) +except Exception: + print("T3 Code"); raise SystemExit(0) +version = pkg.get("version", "") +# resolveDesktopUpdateChannel: /-nightly\.\d{8}\.\d+$/ -> nightly, else latest +if re.search(r"-nightly\.\d{8}\.\d+$", version): + print("T3 Code (Nightly)") +else: + print(pkg.get("productName") or "T3 Code") +PY +} + +# Resolve the .app basename (without ".app"). Prefers ground truth over prediction: +# explicit env override > the actual name inside the built dmg > upstream-mirrored guess. +resolve_app_name() { + local dmg="$1" mnt app + if [[ -n "${T3X_AUTOBUILD_APP_NAME:-}" ]]; then printf '%s' "$T3X_AUTOBUILD_APP_NAME"; return 0; fi + if [[ -n "$dmg" && -f "$dmg" ]]; then + mnt="$(mktemp -d "${TMPDIR:-/tmp}/t3x-peek.XXXXXX")" + # -readonly: peeking must never modify the artifact, even in a dry run. + if hdiutil attach -nobrowse -readonly -quiet "$dmg" -mountpoint "$mnt" >/dev/null 2>&1; then + app="$(find "$mnt" -maxdepth 1 -name '*.app' | head -1)" + hdiutil detach "$mnt" -quiet >/dev/null 2>&1 || true + rmdir "$mnt" 2>/dev/null || true + if [[ -n "$app" ]]; then printf '%s' "$(basename "$app" .app)"; return 0; fi + else + rmdir "$mnt" 2>/dev/null || true + fi + fi + predicted_app_name +} + install_dmg() { local dmg="$1" + + # The whole point of `--install --dry-run` is to answer "which app gets replaced?" + # before you let it touch /Applications. It used to answer with a hardcoded + # "${T3X_AUTOBUILD_APP_NAME:-T3 Code}.app" and never look at the dmg at all, so on this + # repo it always claimed `T3 Code.app` — an app that does not exist — while a real + # install replaced `T3 Code (Alpha).app`. It also bailed out entirely before printing + # anything when no dmg had been built yet, which is every first-time user. + if [[ $DRY_RUN -eq 1 ]]; then + local d_appbase d_target + d_appbase="$(resolve_app_name "$dmg").app" + d_target="$APPLICATIONS_DIR/$d_appbase" + if [[ -n "$dmg" && -f "$dmg" ]]; then + log "DRY-RUN install: read '$d_appbase' from $dmg" + else + log "DRY-RUN install: no .dmg built yet; '$d_appbase' predicted from apps/desktop/package.json" + fi + log "install: '$d_appbase' -> '$d_target'" + log "DRY-RUN would: quit app '${d_appbase%.app}'" + log "DRY-RUN would: rm -rf '$d_target' && cp -R '$d_target'" + log "DRY-RUN would: xattr -dr com.apple.quarantine '$d_target' (unsigned local build)" + [[ $DO_RELAUNCH -eq 1 ]] && log "DRY-RUN would: open '$d_target'" + # The footgun this preview exists to catch: if the target is absent, a real --install + # CREATES a new app and silently leaves the one you actually launch untouched. + if [[ -e "$d_target" ]]; then + log "DRY-RUN: '$d_target' exists and WOULD be replaced" + else + log "DRY-RUN WARNING: '$d_target' does NOT exist — a real --install would create a NEW app" + log "DRY-RUN WARNING: and leave whatever you currently run untouched. See docs/t3x/auto-build-runbook.md" + fi + return 0 + fi + [[ -z "$dmg" || ! -f "$dmg" ]] && { log "install: no .dmg to install ($dmg)"; return 1; } local mnt app appbase target @@ -199,36 +290,15 @@ install_dmg() { # shellcheck disable=SC2064 trap "hdiutil detach '$mnt' -quiet >/dev/null 2>&1 || true; rmdir '$mnt' 2>/dev/null || true" RETURN - if [[ $DRY_RUN -eq 1 ]]; then - log "DRY-RUN install: hdiutil attach '$dmg' -> '$mnt'" - else - hdiutil attach -nobrowse -quiet "$dmg" -mountpoint "$mnt" - fi + hdiutil attach -nobrowse -quiet "$dmg" -mountpoint "$mnt" - # In dry-run we can't read the mount, so infer the app name from env or default. - if [[ $DRY_RUN -eq 1 ]]; then - appbase="${T3X_AUTOBUILD_APP_NAME:-T3 Code}.app" - app="$mnt/$appbase" - else - app="$(find "$mnt" -maxdepth 1 -name '*.app' | head -1)" - [[ -z "$app" ]] && { log "install: no .app found in $dmg"; return 1; } - appbase="$(basename "$app")" - fi + app="$(find "$mnt" -maxdepth 1 -name '*.app' | head -1)" + [[ -z "$app" ]] && { log "install: no .app found in $dmg"; return 1; } + appbase="$(basename "$app")" target="$APPLICATIONS_DIR/$appbase" log "install: '$appbase' -> '$target'" - # NOTE: the dry-run return must come BEFORE the quit below. `osascript quit` is a real - # side effect on the user's session, so running it here would make `--install --dry-run` - # close their running app — exactly what a dry run promises not to do. - if [[ $DRY_RUN -eq 1 ]]; then - log "DRY-RUN would: quit app '${appbase%.app}'" - log "DRY-RUN would: rm -rf '$target' && cp -R '$app' '$target'" - log "DRY-RUN would: xattr -dr com.apple.quarantine '$target' (unsigned local build)" - [[ $DO_RELAUNCH -eq 1 ]] && log "DRY-RUN would: open '$target'" - return 0 - fi - # Best-effort quit of the running app so we can replace it. osascript -e "quit app \"${appbase%.app}\"" >/dev/null 2>&1 || true @@ -350,7 +420,60 @@ build_once() { # launchctl silently refuses to load. xml_escape() { printf '%s' "${1-}" | sed 's/&/\&/g; s//\>/g'; } +# macOS TCC (Privacy & Security) gates ~/Desktop, ~/Documents, ~/Downloads and iCloud +# Drive. GUI apps get a consent prompt; launchd agents never do — they just receive +# EPERM. Echo the containing protected root, or return 1. +tcc_protected_path() { + local p="$1" d + for d in "$HOME/Desktop" "$HOME/Documents" "$HOME/Downloads" "$HOME/Library/Mobile Documents"; do + case "$p" in "$d" | "$d"/*) printf '%s' "$d"; return 0 ;; esac + done + return 1 +} + print_launchd() { + # Refuse to hand back a plist that provably cannot run. Emitting one anyway is the worst + # outcome: `launchctl bootstrap` succeeds, `launchctl print` reports the job as loaded, + # and the only evidence of failure is `last exit code = 126` — so it looks installed and + # silently never builds anything. + local protected + if protected="$(tcc_protected_path "$SCRIPT_DIR")"; then + if [[ $FORCE -eq 0 ]]; then + cat >&2 < Privacy & Security + (Cmd+Shift+G to reach /bin). This grants FDA to EVERY bash script on the machine — + broad and permanent; not recommended for this. + + 3. Skip launchd entirely and run the watcher in a terminal: + $SCRIPT_DIR/auto-build-desktop.sh --watch --install --interval $INTERVAL + +If you redirected stdout to a .plist, that file is now empty — delete it. +Re-run with --force to emit the plist anyway (it will not work). +EOF + exit 2 + fi + log "WARNING: repo is under $protected (TCC-protected); this agent will fail with exit 126" + fi + local label="dev.t3x.autobuild" local x_script x_repo x_log x_script="$(xml_escape "$SCRIPT_DIR/auto-build-desktop.sh")" @@ -394,6 +517,9 @@ $( [[ $DO_RELAUNCH -eq 1 ]] && printf ' --relaunch' || true T3X_AUTOBUILD_APPLICATIONS_DIR${x_apps} T3CODE_DESKTOP_OUTPUT_DIR${x_out} T3X_AUTOBUILD_KEEP_DMGS${KEEP_DMGS} + + T3X_AUTOBUILD_STDERR_IS_LOG1