Skip to content

Commit 7cbafdb

Browse files
authored
Remove issue #135 Phase 4A diagnostic markers (#140)
1 parent 6996c58 commit 7cbafdb

2 files changed

Lines changed: 7 additions & 61 deletions

File tree

Sources/PreviewsCLI/MCPServer.swift

Lines changed: 7 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -504,23 +504,6 @@ private func startMacOSPreview(
504504
}
505505

506506
private func handlePreviewSnapshot(params: CallTool.Parameters) async throws -> CallTool.Result {
507-
// TODO(#135-4B): remove these markers once the actual post-reload
508-
// wedge fix lands. Instrumentation for issue #135 Phase 4A — the
509-
// post-reload wedge observed in CI (PRs #133/#134 history) manifests
510-
// as the daemon logging "Reloaded!" and then never responding to the
511-
// next `preview_snapshot`. Stderr goes silent at that point, so we
512-
// can't tell whether the handler entered at all, hung on the main
513-
// actor, or wedged inside `Snapshot.capture`. These prints let
514-
// the next CI failure (captured by the PR #134 post-failure dump
515-
// step) localize the hang to a specific await point.
516-
//
517-
// Cheap (fputs + fflush, no Swift concurrency), additive (all
518-
// go to the same stderr the tests already capture), and not
519-
// user-visible — `DaemonClient.registerStderrLogForwarder`
520-
// forwards MCP `LogMessageNotification` payloads, not raw daemon
521-
// stderr. macOS snapshot handler only; iOS returns early above.
522-
fputs("[snapshot] enter\n", stderr); fflush(stderr)
523-
524507
let sessionID: String
525508
do { sessionID = try extractString("sessionID", from: params) } catch {
526509
return CallTool.Result(content: [.text(error.localizedDescription)], isError: true)
@@ -543,62 +526,30 @@ private func handlePreviewSnapshot(params: CallTool.Parameters) async throws ->
543526
// macOS path. Verify existence upfront so a typo'd sessionID
544527
// surfaces as a clean "No session found" rather than the misleading
545528
// "capture failed" from `window(for:)` returning nil.
546-
fputs("[snapshot] pre session-check\n", stderr); fflush(stderr)
547529
let isMacOSSession = await MainActor.run {
548530
host.allSessions[sessionID] != nil
549531
}
550-
fputs("[snapshot] post session-check\n", stderr); fflush(stderr)
551532
guard isMacOSSession else {
552533
return CallTool.Result(
553534
content: [.text("No session found for \(sessionID).")],
554535
isError: true
555536
)
556537
}
557538

558-
// Previously: `try await Task.sleep(for: .milliseconds(300))` with
559-
// the comment "Wait briefly for SwiftUI to finish layout" (present
560-
// since the initial commit, line 820b0cd). CI evidence in PR #139
561-
// (run 72439165932) showed the handler wedging here under
562-
// cooperative-pool starvation: the `[snapshot] post session-check`
563-
// marker fired but the subsequent `[snapshot] post 300ms sleep`
564-
// never did, across many snapshot cycles of `hotReloadLiteralOnly`.
565-
// `Task.sleep` ultimately depends on the Swift concurrency
566-
// scheduler to resume; accumulating load from rapid polling
567-
// snapshots (the literal-only path doesn't have swiftc breaks to
568-
// let the pool drain) left the timer's continuation unscheduled.
569-
//
570-
// The sleep is unnecessary in practice: `Snapshot.capture` calls
571-
// `NSView.cacheDisplay(in:to:)`, which forces layout synchronously
572-
// when the view is dirty. The network round-trip from client to
573-
// daemon already gives the UI plenty of time to settle. All 7
574-
// MacOSMCPTests pass without the sleep, including the two that
575-
// verify image bytes actually change after a reload.
539+
// Don't add a pre-capture `Task.sleep` here for "layout settling" —
540+
// `cacheDisplay` below forces layout synchronously, and under rapid
541+
// snapshot polling a cooperative sleep can starve (see issue #135).
576542

577543
let format: Snapshot.ImageFormat = usePNG ? .png : .jpeg(quality: quality)
578-
let imageData: Data
579-
do {
580-
imageData = try await MainActor.run {
581-
fputs("[snapshot] main-actor capture enter\n", stderr); fflush(stderr)
582-
guard let window = host.window(for: sessionID) else {
583-
throw SnapshotError.captureFailed
584-
}
585-
let data = try Snapshot.capture(window: window, format: format)
586-
fputs("[snapshot] main-actor capture done (\(data.count) bytes)\n", stderr)
587-
fflush(stderr)
588-
return data
544+
let imageData: Data = try await MainActor.run {
545+
guard let window = host.window(for: sessionID) else {
546+
throw SnapshotError.captureFailed
589547
}
590-
} catch {
591-
// Marker fires on both the `captureFailed` and any other throw
592-
// from the MainActor block so the dump shows "we exited the
593-
// block one way or another" — not just the happy path.
594-
fputs("[snapshot] main-actor capture threw: \(error)\n", stderr); fflush(stderr)
595-
throw error
548+
return try Snapshot.capture(window: window, format: format)
596549
}
597-
fputs("[snapshot] encoding\n", stderr); fflush(stderr)
598550

599551
let base64 = imageData.base64EncodedString()
600552

601-
fputs("[snapshot] returning\n", stderr); fflush(stderr)
602553
return CallTool.Result(content: [
603554
.image(data: base64, mimeType: mimeType, metadata: nil)
604555
])

Sources/PreviewsMacOS/HostApp.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,6 @@ public class PreviewHost: NSObject, NSApplicationDelegate {
212212
fputs("Reload failed: \(error)\n", stderr); fflush(stderr)
213213
}
214214
}
215-
// Phase 4A instrumentation (issue #135): marks the end
216-
// of the MainActor.run block so we can tell in the CI
217-
// stderr dump whether the wedge happens inside the
218-
// reload or in the handler that runs after it.
219-
fputs("[reload] main-actor block returned\n", stderr); fflush(stderr)
220215
} catch {
221216
fputs("Recompilation failed: \(error)\n", stderr)
222217
}

0 commit comments

Comments
 (0)