fix: resolve the findings from the release smoke run - #245
Merged
Conversation
Five defects surfaced while smoke testing main on a fresh bottle ahead of 3.7.0, none of them new to this cycle: - Five interpolated labels rendered their raw key (Recommended graphics card "currently using" line and helper, Virtual Desktop detected display size, console exit code badge and footer). The catalog only carried the plain key, so SwiftUI could not resolve the %@ / %lld form. - "Analyze last run" and "View Latest Diagnosis" presented a small empty sheet. Both used .sheet(isPresented:) with the content reading a separate optional that SwiftUI can evaluate before the value lands. They present from the value now via .sheet(item:). - The bottle's "Export Diagnostic Report" and "View Latest Diagnosis" buttons could never enable: nothing ever stamped lastDiagnosisDate. triggerCrashClassification sets it when it records a diagnosis. - DiagnosticsView had no control of its own; every presentation is a sheet, so the only exit was cmd-period. It has a Done button bound to the cancel action. - "Analyze last run" clicked through to nothing before the first run (the handler guards on the last log). It is disabled until a log exists.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Four more from the same local run, all pre-existing: - A bottle created during the session kept writing to an instance the rest of the app no longer read: createNewBottle cleared inFlight and then reloaded the bottle list, and the reload only keeps in-flight instances. Its first pin never reached the library, the Dock menu, or the menu bar extra until the next reload. The reload now runs while the bottle is still in flight so the instance survives, and it is marked available afterwards. - Kill-on-quit never fired. The Settings toggle is an AppStorage that defaults to on without writing the key, and the handler read it with bool(forKey:), which reports an absent key as off. Behind that, applicationWillTerminate queued the wineserver kill in a Task and the process exited before it ran. The read mirrors the AppStorage default and a synchronous variant runs from the termination handler. - The Audio Troubleshooting wizard presented as an empty sheet, the same isPresented-plus-optional race as the diagnosis sheet. Item-based now. - The crash diagnosis history on a program's page did not reload when a diagnosis was recorded while the page was open.
Owner
Author
|
Reviewed hands-on rather than from the diff: every item was reproduced on a Release build with throwaway bottles before the fix and re-checked after a rebuild.
codecov/patch is red because these are view and app-delegate changes with no unit coverage; nothing in WhiskyKit's logic changed except the synchronous kill helper, which needs a live wineserver to exercise. Merging. |
This was referenced Aug 29, 2026
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.
Findings from a local smoke run of main on a fresh bottle before cutting 3.7.0. All nine are pre-existing (they date back to the v1.0 tree), but they sit on paths the 3.7.0 changelog advertises, so they should not ship again.
What was broken
config.graphics.currentlyUsing,config.virtualDesktop.matchDisplay,console.exitCode, and friends verbatim. The%@/%lldcatalog forms are added at their sorted positions (en + en-GB, no re-serialization).Analyze last runandView Latest Diagnosisused.sheet(isPresented:)with the content closure reading a separate optional. SwiftUI can evaluate that closure before the value lands, which presents a small blank sheet. Both are.sheet(item:)now, so the sheet is built from the value that presents it.lastDiagnosisDate, and nothing ever set it.triggerCrashClassificationstamps it when it records a diagnosis, so the ZIP export is reachable.DiagnosticsViewis a sheet and it had no control of its own; the only exit was cmd-period. It has a Done button on the cancel action, matching the other sheets.lastLogFileURLand returned silently. The button is disabled until a log exists.createNewBottleclearedinFlightand then calledloadBottles(), which only keeps in-flight instances, so the selected bottle page kept writing to an instanceBottleVMno longer held. Reproduced twice (pin on disk, "No pinned programs" in the extra, no card on the page) and gone after the fix: the reload runs while the bottle is still in flight and the kept instance is marked available.@AppStorage("killOnTerminate") = true, which never writes its default, and the handler read it withbool(forKey:), which reports an absent key as off: for anyone who never touched the toggle the setting was silently off. Behind that,applicationWillTerminatequeuedwineserver -kin aTaskand the process exited before it ran. Verified with a running wordpad surviving quit before the fix. The read mirrors the AppStorage default now and a synchronouskillBottleAndWaitruns from the handler.isPresentedplus separate optional race as the diagnosis sheet; presented from the engine now..crashDiagnosisAvailable.Verification
Second round (same Release build recipe, throwaway bottles only): crash classification end to end via the late-crash path (banner, View Diagnosis, history, "Last analyzed", export enabled), redacted ZIP (
"arguments": "-token <redacted>",/Users/<redacted>, no token anywhere) and the sensitive variant with the raw value, remediation card apply, menu bar extra launch of a pin, test tone (WhiskyAudioTest.exe --beepreturnedstatus: ok), and the in-session bottle pin before and after the fix.user.reghashes identical before and after).swift test --package-path WhiskyKitgreen; pinned SwiftFormat 0.58.7 lint andswiftlint --strictclean;british-english.py --checkpasses.