diff --git a/CHANGELOG.md b/CHANGELOG.md index c11f052..c8edb68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/). - `skills` module on Bash and native Windows: updates agent skills in both project and global scopes via `skills update --project --global` (matching the interactive "Both" option without prompting), falling back to `npx --yes skills update` when the `skills` CLI is not installed; `--non-interactive` appends `--yes` to skip upstream-deletion prompts. +### Changed + +- Show Unix command phases and elapsed progress every 30 seconds at info/debug log levels, preserving JSONL stdout and allowing long installations to continue until cancelled. +- Stream Unix npm installation stderr and guarded pip installation output; report parallel pip workers as they finish while retaining separate package logs. + +### Fixed + +- Make Unix command and capture waits interruptible, preserve interactive stdin and exit statuses, and clean owned children, output helpers, and temporary resources on SIGINT/SIGTERM. +- Disable prompts for background pip installations and for discovery/planning under `--non-interactive`. +- Honor `--non-interactive` for Homebrew upgrade-table confirmation with command-scoped `HOMEBREW_NO_ASK=1`; explain the confirmation prompt during interactive runs. + ## [2.1.3] - 2026-08-10 ### Fixed diff --git a/PLAN.md b/PLAN.md index cd30fe4..cecc284 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1,3 +1,30 @@ +# Plan: macOS stalls and visible update progress + +## Goal + +- Make Unix updates cancellable and expose the active phase without automatic installation timeouts or changes to public flags and JSONL events. + +## Live diagnosis + +- Reproduced the apparent stall after Homebrew 6.0.22's upgrade table: its default ask mode was waiting for confirmation, rather than a frozen command. +- The initial diagnostic run updated Homebrew itself, shell repositories, and one npm package. The first Ctrl+C failed the brew module but allowed later modules to run; the second stopped the run at npm preflight. The identified pause is covered by isolated Homebrew prompt regressions; no additional live upgrade is needed for validation. + +## Execution checklist + +- [x] Introduce Bash 3.2-compatible managed execution/capture helpers with interruptible waits, owned-child cleanup, preserved stdin, and accurate exit statuses. +- [x] Announce slow phases and emit elapsed human progress every 30 seconds at info/debug log levels, routing human output to stderr with JSON. +- [x] Stream npm stderr and guarded pip installation diagnostics; report parallel pip completion with separate package logs. +- [x] Disable background pip prompts and discovery/planning prompts under `-n`. +- [x] Scope `HOMEBREW_NO_ASK=1` to brew commands under `-n`; preserve interactive confirmation with a clear hint. +- [x] Document cancellation, progress, and diagnostics in README, SPEC, and CHANGELOG without a release/version bump. +- [x] Verify synchronized SIGINT/SIGTERM cleanup for commands, captures, npm preflights, and parallel pip workers, plus interactive stdin under a pseudo-terminal. +- [x] Verify live diagnostics, failure propagation, JSONL purity with logging, and existing npm/Python safety behavior. +- [x] Run lint and full tests on macOS Bash 3.2 and shared-runner coverage on Linux; use isolated homes and stub commands for installed-copy checks. +- [x] Reproduce the table pause in a real update run and identify Homebrew confirmation as the cause. +- [x] Validate Homebrew non-interactive environment scoping and interactive prompt behavior. + +--- + # Plan: skills module updates ## Goal diff --git a/README.md b/README.md index 17cf172..4bc0465 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,10 @@ Tests require `python3` with either public `packaging` or pip's vendored packagi - This script updates _global_ environments (`npm -g`, `pip`), which can be disruptive. - Use `--dry-run` first, and consider `--only`/`--skip` to control scope. +- Homebrew can pause after its upgrade table to request confirmation. Interactive runs retain this prompt and show a reminder; `updates -n` scopes `HOMEBREW_NO_ASK=1` to brew commands so unattended upgrades proceed without changing your shell or Homebrew configuration. +- On macOS/Linux, slow commands and discovery steps show their current phase and a progress message every 30 seconds with elapsed time. These messages follow `--log-level` (`info` or `debug`) and go to stderr with `--json`. Long updates have no automatic installation timeout; use Ctrl+C to cancel. +- Unix cancellation stops owned commands and output helpers, cleans temporary resources, and exits `130` for SIGINT or `143` for SIGTERM. Foreground interactive commands retain terminal input. An interrupted package installation may be incomplete. +- Unix npm installation stderr and guarded pip installation output appear while commands run. Parallel pip packages keep separate logs and report completion as workers finish; background pip installations always disable prompts. `-n` also disables pip discovery and planning prompts. - For npm 11+ global installs, `updates` may retry once with npm's suggested one-shot `--allow-scripts=...` list so package postinstall steps can finish without changing persistent npm config. - Node updates are filtered against the active Node runtime and installed per package. An unexpected incompatible or otherwise failed package does not prevent later compatible packages from being attempted, but still fails the node module. - Git-backed `shell`/`repos` updates never infer tracking branches or alter local work. Detached HEADs, branches without upstreams, and dirty worktrees warn and skip; diverged histories and failed pulls/post-pull actions fail the module. diff --git a/SPEC.md b/SPEC.md index 17e8767..d2fb704 100644 --- a/SPEC.md +++ b/SPEC.md @@ -283,6 +283,7 @@ When passed, `~/.updatesrc` is not read. Useful for CI, testing, and debugging. Output is intended to be stable and easy to grep. - Normal progress goes to **stdout** (or **stderr** when `--json` is active). +- On Unix, managed commands and potentially blocking npm, pip, and Git captures announce their module/package and phase before waiting. While waiting, human progress messages include elapsed time every 30 seconds at `info`/`debug` log levels. They add no JSONL event types or fields. - Warnings and errors go to **stderr** and are prefixed: - `WARN: ...` - `ERROR: ...` @@ -312,7 +313,13 @@ See [Section 3.8](#38---json-jsonl-streaming-output) for the full event type tab When `--json` is active, the log file receives the human-readable stderr output, not the JSONL stream. -### 5.4 Color / emoji +### 5.4 Unix command execution and cancellation + +- Shared Bash 3.2-compatible execution and capture helpers preserve command exit statuses and foreground interactive stdin. +- SIGINT/SIGTERM interrupt waits, terminate and reap owned children and output helpers, clean temporary resources, and exit `130`/`143`, respectively. No later module runs after cancellation; cleanup never uses broad process-name kills. +- Long installations continue until completion or cancellation; no automatic installation timeout is added. Cancellation does not roll back changes already made by an underlying tool. + +### 5.5 Color / emoji - ANSI colors are enabled when stderr/stdout are TTYs and `NO_COLOR` is not set. - `--no-color` or `NO_COLOR=1` disables colors globally. @@ -404,6 +411,7 @@ Purpose: update and upgrade Homebrew formulae (and optionally casks). - `--brew-mode casks`: `brew upgrade` - `--brew-mode greedy`: `brew upgrade --greedy` - If `--brew-cleanup` (default): `brew cleanup` +- On Unix, `-n` / `--non-interactive` sets `HOMEBREW_NO_ASK=1` only for brew commands to disable Homebrew upgrade-table confirmation. Interactive runs retain Homebrew prompts and print an info-level hint before upgrading. No persistent environment or Homebrew configuration changes are made. - Side effects: upgrades Homebrew-managed packages. ### 8.2 `shell` @@ -480,7 +488,7 @@ Purpose: upgrade global npm packages using `npm-check-updates`. - Per package, if npm fails with `ERESOLVE`, retries once with `--legacy-peer-deps`; configured npm flags are retained, duplicate configured `--legacy-peer-deps` is removed for the retry, and the forced retry flag is appended once. - Per package, if npm succeeds but reports pending global install scripts, retries once with npm's suggested one-shot `--allow-scripts=...` list while retaining configured npm flags. - `EBADENGINE` and other fatal failures are not retried. Remaining compatible packages are still attempted; any final package failure makes the node module fail. -- Superseded first-attempt diagnostics are suppressed after a successful retry. Final failures retain raw npm diagnostics plus a concise package-specific error. +- On Unix, installation stderr streams live while being retained for engine checks and bounded retries; earlier attempt diagnostics therefore remain visible even after a successful retry. Native Windows suppresses superseded first-attempt diagnostics after a successful retry. Final failures retain raw npm diagnostics plus a concise package-specific error. - Side effects: upgrades global npm packages. The Git and Node hardening above is implemented behind private command-outcome seams. The authoritative Node engine preflight adds no runtime dependency. These mechanics do not add or change public v2 flags, configuration keys, exit codes, JSONL event types, or summary fields. @@ -507,7 +515,8 @@ Purpose: upgrade global Python packages with `pip`. - Bash implementation, normal or `--pip-force`: ` -m pip list --outdated --format=json [--user]`, then ` -m pip install -U ` in parallel batches of `--parallel `. - Bash implementation, externally-managed default: ` -m pip list --outdated --format=json --user`, then per-package `pip install -U --user [--break-system-packages if supported] --only-binary=:all: --dry-run --report ` guard checks. If pip lacks `--dry-run --report`, this path errors under `--only` and skips otherwise. Packages are skipped if the report would install packages absent from the user site, use source distributions, or violate installed/planned dependency requirements. The safe subset is checked again in one combined dry-run, installed in one wheel-only user-site transaction only if that combined plan is safe, then `pip check` runs. If post-install `pip check` failures were already present before install, the guarded path warns instead of failing on the pre-existing environment issue. - Native Windows PowerShell implementation: same discovery/install flow as the normal path, but upgrades run sequentially and `--parallel ` is rejected. -- With `-n`: adds `--no-input` to pip calls. +- With `-n`: disables pip input, including Unix discovery and planning calls. Unix background pip installations always disable input, even without `-n`; foreground interactive commands retain input otherwise. +- On Unix, guarded pip installation output streams live. Parallel workers retain separate package logs, identify active packages, and replay each package log when that worker completes, preserving its exit status. - Side effects: upgrades Python packages; does not upgrade the Python interpreter itself. ### 8.9 `uv` @@ -691,7 +700,7 @@ Purpose: list available macOS software updates. - **PII:** No user data is collected or transmitted. - **Abuse cases:** - Malicious or tampered GitHub release asset: mitigated by immutable releases, GitHub asset digests, `SHA256SUMS`, manifest validation, and HTTPS. - - pip parallel upgrades: stderr interleaving is cosmetic, not a security issue. + - pip parallel upgrades retain separate package logs; replay happens as each worker completes. - `--pip-force` is explicitly opt-in and documented as unsafe. - **Privilege escalation:** `sudo` is only used for Linux system package upgrades. Native Windows self-update never elevates; unknown or non-writable layouts warn and skip. diff --git a/scripts/test.sh b/scripts/test.sh index c8b58b7..a355fe5 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -5,6 +5,7 @@ set -euo pipefail cd "$(dirname "${BASH_SOURCE[0]}")/.." ./tests/test_cli.sh +python3 ./tests/test_execution.py bash ./tests/test_release.sh if command -v pwsh >/dev/null 2>&1; then diff --git a/tests/test_cli.sh b/tests/test_cli.sh index 24bc7af..9210cb2 100755 --- a/tests/test_cli.sh +++ b/tests/test_cli.sh @@ -38,12 +38,12 @@ if [ -z "$SYSTEM_PYTHON3" ]; then echo "python3 is required for tests/test_cli.sh" >&2 exit 1 fi -if ! "$SYSTEM_PYTHON3" - <<'PY' >/dev/null 2>&1; then +if ! "$SYSTEM_PYTHON3" -c ' try: import packaging.requirements except Exception: import pip._vendor.packaging.requirements -PY +' >/dev/null 2>&1; then echo "python3 with packaging or pip vendored packaging is required for guard helper tests" >&2 exit 1 fi @@ -1241,9 +1241,9 @@ setup_python_guard_fixture : >"$CALL_LOG" "$SCRIPT" --only python --pip-force --no-emoji >/dev/null grep -q '^python -m pip list --outdated --format=json$' "$CALL_LOG" -grep -q '^python -m pip install -U --break-system-packages idna$' "$CALL_LOG" -grep -q '^python -m pip install -U --break-system-packages pyelftools$' "$CALL_LOG" -grep -q '^python -m pip install -U --break-system-packages unicorn$' "$CALL_LOG" +grep -q '^python -m pip install -U --break-system-packages --no-input idna$' "$CALL_LOG" +grep -q '^python -m pip install -U --break-system-packages --no-input pyelftools$' "$CALL_LOG" +grep -q '^python -m pip install -U --break-system-packages --no-input unicorn$' "$CALL_LOG" UPDATES_TEST_CASE @@ -2066,14 +2066,8 @@ echo "$out" | grep -q '^==> node END (OK)' grep -q '^npm install -g -- @tarquinen/opencode-dcp@3.1.13$' "$CALL_LOG" grep -q '^npm install -g --legacy-peer-deps -- @tarquinen/opencode-dcp@3.1.13$' "$CALL_LOG" grep -q '^npm install -g --allow-scripts=opencode-ai,koffi --legacy-peer-deps -- @tarquinen/opencode-dcp@3.1.13$' "$CALL_LOG" -if grep -q 'npm error code ERESOLVE' "$npm_eresolve_stderr"; then - echo "Expected successful ERESOLVE retry to suppress first-pass npm error details" >&2 - exit 1 -fi -if grep -q 'npm warn allow-scripts' "$npm_eresolve_stderr"; then - echo "Expected successful allow-scripts retry to suppress first-pass npm warning details" >&2 - exit 1 -fi +grep -q 'npm error code ERESOLVE' "$npm_eresolve_stderr" +grep -q 'npm warn allow-scripts' "$npm_eresolve_stderr" grep -q 'retrying with --legacy-peer-deps' "$npm_eresolve_stderr" grep -q 'retrying once with npm-provided allow-scripts list' "$npm_eresolve_stderr" @@ -2173,10 +2167,7 @@ out="$("$SCRIPT" --only node --no-emoji --no-color 2>"$npm_allow_scripts_stderr" echo "$out" | grep -q '^==> node END (OK)' grep -q '^npm install -g -- opencode-ai@1.17.8$' "$CALL_LOG" grep -q '^npm install -g --allow-scripts=opencode-ai,koffi -- opencode-ai@1.17.8$' "$CALL_LOG" -if grep -q 'npm warn allow-scripts' "$npm_allow_scripts_stderr"; then - echo "Expected successful allow-scripts retry to suppress npm warning details" >&2 - exit 1 -fi +grep -q 'npm warn allow-scripts' "$npm_allow_scripts_stderr" grep -q 'retrying once with npm-provided allow-scripts list' "$npm_allow_scripts_stderr" UPDATES_TEST_CASE @@ -2224,10 +2215,7 @@ out="$("$SCRIPT" --only node --no-emoji --no-color 2>"$npm_allow_scripts_flag_on echo "$out" | grep -q '^==> node END (OK)' grep -q '^npm install -g -- opencode-ai@1.17.8$' "$CALL_LOG" grep -q '^npm install -g --allow-scripts=opencode-ai,koffi -- opencode-ai@1.17.8$' "$CALL_LOG" -if grep -q 'npm warn allow-scripts' "$npm_allow_scripts_flag_only_stderr"; then - echo "Expected successful flag-only allow-scripts retry to suppress npm warning details" >&2 - exit 1 -fi +grep -q 'npm warn allow-scripts' "$npm_allow_scripts_flag_only_stderr" grep -q 'retrying once with npm-provided allow-scripts list' "$npm_allow_scripts_flag_only_stderr" UPDATES_TEST_CASE diff --git a/tests/test_execution.py b/tests/test_execution.py new file mode 100644 index 0000000..6ea3e00 --- /dev/null +++ b/tests/test_execution.py @@ -0,0 +1,348 @@ +#!/usr/bin/env python3 +"""Subprocess regressions using isolated, marker-controlled command stubs.""" +import json +import os +from pathlib import Path +import pty +import signal +import subprocess +import sys +import tempfile +import time +import unittest + + +ROOT = Path(__file__).resolve().parents[1] +STUB = r''' +import json, os, pathlib, subprocess, sys, time +root = pathlib.Path(os.environ['PROBE_DIR']) +name = pathlib.Path(sys.argv[0]).name +args = sys.argv[1:] +with (root / 'calls').open('a') as stream: + stream.write(json.dumps([name] + args) + '\n') +mode = os.environ['PROBE_MODE'] +def pause(): + (root / 'child.pid').write_text(str(os.getpid())) + print('stub diagnostic before completion', file=sys.stderr, flush=True) + (root / 'ready').touch() + while not (root / 'release').exists(): + time.sleep(.02) +if name == 'sleep': + with (root / 'timer.pids').open('a') as stream: + stream.write(str(os.getpid()) + '\n') + os.execv('/bin/sleep', ['/bin/sleep'] + args) +elif name == 'uname': + print('Darwin') +elif name == 'brew': + if args == ['update']: + (root / 'brew-no-ask').write_text(os.environ.get('HOMEBREW_NO_ASK', '')) + if mode == 'brew130': + sys.exit(130) + if mode == 'pty': + print('Type confirmation:', flush=True) + (root / 'ready').touch() + if input() != 'confirmed': + sys.exit(7) + (root / 'answered').touch() + else: + pause() +elif name == 'ncu': + if args[:1] == ['--help']: + print('--enginesNode') + else: + if mode == 'ncu': + pause() + print('{"execution-probe":"2.0.0"}') +elif name == 'npm': + if '--dry-run' in args: + if mode == 'preflight': + pause() + else: + if mode == 'npm-descendant': + child = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)']) + (root / 'descendant.pid').write_text(str(child.pid)) + print('inherited descriptor diagnostic', file=sys.stderr, flush=True) + (root / 'ready').touch() + else: + pause() + sys.exit(19 if mode == 'npm-fail' else 0) +elif name in ('python', 'python3'): + if args[:1] == ['-c']: + if 'EXTERNALLY-MANAGED' in args[1]: + print('1' if mode == 'pip-help' else '0') + else: + os.execv(os.environ['REAL_PYTHON'], [os.environ['REAL_PYTHON']] + args) + elif args[:2] == ['-m', 'pip']: + if '--version' in args: + if mode == 'pip-version': + pause() + print('pip 25.0') + elif 'list' in args: + if mode == 'pip-scheduler': + print(json.dumps([{'name': name} for name in ('first', 'second', 'third')])) + else: + print('[{"name":"execution-probe"}]') + elif 'install' in args: + if mode == 'pip-scheduler' and args[-1] != 'first': + (root / (args[-1] + '.started')).touch() + print('scheduler ' + args[-1] + ' complete', flush=True) + else: + pause() + else: + sys.exit(3) + else: + sys.exit(4) +else: + sys.exit(5) +''' + + +class ExecutionTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix='updates-execution-') + self.root = Path(self.temp.name) + self.bin = self.root / 'bin' + self.bin.mkdir() + self.command_tmp = self.root / 'tmp' + self.command_tmp.mkdir() + install_bin = self.root / 'home' / 'bin' + install_bin.mkdir(parents=True) + self.script = install_bin / 'updates' + self.script.write_bytes((ROOT / 'updates').read_bytes()) + self.script.chmod(0o755) + self.processes = [] + self.handles = [] + self.env = {key: value for key, value in os.environ.items() + if key not in ('BASH_ENV', 'ENV', 'ZSH', 'ZSH_CUSTOM', 'NVM_DIR', + 'FNM_DIR', 'FNM_MULTISHELL_PATH', 'PYTHONPATH', + 'PYTHONHOME') and not key.startswith('UPDATES_')} + self.env.update(HOME=str(self.root / 'home'), PATH=str(self.bin) + ':/usr/bin:/bin:/usr/sbin:/sbin', + PROBE_DIR=str(self.root), REAL_PYTHON=sys.executable, TMPDIR=str(self.command_tmp), + UPDATES_SELF_UPDATE='0') + self.env.pop('HOMEBREW_NO_ASK', None) + for name in ('uname', 'brew', 'ncu', 'npm', 'python', 'python3', 'sleep'): + path = self.bin / name + path.write_text('#!' + sys.executable + '\n' + STUB) + path.chmod(0o755) + + def tearDown(self): + (self.root / 'release').touch() + for process in self.processes: + if process.poll() is None: + process.terminate() + try: + process.wait(timeout=2) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=2) + marker = self.root / 'child.pid' + if marker.exists(): + pid = int(marker.read_text()) + if self.alive(pid): + os.kill(pid, signal.SIGTERM) + descendant = self.root / 'descendant.pid' + if descendant.exists() and self.alive(int(descendant.read_text())): + os.kill(int(descendant.read_text()), signal.SIGTERM) + for pid in self.timer_pids(): + if self.alive(pid): + os.kill(pid, signal.SIGTERM) + for handle in self.handles: + handle.close() + self.temp.cleanup() + + def module_calls(self): + return [json.loads(line) for line in self.output('calls').splitlines() + if json.loads(line)[0] != 'sleep'] + + def timer_pids(self): + return [int(line) for line in self.output('timer.pids').splitlines()] + + @staticmethod + def alive(pid): + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + + def until(self, predicate, timeout=5): + end = time.monotonic() + timeout + while time.monotonic() < end: + if predicate(): + return + time.sleep(.02) + self.fail('Timed out waiting for stub condition; stdout=' + self.output('stdout') + + '; stderr=' + self.output('stderr')) + + def output(self, name): + path = self.root / name + return path.read_text() if path.exists() else '' + + def start(self, mode, module, extra=(), tty=None): + env = dict(self.env, PROBE_MODE=mode) + streams = [] + for name in ('stdout', 'stderr'): + handle = (self.root / name).open('wb') + self.handles.append(handle) + streams.append(handle) + process = subprocess.Popen(['/bin/bash', str(self.script), '--no-config', + '--no-self-update', '--no-emoji', '--no-color', + '--only', module] + list(extra), env=env, + stdin=tty if tty is not None else subprocess.DEVNULL, + stdout=tty if tty is not None else streams[0], + stderr=tty if tty is not None else streams[1], + start_new_session=True) + self.processes.append(process) + return process + + def test_parent_signals_stop_active_commands(self): + for mode, module in (('brew', 'brew'), ('ncu', 'node'), + ('preflight', 'node'), ('pip', 'python'), + ('pip-version', 'python'), ('pip-help', 'python')): + for sig, status in ((signal.SIGINT, 130), (signal.SIGTERM, 143)): + with self.subTest(mode=mode, signal=sig): + for name in ('ready', 'release', 'child.pid', 'calls', 'timer.pids'): + (self.root / name).unlink(missing_ok=True) + process = self.start(mode, module) + self.until(lambda: (self.root / 'ready').exists()) + pid = int((self.root / 'child.pid').read_text()) + calls = self.module_calls() + process.send_signal(sig) + self.assertEqual(process.wait(timeout=5), status) + self.until(lambda: not self.alive(pid)) + self.until(lambda: not any(self.alive(timer) for timer in self.timer_pids())) + self.assertEqual(self.module_calls(), calls, 'Commands ran after cancellation') + self.assertIn('Interrupted', self.output('stderr')) + self.assertEqual(list(self.command_tmp.glob('updates-command.*')), [], + 'Command temporary directories survived cancellation') + + def test_interactive_stdin_is_preserved(self): + master, slave = pty.openpty() + try: + process = self.start('pty', 'brew', tty=slave) + os.close(slave) + slave = None + self.until(lambda: (self.root / 'ready').exists()) + os.write(master, b'confirmed\n') + self.assertEqual(process.wait(timeout=5), 0) + self.assertTrue((self.root / 'answered').exists()) + finally: + os.close(master) + if slave is not None: + os.close(slave) + + def test_terminal_group_interrupt_stops_runner(self): + process = self.start('brew', 'brew,node') + self.until(lambda: (self.root / 'ready').exists()) + pid = int((self.root / 'child.pid').read_text()) + # The fixture owns this newly created session and its process group. + os.killpg(process.pid, signal.SIGINT) + self.assertEqual(process.wait(timeout=5), 130) + self.until(lambda: not self.alive(pid)) + self.until(lambda: not any(self.alive(timer) for timer in self.timer_pids())) + self.assertNotIn('"ncu"', self.output('calls')) + + def test_child_interrupt_status_stops_later_modules(self): + process = self.start('brew130', 'brew,node') + self.assertEqual(process.wait(timeout=5), 130) + self.assertNotIn('"ncu"', self.output('calls')) + self.assertNotIn('"upgrade"', self.output('calls')) + + def test_brew_no_ask_is_scoped_to_noninteractive_mode(self): + for extra, expected in (((), ''), (('-n',), '1')): + with self.subTest(extra=extra): + for name in ('ready', 'release'): + (self.root / name).unlink(missing_ok=True) + process = self.start('brew', 'brew', extra) + self.until(lambda: (self.root / 'ready').exists()) + self.assertEqual(self.output('brew-no-ask'), expected) + (self.root / 'release').touch() + self.assertEqual(process.wait(timeout=5), 0) + + def test_heartbeat_is_live_and_quiet_mode_has_no_timer(self): + process = self.start('brew', 'brew') + self.until(lambda: (self.root / 'ready').exists()) + self.until(lambda: 'still running (' in self.output('stdout'), timeout=35) + self.assertIsNone(process.poll(), 'Heartbeat only appeared after completion') + self.assertIn('Ctrl+C to cancel', self.output('stdout')) + self.assertTrue(self.timer_pids()) + (self.root / 'release').touch() + self.assertEqual(process.wait(timeout=5), 0, self.output('stdout') + self.output('stderr')) + self.until(lambda: not any(self.alive(pid) for pid in self.timer_pids())) + for name in ('ready', 'release', 'timer.pids'): + (self.root / name).unlink(missing_ok=True) + quiet = self.start('brew', 'brew', ('--log-level', 'warn')) + self.until(lambda: (self.root / 'ready').exists()) + time.sleep(.15) + self.assertEqual(self.timer_pids(), [], 'Quiet command started a heartbeat helper') + (self.root / 'release').touch() + self.assertEqual(quiet.wait(timeout=5), 0) + self.assertNotIn('still running', self.output('stdout')) + self.assertNotIn('still running', self.output('stderr')) + + def test_live_npm_stderr_and_json_log_file(self): + logfile = self.root / 'updates.log' + process = self.start('npm-fail', 'node', ('--json', '--log-file', str(logfile))) + self.until(lambda: (self.root / 'ready').exists()) + self.until(lambda: 'stub diagnostic before completion' in self.output('stderr')) + self.assertIsNone(process.poll(), 'Diagnostic only became visible after completion') + (self.root / 'release').touch() + self.assertEqual(process.wait(timeout=5), 1) + self.until(lambda: 'stub diagnostic before completion' in self.output('updates.log')) + rows = [json.loads(line) for line in self.output('stdout').splitlines()] + summary = next(row for row in rows if row['event'] == 'summary') + self.assertEqual(summary['fail'], 1) + self.assertEqual(summary['failures'], ['node']) + self.assertEqual(self.output('stderr').count('stub diagnostic before completion'), 1) + self.assertNotIn('"event":', self.output('updates.log')) + self.assertEqual(list(self.command_tmp.glob('updates-command.*')), [], + 'Command temporary directories survived streaming completion') + + def test_inherited_npm_stderr_does_not_block_completion(self): + process = self.start('npm-descendant', 'node') + self.until(lambda: (self.root / 'ready').exists()) + descendant = int((self.root / 'descendant.pid').read_text()) + self.assertEqual(process.wait(timeout=5), 0, self.output('stderr')) + self.assertTrue(self.alive(descendant), 'Fixture descendant did not retain its descriptor') + self.assertIn('inherited descriptor diagnostic', self.output('stderr')) + self.assertEqual(list(self.command_tmp.glob('updates-command.*')), [], + 'Command temporary directories survived inherited-stderr completion') + # This independently started fixture process is intentionally not the + # updater's direct child. tearDown terminates its recorded PID. + + def test_parallel_pip_reuses_finished_worker_before_first_completes(self): + process = self.start('pip-scheduler', 'python', ('--parallel', '2')) + self.until(lambda: (self.root / 'ready').exists()) + first_pid = int((self.root / 'child.pid').read_text()) + self.until(lambda: (self.root / 'second.started').exists()) + self.until(lambda: 'scheduler second complete' in self.output('stdout')) + self.until(lambda: (self.root / 'third.started').exists()) + self.assertTrue(self.alive(first_pid), 'First worker stopped before its release') + self.assertIsNone(process.poll()) + self.assertIn('python: finished second', self.output('stdout')) + self.assertNotIn('python: finished first', self.output('stdout')) + (self.root / 'release').touch() + self.assertEqual(process.wait(timeout=5), 0, self.output('stderr')) + self.until(lambda: not self.alive(first_pid)) + + def test_pip_background_and_discovery_are_noninteractive(self): + for extra in ((), ('-n',)): + with self.subTest(extra=extra): + for name in ('ready', 'release', 'calls'): + (self.root / name).unlink(missing_ok=True) + process = self.start('pip', 'python', extra) + self.until(lambda: (self.root / 'ready').exists()) + calls = [json.loads(line) for line in self.output('calls').splitlines()] + installs = [call for call in calls if 'install' in call] + self.assertTrue(installs) + self.assertTrue(all('--no-input' in call for call in installs)) + if extra: + queries = [call for call in calls if 'list' in call] + self.assertTrue(queries) + self.assertTrue(all('--no-input' in call for call in queries)) + (self.root / 'release').touch() + self.assertEqual(process.wait(timeout=5), 0) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/updates b/updates index 48138e2..1570dcb 100755 --- a/updates +++ b/updates @@ -53,6 +53,8 @@ skipped_count=0 failures_count=0 CURRENT_MODULE="main" +COMMAND_PIDS=() +COMMAND_TEMPS=() COLOR_STDOUT=0 COLOR_STDERR=0 @@ -685,15 +687,155 @@ validate_only_modules_supported() { # SECTION: utilities — run wrapper, semver comparison ############################################################################### +# Keep progress separate from command stdout/stderr captures. FD 4 is opened +# after log redirection in main; these messages never enter the JSONL stream. +command_progress() { + [ "$LOG_LEVEL_NUM" -ge 2 ] || return 0 + printf '%s\n' "$*" >&4 +} + +command_forget_pid() { + local target="$1" pid + local -a remaining=() + for pid in ${COMMAND_PIDS[@]+"${COMMAND_PIDS[@]}"}; do + [ "$pid" = "$target" ] || remaining+=("$pid") + done + COMMAND_PIDS=(${remaining[@]+"${remaining[@]}"}) +} + +command_temp_dir() { + local target="$1" created + created="$(mktemp -d "${TMPDIR:-/tmp}/updates-command.XXXXXX")" || return 1 + COMMAND_TEMPS+=("$created") + printf -v "$target" '%s' "$created" +} + +command_remove_temp() { + local target="$1" path + local -a remaining=() + rm -rf -- "$target" + for path in ${COMMAND_TEMPS[@]+"${COMMAND_TEMPS[@]}"}; do + [ "$path" = "$target" ] || remaining+=("$path") + done + COMMAND_TEMPS=(${remaining[@]+"${remaining[@]}"}) +} + +command_heartbeat() { + local label="$1" started="$SECONDS" timer="" + trap - EXIT + # An interruptible wait lets the parent reap this helper immediately. + trap '[ -z "$timer" ] || { kill "$timer" 2>/dev/null; wait "$timer" 2>/dev/null; }; exit 0' TERM INT + while :; do + sleep 30 & + timer=$! + wait "$timer" || return 0 + timer="" + command_progress "$label: still running ($((SECONDS - started))s elapsed; Ctrl+C to cancel)" + done +} + +command_wait() { + local child="$1" label="$2" heartbeat="" status=0 + if [ "$LOG_LEVEL_NUM" -ge 2 ]; then + command_heartbeat "$label" & + heartbeat=$! + COMMAND_PIDS+=("$heartbeat") + fi + wait "$child" || status=$? + command_forget_pid "$child" + if [ -n "$heartbeat" ]; then + kill "$heartbeat" 2>/dev/null || true + wait "$heartbeat" 2>/dev/null || true + command_forget_pid "$heartbeat" + fi + # Some tools consume Ctrl+C and return its status instead of leaving a + # pending signal for Bash. Do not mistake that for an ordinary failure. + case "$status" in + 130) on_interrupt 130 SIGINT ;; + 143) on_interrupt 143 SIGTERM ;; + esac + return "$status" +} + +command_execute() { + local label="$1" child + shift + command_progress "$label" + # Explicit stdin redirection prevents Bash from substituting /dev/null + # for an asynchronous command. Leave job control off to retain the TTY. + "$@" <&0 & + child=$! + COMMAND_PIDS+=("$child") + command_wait "$child" "$label" +} + +command_capture() { + local destination="$1" label="$2" capture_dir="" status=0 captured + shift 2 + command_temp_dir capture_dir || return 1 + command_execute "$label" "$@" >"$capture_dir/stdout" || status=$? + captured="$(cat "$capture_dir/stdout")" + command_remove_temp "$capture_dir" + printf -v "$destination" '%s' "$captured" + return "$status" +} + +# Follow a regular file so a detached install-script child cannot hold a +# logging pipe open after the command we launched has finished. +command_follow_log() { + local log_path="$1" done_path="$2" timer="" line="" finished=0 + trap - EXIT + trap '[ -z "$timer" ] || { kill "$timer" 2>/dev/null; wait "$timer" 2>/dev/null; }; exit 0' TERM INT + exec 7<"$log_path" || return 1 + while :; do + [ ! -e "$done_path" ] || finished=1 + line="" + while IFS= read -r -u7 line; do + printf '%s\n' "$line" >&2 + line="" + done + # EOF can occur halfway through a prompt without a trailing newline. + [ -z "$line" ] || printf '%s' "$line" >&2 + [ "$finished" -eq 0 ] || return 0 + sleep 0.1 & + timer=$! + wait "$timer" || return 0 + timer="" + done +} + +command_logged() { + local label="$1" log_path="$2" stream="$3" stream_dir="" reader status=0 + shift 3 + command_temp_dir stream_dir || return 1 + : >"$log_path" || { + command_remove_temp "$stream_dir" + return 1 + } + command_follow_log "$log_path" "$stream_dir/done" & + reader=$! + COMMAND_PIDS+=("$reader") + if [ "$stream" = stderr ]; then + command_execute "$label" "$@" 2>"$log_path" || status=$? + else + command_execute "$label" "$@" >"$log_path" 2>&1 || status=$? + fi + : >"$stream_dir/done" + wait "$reader" || { [ "$status" -ne 0 ] || status=1; } + command_forget_pid "$reader" + command_remove_temp "$stream_dir" + return "$status" +} + run() { debug "+ $*" if [ "$DRY_RUN" -eq 1 ]; then return 0 fi if [ "$JSON" -eq 1 ]; then - "$@" 1>&2 + command_execute "$CURRENT_MODULE: $*" "$@" 1>&2 else - "$@" + command_execute "$CURRENT_MODULE: $*" "$@" fi } @@ -753,9 +895,9 @@ run_npm_global_install_logged() { debug "+ npm install -g $*" if [ "$JSON" -eq 1 ]; then - npm install -g "$@" 1>&2 2>"$npm_err_log" + command_logged "node: installing ${*: -1}" "$npm_err_log" stderr npm install -g "$@" 1>&2 else - npm install -g "$@" 2>"$npm_err_log" + command_logged "node: installing ${*: -1}" "$npm_err_log" stderr npm install -g "$@" fi } @@ -830,23 +972,23 @@ npm_preflight_candidate_engine() { preflight_options+=("$flag") done - local tmp_base="${TMPDIR:-/tmp}" - local npm_err_log="" - npm_err_log="$(mktemp "${tmp_base%/}/updates-npm-engine.XXXXXX" 2>/dev/null || mktemp -t updates-npm-engine.XXXXXX 2>/dev/null)" || { + local npm_log_dir="" npm_err_log="" + command_temp_dir npm_log_dir || { command_outcome_set ok command-succeeded "" return 0 } + npm_err_log="$npm_log_dir/stderr" debug "+ npm install -g ${preflight_options[*]} --dry-run --ignore-scripts --engine-strict --loglevel=error -- ${package}" npm_config_force=false npm_config_engine_strict=true npm_config_loglevel=error \ NPM_CONFIG_FORCE=false NPM_CONFIG_ENGINE_STRICT=true NPM_CONFIG_LOGLEVEL=error \ - npm install -g "${preflight_options[@]}" --dry-run --ignore-scripts --engine-strict --loglevel=error -- "$package" >/dev/null 2>"$npm_err_log" + command_execute "node: checking engine compatibility for ${package}" npm install -g "${preflight_options[@]}" --dry-run --ignore-scripts --engine-strict --loglevel=error -- "$package" >/dev/null 2>"$npm_err_log" if grep -q 'EBADENGINE' "$npm_err_log"; then command_outcome_set skip incompatible-engine "package is incompatible with the active Node runtime" else command_outcome_set ok command-succeeded "" fi - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" } run_npm_global_install_one() { @@ -862,11 +1004,10 @@ run_npm_global_install_one() { local peer_retried=0 local scripts_retried=0 - local tmp_base="${TMPDIR:-/tmp}" while :; do - local npm_err_log="" - npm_err_log="$(mktemp "${tmp_base%/}/updates-npm.XXXXXX" 2>/dev/null || mktemp -t updates-npm.XXXXXX 2>/dev/null)" || { + local npm_log_dir="" npm_err_log="" + command_temp_dir npm_log_dir || { if run npm install -g "${install_options[@]}" -- "$package"; then return 0 fi @@ -874,6 +1015,7 @@ run_npm_global_install_one() { return 1 } + npm_err_log="$npm_log_dir/stderr" run_npm_global_install_logged "$npm_err_log" "${install_options[@]}" -- "$package" local status=$? npm_classify_install_outcome "$status" "$npm_err_log" @@ -883,21 +1025,17 @@ run_npm_global_install_one() { if npm_allow_scripts_warning_in_log "$npm_err_log"; then warn "node: npm reported install scripts needing approval, but no allow-scripts list could be parsed" fi - if [ -s "$npm_err_log" ]; then - cat "$npm_err_log" >&2 - fi - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" return 0 ;; retry:peer-resolution) if [ "$peer_retried" -eq 1 ]; then err "node: npm install failed for ${package} after peer dependency retry" - [ ! -s "$npm_err_log" ] || cat "$npm_err_log" >&2 - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" return 1 fi warn "node: npm peer dependency resolution failed for ${package}; retrying with --legacy-peer-deps" - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" local -a retry_options=() local flag for flag in "${install_options[@]}"; do @@ -911,25 +1049,22 @@ run_npm_global_install_one() { retry:install-scripts) if [ "$scripts_retried" -eq 1 ]; then warn "node: npm install completed for ${package}, but npm still reports install scripts needing approval after retry" - [ ! -s "$npm_err_log" ] || cat "$npm_err_log" >&2 - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" return 0 fi warn "node: npm install scripts need approval for ${package}; retrying once with npm-provided allow-scripts list" - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" install_options=("$COMMAND_OUTCOME_RETRY_ARG" "${install_options[@]}") scripts_retried=1 ;; fail:incompatible-engine) err "node: ${package} is incompatible with the active Node runtime" - [ ! -s "$npm_err_log" ] || cat "$npm_err_log" >&2 - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" return 1 ;; *) err "node: npm install failed for ${package}" - [ ! -s "$npm_err_log" ] || cat "$npm_err_log" >&2 - rm -f "$npm_err_log" + command_remove_temp "$npm_log_dir" return 1 ;; esac @@ -970,6 +1105,14 @@ python_launcher_reset() { PYTHON_CMD_LABEL="" } +python_launcher_probe() { + if [ "$DOCTOR" -eq 1 ]; then + "$@" -c 'import sys' >/dev/null 2>&1 + else + command_execute "python: checking interpreter $*" "$@" -c 'import sys' >/dev/null 2>&1 + fi +} + python_launcher_resolve() { if [ "$PYTHON_LAUNCHER_READY" -eq 1 ]; then [ -n "$PYTHON_CMD" ] @@ -978,14 +1121,14 @@ python_launcher_resolve() { python_launcher_reset - if cmd_exists py && py -3 -c 'import sys' >/dev/null 2>&1; then + if cmd_exists py && python_launcher_probe py -3; then PYTHON_CMD="py" PYTHON_CMD_ARG="-3" PYTHON_CMD_LABEL="py -3" - elif cmd_exists python && python -c 'import sys' >/dev/null 2>&1; then + elif cmd_exists python && python_launcher_probe python; then PYTHON_CMD="python" PYTHON_CMD_LABEL="python" - elif cmd_exists python3 && python3 -c 'import sys' >/dev/null 2>&1; then + elif cmd_exists python3 && python_launcher_probe python3; then PYTHON_CMD="python3" PYTHON_CMD_LABEL="python3" fi @@ -1007,6 +1150,15 @@ python_run_cmd() { "$PYTHON_CMD" "$@" } +python_capture() { + local destination="$1" label="$2" + shift 2 + python_launcher_resolve || return 1 + local -a interpreter=("$PYTHON_CMD") + [ -z "$PYTHON_CMD_ARG" ] || interpreter+=("$PYTHON_CMD_ARG") + command_capture "$destination" "$label" "${interpreter[@]}" "$@" +} + resolve_ncu_runner() { init_node_tools @@ -1048,7 +1200,9 @@ resolve_ncu_runner() { ncu_direct_supports_engines_node() { local cmd="$1" - "$cmd" --help enginesNode 2>&1 | grep -q -- '--enginesNode' + local help="" + command_capture help "node: checking npm-check-updates capabilities" "$cmd" --help enginesNode 2>/dev/null || return 1 + [[ "$help" = *--enginesNode* ]] } init_node_tools() { @@ -1076,10 +1230,10 @@ init_fnm_node_tools() { [ -n "$fnm_cmd" ] || return 1 local fnm_env="" - fnm_env="$("$fnm_cmd" env --shell bash 2>/dev/null)" || return 1 + command_capture fnm_env "node: initializing fnm" "$fnm_cmd" env --shell bash 2>/dev/null || return 1 [ -n "$fnm_env" ] || return 1 eval "$fnm_env" >/dev/null 2>&1 || return 1 - "$fnm_cmd" use --silent-if-unchanged default >/dev/null 2>&1 || return 1 + command_execute "node: selecting default fnm runtime" "$fnm_cmd" use --silent-if-unchanged default >/dev/null 2>&1 || return 1 return 0 } @@ -1131,18 +1285,6 @@ ncu_command_label() { esac } -ncu_run_json_upgraded() { - if ! resolve_ncu_runner; then - return 1 - fi - - case "$NCU_MODE" in - cmd) "$NCU_CMD" -g --enginesNode --jsonUpgraded ;; - npx) npx --yes npm-check-updates -g --enginesNode --jsonUpgraded ;; - *) return 1 ;; - esac -} - bun_is_standalone_install() { if ! cmd_exists bun; then return 1 @@ -2517,13 +2659,14 @@ ensure_cmd_or_skip() { } json_pairs() { - # Prints tab-delimited key/value pairs from a JSON object on stdin. + local destination="$1" + # Capture tab-delimited key/value pairs from a JSON object on stdin. if python_launcher_resolve; then - python_run_cmd -c 'import sys, json; obj=json.load(sys.stdin); [print("%s\t%s" % (k, v)) for k, v in obj.items()]' + python_capture "$destination" "node: reading upgrade plan" -c 'import sys, json; obj=json.load(sys.stdin); [print("%s\t%s" % (k, v)) for k, v in obj.items()]' return $? fi if cmd_exists node; then - node -e 'let d="";process.stdin.on("data",c=>d+=c);process.stdin.on("end",()=>{const o=JSON.parse(d||"{}");for(const k of Object.keys(o)){process.stdout.write(k+"\t"+o[k]+"\n");}});' + command_capture "$destination" "node: reading upgrade plan" node -e 'let d="";process.stdin.on("data",c=>d+=c);process.stdin.on("end",()=>{const o=JSON.parse(d||"{}");for(const k of Object.keys(o)){process.stdout.write(k+"\t"+o[k]+"\n");}});' return $? fi return 1 @@ -2540,21 +2683,21 @@ python_externally_managed() { fi local out - if ! out="$( - python_run_cmd -c 'import sysconfig, pathlib; paths=sysconfig.get_paths(); c=[pathlib.Path(paths[k])/"EXTERNALLY-MANAGED" for k in ("stdlib","platstdlib","purelib","platlib") if paths.get(k)]; print("1" if any(p.exists() for p in c) else "0")' 2>/dev/null - )"; then + if ! python_capture out "python: checking environment" -c 'import sysconfig, pathlib; paths=sysconfig.get_paths(); c=[pathlib.Path(paths[k])/"EXTERNALLY-MANAGED" for k in ("stdlib","platstdlib","purelib","platlib") if paths.get(k)]; print("1" if any(p.exists() for p in c) else "0")' 2>/dev/null; then return 1 fi [ "$out" = "1" ] } python_pip_supports_break_system_packages() { - python_run_cmd -m pip install --help 2>/dev/null | grep -q -- '--break-system-packages' + local help="" + python_capture help "python: checking pip capabilities" -m pip install --help 2>/dev/null || return 1 + [[ "$help" = *--break-system-packages* ]] } python_pip_supports_guard_reports() { local help - if ! help="$(python_run_cmd -m pip install --help 2>/dev/null)"; then + if ! python_capture help "python: checking pip report support" -m pip install --help 2>/dev/null; then return 1 fi printf '%s\n' "$help" | grep -q -- '--dry-run' || return 1 @@ -2584,10 +2727,10 @@ file_lines_subset_of() { } python_guard_plan_decision() { - local report_path="$1" - shift + local destination="$1" report_path="$2" + shift 2 - python_run_cmd - "$report_path" "$@" <<'PY' + python_capture "$destination" "python: inspecting dependency plan" - "$report_path" "$@" <<'PY' import json import os import site @@ -2781,33 +2924,40 @@ module_brew() { warn "If you see a system notification like \" tried modifying your system but was stopped/prevented\", enable App Management for your terminal app (e.g. Ghostty), or rerun with --brew-mode formula." fi + local -a brew_command=(brew) + if [ "$NON_INTERACTIVE" -eq 1 ]; then + brew_command=(env HOMEBREW_NO_ASK=1 brew) + elif [ -t 0 ]; then + log "Homebrew may ask for confirmation after the upgrade table. Answer its prompt, or use --non-interactive to proceed without confirmation." + fi + if [ "$DRY_RUN" -eq 1 ]; then - log "DRY RUN: brew update" + log "DRY RUN: ${brew_command[*]} update" if [ "$BREW_CASKS" -eq 0 ]; then - log "DRY RUN: brew upgrade --formula" + log "DRY RUN: ${brew_command[*]} upgrade --formula" elif [ "$BREW_GREEDY" -eq 1 ]; then - log "DRY RUN: brew upgrade --greedy" + log "DRY RUN: ${brew_command[*]} upgrade --greedy" else - log "DRY RUN: brew upgrade" + log "DRY RUN: ${brew_command[*]} upgrade" fi if [ "$BREW_CLEANUP" -eq 1 ]; then - log "DRY RUN: brew cleanup" + log "DRY RUN: ${brew_command[*]} cleanup" fi return 0 fi - run brew update || return 1 + run "${brew_command[@]}" update || return 1 if [ "$BREW_CASKS" -eq 0 ]; then - run brew upgrade --formula || return 1 + run "${brew_command[@]}" upgrade --formula || return 1 elif [ "$BREW_GREEDY" -eq 1 ]; then - run brew upgrade --greedy || return 1 + run "${brew_command[@]}" upgrade --greedy || return 1 else - run brew upgrade || return 1 + run "${brew_command[@]}" upgrade || return 1 fi if [ "$BREW_CLEANUP" -eq 1 ]; then - run brew cleanup || return 1 + run "${brew_command[@]}" cleanup || return 1 fi return 0 @@ -2850,19 +3000,19 @@ shell_detect_omz_custom_dir() { git_repo_preflight() { local repo="$1" local branch="" - if ! branch="$(git -C "$repo" symbolic-ref --quiet --short HEAD 2>/dev/null)" || [ -z "$branch" ]; then + if ! command_capture branch "git: inspecting $repo (branch)" git -C "$repo" symbolic-ref --quiet --short HEAD 2>/dev/null || [ -z "$branch" ]; then command_outcome_set skip detached-head "git: ${repo}: detached HEAD; skipping" return 0 fi local upstream="" - if ! upstream="$(git -C "$repo" rev-parse --abbrev-ref --symbolic-full-name '@{upstream}' 2>/dev/null)" || [ -z "$upstream" ]; then + if ! command_capture upstream "git: inspecting $repo (upstream)" git -C "$repo" rev-parse --abbrev-ref --symbolic-full-name '@{upstream}' 2>/dev/null || [ -z "$upstream" ]; then command_outcome_set skip missing-upstream "git: ${repo}: branch ${branch} has no upstream; skipping" return 0 fi local worktree_status="" - if ! worktree_status="$(git -C "$repo" status --porcelain --untracked-files=no 2>/dev/null)"; then + if ! command_capture worktree_status "git: inspecting $repo (worktree_status)" git -C "$repo" status --porcelain --untracked-files=no 2>/dev/null; then command_outcome_set fail pull-failed "git: ${repo}: failed to inspect worktree state" return 0 fi @@ -2872,7 +3022,7 @@ git_repo_preflight() { fi local counts="" - if ! counts="$(git -C "$repo" rev-list --left-right --count "HEAD...@{upstream}" 2>/dev/null)"; then + if ! command_capture counts "git: inspecting $repo (counts)" git -C "$repo" rev-list --left-right --count "HEAD...@{upstream}" 2>/dev/null; then command_outcome_set fail pull-failed "git: ${repo}: failed to compare HEAD with ${upstream}" return 0 fi @@ -2910,12 +3060,15 @@ run_git_pull() { } run_git_post_pull() { - local repo="$1" + local repo="$1" previous="$PWD" status=0 + cd "$repo" || return 1 if [ "$NON_INTERACTIVE" -eq 1 ]; then - (cd "$repo" && run env GIT_TERMINAL_PROMPT=0 ./scripts/update.sh) - return $? + run env GIT_TERMINAL_PROMPT=0 ./scripts/update.sh || status=$? + else + run ./scripts/update.sh || status=$? fi - (cd "$repo" && run ./scripts/update.sh) + cd "$previous" || return 1 + return "$status" } git_sync_repositories() { @@ -3232,13 +3385,15 @@ module_node() { fi local json - if ! json="$(ncu_run_json_upgraded)"; then + local -a ncu_args=("$NCU_CMD") + if [ "$NCU_MODE" = npx ]; then ncu_args+=(--yes npm-check-updates); fi + if ! command_capture json "node: checking global package updates" "${ncu_args[@]}" -g --enginesNode --jsonUpgraded; then err "node: ncu failed" return 1 fi local pairs - if ! pairs="$(printf '%s' "$json" | json_pairs)"; then + if ! json_pairs pairs <<<"$json"; then err "node: failed to parse ncu JSON output" return 1 fi @@ -3299,7 +3454,10 @@ module_python() { return 2 fi - if ! python_run_cmd -m pip --version >/dev/null 2>&1; then + # Captured only to discard version output while retaining the exit status. + # shellcheck disable=SC2034 + local pip_version="" + if ! python_capture pip_version "python: checking pip version" -m pip --version 2>/dev/null; then if missing_dependency_is_error; then err "python: required module not found: pip (${PYTHON_CMD_LABEL} -m pip)" return 1 @@ -3326,6 +3484,7 @@ module_python() { local -a pip_install_args=("${python_args[@]}" -m pip --disable-pip-version-check install -U) if [ "$NON_INTERACTIVE" -eq 1 ]; then pip_install_args+=(--no-input) + pip_list_args+=(--no-input) fi if [ "$use_user" -eq 1 ]; then pip_list_args+=(--user) @@ -3367,13 +3526,13 @@ module_python() { fi local outdated_json - if ! outdated_json="$("${pip_list_args[@]}")"; then + if ! command_capture outdated_json "python: checking outdated packages" "${pip_list_args[@]}"; then err "python: failed to query outdated packages" return 1 fi local names - if ! names="$(printf '%s' "$outdated_json" | python_run_cmd -c 'import sys, json; data=json.load(sys.stdin); print("\n".join([p.get("name","") for p in data if p.get("name")]))' 2>/dev/null)"; then + if ! python_capture names "python: reading outdated package list" -c 'import sys, json; data=json.load(sys.stdin); print("\n".join([p.get("name","") for p in data if p.get("name")]))' <<<"$outdated_json" 2>/dev/null; then err "python: failed to parse pip output" return 1 fi @@ -3398,7 +3557,7 @@ module_python() { log "python: guarded user-site mode; dry-running wheel-only plans before installing." local guard_log_dir="" - guard_log_dir="$(mktemp -d 2>/dev/null || mktemp -d -t updates-pip-guard)" || { + command_temp_dir guard_log_dir || { err "python: failed to create temp dir for pip guard logs" return 1 } @@ -3412,7 +3571,7 @@ module_python() { report_path="${guard_log_dir}/${pkg}.json" plan_log="${guard_log_dir}/${pkg}.log" debug "+ ${pip_guard_install_args[*]} --dry-run --report $report_path $pkg" - if ! "${pip_guard_install_args[@]}" --dry-run --report "$report_path" "$pkg" >"$plan_log" 2>&1; then + if ! command_execute "python: planning $pkg" "${pip_guard_install_args[@]}" --dry-run --report "$report_path" "$pkg" >"$plan_log" 2>&1; then warn "python: skipping $pkg: pip dry-run failed" if [ -s "$plan_log" ]; then cat "$plan_log" >&"$HUMAN_FD" @@ -3420,9 +3579,9 @@ module_python() { continue fi - if ! decision="$(python_guard_plan_decision "$report_path" "$pkg")"; then + if ! python_guard_plan_decision decision "$report_path" "$pkg"; then err "python: failed to inspect pip dry-run report for $pkg" - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 1 fi old_ifs="$IFS" @@ -3439,7 +3598,7 @@ module_python() { ;; *) err "python: unexpected pip guard response for $pkg: $decision" - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 1 ;; esac @@ -3447,24 +3606,24 @@ module_python() { if [ "${#safe_packages[@]}" -eq 0 ]; then log "python: no guarded user-site packages are safe to upgrade." - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 0 fi report_path="${guard_log_dir}/safe-set.json" plan_log="${guard_log_dir}/safe-set.log" debug "+ ${pip_guard_install_args[*]} --dry-run --report $report_path ${safe_packages[*]}" - if ! "${pip_guard_install_args[@]}" --dry-run --report "$report_path" "${safe_packages[@]}" >"$plan_log" 2>&1; then + if ! command_execute "python: planning combined package set" "${pip_guard_install_args[@]}" --dry-run --report "$report_path" "${safe_packages[@]}" >"$plan_log" 2>&1; then warn "python: skipping guarded user-site install: combined pip dry-run failed" if [ -s "$plan_log" ]; then cat "$plan_log" >&"$HUMAN_FD" fi - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 0 fi - if ! decision="$(python_guard_plan_decision "$report_path" "${safe_packages[@]}")"; then + if ! python_guard_plan_decision decision "$report_path" "${safe_packages[@]}"; then err "python: failed to inspect combined pip dry-run report" - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 1 fi old_ifs="$IFS" @@ -3473,7 +3632,7 @@ module_python() { IFS="$old_ifs" if [ "$status" != "ok" ]; then warn "python: skipping guarded user-site install: $detail" - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 0 fi @@ -3481,29 +3640,23 @@ module_python() { check_baseline_err="${guard_log_dir}/pip-check-before.err" check_baseline_failed=0 debug "+ ${pip_check_args[*]}" - if ! "${pip_check_args[@]}" >"$check_baseline_log" 2>"$check_baseline_err"; then + if ! command_execute "python: checking installed dependencies" "${pip_check_args[@]}" >"$check_baseline_log" 2>"$check_baseline_err"; then check_baseline_failed=1 fi log "Upgrading ${#safe_packages[@]} guarded Python packages..." install_log="${guard_log_dir}/safe-set-install.log" debug "+ ${pip_guard_install_args[*]} ${safe_packages[*]}" - if ! "${pip_guard_install_args[@]}" "${safe_packages[@]}" >"$install_log" 2>&1; then + if ! command_logged "python: installing guarded package set" "$install_log" combined "${pip_guard_install_args[@]}" "${safe_packages[@]}"; then err "python: pip upgrade failed: guarded user-site package set" - if [ -s "$install_log" ]; then - cat "$install_log" >&"$HUMAN_FD" - fi - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 1 fi - if [ -s "$install_log" ]; then - cat "$install_log" >&"$HUMAN_FD" - fi check_log="${guard_log_dir}/pip-check.log" check_err="${guard_log_dir}/pip-check.err" debug "+ ${pip_check_args[*]}" - if ! "${pip_check_args[@]}" >"$check_log" 2>"$check_err"; then + if ! command_execute "python: checking installed dependencies" "${pip_check_args[@]}" >"$check_log" 2>"$check_err"; then if [ "$check_baseline_failed" -eq 1 ] && file_lines_subset_of "$check_log" "$check_baseline_log"; then warn "python: pip check still reports pre-existing issues after guarded upgrade" if [ -s "$check_log" ]; then @@ -3512,7 +3665,7 @@ module_python() { if [ -s "$check_err" ]; then cat "$check_err" >&"$HUMAN_FD" fi - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 0 fi err "python: pip check failed after guarded upgrade" @@ -3522,7 +3675,7 @@ module_python() { if [ -s "$check_err" ]; then cat "$check_err" >&"$HUMAN_FD" fi - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 1 fi if [ -s "$check_log" ]; then @@ -3532,7 +3685,7 @@ module_python() { cat "$check_err" >&"$HUMAN_FD" fi - rm -rf "$guard_log_dir" + command_remove_temp "$guard_log_dir" return 0 fi @@ -3543,48 +3696,63 @@ module_python() { log "Upgrading ${#packages[@]} Python packages (parallel: ${PARALLEL})..." local pip_log_dir="" - pip_log_dir="$(mktemp -d 2>/dev/null || mktemp -d -t updates-pip)" || { + command_temp_dir pip_log_dir || { err "python: failed to create temp dir for pip logs" return 1 } - local -a pids=() - local -a pid_pkgs=() - local failed=0 - - for pkg in "${packages[@]}"; do - debug "+ ${pip_install_args[*]} $pkg" - "${pip_install_args[@]}" "$pkg" >"${pip_log_dir}/${pkg}.log" 2>&1 & - pids+=("$!") - pid_pkgs+=("$pkg") - - if [ "${#pids[@]}" -ge "$PARALLEL" ]; then - local pid="${pids[0]}" - local pid_pkg="${pid_pkgs[0]}" - if ! wait "$pid"; then - err "python: pip upgrade failed: $pid_pkg" + # Background workers cannot safely share prompts on stdin. + if [ "$NON_INTERACTIVE" -eq 0 ]; then pip_install_args+=(--no-input); fi + local -a pids=() pid_pkgs=() + local failed=0 next=0 i child status started="$SECONDS" last_progress="$SECONDS" + while [ "$next" -lt "${#packages[@]}" ] || [ "${#pids[@]}" -gt 0 ]; do + while [ "$next" -lt "${#packages[@]}" ] && [ "${#pids[@]}" -lt "$PARALLEL" ]; do + pkg="${packages[$next]}" + command_progress "python: installing $pkg" + "${pip_install_args[@]}" "$pkg" "${pip_log_dir}/${pkg}.log" 2>&1 & + child=$! + COMMAND_PIDS+=("$child") + pids+=("$child") + pid_pkgs+=("$pkg") + next=$((next + 1)) + done + local -a pending=() pending_pkgs=() + for i in "${!pids[@]}"; do + child="${pids[$i]}" + pkg="${pid_pkgs[$i]}" + if kill -0 "$child" 2>/dev/null; then + pending+=("$child") + pending_pkgs+=("$pkg") + continue + fi + status=0 + wait "$child" || status=$? + command_forget_pid "$child" + case "$status" in 130) on_interrupt 130 SIGINT ;; 143) on_interrupt 143 SIGTERM ;; esac + if [ "$status" -ne 0 ]; then + err "python: pip upgrade failed: $pkg" failed=1 + else + command_progress "python: finished $pkg" fi - if [ -s "${pip_log_dir}/${pid_pkg}.log" ]; then - cat "${pip_log_dir}/${pid_pkg}.log" >&"$HUMAN_FD" + [ ! -s "${pip_log_dir}/${pkg}.log" ] || cat "${pip_log_dir}/${pkg}.log" >&"$HUMAN_FD" + done + pids=("${pending[@]}") + pid_pkgs=("${pending_pkgs[@]}") + if [ "${#pids[@]}" -gt 0 ]; then + if [ "$((SECONDS - last_progress))" -ge 30 ]; then + command_progress "python: still running ($((SECONDS - started))s elapsed): ${pid_pkgs[*]}; Ctrl+C to cancel" + last_progress="$SECONDS" fi - pids=("${pids[@]:1}") - pid_pkgs=("${pid_pkgs[@]:1}") - fi - done - - local i - for i in "${!pids[@]}"; do - if ! wait "${pids[$i]}"; then - err "python: pip upgrade failed: ${pid_pkgs[$i]}" - failed=1 - fi - if [ -s "${pip_log_dir}/${pid_pkgs[$i]}.log" ]; then - cat "${pip_log_dir}/${pid_pkgs[$i]}.log" >&"$HUMAN_FD" + sleep 0.1 & + child=$! + COMMAND_PIDS+=("$child") + wait "$child" || true + command_forget_pid "$child" fi done - rm -rf "$pip_log_dir" + command_remove_temp "$pip_log_dir" if [ "$failed" -ne 0 ]; then return 1 @@ -3886,14 +4054,32 @@ run_selected_modules() { return 0 } -on_interrupt() { - local code="$1" - local sig="$2" - local pid - for pid in $(jobs -pr); do +command_cleanup() { + local pid path + for pid in ${COMMAND_PIDS[@]+"${COMMAND_PIDS[@]}"}; do kill "$pid" 2>/dev/null || true done - err "Interrupted ($sig)." + if [ "${#COMMAND_PIDS[@]}" -gt 0 ]; then + # Give helpers time to reap their timer before escalating. + sleep 0.1 & + local grace=$! + wait "$grace" 2>/dev/null || true + fi + # Cancellation must not wait indefinitely for a tool ignoring SIGTERM. + for pid in ${COMMAND_PIDS[@]+"${COMMAND_PIDS[@]}"}; do + kill -KILL "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + done + COMMAND_PIDS=() + for path in ${COMMAND_TEMPS[@]+"${COMMAND_TEMPS[@]}"}; do rm -rf -- "$path"; done + COMMAND_TEMPS=() +} + +on_interrupt() { + local code="$1" sig="$2" + trap '' INT TERM + command_cleanup + err "Interrupted ($sig)." 1>&5 2>&6 exit "$code" } @@ -3925,6 +4111,7 @@ main() { fi fi + exec 4>&"$HUMAN_FD" 5>&1 6>&2 init_platform_cache init_colors apply_platform_defaults @@ -3934,6 +4121,7 @@ main() { trap 'on_interrupt 130 SIGINT' INT trap 'on_interrupt 143 SIGTERM' TERM + trap command_cleanup EXIT maybe_self_update "$@"