diff --git a/devlog/_plan/260904_cross_platform_parity/000_problem_model.md b/devlog/_plan/260904_cross_platform_parity/000_problem_model.md new file mode 100644 index 0000000000..8b83e2e755 --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/000_problem_model.md @@ -0,0 +1,72 @@ +# 000 - Problem model: what "macOS-only" actually means here + +Unit: cross-platform parity + Windows compatibility fixes. +Branch base: `dev` at `072df52eb`. Date: 2026-09-04. + +## The request + +Make the capabilities that only work on macOS work on Windows and Linux too, and +land the Windows-compatibility bug fixes the backlog already documents. Delivered +as a stacked pull-request chain against `dev`. + +## The claim that had to be tested first + +"opencodex is macOS-only in places" is the starting hypothesis, not a finding. A +read-only inventory of every `darwin` gate in `src/` (recorded in `001`) shows the +claim is mostly FALSE and the exceptions are concentrated: + +- 21 darwin-referencing sites were classified. +- 13 are ALREADY-HANDLED: they carry real win32 and linux branches today + (`open-url.ts`, `cursor-detect.ts`, `desktop-3p-paths.ts`, `kiro-credentials.ts`, + `app-server-processes.ts`, `service.ts` backend dispatch, `key-store.ts`, and the + Claude credential file fallback in `local-token-detect.ts`). +- 6 are a single subsystem: `src/server/system-env.ts`, which refuses with + `reason: "not macOS"` at five entry points and holds the launchctl calls behind them. +- 1 is a hard throw: `src/oauth/meta-muse.ts` refuses every non-darwin host. +- 1 is a missing developer script: `scripts/ocx-restart.sh` has no Windows counterpart. + +So this unit is not a porting sweep. After four audit rounds it is three phases, +each its own PR in a stacked chain: + +- **wp1** - `meta-muse` refuses on Windows and Linux with accurate reasons + instead of a false macOS-Keychain one. +- **wp2** - a platform-support reference page, so the capabilities that stay + macOS-only have a written answer rather than a silent dead end. +- **wp3** - the Windows identity decode fix, the one defect proven to exist in + the tree. + +Everything else the audits removed is in `050` with its blocking reason. + +## The second problem, found while looking + +While inventorying the Windows paths, a real defect surfaced in the tree: the +identity ACQUISITION path decodes PowerShell stdout as UTF-8 when Windows +PowerShell 5.1 emits the console code page, so a non-ASCII account name is +mojibaked before any comparison happens. The `` comparison itself is +correct. Details and evidence: `003`. + +Its relationship to issue #3320 is CANDIDATE, not proven. The reporter's evidence +was collected after a local patch and repair, so the original registration shape +is unknown and nothing here establishes that this defect produced that user's +failure. What can be said conditionally: #3134 moved new registrations to SID +form, and a SID is ASCII, so freshly registered tasks stay healthy; a task +registered by v2.39.0 or earlier carries a name-form ``, and for a +non-ASCII account both sides of that comparison are separately corrupted, which +would make `ocx service repair` refuse it permanently. That is a plausible route +to the reported symptom, not a demonstrated one. + +## What "done" means for this unit + +Every phase below ships as its own reviewable PR against `dev`, stacked so each +child bases on its parent's head branch (DEV-STACK-01), with CI as the verification +authority. The user has forbidden running the full local suite, so no phase may +claim a green suite as evidence; each phase names the focused reasoning or the CI +run that backs it. + +## Non-goals + +- No provider catalog or model metadata churn. +- No GUI redesign. +- No release promotion to `preview` or `main`. +- No security-triage writeup in this directory (AGENTS.md: scratch only). +- The `go/` directory is untouched. diff --git a/devlog/_plan/260904_cross_platform_parity/001_darwin_surface_inventory.md b/devlog/_plan/260904_cross_platform_parity/001_darwin_surface_inventory.md new file mode 100644 index 0000000000..1e1800d1aa --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/001_darwin_surface_inventory.md @@ -0,0 +1,101 @@ +# 001 - Darwin surface inventory + +Read-only sweep of every `darwin`, `macOS`, `Keychain`, `launchctl`, `osascript`, +`/Applications` and `plist` reference under `src/` and `scripts/`, classified for +portability. Verdicts: PORTABLE, DOCUMENT-ONLY, ALREADY-HANDLED. + +## Already handled - no work needed + +These carry real win32 and linux branches today. Listed so a future reader does +not re-open them. + +| Site | Why it is fine | +|---|---| +| `src/lib/open-url.ts:14` | Three-way branch; `rundll32 url.dll,FileProtocolHandler` on win32, `xdg-open` on linux, with an ENOENT listener so a headless host cannot kill the proxy | +| `src/integrations/cursor-detect.ts:69` | `/Applications` is one of three branches; win32 scans `LOCALAPPDATA\\Programs` and `ProgramFiles`, linux scans `/opt` and `~/.local/share` | +| `src/integrations/cursor-effort-table.ts:45` | `Contents/Resources/app` vs `resources/app`, the correct Electron layout for each | +| `src/claude/desktop-3p-paths.ts:47` | Pure resolver with `APPDATA`/`LOCALAPPDATA` and `XDG_CONFIG_HOME` branches | +| `src/oauth/kiro-credentials.ts:166,224` | Full win32 and linux branches for the session DB and executable | +| `src/oauth/local-token-detect.ts:78` | Keychain returns null off darwin and falls through to `.credentials.json`, which is what Claude Code writes on Windows and Linux | +| `src/claude/auth-detect.ts:207` | Metadata-only presence probe; "absent" off darwin is correct because the file source covers those platforms | +| `src/oauth/anthropic.ts:172` | Error text only; prints the file-only variant off darwin | +| `src/service.ts:3454` and around | Three-backend dispatch: launchd, Task Scheduler/WinSW, systemd user unit | +| `src/codex/app-server-processes.ts:523,626,676` | Named win32 and linux branches with their own timeout bounds | +| `src/codex/log-guard/path-safety.ts:13` | `/var` to `/private/var` alias normalization is genuinely macOS-shaped; a Windows canonical comparator sits alongside it | +| `src/providers/key-store.ts:33` | Not darwin-gated at all; `@napi-rs/keyring` maps to Credential Manager and libsecret | + +## The real gaps + +### 1. `src/server/system-env.ts` - five refusals, one subsystem + +| Line | Function | Behavior off darwin | +|---|---|---| +| 142 | `installShellHook` | `{ installed: false, reason: "not macOS" }` | +| 159 | `uninstallShellHook` | `{ removed: false, reason: "not macOS" }` | +| 223 | `reconcileShellHook` | `{ changed: false, state: "absent", reason: "not macOS" }` | +| 372 | `injectSystemEnv` | `{ injected: false, reason: "not macOS" }` | +| 489 | `revertSystemEnv` | `{ reverted: false, reason: "not macOS" }` | + +What it does on macOS: writes `~/.opencodex/claude-env.sh` (platform-neutral), +appends a marked hook line to `~/.zshrc`, and injects `ANTHROPIC_BASE_URL`, +`CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY`, conditionally `ANTHROPIC_AUTH_TOKEN`, +plus seven lever keys into the launchd user domain via `launchctl setenv`. + +Ownership and rollback are stronger than the surface suggests, and any port must +preserve them: a tracking file `~/.opencodex/system-env-port` (0600) holds pid, +port and `injectedKeys`; revert unsets ONLY tracked keys so a pre-existing user +value survives; `injectedKeys` is re-persisted after every single `setenv` so a +crash mid-injection still leaves a complete undo list; lever keys are user-wins +(`injectLever` skips a key already present); revert refuses on ownership mismatch; +and `rollbackInjectedKeys` rewrites tracking with only the keys whose unset failed +so a partial rollback stays resumable. + +Callers: `ocx start` (`src/cli/index.ts:443` and the already-running path at 537), +`syncCleanup` at 378, `ocx stop` at 957, `ocx uninstall` at 1239, and +`applySystemEnvToggle` from `agent-settings-routes.ts:1384`. + +Verdicts: the SHELL HOOK half (142/159/223) is PORTABLE and cheap - the writer is +already platform-neutral and the marker install/remove/verify logic is already +written; the only blocker is the `!== "darwin"` guard plus a hardcoded `~/.zshrc`. +The ENV INJECTION half (372/489) is PORTABLE but expensive and carries a real +security question: moving `ANTHROPIC_AUTH_TOKEN` from a per-boot launchd domain +into a persistent `HKCU\\Environment` hive changes secret exposure, which AGENTS.md +routes to explicit security review. It is deliberately NOT in this unit. + +### 2. `src/oauth/meta-muse.ts:136` - the only hard throw + +`loginMetaMuse` throws on every non-darwin platform with a message blaming the +macOS Keychain. Measurement in `002` shows the message states the wrong reason. +Verdict: DOCUMENT-ONLY for Windows, PORTABLE for Linux pending a measured pointer. + +### 3. `scripts/ocx-restart.sh` - no Windows counterpart + +Bash-only detached restart helper for agent sessions. `restart-codex-desktop-app.ps1` +is a different tool. The `darwin` mention inside it is only a `setsid` fallback. +Verdict: PORTABLE, small. + +### 4. `src/server/management/agent-settings-routes.ts:1091` + +`autoConnectSupported` hardcodes `platform === "darwin"`. Honest today, since the +capability really is macOS-only, and the GUI fails closed on it. Three distinct +things must not be conflated when any of this moves: launchctl ENV INJECTION +(macOS only), writing the `claude-env.sh` SHELL FILE (deferred to its own unit, +`050`), and the SHELL HOOK that sources it. `autoConnectSupported` names the +first. A port of the second or third needs its own field rather than overloading +this one, because `tests/claude-management-api.test.ts:653-664` correctly pins +this flag false off darwin. + +## One latent hazard worth recording + +`src/cli/index.ts:1240` and `:1245` allowlist the literal reason strings +`"not macOS"` and `"not installed"` so `ocx uninstall` treats them as benign. A +future backend returning a different reason string turns a benign no-op into a +failed uninstall step. + +Audit round 3 showed this is not a free refactor: four exact `toEqual` +assertions pin the current return objects +(`tests/claude-shell-hook.test.ts:63,79,107,173`), and a discriminant applied to +"every refusal path" would classify genuine failures (`no HOME`, +`read/write failed`) as benign skips. It belongs WITH the port that needs it, as +a discriminated union designed against those call sites - deferred to `050`, +not part of any phase in this unit. diff --git a/devlog/_plan/260904_cross_platform_parity/002_muse_cli_storage_measurement.md b/devlog/_plan/260904_cross_platform_parity/002_muse_cli_storage_measurement.md new file mode 100644 index 0000000000..709dc72697 --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/002_muse_cli_storage_measurement.md @@ -0,0 +1,78 @@ +# 002 - Measured: what the Muse Code CLI stores off macOS + +The `meta-muse` provider is the only hard platform throw in the runtime +(`src/oauth/meta-muse.ts:136`). This document records what is known, what is +sourced, and what is still unmeasured, because the module's own contract is to +refuse any storage backend it has not verified (`meta-muse.ts:160`). + +## What was measured on macOS (prior unit, not re-derived here) + +`devlog/_plan/260903_muse_spark_plan_oauth/003` records the shipped shape: + +- `~/.config/muse/auth.json` (0600) is a POINTER carrying no secret. Its + `providers.meta` object declares `mechanism: "oauth"` and `storage: "keychain"`. +- The secret is a macOS Keychain generic-password item, service + `ai.meta.dev.credentials`, account `meta`. +- Only `api_key` authenticates the Model API; `access_token` returns 401. The key + matches `/LLM\|\d+\|[A-Za-z0-9_-]{10,}/`. + +The important detail for this unit is that `storage` is a DECLARED field in the +pointer. The CLI tells us where it put the secret. That is the extension point: +a non-keychain host will declare a different value, and the module already +refuses unknown values rather than guessing. + +## What the vendor documents for other platforms + +Meta's quickstart documents installation for macOS and Linux only, through +`curl -fsSL https://dev.meta.ai/install.sh | bash`. There is no native Windows +installer; the documented Windows route is WSL2. + +Consequence, and it reframes the whole task: **there is no native Windows Muse +CLI to import a credential from.** The current error message is wrong about the +reason. It blames the macOS Keychain when the real reason on Windows is that the +vendor ships no Windows CLI at all. + +## Linux: sourced, NOT yet measured + +Third-party setup writeups describe the Linux credential living in +`~/.config/muse/auth.json` and honoring `$XDG_CONFIG_HOME`, with the secret in +that JSON file rather than an OS keyring. That is plausible - it matches how the +pointer already declares its own backend - but it is **unverified**. No Muse CLI +install exists on this host (`~/.config/muse/auth.json` is absent, checked +2026-09-04), so the exact `storage` value a Linux install writes has not been +observed. + +This is the single fact that gates wp1's Linux half. The module must not invent a +`storage` value. Two honest routes: + +1. Implement the Linux branch keyed on the DECLARED `storage` value, accepting a + file-backed secret only when the pointer says so, and keep refusing unknown + values. If a Linux install declares `storage: "keychain"`, the refusal still + fires and nothing is silently wrong. +2. Do not guess a specific value: accept the shapes we can validate structurally + (a secret embedded in the pointer, or a sibling file the pointer names) and + refuse everything else with a message that says what was found. + +**Audit round 1 rejected both routes as premature (blocker 6).** The pointer +interface (`src/oauth/meta-muse.ts:58-60`) declares only `mechanism`, `storage` +and `user_email`. There is no path field and no inline-key field. Writing a reader +against fields nobody has observed is the unverified-credential path this module +refuses everywhere else, and no amount of structural validation makes an invented +schema measured. + +So this unit ships neither route. Linux keeps refusing, with a message that states +the real reason instead of blaming the macOS Keychain. `010` implements that, and +`050` records the exact measurement that would unblock a Linux reader: a real +`~/.config/muse/auth.json` from a Linux install, with its `storage` value and, if +the secret is file-backed, the field naming the file. + +## What wp1 must therefore deliver + +- Windows: replace the misleading macOS-Keychain refusal with an accurate one + that names WSL2 and the supported `META_MODEL_API_KEY` alternative. No WSL2 + pointer read: reachability was never measured, and a refusal that tells the + truth is a fix while a guess is not. +- Linux: a refusal naming the unmeasured storage rather than the Keychain. No + reader until a real pointer is measured. +- Neither platform may weaken the ToS consent warning, which is the CLI's only + warning surface. diff --git a/devlog/_plan/260904_cross_platform_parity/003_issue_3320_root_cause.md b/devlog/_plan/260904_cross_platform_parity/003_issue_3320_root_cause.md new file mode 100644 index 0000000000..8f5c5a8eb4 --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/003_issue_3320_root_cause.md @@ -0,0 +1,121 @@ +# 003 - Verified latent defect: Windows non-ASCII identity decode (candidate cause of #3320) + +Verified against `dev` at `072df52eb` on 2026-09-04. +Revised after audit round 1: the causal link to #3320 is a CANDIDATE, not proven. +The reporter's SID evidence was collected after a local patch and repair, so it +does not establish the original registration shape. What follows is proven about +the CODE; the connection to that user's stock-v2.40.0 failure is not. + +## Verdict + +A real defect exists in the tree, and it is NOT where the issue title points. + +The reporter's `` is a SID (`S-1-5-21-...`). A SID is pure ASCII, so it +survives any code page, and `taskXmlDecodedValueEquals` (`src/service.ts:1992`) +matches it exactly. **The `` comparison is correct and is not the bug.** + +The defect is one layer up, in how the expected identity is ACQUIRED. + +## The break point + +`src/lib/windows-user-principal.ts:141`, and its async twin at `:156`: + +```ts +stdout: result.stdout ? result.stdout.toString() : "", +``` + +`result.stdout` is a Buffer, and bare `.toString()` is UTF-8. The child is +`powershell.exe` (`windows-user-principal.ts:112`) with stdout piped, so Windows +PowerShell 5.1 encodes using the console output code page - CP949, CP936, CP932 - +not UTF-8. `$identity.Name` returns `DOMAIN\\account`. For a non-ASCII account +those bytes are not valid UTF-8, so the decode yields U+FFFD mojibake, and +`identityFromResult` (`:225`) freezes the corrupted string into the process cache +as `identity.name`. + +The repository already owns the correct decoder for exactly this class of bug: +`decodeWindowsTextBytes` (`src/lib/windows-text.ts:101`), which tries UTF-16 with +and without BOM, then STRICT UTF-8, then the locale's legacy code page. This +module never calls it. The SID on the adjacent line is ASCII, so the corruption +is silent. + +## What the defect would cause, conditionally + +These are consequences of the CODE. Whether any of them produced the failure in +#3320 is not established; see the header. + +1. `identity.name` is corrupt whenever the account name is non-ASCII. A + SID-registered task still matches on the SID, so health survives. A + SID-form task is therefore unaffected. +2. **A legacy name-form task becomes permanently unrepairable.** Versions through + v2.39.0 wrote the account NAME into ``. For a non-ASCII account the + reported name is code-page mangled by schtasks AND the expected name is + mojibaked by the UTF-8 decode - two different corruptions, so they never + match. `windowsTaskRegistrationHealthy` returns false; + `windowsTaskRegistrationRefreshableLegacy` (`service.ts:2202`) also rejects it + because it DOES carry session triggers. Repair then throws "not a recognized + legacy OpenCodex definition; it was preserved for manual review" + (`service.ts:2948`) and changes nothing. A permanent dead end, and a plausible + route to the reported symptom - but the reporter's original registration shape + was never observed, so this remains a hypothesis. +3. `src/lib/windows-secret-acl.ts:565,583` compares against `identity.name` for + ACL checks; a mojibaked name silently fails that compare. Consequences beyond + "returns false" are unverified. + +Normalization and case are NOT implicated. Case is already handled. NFC/NFD +normalization is absent on both sides but no evidence shows it triggering here, +so no claim is made. + +## What #3134 did and did not cover + +`b14b741dc` (#3134, shipped in v2.40.0) closed #3064. It added +`taskXmlLossyValueEquals` (`service.ts:2026`) and applied it ONLY to `` +and `` (`service.ts:2175`). The commit states the exclusion outright: +applying lossy comparison to `` would let `MACHINE\` match +`MACHINE\Admin`. That refusal is correct and must be preserved. #3134 also moved +new registrations from name form to SID form. + +Not covered: the identity ACQUISITION decode, and any pre-existing name-form task +belonging to a non-ASCII account. No commit references #3320. + +## The probe path is already correct + +`src/service-manager-probe.ts` decodes through `decodeWindowsTextBytes` with an +explicit locale at `:651`, `:658`, `:674`, `:726`, `:864`, `:883`, `:945`. Nothing +there compares against a localized or non-ASCII string; +`windowsTaskListContains` (`:600`) compares only the ASCII task name. + +Worth recording as latent, not active: `service.ts` and `service-manager-probe.ts` +use two different decoders for the same schtasks output. `decodeSchtasksOutput` +(`service.ts:902`) handles UTF-16 then falls back to plain UTF-8 with no code-page +branch. For `/query /xml` that is fine because the payload is UTF-16. + +## Test coverage today + +Covered: `tests/windows-text-decoding.test.ts` exercises CP949, CP936, CP932, +Big5, Windows-1252 and the refusal to guess CP1251 - but only against +`decodeWindowsTextBytes`, which the identity path never calls. +`tests/service.test.ts:643` pins that an explicit identity is never code-page +folded, using `MACHINE\\`. `service.test.ts:2541` covers legacy name-to-SID +migration with a pure-ASCII `MACHINE\\installer`, so both sides match and the bug +cannot appear. + +Not covered: every fixture in `tests/windows-user-principal.test.ts` uses +`EXAMPLE\\Owner`. No test anywhere feeds non-ASCII BYTES through the principal +runner, and the injected-runner seam hands over a pre-decoded `string`, so the +`Buffer.toString()` boundary is structurally untestable through the current seam. +That seam gap is itself part of the fix. + +## Conditional reproduction + +This is the reproduction the code PREDICTS, not one that has been executed +end-to-end against a reporter's machine. Non-ASCII account on a ko-KR host with +console code page CP949, holding an `opencodex-proxy` task registered by v2.39.0 +or earlier so `` is name form. Run `ocx service` then +`ocx service repair` on current `dev`. Predicted: repair throws "preserved for +manual review" and changes nothing. Confirming this on a real host, or obtaining +pre-repair task XML from the reporter, is what would upgrade the #3320 link from +candidate to established. + +Unit-level: give `defaultWindowsPrincipalRunner` a Buffer-returning seam, feed +CP949 bytes for `S-1-5-21-1-2-3-1001\r\nMACHINE\\\r\n`, and observe +`cachedCurrentWindowsIdentity().name` return U+FFFD instead of the account name. diff --git a/devlog/_plan/260904_cross_platform_parity/010_wp1_muse_platform_refusals.md b/devlog/_plan/260904_cross_platform_parity/010_wp1_muse_platform_refusals.md new file mode 100644 index 0000000000..b1f23fadb7 --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/010_wp1_muse_platform_refusals.md @@ -0,0 +1,103 @@ +# 010 - wp1: meta-muse honest platform refusals + +One PR. Base `dev`. Branch `codex/260904-muse-platform-refusals`. +Evidence: `002`. Revised after audit round 1 (FAIL, blocker 6). + +## What the audit changed + +The first draft accepted `storage: "file"` and "the path the pointer names". Both +were INVENTED. `MusePointer` (`src/oauth/meta-muse.ts:58-60`) declares only +`mechanism`, `storage` and `user_email` - there is no path field and no inline-key +field, and `002` itself records that no Linux pointer has ever been observed. +Writing a reader for fields nobody has seen is precisely the unverified-credential +path the module refuses everywhere else. + +The XDG change was also wrong as drafted: making `XDG_CONFIG_HOME` authoritative +on ALL platforms would redirect the MEASURED macOS path whenever that variable +happens to be set, with no evidence the macOS CLI honors it. + +So this phase ships what is actually provable: refusals that tell the truth. + +## The change in one sentence + +Replace the single `platform !== "darwin"` throw, which blames the macOS Keychain +on every platform, with per-platform refusals that state the real reason - and +keep refusing Linux until a real pointer is measured. + +## What must not change + +- The consent warning fires before any credential read (`meta-muse.ts:128-143`, + audit-confirmed: it precedes platform selection). +- Import-only. Nothing spawns `muse login`. +- The `LLM|` grammar check, the `access_token` prohibition, the refusal of any + unmeasured shape. +- `refreshMetaMuseToken` does not re-read storage. + +## MODIFY `src/oauth/meta-muse.ts` + +### 1. Windows gets a true refusal + +```ts +if (platform === "win32") { + throw new Error( + "Meta does not ship a native Windows Muse Code CLI, so there is no Windows credential to import. " + + "Install the CLI inside WSL2 and import there, or use the meta-model provider with your own key (META_MODEL_API_KEY).", + ); +} +``` + +### 2. Linux gets a true refusal, not a guess + +```ts +if (platform !== "darwin") { + throw new Error( + "Meta Muse Code import is verified only on macOS. The Muse CLI runs on Linux, but the credential " + + "storage it writes there has not been measured, and importing an unverified credential shape is refused. " + + "Use the meta-model provider with your own key (META_MODEL_API_KEY).", + ); +} +``` + +This is a real fix even though Linux still refuses. Today's message tells a Linux +user their Keychain is the problem, which is false and sends them nowhere. The new +message states what is actually true and names the path that works. + +### 3. XDG lookup is Linux-only and inert for now + +Deferred with the Linux reader. When `002` is updated with a measured pointer, +the resolver lands with it and is gated to non-darwin platforms so the measured +macOS path cannot move. + +## MODIFY `src/providers/registry.ts` + +The `meta-muse` `note` says "macOS only". Make it precise: requires the Muse Code +CLI signed in on macOS; not available on Windows (no native CLI); Linux import is +not yet verified. No other field changes. + +## Tests in `tests/meta-muse-oauth.test.ts` + +The existing table at `:181` already asserts `{ platform: "linux" }` rejects, so +that case stays green. Added: + +1. win32 refusal message names WSL2 and `META_MODEL_API_KEY`, and does NOT claim + the macOS Keychain is the reason. +2. linux refusal message names the unmeasured storage and `META_MODEL_API_KEY`, + and does NOT claim the macOS Keychain is the reason. +3. The consent warning is emitted before the throw on both refusal paths. + +Focused run: `bun test tests/meta-muse-oauth.test.ts`. + +## Acceptance + +- `bun x tsc --noEmit` clean. +- The focused file passes; no existing case changes behavior. +- No code reads a pointer field that has not been observed. +- CI green. + +## What this phase deliberately does not do + +Ship a Linux credential reader. `050` records the measurement that would unblock +it: a real `~/.config/muse/auth.json` from a Linux install, with its exact +`storage` value and, if the secret is file-backed, the exact field naming the +file. That is a measurement task, not an implementation guess. + diff --git a/devlog/_plan/260904_cross_platform_parity/020_wp2_platform_support_docs.md b/devlog/_plan/260904_cross_platform_parity/020_wp2_platform_support_docs.md new file mode 100644 index 0000000000..454f9a7e1d --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/020_wp2_platform_support_docs.md @@ -0,0 +1,106 @@ +# 020 - wp2: the platform-support reference page + +One PR. Bases on wp1's head branch (stacked child). +Branch `codex/260904-platform-support-docs`. Evidence: `001`. +Revised after audit rounds 1, 2 and 3. + +## Why this phase is now only a docs page + +This phase has been cut three times, and the reason is worth recording because it +is the most useful thing this unit learned. + +Round 1: the Linux shell-hook port had three defects. Round 2: the same phase, +rewritten, produced six more - it was not a platform guard to delete but a +credential-bearing file lifecycle on a new platform, and it went to `050`. +Round 3 then found that the REPLACEMENT scope was also partly invented: + +- The `skip` discriminant would break four exact `toEqual` assertions + (`tests/claude-shell-hook.test.ts:63,79,107,173`) and, worse, risked + classifying genuine failures like `no HOME` and `read/write failed` as benign + skips at `src/cli/index.ts:1244`. That is a correctness regression traded for a + refactor nobody asked for. +- The GUI "disabled reason" already EXISTS. `gui/src/pages/claude-code-settings.tsx:43-54` + renders a localized `claude.systemEnvUnsupported` explanation for exactly this + case, with coverage at `gui/tests/claude-code-autoconnect.test.tsx:64` and copy + in every locale from `gui/src/i18n/en.ts:2088`. The premise that a non-macOS + user sees an unexplained disabled control was simply false. + +So both halves are dropped. What survives is the piece that was never in dispute +and that criterion c-3 actually asks for: a written, accurate platform-capability +answer. + +## The change + +NEW `docs-site/src/content/docs/reference/platform-support.md`, a per-platform +capability matrix stating what works where and, when something does not, WHY: + +- Proxy, routing, and provider adapters: all three platforms. +- Background service: all three - launchd on macOS, Task Scheduler OR native + WinSW on Windows, systemd user unit on Linux. The two Windows backends are + mutually exclusive (`ServiceBackend = "scheduler" | "native"`, + `src/service.ts:64,324-326`) and holding both states at once is a conflict + repair refuses (`:417-420`), so the page must say "or", never "with". +- Provider key storage in the OS credential store: all three via + `@napi-rs/keyring`, WHEN an unlocked OS credential service is available. + `src/providers/key-store.ts:95-107` fails closed on a locked or headless + session, and `docs-site/src/content/docs/reference/configuration/providers.md:656-662` + already states that limitation - this page must not contradict it. +- Claude Code auto-connect by environment injection: macOS only, because it + writes to the launchd user domain. Linux has no single equivalent + (`systemctl --user set-environment` reaches only systemd units, `~/.profile` + only login shells, `~/.bashrc` only interactive non-login shells) and the + Windows equivalent would move a bearer token into a persistent registry hive. + Both are tracked in `050`. +- Meta Muse Code credential import: macOS only. Meta ships no native Windows CLI + (WSL2 is the documented route), and the Linux credential shape has not been + measured. +- Browser open, Cursor detection, Claude Desktop paths, Kiro credentials: all + three platforms, already. + +MODIFY `docs-site/astro.config.mjs`: the Reference group is manually enumerated +(`:125-153`), so a page absent from it is not discoverable. + +The entry form matters. Starlight resolves an internal `slug` per locale as +`/` and throws when the localized entry is missing, and this site +ships all seven locales complete - every locale directory carries the same 13 +reference pages. An English-only `slug` entry would therefore break the +localized build. + +A site-relative `link` does NOT avoid the problem. Starlight treats only +`http://` and `https://` as absolute (`utils/url.ts`), and +`linkFromSidebarLinkItem` (`utils/navigation.ts:121-127`) prefixes anything else +with the active locale - so `"/reference/platform-support"` becomes +`/fr/reference/platform-support`, a route that does not exist. Worse than the +`slug` case: a `link` is not build-validated, so the build gate would pass while +navigation is quietly broken. + +Two admissible options, and the PR must pick one explicitly: + +1. **Two files (preferred).** Use a genuinely absolute link built from the + config's own constant: `link: \`\${SITE_URL}/reference/platform-support\`` + (`SITE_URL` is already defined at `astro.config.mjs:7`). `isAbsoluteUrl` + returns true, so no locale prefix is injected and every locale points at the + canonical English page. +2. **Eight files.** Keep `slug: "reference/platform-support"` and add the page + for all seven locales - each locale directory already carries the same 13 + reference pages, so a missing one is a real gap. + +Option 1 ships first. Option 2 is a larger PR, not a tweak. + +## What this phase does NOT touch + +No runtime source. No GUI. No test behavior. The three shell-hook functions keep +their darwin gate, their reason strings, and their exact return shapes, so every +existing assertion stays green by construction. + +## Acceptance + +- `bun install --frozen-lockfile` then `bun run build` in `docs-site/` succeeds + (`docs-site/AGENTS.md:20-30`). +- The sidebar href is verified BY HAND for one non-English locale. The build does + not validate manual `link` entries, so "the build passed" is not evidence for + this specific risk. +- The page is reachable from the Reference sidebar. +- No claim contradicts `providers.md:656-662` on keyring availability. +- No source file outside `docs-site/` is modified. +- CI green. diff --git a/devlog/_plan/260904_cross_platform_parity/030_wp3_windows_identity_decode.md b/devlog/_plan/260904_cross_platform_parity/030_wp3_windows_identity_decode.md new file mode 100644 index 0000000000..7092f036e1 --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/030_wp3_windows_identity_decode.md @@ -0,0 +1,142 @@ +# 030 - wp3: Windows identity decode + +One PR. Bases on wp2's head branch (stacked child). +Branch `codex/260904-windows-identity-decode`. Evidence: `003`. +Revised after audit rounds 1 and 2. + +## Scope, after two audits + +Round 1 rejected the legacy-task refresh widening as a security hole: matching +command and launcher does not prove the task is ours, and a different user's task +could have been silently re-registered. + +Round 2 examined the replacement - resolve the reported name to a SID and require +equality with the current SID - and found it underspecified at a security +boundary: no API, no trusted execution channel, no SID validation, no rule for +prefixed, duplicated or mixed `` elements, and a name flowing into a +command line is an injection surface. Round 2 did confirm the SID-equality IDEA +is sound (a mojibaked name resolving to a foreign account is safely rejected), +but sound-in-principle is not a specification. + +Writing that specification means designing a trusted principal-resolution channel +(`LookupAccountNameW`, or a static trusted PowerShell command receiving the name +strictly as data), with fail-closed rules for every malformed XML shape +`src/service.ts:2117-2136` already guards. That is its own phase, and it belongs +with someone who can make the trust decision. + +**So this PR ships the decode fix alone.** The auditor stated it is +independently correct, and it is the defect actually proven to exist in the tree. +The legacy migration moves to `050`. + +## The defect + +`src/lib/windows-user-principal.ts:141` and `:156` decode `powershell.exe` +stdout with a bare `Buffer.toString()`, which is UTF-8. Windows PowerShell 5.1 +emits the console output code page, so a non-ASCII account name becomes U+FFFD +mojibake and is frozen into the process identity cache by `identityFromResult` +(`:225`). + +Audit-confirmed: `decodeWindowsTextBytes` tries strict UTF-8 BEFORE the locale +code page (`src/lib/windows-text.ts:119-126`), so a UTF-8 host is unaffected. The +`` comparison itself is correct - case-insensitive, no lossy folding +(`src/service.ts:1992-2000, 2135-2136`). + +## MODIFY `src/lib/windows-user-principal.ts` + +### 1. The runner seam carries bytes + +```ts +export interface WindowsPrincipalLookupResult { + success: boolean; + exitCode: number | null; + timedOut: boolean; + /** Raw child stdout. Bytes, so the decode under test is the real one. */ + stdout: string | Uint8Array; +} +``` + +Audit-confirmed source-compatible: nothing outside the module reads `.stdout`, +and every test seam only CONSTRUCTS results (`service.test.ts:29`, +`responses-state.test.ts:124`, `lab-public-security-regressions.test.ts:229`, +`windows-secret-acl.test.ts`, `windows-user-principal.test.ts`, +`openai-provider-option-e2e.test.ts:283`, and the migration child fixture). + +This widening is what makes the bug testable at all: the current seam hands over +an already-decoded string, so the `Buffer.toString()` boundary can never be +exercised. A fix without it ships untested. + +### 2. Decode through the repository's own decoder, with an explicit locale seam + +```ts +let principalLocaleForTests: string | undefined; + +/** Test seam: pin the locale used to select the legacy code page. */ +export function setWindowsPrincipalLocaleForTests(locale: string | null): void { + // Same in-flight guard as the runner setters (:326-341): decoding happens + // AFTER the async runner resolves (:311), so a locale swapped mid-lookup would + // silently change that lookup's decode. + if (asyncLookupInFlight) { + throw new Error("Cannot change the Windows principal locale while a lookup is in flight."); + } + principalLocaleForTests = locale ?? undefined; + // Same contract as the runner setters (:323-341): a successful identity is + // returned from cache BEFORE any decode (:245-256, :296-299), so a locale + // change that left the cache intact would silently re-assert the first decode. + cachedIdentity = null; +} + +function decodePrincipalStdout(stdout: string | Uint8Array): string { + if (typeof stdout === "string") return stdout; + return decodeWindowsTextBytes( + stdout, + principalLocaleForTests ? { locale: principalLocaleForTests } : {}, + ); +} +``` + +The locale seam is REQUIRED, not a convenience. `decodeWindowsTextBytes` picks a +single legacy encoding from the active locale +(`src/lib/windows-text.ts:19-30, 70-77`), so one CI process cannot decode CP949, +CP932 and CP936 fixtures correctly without being told which to expect. Every +existing codec test already passes an explicit locale +(`tests/windows-text-decoding.test.ts:11,35,45`); this seam gives the principal +path the same determinism. Production passes nothing and keeps the ambient locale. + +`defaultWindowsPrincipalRunner` returns the Buffer unchanged; the async runner +returns bytes rather than `Response.text()`; `identityFromResult` decodes before +splitting lines. `SID_PATTERN` and `sid.toUpperCase()` are untouched - the SID is +ASCII by construction, which is why the corruption was silent. + +## What this fixes, stated exactly + +The EXPECTED side of every identity comparison, and `identity.name` for the ACL +comparisons at `src/lib/windows-secret-acl.ts:565,583`. It does not rescue a +task already registered with a name-form ``, because the REPORTED side is +separately mangled by schtasks. That migration is `050`. + +## Tests + +New `tests/windows-user-principal-nonascii.test.ts`, each guard driven RED first: + +1. CP949 bytes for `S-1-5-21-1-2-3-1001\r\nMACHINE\\\r\n`, locale seam + pinned to `ko-KR`, yield the exact account name, no U+FFFD. RED today. +2. Same for CP932 with `ja-JP` and CP936 with `zh-CN`. Each case pins its own + locale; without that the three fixtures are mutually exclusive in one process. +3. UTF-8 bytes decode identically under every pinned locale - the + strict-UTF-8-first guard that keeps ordinary hosts unaffected. +4. ASCII `EXAMPLE\\Owner` is byte-identical before and after. +5. A string-returning legacy runner still works, proving the widened type is + backward compatible. +6. A timed-out or failed lookup still throws `EACLIDENTITY`, unchanged. +7. The locale seam resets in `afterEach`, so no case leaks a locale into another + file's expectations. + +Focused run: `bun test tests/windows-user-principal-nonascii.test.ts`. +`tests/service.test.ts` is unchanged by this PR and needs no new case. + +## Acceptance + +- Each guard driven red before the fix; red output recorded in `004`. +- `bun x tsc --noEmit` clean. +- The PR references #3320 as a CANDIDATE cause and does not say `Closes`. +- CI green, Windows leg specifically. diff --git a/devlog/_plan/260904_cross_platform_parity/040_wp4_stack_closeout.md b/devlog/_plan/260904_cross_platform_parity/040_wp4_stack_closeout.md new file mode 100644 index 0000000000..a325edcb46 --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/040_wp4_stack_closeout.md @@ -0,0 +1,41 @@ +# 040 - wp4: stack close-out (administrative, NOT a fourth PR) + +This unit ships exactly THREE pull requests: wp1, wp2, wp3. wp4 opens no fourth +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. + +## 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`. +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. +4. Record the outcome in `004_implementation_outcome.md`: what landed, what review + changed, what the plan got wrong. This is a devlog-only commit on the last + child branch, never a new PR. +5. Confirm `docs-site/` matches shipped behavior. wp2 adds the platform-support + page; wp1 changes the meta-muse refusal wording. English source only, and no + claim may contradict + `docs-site/src/content/docs/reference/configuration/providers.md`. + +## Verification stance + +The user forbade running the full local suite, so CI is the verification +authority for this unit. Each phase names its focused test file; the suite-wide +answer comes from the GitHub Actions run on the PR. A phase may not claim a green +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 + `.github/PULL_REQUEST_TEMPLATE.md`. No fourth PR exists. +- CI conclusion captured per PR as goalplan evidence. +- `004` written. +- `050` lists every deliberate follow-up with its reason. diff --git a/devlog/_plan/260904_cross_platform_parity/050_followups.md b/devlog/_plan/260904_cross_platform_parity/050_followups.md new file mode 100644 index 0000000000..74f29c872e --- /dev/null +++ b/devlog/_plan/260904_cross_platform_parity/050_followups.md @@ -0,0 +1,128 @@ +# 050 - Follow-ups this unit deliberately does not do + +## Linux Claude-Code auto-connect via a shell env file (deferred after audit round 2) + +`020` originally proposed this and it collected six blockers in one audit round. +They are recorded in `020` because the pattern matters more than any single +finding: it is not a platform guard to delete, it is a credential-bearing file +lifecycle on a new platform. + +What the unit has to specify before any code: + +- Where the Linux branch computes `modelEnv` and `auto`, which today exist only + AFTER the darwin early return (`src/server/system-env.ts:436-438` vs `:372`). +- Ownership and cleanup for graceful stop, toggle-off, uninstall, and stale or + crash recovery. `revertSystemEnv` returns immediately off darwin (`:488-490`), + toggle-off follows the same path (`:483-485`), and `cleanStaleSystemEnv` + delegates to it (`:521-536`), so a written file would currently have no + reaper and could keep pointing at a stopped proxy. +- A result type both callers can read: they append + `.catch(() => ({ injected: false }))` (`src/cli/index.ts:443,537`) and + `SystemEnvResult` is not exported. +- Real permission enforcement. `writeFileSync(..., { mode: 0o600 })` sets the + mode at creation only and does not tighten an existing 0644 file. +- GUI capability semantics. The control is gated on `autoConnectSupported` + (`gui/src/pages/claude-autoconnect.ts:10-13`, + `claude-code-sections.tsx:88-92`), so a new API field alone changes nothing + visible. +- An update to `tests/claude-shell-hook.test.ts:180-184`, which asserts the exact + source shape `reconcileShellHook(systemEnv.injected)` twice. +- The security review AGENTS.md requires: the file can contain + `ANTHROPIC_AUTH_TOKEN` (`src/server/system-env.ts:95-100`), so this writes a + bearer token to a new platform's disk. + +## Legacy name-form scheduler task migration (deferred after audit round 2) + +`030` ships the decode fix alone. Migrating a task whose `` is name form +needs an authoritative name-to-SID resolution requiring equality with the current +user's SID - the idea is sound, and audit round 2 confirmed a mojibaked name +resolving to a foreign account is safely rejected by SID inequality. What is +missing is the specification: the resolver API, a trusted execution channel +(`LookupAccountNameW`, or a static trusted command taking the name strictly as +DATA so it cannot become an injection surface), strict SID validation, and +fail-closed rules for prefixed, duplicated and mixed `` elements of the +kind `src/service.ts:2117-2136` already guards. Tests must cover +mixed-current/foreign, duplicate, prefixed, metacharacter, and failed-lookup +cases. + +## Env injection on Windows + +`injectSystemEnv` / `revertSystemEnv` (`src/server/system-env.ts:372,489`) stay +darwin-only; the Linux half is the entry above. A Windows port writes +`ANTHROPIC_AUTH_TOKEN` into `HKCU\\Environment`, +moving a bearer token from a per-boot launchd domain into a persistent registry +hive readable by every process in the session. AGENTS.md routes credential +handling to explicit security review, and that is a maintainer decision. + +The design it would need: a backend interface (`get`/`set`/`unset`) with launchd, +registry, and shell-file implementations, preserving the tracking file, the +user-wins lever rule, ownership-mismatch refusal, and resumable partial rollback +described in `001`. On Windows the registry write must be followed by a +`WM_SETTINGCHANGE` broadcast with `lParam="Environment"`, or only newly spawned +processes see the change. `setx` is the naive route but truncates at 1024 +characters. + +On Linux there is no single equivalent at all: `systemctl --user set-environment` +reaches only systemd-spawned units, `~/.profile` reaches login shells, +`~/.bashrc` reaches interactive non-login shells. That is the honest reason it +never shipped. `devlog/_fin/260723_issue_triage/030_fix_287_linux_autoconnect.md` +already scoped it. + +## WSL2 credential bridge for meta-muse + +`010` refuses on Windows with an accurate message instead of reading a WSL2 +pointer at `\\\\wsl$\\\\home\\\\.config\\muse\\auth.json`. Doing that +properly needs distro enumeration, Linux-user mapping, and a reachability probe, +none of which were measured. A guess would ship an unverified credential path, +which is exactly what `meta-muse.ts` refuses to do everywhere else. + +## Linux credential reader for meta-muse (blocked on a measurement) + +Audit round 1 rejected the drafted Linux reader: the pointer interface +(`src/oauth/meta-muse.ts:58-60`) has no path field and no inline-key field, so a +reader would have been written against invented schema. `010` therefore ships a +truthful Linux REFUSAL instead. + +What unblocks it is a measurement, not a decision. Someone with a Linux Muse Code +install needs to record, from a real `~/.config/muse/auth.json`: + +- the exact `storage` value the CLI writes there; +- whether the secret is inline in the pointer, in a sibling file, or in a keyring; +- if file-backed, the exact field naming that file, and the file's permissions; +- whether the CLI honors `XDG_CONFIG_HOME` on Linux. + +With those four facts `002` gets an evidence section and the reader is a small, +safe phase. Without them it is a guess wearing a validator. + +## Unifying the two schtasks decoders + +`003` records that `service.ts:902` (`decodeSchtasksOutput`) and +`service-manager-probe.ts` use different decoders for the same command's output. +Latent, not active: `/query /xml` emits UTF-16, which both handle. Worth +unifying on `decodeWindowsTextBytes`, but it is not the reported defect and +changing a decoder used across the service path deserves its own unit. + +## PowerShell counterpart for ocx-restart.sh + +`scripts/ocx-restart.sh` has no `.ps1` counterpart, so a Windows agent session has +no detached-restart helper and a proxy started during a turn dies with the turn. +Small and self-contained (`Start-Process -WindowStyle Hidden`, then poll +`runtime-port.json`), but it is a developer script outside the runtime and does +not belong in a stack about user-facing platform parity. + +## Structured reasons across the uninstall path (deferred, not mechanical) + +`ocx uninstall` decides whether a shell-hook refusal is benign by matching the +literal strings `"not macOS"` and `"not installed"` (`src/cli/index.ts:1242-1244`). +An earlier draft put a `skip` discriminant in `020`; audit round 3 removed it and +round 4 confirmed the reason. + +It is not mechanical. The union has to distinguish benign ABSENCE from genuine +FAILURE - `no HOME` and `read/write failed` must never become benign skips - and +four exact `toEqual` assertions pin the current return objects +(`tests/claude-shell-hook.test.ts:63,79,107,173`), so every one of them changes +with it. + +It belongs with the port that needs it: the Linux env-file unit above is what +introduces a second backend and therefore a second reason vocabulary. Doing it +standalone changes call-site semantics and four tests to buy nothing.