From 60bf256345d5aa8650cbab1b7a441d6b315668fc Mon Sep 17 00:00:00 2001
From: Eugene Petrenko <me@jonnyzzz.com>
Date: Tue, 28 Jul 2026 15:49:28 +0200
Subject: [PATCH] Reject non-positive @PPI hints

Codex review flagged that a mistyped "@0" or "@-1" parsed to a non-nil
ppi that persisted and was fed straight into
VZMacGraphicsDisplayConfiguration, bypassing the 72 fallback and risking
a display that fails to build until the user manually resets it.

Guard both ends: the --display parser now keeps only a positive PPI
(zero/negative ignored just like non-numeric input), and
effectivePixelsPerInch treats a non-positive stored value (e.g. from a
hand-edited config) as unset, falling back to 72.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 Sources/tart/Commands/Set.swift     |  6 ++++--
 Sources/tart/VMConfig.swift         | 13 +++++++++----
 Tests/TartTests/VMConfigTests.swift | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/Sources/tart/Commands/Set.swift b/Sources/tart/Commands/Set.swift
index e0016c8..6a8b737 100644
--- a/Sources/tart/Commands/Set.swift
+++ b/Sources/tart/Commands/Set.swift
@@ -95,9 +95,11 @@ extension VMDisplayConfig: ExpressibleByArgument {
     var ppi: Int? = nil
 
     // Optional "@PPI" pixels-per-inch hint, e.g. "3200x1800px@220". Parsed
-    // before the unit suffix since it always trails the whole spec.
+    // before the unit suffix since it always trails the whole spec. Only a
+    // positive value is kept; zero/negative (or non-numeric) is ignored so the
+    // display falls back to the 72 default rather than a broken configuration.
     if let atIndex = argument.lastIndex(of: "@") {
-      ppi = Int(argument[argument.index(after: atIndex)...])
+      ppi = Int(argument[argument.index(after: atIndex)...]).flatMap { $0 > 0 ? $0 : nil }
       argument = String(argument[..<atIndex])
     }
 
diff --git a/Sources/tart/VMConfig.swift b/Sources/tart/VMConfig.swift
index ff1b7cd..e163481 100644
--- a/Sources/tart/VMConfig.swift
+++ b/Sources/tart/VMConfig.swift
@@ -44,11 +44,16 @@ struct VMDisplayConfig: Codable, Equatable {
   var ppi: Int?
 
   // Pixels-per-inch handed to the guest's pixel-unit framebuffer. Defaults to
-  // 72 (non-Retina) when no hint is given, matching Virtualization.framework's
-  // historical assumption; a Retina-class value (e.g. 220) makes the guest
-  // expose a HiDPI (scaling:on) mode regardless of the host display.
+  // 72 (non-Retina) when no positive hint is given, matching
+  // Virtualization.framework's historical assumption; a Retina-class value
+  // (e.g. 220) makes the guest expose a HiDPI (scaling:on) mode regardless of
+  // the host display. A non-positive stored value (e.g. from a hand-edited
+  // config) is treated as unset so it never reaches the display configuration.
   var effectivePixelsPerInch: Int {
-    ppi ?? 72
+    if let ppi, ppi > 0 {
+      return ppi
+    }
+    return 72
   }
 }
 
diff --git a/Tests/TartTests/VMConfigTests.swift b/Tests/TartTests/VMConfigTests.swift
index f85740c..b590a84 100644
--- a/Tests/TartTests/VMConfigTests.swift
+++ b/Tests/TartTests/VMConfigTests.swift
@@ -57,4 +57,19 @@ final class VMConfigTests: XCTestCase {
     XCTAssertEqual(VMDisplayConfig(width: 3200, height: 1800, unit: .pixel, ppi: nil), config)
     XCTAssertEqual(72, config.effectivePixelsPerInch)
   }
+
+  func testRejectsNonPositivePixelsPerInchWhenParsing() throws {
+    // A zero or negative PPI is meaningless and must not persist — it would
+    // otherwise reach VZMacGraphicsDisplayConfiguration and fail to build the
+    // display instead of falling back to the 72 default.
+    XCTAssertNil(VMDisplayConfig(argument: "3200x1800px@0").ppi)
+    XCTAssertNil(VMDisplayConfig(argument: "3200x1800px@-5").ppi)
+  }
+
+  func testEffectivePixelsPerInchIgnoresNonPositiveStoredValue() throws {
+    // Defense for a hand-edited config.json: a stored value <= 0 falls back to
+    // 72 rather than being handed to VZMacGraphicsDisplayConfiguration.
+    XCTAssertEqual(72, VMDisplayConfig(width: 100, height: 100, unit: .pixel, ppi: 0).effectivePixelsPerInch)
+    XCTAssertEqual(72, VMDisplayConfig(width: 100, height: 100, unit: .pixel, ppi: -5).effectivePixelsPerInch)
+  }
 }