From 7c82f5af2b9748472c7e37960b6001860a2e72fe Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 22 May 2026 00:06:20 -0700 Subject: [PATCH 1/6] ci: lint that every cmuxTests Swift file is wired into pbxproj MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Catches the class of bug surfaced during the https://github.com/manaflow-ai/cmux/issues/4529 investigation: a test file added to the worktree without a matching entry in cmux.xcodeproj/project.pbxproj is silently ignored by Xcode and never compiles or runs on CI. Both bot reviews and `xcodebuild test -only-testing:cmuxTests/` pass with "Executed 0 tests" — so the missing wiring is indistinguishable from a clean two-commit red/green regression test until a real user hits the bug the test was supposed to catch. This is the RED commit of a two-commit pattern: introducing the lint immediately flags two pre-existing orphans on main (SessionIndexViewTests.swift and SidebarMarkdownRendererTests.swift) and the new `workflow-guard-tests` step turns red. The follow-up commit wires those two files into the cmuxTests target so the lint goes green. Refs https://github.com/manaflow-ai/cmux/issues/4559 Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 3 + scripts/lint-pbxproj-test-wiring.sh | 96 ++++++++++++++++++++++++++++ tests/test_ci_pbxproj_test_wiring.sh | 48 ++++++++++++++ 3 files changed, 147 insertions(+) create mode 100755 scripts/lint-pbxproj-test-wiring.sh create mode 100755 tests/test_ci_pbxproj_test_wiring.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fef02f61e67d..21a2e60dff66 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,9 @@ jobs: - name: Validate auxiliary window close shortcut lint run: ./tests/test_ci_auxiliary_window_close_shortcuts.sh + - name: Validate pbxproj test-wiring lint + run: ./tests/test_ci_pbxproj_test_wiring.sh + # Paused: stale-base merge races (two PRs each fitting the budget can # overshoot when merged back-to-back without rebasing). CodeRabbit and # Greptile already flag large-file growth on PRs. Re-enable by uncommenting diff --git a/scripts/lint-pbxproj-test-wiring.sh b/scripts/lint-pbxproj-test-wiring.sh new file mode 100755 index 000000000000..d917efcf0d34 --- /dev/null +++ b/scripts/lint-pbxproj-test-wiring.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# Lint: every Swift file under cmuxTests/ must be wired into +# cmux.xcodeproj/project.pbxproj. +# +# A test file added to the worktree but not registered as a PBXFileReference + +# PBXSourcesBuildPhase entry in project.pbxproj is silently ignored by Xcode and +# never compiles or runs on CI. Both bot reviews and +# `xcodebuild test -only-testing:cmuxTests/` pass with +# "Executed 0 tests" — so missing wiring is indistinguishable from a passing +# regression test until a real user hits the bug the test was supposed to catch. +# +# Originally surfaced during the https://github.com/manaflow-ai/cmux/issues/4529 +# investigation, where SessionIndexJSONLStreamTests.swift on +# https://github.com/manaflow-ai/cmux/pull/4536 looked like a clean two-commit +# red/green test fix but never actually ran on CI. +# +# Usage: +# ./scripts/lint-pbxproj-test-wiring.sh [--repo-root ] +# +# Exit codes: +# 0 — all test files wired correctly (or no test files present) +# 1 — at least one test file is missing pbxproj wiring +# 2 — invocation error (e.g. project.pbxproj not found) + +set -euo pipefail + +REPO_ROOT="" +while [ "$#" -gt 0 ]; do + case "$1" in + --repo-root) + REPO_ROOT="$2" + shift 2 + ;; + -h|--help) + sed -n '1,25p' "$0" | sed 's/^# *//' + exit 0 + ;; + *) + echo "Unknown argument: $1" >&2 + exit 2 + ;; + esac +done + +if [ -z "$REPO_ROOT" ]; then + REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd -P)" +fi + +PBXPROJ="$REPO_ROOT/cmux.xcodeproj/project.pbxproj" +TESTS_DIR="$REPO_ROOT/cmuxTests" + +if [ ! -f "$PBXPROJ" ]; then + echo "lint-pbxproj-test-wiring: not found: $PBXPROJ" >&2 + echo " (run from the cmux repo root or pass --repo-root)" >&2 + exit 2 +fi +if [ ! -d "$TESTS_DIR" ]; then + echo "lint-pbxproj-test-wiring: not found: $TESTS_DIR" >&2 + exit 2 +fi + +missing=() +checked=0 + +while IFS= read -r -d '' file; do + base="$(basename "$file")" + checked=$((checked + 1)) + # Each wired test file shows up 4x in pbxproj: build file UUID, + # file reference UUID, group children list, and target sources phase. + # Require at least 2 references so we flag both "added but missing + # entirely" and "stub reference but no sources phase" failures. + hits="$(grep -c -- "$base" "$PBXPROJ" || true)" + if [ "$hits" -lt 2 ]; then + missing+=("$base (hits=$hits)") + fi +done < <(find "$TESTS_DIR" -maxdepth 1 -type f -name '*.swift' -print0) + +if [ "${#missing[@]}" -eq 0 ]; then + echo "lint-pbxproj-test-wiring: ok (checked $checked test files)" + exit 0 +fi + +echo "lint-pbxproj-test-wiring: ${#missing[@]} test file(s) not wired into cmux.xcodeproj/project.pbxproj" +for entry in "${missing[@]}"; do + echo " - $entry" +done +echo "" +echo "Each cmuxTests/.swift must appear in cmux.xcodeproj/project.pbxproj as:" +echo " 1. a PBXBuildFile entry (UUID = '.swift in Sources')" +echo " 2. a PBXFileReference entry (UUID = '.swift')" +echo " 3. an entry in the cmuxTests group children list" +echo " 4. an entry in the cmuxTests target's PBXSourcesBuildPhase files" +echo "" +echo "Add via Xcode (drag the file into the cmuxTests target) or hand-edit" +echo "the four blocks (see any wired sibling test as a template)." +exit 1 diff --git a/tests/test_ci_pbxproj_test_wiring.sh b/tests/test_ci_pbxproj_test_wiring.sh new file mode 100755 index 000000000000..5895a08439e5 --- /dev/null +++ b/tests/test_ci_pbxproj_test_wiring.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# CI guard for ./scripts/lint-pbxproj-test-wiring.sh. +# +# Verifies the lint script (a) reports "ok" on the real cmux repo, and (b) +# correctly fails when a test file is dropped in without pbxproj wiring. +# The second case is what prevents the lint itself from rotting into a no-op. + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$ROOT_DIR" + +LINT="$ROOT_DIR/scripts/lint-pbxproj-test-wiring.sh" +if [ ! -x "$LINT" ]; then + echo "test_ci_pbxproj_test_wiring: lint not executable at $LINT" >&2 + exit 1 +fi + +# (a) Real repo must lint clean. +"$LINT" --repo-root "$ROOT_DIR" + +# (b) Synthetic regression — drop an unwired test file in a sandbox repo and +# confirm the lint flags it. +SANDBOX="$(mktemp -d)" +trap 'rm -rf "$SANDBOX"' EXIT +mkdir -p "$SANDBOX/cmuxTests" +mkdir -p "$SANDBOX/cmux.xcodeproj" + +cat > "$SANDBOX/cmuxTests/FakeOrphanTests.swift" <<'SWIFT' +import XCTest +final class FakeOrphanTests: XCTestCase { + func testNoop() { XCTAssert(true) } +} +SWIFT + +cat > "$SANDBOX/cmux.xcodeproj/project.pbxproj" <<'PBX' +// pretend-pbxproj with no reference to FakeOrphanTests.swift +PBX + +if "$LINT" --repo-root "$SANDBOX" >"$SANDBOX/out" 2>&1; then + echo "test_ci_pbxproj_test_wiring: lint should have failed on the unwired sandbox test" >&2 + cat "$SANDBOX/out" >&2 + exit 1 +fi +grep -q "FakeOrphanTests.swift" "$SANDBOX/out" +grep -q "hits=0" "$SANDBOX/out" + +echo "test_ci_pbxproj_test_wiring: ok" From 44897584a560bdd75399933c021efbb9d5d4701f Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 22 May 2026 00:13:04 -0700 Subject: [PATCH 2/6] ci: wire SessionIndexViewTests and SidebarMarkdownRendererTests + document pitfalls GREEN commit of the two-commit pattern started in the previous commit. `scripts/lint-pbxproj-test-wiring.sh` (added previously) flagged two pre-existing test files on `main` that have never compiled or run on CI because they were never added to the cmuxTests target: - cmuxTests/SessionIndexViewTests.swift - cmuxTests/SidebarMarkdownRendererTests.swift Both contain real-looking XCTest coverage (Claude local-command-caveat title formatting, markdown inline-attribute preservation). Wiring them into `cmux.xcodeproj/project.pbxproj` so they actually run, which also closes https://github.com/manaflow-ai/cmux/issues/4559 and lets the new `workflow-guard-tests` lint step go green on this PR. Also adds two CLAUDE.md "Pitfalls" entries based on what fell out of the https://github.com/manaflow-ai/cmux/issues/4529 investigation: - Foundation/SwiftUI/AttributeGraph/WebKit semantics change silently between macOS versions (concrete `URL.deletingLastPathComponent` example from #4529). Recommends AWS M4 Pro builders for empirical repro and points to the `regression-hunt` skill. - Test files in cmuxTests/ must be wired into project.pbxproj or they're silently skipped. References the new lint script and the PR #4536 incident that surfaced the class of bug. Self-test of `tests/test_ci_pbxproj_test_wiring.sh` also got a small hardening: the synthetic sandbox pbxproj no longer mentions the orphan test file by name in a comment, which used to produce a spurious `hits=1` and mask the failure detection inside the wrapper. Closes https://github.com/manaflow-ai/cmux/issues/4559 Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 2 ++ cmux.xcodeproj/project.pbxproj | 8 ++++++++ tests/test_ci_pbxproj_test_wiring.sh | 14 +++++++++++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7523b23b8ec6..82d739dd43c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -235,6 +235,8 @@ The app has a **Debug** menu in the macOS menu bar (only in DEBUG builds). Use i - **Shortcut policy:** Every new cmux-owned keyboard shortcut must be added to `KeyboardShortcutSettings`, visible/editable in Settings, supported in `~/.config/cmux/cmux.json`, and documented in the keyboard shortcut and configuration docs. - **Snapshot boundary for list subtrees.** In any SwiftUI panel whose `body` contains a `LazyVStack` / `LazyHStack` / `List` / `ForEach` of rows, no view below that boundary may hold a reference to an `ObservableObject` / `@Observable` store (no `@ObservedObject`, `@EnvironmentObject`, `@StateObject`, `@Bindable`, or even a plain `let store: SomeStore` property). Rows and drop-gaps receive immutable value snapshots plus closure action bundles only. Violating this reintroduces the "orthogonal @Published change invalidates every row and thrashes `LazyLayoutViewCache`" class of 100% CPU spin loop that hit the Sessions panel and the workspace sidebar (https://github.com/manaflow-ai/cmux/issues/2586). Reference pattern: `IndexSectionActions` / `SectionGapActions` / `SessionSearchFn` in `Sources/SessionIndexView.swift`. - **No state mutation inside view-body computations.** A function called from `body` (directly or through a helper) must not write `@Published` state, schedule a `Task { @MainActor in store.x = … }`, or `DispatchQueue.main.async` a store write. That creates a re-render feedback loop and pegs the main thread (same root-cause family as the snapshot-boundary rule). State-changing work triggered by "new data appeared" belongs in a `reload()` completion, a `didSet`, or a property-observer — never in the projection that feeds `ForEach`. +- **Foundation, SwiftUI, AttributeGraph, and WebKit semantics change silently between macOS major versions.** A function that "obviously" returns the same value on every macOS is not a reliable assumption. Concrete case from https://github.com/manaflow-ai/cmux/issues/4529: `URL(fileURLWithPath: "/").deletingLastPathComponent().path` returns `"/.."` on macOS 14 and 15 but `"/"` on macOS 26 — Apple silently fixed the underlying CFURL normalization. The repo's `macos-26` CI and every maintainer's dev machine were on the fixed-behavior side; every reporter on the issue was on the broken side. Always test on the reporter's macOS before declaring a user-reported repro disproven. AWS M4 Pro builders (`cmux-aws-mac`, `cmux-aws-m4pro`, `aws-m4pro-1..6`) are pre-provisioned on macOS 15.7.4 and the preferred empirical-repro path; see the `regression-hunt` skill in the cmuxterm-hq sibling repo for the full playbook. +- **Test files in `cmuxTests/` must be wired into `cmux.xcodeproj/project.pbxproj`.** A `.swift` file added to the worktree without a matching `PBXFileReference` + `PBXSourcesBuildPhase` entry is silently ignored by Xcode and never compiles or runs on CI. Both `xcodebuild test -only-testing:cmuxTests/` and bot reviews pass with "Executed 0 tests" — so the missing wiring is indistinguishable from a clean two-commit red/green regression test until a real user hits the bug. The `workflow-guard-tests` job runs `./scripts/lint-pbxproj-test-wiring.sh` to catch this at PR time; surfaced during the https://github.com/manaflow-ai/cmux/issues/4529 investigation against https://github.com/manaflow-ai/cmux/pull/4536. Add via Xcode (drag the file into the cmuxTests target) or hand-edit the four pbxproj entries; reference any wired sibling like `TabManagerUnitTests.swift` as a template. ## Test quality policy diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 26700b2e38ae..5521ac962aca 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -373,6 +373,8 @@ B7F9A602B7F9A602B7F9A602 /* WindowChromeMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7F9A603B7F9A603B7F9A603 /* WindowChromeMetrics.swift */; }; B7F9A604B7F9A604B7F9A604 /* RightSidebarChromeStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7F9A605B7F9A605B7F9A605 /* RightSidebarChromeStyle.swift */; }; B6BF3DC98DB1495E57900199 /* TabManagerUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42092CDB2109E250F7F2A76E /* TabManagerUnitTests.swift */; }; + 8A3392FE64E0605D942213D1 /* SessionIndexViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42D69572C8D276745E502B94 /* SessionIndexViewTests.swift */; }; + 385C6BA7E78DB87460E5D930 /* SidebarMarkdownRendererTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F1C3F1DBF6BF5D7223C4A30C /* SidebarMarkdownRendererTests.swift */; }; B8F266236A1A3D9A45BD840F /* SidebarResizeUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 818DBCD4AB69EB72573E8138 /* SidebarResizeUITests.swift */; }; C0DE34020000000000000005 /* HelpMenuUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE34020000000000000006 /* HelpMenuUITests.swift */; }; B8F266246A1A3D9A45BD840F /* SidebarHelpMenuUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8F266256A1A3D9A45BD840F /* SidebarHelpMenuUITests.swift */; }; @@ -607,6 +609,8 @@ A9D9000000000000000F0016 /* markdown-viewer */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = folder; path = "markdown-viewer"; sourceTree = ""; }; FEEDC0DEC0DEC0DEC0DE0002 /* FeedCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedCoordinatorTests.swift; sourceTree = ""; }; 42092CDB2109E250F7F2A76E /* TabManagerUnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabManagerUnitTests.swift; sourceTree = ""; }; + 42D69572C8D276745E502B94 /* SessionIndexViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionIndexViewTests.swift; sourceTree = ""; }; + F1C3F1DBF6BF5D7223C4A30C /* SidebarMarkdownRendererTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarMarkdownRendererTests.swift; sourceTree = ""; }; 43430FA5929121E2EAAB3091 /* AuthEnvironment.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AuthEnvironment.swift; sourceTree = ""; }; C3677001000000000000002 /* CmuxSSHURLRequestTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CmuxSSHURLRequestTests.swift; sourceTree = ""; }; 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketSecurityTests.swift; sourceTree = ""; }; @@ -1628,6 +1632,8 @@ B09C007F42697761B5F1A2AB /* OmnibarAndToolsTests.swift */, D2C075029771815DD5DA1332 /* NotificationAndMenuBarTests.swift */, 42092CDB2109E250F7F2A76E /* TabManagerUnitTests.swift */, + 42D69572C8D276745E502B94 /* SessionIndexViewTests.swift */, + F1C3F1DBF6BF5D7223C4A30C /* SidebarMarkdownRendererTests.swift */, 14A7DC53B9CA33BE2A421711 /* WorkspacePullRequestSidebarTests.swift */, FEEDC0DEC0DEC0DEC0DE0002 /* FeedCoordinatorTests.swift */, 1D301919B10F22B8708E8883 /* WorkspaceManualUnreadTests.swift */, @@ -2415,6 +2421,8 @@ C2B6A97D1F2E4C71A8B9D001 /* BrowserOmnibarPerformanceSupportTests.swift in Sources */, 734F49D37E543DD01C2F4FEF /* NotificationAndMenuBarTests.swift in Sources */, B6BF3DC98DB1495E57900199 /* TabManagerUnitTests.swift in Sources */, + 8A3392FE64E0605D942213D1 /* SessionIndexViewTests.swift in Sources */, + 385C6BA7E78DB87460E5D930 /* SidebarMarkdownRendererTests.swift in Sources */, DCC935C5F55C1DCB33E25521 /* WorkspacePullRequestSidebarTests.swift in Sources */, FEEDC0DEC0DEC0DEC0DE0001 /* FeedCoordinatorTests.swift in Sources */, 0F2C25F9170130F8DC09DD1B /* WorkspaceManualUnreadTests.swift in Sources */, diff --git a/tests/test_ci_pbxproj_test_wiring.sh b/tests/test_ci_pbxproj_test_wiring.sh index 5895a08439e5..653bd98bca27 100755 --- a/tests/test_ci_pbxproj_test_wiring.sh +++ b/tests/test_ci_pbxproj_test_wiring.sh @@ -34,7 +34,7 @@ final class FakeOrphanTests: XCTestCase { SWIFT cat > "$SANDBOX/cmux.xcodeproj/project.pbxproj" <<'PBX' -// pretend-pbxproj with no reference to FakeOrphanTests.swift +// pretend-pbxproj with no test references PBX if "$LINT" --repo-root "$SANDBOX" >"$SANDBOX/out" 2>&1; then @@ -42,7 +42,15 @@ if "$LINT" --repo-root "$SANDBOX" >"$SANDBOX/out" 2>&1; then cat "$SANDBOX/out" >&2 exit 1 fi -grep -q "FakeOrphanTests.swift" "$SANDBOX/out" -grep -q "hits=0" "$SANDBOX/out" +if ! grep -q "FakeOrphanTests.swift" "$SANDBOX/out"; then + echo "test_ci_pbxproj_test_wiring: lint output missing FakeOrphanTests.swift" >&2 + cat "$SANDBOX/out" >&2 + exit 1 +fi +if ! grep -q "hits=0" "$SANDBOX/out"; then + echo "test_ci_pbxproj_test_wiring: lint output missing hits=0" >&2 + cat "$SANDBOX/out" >&2 + exit 1 +fi echo "test_ci_pbxproj_test_wiring: ok" From fa72373112c2b13b35692de8eb39512374b5f074 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 22 May 2026 01:02:31 -0700 Subject: [PATCH 3/6] fix: handle .registered SessionAgent case in test helper to compile cmuxTests/SessionIndexViewTests.swift was not in the test target until the previous commit wired it into project.pbxproj. While it was orphan, the SessionAgent enum gained a `.registered(RegisteredSessionAgent)` case (Sources/SessionIndexModels.swift:44) that the test's `defaultSpecificsForTesting` switch never accounted for. With the test file now actually compiling on CI, the switch fails: SessionIndexViewTests.swift:391:9: error: switch must be exhaustive note: add missing case: '.registered(_)' The test's call sites only ever pass built-in agents (`.claude`, `.grok`). Adding a `fatalError` on `.registered` keeps the switch exhaustive without inventing fake `CmuxVaultAgentRegistration` data that future readers would have to reconcile; if anyone extends the suite to cover Vault-registered agents, the fatalError points them at the missing helper. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmuxTests/SessionIndexViewTests.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmuxTests/SessionIndexViewTests.swift b/cmuxTests/SessionIndexViewTests.swift index 91b15e273ff3..0db2b2d09e53 100644 --- a/cmuxTests/SessionIndexViewTests.swift +++ b/cmuxTests/SessionIndexViewTests.swift @@ -405,6 +405,11 @@ private extension SessionAgent { return .rovodev case .hermesAgent: return .hermesAgent(source: nil, model: nil, hermesHome: nil) + case .registered: + // Registered (Vault) agents aren't exercised by these tests; if a + // future test reaches this branch, point them at the missing + // helper instead of silently returning a misleading default. + fatalError("defaultSpecificsForTesting does not support .registered SessionAgent; extend the helper when adding registered-agent coverage") } } } From a62e26b61862363f44ab3244a1513c7dbf5d1fb8 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 22 May 2026 01:05:04 -0700 Subject: [PATCH 4/6] ci: tighten pbxproj lint to require target membership, not just file reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Earlier version of the lint counted any pbxproj line that mentioned the test file basename. That's too permissive: a file can have a PBXFileReference and a group children entry but still not be a member of the cmuxTests target's PBXSourcesBuildPhase, in which case Xcode silently skips it — the exact failure mode that lets a regression test land green without ever running. Switch to counting only lines that end with ".swift in Sources */", which appear in: 1. the PBXBuildFile entry, and 2. the cmuxTests target's PBXSourcesBuildPhase files list. A target member has hits >= 2 in both. A "group-only" file (referenced in the project tree but not part of the test target) has hits = 0, which is exactly the silent-skip case the lint must catch. Also adds a new (c) sandbox case in `tests/test_ci_pbxproj_test_wiring.sh` that drops a file with a PBXFileReference + group child but no PBXBuildFile / SourcesBuildPhase entry, and asserts the lint flags it. Without this case the wrapper would still pass against the looser bare-filename check. Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/lint-pbxproj-test-wiring.sh | 28 +++++++++++------ tests/test_ci_pbxproj_test_wiring.sh | 47 ++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/scripts/lint-pbxproj-test-wiring.sh b/scripts/lint-pbxproj-test-wiring.sh index d917efcf0d34..77a93e6afcb9 100755 --- a/scripts/lint-pbxproj-test-wiring.sh +++ b/scripts/lint-pbxproj-test-wiring.sh @@ -65,13 +65,20 @@ checked=0 while IFS= read -r -d '' file; do base="$(basename "$file")" checked=$((checked + 1)) - # Each wired test file shows up 4x in pbxproj: build file UUID, - # file reference UUID, group children list, and target sources phase. - # Require at least 2 references so we flag both "added but missing - # entirely" and "stub reference but no sources phase" failures. - hits="$(grep -c -- "$base" "$PBXPROJ" || true)" + # Target membership is what determines whether Xcode actually compiles/runs + # the file. Only two pbxproj entries prove target membership, and both carry + # the literal ` in Sources` suffix: + # 1. PBXBuildFile: " /* in Sources */ = { ... };" + # 2. PBXSourcesBuildPhase: " /* in Sources */," (inside the + # cmuxTests target's Sources build phase) + # The bare filename also appears in PBXFileReference + group children, but + # those entries are present even when the file is in the project tree but + # NOT a member of the cmuxTests target — which is the silently-skipped case + # that prompted this lint. Counting only `in Sources` lines guarantees we + # catch missing target membership. + hits="$(grep -c -- "$base in Sources" "$PBXPROJ" || true)" if [ "$hits" -lt 2 ]; then - missing+=("$base (hits=$hits)") + missing+=("$base (in-Sources hits=$hits)") fi done < <(find "$TESTS_DIR" -maxdepth 1 -type f -name '*.swift' -print0) @@ -80,16 +87,19 @@ if [ "${#missing[@]}" -eq 0 ]; then exit 0 fi -echo "lint-pbxproj-test-wiring: ${#missing[@]} test file(s) not wired into cmux.xcodeproj/project.pbxproj" +echo "lint-pbxproj-test-wiring: ${#missing[@]} test file(s) not a member of the cmuxTests target in cmux.xcodeproj/project.pbxproj" for entry in "${missing[@]}"; do echo " - $entry" done echo "" echo "Each cmuxTests/.swift must appear in cmux.xcodeproj/project.pbxproj as:" -echo " 1. a PBXBuildFile entry (UUID = '.swift in Sources')" -echo " 2. a PBXFileReference entry (UUID = '.swift')" +echo " 1. a PBXBuildFile entry (line ends with '.swift in Sources */ = { ... };')" +echo " 2. a PBXFileReference entry" echo " 3. an entry in the cmuxTests group children list" echo " 4. an entry in the cmuxTests target's PBXSourcesBuildPhase files" +echo " (line ends with '.swift in Sources */,')" +echo "" +echo "Entries 1 and 4 are the target-membership lines this lint counts." echo "" echo "Add via Xcode (drag the file into the cmuxTests target) or hand-edit" echo "the four blocks (see any wired sibling test as a template)." diff --git a/tests/test_ci_pbxproj_test_wiring.sh b/tests/test_ci_pbxproj_test_wiring.sh index 653bd98bca27..57df5b94dce5 100755 --- a/tests/test_ci_pbxproj_test_wiring.sh +++ b/tests/test_ci_pbxproj_test_wiring.sh @@ -47,10 +47,53 @@ if ! grep -q "FakeOrphanTests.swift" "$SANDBOX/out"; then cat "$SANDBOX/out" >&2 exit 1 fi -if ! grep -q "hits=0" "$SANDBOX/out"; then - echo "test_ci_pbxproj_test_wiring: lint output missing hits=0" >&2 +if ! grep -q "in-Sources hits=0" "$SANDBOX/out"; then + echo "test_ci_pbxproj_test_wiring: lint output missing 'in-Sources hits=0'" >&2 cat "$SANDBOX/out" >&2 exit 1 fi +# (c) Target-membership regression — drop a file that is referenced in the +# pbxproj (PBXFileReference + group child) but NOT a member of the cmuxTests +# target (no PBXBuildFile / PBXSourcesBuildPhase entry). This is the silent +# failure mode the original lint missed: bare filename hits >= 2 but Xcode +# still skips the file. The lint must still flag it. +SANDBOX2="$(mktemp -d)" +trap 'rm -rf "$SANDBOX" "$SANDBOX2"' EXIT +mkdir -p "$SANDBOX2/cmuxTests" +mkdir -p "$SANDBOX2/cmux.xcodeproj" + +cat > "$SANDBOX2/cmuxTests/FakeGroupOnlyTests.swift" <<'SWIFT' +import XCTest +final class FakeGroupOnlyTests: XCTestCase { + func testNoop() { XCTAssert(true) } +} +SWIFT + +# Two filename hits, zero target-membership hits. +cat > "$SANDBOX2/cmux.xcodeproj/project.pbxproj" <<'PBX' +// PBXFileReference entry (would be inside /* Begin PBXFileReference section */) +ABCDEF0000000000000000A1 /* FakeGroupOnlyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FakeGroupOnlyTests.swift; sourceTree = ""; }; +// cmuxTests group children list (would be inside /* Begin PBXGroup section */) + ABCDEF0000000000000000A1 /* FakeGroupOnlyTests.swift */, +// NOTE: no PBXBuildFile and no PBXSourcesBuildPhase reference, so the file +// is in the project but not a member of the cmuxTests target. +PBX + +if "$LINT" --repo-root "$SANDBOX2" >"$SANDBOX2/out" 2>&1; then + echo "test_ci_pbxproj_test_wiring: lint should have failed on file with group membership only" >&2 + cat "$SANDBOX2/out" >&2 + exit 1 +fi +if ! grep -q "FakeGroupOnlyTests.swift" "$SANDBOX2/out"; then + echo "test_ci_pbxproj_test_wiring: lint output missing FakeGroupOnlyTests.swift" >&2 + cat "$SANDBOX2/out" >&2 + exit 1 +fi +if ! grep -q "in-Sources hits=0" "$SANDBOX2/out"; then + echo "test_ci_pbxproj_test_wiring: lint output missing 'in-Sources hits=0' for group-only fixture" >&2 + cat "$SANDBOX2/out" >&2 + exit 1 +fi + echo "test_ci_pbxproj_test_wiring: ok" From c59e00ea3416337d57eeafe66080e95bad760ada Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 22 May 2026 01:15:34 -0700 Subject: [PATCH 5/6] ci: scope pbxproj test-wiring check to cmuxTests target's Sources phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Earlier iterations counted matches in the whole pbxproj. That accepted two silent-skip cases: 1. A file with PBXFileReference + group child but no PBXBuildFile / SourcesBuildPhase entry (filename appears 2x globally, but the file is not a member of any target — Xcode does not compile it). 2. A file wired into the wrong target (e.g. cmuxUITests instead of cmuxTests). `.swift in Sources` appears 2x in the pbxproj — once in PBXBuildFile, once inside cmuxUITests' Sources phase — but Xcode still does not compile it into the cmuxTests bundle, so the regression test never runs. Resolve the cmuxTests PBXNativeTarget and its Sources build phase UUID, slice that phase block, and look for the file's `in Sources` entry only inside that block. Threshold becomes 1 hit (membership) rather than a global count, which is exactly what determines whether Xcode compiles the file into cmuxTests. Tighten the test wrapper to exercise all three failure modes against synthetic pbxprojs that include a real cmuxTests PBXNativeTarget + Sources phase stub. Verified by inverse: with this change reverted on a scratch copy of the real pbxproj, the lint misses the wrong-target case; with it applied, the lint flags it. Co-Authored-By: Claude Opus 4.7 --- scripts/lint-pbxproj-test-wiring.sh | 90 ++++++++--- tests/test_ci_pbxproj_test_wiring.sh | 217 ++++++++++++++++++++------- 2 files changed, 233 insertions(+), 74 deletions(-) diff --git a/scripts/lint-pbxproj-test-wiring.sh b/scripts/lint-pbxproj-test-wiring.sh index 77a93e6afcb9..ad8bcc761015 100755 --- a/scripts/lint-pbxproj-test-wiring.sh +++ b/scripts/lint-pbxproj-test-wiring.sh @@ -59,26 +59,78 @@ if [ ! -d "$TESTS_DIR" ]; then exit 2 fi +# Locate the cmuxTests PBXNativeTarget and resolve its Sources build phase +# UUID. We then slice out just that build phase block and look for files inside +# it — which is exactly the set of files Xcode compiles into cmuxTests. +# +# Targeting the cmuxTests Sources phase specifically (instead of the whole +# pbxproj) catches three failure modes: +# 1. File missing entirely (no `.swift in Sources` anywhere). +# 2. File has a PBXFileReference + group child but no PBXBuildFile / +# Sources phase entry (in the project tree but not a member of any +# target). +# 3. File is a member of the wrong target (e.g. cmuxUITests or cmux). Its +# `.swift in Sources` lines exist in the pbxproj, so a global grep +# would pass, but they are not inside the cmuxTests Sources block. +# `/* cmuxTests */ = {` appears twice in a typical pbxproj: once for the +# PBXGroup that holds the test files, and once for the PBXNativeTarget. We +# only care about the native-target block. Use awk to capture every +# `/* cmuxTests */ = { ... };` block and keep only the one whose `isa = +# PBXNativeTarget;` line is present. +tests_target_block="$(awk ' + /\/\* cmuxTests \*\/ = \{/ { capture = 1; buf = "" } + capture { buf = buf $0 "\n" } + capture && /^[[:space:]]*\};[[:space:]]*$/ { + if (buf ~ /isa = PBXNativeTarget;/) { + print buf + exit + } + capture = 0 + buf = "" + } +' "$PBXPROJ")" + +if [ -z "$tests_target_block" ]; then + echo "lint-pbxproj-test-wiring: could not locate cmuxTests PBXNativeTarget in $PBXPROJ" >&2 + exit 2 +fi + +# Xcode UUIDs are conventionally 24 uppercase hex chars, but hand-edited +# pbxprojs occasionally use 24-char identifiers that include other uppercase +# letters or digits. Match both. +tests_sources_uuid="$(printf '%s\n' "$tests_target_block" \ + | grep -oE '[A-Z0-9]{24} /\* Sources \*/' \ + | head -n 1 \ + | awk '{print $1}')" + +if [ -z "$tests_sources_uuid" ]; then + echo "lint-pbxproj-test-wiring: cmuxTests target has no Sources build phase reference" >&2 + exit 2 +fi + +# Slice the PBXSourcesBuildPhase block whose UUID matches the cmuxTests +# target's Sources phase reference. The block begins with the UUID/Sources +# header and ends at the next standalone "};" line. +tests_sources_block="$(awk -v uuid="$tests_sources_uuid" ' + $0 ~ uuid " /\\* Sources \\*/ = \\{" { capture = 1 } + capture { print } + capture && /^[[:space:]]*\};[[:space:]]*$/ { exit } +' "$PBXPROJ")" + +if [ -z "$tests_sources_block" ]; then + echo "lint-pbxproj-test-wiring: could not slice cmuxTests Sources build phase (uuid=$tests_sources_uuid)" >&2 + exit 2 +fi + missing=() checked=0 while IFS= read -r -d '' file; do base="$(basename "$file")" checked=$((checked + 1)) - # Target membership is what determines whether Xcode actually compiles/runs - # the file. Only two pbxproj entries prove target membership, and both carry - # the literal ` in Sources` suffix: - # 1. PBXBuildFile: " /* in Sources */ = { ... };" - # 2. PBXSourcesBuildPhase: " /* in Sources */," (inside the - # cmuxTests target's Sources build phase) - # The bare filename also appears in PBXFileReference + group children, but - # those entries are present even when the file is in the project tree but - # NOT a member of the cmuxTests target — which is the silently-skipped case - # that prompted this lint. Counting only `in Sources` lines guarantees we - # catch missing target membership. - hits="$(grep -c -- "$base in Sources" "$PBXPROJ" || true)" - if [ "$hits" -lt 2 ]; then - missing+=("$base (in-Sources hits=$hits)") + # Look for the file's entry inside the cmuxTests Sources phase only. + if ! printf '%s\n' "$tests_sources_block" | grep -q -- "$base in Sources"; then + missing+=("$base") fi done < <(find "$TESTS_DIR" -maxdepth 1 -type f -name '*.swift' -print0) @@ -87,19 +139,23 @@ if [ "${#missing[@]}" -eq 0 ]; then exit 0 fi -echo "lint-pbxproj-test-wiring: ${#missing[@]} test file(s) not a member of the cmuxTests target in cmux.xcodeproj/project.pbxproj" +echo "lint-pbxproj-test-wiring: ${#missing[@]} test file(s) not a member of the cmuxTests target's Sources build phase (uuid=$tests_sources_uuid) in cmux.xcodeproj/project.pbxproj" for entry in "${missing[@]}"; do echo " - $entry" done echo "" -echo "Each cmuxTests/.swift must appear in cmux.xcodeproj/project.pbxproj as:" +echo "Each cmuxTests/.swift must be wired into cmux.xcodeproj/project.pbxproj" +echo "as a full target member of cmuxTests:" echo " 1. a PBXBuildFile entry (line ends with '.swift in Sources */ = { ... };')" echo " 2. a PBXFileReference entry" echo " 3. an entry in the cmuxTests group children list" echo " 4. an entry in the cmuxTests target's PBXSourcesBuildPhase files" echo " (line ends with '.swift in Sources */,')" echo "" -echo "Entries 1 and 4 are the target-membership lines this lint counts." +echo "This lint slices the cmuxTests Sources phase and looks for entry 4 there." +echo "Files wired only into cmuxUITests, cmux, or the project tree (without" +echo "cmuxTests target membership) are silently skipped by Xcode and will be" +echo "flagged here." echo "" echo "Add via Xcode (drag the file into the cmuxTests target) or hand-edit" echo "the four blocks (see any wired sibling test as a template)." diff --git a/tests/test_ci_pbxproj_test_wiring.sh b/tests/test_ci_pbxproj_test_wiring.sh index 57df5b94dce5..5db5b3d65f48 100755 --- a/tests/test_ci_pbxproj_test_wiring.sh +++ b/tests/test_ci_pbxproj_test_wiring.sh @@ -1,9 +1,19 @@ #!/usr/bin/env bash # CI guard for ./scripts/lint-pbxproj-test-wiring.sh. # -# Verifies the lint script (a) reports "ok" on the real cmux repo, and (b) -# correctly fails when a test file is dropped in without pbxproj wiring. -# The second case is what prevents the lint itself from rotting into a no-op. +# Verifies the lint script reports "ok" on the real cmux repo and correctly +# fails on every silent-skip failure mode the lint is meant to catch. The +# negative cases are what prevent the lint itself from rotting into a no-op. +# +# Cases: +# (a) Real cmux repo lints clean. +# (b) Test file has no pbxproj references at all (hits=0). +# (c) Test file has PBXFileReference + group child but is not a member of +# any target (Xcode silently skips it). +# (d) Test file is a member of a non-cmuxTests target (e.g. cmuxUITests). +# Its "in Sources" lines exist in the pbxproj, but not inside the +# cmuxTests Sources build phase; Xcode does not compile it into the +# cmuxTests bundle. set -euo pipefail @@ -19,80 +29,173 @@ fi # (a) Real repo must lint clean. "$LINT" --repo-root "$ROOT_DIR" -# (b) Synthetic regression — drop an unwired test file in a sandbox repo and -# confirm the lint flags it. -SANDBOX="$(mktemp -d)" -trap 'rm -rf "$SANDBOX"' EXIT -mkdir -p "$SANDBOX/cmuxTests" -mkdir -p "$SANDBOX/cmux.xcodeproj" +# Shared sandbox cleanup. +SANDBOX_PARENT="$(mktemp -d)" +trap 'rm -rf "$SANDBOX_PARENT"' EXIT -cat > "$SANDBOX/cmuxTests/FakeOrphanTests.swift" <<'SWIFT' -import XCTest -final class FakeOrphanTests: XCTestCase { - func testNoop() { XCTAssert(true) } -} -SWIFT +# Helper: write a minimal pbxproj that contains a cmuxTests PBXNativeTarget with +# a Sources build phase. Each fixture appends its own orphan/wrong-target entries +# outside the Sources block. The block is intentionally small enough to read by +# eye; the lint only cares about the `cmuxTests` target marker, the Sources +# phase UUID lookup inside it, and the contents of the matching +# PBXSourcesBuildPhase block. +write_base_pbxproj() { + local pbxproj="$1" + local extra_after_sources="${2:-}" + + cat > "$pbxproj" < "$SANDBOX/cmux.xcodeproj/project.pbxproj" <<'PBX' -// pretend-pbxproj with no test references +/* Begin PBXSourcesBuildPhase section */ + AAAA000000000000000000S1 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + runOnlyForDeploymentPostprocessing = 0; + }; +/* End PBXSourcesBuildPhase section */ +${extra_after_sources} PBX +} -if "$LINT" --repo-root "$SANDBOX" >"$SANDBOX/out" 2>&1; then - echo "test_ci_pbxproj_test_wiring: lint should have failed on the unwired sandbox test" >&2 - cat "$SANDBOX/out" >&2 +# --------------------------------------------------------------------------- +# (b) File has no references at all in pbxproj. +SANDBOX_B="$SANDBOX_PARENT/b" +mkdir -p "$SANDBOX_B/cmuxTests" "$SANDBOX_B/cmux.xcodeproj" +cat > "$SANDBOX_B/cmuxTests/FakeOrphanTests.swift" <<'SWIFT' +import XCTest +final class FakeOrphanTests: XCTestCase { func testNoop() { XCTAssert(true) } } +SWIFT +write_base_pbxproj "$SANDBOX_B/cmux.xcodeproj/project.pbxproj" + +if "$LINT" --repo-root "$SANDBOX_B" >"$SANDBOX_B/out" 2>&1; then + echo "test_ci_pbxproj_test_wiring: (b) lint should have failed on the no-reference orphan" >&2 + cat "$SANDBOX_B/out" >&2 + exit 1 +fi +if ! grep -q "FakeOrphanTests.swift" "$SANDBOX_B/out"; then + echo "test_ci_pbxproj_test_wiring: (b) lint output missing FakeOrphanTests.swift" >&2 + cat "$SANDBOX_B/out" >&2 exit 1 fi -if ! grep -q "FakeOrphanTests.swift" "$SANDBOX/out"; then - echo "test_ci_pbxproj_test_wiring: lint output missing FakeOrphanTests.swift" >&2 - cat "$SANDBOX/out" >&2 + +# --------------------------------------------------------------------------- +# (c) File has PBXFileReference + group child only — not a target member. +SANDBOX_C="$SANDBOX_PARENT/c" +mkdir -p "$SANDBOX_C/cmuxTests" "$SANDBOX_C/cmux.xcodeproj" +cat > "$SANDBOX_C/cmuxTests/FakeGroupOnlyTests.swift" <<'SWIFT' +import XCTest +final class FakeGroupOnlyTests: XCTestCase { func testNoop() { XCTAssert(true) } } +SWIFT +write_base_pbxproj "$SANDBOX_C/cmux.xcodeproj/project.pbxproj" " +/* Begin PBXFileReference section */ + BBBB000000000000000000F1 /* FakeGroupOnlyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FakeGroupOnlyTests.swift; sourceTree = \"\"; }; +/* End PBXFileReference section */ + +/* Begin PBXGroup section */ + BBBB000000000000000000G1 /* cmuxTests */ = { + isa = PBXGroup; + children = ( + BBBB000000000000000000F1 /* FakeGroupOnlyTests.swift */, + ); + }; +/* End PBXGroup section */ +" + +if "$LINT" --repo-root "$SANDBOX_C" >"$SANDBOX_C/out" 2>&1; then + echo "test_ci_pbxproj_test_wiring: (c) lint should have failed on group-only file" >&2 + cat "$SANDBOX_C/out" >&2 exit 1 fi -if ! grep -q "in-Sources hits=0" "$SANDBOX/out"; then - echo "test_ci_pbxproj_test_wiring: lint output missing 'in-Sources hits=0'" >&2 - cat "$SANDBOX/out" >&2 +if ! grep -q "FakeGroupOnlyTests.swift" "$SANDBOX_C/out"; then + echo "test_ci_pbxproj_test_wiring: (c) lint output missing FakeGroupOnlyTests.swift" >&2 + cat "$SANDBOX_C/out" >&2 exit 1 fi -# (c) Target-membership regression — drop a file that is referenced in the -# pbxproj (PBXFileReference + group child) but NOT a member of the cmuxTests -# target (no PBXBuildFile / PBXSourcesBuildPhase entry). This is the silent -# failure mode the original lint missed: bare filename hits >= 2 but Xcode -# still skips the file. The lint must still flag it. -SANDBOX2="$(mktemp -d)" -trap 'rm -rf "$SANDBOX" "$SANDBOX2"' EXIT -mkdir -p "$SANDBOX2/cmuxTests" -mkdir -p "$SANDBOX2/cmux.xcodeproj" - -cat > "$SANDBOX2/cmuxTests/FakeGroupOnlyTests.swift" <<'SWIFT' +# --------------------------------------------------------------------------- +# (d) File is in cmuxUITests target's Sources phase, NOT in cmuxTests. +SANDBOX_D="$SANDBOX_PARENT/d" +mkdir -p "$SANDBOX_D/cmuxTests" "$SANDBOX_D/cmux.xcodeproj" +cat > "$SANDBOX_D/cmuxTests/FakeWrongTargetTests.swift" <<'SWIFT' import XCTest -final class FakeGroupOnlyTests: XCTestCase { - func testNoop() { XCTAssert(true) } -} +final class FakeWrongTargetTests: XCTestCase { func testNoop() { XCTAssert(true) } } SWIFT -# Two filename hits, zero target-membership hits. -cat > "$SANDBOX2/cmux.xcodeproj/project.pbxproj" <<'PBX' -// PBXFileReference entry (would be inside /* Begin PBXFileReference section */) -ABCDEF0000000000000000A1 /* FakeGroupOnlyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FakeGroupOnlyTests.swift; sourceTree = ""; }; -// cmuxTests group children list (would be inside /* Begin PBXGroup section */) - ABCDEF0000000000000000A1 /* FakeGroupOnlyTests.swift */, -// NOTE: no PBXBuildFile and no PBXSourcesBuildPhase reference, so the file -// is in the project but not a member of the cmuxTests target. +# Write a pbxproj that contains BOTH a cmuxTests target (with empty Sources +# phase) AND a separate cmuxUITests target whose Sources phase wires +# FakeWrongTargetTests.swift. The file therefore appears in two `in Sources` +# lines (PBXBuildFile + cmuxUITests Sources phase), satisfying a naive global +# grep, but it is NOT a member of the cmuxTests Sources phase. +cat > "$SANDBOX_D/cmux.xcodeproj/project.pbxproj" <<'PBX' +// Minimal synthetic project for lint testing — wrong-target case. +/* Begin PBXBuildFile section */ + CCCC000000000000000000B1 /* FakeWrongTargetTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CCCC000000000000000000F1 /* FakeWrongTargetTests.swift */; }; +/* End PBXBuildFile section */ + +/* Begin PBXFileReference section */ + CCCC000000000000000000F1 /* FakeWrongTargetTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FakeWrongTargetTests.swift; sourceTree = ""; }; +/* End PBXFileReference section */ + +/* Begin PBXNativeTarget section */ + AAAA000000000000000000T1 /* cmuxTests */ = { + isa = PBXNativeTarget; + buildPhases = ( + AAAA000000000000000000S1 /* Sources */, + ); + name = cmuxTests; + }; + CCCC000000000000000000T1 /* cmuxUITests */ = { + isa = PBXNativeTarget; + buildPhases = ( + CCCC000000000000000000S1 /* Sources */, + ); + name = cmuxUITests; + }; +/* End PBXNativeTarget section */ + +/* Begin PBXSourcesBuildPhase section */ + AAAA000000000000000000S1 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + runOnlyForDeploymentPostprocessing = 0; + }; + CCCC000000000000000000S1 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + CCCC000000000000000000B1 /* FakeWrongTargetTests.swift in Sources */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; +/* End PBXSourcesBuildPhase section */ PBX -if "$LINT" --repo-root "$SANDBOX2" >"$SANDBOX2/out" 2>&1; then - echo "test_ci_pbxproj_test_wiring: lint should have failed on file with group membership only" >&2 - cat "$SANDBOX2/out" >&2 +if "$LINT" --repo-root "$SANDBOX_D" >"$SANDBOX_D/out" 2>&1; then + echo "test_ci_pbxproj_test_wiring: (d) lint should have failed on file wired to wrong target (cmuxUITests instead of cmuxTests)" >&2 + cat "$SANDBOX_D/out" >&2 exit 1 fi -if ! grep -q "FakeGroupOnlyTests.swift" "$SANDBOX2/out"; then - echo "test_ci_pbxproj_test_wiring: lint output missing FakeGroupOnlyTests.swift" >&2 - cat "$SANDBOX2/out" >&2 +if ! grep -q "FakeWrongTargetTests.swift" "$SANDBOX_D/out"; then + echo "test_ci_pbxproj_test_wiring: (d) lint output missing FakeWrongTargetTests.swift" >&2 + cat "$SANDBOX_D/out" >&2 exit 1 fi -if ! grep -q "in-Sources hits=0" "$SANDBOX2/out"; then - echo "test_ci_pbxproj_test_wiring: lint output missing 'in-Sources hits=0' for group-only fixture" >&2 - cat "$SANDBOX2/out" >&2 +if ! grep -q "cmuxTests target's Sources build phase" "$SANDBOX_D/out"; then + echo "test_ci_pbxproj_test_wiring: (d) lint output missing cmuxTests-target diagnostic" >&2 + cat "$SANDBOX_D/out" >&2 exit 1 fi From d79f42dd8e2d70bc438223b3051d3c5e73f3e2d9 Mon Sep 17 00:00:00 2001 From: Lawrence Chen Date: Fri, 22 May 2026 01:22:09 -0700 Subject: [PATCH 6/6] ci: anchor pbxproj membership match against suffix-overlap false negatives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous check looked for ` in Sources` inside the cmuxTests Sources phase. That's vulnerable to filename-suffix overlap: if the lint target is a substring of another wired file, the longer match still satisfies the grep. The repo already contains an overlapping pair — `SearchIndexTests.swift` is a suffix of `SettingsSearchIndexTests.swift` — so accidentally removing `SearchIndexTests.swift` from the cmuxTests Sources phase would pass the lint. Switch to a fixed-string match against the full PBX comment `/* in Sources */`. The leading `/* ` and trailing ` */` disambiguate the basename. Verified by inverse: scratch-removing `SearchIndexTests.swift` from the real cmuxTests Sources phase now flags the file specifically, while `SettingsSearchIndexTests.swift` is unaffected. Add a fifth sandbox case (e) in the wrapper that wires `PrefixFooTests.swift` but leaves `FooTests.swift` orphan, and asserts the lint flags only the orphan. Without this fixture a later loosening of the grep would not be caught. Co-Authored-By: Claude Opus 4.7 --- scripts/lint-pbxproj-test-wiring.sh | 9 +++- tests/test_ci_pbxproj_test_wiring.sh | 70 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/scripts/lint-pbxproj-test-wiring.sh b/scripts/lint-pbxproj-test-wiring.sh index ad8bcc761015..a0278833c035 100755 --- a/scripts/lint-pbxproj-test-wiring.sh +++ b/scripts/lint-pbxproj-test-wiring.sh @@ -129,7 +129,14 @@ while IFS= read -r -d '' file; do base="$(basename "$file")" checked=$((checked + 1)) # Look for the file's entry inside the cmuxTests Sources phase only. - if ! printf '%s\n' "$tests_sources_block" | grep -q -- "$base in Sources"; then + # + # Match the full PBX comment `/* in Sources */` as a fixed string + # (grep -F) so we don't get a false positive when `` is a substring + # of another wired file. Example: `SearchIndexTests.swift` is a suffix of + # `SettingsSearchIndexTests.swift`; without these anchors, removing the + # former from the Sources phase would still match the latter and the lint + # would pass. + if ! printf '%s\n' "$tests_sources_block" | grep -qF -- "/* $base in Sources */"; then missing+=("$base") fi done < <(find "$TESTS_DIR" -maxdepth 1 -type f -name '*.swift' -print0) diff --git a/tests/test_ci_pbxproj_test_wiring.sh b/tests/test_ci_pbxproj_test_wiring.sh index 5db5b3d65f48..fd672566bbcf 100755 --- a/tests/test_ci_pbxproj_test_wiring.sh +++ b/tests/test_ci_pbxproj_test_wiring.sh @@ -14,6 +14,9 @@ # Its "in Sources" lines exist in the pbxproj, but not inside the # cmuxTests Sources build phase; Xcode does not compile it into the # cmuxTests bundle. +# (e) Test file's basename is a suffix of another file already wired into +# the cmuxTests Sources phase. An unanchored grep would match the +# longer name and falsely report the shorter one as wired. set -euo pipefail @@ -199,4 +202,71 @@ if ! grep -q "cmuxTests target's Sources build phase" "$SANDBOX_D/out"; then exit 1 fi +# --------------------------------------------------------------------------- +# (e) Filename-suffix overlap. Two files share a suffix: only the longer one +# is wired into the cmuxTests Sources phase. The shorter file should be +# flagged. Without anchoring the grep, an unanchored substring match against +# the longer wired entry would falsely report the shorter file as wired. +SANDBOX_E="$SANDBOX_PARENT/e" +mkdir -p "$SANDBOX_E/cmuxTests" "$SANDBOX_E/cmux.xcodeproj" +cat > "$SANDBOX_E/cmuxTests/FooTests.swift" <<'SWIFT' +import XCTest +final class FooTests: XCTestCase { func testNoop() { XCTAssert(true) } } +SWIFT +cat > "$SANDBOX_E/cmuxTests/PrefixFooTests.swift" <<'SWIFT' +import XCTest +final class PrefixFooTests: XCTestCase { func testNoop() { XCTAssert(true) } } +SWIFT + +# pbxproj: cmuxTests target's Sources phase only wires PrefixFooTests.swift. +# FooTests.swift has no entries; it should be flagged. +cat > "$SANDBOX_E/cmux.xcodeproj/project.pbxproj" <<'PBX' +// Minimal synthetic project for lint testing — suffix-overlap case. +/* Begin PBXBuildFile section */ + DDDD000000000000000000B1 /* PrefixFooTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DDDD000000000000000000F1 /* PrefixFooTests.swift */; }; +/* End PBXBuildFile section */ + +/* Begin PBXFileReference section */ + DDDD000000000000000000F1 /* PrefixFooTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrefixFooTests.swift; sourceTree = ""; }; +/* End PBXFileReference section */ + +/* Begin PBXNativeTarget section */ + AAAA000000000000000000T1 /* cmuxTests */ = { + isa = PBXNativeTarget; + buildPhases = ( + AAAA000000000000000000S1 /* Sources */, + ); + name = cmuxTests; + }; +/* End PBXNativeTarget section */ + +/* Begin PBXSourcesBuildPhase section */ + AAAA000000000000000000S1 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + DDDD000000000000000000B1 /* PrefixFooTests.swift in Sources */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; +/* End PBXSourcesBuildPhase section */ +PBX + +if "$LINT" --repo-root "$SANDBOX_E" >"$SANDBOX_E/out" 2>&1; then + echo "test_ci_pbxproj_test_wiring: (e) lint should have failed on FooTests.swift (suffix-overlap false negative)" >&2 + cat "$SANDBOX_E/out" >&2 + exit 1 +fi +if ! grep -q "FooTests.swift" "$SANDBOX_E/out"; then + echo "test_ci_pbxproj_test_wiring: (e) lint output missing FooTests.swift" >&2 + cat "$SANDBOX_E/out" >&2 + exit 1 +fi +# Confirm the lint only flagged the suffix-orphan, not the wired prefix file. +if grep -q " - PrefixFooTests.swift" "$SANDBOX_E/out"; then + echo "test_ci_pbxproj_test_wiring: (e) lint should NOT flag PrefixFooTests.swift (it is wired)" >&2 + cat "$SANDBOX_E/out" >&2 + exit 1 +fi + echo "test_ci_pbxproj_test_wiring: ok"