[malpern]

Root cause

Since 2026-10-20 (between run 39469912007 at 17:53Z, clean, and run 39509801293 at 22:40Z), the KeyPathTests xctest process traps with signal 5 every run in LauncherStoreTransactionTests.testQueuedEditsMergeWithoutLosingOtherMappings.

• Every suite after that test in the bundle never runs. That includes OverlayLayerRevealPolicyTests and PreferencesServicePersistenceTests from Overlay setting: show when a layer is active #1337.
• Earlier runs, for example 39342659139, ran those suites.
• CI stayed green because KEYPATH_ALLOW_TEST_RUNNER_CRASH_SUCCESS=1 passes any signal crash with zero parsed failures. Run 40119737918 logged Allowed SwiftPM-wrapped test runner signal crash after 3201 parsed passes. The allowance was meant for crashes after the tests finish, not in the middle.
• The code diff across the boundary is small and unrelated (session Caps Lock files), and the runner and Xcode were the same. So the trigger looks environmental.
• That test is the only one whose launcher edit warms a URL favicon. That does a live network fetch, then AppKit image drawing, on a runner that became a headless system LaunchDaemon on 2026-10-19. This is the suspected trap. I couldn't read the runner's crash report, because the mini is offline.

Changes

• Scripts/run-tests-safe.sh:
  • The crash allowance applies only when every started test case and every .xctest bundle finished. Otherwise it fails and names the interrupted test.
  • On any signal crash it prints the triggering thread from the newest xctest .ips report (user and /Library DiagnosticReports).
  • Checked against the real logs: the 40119737918 log is flagged as Interrupted test: -[KeyPathTests.LauncherStoreTransactionTests testQueuedEditsMergeWithoutLosingOtherMappings], and the clean 39342659139 log passes.
• FaviconFetcher: no network fetch under tests.
• Contract test for the allowance, in DeploymentScriptContractTests.

Verification

Not yet run in CI. Both self-hosted runners (keypath-mini, keypath-mini-2) are offline, because the mini has been off the tailnet since about 2026-10-24.

• If the favicon guess is right, the next run passes and the Test Suite 'OverlayLayerRevealPolicyTests' passed and Test Suite 'PreferencesServicePersistenceTests' passed lines appear.
• If it's wrong, the run now fails and prints the crash thread, which names the real cause.

The local review gate couldn't authenticate, so the remote review gate is selected. Don't merge until GitHub claude-review passes and the CI evidence above is in.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-25T14:15:06.672514Z	80c56df	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Review (diff only)

The change is sound: a mid-bundle crash can no longer pass as a teardown crash. Two things to check:

1. print_crash_report_excerpt may abort the script under set -e/pipefail. report=$(find ... 2>/dev/null | sort | tail -n 1) returns find's exit status when pipefail is on. find exits non-zero if /Library/Logs/DiagnosticReports or the user dir is missing, which is likely on CI runners and Linux. If the script uses set -e, it would exit before the crash-allowance logic and replace the intended message with a bare failure. Add || true to that pipeline. Also confirm TEST_START_SECONDS is always set before this point, since set -u would trip on it.
2. crash_interrupted_run only sees XCTest-style Test Case and Test Suite ... .xctest lines. A crash during Swift Testing (@Test) output leaves nothing open, so it is treated as not interrupted and can still pass under the allowance. If Swift Testing suites run here, parse its Test ... started and passed lines as well, or document the gap.

Minor: testRunnerCrashAllowanceNeverHidesSkippedTests asserts on exact source substrings, including a two-space indent. It will break on harmless reformatting. It also doesn't exercise crash_interrupted_run against a sample log, and a small fixture-based test would cover that. The FaviconFetcher guard looks fine.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 80c56df14d

ℹ️ 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".

[chatgpt-codex-connector]

Make crash-report discovery tolerate missing directories

If either DiagnosticReports directory is absent or unreadable, this assignment returns a nonzero status and terminates the runner because the script enables set -euo pipefail. Bash's help set describes -e as “Exit immediately if a command exits with a non-zero status.” Both signal-crash branches call this function unconditionally, so an optional diagnostic lookup prevents the interrupted-test check and the explicitly enabled teardown-crash allowance from running, even if the other directory contains a usable report. Reproducing the extracted function with a nonexistent report directory exits before the fallback message. Filter out unavailable directories or explicitly tolerate discovery errors so missing diagnostics cannot determine the test result.

Useful? React with 👍 / 👎.

[malpern]

Local evidence (MacBook Air, interactive GUI session), 2026-10-25.

This is the full suite on current master 044a60a4, without this PR, so the favicon network fetch still ran. It used KEYPATH_DEV_XCODE_DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer ./Scripts/run-tests-safe.sh with Xcode 27.0 (27A266a), because the pinned 26.6 isn't installed here.

• Result: ✅ All tests passed (5831 passed), runner exit 0, with no signal and no crash allowance used (build 62 s, test 98 s).
• The crashing test: LauncherStoreTransactionTests.testQueuedEditsMergeWithoutLosingOtherMappings passed (0.296 s), and the rest of its suite passed.
• The suites the CI crash cut off: OverlayLayerRevealPolicyTests and PreferencesServicePersistenceTests both ran and passed.

So the trap doesn't reproduce in a logged-in GUI session with the same test. That fits the headless-runner explanation, where the URL favicon fetch and AppKit drawing happen under the system LaunchDaemon, but it doesn't prove it. The CI run with this PR remains the real check.

🤖 Generated with Claude Code