Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- "Audio Troubleshooting" no longer opens as a small empty sheet. Same cause
as the diagnosis sheet: presented on a flag while the content read a
separate optional; it is presented from the engine itself now.
- The bottle's Terminal button works for bottles whose name contains a space.
The name was backslash-escaped inside double quotes, so the shell passed
the backslashes through and WhiskyCmd reported that no such bottle exists.
- Guided troubleshooting no longer dead-ends on a findings card. Info steps
such as "Missing dependencies found" only carry a Continue transition, and
nothing followed it; the wizard now shows a Continue button there, and Skip
moves on as well.
- Guided troubleshooting no longer reports "Problem resolved" when it has
run out of automated steps. The flows' escalation node shares the export
phase with the resolved node, and the wizard drew both the same way; a node
that hands off to the escalation fragment now shows the escalation screen
with its export and retry options.
- Diagnostic exports with "Include sensitive details" off no longer carry
credentials in plain sight. Launch arguments were written to the archive
verbatim regardless of the toggle, and log redaction only rewrote the home
Expand Down
11 changes: 8 additions & 3 deletions Whisky/Extensions/Bottle+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,14 @@ extension Bottle {
func openTerminal() {
guard let whiskyCmdURL = Bundle.main.url(forResource: "WhiskyCmd", withExtension: nil) else { return }

// Build a shell command that sources the WhiskyCmd environment
// Use .esc to escape shell metacharacters and prevent command injection
let command = "eval \"$(\"\(whiskyCmdURL.esc)\" shellenv \"\(settings.name.esc)\")\""
// Build a shell command that sources the WhiskyCmd environment.
// Single-quoted through ShellQuoting: `.esc` backslash-escapes spaces,
// and inside double quotes bash keeps those backslashes, so any bottle
// name with a space reached WhiskyCmd as `QA\ Smoke` and failed to
// resolve.
let whiskyCmd = whiskyCmdURL.path(percentEncoded: false)
let shellenv = ShellQuoting.commandLine([whiskyCmd, "shellenv", settings.name])
let command = "eval \"$(\(shellenv))\""
let scriptContent = "#!/bin/bash\n\(command)\n"

// Write to temp script file to handle all terminal apps consistently
Expand Down
16 changes: 16 additions & 0 deletions Whisky/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -95267,6 +95267,22 @@
}
}
},
"troubleshooting.wizard.continue" : {
"localizations" : {
"en" : {
"stringUnit" : {
"state" : "translated",
"value" : "Continue"
}
},
"en-GB" : {
"stringUnit" : {
"state" : "translated",
"value" : "Continue"
}
}
}
},
"troubleshooting.wizard.runningChecks" : {
"localizations" : {
"en" : {
Expand Down
4 changes: 4 additions & 0 deletions Whisky/Views/Troubleshooting/TroubleshootingWizardView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ extension TroubleshootingWizardView {
.disabled(engine.session.stepHistory.count <= 1)
Button("troubleshooting.wizard.skip") { engine.skipStep() }
.disabled(engine.currentNode == nil)
if engine.canContinue {
Button("troubleshooting.wizard.continue") { engine.continueStep() }
.keyboardShortcut(.defaultAction)
}
}
}
.padding(.horizontal, 16)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,16 @@ public final class TroubleshootingFlowEngine: ObservableObject {
return
}

// A flow's own hand-off to the escalation fragment. Every flow ends
// its unresolved branch on an info node that references it, tagged
// with the export phase like the resolved node, so without this the
// wizard showed "Problem resolved" to someone it had given up on.
if node.fragmentRef == "export-escalation" {
logger.debug("Node \(node.id) hands off to the escalation fragment")
escalate()
return
}

// Cycle protection
let isUserInteraction = node.type == .fix || node.type == .verify
if isUserInteraction {
Expand Down Expand Up @@ -348,15 +358,36 @@ public final class TroubleshootingFlowEngine: ObservableObject {
/// Skips the current step, navigating to the "skipped" or "default" target.
///
/// Per the locked "skip for now" decision, users can skip any step.
/// Whether the current node has a `continue` transition, which is how an
/// info node hands over to the next step.
public var canContinue: Bool {
currentNode?.on?["continue"] != nil
}

/// Follows the current node's `continue` transition.
///
/// Info nodes (a findings card between a check and its fix) only carry
/// this transition. Nothing followed it, so every flow that reached one
/// stopped there with Skip and Back as the only controls.
public func continueStep() {
follow(["continue", "default"], reason: "Continue")
}

/// Skips the current step. `continue` is the last fallback: skipping an
/// info node means moving on.
public func skipStep() {
guard let node = currentNode else { return }
follow(["skipped", "default", "continue"], reason: "Skip")
}

if let nextNodeId = node.on?["skipped"] ?? node.on?["default"] {
logger.debug("Skipping step \(node.id) -> \(nextNodeId)")
navigateToNode(nextNodeId)
} else {
logger.warning("No skip target for node \(node.id)")
private func follow(_ keys: [String], reason: String) {
guard let node = currentNode else { return }
guard let nextNodeId = keys.lazy.compactMap({ node.on?[$0] }).first else {
logger.warning("No \(reason) target for node \(node.id)")
return
}
logger.debug("\(reason): step \(node.id) -> \(nextNodeId)")
session.recordBranch(from: node.id, targetNodeId: nextNodeId, reason: reason)
navigateToNode(nextNodeId)
}

/// Goes back to the previous step in the history.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
//
// TroubleshootingFlowEngineTransitionTests.swift
// WhiskyKitTests
//
// This file is part of Whisky.
//
// Whisky is free software: you can redistribute it and/or modify it under the terms
// of the GNU General Public License as published by the Free Software Foundation,
// either version 3 of the License, or (at your option) any later version.
//
// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
// See the GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License along with Whisky.
// If not, see https://www.gnu.org/licenses/.
//

@testable import WhiskyKit
import XCTest

/// Transitions the wizard drives by hand: Continue on an info node, Skip's
/// fallback to it, and the hand-off to the escalation fragment.
@MainActor
final class TroubleshootingFlowEngineTransitionTests: XCTestCase {
private func infoNode(
_ nodeId: String,
phase: FlowPhase = .checks,
transitions: [String: String]? = nil,
fragmentRef: String? = nil
) -> FlowStepNode {
FlowStepNode(
id: nodeId, type: .info, phase: phase, title: "Info \(nodeId)", on: transitions, fragmentRef: fragmentRef
)
}

private func makeEngine(
nodes: [String: FlowStepNode],
fragments: [String: FlowDefinition] = [:]
) -> TroubleshootingFlowEngine {
let flow = FlowDefinition(version: 1, categoryId: "graphics", nodes: nodes, entryNodeId: "start")
var session = TroubleshootingSession(bottleURL: URL(filePath: "/tmp/engine-transition-test-bottle"))
session.currentFlowCategoryId = "graphics"
return TroubleshootingFlowEngine(
flowDefinitions: ["graphics": flow],
fragments: fragments,
checkRegistry: CheckRegistry(),
sessionStore: SpySessionStore(),
session: session
)
}

func testContinueFollowsTheInfoNodeTransition() {
let engine = makeEngine(nodes: [
"start": infoNode("start", transitions: ["continue": "next"]),
"next": infoNode("next")
])

engine.navigateToNode("start")
XCTAssertTrue(engine.canContinue)

engine.continueStep()

XCTAssertEqual(engine.currentNode?.id, "next")
XCTAssertFalse(engine.canContinue)
}

func testSkipFallsBackToTheContinueTransition() {
let engine = makeEngine(nodes: [
"start": infoNode("start", transitions: ["continue": "next"]),
"next": infoNode("next")
])

engine.navigateToNode("start")
engine.skipStep()

XCTAssertEqual(engine.currentNode?.id, "next")
}

func testNodeReferencingTheEscalationFragmentEscalates() {
let fragment = FlowDefinition(
version: 1,
categoryId: "export-escalation",
nodes: ["export-start": infoNode("export-start", phase: .export)],
entryNodeId: "export-start"
)
let engine = makeEngine(
nodes: [
"start": infoNode("start", transitions: ["continue": "escalate"]),
"escalate": infoNode("escalate", phase: .export, fragmentRef: "export-escalation")
],
fragments: ["export-escalation": fragment]
)

engine.navigateToNode("start")
engine.continueStep()

XCTAssertEqual(engine.session.phase, .escalation)
XCTAssertEqual(engine.currentNode?.id, "export-start")
}
}
Loading