From fc246f533772eda8b3d30d9664750c750a115a26 Mon Sep 17 00:00:00 2001 From: Todd Schulman Date: Mon, 20 Jul 2026 11:30:57 -0400 Subject: [PATCH 1/5] test: add BLACKOUTD_DEFAULTS_SUITE isolation seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route the CLI preference helpers (status/config reads and the auto/verbosity/recovery write-backs) through cliSuiteName(), which returns the BLACKOUTD_DEFAULTS_SUITE env override when set and the real "blackoutd" suite otherwise. When the override is present the write-back subcommands also skip signalling the daemon and print an "isolated test suite" marker. This lets the RSpec CLI harness exercise the accept-paths without mutating the user's real preferences or perturbing the running daemon. The daemon (AppDelegate.m) is deliberately NOT wired to the seam — it always reads the hardcoded kSuiteName — so a test suite can never change real daemon behavior. Env unset reproduces today's behavior exactly. Verified: an isolated `recovery none` leaves the real suite's recoveryStrategy unchanged and emits the isolation marker. Co-Authored-By: Claude Opus 4.8 --- src/main.m | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/main.m b/src/main.m index d0029c0..e1a546e 100644 --- a/src/main.m +++ b/src/main.m @@ -35,6 +35,25 @@ static NSString *const kRecoveryKey = @"recoveryStrategy"; static NSString *const kAgentLabel = @BD_BUNDLE_ID; +// Test-only isolation seam. When BLACKOUTD_DEFAULTS_SUITE is set to a +// non-empty value in the environment, the CLI preference helpers read and +// write that throwaway suite instead of the real "blackoutd" suite, and the +// write-back subcommands (auto / verbosity / recovery) skip signalling the +// daemon. This lets the RSpec CLI harness exercise the accept-paths without +// mutating the user's real preferences or perturbing the running daemon, +// which always reads the hardcoded kSuiteName — this seam is deliberately +// NOT wired into AppDelegate.m. Unset (normal operation) ⇒ identical to +// targeting kSuiteName directly with the daemon signalled as before. +static NSString *cliSuiteName(void) { + const char *override = getenv("BLACKOUTD_DEFAULTS_SUITE"); + return (override && override[0] != '\0') ? @(override) : kSuiteName; +} + +static BOOL cliDefaultsIsolated(void) { + const char *override = getenv("BLACKOUTD_DEFAULTS_SUITE"); + return override != NULL && override[0] != '\0'; +} + static NSString *agentPlistPath(void) { return [NSHomeDirectory() stringByAppendingPathComponent: @@ -554,7 +573,7 @@ static void appendConnectionModes(NSMutableString *r) { static NSString *buildReport(void) { NSProcessInfo *info = NSProcessInfo.processInfo; NSUserDefaults *defaults = - [[NSUserDefaults alloc] initWithSuiteName:kSuiteName]; + [[NSUserDefaults alloc] initWithSuiteName:cliSuiteName()]; BOOL autoMode = [defaults objectForKey:kAutoBlackoutKey] != nil ? [defaults boolForKey:kAutoBlackoutKey] : YES; @@ -823,7 +842,7 @@ static int runDiagnose(int minutes, NSString *start, NSString *end, BOOL quiet, static int printStatus(void) { pid_t pid = daemonPid(); NSUserDefaults *defaults = - [[NSUserDefaults alloc] initWithSuiteName:kSuiteName]; + [[NSUserDefaults alloc] initWithSuiteName:cliSuiteName()]; BOOL autoMode = [defaults objectForKey:kAutoBlackoutKey] != nil ? [defaults boolForKey:kAutoBlackoutKey] : YES; @@ -850,9 +869,15 @@ static int setAutoBlackout(const char *value) { } BOOL enable = strcmp(value, "on") == 0; NSUserDefaults *defaults = - [[NSUserDefaults alloc] initWithSuiteName:kSuiteName]; + [[NSUserDefaults alloc] initWithSuiteName:cliSuiteName()]; [defaults setBool:enable forKey:kAutoBlackoutKey]; [defaults synchronize]; + if (cliDefaultsIsolated()) { + printf("auto-blackout: %s (isolated test suite '%s'; daemon not " + "notified)\n", + enable ? "enabled" : "disabled", cliSuiteName().UTF8String); + return 0; + } printf("auto-blackout: %s\n", enable ? "enabled" : "disabled"); return sendSignalToDaemon(SIGHUP); } @@ -872,10 +897,15 @@ static int setRecoveryStrategy(const char *value) { return 1; } NSUserDefaults *defaults = - [[NSUserDefaults alloc] initWithSuiteName:kSuiteName]; + [[NSUserDefaults alloc] initWithSuiteName:cliSuiteName()]; [defaults setObject:@(value) forKey:kRecoveryKey]; [defaults synchronize]; NSString *applied = [defaults stringForKey:kRecoveryKey]; + if (cliDefaultsIsolated()) { + printf("recovery: %s (isolated test suite '%s'; daemon not notified)\n", + applied.UTF8String, cliSuiteName().UTF8String); + return 0; + } pid_t pid = daemonPid(); if (pid > 0) { if (kill(pid, SIGHUP) != 0) { @@ -927,12 +957,17 @@ static int setVerbosity(const char *value) { } NSUserDefaults *defaults = - [[NSUserDefaults alloc] initWithSuiteName:kSuiteName]; + [[NSUserDefaults alloc] initWithSuiteName:cliSuiteName()]; [defaults setInteger:level forKey:kVerbosityKey]; [defaults synchronize]; // Read the persisted value back so the reported number is what the daemon // will load on reload, not merely the parsed input. long applied = (long)[defaults integerForKey:kVerbosityKey]; + if (cliDefaultsIsolated()) { + printf("verbosity: %ld (isolated test suite '%s'; daemon not notified)\n", + applied, cliSuiteName().UTF8String); + return 0; + } pid_t pid = daemonPid(); if (pid > 0) { if (kill(pid, SIGHUP) != 0) { From d4cb07e9eef07db91659c8aec67fed32de8ec875 Mon Sep 17 00:00:00 2001 From: Todd Schulman Date: Mon, 20 Jul 2026 11:31:14 -0400 Subject: [PATCH 2/5] test: add CLI behavior spec (31 examples) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First automated coverage of the blackoutd CLI, none of which existed before. Builds the binary once in before(:all) (the RSpec CI job runs on macOS but does not build) and exercises the pure surface — top-level dispatch and usage, --version, and every validation/rejection path for verbosity/recovery/auto/recover/repro — plus the --dry-run repro walkthrough, whose ordering assertions pin the W1 fix (trigger precedes scheduling the wake; lock precedes schedule; an "awake" cue lands between sleep and capture; --wake 0 skips sudo). The auto/verbosity/recovery accept-paths run against a throwaway NSUserDefaults suite via BLACKOUTD_DEFAULTS_SUITE, asserting the isolation marker; one example confirms an isolated write leaves the real suite untouched. Hardware- and daemon-affecting behavior stays in spec/manual/TESTING.md. Co-Authored-By: Claude Opus 4.8 --- spec/integration/cli_spec.rb | 331 +++++++++++++++++++++++++++++++++++ 1 file changed, 331 insertions(+) create mode 100644 spec/integration/cli_spec.rb diff --git a/spec/integration/cli_spec.rb b/spec/integration/cli_spec.rb new file mode 100644 index 0000000..f57961b --- /dev/null +++ b/spec/integration/cli_spec.rb @@ -0,0 +1,331 @@ +# SPDX-FileCopyrightText: Copyright 2026 Todd Schulman +# +# SPDX-License-Identifier: GPL-3.0-or-later + +require "open3" + +# Behavioral tests for the blackoutd CLI's argument handling — the surface +# the manual checklist (spec/manual/TESTING.md) cannot cover cheaply: +# usage/validation/rejection paths, --version, --dry-run walkthroughs, and +# the preference write-back subcommands. Hardware- and daemon-affecting +# behavior (actual blackout, real sleep/wake recovery, launchctl lifecycle, +# live display cycling) stays in the manual checklist by design. +# +# The write-back subcommands (auto / verbosity / recovery) normally mutate +# the real "blackoutd" NSUserDefaults suite and SIGHUP the running daemon. +# The accept-path examples here set BLACKOUTD_DEFAULTS_SUITE to a throwaway +# suite (the src/main.m test seam): writes divert to that suite and the +# daemon is not signalled, so these tests never touch the user's real +# preferences or the running daemon. +# +# The macOS-only ObjC binary is built once in before(:all); the RSpec CI job +# runs on macos-latest but does not build, so the suite builds it itself. + +BLACKOUTD_BIN = File.join(REPO_ROOT, "build", "blackoutd") + +module CLISpecHelpers + # Runs the built binary with args and optional env overrides. + # Returns [stdout, stderr, Process::Status]. Output is forced to UTF-8: + # Open3 returns ASCII-8BIT, and the usage text carries em-dashes that + # would raise "invalid byte sequence" on a US-ASCII regex match. + def blackoutd(*args, env: {}) + out, err, status = Open3.capture3(env, BLACKOUTD_BIN, *args) + [out.force_encoding("UTF-8"), err.force_encoding("UTF-8"), status] + end + + # Yields a unique throwaway NSUserDefaults suite name for accept-path + # isolation, then best-effort removes it. Cleanup failure is ignored: the + # suite name is unique per example and read by nothing. + def with_isolated_suite + suite = "blackoutd-test-#{Process.pid}-#{rand(1_000_000)}" + yield suite + ensure + system("defaults", "delete", suite, out: File::NULL, err: File::NULL) + end + + # Raw `defaults read` output for a key in the real suite (value line or the + # "does not exist" message), used to assert the real suite is untouched. + def real_default(key) + out, = Open3.capture2e("defaults", "read", "blackoutd", key) + out.force_encoding("UTF-8") + end +end + +RSpec.describe "blackoutd CLI" do + include CLISpecHelpers + + before(:all) do + skip "macOS-only ObjC binary" unless RUBY_PLATFORM.include?("darwin") + out, err, status = Open3.capture3("make", "-C", REPO_ROOT) + raise "make failed before CLI specs:\n#{out}#{err}" unless status.success? + unless File.executable?(BLACKOUTD_BIN) + raise "binary missing after build: #{BLACKOUTD_BIN}" + end + end + + describe "top-level dispatch" do + it "prints usage to stderr and exits 1 with no arguments" do + _out, err, status = blackoutd + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd") + end + + it "prints usage and exits 1 on an unknown command" do + _out, err, status = blackoutd("frobnicate") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd") + end + + it "lists every subcommand in the usage text" do + _out, err, _status = blackoutd + %w[on off status diagnose recover repro verbosity recovery auto + daemon].each do |cmd| + expect(err).to match(/^\s+#{Regexp.escape(cmd)}\b/) + end + end + + it "prints a semver version and build stamp on --version" do + out, _err, status = blackoutd("--version") + expect(status.exitstatus).to eq(0) + expect(out).to match(/^blackoutd \d+\.\d+\.\d+ /) + expect(out).to include("built:") + end + end + + describe "verbosity" do + it "rejects a missing argument" do + _out, err, status = blackoutd("verbosity") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd verbosity") + end + + it "rejects a non-numeric level" do + _out, err, status = blackoutd("verbosity", "high") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd verbosity") + end + + it "rejects an out-of-range level" do + _out, _err, status = blackoutd("verbosity", "3") + expect(status.exitstatus).to eq(1) + end + + it "persists a valid level to an isolated suite without signalling" do + with_isolated_suite do |suite| + out, _err, status = + blackoutd("verbosity", "2", + env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + expect(status.exitstatus).to eq(0) + expect(out).to match(/verbosity: 2 .*isolated test suite/) + end + end + end + + describe "recovery" do + it "rejects a missing argument" do + _out, err, status = blackoutd("recovery") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd recovery ") + end + + it "rejects an unknown strategy" do + _out, err, status = blackoutd("recovery", "bogus") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd recovery ") + end + + it "accepts displaysleep against an isolated suite" do + with_isolated_suite do |suite| + out, _err, status = + blackoutd("recovery", "displaysleep", + env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + expect(status.exitstatus).to eq(0) + expect(out).to match(/recovery: displaysleep .*isolated test suite/) + end + end + + it "accepts none against an isolated suite" do + with_isolated_suite do |suite| + out, _err, status = + blackoutd("recovery", "none", + env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + expect(status.exitstatus).to eq(0) + expect(out).to match(/recovery: none .*isolated test suite/) + end + end + + it "leaves the real blackoutd suite untouched when isolated" do + before_val = real_default("recoveryStrategy") + with_isolated_suite do |suite| + env = { "BLACKOUTD_DEFAULTS_SUITE" => suite } + blackoutd("recovery", "none", env: env) + blackoutd("recovery", "displaysleep", env: env) + end + expect(real_default("recoveryStrategy")).to eq(before_val) + end + end + + describe "auto" do + it "rejects a missing argument" do + _out, err, status = blackoutd("auto") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd auto [on|off]") + end + + it "rejects an unknown value" do + _out, err, status = blackoutd("auto", "maybe") + expect(status.exitstatus).to eq(1) + expect(err).to include("Usage: blackoutd auto [on|off]") + end + + it "accepts off against an isolated suite" do + with_isolated_suite do |suite| + out, _err, status = + blackoutd("auto", "off", + env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + expect(status.exitstatus).to eq(0) + expect(out).to match(/auto-blackout: disabled .*isolated test suite/) + end + end + end + + describe "recover" do + it "rejects an unknown method and lists the known ones" do + _out, err, status = blackoutd("recover", "--method", "bogus") + expect(status.exitstatus).to eq(1) + expect(err).to include("displaysleep") + expect(err).to include("extcycle") + expect(err).to include("fbpower") + end + + it "rejects an unknown option" do + _out, err, status = blackoutd("recover", "--nope") + expect(status.exitstatus).to eq(1) + expect(err).to include("unknown recover option") + end + + it "previews the displaysleep cycle under --dry-run" do + out, _err, status = + blackoutd("recover", "--method", "displaysleep", "--dry-run") + expect(status.exitstatus).to eq(0) + expect(out).to include("[dry-run]") + expect(out).to include("pmset displaysleepnow") + expect(out).to include("caffeinate") + end + + it "previews the extcycle sequence under --dry-run" do + out, _err, status = + blackoutd("recover", "--method", "extcycle", "--dry-run") + expect(status.exitstatus).to eq(0) + expect(out).to match(/extcycle: disable external.*re-enable/) + end + + it "previews the fbpower probe under --dry-run" do + out, _err, status = + blackoutd("recover", "--method", "fbpower", "--dry-run") + expect(status.exitstatus).to eq(0) + expect(out).to include("fbpower: open external-0") + end + end + + describe "repro argument validation" do + it "rejects an unknown trigger before sleeping" do + _out, err, status = blackoutd("repro", "--trigger", "bogus") + expect(status.exitstatus).to eq(1) + expect(err).to include("unknown trigger") + end + + it "rejects a non-alphanumeric group" do + _out, err, status = blackoutd("repro", "--group", "!!") + expect(status.exitstatus).to eq(1) + expect(err).to include("--group must be") + end + + it "rejects an empty --wake" do + _out, err, status = blackoutd("repro", "--wake", "") + expect(status.exitstatus).to eq(1) + expect(err).to include("non-negative integer") + end + + it "rejects a non-numeric --settle" do + _out, err, status = blackoutd("repro", "--settle", "abc") + expect(status.exitstatus).to eq(1) + expect(err).to include("non-negative integer") + end + + it "rejects an unknown option" do + _out, err, status = blackoutd("repro", "--frob") + expect(status.exitstatus).to eq(1) + expect(err).to include("unknown repro option") + end + end + + describe "repro --dry-run walkthrough" do + # The step lines (sayCue prints " [step] ") plus the sudo-prime + # and schedule notices, in emission order. + def dry_run_lines(*extra_args) + out, err, status = blackoutd("repro", "--dry-run", *extra_args) + expect(status.exitstatus).to eq(0) + "#{out}#{err}".lines.map(&:chomp) + end + + def index_of(lines, needle) + lines.index { |l| l.include?(needle) } + end + + it "orders the trigger BEFORE scheduling the wake (the W1 fix)" do + lines = dry_run_lines("--wake", "15", "--trigger", "extcycle", + "--recover", "displaysleep") + prime = index_of(lines, "priming sudo") + trigger = index_of(lines, "triggering external cycle") + schedule = index_of(lines, "scheduling wake") + sleep_now = index_of(lines, "[step] sleeping now") + awake = index_of(lines, "[step] awake") + capture = index_of(lines, "capturing post wake") + recover = index_of(lines, "[step] recovering") + [prime, trigger, schedule, sleep_now, awake, capture, + recover].each { |i| expect(i).not_to be_nil } + # prime < trigger < schedule < sleepnow < awake < capture < recover + expect(prime).to be < trigger + expect(trigger).to be < schedule + expect(schedule).to be < sleep_now + expect(sleep_now).to be < awake + expect(awake).to be < capture + expect(capture).to be < recover + end + + it "locks before scheduling the wake" do + lines = dry_run_lines("--wake", "15", "--lock") + lock = index_of(lines, "locking session") + schedule = index_of(lines, "scheduling wake") + expect(lock).not_to be_nil + expect(schedule).not_to be_nil + expect(lock).to be < schedule + end + + it "speaks an awake cue between sleep and capture" do + lines = dry_run_lines("--wake", "0") + sleep_now = index_of(lines, "[step] sleeping now") + awake = index_of(lines, "[step] awake") + capture = index_of(lines, "capturing post wake") + expect(awake).to be_between(sleep_now + 1, capture - 1) + end + + it "skips sudo entirely for a manual wake (--wake 0)" do + lines = dry_run_lines("--wake", "0") + expect(lines.any? { |l| l.include?("priming sudo") }).to be false + expect(lines.any? { |l| l.include?("scheduling wake") }).to be false + end + end + + describe "help lists the new subcommands and methods" do + it "documents recovery, recover methods, and repro flags in usage" do + _out, err, _status = blackoutd + expect(err).to include("recovery ") + expect(err).to include("extcycle") + expect(err).to include("fbpower") + expect(err).to include("--trigger") + expect(err).to include("--lock") + end + end +end From 4434b46c920d72f3e2eca978c8a26c6b5480d32a Mon Sep 17 00:00:00 2001 From: Todd Schulman Date: Mon, 20 Jul 2026 11:31:29 -0400 Subject: [PATCH 3/5] docs: add manual tests for recovery and repro Record the hardware- and daemon-dependent behaviors that the CLI spec cannot cover: auto-recovery firing (and not firing) at wake per the recoveryStrategy pref, manual recover methods (extcycle unlocked-only), and the repro harness (trigger, awake cue, --lock). Satisfies P20's "manual repro test added to spec/manual/TESTING.md" acceptance item. Co-Authored-By: Claude Opus 4.8 --- spec/manual/TESTING.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/spec/manual/TESTING.md b/spec/manual/TESTING.md index faed0fc..10ab422 100644 --- a/spec/manual/TESTING.md +++ b/spec/manual/TESTING.md @@ -55,6 +55,37 @@ These tests require a MacBook with an external display connected via USB-C. - [ ] Works on AC power - [ ] External does not go black ~30s after wake (the pre-fix failure mode) +## Cursor-on-black auto-recovery (P20/P29) + +The CLI argument surface for these commands is covered by +`spec/integration/cli_spec.rb`; the items below need real hardware, a +running daemon, and an eyewitness, so they stay manual. Use `blackoutd +repro` (see `docs/debug/cursor-on-black-matrix.md`) to provoke the black. + +- [ ] `blackoutd recovery displaysleep` reports the value and, with the + daemon running, notifies it (`status` then shows `recovery : + displaysleep`) +- [ ] `blackoutd recovery none` disables auto-recovery; `status` reflects it +- [ ] With `recovery displaysleep` and an external attached, a + cursor-on-black wake self-clears within a few seconds of settle + (daemon log shows `[wake] recovery=displaysleep marker=present`) +- [ ] With `recovery none`, the same wake stays black (marker logged, + no recovery issued) — the control case for data collection +- [ ] A clean wake logs `marker=absent` and does not cycle the display +- [ ] `blackoutd recover --method displaysleep` clears a black manually +- [ ] `blackoutd recover --method extcycle` clears a black in an + **unlocked** session (do NOT run while locked — induces blacks; + see the matrix group C notes) + +## repro harness (maintainer-run) + +- [ ] `blackoutd repro --wake 15 --trigger extcycle` provokes the black + and the scheduled wake fires (no manual wake needed) +- [ ] The spoken "awake" cue lands at first light; the run sheet and + bundles are written under `docs/debug/` +- [ ] `--lock` locks the session before sleep; the run stays locked + through capture (keep the Apple Watch out of range) + ## Build/Install Cycle - [ ] `make clean; make; make reinstall` succeeds From 7ee6b76cc1136e6bf9e770d3c7cf68505049c158 Mon Sep 17 00:00:00 2001 From: Todd Schulman Date: Mon, 20 Jul 2026 12:05:18 -0400 Subject: [PATCH 4/5] fix: scope and validate the defaults-isolation seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the isolation env var BLACKOUTD_DEFAULTS_SUITE -> BLACKOUTD_TEST_DEFAULTS_SUITE to name it for its purpose (the project's own test suite; not a supported interface), and validate it once at startup in initCliDefaults(): an override equal to the real "blackoutd" suite, or one that is not valid UTF-8, is now a hard error (exit 2) printed before any subcommand runs — so it cannot write real or global preferences while skipping the daemon signal, and the rejection paths are safe to exercise from the test harness. cliSuiteName() and cliDefaultsIsolated() read a single resolved result, so they can never disagree. Also corrects the seam comments to en_US spelling (signaling / signaled) per the project convention. Addresses PR #30 review (CodeRabbit): the =blackoutd and invalid-UTF-8 edge cases. Env unset ⇒ behavior identical to before. Verified: valid override isolates; =blackoutd and non-UTF-8 abort with exit 2 and no write; normal `status` unaffected. Co-Authored-By: Claude Opus 4.8 --- src/main.m | 72 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/src/main.m b/src/main.m index e1a546e..cf451f3 100644 --- a/src/main.m +++ b/src/main.m @@ -35,24 +35,58 @@ static NSString *const kRecoveryKey = @"recoveryStrategy"; static NSString *const kAgentLabel = @BD_BUNDLE_ID; -// Test-only isolation seam. When BLACKOUTD_DEFAULTS_SUITE is set to a -// non-empty value in the environment, the CLI preference helpers read and -// write that throwaway suite instead of the real "blackoutd" suite, and the -// write-back subcommands (auto / verbosity / recovery) skip signalling the -// daemon. This lets the RSpec CLI harness exercise the accept-paths without -// mutating the user's real preferences or perturbing the running daemon, -// which always reads the hardcoded kSuiteName — this seam is deliberately -// NOT wired into AppDelegate.m. Unset (normal operation) ⇒ identical to -// targeting kSuiteName directly with the daemon signalled as before. -static NSString *cliSuiteName(void) { - const char *override = getenv("BLACKOUTD_DEFAULTS_SUITE"); - return (override && override[0] != '\0') ? @(override) : kSuiteName; +// Test-only isolation seam. Its sole consumer is the project's own test +// suite; it is not a supported public interface. When +// BLACKOUTD_TEST_DEFAULTS_SUITE names a valid throwaway suite, the CLI +// preference helpers read and write that suite instead of the real +// "blackoutd" suite, and the write-back subcommands (auto / verbosity / +// recovery) skip signaling the daemon. This lets the CLI harness exercise +// the accept-paths without mutating the user's real preferences or +// perturbing the running daemon, which always reads the hardcoded +// kSuiteName — this seam is deliberately NOT wired into AppDelegate.m. +// Unset (normal operation) ⇒ identical to targeting kSuiteName directly +// with the daemon signaled as before. +static const char *const kTestSuiteEnv = "BLACKOUTD_TEST_DEFAULTS_SUITE"; + +// Resolved once by initCliDefaults() at startup, then read by the helpers. +static NSString *gCliSuite = nil; +static BOOL gCliDefaultsIsolated = NO; + +// Validates the test-suite env var and sets the file-statics above. Returns +// 0 for normal operation and for a valid isolation suite. When the var is +// present but invalid — not decodable as UTF-8, or equal to the real suite +// (which would write real/global preferences while skipping the daemon +// signal, the opposite of isolation) — prints an error and returns a +// non-zero exit code so main() aborts BEFORE any subcommand writes defaults +// or signals the daemon. A test can therefore exercise the rejection paths +// with no real-state side effects. +static int initCliDefaults(void) { + const char *raw = getenv(kTestSuiteEnv); + if (raw == NULL || raw[0] == '\0') { + gCliSuite = kSuiteName; + gCliDefaultsIsolated = NO; + return 0; + } + NSString *name = [NSString stringWithUTF8String:raw]; + if (name == nil) { + fprintf(stderr, "blackoutd: %s is not valid UTF-8; refusing to run\n", + kTestSuiteEnv); + return 2; + } + if ([name isEqualToString:kSuiteName]) { + fprintf(stderr, + "blackoutd: %s must not be the real suite '%s'; refusing to run\n", + kTestSuiteEnv, kSuiteName.UTF8String); + return 2; + } + gCliSuite = name; + gCliDefaultsIsolated = YES; + return 0; } -static BOOL cliDefaultsIsolated(void) { - const char *override = getenv("BLACKOUTD_DEFAULTS_SUITE"); - return override != NULL && override[0] != '\0'; -} +static NSString *cliSuiteName(void) { return gCliSuite ?: kSuiteName; } + +static BOOL cliDefaultsIsolated(void) { return gCliDefaultsIsolated; } static NSString *agentPlistPath(void) { return [NSHomeDirectory() @@ -2232,6 +2266,12 @@ int main(int argc, const char *argv[]) { setvbuf(stderr, NULL, _IONBF, 0); @autoreleasepool { + // Resolve the CLI defaults suite before any subcommand runs; an invalid + // test-suite override aborts here, before any write or daemon signal. + int suiteRC = initCliDefaults(); + if (suiteRC != 0) + return suiteRC; + if (argc < 2) { printUsage(); return 1; From cc0c8ea61e2d653670e578f4f57857d7168fa899 Mon Sep 17 00:00:00 2001 From: Todd Schulman Date: Mon, 20 Jul 2026 12:05:44 -0400 Subject: [PATCH 5/5] test: stabilize real-suite check; add guard cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the CI failure in "leaves the real blackoutd suite untouched": compare `defaults read` stdout only. An absent key (the CI case) sends a timestamped `defaults[pid]` diagnostic to stderr that differs between the before/after calls; stdout is the value or empty and is stable. The local machine had the key set, hiding the flake. Rename the isolation env var to BLACKOUTD_TEST_DEFAULTS_SUITE and add a "test-suite isolation guard" group: the real-suite name and a non-UTF-8 value each abort with exit 2 and leave the real suite untouched — now testable because the seam errors before any write rather than falling back to the real suite. 34 examples, 0 failures. Co-Authored-By: Claude Opus 4.8 --- spec/integration/cli_spec.rb | 61 ++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/spec/integration/cli_spec.rb b/spec/integration/cli_spec.rb index f57961b..34f49ea 100644 --- a/spec/integration/cli_spec.rb +++ b/spec/integration/cli_spec.rb @@ -13,10 +13,12 @@ # # The write-back subcommands (auto / verbosity / recovery) normally mutate # the real "blackoutd" NSUserDefaults suite and SIGHUP the running daemon. -# The accept-path examples here set BLACKOUTD_DEFAULTS_SUITE to a throwaway -# suite (the src/main.m test seam): writes divert to that suite and the -# daemon is not signalled, so these tests never touch the user's real -# preferences or the running daemon. +# The accept-path examples here set BLACKOUTD_TEST_DEFAULTS_SUITE to a +# throwaway suite (the src/main.m test seam): writes divert to that suite and +# the daemon is not signaled, so these tests never touch the user's real +# preferences or the running daemon. An invalid value for that var (the real +# suite name, or non-UTF-8) is a hard error before any write — exercised by +# the "test-suite isolation guard" examples. # # The macOS-only ObjC binary is built once in before(:all); the RSpec CI job # runs on macos-latest but does not build, so the suite builds it itself. @@ -43,10 +45,12 @@ def with_isolated_suite system("defaults", "delete", suite, out: File::NULL, err: File::NULL) end - # Raw `defaults read` output for a key in the real suite (value line or the - # "does not exist" message), used to assert the real suite is untouched. + # The value of a key in the real suite, or "" when absent. stdout only: + # an absent key sends a timestamped `defaults[pid]` diagnostic to stderr + # (differs between calls), while stdout is the value or empty — the stable + # thing to compare when asserting the real suite is untouched. def real_default(key) - out, = Open3.capture2e("defaults", "read", "blackoutd", key) + out, = Open3.capture3("defaults", "read", "blackoutd", key) out.force_encoding("UTF-8") end end @@ -110,11 +114,11 @@ def real_default(key) expect(status.exitstatus).to eq(1) end - it "persists a valid level to an isolated suite without signalling" do + it "persists a valid level to an isolated suite without signaling" do with_isolated_suite do |suite| out, _err, status = blackoutd("verbosity", "2", - env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => suite }) expect(status.exitstatus).to eq(0) expect(out).to match(/verbosity: 2 .*isolated test suite/) end @@ -138,7 +142,7 @@ def real_default(key) with_isolated_suite do |suite| out, _err, status = blackoutd("recovery", "displaysleep", - env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => suite }) expect(status.exitstatus).to eq(0) expect(out).to match(/recovery: displaysleep .*isolated test suite/) end @@ -148,7 +152,7 @@ def real_default(key) with_isolated_suite do |suite| out, _err, status = blackoutd("recovery", "none", - env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => suite }) expect(status.exitstatus).to eq(0) expect(out).to match(/recovery: none .*isolated test suite/) end @@ -157,7 +161,7 @@ def real_default(key) it "leaves the real blackoutd suite untouched when isolated" do before_val = real_default("recoveryStrategy") with_isolated_suite do |suite| - env = { "BLACKOUTD_DEFAULTS_SUITE" => suite } + env = { "BLACKOUTD_TEST_DEFAULTS_SUITE" => suite } blackoutd("recovery", "none", env: env) blackoutd("recovery", "displaysleep", env: env) end @@ -182,13 +186,44 @@ def real_default(key) with_isolated_suite do |suite| out, _err, status = blackoutd("auto", "off", - env: { "BLACKOUTD_DEFAULTS_SUITE" => suite }) + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => suite }) expect(status.exitstatus).to eq(0) expect(out).to match(/auto-blackout: disabled .*isolated test suite/) end end end + # The seam must fail loudly on an invalid isolation value rather than + # silently falling back to the real suite (which would write real + # preferences while skipping the daemon signal). Because the guard aborts + # before any subcommand runs, these rejection paths are safe to automate. + describe "test-suite isolation guard" do + it "refuses to run when the test suite names the real suite" do + out, err, status = + blackoutd("recovery", "displaysleep", + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => "blackoutd" }) + expect(status.exitstatus).to eq(2) + expect(err).to include("must not be the real suite") + expect(out).not_to include("isolated test suite") + end + + it "refuses to run on a non-UTF-8 test suite" do + out, err, status = + blackoutd("recovery", "none", + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => "\xFF\xFE".b }) + expect(status.exitstatus).to eq(2) + expect(err).to include("not valid UTF-8") + expect(out).not_to include("isolated test suite") + end + + it "does not touch the real suite when it rejects the value" do + before_val = real_default("recoveryStrategy") + blackoutd("recovery", "none", + env: { "BLACKOUTD_TEST_DEFAULTS_SUITE" => "blackoutd" }) + expect(real_default("recoveryStrategy")).to eq(before_val) + end + end + describe "recover" do it "rejects an unknown method and lists the known ones" do _out, err, status = blackoutd("recover", "--method", "bogus")