Skip to content

Exempt quoted script-path strings from Ghostty Zig workflow guard - #9201

Closed
azooz2003-bit wants to merge 2 commits into
mainfrom
fix-ghostty-zig-guard-strings
Closed

azooz2003-bit wants to merge 2 commits into
mainfrom
fix-ghostty-zig-guard-strings

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 30, 2026 •

Copy link
Copy Markdown
Collaborator

Two guard steps in workflow-guard-tests fail on every branch containing current main; this PR fixes both so the cheap CI layer gates real regressions again.

  1. Ghostty Zig sync guard: since Batch iOS TestFlight uploads on a schedule instead of per merge #9165, tests/check_ghostty_zig_workflows.py flags ios-testflight.yml's decide job because it lists scripts/install-zig-ci.sh and scripts/ghostty-zig-version.sh as plain JS string entries in a change-detection path array inside a github-script step. The job never executes them. The checker now skips lines that consist solely of a quoted scripts/... path (optionally with a trailing comma), matching the existing exemption for YAML list items. Verified: passes against current workflows, still fails a synthetic workflow that really invokes the resolver without prior submodule init.

  2. Test determinism gate: scripts/check-test-determinism.py --strict reports 5 sleep-then-assert findings that landed via Keep iOS control connections warm across paired Macs #8931 (MobileCoreRPCTransportDrainTests) and iOS: keep Mac discovery alive through onboarding so the connect page is ready on arrival #9163 (OnboardingMacDiscoveryKeepAliveTests). Grandfathered into .github/test-determinism-allowlist.txt with provenance notes; determinizing those tests remains follow-up work, and removing the entries re-arms the gate.

Summary by CodeRabbit

  • Bug Fixes
    • Improved workflow validation to ignore standalone quoted script-path declarations.
    • Prevented these declarations from being incorrectly treated as requiring submodule initialization.
  • Tests
    • Updated the test determinism allowlist by removing two legacy suppressed entries and adding targeted suppressions for specific iOS Swift tests until they can be fully determinized.

The guard flagged ios-testflight.yml's decide job after #9165 added
scripts/install-zig-ci.sh and scripts/ghostty-zig-version.sh as plain
JS string entries in a change-detection path list. A line that is only
a quoted script path names the consumer without executing it, so it
does not need prior submodule init.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 30, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

workflow_failures() now skips standalone quoted scripts/... YAML paths, and the test-determinism allowlist replaces two legacy suppressions with temporary iOS test entries.

Changes

Validation updates

Layer / File(s) Summary
Filter standalone script paths
tests/check_ghostty_zig_workflows.py
workflow_failures() ignores standalone quoted scripts/... declarations before consumer-reference checks.
Update determinism suppressions
.github/test-determinism-allowlist.txt
Removes two legacy cmuxTests entries and adds two temporary iOS test suppressions marked for later removal.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the changes well, but it omits required template sections like Testing, Demo Video, Review Trigger, and Checklist. Add the missing template sections, especially Testing, Review Trigger, and Checklist, and include a demo video link only if applicable.
✅ Passed checks (24 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed This commit only changes a test allowlist; no production Swift files or actor-isolation-relevant declarations are introduced or worsened.
Cmux Swift Blocking Runtime ✅ Passed PR only updates .github/test-determinism-allowlist.txt; no Swift sources or blocking/timing primitives were introduced or expanded.
Cmux Browser Automation Off-Main ✅ Passed PR only changes .github/test-determinism-allowlist.txt; it does not touch browser-automation code, so the rule is out of scope.
Cmux Expensive Synchronous Load ✅ Passed No production Swift files changed; the diff only updates a test-determinism allowlist, so the expensive sync-load rule is not implicated.
Cmux Cache Substitution Correctness ✅ Passed No cache-substitution change here: HEAD only updates a test-determinism allowlist, with no production Swift/TS/JS persistence/history/undo/snapshot path touched.
Cmux No Hacky Sleeps ✅ Passed Only a test-determinism allowlist changed; no runtime sleeps or delay logic were introduced, and workflow YAML is out of scope.
Cmux Algorithmic Complexity ✅ Passed Only a test checker and allowlist changed; the new regex is constant-time and no scalable production path was added.
Cmux Swift Concurrency ✅ Passed The diff only changes a test allowlist entry; no cmux-owned Swift code or legacy concurrency patterns were introduced or expanded.
Cmux Swift @Concurrent ✅ Passed No Swift source files changed; the PR only edits a text allowlist, so the @concurrent rule is not applicable.
Cmux Swift Package Boundaries ✅ Passed Diff only updates a test-determinism allowlist; no production Swift files changed, so the Swift package-boundary rule is not implicated.
Cmux Swiftpm Lockfiles ✅ Passed Only .github/test-determinism-allowlist.txt changed; no SwiftPM, Xcode, .gitignore, or Package.resolved files were touched.
Cmux Swift Logging ✅ Passed PR changes only Python and allowlist text; no production Swift code or logging was added or modified.
Cmux User-Facing Error Privacy ✅ Passed Only the test-determinism allowlist changed; no user-facing errors, alerts, output, or recovery copy were modified.
Cmux Full Internationalization ✅ Passed Diff only changes a test-determinism allowlist; no user-facing Swift/web text or locale files were touched.
Cmux Swiftui State Layout ✅ Passed The diff only changes a Python test and an allowlist; no SwiftUI views, state, GeometryReader, or render-time mutations are introduced.
Cmux Architecture Rethink ✅ Passed PASS: The PR only changes a Python workflow-checker and an allowlist; no Swift architecture changes or symptom-patch patterns are introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR only changes Python and an allowlist; no Swift window code or cmuxAuxiliaryWindowIdentifiers changes, so the rule doesn’t apply.
Cmux Source Artifacts ✅ Passed Only .github/test-determinism-allowlist.txt changed, which is an intentional repo config file; no logs, caches, build output, or other artifacts were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No production Swift Sources files changed; only a .github allowlist file was modified, so the seam rule is not implicated.
Cmux No Ambient Global State ✅ Passed The PR only changes a Python test helper and an allowlist; there are no Swift production files in the diff.
Title check ✅ Passed The title clearly and concisely summarizes the main Ghostty workflow guard change.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-ghostty-zig-guard-strings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

scripts/check-test-determinism.py --strict fails on current main for
every branch because MobileCoreRPCTransportDrainTests and
OnboardingMacDiscoveryKeepAliveTests merged with sleep-then-assert
patterns. Grandfather them with provenance so the guard gates new
findings again; the tests still need determinizing as follow-up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@azooz2003-bit

Copy link
Copy Markdown
Collaborator Author

Superseded: #9209 landed a bashlex-based fix for the same zig-guard false positive plus the determinism allowlist, and #9213 fixed the CmuxFoundation type-check timeout. Nothing left for this PR to fix.

@azooz2003-bit
azooz2003-bit deleted the fix-ghostty-zig-guard-strings branch July 30, 2026 19:33
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.

1 participant