From 55e454aedc330a661d3a15878bdd503fbc4a6dc5 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 7 Sep 2026 07:51:41 +0200 Subject: [PATCH] fix(headless): S1 ACDREAM_HEADLESS_CONSOLE=0 disables even on a real terminal HeadlessConsoleOptions.Resolve tested the environment variable against the literal "1", so ACDREAM_HEADLESS_CONSOLE=0 silently fell through to the terminal-shaped default (on when stdin is a real console) instead of acting as an A/B off-switch. Now: once the variable is SET AT ALL, any value other than "0" enables and "0" disables -- the same idiom ACDREAM_RETAIL_CLOSE_DEGRADES / ACDREAM_RETAIL_UI already use. An unset variable still falls through to the terminal default. Registered the flag as the sixth entry in LaunchOptionsDocumentationTests.DefaultOnBehaviorFlags and updated the Conventions section of docs/launch-options.md plus the flag's own row (side-effects column corrected to describe the real precedence). ResolvePrefersFlagThenEnvironmentThenTerminalDefault's env="0"/terminal =true case was shown to fail against the prior `== "1"` implementation (expected false, old code returned true) before the fix landed; the env="yes" case also failed on the same mutation (old code required the literal "1", so "yes" fell through to terminal=false instead of enabling). Co-Authored-By: Claude Fable 5.1 --- docs/launch-options.md | 10 ++++--- .../Configuration/HeadlessConsoleOptions.cs | 27 ++++++++++++++----- .../LaunchOptionsDocumentationTests.cs | 15 +++++++---- .../HeadlessConsoleTests.cs | 14 +++++++--- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/docs/launch-options.md b/docs/launch-options.md index f2b1630c..73b6e188 100644 --- a/docs/launch-options.md +++ b/docs/launch-options.md @@ -40,12 +40,16 @@ Assume a flag has a side effect until its row says otherwise. - **Everything diagnostic is OFF by default.** Every probe, dump, capture, and measurement flag in this document is inert until its variable is explicitly set — an unset environment runs zero diagnostics. Exactly - five flags default ON, and none is a diagnostic: `ACDREAM_RETAIL_CHASE`, + six flags default ON, and none is a diagnostic: `ACDREAM_RETAIL_CHASE`, `ACDREAM_CAMERA_COLLIDE`, `ACDREAM_CAMERA_ALIGN_SLOPE`, and `ACDREAM_RETAIL_CLOSE_DEGRADES` are retail *behaviors* wearing an A/B off-switch (`=0` disables the behavior for a comparison run), while `ACDREAM_RETAIL_UI` is the product's only gameplay presentation and uses - the same explicit diagnostic opt-out. That five-flag set is frozen by + the same explicit diagnostic opt-out. `ACDREAM_HEADLESS_CONSOLE` is the + sixth: its unset default is terminal-shaped (on when stdin is a real + console, off when redirected — not unconditionally on like the other + five), but once the variable is SET AT ALL it uses the identical `=0` + override (any other value enables). That six-flag set is frozen by `LaunchOptionsDocumentationTests` — a new default-on flag fails the build. - `=1` means the code tests for exactly the string `1`. Setting `true`, @@ -93,7 +97,7 @@ dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Release | `ACDREAM_DAT_DIR` | `=` | Fallback dat-directory when no positional argument is given. App: single read at `Program.cs:58`. Cli: read independently per-subcommand (each subcommand does `args.ElementAtOrDefault(N) ?? Env.GetEnvironmentVariable("ACDREAM_DAT_DIR")`) plus once more for the default (no-subcommand) asset-inventory mode at line 152. | Two of the four `Program.cs` line numbers in the raw grep (91, 135) are **not reads** — they're the literal string `ACDREAM_DAT_DIR` inside `Log.Error` usage-text messages, not `GetEnvironmentVariable` calls. Only line 58 is a real read in `AcDream.App`. | none — hard usage error (exit 2) if unset and no positional arg | `Program.cs:58` (App); `Cli/Program.cs:24,35,47,59,71,84,113,125,137,152` (every Cli subcommand) | | `ACDREAM_DISPLAY_PROTOCOL` | `="auto"` / `"x11"` / `"wayland"` (case-insensitive, trimmed); any other value throws `InvalidOperationException` at startup | Linux-only: forces the GLFW 3.4 platform-init hint (X11 vs Wayland vs auto) before any window is created; ignored entirely on Windows (always `Windows` protocol) | An invalid value is fatal at startup (throws before any window exists), not a silent fallback | unset → auto-detected from `XDG_SESSION_TYPE`/`WAYLAND_DISPLAY`/`DISPLAY`, falling back to GLFW `Automatic` | `GraphicalWindowBackendSelection.Resolve` (`GraphicalWindowBackendSelection.cs:26-58`) | | `ACDREAM_FAR_RADIUS` | `=` | Overrides preset's `FarRadius` (outer streaming/reveal window, landblocks) | Enlarging changes streaming memory budget and what's resident/rendered — CLAUDE.md: leave unset for measurement/gate runs (same family as legacy `ACDREAM_STREAM_RADIUS`) | preset's `FarRadius` (Low=5, Medium=8, High=12, Ultra=15) | `QualitySettings.WithEnvOverrides` (`QualityPreset.cs:47`) | -| `ACDREAM_HEADLESS_CONSOLE` | `=1` enables | Turns on the headless host's interactive console (docs/plans/2026-09-07-headless-console.md): a background thread reads stdin lines, each drained on the session tick through the SAME plugin-verb/client-slash-command pipeline the graphical chat box uses, with chat/lifecycle/portal output rendered to stdout. Only takes effect for `run` with a single configured session — the launcher's multi-session mode is unaffected. | Starts a background stdin-reader thread and writes plain-text lines to the same stdout stream `HeadlessDiagnosticWriter` already uses for its JSON lines — the two interleave. Only applies to `run`; `--console` (bare flag, no value) always wins over this variable, which in turn always wins over the terminal-shaped default. | unset → on when stdin is a real console, off when redirected (`!Console.IsInputRedirected`, checked once in `Program.cs`) | `HeadlessConsoleOptions.Resolve` (`Configuration/HeadlessConsoleOptions.cs`) → `HeadlessEntryPoint.Run` → `HeadlessProcessHost`'s `consoleEnabled` | +| `ACDREAM_HEADLESS_CONSOLE` | `=0` disables (once set at all); any other value enables; unset falls through to the terminal-shaped default | Turns on the headless host's interactive console (docs/plans/2026-09-07-headless-console.md): a background thread reads stdin lines, each drained on the session tick through the SAME plugin-verb/client-slash-command pipeline the graphical chat box uses, with chat/lifecycle/portal output rendered to stdout. Only takes effect for `run` with a single configured session — a multi-session process reports `console: single-session only` via the diagnostics stream and does not attach one. | Starts a background stdin-reader thread and writes plain-text lines to the same stdout stream `HeadlessDiagnosticWriter` already uses for its JSON lines — the two interleave. Only applies to `run`; `--console` (bare flag, no value) always wins over this variable. S1 fix (2026-09-07): the variable itself now wins outright once SET AT ALL — `=0` disables even when stdin is a real terminal, matching every other `=0`-disables flag in this table; only an UNSET variable falls through to the terminal-shaped default. | unset → on when stdin is a real console, off when redirected (`!Console.IsInputRedirected`, checked once in `Program.cs`); set → `!= "0"` | `HeadlessConsoleOptions.Resolve` (`Configuration/HeadlessConsoleOptions.cs`) → `HeadlessEntryPoint.Run` → `HeadlessProcessHost`'s `consoleEnabled` | | `ACDREAM_LIVE` | `=1` (exactly the literal string `"1"`) | Core switch: connect to a live ACE server instead of running offline/no-connect. | The 4 non-`RuntimeOptions.cs` line numbers in the raw grep are **all comments or log-message text**, not reads — `SessionStartComposition.cs:39` is inside the string `"live: ACDREAM_LIVE set but TEST_USER/TEST_PASS missing; skipping"`; `Program.cs:126` is inside a `--session-config` override log line; `GameWindow.cs:614,627` are doc comments. The only actual parse is `RuntimeOptions.cs:141`. Requires `ACDREAM_TEST_USER`/`ACDREAM_TEST_PASS` too (`HasLiveCredentials`) or the session silently reports `MissingCredentials` and skips. Forced to effectively-on (LiveMode=true) unconditionally by `--session-config` launches regardless of this var. | `false` | `RuntimeOptions.LiveMode` → `SessionStartComposition.cs` (log text only), `Program.cs:126` (log text only), `GameWindow.cs:614,627` (comments only), consumed for real via `RuntimeOptions.HasLiveCredentials` and `WorldSession`/`GameRuntime` session-start gating | | `ACDREAM_MAX_COMPLETIONS_PER_FRAME` | `=` | Overrides preset's per-frame streaming-completion throughput cap | Directly changes the streaming admission budget measured by perf/completion gates — do not vary during a measurement run | preset's value (Low=2, Medium=3, High=4, Ultra=6) | `QualitySettings.WithEnvOverrides` (`QualityPreset.cs:59`) | | `ACDREAM_MSAA_SAMPLES` | `=` (0/2/4/8) | Overrides preset's MSAA sample count | Changes GPU multisample anti-aliasing (visual + GPU-cost change) | preset's `MsaaSamples` (Low=0, Medium=2, High/Ultra=4) | `QualitySettings.WithEnvOverrides` (`QualityPreset.cs:48`) | diff --git a/src/AcDream.Headless/Configuration/HeadlessConsoleOptions.cs b/src/AcDream.Headless/Configuration/HeadlessConsoleOptions.cs index fd1aed15..88239404 100644 --- a/src/AcDream.Headless/Configuration/HeadlessConsoleOptions.cs +++ b/src/AcDream.Headless/Configuration/HeadlessConsoleOptions.cs @@ -4,13 +4,24 @@ namespace AcDream.Headless.Configuration; /// Typed resolution for the headless interactive console (docs/plans/ /// 2026-09-07-headless-console.md). Three inputs, first match wins: /// the --console command-line flag, the -/// ACDREAM_HEADLESS_CONSOLE=1 environment variable, and finally a +/// ACDREAM_HEADLESS_CONSOLE environment variable, and finally a /// terminal-shaped default — on when stdin is a real console (an operator /// typing at a keyboard), off when it is redirected (a script, CI runner, or /// piped fixture, where a background reader thread blocked on /// ReadLine would never see input and would just sit idle). See /// docs/launch-options.md for the documented row this owns. /// +/// +/// S1 fix (2026-09-07 review round): the environment variable is a +/// default-on override once it is SET at all, not a bare "equals 1" test — +/// ACDREAM_HEADLESS_CONSOLE=0 must disable the console even when +/// stdin is a real terminal, matching the +/// ACDREAM_RETAIL_CLOSE_DEGRADES / ACDREAM_RETAIL_UI +/// convention (any value other than the literal string "0" enables). +/// An UNSET variable still falls through to the terminal-shaped default — +/// this flag's "default on" is conditional on stdin, unlike those two, but +/// once set at all it behaves identically. +/// internal static class HeadlessConsoleOptions { internal const string EnvironmentVariable = "ACDREAM_HEADLESS_CONSOLE"; @@ -25,14 +36,18 @@ internal static class HeadlessConsoleOptions internal static bool Resolve( bool commandLineFlag, - Func environment, + Func env, bool standardInputIsTerminal) { - ArgumentNullException.ThrowIfNull(environment); + ArgumentNullException.ThrowIfNull(env); if (commandLineFlag) return true; - if (environment(EnvironmentVariable) == "1") - return true; - return standardInputIsTerminal; + if (env(EnvironmentVariable) is null) + return standardInputIsTerminal; + // Default-on once the flag is set at all: any value other than the + // literal string "0" enables the console — the same + // ACDREAM_RETAIL_CLOSE_DEGRADES / ACDREAM_RETAIL_UI idiom. + return !string.Equals( + env("ACDREAM_HEADLESS_CONSOLE"), "0", StringComparison.Ordinal); } } diff --git a/tests/AcDream.App.Tests/Diagnostics/LaunchOptionsDocumentationTests.cs b/tests/AcDream.App.Tests/Diagnostics/LaunchOptionsDocumentationTests.cs index 4281ebe5..460913cb 100644 --- a/tests/AcDream.App.Tests/Diagnostics/LaunchOptionsDocumentationTests.cs +++ b/tests/AcDream.App.Tests/Diagnostics/LaunchOptionsDocumentationTests.cs @@ -107,10 +107,14 @@ public sealed class LaunchOptionsDocumentationTests } /// - /// The five flags that default ON. All are product/retail behaviors wearing an - /// A/B off-switch (=0 disables) — none is a diagnostic. FROZEN: - /// a diagnostic that activates without its env var set taxes every run - /// and every measurement silently, so growing this set fails. + /// The six flags that default ON. All are product/retail behaviors wearing an + /// A/B off-switch (=0 disables) — none is a diagnostic. + /// ACDREAM_HEADLESS_CONSOLE (added 2026-09-07, S1 fix round) is the + /// odd one out: its UNSET default is terminal-shaped, not unconditionally + /// on — but once it is SET AT ALL it reads the identical =0-disables + /// idiom this regex detects, so it belongs in this set on the same terms. + /// FROZEN: a diagnostic that activates without its env var set taxes every + /// run and every measurement silently, so growing this set fails. /// private static readonly IReadOnlySet DefaultOnBehaviorFlags = new HashSet(StringComparer.Ordinal) @@ -120,6 +124,7 @@ public sealed class LaunchOptionsDocumentationTests "ACDREAM_CAMERA_ALIGN_SLOPE", "ACDREAM_RETAIL_CLOSE_DEGRADES", "ACDREAM_RETAIL_UI", + "ACDREAM_HEADLESS_CONSOLE", }; /// @@ -134,7 +139,7 @@ public sealed class LaunchOptionsDocumentationTests RegexOptions.Compiled); [Fact] - public void OnlyTheFiveProductBehaviorFlagsDefaultOn() + public void OnlyTheSixProductBehaviorFlagsDefaultOn() { var defaultOn = new HashSet(StringComparer.Ordinal); foreach ((string path, _) in SourceFiles()) diff --git a/tests/AcDream.Headless.Tests/HeadlessConsoleTests.cs b/tests/AcDream.Headless.Tests/HeadlessConsoleTests.cs index edb722f7..a29218c6 100644 --- a/tests/AcDream.Headless.Tests/HeadlessConsoleTests.cs +++ b/tests/AcDream.Headless.Tests/HeadlessConsoleTests.cs @@ -31,10 +31,16 @@ public sealed class HeadlessConsoleTests // ── HeadlessConsoleOptions (typed option resolution) ───────────────── [Theory] - [InlineData(true, "0", false, true)] // CLI flag always wins - [InlineData(false, "1", false, true)] // env var wins over terminal default - [InlineData(false, "0", true, true)] // env var "0" does not disable the terminal default - [InlineData(false, null, true, true)] // no flag/env -> terminal-shaped default (on) + [InlineData(true, "0", false, true)] // CLI flag always wins, even over env "0" + [InlineData(false, "1", false, true)] // env var "1" wins over terminal default + [InlineData(false, "yes", false, true)] // any non-"0" env value enables (RETAIL_CLOSE_DEGRADES/RETAIL_UI idiom) + // S1 fix (2026-09-07 review round): ACDREAM_HEADLESS_CONSOLE=0 must + // disable the console even when stdin IS a real terminal — the earlier + // `== "1"` test let "0" silently fall through to the terminal-shaped + // default instead of acting as the documented A/B off-switch. + [InlineData(false, "0", true, false)] // env var "0" disables even when stdin is a terminal + [InlineData(false, "0", false, false)] // env var "0" disables when stdin is redirected too + [InlineData(false, null, true, true)] // no flag/env -> terminal-shaped default (on) [InlineData(false, null, false, false)] // no flag/env -> terminal-shaped default (off) public void ResolvePrefersFlagThenEnvironmentThenTerminalDefault( bool commandLineFlag,