Skip to content

fix: terminal quoting and the guided troubleshooting dead end - #246

Merged
frankea merged 2 commits into
mainfrom
fix/smoke-round-three
Aug 29, 2026
Merged

fix: terminal quoting and the guided troubleshooting dead end#246
frankea merged 2 commits into
mainfrom
fix/smoke-round-three

Conversation

@frankea

@frankea frankea commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Third round of the local release smoke run (Release build, throwaway bottles), following #245. All three findings are pre-existing and both sit on paths the 3.7.0 changelog touches (the winetricks quoting fix in #242, the troubleshooting fixes in #213).

What was broken

  1. Terminal button fails for bottle names with a space. openTerminal built eval "$("<cmd>" shellenv "<name>")" with .esc, which backslash-escapes spaces. Inside double quotes bash keeps the backslash, so WhiskyCmd received QA\ Smoke\ 7\ Copy and printed "A bottle with that name doesn't exist." Reproduced through Terminal.app; WhiskyCmd shellenv "QA Smoke 7 Copy" works when called directly. The command is now assembled with ShellQuoting.commandLine.

  2. Guided troubleshooting dead-ends on info nodes. In install-dependencies.json the entry check fails into show_missing_deps, whose only transition is continue. Nothing in the engine or the wizard follows a continue transition and Skip only looks at skipped/default, so the wizard sat on "Missing dependencies found" with Back as the only working control. The same shape exists in five of the eight flows. TroubleshootingFlowEngine.continueStep() follows it, the toolbar shows a Continue button (default action) when the node has one, and Skip falls back to continue.

  3. The wizard says "Problem resolved" after giving up. Every flow ends its unresolved branch on an info node tagged phase: export (the same phase as resolved) with fragmentRef: export-escalation, and the wizard renders every export-phase node as the resolved summary. Walked the dependency flow to its end: findings, findings, game-config check, then a green "Problem resolved" for a bottle nothing had been fixed in. A node referencing the escalation fragment now routes through escalate(), which lands on the escalation screen with its export and retry options. Skip and Continue share one transition helper; three engine tests cover Continue, the Skip fallback, and the hand-off.

Verification

  • Terminal.app: bottle "QA Smoke 7 Copy", Terminal button, the tab shows the shellenv exports instead of the error.
  • Wizard: Install / dependency problems on a program with a dependency-loading diagnosis, Continue on the findings card reaches the Fix step.
  • Wizard end state: the same walk now ends on the escalation screen.
  • Kit troubleshooting and quoting suites green (21 engine tests incl. the 3 new ones); SwiftFormat 0.58.7 lint and SwiftLint strict clean; en-GB check passes (new key troubleshooting.wizard.continue).

Third round of the local release smoke run, both pre-existing:

- The bottle's Terminal button failed for any bottle name with a space.
  The shellenv command escaped the name with backslashes inside double
  quotes; bash keeps those backslashes, so WhiskyCmd saw `QA\ Smoke` and
  answered that no such bottle exists. The command is built with
  ShellQuoting now, the same helper the winetricks script uses.
- Guided troubleshooting stopped on every info node. Findings cards such
  as "Missing dependencies found" only carry a `continue` transition and
  neither the engine nor the wizard followed it, so five of the eight flows
  could never reach their fix step. The engine gains continueStep and the
  wizard a Continue button on those nodes; Skip falls back to `continue`
  as well.
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.37500% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Whisky/Extensions/Bottle+Extensions.swift 0.00% 8 Missing ⚠️
...ws/Troubleshooting/TroubleshootingWizardView.swift 0.00% 3 Missing ⚠️
...it/Troubleshooting/TroubleshootingFlowEngine.swift 90.47% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Every flow ends its unresolved branch on an info node that references
the export-escalation fragment, tagged with the export phase like the
resolved node. The wizard draws export-phase nodes as "Problem resolved",
so a flow that had given up told the user their problem was fixed. A node
referencing the fragment now routes through escalate(), which sets the
escalation phase and lands on the fragment's entry node.

Skip and Continue share one transition helper, with tests for both and
for the hand-off.
@frankea

frankea commented Aug 29, 2026

Copy link
Copy Markdown
Owner Author

Reproduced each one on a Release build before the fix and re-checked after a rebuild.

  • Terminal: with the bottle "QA Smoke 7 Copy" selected, Terminal.app's tab showed Error: A bottle with that name doesn't exist. before and a clean shellenv after. WhiskyCmd shellenv itself was never at fault.
  • Wizard Continue: "Install / dependency problems" on a program with a dependency-loading diagnosis. Before: stuck on "Missing dependencies found" with Skip doing nothing. After: Continue reaches the Fix step, and the second findings card too.
  • Escalation: the same walk previously ended on a green "Problem resolved"; with the hand-off routed through escalate() it ends on the escalation screen. Covered by the three new engine tests.

Not in this PR, noted for later: the fix card previews the verb as "unknown" because fix_install_deps carries no verb parameter, and a resolved session still offers "Resume troubleshooting" on the program page.

SwiftFormat 0.58.7 and SwiftLint strict clean, en-GB check passes, engine suite 21/21. Merging.

@frankea
frankea merged commit 94183ff into main Aug 29, 2026
9 checks passed
@frankea
frankea deleted the fix/smoke-round-three branch August 29, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant