Repository navigation
feat: implement GUIHO CLI Convention 0001 - #7
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de6ed1dbc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // The shared .guiho and bin directories are never targets. Only Buda's | ||
| // stable launcher may be removed from the shared binary directory. | ||
| add(layout.Launcher, layout.Bin, "buda stable launcher", false) |
There was a problem hiding this comment.
On Windows, an installed buda uninstall runs as a payload child while the stable launcher remains alive in exec.Command(...).Run(). This plan removes that still-running launcher and later its active payload synchronously, which Windows rejects for mapped executables, so a normal Windows uninstall fails after any earlier plan items have already been deleted. The Windows path needs a detached cleanup/helper mechanism rather than applying these removals in-process.
Useful? React with 👍 / 👎.
| # The installed Cobra command is the canonical manifest-driven planner. The | ||
| # script remains usable for recovery when the launcher is missing or corrupt. | ||
| if [ -x "$BIN_DIR/buda" ] && [ "${BUDA_UNINSTALL_FALLBACK:-0}" != "1" ]; then | ||
| set -- uninstall --yes |
There was a problem hiding this comment.
When the normal installed launcher exists, invoking uninstall.sh without --yes still delegates as buda uninstall --yes, so it immediately removes configuration and data without the advertised interactive confirmation; noninteractive calls also no longer fail closed. The PowerShell script mirrors this behavior. Append the confirmation flag only when the caller actually supplied it.
Useful? React with 👍 / 👎.
| case "$(uname -s):$(uname -m)" in | ||
| Linux:x86_64|Linux:amd64) OS=linux; ARCH=amd64; BINARY=buda-linux-amd64; LAUNCHER=buda-launcher-linux-amd64 ;; | ||
| Linux:aarch64|Linux:arm64) OS=linux; ARCH=arm64; BINARY=buda-linux-arm64; LAUNCHER=buda-launcher-linux-arm64 ;; | ||
| Darwin:x86_64|Darwin:amd64) OS=darwin; ARCH=amd64; BINARY=buda-darwin-amd64; LAUNCHER=buda-launcher-darwin-amd64 ;; | ||
| Darwin:arm64) OS=darwin; ARCH=arm64; BINARY=buda-darwin-arm64; LAUNCHER=buda-launcher-darwin-arm64 ;; | ||
| *) printf 'unsupported platform: %s:%s\n' "$(uname -s)" "$(uname -m)" >&2; exit 2 ;; |
There was a problem hiding this comment.
On Linux ARMv6/ARMv7 hosts, uname -m commonly returns armv6l or armv7l, but both now reach the unsupported-platform branch even though the release builder still publishes matching payloads and launchers. Restore mappings to buda-linux-armv6/armv7 and their launcher assets so the published targets remain installable on native hosts.
AGENTS.md reference: AGENTS.md:L54-L57
Useful? React with 👍 / 👎.
| if !yes { | ||
| fmt.Fprint(command.OutOrStdout(), "Remove all Buda-owned artifacts listed above? [y/N] ") |
There was a problem hiding this comment.
For a direct interactive buda uninstall, the plan is printed only in the --dry-run branch; the ordinary path instead asks users to remove artifacts “listed above” even though no list was emitted. Users therefore cannot inspect which configuration, data, skills, or wiki files will be preserved or removed before approving the destructive operation.
Useful? React with 👍 / 👎.
| if (ai ~ /^[0-9]+$/) return 1 | ||
| if (bi ~ /^[0-9]+$/) return 0 |
There was a problem hiding this comment.
When channel discovery compares versions whose corresponding prerelease identifiers differ by type, such as 1.0.0-beta.1 and 1.0.0-beta.alpha, SemVer gives the numeric identifier lower precedence, but these branches report it as greater. The installer can consequently select an older channel release instead of the highest compatible version; reverse the mixed numeric/non-numeric results.
Useful? React with 👍 / 👎.
Close the release blockers from the review round with evidence: - chmod staged POSIX candidates before execution (install.sh and the upgrade transaction); - capture replacement edits known fields only and preserves unknown OKF metadata; - manifest projection snapshot/apply/retire/rollback plus journal recovery states are tested; - symlink/reparse-safe quarantined transactional uninstall with same-filesystem staging, instruction snapshots, and data-loss-free rollback; Windows reparse points detected through file attributes; - init discovers every unanswered policy before any write and migrates the legacy global config strictly; - complete 24-binary release matrix with duplicate rejection and typed launcher metadata in Go, jq, and PowerShell selectors; - meaningful zero-mutation self-test and precise --version/__self-test instance bypass; - legacy 0.1.x direct-binary migration only after verified launcher activation from the exact historical paths; - explicit wiki and grouped REMOVE/PRESERVE plans before uninstall confirmation; the plan removes the payload so the legacy-executable step no longer fails after success; - embedded schema/example runtime parity tests; - pointer EOF and canonical payload-name validation, all-zero digest rejection except artifacts.json, and strict checksum parsing; - BUDA_DISABLE_MAINTENANCE is now honored by the maintenance scheduler.
- CI gains POSIX and Windows native lifecycle jobs: install, repair, synthetic 0.1.1 migration, and synchronous uninstall in disposable homes with qmd compatibility stubs and sentinel preservation; - native launcher smokes run through a disposable pointer fixture on every hosted runner matrix; - actionlint-clean workflows; lifecycle transaction and interruption acceptance is an explicit CI step and a RunX catalog entry; - RunX cross targets build through the target-aware release builder; - TODO records the blocker-to-evidence table for the review fix pass; - README documents legacy 0.1.x migration truth; XDocs workflow descriptors match the implemented jobs.
…r Windows native smoke
1 participant
Summary
Implements the approved GUIHO CLI Convention 0001 migration for Buda while preserving the explicit single-wiki, qmd-only, OKF-compatible product contract.
Validation
go mod tidy -diffgo test -count=1 ./...go vet ./...go build ./...0.2.0-dev.0runx check --format json,runx list --format json, all 28 catalog dry-runsxdocs meta . --documents --strict,xdocs tree,xdocs doctor . --warnings-as-errorsmirror config check,mirror version current(0.1.1),mirror version plan minor(0.2.0,buda/v0.2.0)artifacts.json,checksums.txt; 25 files / 24 manifest artifacts; all checksums, native payload version/self-test, and isolated launcher smoke passed.Review-gated boundaries
Native installer/migration mutation against a personal installation and crash/locked-file recovery scenarios remain explicitly review-gated; the PR validation uses temporary fixtures. No tag, GitHub Release, merge, deployment, DNS, database, secret, or production action was performed.
The PR is ready for independent review. Please review the exact PR head against the durable acceptance matrix in
docs/GUIHO_CLI_CONVENTION_0001_ACCEPTANCE_MATRIX.mdbefore merging.