Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
61 changes: 61 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,67 @@ The `qrc://` handler did `arg.split("login_successful.html?").collect::<Vec<&str

---

## Open issues being investigated

### v0.2.1 — nothing appears on TF2 launch from Steam (UI/TUI/CLI all invisible)

**Reported 2026-05-14.** User on v0.2.1 (latest, with `AllocConsole` fix in `maxima-cli` and full UI/TUI in installer). When launching TF2 from Steam, no Maxima window appears at all — not the CLI console, not the UI, not the TUI. In earlier versions the CLI window at least popped up. Diagnostics from the user are still pending.

**Code audit conducted 2026-05-14. Findings, in priority order (each independently capable of producing the reported symptom):**

#### Finding 1 — NSIS `SetRegView` chaos (most likely root cause)

[installer/maxima-setup.nsi:173](installer/maxima-setup.nsi:173) sets `SetRegView 64` for the `HKLM\SOFTWARE\WOW6432Node\Origin` writes and **never resets it** before the HKCR protocol writes at lines ~186-201 or the `BackupProtocol` calls at lines ~176-178. Consequences:

- 32-bit Wine consumers (TF2, Origin, anything emitting `link2ea://` from a 32-bit process) resolve HKCR through the **32-bit view** (`HKLM\Software\Classes\Wow6432Node\...`). They never see the entries v0.2.1 wrote under the 64-bit view.
- `BackupProtocol` (lines 58-79) and `RestoreProtocol` (lines 83-107) inherit whatever view leaked in from the caller — i.e. they back up the 64-bit view and miss any 32-bit-view registrations entirely. On a v0.2.0→v0.2.1 upgrade this means the v0.2.0 entries (written under whatever view the old `.nsi` used — most likely 32-bit by default for a 32-bit NSIS installer) are never read, never overwritten, never even noticed.
- Possible end state after upgrade: v0.2.1's "Maxima" entries are in 64-bit view; TF2 still resolves via 32-bit view and finds v0.2.0's stale entries (or nothing at all if the old uninstaller fired). Either way the dispatch is unpredictable and on at least some setups Wine ends up with no working handler.

**Fix:** In [installer/maxima-setup.nsi](installer/maxima-setup.nsi):
- Add `SetRegView 32` (or `SetRegView default`) immediately before line 186 (HKCR writes) and before line 176-178 (`BackupProtocol` calls).
- Inside both `BackupProtocol` and `RestoreProtocol` macros, set the view explicitly at entry — ideally back up *both* views and document the choice.
- In `BackupProtocol`, also guard against backing up Maxima's own values: read the existing `\shell\open\command` first, and if it points at `maxima-bootstrap.exe`, set `_existed=0` (treat as "no prior handler") so the uninstaller will simply delete the keys instead of restoring stale Maxima paths.

#### Finding 2 — `AllocConsole()` does not reattach stdio

[maxima-cli/src/main.rs:140-150](maxima-cli/src/main.rs:140) calls `AllocConsole()` but never reattaches `STD_OUTPUT_HANDLE` / `STD_ERROR_HANDLE` / Rust's stdio FDs. When bootstrap (GUI subsystem) spawns maxima-cli, the child inherits invalid stdio handles. `AllocConsole` creates a window but does NOT redirect existing handles — meaning the console pops up but `println!` and the `log.rs` writer at line 89 write to dead pipes. User sees a blank/silent console, or (depending on timing) no console at all.

**Fix:** After `AllocConsole()` succeeds, do `SetStdHandle(STD_OUTPUT_HANDLE, CreateFileW(L"CONOUT$", GENERIC_READ|GENERIC_WRITE, ...))` for stdout and stderr, and call `freopen("CONOUT$", "w", ...)` equivalents for the C runtime (Rust's stdout/stderr are buffered on top of these FDs). Without this, AllocConsole is decorative only.

#### Finding 3 — `#[tokio::main]` runs before `ensure_console_attached`

[maxima-cli/src/main.rs:155](maxima-cli/src/main.rs:155): `#[tokio::main]` desugars to runtime construction *before* user code. Any panic in IOCP/thread-pool init under Wine kills the process before AllocConsole. Same risk applies to `init_logger_named`, which runs after `Args::parse()` at line 246.

**Fix:** Convert `main` to a plain `fn main()` that (1) calls `ensure_console_attached()`, (2) installs a panic hook that writes to `%LOCALAPPDATA%\Maxima\Logs\maxima-cli.panic.log`, (3) calls `init_logger_named()`, (4) parses args, (5) builds the tokio runtime manually with `tokio::runtime::Runtime::new()` and `block_on(startup())`. This guarantees console + file logger exist before *anything* fallible runs.

#### Finding 4 — `Args::parse()` before logger init

If clap exits because of a malformed argv (or because the `MAXIMA_LAUNCH_ARGS` env var is mis-encoded), the error message goes to stderr — which is the unattached pipe. User sees nothing, no log line on disk. Init the logger first, then parse args.

#### Finding 5 — Bootstrap swallows non-zero exit codes

[maxima-bootstrap/src/main.rs:271](maxima-bootstrap/src/main.rs:271): `child.spawn()?.wait().await?` only propagates an error on `wait()` itself; a non-zero exit from maxima-cli returns `Ok(ExitStatus)` and bootstrap logs "Result: Success". If maxima-cli crashes or auth fails, bootstrap pretends everything worked. Not the root cause but blinds diagnostics — every other finding becomes invisible because we can't tell whether maxima-cli even ran.

**Fix:** After `wait().await?`, check `status.success()` and log the exit code to `maxima_execution.log` if non-zero.

#### Ruled out

- `is_valid_ea_offer_id("Origin.OFR.50.0002694")` returns `true` — leading zeros are fine.
- `url::Url::parse("link2ea://launchgame/Origin.OFR.50.0002694?...").path_segments()` correctly yields `["Origin.OFR.50.0002694"]` (the action is the host, not a path segment). `segments[0]` is the offer id, as intended.
- `LOG_FILE` mutex deadlock — not a realistic failure mode; lock scope is short and not reentrant.
- `winapi` features `consoleapi` / `wincon` are correctly enabled in [maxima-cli/Cargo.toml:22](maxima-cli/Cargo.toml:22).

#### Recommended order of work when picking this up

1. Ship Finding 5 (bootstrap exit-code logging) first — it's a one-liner and makes every subsequent change debuggable.
2. Then Finding 1 (NSIS `SetRegView` reset + Maxima-ownership guard in `BackupProtocol`). High likelihood of being the user-facing root cause.
3. Then Findings 2+3+4 together (rewrite the prologue of `maxima-cli/main.rs`: plain main, panic hook, logger before args, manual tokio runtime, stdio reattach after AllocConsole).
4. Have the user share the contents of `maxima_execution.log` and `%LOCALAPPDATA%\Maxima\Logs\maxima-cli.log` after the fixes ship — those should now be populated and tell us whether the symptom is gone or whether a new layer is exposed.

Do not delete this section until the user confirms TF2 launches with Maxima visible again.

---

## Known remaining gaps

- **`maxima-tui` / `maxima-ui`**: The UI crates exist and compile but are not wired into the Draconis flow at all. They are upstream components that may be useful in a future Draconis "standalone mode" but need significant work to be production-ready.
Expand Down
73 changes: 69 additions & 4 deletions installer/maxima-setup.nsi
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,31 @@
; Captures the three relevant fields: the default value, "URL Protocol", and
; shell\open\command's default value. Anything else (e.g. DefaultIcon) is not
; round-tripped, but EA Launcher's protocol entries don't rely on extras.
;
; Upgrade guard: if Maxima is already installed (detected via the
; HKLM\Software\Maxima\InstallPath reg key the prior install wrote), this
; is an upgrade. We skip the backup phase entirely so that:
; - we don't overwrite a real EA-handler backup with Maxima's own values,
; - we don't write a brand-new backup that points at a Maxima binary
; (which the uninstaller would then "restore" to a deleted path).
; The first install captures the pre-Maxima state; subsequent upgrades
; leave that backup untouched.
;
; View safety: SetRegView default before HKCR reads so this macro is
; independent of caller state (the install section sets view 64 for HKLM
; ops, which would otherwise leak into HKCR).
;
; Usage: !insertmacro BackupProtocol "qrc"
!macro BackupProtocol PROTOCOL
SetRegView default

ClearErrors
ReadRegStr $9 HKLM "Software\Maxima" "InstallPath"
${IfNot} ${Errors}
DetailPrint "BackupProtocol(${PROTOCOL}): upgrade detected, preserving original backup"
Goto skip_backup_${PROTOCOL}
${EndIf}

ClearErrors
ReadRegStr $0 HKCR "${PROTOCOL}" ""
${If} ${Errors}
Expand All @@ -76,11 +99,30 @@
WriteRegStr HKLM "${BACKUP_ROOT}\Protocol_${PROTOCOL}" "command" "$0"
${EndIf}
${EndIf}
skip_backup_${PROTOCOL}:
!macroend

; Upgrade-safe wrapper around BackupValue with the same guard as
; BackupProtocol. Use for HKLM values that must be round-tripped on
; uninstall (Origin\ClientPath, EA Desktop\InstallSuccessful).
; Usage: !insertmacro BackupValueUpgradeSafe HKLM "Key" "Name" "BackupId"
!macro BackupValueUpgradeSafe ROOT KEY VALUENAME BACKUPID
ClearErrors
ReadRegStr $9 HKLM "Software\Maxima" "InstallPath"
${If} ${Errors}
!insertmacro BackupValue ${ROOT} "${KEY}" "${VALUENAME}" "${BACKUPID}"
${Else}
DetailPrint "BackupValueUpgradeSafe(${BACKUPID}): upgrade detected, preserving original backup"
${EndIf}
!macroend

; Restore (or delete) a URL-protocol handler subtree at uninstall time.
; Forces view default so HKCR reads/writes match the install-time store
; (BackupProtocol also runs under view default - they must agree or the
; uninstaller "restores" nothing and leaves dangling Maxima keys behind).
; Usage: !insertmacro RestoreProtocol "qrc"
!macro RestoreProtocol PROTOCOL
SetRegView default
ClearErrors
ReadRegStr $0 HKLM "${BACKUP_ROOT}\Protocol_${PROTOCOL}" "_existed"
${If} $0 == "1"
Expand Down Expand Up @@ -114,7 +156,7 @@
!define PRODUCT_UNINST_KEY "Software\Microsoft\Windows\CurrentVersion\Uninstall\${PRODUCT_NAME}"
!define PRODUCT_UNINST_ROOT_KEY "HKLM"

; Binary directory override at compile time with /DBIN_DIR="..."
; Binary directory - override at compile time with /DBIN_DIR="..."
; Default is the macOS cross-compilation path (installer/build.sh).
; CI Windows native builds pass /DBIN_DIR="..\target\release"
!ifndef BIN_DIR
Expand Down Expand Up @@ -170,20 +212,40 @@ Section "Maxima Core" SEC_CORE

; Back up any pre-existing values BEFORE we overwrite them, so the
; uninstaller can restore the user's previous EA Launcher / Origin setup.
;
; HKLM\SOFTWARE\WOW6432Node\* and HKLM\SOFTWARE\Electronic Arts\* are
; backed up under the 64-bit view so the values match where EA Desktop
; / Origin writes them on a real 64-bit Windows install.
SetRegView 64
!insertmacro BackupValue HKLM "SOFTWARE\WOW6432Node\Origin" "ClientPath" "Origin_ClientPath"
!insertmacro BackupValue HKLM "SOFTWARE\Electronic Arts\EA Desktop" "InstallSuccessful" "EADesktop_InstallSuccessful"
!insertmacro BackupValueUpgradeSafe HKLM "SOFTWARE\WOW6432Node\Origin" "ClientPath" "Origin_ClientPath"
!insertmacro BackupValueUpgradeSafe HKLM "SOFTWARE\Electronic Arts\EA Desktop" "InstallSuccessful" "EADesktop_InstallSuccessful"

; HKCR protocol handlers are SHARED keys on Windows (not WOW64-redirected
; for URL protocol associations), but NSIS still resolves HKCR through
; whichever view is currently active. Force back to default so the
; backups and the subsequent writes target the same store the OS
; serves to both 32-bit and 64-bit URL-resolving processes.
;
; This is the critical fix for the v0.2.0 -> v0.2.1 upgrade regression
; where view-64 leaked into HKCR writes and left 32-bit consumers
; (Titanfall2.exe, Origin emitting link2ea://) looking at stale or
; missing handlers.
SetRegView default
!insertmacro BackupProtocol "qrc"
!insertmacro BackupProtocol "link2ea"
!insertmacro BackupProtocol "origin2"

; Origin compatibility: Point EA games to maxima-bootstrap.exe
; (back under view 64 - same store as the BackupValue above).
SetRegView 64
WriteRegStr HKLM "SOFTWARE\WOW6432Node\Origin" "ClientPath" "$INSTDIR\maxima-bootstrap.exe"

; EA Desktop flag
WriteRegStr HKLM "SOFTWARE\Electronic Arts\EA Desktop" "InstallSuccessful" "true"

; ---- Protocol Handlers ----
; Reset view for HKCR writes - see comment above the BackupProtocol calls.
SetRegView default

; qrc:// protocol (EA login redirection)
WriteRegStr HKCR "qrc" "" "URL:Maxima Protocol"
Expand Down Expand Up @@ -272,11 +334,14 @@ Section "Uninstall"
; (typically EA Desktop / Origin Launcher). If they didn't exist pre-install
; the macro just deletes the key, which leaves the system in a clean state
; ready for EA Launcher to register itself on its next run.
;
; RestoreProtocol forces SetRegView default internally so HKCR
; operations target the same store the installer's BackupProtocol used.
!insertmacro RestoreProtocol "qrc"
!insertmacro RestoreProtocol "link2ea"
!insertmacro RestoreProtocol "origin2"

; Restore Origin compatibility values
; Restore Origin compatibility values (view 64 - matches install-time)
SetRegView 64
!insertmacro RestoreValue HKLM "SOFTWARE\WOW6432Node\Origin" "ClientPath" "Origin_ClientPath"
!insertmacro RestoreValue HKLM "SOFTWARE\Electronic Arts\EA Desktop" "InstallSuccessful" "EADesktop_InstallSuccessful"
Expand Down
35 changes: 33 additions & 2 deletions maxima-bootstrap/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,25 @@ async fn run(args: &[String]) -> Result<bool, RunError> {
}

child.args(["launch", offer_id]);
child.spawn()?.wait().await?;
let status = child.spawn()?.wait().await?;

// Bootstrap historically ignored maxima-cli's exit code: if the
// child crashed or aborted, bootstrap still returned Ok(true) and
// the log read "Result: Success", making the failure invisible.
// Surface non-zero exits so maxima_execution.log shows when the
// launch actually failed.
if !status.success() {
let temp_dir = std::env::temp_dir();
let debug_log = temp_dir.join("maxima_execution.log");
if let Ok(mut file) = std::fs::OpenOptions::new().create(true).append(true).open(&debug_log) {
use std::io::Write;
let _ = writeln!(
file,
"maxima-cli (link2ea) exited non-zero: code={:?}",
status.code()
);
}
}
Comment on lines +278 to +284
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The manual logging of the non-zero exit code to maxima_execution.log is redundant because the handle_launch_args function already logs the result. By returning an error here, you can leverage the existing infrastructure. Additionally, ensure that if Stdio::piped() is used, the stderr output is captured and included in the error message to provide better diagnostic information.

            if !status.success() {
                return Err(std::io::Error::new(
                    std::io::ErrorKind::Other,
                    format!("maxima-cli (link2ea) exited non-zero: code={:?}", status.code())
                ).into());
            }
References
  1. When capturing command output in Rust using Stdio::piped(), ensure that stderr is also read and incorporated into error reporting to avoid losing diagnostic information.


return Ok(true);
}
Expand Down Expand Up @@ -313,7 +331,20 @@ async fn run(args: &[String]) -> Result<bool, RunError> {
}

child.args(["launch", offer_id]);
child.spawn()?.wait().await?;
let status = child.spawn()?.wait().await?;

if !status.success() {
let temp_dir = std::env::temp_dir();
let debug_log = temp_dir.join("maxima_execution.log");
if let Ok(mut file) = std::fs::OpenOptions::new().create(true).append(true).open(&debug_log) {
use std::io::Write;
let _ = writeln!(
file,
"maxima-cli (origin2) exited non-zero: code={:?}",
status.code()
);
}
}
Comment on lines +331 to +337
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the link2ea block, returning an error here is cleaner than manual logging. This ensures consistency and utilizes centralized logging. Note that if Stdio::piped() is used, stderr should be read and incorporated into the error reporting to avoid losing diagnostic information.

            if !status.success() {
                return Err(std::io::Error::new(
                    std::io::ErrorKind::Other,
                    format!("maxima-cli (origin2) exited non-zero: code={:?}", status.code())
                ).into());
            }
References
  1. When capturing command output in Rust using Stdio::piped(), ensure that stderr is also read and incorporated into error reporting to avoid losing diagnostic information.


return Ok(true);
}
Expand Down
2 changes: 1 addition & 1 deletion maxima-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ inquire = "0.6.2"
futures = "0.3.30"

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3.9", features = [ "memoryapi", "handleapi", "synchapi", "wincon", "consoleapi" ] }
winapi = { version = "0.3.9", features = [ "memoryapi", "handleapi", "synchapi", "wincon", "consoleapi", "processenv", "fileapi", "winbase", "winnt" ] }
winreg = "0.50.0"
is_elevated = "0.1.2"

Expand Down
Loading
Loading