Remove issue #135 Phase 4A diagnostic markers#140
Merged
Conversation
The markers were added on the agreement that they'd be removed once Phase 4B landed (the TODO(#135-4B) tag committed to this explicitly). 4B landed in the same PR (#139) and closed issue #135, so the markers have served their purpose. Also trims the 15-line "Previously: Task.sleep(for: .milliseconds(300)) with the comment 'Wait briefly for SwiftUI to finish layout' ..." historical narrative down to a short forward-facing note warning future authors not to re-add a cooperative sleep there. Git history has the full root-cause walkthrough for anyone who needs it. Net removal: ~60 lines of transient investigation context from production code. Architectural notes (`AGENTS.md`, `StallTimer` / `runMCPServer` docstrings) are unaffected — those describe current behavior, not past bug hunts. Verification: 7/7 MacOSMCPTests pass in 93s. `build` + `swift-format lint` clean. The `fflush(stderr)` additions on `Reloaded!` / `Reload failed:` stay — those are a real correctness fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
obj-p
added a commit
that referenced
this pull request
Apr 22, 2026
First CI run on this PR caught two ios-tests failing with the new AsyncProcessTimeout at 15s — both passed on prior green CI runs (PR #140 had `Boot and shutdown a device` in 32s and `End-to-end... screenshot` in 223s total). That tells us the IOSurface → simctl fallback path can legitimately take longer than 15s on slow CI runners, not that simctl is always hung. Bump to 60s. Still catches the pathological 10-minute hang (72501335737 showed 15+ minutes of silence), still gives simctl ample room on realistic CI variance. Fast local runs are unaffected — the timer only fires on actual hangs. Note that local `AsyncProcessTests.timeoutFiresOnHungChild` still runs with a 500ms timeout against `/bin/sleep 30` — it validates the timeout *mechanism* on a guaranteed-hung child regardless of what production code chooses for the bound. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
obj-p
added a commit
that referenced
this pull request
Apr 22, 2026
Prior "iOS simulator tests" bullet list predated this branch. Replace with the architecture that actually exists after the stability + observability work: per-test distinct device via IOSSimulatorPicker, bootDevice blocking via simctl bootstatus, display-attach retry in screenshotData, CI boot-variance budget, iPhone-only filter, AsyncProcessTimeout's captured output. Follows the same rot-avoidance as the PR #140 cleanup: describes current behavior, not investigation history or PR numbers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleanup follow-up to PR #139. The markers (`[snapshot] enter`, `[snapshot] pre session-check`, `[reload] main-actor block returned`, etc.) were added on the explicit understanding — and a `TODO(#135-4B)` tag — that they'd be removed once Phase 4B landed. 4B landed in the same PR (#139) and issue #135 is closed, so the markers have served their purpose.
What's removed
What stays
Rationale
Own repo guidance on code comments says don't reference specific task/PR/CI IDs in code since they rot. The removed comments name specific PR numbers, CI run IDs, and phase labels — exactly that category.
Test plan
Net: 61 lines removed, 7 added. Pure code deletion / comment trim. No behavior change.
🤖 Generated with Claude Code