Guard direct GhosttyKit setup against incompatible Zig - #4706
teamleaderleo merged 11 commits into
Conversation
|
@Bortlesboat is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a centralized Zig version guard used by setup flows, regression tests covering version scenarios, a CI step running the tests, and contributor instructions for Ghostty’s required Zig version. ChangesZig version guard enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Contributor
participant SetupScripts
participant ZigGuard
participant ZigCLI
participant CI
Contributor->>SetupScripts: Run setup or GhosttyKit setup
SetupScripts->>ZigGuard: Validate required Zig version
ZigGuard->>ZigCLI: Run zig version
ZigCLI-->>ZigGuard: Return installed version
ZigGuard-->>SetupScripts: Return validation status
CI->>ZigGuard: Run regression scenarios
ZigGuard-->>CI: Report pass or failure
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds early Zig compatibility checks for GhosttyKit setup. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (6): Last reviewed commit: "test(setup): exercise legacy Zig bypass ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
2aa4b9c to
2c6599f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Refreshed onto current main in b16fa20. The CI conflict retains both the Zig guard and the newer upstream Zig-install/virtual-display checks. Ghostty still pins 0.15.2, the upstream-relative diff remains six focused files, staged shell sources pass bash syntax checks, and the upstream-relative diff check is clean. Per repository policy, execution tests are left to GitHub Actions. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 165-166: Update the workflow checkout configuration used by
“Validate Zig version guard” to initialize submodules recursively before running
tests/test_ci_zig_version_guard.sh. Preserve the existing test invocation and
ensure ghostty/build.zig.zon is available on clean runners.
In `@CONTRIBUTING.md`:
- Around line 7-10: Update the Zig requirement in CONTRIBUTING.md to state that
the version must match Ghostty’s required major/minor and have patch version
0.15.2 or newer, including that newer minor releases are rejected. Replace the
placeholder install command value with the concrete copy-pastable example
ZIG_REQUIRED=0.15.2.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a6be3a1-c6bb-4ea9-bac5-e4cd3ec2fff9
📒 Files selected for processing (2)
.github/workflows/ci.ymlCONTRIBUTING.md
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bd3f389. Configure here.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
87-91: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable credential persistence on this checkout.
This job only runs read-only git commands after checkout, so the token does not need to stay in local git config. Add
persist-credentials: falsehere to keep repository-controlled validation steps and submodule content from reusing the checkout token.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 87 - 91, Update the Checkout action configuration in the CI workflow to set persist-credentials to false alongside fetch-depth and submodules, ensuring the checkout token is not retained for subsequent read-only git operations.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 87-91: Update the Checkout action configuration in the CI workflow
to set persist-credentials to false alongside fetch-depth and submodules,
ensuring the checkout token is not retained for subsequent read-only git
operations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7f291f32-666d-4bcb-afc5-a3f029066cbd
📒 Files selected for processing (2)
.github/workflows/ci.ymlCONTRIBUTING.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_ci_zig_version_guard.sh`:
- Around line 109-113: Extend the test around the CI Zig version guard to
execute the guard with CMUX_ZIG_VERSION_CHECKED=1 and a legacy required-version
override configured, using an incompatible Zig version, then assert that
validation fails. Keep the existing source-text checks, but verify the runtime
bypass path through the guard script rather than only checking identifier
absence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a4eb686-336d-4606-9729-c37dbcbc78a1
📒 Files selected for processing (3)
.github/workflows/ci.ymlscripts/check-zig-version.shtests/test_ci_zig_version_guard.sh
b7e7300 to
e5c59ba
Compare
Bortlesboat
changed the title
|
Thank you for this, @Bortlesboat! |
|
All contributors have signed the CLA ✍️ ✅ |
|
Moved the Zig check into the source-build fallback in 0243500. Cached GhosttyKit, a matching local artifact, and a checksum-pinned download now work without invoking Zig; source builds still require the manifest-compatible version. Added a CI regression covering all three reuse paths with Zig missing and incompatible, plus source builds with missing, incompatible, and compatible Zig. The six reuse cases fail before the fix and all nine cases pass afterward. The existing Zig/workflow guard tests also pass. These shell checks ran under Linux; I did not run a macOS app build. Refreshed against upstream |
|
I have read the CLA Document v2.2 and I hereby sign the CLA |
|
Merged, thank you @Bortlesboat! Moving the Zig check to only the build-from-source path was exactly right: cache hits and prebuilt downloads no longer need Zig at all, and a mismatched Zig now fails with a clear message instead of a confusing build error. Squash-merged as 537d53f :) |
|
Merge receipt for |
b63ab49 Fail remote-tmux review when a wait is a timer instead of an event (manaflow-ai#11264) b436c92 Fail closed when CLI forwarding loops back to the GUI binary (manaflow-ai#8788) 136eb2a fix(ios): make the last intermittent CmuxMobileShell tests deterministic (manaflow-ai#14721) dc7f5bd fix(ios): keep terminal composer dock at bottom (manaflow-ai#14702) cb4429b ci: add owned_build_state.py warm-keys for warm admission routing (manaflow-ai#14717) be1ab5e ci(ios): charge in-flight auto runs simulators only when they took the fleet (manaflow-ai#14716) 5110582 ci: clear fixed DerivedData by renaming it aside first (manaflow-ai#14710) 1b8a603 fix(ios): unregister terminal output streams by registration identity; fix stale render-grid tests (manaflow-ai#14711) 3cae7dd refactor: move the browser WebKit support layer into CmuxBrowser (manaflow-ai#14398) 6693884 Run the Codex monitor Stop replay outside the hook handler frame (manaflow-ai#14715) 133a083 ci(e2e): take the owned Mac's gui token just before testing in the build (manaflow-ai#14705) 4c36ace ci: re-run every job of an E2E run whose build did not succeed (manaflow-ai#14712) ab5ac72 ci: start owned E2E builds from the Mac's kept state, upload after tests (manaflow-ai#14692) 6a3d2d0 test(ios): align Mac switch and pool tests with build-scoped identity (manaflow-ai#14708) 0b16a8b Keep closePanel's unmapped fallback from closing another panel's tab (manaflow-ai#14704) 69c9518 close-surface: reject a blank --workspace or --window (manaflow-ai#14706) 8476043 ci: route warm admission by static runner labels, never write labels (manaflow-ai#14696) 537d53f Guard direct GhosttyKit setup against incompatible Zig (manaflow-ai#4706) 4b200e1 Restore Pi wakeup alerts across reloads (manaflow-ai#12861) 9b291c0 ci: resolve an owned Mac's kept Swift packages offline (manaflow-ai#14709) 31c9105 Stop attaching the unverified per-resume relay MAC (manaflow-ai#14694) 5443f43 Merge pull request manaflow-ai#13565 from manaflow-ai/feat-recover-forgotten-computers e817240 Add app.equalizeSplitsOnCreate to balance panes on new splits (manaflow-ai#14703) a358095 fix: never use a shell bootstrap executable in a resume binding (manaflow-ai#5848) 3f177af Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers 8b4c97b Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers dce9509 ci: update iOS checkout routing assertion 5dbf06c ci: update CLA workflow digest after main pin 44c00fb Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers d22189d Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers eeca51a fix: make event reconnect policy instance based 9e29c96 Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers d860564 Model first Cloud receipt before remote graph discovery 87f1305 Verify forgotten Mac recovery with the real paired store 2237f8c Merge main and retain upstream test repairs 9ccaa7a Avoid type-check timeout in process fixture a848b5e Simplify AppKit accessibility test setup 82a9769 Isolate process generation fixture from runner TTY state 4c129bb Enable and restore AppKit assistive access in mounted tree test 9a9acca Enable accessibility output in the hosted SwiftUI test fixture 5ce39d0 Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers a29a7ec Read proxy accessibility children and text through one attribute bridge 12938b6 Test accessibility walkers against modern and legacy proxy nodes 04a30c5 Revoke the original pairing in the targeted dial authority regression ff1888e Import the extracted Cloud module in the team picker 9c9f259 Merge main and adopt the verified focus recovery fixture 5d41fc8 Align hosted UI fixtures with runtime paths and presentation lifecycle 87dbbd9 Preserve unknown legacy restore liveness after main integration 41a8d4b Revert "Establish running agent state in auto-resume fixtures" 6c7d14b Revert "Use the running-agent fixture for second-restore cwd coverage" 9d276b5 Route Kiro permission-mode fixtures through the mock delivery target cdb4eae Restore the host app delegate after registration tests 4bdc410 Use the running-agent fixture for second-restore cwd coverage 0b58cbc Establish running agent state in auto-resume fixtures 20c6415 Wait for mounted project content in accessibility test 3ba7b6a Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers 2c3c680 Make reparent focus test geometry deterministic d2f563b Stabilize canonical cache recipe guard cc60792 Merge current main into forgotten Mac recovery 7dc515f Merge branch 'main' into feat-recover-forgotten-computers cfd4773 Preserve per-instance forgotten Mac recovery 6a95584 Canonicalize recovered directory identities f083246 Preserve queued forgotten Mac refreshes e5202a6 test(ios): use canonical UUID duplicate fixture ca7ee53 fix(ios): serialize forgotten Mac recovery retries db851c2 Merge remote-tracking branch 'origin/main' into feat-recover-forgotten-computers 35269cb fix(ios): rehydrate Macs after forget recovery d0266b7 Merge current CI workflow identity guard into device recovery 45b7c72 fix(i18n): distinguish signed-in Mac recovery from connectivity 730260c Merge remote-tracking branch 'origin/feat-recover-forgotten-computers' into feat-recover-forgotten-computers a17f8dd fix: complete forgotten Mac recovery without replaying revocation d64766a test: reproduce revocation during device recovery registration faf5b55 test: keep recovery authority limited to enrollment a9c437a test: cover duplicate forget and recovery lifecycle gaps 033e7a0 chore: normalize project ordering 28f63ec fix: recover forgotten Macs from revocation events 7b8e322 fix: recover forgotten Mac registrations f1aca40 test: validate recovery enrollment proof 62b4e14 test: cover authenticated forgotten-device recovery 9cb2f97 test: cover recovery after device forget # Conflicts: # .github/workflows/app-host-test-rerun.yml # .github/workflows/ci-guards.yml # .github/workflows/ci-macos.yml # .github/workflows/ci-queue-janitor.yml # .github/workflows/ci.yml # .github/workflows/nightly.yml # .github/workflows/seed-derived-data.yml # .github/workflows/test-e2e.yml # .github/workflows/test-ios.yml
2 participants
Summary
ghostty-zig-version.shhelpersetup.shand directensure-ghosttykit.shentry pointsUpstream alignment
Current
mainadded a shared Ghostty Zig-version helper and setup validation after this PR was opened. This refresh uses that architecture and removes the old duplicate guard script, workflow step, and hard-coded version documentation. Prerelease/build suffixes retain current upstream compatibility semantics.Testing
bash -n scripts/ghostty-zig-version.sh scripts/setup.sh scripts/ensure-ghosttykit.sh tests/test_ghostty_zig_version_sync.sh./tests/test_ghostty_zig_version_sync.sh(with the workflow's pinnedPyYAML==6.0.3andbashlex==0.18; passes)git diff upstream/main...HEAD --checkThe repository's required tagged macOS reload was not runnable from this Windows environment; this PR changes setup shell paths only.
Security / Privacy