From 6dcc0428dd355e99e21371142996c6111931d17b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 19 Sep 2026 19:18:30 -0400 Subject: [PATCH] fix(#4784): Xcode gates pass -project and derive the destination from available simulators (#4879) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4784): failing-first — Xcode gates must pass -project and derive the destination from available simulators * fix(#4784): Xcode gates pass -project '$XCODEPROJ' and resolve the destination from available simulators, skipping loudly when none exist Also surfaces the -collect-test-diagnostics never / workflow.test_gate_timeout guidance on a 124 timeout (a real iOS suite measured ~600s of sysdiagnose collection after the tests passed). * test(#4784): reachability-honest test shapes — apostrophe-confused prose (the path the false positive actually reaches past the splitter) and the file's single-quote convention * fix(#4784): review fold-ins — unanchor the simulator extraction (real simctl lines end in a state suffix), escape apostrophes for the bash -c layer, assign the command variables on the skip path, behavioral extraction test The isolated reviewer's Finding 1 was critical: the first cut's $-anchored sed matched ZERO real simctl lines (every device line ends in (Shutdown)/(Booted)), so the gates would have skipped on every machine. The 46,454-test green could not see it (content assertions do not execute the pipeline); the new behavioral test runs the gate's own extraction against a real-format line. * chore(#4784): backfill changeset PR number (4879) --------- Co-authored-by: sim --- .changeset/gallant-quails-click.md | 5 + .../execute-phase/steps/post-merge-gate.md | 53 +++++++++-- .../platform-conformance-tier.generated.cjs | 1 + tests/test-gate-watch-mode.test.cjs | 91 +++++++++++++++++++ 4 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 .changeset/gallant-quails-click.md diff --git a/.changeset/gallant-quails-click.md b/.changeset/gallant-quails-click.md new file mode 100644 index 000000000..baa6a6439 --- /dev/null +++ b/.changeset/gallant-quails-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4879 +--- +**The post-merge gate's Xcode detection works on real project layouts** — the gate resolved the .xcodeproj path but built its commands without -project (any project one directory down failed with exit 66) and hardcoded the iPhone 16 simulator name (a machine property); commands now carry -project, the destination resolves from the machine's available simulators, gates skip loudly when none exists, and the timeout message names the sysdiagnose collector. (#4784) diff --git a/gsd-core/workflows/execute-phase/steps/post-merge-gate.md b/gsd-core/workflows/execute-phase/steps/post-merge-gate.md index 3795fd954..9636c3112 100644 --- a/gsd-core/workflows/execute-phase/steps/post-merge-gate.md +++ b/gsd-core/workflows/execute-phase/steps/post-merge-gate.md @@ -16,10 +16,33 @@ if [ -z "$BUILD_CMD" ]; then if [ -n "$XCODEPROJ" ]; then # Xcode project: get first scheme from xcodebuild -list -json XCODE_SCHEME=$(xcodebuild -list -json -project "$XCODEPROJ" 2>/dev/null | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('project',{}).get('schemes',[None])[0] or '')" 2>/dev/null || true) - if [ -n "$XCODE_SCHEME" ]; then - BUILD_CMD="xcodebuild build -scheme '$XCODE_SCHEME' -destination 'platform=iOS Simulator,name=iPhone 16'" + # #4784: a concrete simulator NAME is machine state, not project state — the + # hardcoded 'iPhone 16' can never match on machines whose simulators were + # never created. Resolve the first available iOS simulator (id= works for + # both build and test); with none available, skip the Xcode gates loudly + # instead of running commands that are guaranteed to fail. The sed takes the + # LAST ≥8-char hex/dash parenthetical WITHOUT a $ anchor: real simctl lines + # end in a (Shutdown)/(Booted) state suffix (often with trailing space), and + # the state word is never in the hex class, so the UDID is always the last + # in-class group even when the device name carries its own parentheses. + XCODE_SIM=$(xcrun simctl list devices available 2>/dev/null | sed -n 's/.*(\([A-F0-9-]\{8,\}\)).*/\1/p' | head -1) + if [ -z "$XCODE_SIM" ]; then + echo "⚠ No available iOS Simulator found — skipping the Xcode build gate. Set workflow.build_command to run it anyway." + BUILD_CMD="" + XCODEPROJ="" else - BUILD_CMD="xcodebuild build -destination 'platform=iOS Simulator,name=iPhone 16'" + XCODE_DEST="id=$XCODE_SIM" + # #4784 review F2: the command string is re-parsed by `bash -c`, so an + # apostrophe in a repo/scheme name would terminate the quote (unbalanced- + # quote failure; a crafted path shape reaches command injection). Escape + # for the single-quoted span at the only boundary GSD owns. + XCODEPROJ=$(printf '%s' "$XCODEPROJ" | sed "s/'/'\\\\''/g") + XCODE_SCHEME=$(printf '%s' "$XCODE_SCHEME" | sed "s/'/'\\\\''/g") + fi + if [ -n "$XCODEPROJ" ] && [ -n "$XCODE_SCHEME" ]; then + BUILD_CMD="xcodebuild build -project '$XCODEPROJ' -scheme '$XCODE_SCHEME' -destination '${XCODE_DEST}'" + elif [ -n "$XCODEPROJ" ]; then + BUILD_CMD="xcodebuild build -project '$XCODEPROJ' -destination '${XCODE_DEST}'" fi elif [ -f "Makefile" ] && grep -q "^build:" Makefile; then BUILD_CMD="make build" @@ -68,14 +91,28 @@ TEST_CMD=$(gsd_run query config-get workflow.test_command --default "" --raw 2>/ if [ -z "$TEST_CMD" ]; then XCODEPROJ=$(find . -maxdepth 2 -name "*.xcodeproj" -not -path "*/node_modules/*" 2>/dev/null | head -1) if [ -n "$XCODEPROJ" ]; then - # Xcode project: reuse scheme detected above (or re-detect) + # Xcode project: reuse scheme + simulator resolved above (or re-detect) if [ -z "${XCODE_SCHEME:-}" ]; then XCODE_SCHEME=$(xcodebuild -list -json -project "$XCODEPROJ" 2>/dev/null | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('project',{}).get('schemes',[None])[0] or '')" 2>/dev/null || true) fi - if [ -n "$XCODE_SCHEME" ]; then - TEST_CMD="xcodebuild test -scheme '$XCODE_SCHEME' -destination 'platform=iOS Simulator,name=iPhone 16'" + # #4784: reuse the simulator resolved by the build gate; re-detect if this + # block runs in a fresh shell. No simulator → skip loudly, same as Step A. + if [ -z "${XCODE_SIM:-}" ]; then + XCODE_SIM=$(xcrun simctl list devices available 2>/dev/null | sed -n 's/.*(\([A-F0-9-]\{8,\}\)).*/\1/p' | head -1) + fi + if [ -z "$XCODE_SIM" ]; then + echo "⚠ No available iOS Simulator found — skipping the Xcode test gate. Set workflow.test_command to run it anyway." + TEST_CMD="" + XCODEPROJ="" else - TEST_CMD="xcodebuild test -destination 'platform=iOS Simulator,name=iPhone 16'" + XCODE_DEST="id=$XCODE_SIM" + XCODEPROJ=$(printf '%s' "$XCODEPROJ" | sed "s/'/'\\\\''/g") + XCODE_SCHEME=$(printf '%s' "$XCODE_SCHEME" | sed "s/'/'\\\\''/g") + fi + if [ -n "$XCODEPROJ" ] && [ -n "$XCODE_SCHEME" ]; then + TEST_CMD="xcodebuild test -project '$XCODEPROJ' -scheme '$XCODE_SCHEME' -destination '${XCODE_DEST}'" + elif [ -n "$XCODEPROJ" ]; then + TEST_CMD="xcodebuild test -project '$XCODEPROJ' -destination '${XCODE_DEST}'" fi elif [ -f "Makefile" ] && grep -q "^test:" Makefile; then TEST_CMD="make test" @@ -105,7 +142,7 @@ TEST_EXIT=$? if [ "${TEST_EXIT}" -eq 0 ]; then echo "✓ Post-merge test gate passed — no cross-plan conflicts" elif [ "${TEST_EXIT}" -eq 124 ]; then - echo "⚠ POST-MERGE TEST GATE TIMED OUT after ${TEST_GATE_TIMEOUT}s — the runner did not exit, likely stuck in watch/dev mode (e.g. vitest without 'run'). Verify tests with a one-shot command (e.g. 'vitest run') or raise workflow.test_gate_timeout." + echo "⚠ POST-MERGE TEST GATE TIMED OUT after ${TEST_GATE_TIMEOUT}s — the runner did not exit, likely stuck in watch/dev mode (e.g. vitest without 'run'). Verify tests with a one-shot command (e.g. 'vitest run') or raise workflow.test_gate_timeout. For Xcode suites: xcodebuild collects a sysdiagnose into the .xcresult by default (~10 minutes on a passing suite) — add '-collect-test-diagnostics never' to the test command or raise workflow.test_gate_timeout." else echo "✗ Post-merge test gate failed (exit code ${TEST_EXIT})" WAVE_FAILURE_COUNT=$((WAVE_FAILURE_COUNT + 1)) diff --git a/scripts/lib/platform-conformance-tier.generated.cjs b/scripts/lib/platform-conformance-tier.generated.cjs index 1e63fc957..b7478e636 100644 --- a/scripts/lib/platform-conformance-tier.generated.cjs +++ b/scripts/lib/platform-conformance-tier.generated.cjs @@ -255,6 +255,7 @@ module.exports = { "tests/state-todos-render.test.cjs", "tests/state.test.cjs", "tests/teams-status.test.cjs", + "tests/test-gate-watch-mode.test.cjs", "tests/todos-workstream-scope.test.cjs", "tests/undo-commit-selection-4465.test.cjs", "tests/unreachable-guard-drift.test.cjs", diff --git a/tests/test-gate-watch-mode.test.cjs b/tests/test-gate-watch-mode.test.cjs index 086409853..08f87ee5b 100644 --- a/tests/test-gate-watch-mode.test.cjs +++ b/tests/test-gate-watch-mode.test.cjs @@ -31,6 +31,12 @@ const ROOT = path.join(__dirname, '..'); const REGRESSION_GATE = path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase', 'steps', 'regression-gate.md'); const REGRESSION_GATE_RUN = path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase', 'steps', 'regression-gate-run.md'); const POST_MERGE_GATE = path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase', 'steps', 'post-merge-gate.md'); + +// #4784: bounds the one-shot sed pipeline the behavioral extraction test runs +// against a fixture line (a pure text filter on ~50 lines — never the 30s-plus +// subprocess classes in timeouts.cjs; named so the two copies in this file +// cannot drift). +const SIMCTL_EXTRACT_TIMEOUT_MS = 10_000; const AUDIT_FIX = path.join(ROOT, 'gsd-core', 'workflows', 'audit-fix.md'); const EXECUTE_PHASE = path.join(ROOT, 'gsd-core', 'workflows', 'execute-phase.md'); @@ -124,3 +130,88 @@ describe('#2350: every gate resolves build/test commands with --raw', () => { }); } }); + +// ─── #4784: the Xcode arms carry -project and a machine-independent destination ── + +describe('#4784: Xcode gate construction', () => { + const gate = read(POST_MERGE_GATE); + + test('#4784: the Xcode gates pass -project to every constructed xcodebuild command', () => { + // The gate resolves XCODEPROJ (find . -maxdepth 2) and even uses it for + // `xcodebuild -list -json -project` — but the build/test commands used to + // drop it, so any project one directory down (App/App.xcodeproj) failed + // with exit 66: xcodebuild does not search subdirectories. + const constructed = gate.match(/xcodebuild (?:build|test)[^\n]*'/g) || []; + assert.ok(constructed.length >= 4, `expected the four Xcode command constructions, got: ${JSON.stringify(constructed)}`); + for (const cmd of constructed) { + assert.ok( + cmd.includes("-project '$XCODEPROJ'"), + `every constructed xcodebuild command must carry -project '$XCODEPROJ', got: ${cmd}`, + ); + } + }); + + test('#4784: the Xcode destination derives from the machine\'s available simulators', () => { + // A concrete device NAME is machine state (the reporter's machine had zero + // devices matching the hardcoded name). The gate must resolve the first + // available simulator (id= destination form) and must not pin a name. + assert.ok( + gate.includes('simctl list devices available'), + 'the gate must resolve the destination from simctl list devices available', + ); + assert.ok(gate.includes('id=$XCODE_SIM'), 'the destination must reference the resolved simulator id'); + assert.ok( + !gate.includes('name=iPhone 16'), + 'the hardcoded simulator name must be gone', + ); + }); + + test('#4784: with no available simulator the gates skip loudly instead of guaranteed failure', () => { + // Pin BOTH skip echoes: the build arm names workflow.build_command, the + // test arm names workflow.test_command (a deleting-the-echo edit must go red). + assert.ok( + /No available iOS Simulator[^\n]*workflow\.build_command/.test(gate), + 'build-arm skip must name workflow.build_command', + ); + assert.ok( + /No available iOS Simulator[^\n]*workflow\.test_command/.test(gate), + 'test-arm skip must name workflow.test_command', + ); + // set -u robustness: the skip paths must leave the command variables ASSIGNED. + const skipArms = gate.match(/if \[ -z "\$XCODE_SIM" \]; then[\s\S]*?\n {4}fi/g) || []; + assert.ok(skipArms.length >= 2, `expected both skip arms, got ${skipArms.length}`); + for (const arm of skipArms) { + assert.ok( + /(?:BUILD|TEST)_CMD=""/.test(arm), + 'each skip arm must assign its command variable (a set -u agent shell must not abort on an unbound variable)', + ); + } + }); + + test('#4784: the gate\'s simulator-extraction pipeline yields a UDID from a real-format simctl line', () => { + // Adversarial-review Finding 1 (behavioral, would have been red): the first + // cut anchored the sed at end-of-line, but real simctl lines end with a + // (Shutdown)/(Booted) state suffix (often + trailing space) — the anchored + // form matched NOTHING and the gates skipped on every machine. Extract the + // gate's OWN pipeline and execute it against a real-format fixture line. + const pipeMatch = gate.match(/simctl list devices available 2[^\n]*?(sed -n '[^']+')[^\n]*?head -1/); + assert.ok(pipeMatch, 'gate must contain the simctl→sed→head extraction pipeline'); + const realFormatLine = ' iPhone 17 Pro (94B8198C-8CF1-4860-994A-5669D3388BE8) (Shutdown) '; + const { execFileSync } = require('node:child_process'); + const udid = execFileSync('sh', ['-c', `printf '%s\\n' "$1" | ${pipeMatch[1]} | head -1`, 'sh', realFormatLine], { + encoding: 'utf-8', + timeout: SIMCTL_EXTRACT_TIMEOUT_MS, + }).trim(); + assert.match(udid, /^[A-F0-9-]{8,}$/, `the pipeline must extract the UDID from a real-format line, got: ${JSON.stringify(udid)}`); + }); + + test('#4784: the test-gate timeout guidance names the Xcode diagnostics collector', () => { + // A real iOS suite measured 854s wall clock, ~600s of it simctl diagnose + // collecting a sysdiagnose AFTER the suite passed — the default timeout + // returns 124 on a passing suite. The gate text must surface the flag. + assert.ok( + gate.includes('-collect-test-diagnostics never'), + 'the gate must mention -collect-test-diagnostics never for Xcode suites', + ); + }); +});