Skip to content

fix: reconstruct the setup/install safety wave#7

Open
time-attack wants to merge 14 commits into
mainfrom
time-attack/wave-setup-install-isolated-review
Open

fix: reconstruct the setup/install safety wave#7
time-attack wants to merge 14 commits into
mainfrom
time-attack/wave-setup-install-isolated-review

Conversation

@time-attack

Copy link
Copy Markdown
Owner

Summary

Reconstructs the setup/install cleanup wave from the immutable upstream base 7c9df1c568a9ea745508f679a329332b2c338063. The old wave branch was not merged or cherry-picked; each retained problem was independently reimplemented as an atomic commit.

  • Make setup and iOS daemon help paths side-effect free, shell-quote resolved state paths, and strictly validate benchmark timeout input.
  • Preserve malformed settings and foreign skill directories during configuration and uninstall.
  • Preserve explicit Git push protocols and fail closed when transcript repository trust is missing, corrupt, denied, read-only, or belongs to another repository.
  • Bound Playwright downloads and launches, terminate descendant process trees safely, and retain non-browser skill installation after browser failures.
  • Install refreshable, ownership-marked Hermes skills without overwriting foreign collisions.
  • Exclude private iOS touch APIs from Release archives.
  • Stop Conductor from implicitly installing the AskUserQuestion PreToolUse hook while preserving explicit opt-in.
  • Clarify that uninstall state deletion is optional.

This PR intentionally does not modify VERSION, package version, CHANGELOG.md, ETHOS, or workflows. Each accepted problem remains a separate commit for selective review.

Attribution

Accepted or partially reconstructed from PRs garrytan#902, garrytan#1143, garrytan#1219, garrytan#1469, garrytan#1580, garrytan#1727, garrytan#1813, garrytan#1845, garrytan#1900, garrytan#1932, garrytan#2119, garrytan#2148, garrytan#2208, garrytan#2225, garrytan#2232, garrytan#2233, and garrytan#2239. Competing fixes were compared by behavior rather than file overlap; unsafe and overbroad hunks were excluded.

Live verification

Seven isolated pr-test-env campaigns passed. Each used three real consumer repositories that failed on immutable origin/main and passed on this candidate, plus two unchanged controls using the identical probe:

  • command-boundary behavior
  • settings and destructive-uninstall ownership
  • exact HTTPS ls-remote and real Git push
  • transcript trust policy
  • hung Playwright process-tree cleanup
  • real Hermes discovery and loading
  • Conductor 0.74.0 implicit-hook behavior with explicit-opt-in controls

Additional evidence:

  • Linux arm64 container completed real Bun install, Chromium/FFmpeg/headless-shell downloads, setup, rerun, and safe uninstall.
  • Apple Silicon setup-built binaries passed strict code-sign verification.
  • Xcode 26.6 generic Release archive passed a private touch API/symbol scan.
  • Real Chromium 145 launched successfully; a deliberately hung proxy timed out with no surviving marked descendants.
  • Targeted suite: 134 passed, 0 failed before the final Conductor fix; its focused suite adds 12 passed, 0 failed.
  • bun run build: passed on the final head.
  • git diff --check origin/main..HEAD: passed.

Known boundary

Native Windows was unavailable. The Windows-safe aggregate runner reached shard 5 and then hit an unrelated pre-existing terminal-agent source-string assertion (spawnClaude versus maybeSpawnPty); no file in this PR touches that component.

The installed Conductor settings currently demonstrate why automatic migration is unsafe: implicit and explicit historical hook registrations use the same ownership tag. This PR prevents new implicit registrations but does not silently remove existing user settings.

Base and package

  • Reviewed upstream base: 7c9df1c568a9ea745508f679a329332b2c338063
  • Candidate head: 87b0600fb3461f2c450af192bb339128dc1b4c69
  • Verdict: MERGE-WITH-CHANGES

The fork's main is five commits ahead of the reviewed upstream base. A local merge-tree simulation completed without conflict; README was the only path changed on both sides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants