fix(windows): decode the principal lookup with the console code page

devlog/_plan/260904_cross_platform_parity/004_implementation_outcome.md

@@ -0,0 +1,90 @@
+# 004 - Implementation outcome
+
+What actually landed for `260904_cross_platform_parity`, what review changed, and
+what the plan got wrong. Written at the close of wp3.
+
+## The stack
+
+| PR | Phase | Base | Head |
+|---|---|---|---|
+| [#3436](https://github.com/lidge-jun/opencodex/pull/3436) | wp0 roadmap | `dev` | `codex/260904-cross-platform-parity-roadmap` |
+| [#3437](https://github.com/lidge-jun/opencodex/pull/3437) | wp1 Muse manual key | #3436 | `codex/260904-muse-platform-refusals` |
+| [#3440](https://github.com/lidge-jun/opencodex/pull/3440) | wp2 platform-support docs | #3437 | `codex/260904-platform-support-docs` |
+| [#3438](https://github.com/lidge-jun/opencodex/pull/3438) | wp3 identity decode | #3437 | `codex/260904-windows-identity-decode` |
+
+wp2 and wp3 are siblings on wp1 rather than a chain: neither touches the other's
+files, and serializing them would have made the second wait on the first for no
+reason.
+
+## What review changed
+
+**The plan was cut from five phases to three, across six audit rounds.** Two of
+the removals were defects in my own design, not scope trimming:
+
+- The legacy scheduler-task migration would have re-registered a DIFFERENT user's
+  task to the current user. Matching `<Command>` and the launcher proves the task
+  runs our files, not that its session triggers belong to this account.
+  `tests/service.test.ts:628-641` already pinned that rejection, and my proposed
+  test only checked a foreign command, never a foreign user.
+- The Linux env-file port would have written a token-bearing `claude-env.sh` with
+  no reaper: `revertSystemEnv`, toggle-off and `cleanStaleSystemEnv` all return
+  early off darwin. It also referenced `modelEnv` and `auto` before they exist
+  and would not have compiled.
+
+**Three more were things the tree already had, or already forbade.** A GUI
+"disabled reason" I planned to add exists, localized, at
+`gui/src/pages/claude-code-settings.tsx:43-54`. A `skip` discriminant would have
+broken four exact `toEqual` assertions and reclassified real failures as benign.
+The Muse plan invented pointer fields that `MusePointer` does not declare.
+
+**Implementation review then found five more in wp1 alone**, including two worth
+recording: `refreshMetaMuseToken` hardcoded `source: "local-cli"`, which
+`merged()` would have used to relabel a hand-pasted key as an imported one; and
+the credential-leak test caught its own sentinel, so a case that unexpectedly
+SUCCEEDED passed vacuously. The second is the more instructive failure - the test
+was measuring itself.
+
+## What the plan got wrong
+
+**wp1's scope was wrong until the repository owner corrected it.** Both drafts
+shipped refusals, on the reasoning that we cannot read the credential store on
+Windows or Linux. That is true and beside the point: the Muse Code API key is
+visible in Meta's own console, so refusing the platform reported a limitation of
+our importer as a limitation of the platform. The phase became manual key entry.
+
+**#3320's causal claim was overstated.** `003` originally called the decode
+defect the root cause. The reporter's evidence was collected after a local
+repair, so the original registration shape was never observed. The defect is real
+and verified in the tree; the link to that report is a candidate, which is why
+#3438 references the issue instead of closing it.
+
+## Verification
+
+The user forbade running the full local suite, so CI is the suite authority. Per
+phase:
+
+- wp0: docs-only; all 8 workflow runs on the branch concluded success.
+- wp1: `tests/meta-muse-oauth.test.ts` + `tests/oauth-manual-code.test.ts`, 52
+  pass. Leak guard driven red first.
+- wp2: `bun run --cwd docs-site build`, 425 pages, exit 0, plus a hand check of
+  the localized sidebar href because a manual `link` is not build-validated.
+- wp3: `windows-user-principal-nonascii` + `windows-user-principal`, 25 pass;
+  `windows-secret-acl` (the `identity.name` consumer), 169 pass. Guards driven
+  red first: 5 of 9 fail against the old UTF-8 decode.
+
+`bun x tsc --noEmit` clean at every commit.
+
+## One process note
+
+The subagent review lane died with a provider 401 for the last three phases
+(`No eligible Codex account supports this model`). wp2 and wp3 were therefore
+audited first-hand and their attests say so, with `near-pass` rather than
+`pass` and the residual recorded. An audit nobody independent performed should
+not be labelled as though someone did.
+
+## Not done
+
+Everything in `050`, each with its blocking reason. The two that matter most: the
+legacy name-form task migration needs a trusted name-to-SID resolution channel,
+and the Linux env-file port needs the credential review `AGENTS.md` mandates.
+

devlog/_plan/260904_cross_platform_parity/040_wp4_stack_closeout.md

@@ -1,19 +1,21 @@
-# 040 - wp4: stack close-out (administrative, NOT a fourth PR)
+# 040 - wp4: stack close-out (administrative, opens no PR of its own)
 
-This unit ships exactly THREE pull requests: wp1, wp2, wp3. wp4 opens no fourth
+This unit ships FOUR pull requests: the wp0 roadmap plus wp1, wp2 and wp3. wp4 opens no further
 PR and introduces no code. It is the administrative work performed ON the
 existing stack - CI triage, review responses, retargeting, and the closeout
 record - and its one artifact, `004_implementation_outcome.md`, is a devlog
 commit on the last child branch in the chain.
 
-Evidence: the three PRs from wp1, wp2, wp3.
+Evidence: #3436, #3437, #3440 and #3438.
 
 ## What this phase does
 
-1. Confirm each PR in the chain is open against the right base: wp1 on `dev`,
-   wp2 on wp1's head, wp3 on wp2's head. `enforce-target` skips the wrong-base
-   gate for children of an open PR; after a parent lands, retarget the child to
-   `dev`.
+1. Confirm each PR is open against the right base. The chain is NOT linear:
+   #3436 on `dev`, #3437 on #3436, then #3440 and #3438 BOTH on #3437 as
+   siblings. wp2 and wp3 share no files, so chaining them would have made one
+   wait on the other for nothing. `enforce-target` skips the wrong-base gate
+   for children of an open PR; retarget each child to `dev` once its parent
+   lands.
 2. Read CI on each PR. Triage any failure and fix it in the owning PR rather than
    the tip of the stack, so each commit stays independently reviewable.
 3. Answer Codex and CodeRabbit review findings on every PR in the chain.
@@ -34,8 +36,10 @@ suite from memory or from a local run that did not happen.
 
 ## Definition of done
 
-- Exactly three PRs open or landed against `dev`, each filled from
+- Four PRs open or landed, each filled from
   `.github/PULL_REQUEST_TEMPLATE.md`. No fourth PR exists.
-- CI conclusion captured per PR as goalplan evidence.
+- CI conclusion captured per PR as goalplan evidence, with any failure either
+  fixed here or PROVEN inherited by reproducing it on clean `origin/dev`.
+  "Unrelated" is a claim that needs evidence.
 - `004` written.
 - `050` lists every deliberate follow-up with its reason.

devlog/_plan/260904_cross_platform_parity/041_ci_triage.md

@@ -0,0 +1,49 @@
+# 041 - CI triage for the stack
+
+Recorded at wp4. Every failure below was reproduced on clean `origin/dev` before
+being called inherited, because "not mine" is a claim that needs evidence rather
+than an assumption.
+
+## The rebase that was actually required
+
+The first CI run on #3437 failed `release version line`:
+
+> package.json version 2.42.0 equals release tag v2.42.0, but this commit is not
+> the one that tag names. The tree claims an already-published version.
+
+Real, and ours to fix: the stack branched when `dev` was at 2.42.0, v2.42.0 then
+shipped, and `dev` moved to 2.43.0. The whole chain was rebased onto current
+`dev` and force-pushed with `--force-with-lease`, bottom-up so each child kept
+its parent. `tests/release-version-line.test.ts` passes locally afterwards.
+
+## Inherited failures, reproduced on clean dev
+
+Two suites fail on `origin/dev` at `20011a1c4` with no change of ours applied.
+Verified in a scratch worktree (`git worktree add .tmp/devcheck origin/dev`),
+not inferred:
+
+**`tests/loopback-listener-integration.test.ts:366`** - "admits the exact
+standalone Images POST routes so they reach the relay (#3428)". Expects the
+status to be 400 or 503, receives 401. Clean dev: 30 pass, 1 fail. Our branch:
+identical, 30 pass, 1 fail. The test arrived with #3430
+(`fix(server): allow image routes on loopback listener`) and exercises image
+routes on the loopback listener, which no file in this stack touches.
+
+**`tests/star-deferral.test.ts:102`** - "agent deferral fires once per version,
+never writes the marker, and a human run still prompts". Expects `> 0`, receives
+`0`. Clean dev: 6 pass, 1 fail. Same on our branch.
+
+Neither is in this unit's blast radius. The stack changes
+`src/lib/windows-user-principal.ts`, `src/oauth/meta-muse.ts`,
+`src/providers/registry.ts`, two docs files and three test files.
+
+## Disposition
+
+The version-line failure was ours and is fixed. The other two are open defects on
+`dev` that any PR opened today inherits; they are not this stack's to fix, and
+fixing them here would smuggle unrelated work into a scoped chain. They should be
+raised as their own issues against the units that introduced them.
+
+Worth stating plainly: this means the stack cannot show an all-green CI run until
+`dev` is green. The honest report is "no new failures", not "all checks pass".
+

src/lib/windows-user-principal.ts

@@ -23,6 +23,7 @@
 import { existsSync } from "node:fs";
 import { win32 as windowsPath } from "node:path";
 import { waitForSubprocessExit } from "./bounded-subprocess";
+import { decodeWindowsTextBytes } from "./windows-text";
 
 import {
   resolveTrustedWindowsPowerShellExe,
@@ -98,7 +99,12 @@ export interface WindowsPrincipalLookupResult {
   success: boolean;
   exitCode: number | null;
   timedOut: boolean;
-  stdout: string;
+  /**
+   * Raw child stdout. Bytes are allowed because `powershell.exe` writes the console
+   * output code page, not UTF-8, and the decode below is the thing under test: a seam
+   * that only carried a decoded string could never exercise it.
+   */
+  stdout: string | Uint8Array;
 }
 
 export type WindowsPrincipalRunner = (
@@ -138,7 +144,10 @@ function defaultWindowsPrincipalRunner(timeoutMs: number): WindowsPrincipalLooku
     success: result.success,
     exitCode: result.exitCode,
     timedOut: result.exitedDueToTimeout ?? false,
-    stdout: result.stdout ? result.stdout.toString() : "",
+    // Bytes, NOT .toString(): that is UTF-8, and Windows PowerShell 5.1 emits the
+    // console output code page. A non-ASCII account name decoded as UTF-8 becomes
+    // U+FFFD and is then frozen into the identity cache.
+    stdout: result.stdout ?? new Uint8Array(),
   };
 }
 
@@ -152,8 +161,9 @@ async function defaultAsyncWindowsPrincipalRunner(
     windowsHide: true,
   });
   const { exitCode, timedOut } = await waitForSubprocessExit(proc, timeoutMs);
-  const stdout = !timedOut && proc.stdout
-    ? await new Response(proc.stdout).text().catch(() => "")
+  // `.bytes()` rather than `.text()`, for the same reason as the sync runner above.
+  const stdout: string | Uint8Array = !timedOut && proc.stdout
+    ? await new Response(proc.stdout).bytes().catch(() => new Uint8Array())
     : "";
   return {
     success: !timedOut && exitCode === 0,
@@ -165,6 +175,24 @@ async function defaultAsyncWindowsPrincipalRunner(
 
 let principalRunner: WindowsPrincipalRunner = defaultWindowsPrincipalRunner;
 let asyncPrincipalRunner: AsyncWindowsPrincipalRunner = defaultAsyncWindowsPrincipalRunner;
+let principalLocaleForTests: string | undefined;
+
+/**
+ * Decode child stdout the way the rest of this repository already decodes Windows
+ * console output: UTF-16 with or without a BOM, then STRICT UTF-8, then the locale's
+ * legacy code page. Strict-UTF-8-first is what keeps an ordinary UTF-8 host unaffected.
+ *
+ * The SID on the first line is ASCII by construction and survives either way, which is
+ * why this corruption stayed silent: only the account name on the second line breaks.
+ */
+function decodePrincipalStdout(stdout: string | Uint8Array): string {
+  if (typeof stdout === "string") return stdout;
+  return decodeWindowsTextBytes(
+    stdout,
+    principalLocaleForTests ? { locale: principalLocaleForTests } : {},
+  );
+}
+
 export interface WindowsPrincipalIdentity {
   readonly sid: string;
   readonly name: string;
@@ -220,7 +248,7 @@ function identityFromResult(result: WindowsPrincipalLookupResult): WindowsPrinci
       ? "timed out"
       : `exited ${result.exitCode ?? "null"}`);
   }
-  const lines = result.stdout.trim().split(/\r?\n/);
+  const lines = decodePrincipalStdout(result.stdout).trim().split(/\r?\n/);
   const sid = lines[0]?.trim() ?? "";
   const name = lines[1]?.trim() ?? "";
   if (!SID_PATTERN.test(sid)) {
@@ -341,6 +369,26 @@ export function setAsyncWindowsPrincipalRunnerForTests(
   cachedIdentity = null;
 }
 
+/**
+ * Test seam: pin the locale that selects the legacy code page.
+ *
+ * Required rather than convenient. `decodeWindowsTextBytes` picks ONE legacy encoding
+ * from the ambient locale, so CP949, CP932 and CP936 fixtures cannot all decode
+ * correctly in a single process without being told which to expect. Production passes
+ * nothing and keeps the ambient locale.
+ *
+ * Clears the cache and refuses mid-flight for the same reasons the runner setters do:
+ * a successful identity is returned from cache BEFORE any decode, and the async path
+ * decodes after its runner resolves.
+ */
+export function setWindowsPrincipalLocaleForTests(locale: string | null): void {
+  if (asyncLookupInFlight) {
+    throw new Error("Cannot change the Windows principal locale while a lookup is in flight.");
+  }
+  principalLocaleForTests = locale ?? undefined;
+  cachedIdentity = null;
+}
+
 /** Test seam: clear only process-local principal state. */
 export function resetWindowsPrincipalForTests(): void {
   if (asyncLookupInFlight) {