From fc78bc37d419576061b995281baf39c46655eaa9 Mon Sep 17 00:00:00 2001 From: Joonsuh Park Date: Mon, 7 Sep 2026 20:45:16 +0900 Subject: [PATCH] fix(init): explain configuration publication recovery --- .../010_implementation.md | 28 +++++++++++ .../docs/getting-started/quickstart.md | 28 +++++++++++ src/config/initialize.ts | 13 +++-- structure/02_config-and-codex-home.md | 6 +++ tests/config/config-mutation-lock.test.ts | 40 +++++++++++---- tests/service/init-eof.test.ts | 49 +++++++++++++++++++ 6 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 devlog/_plan/260907_init_publication_guidance/010_implementation.md diff --git a/devlog/_plan/260907_init_publication_guidance/010_implementation.md b/devlog/_plan/260907_init_publication_guidance/010_implementation.md new file mode 100644 index 0000000000..80b5954924 --- /dev/null +++ b/devlog/_plan/260907_init_publication_guidance/010_implementation.md @@ -0,0 +1,28 @@ +# Issue #3893: implementation plan + +Satisfy-spec work, triggered by issue #3893 and the request to implement separate draft PRs. Goal: actionable first-run publication diagnostics. Non-goals: changing file writes, permissions, replacement/cleanup guarantees, or adding a filesystem fallback. Stop after a verified draft PR; unresolved platform checks are reported, never marked passed. Escalate if resolving the issue requires weakening publication guarantees. This file records the plan and eventual evidence. + +Class C2: diagnostic propagation and user documentation. One independent branch from 522ce5f8c; no branch dependencies or orchestration state changes. + +File map: +- MODIFY src/config/initialize.ts: add an optional hardeningFailed flag to constructor options; select a fixed privacy-safe permission diagnostic when hardening throws. Track the flag around the existing harden call only, and pass it in the existing error options. Append supported-location guidance to denied-link diagnostics. All I/O order and cleanup remain identical. +- MODIFY tests/config/config-mutation-lock.test.ts: inject a harden failure and prove write/link never happen, target remains absent, no residue remains, and raw error details do not appear. Assert all five denied-link codes provide recovery guidance while retaining uncertain-publication state. Partial-write errors must not be mislabeled as permission failures. +- MODIFY tests/service/init-eof.test.ts: use its existing child bootstrap seam to inject publication errors during the real CLI wizard; verify exit=1, diagnostics and residue warnings, no configuration/backup damage or integration prompts. +- MODIFY docs-site/src/content/docs/getting-started/quickstart.md and structure/02_config-and-codex-home.md: explain supported locations, inspection before retry, separate permission and link failures, and fresh-install OPENCODEX_HOME examples. Existing translations reviewed for contradictions. + +Optional constructor input chain: created by the publication function; consumed by Error.message; no config serialization, migration, or persistent state. Existing constructor calls keep their meaning. + +Verification: focused config/init tests read the real publication and CLI code; typecheck includes src; privacy scan; required docs-site build. Baseline focused run: 35 pass, 3 skip, 2 fail (Windows file-symlink privilege: symlinkSync EPERM and dependent missing-residue assertion). No baseline failure will be hidden by changing tests. New regression checks must pass. Windows-native filesystem support remains bounded by the host. + +Audit: direct O_EXCL and replacement fallbacks rejected because they change complete-file/no-replace guarantees. Reuse the existing error and test seams; no new diagnostic module. Guidance never prints raw cause text or candidate bytes. + +## Verification before draft publication + +- `bun install --frozen-lockfile`: passed; lockfile unchanged. +- New diagnostics were observed failing before implementation: 9 failures across the focused hardening/link/CLI fault cases. After implementation: 9 passed. +- `bun test tests/config/config-mutation-lock.test.ts tests/service/init-eof.test.ts`: 38 passed, 3 skipped, 2 failed. The same two tests failed on unchanged 522ce5f8c: file-symlink creation is denied on this Windows host, and the swapped-symlink test then lacks its expected residue. New recovery tests pass; no skips or weakened assertions were added. +- `bun run typecheck`: passed. +- `bun run privacy:scan`: passed. +- `cd docs-site; bun install --frozen-lockfile; bun run build`: passed, 425 pages. Translated quickstarts contain no conflicting recovery/fallback instructions. +- CLI fault scenarios verify exit=1, distinct permission/link messages, uncertain-publication/residue warnings, backup preservation and no integration prompts. Partial-write errors keep the generic diagnostic. +- No physical non-NTFS filesystem support is claimed. Maintainer review remains required; this is a draft handoff. diff --git a/docs-site/src/content/docs/getting-started/quickstart.md b/docs-site/src/content/docs/getting-started/quickstart.md index 1fdcf922d8..867a06cec1 100644 --- a/docs-site/src/content/docs/getting-started/quickstart.md +++ b/docs-site/src/content/docs/getting-started/quickstart.md @@ -36,6 +36,34 @@ stop setup without falling back to an overwrite. If publication or temporary-fil finish, inspect the config directory before retrying: a complete config or private temporary file may remain. +If setup reports that initial config permissions could not be secured, the filesystem or account +could not apply the required private permissions (NTFS ACLs on Windows). This happens before +config contents are written. A hard-link publication error is a separate failure: private +permissions were applied, but publishing the completed file failed or its outcome is uncertain. + +Inspect the selected config directory before retrying. Preserve any existing `config.json`; +do not delete it to force setup to proceed. For a fresh installation, choose a writable location +that supports both hard links and private permissions. A local NTFS directory is a suitable +Windows choice when your account can apply its ACLs. For example, select a new location in the +same terminal before running setup: + +```powershell +# Windows PowerShell: choose a fresh directory on a local NTFS volume. +$env:OPENCODEX_HOME = Join-Path $env:LOCALAPPDATA "opencodex-local" +ocx init +``` + +```sh +# macOS/Linux: choose a fresh directory on a filesystem with hard links and Unix permissions. +export OPENCODEX_HOME="$HOME/.opencodex-local" +ocx init +``` + +Use the same `OPENCODEX_HOME` for subsequent commands and the service that runs the proxy. +Changing this variable selects a separate configuration location; it does not migrate an existing +installation. Setup intentionally has no direct-write or replacing-rename fallback: creating an +exclusive file and then writing to it could expose partial config contents. + :::note[GPT-5.6 rollout entries] The current stable release seeds GPT-5.6 Sol/Terra/Luna for ChatGPT passthrough, OpenAI API-key, OpenRouter, and diff --git a/src/config/initialize.ts b/src/config/initialize.ts index 864b09b036..1358fcf513 100644 --- a/src/config/initialize.ts +++ b/src/config/initialize.ts @@ -15,10 +15,12 @@ export class InitialConfigPublicationError extends Error { readonly publication: PublicationState, readonly residualTemp: boolean, readonly hardLinkUnavailable: boolean, - options?: ErrorOptions, + options?: ErrorOptions & { hardeningFailed?: boolean }, ) { - super(hardLinkUnavailable - ? "Initial config requires hard-link publication; the filesystem or its permissions denied it." + super(options?.hardeningFailed + ? "Initial config permissions could not be secured. Choose an OPENCODEX_HOME location that supports private file permissions (NTFS ACLs on Windows), then rerun `ocx init`." + : hardLinkUnavailable + ? "Initial config requires hard-link publication; the filesystem or its permissions denied it. Inspect the config directory before retrying. Choose an OPENCODEX_HOME location that supports hard links and private file permissions, then rerun `ocx init`." : "Initial config publication did not finish.", options); this.name = "InitialConfigPublicationError"; } @@ -90,10 +92,13 @@ export function publishInitialConfigNoReplace( let failure: unknown; let failed = false; let hardLinkUnavailable = false; + let hardeningFailed = false; let residualTemp = false; try { fd = openSync(temp, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); + hardeningFailed = true; (io.harden ?? hardenInitialConfig)(fd, temp, target); + hardeningFailed = false; verifyPrivateTemp(fd, temp); (io.write ?? ((descriptor: number, value: string) => writeFileSync(descriptor, value, { encoding: "utf8" })))(fd, bytes); verifyPrivateTemp(fd, temp); @@ -126,7 +131,7 @@ export function publishInitialConfigNoReplace( } } if (failed || residualTemp) { - throw new InitialConfigPublicationError(publication, residualTemp, hardLinkUnavailable, { cause: failure }); + throw new InitialConfigPublicationError(publication, residualTemp, hardLinkUnavailable, { cause: failure, hardeningFailed }); } return !collided; } diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 9478343d19..572c73f25f 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -284,6 +284,12 @@ and publication followed by a later failure can leave a complete config or priva foreign winner's ownership under future uninstall; the existing ownership manifest and global CLI shim preflight keep their separate contracts. +Initial publication diagnostics distinguish required permission-hardening failures from denied +hard-link publication without exposing raw filesystem causes. Both identify `OPENCODEX_HOME` +as the supported-location recovery path; uncertain publication and cleanup warnings remain in +the CLI. The quickstart documents inspection before retry, private-permission requirements, +and fresh-location examples. Diagnostics do not introduce a fallback or alter file I/O ordering. + `src/config/paths.ts` is the single owner of `OPENCODEX_HOME` expansion and resolution. It exposes the config directory and `config.json` path and retains the existing cache rule: a relative home is resolved once for each distinct raw environment value, so a later working-directory change cannot diff --git a/tests/config/config-mutation-lock.test.ts b/tests/config/config-mutation-lock.test.ts index 06a18dd99d..56a4d74659 100644 --- a/tests/config/config-mutation-lock.test.ts +++ b/tests/config/config-mutation-lock.test.ts @@ -230,22 +230,38 @@ test("exclusive temp collision does not remove or modify somebody else's file", test("failed hardening occurs before candidate bytes are written", () => { let wrote = false; - expect(() => initializePersistedConfigIfMissing(config(), { - harden(_fd, temp) { - expect(readFileSync(temp, "utf8")).toBe(""); - throw new Error("ACL denied"); - }, - write() { wrote = true; }, - })).toThrow(InitialConfigPublicationError); + let linked = false; + let failure: unknown; + try { + initializePersistedConfigIfMissing(config(), { + harden(_fd, temp) { + expect(readFileSync(temp, "utf8")).toBe(""); + throw new Error("private ACL failure detail"); + }, + write() { wrote = true; }, + link() { linked = true; }, + }); + } catch (error) { failure = error; } + expect(failure).toBeInstanceOf(InitialConfigPublicationError); + expect((failure as Error).message).toContain("permissions could not be secured"); + expect((failure as Error).message).toContain("OPENCODEX_HOME"); + expect((failure as Error).message).not.toContain("private ACL failure detail"); + expect(failure).toMatchObject({ publication: "not-published", hardLinkUnavailable: false, residualTemp: false }); expect(wrote).toBe(false); + expect(linked).toBe(false); expect(existsSync(getConfigPath())).toBe(false); expect(initTemps()).toEqual([]); }); test("partial write failure removes only the unpublished temporary name", () => { - expect(() => initializePersistedConfigIfMissing(config(), { - write(fd, bytes) { writeFileSync(fd, bytes.slice(0, 10)); throw new Error("disk full"); }, - })).toThrow(InitialConfigPublicationError); + let failure: unknown; + try { + initializePersistedConfigIfMissing(config(), { + write(fd, bytes) { writeFileSync(fd, bytes.slice(0, 10)); throw new Error("disk full"); }, + }); + } catch (error) { failure = error; } + expect(failure).toBeInstanceOf(InitialConfigPublicationError); + expect((failure as Error).message).toBe("Initial config publication did not finish."); expect(existsSync(getConfigPath())).toBe(false); expect(initTemps()).toEqual([]); }); @@ -259,6 +275,10 @@ test.each(["EOPNOTSUPP", "ENOTSUP", "ENOSYS", "EXDEV", "EPERM"])("unsupported/de } catch (error) { expect(error).toBeInstanceOf(InitialConfigPublicationError); expect((error as InitialConfigPublicationError).hardLinkUnavailable).toBe(true); + expect((error as Error).message).toContain("OPENCODEX_HOME"); + expect((error as Error).message).toContain("private file permissions"); + expect((error as Error).message).not.toContain("do not print raw error"); + expect((error as Error).message).not.toContain("permissions could not be secured"); } expect(existsSync(getConfigPath())).toBe(false); expect(initTemps()).toEqual([]); diff --git a/tests/service/init-eof.test.ts b/tests/service/init-eof.test.ts index e098e5c56c..df863f13af 100644 --- a/tests/service/init-eof.test.ts +++ b/tests/service/init-eof.test.ts @@ -173,6 +173,55 @@ describe("ocx init piped stdin (#754)", () => { } finally { await stop(proc); } }, 30_000); + test.each(["permissions", "link", "link-residue"])("publication recovery guidance reaches the CLI (%s)", async failure => { + const home = makeHome(); + const backup = join(home, "config.json.pre-openai-tiers-v2.bak"); + writeFileSync(backup, "preserve backup on publication failure"); + const bootstrap = ` + import { mock } from "bun:test"; + const configApi = { ...await import("./src/config.ts") }; + const failure = ${JSON.stringify(failure)}; + const io = failure === "permissions" + ? { harden() { throw new Error("private permission detail"); } } + : { + link() { throw Object.assign(new Error("private link detail"), { code: "EPERM" }); }, + ...(failure === "link-residue" ? { unlink() { throw new Error("private cleanup detail"); } } : {}), + }; + mock.module("./src/config.ts", () => ({ + ...configApi, + initializePersistedConfigIfMissing(config) { + return configApi.initializePersistedConfigIfMissing(config, io); + }, + })); + const { runInit } = await import("./src/cli/init.ts"); + await runInit(); + `; + const proc = launch(home, "init", bootstrap); + const stderr = new Response(proc.stderr).text(); + try { + await reachPortPrompt(proc); + proc.stdin.write("21001\n"); + await proc.stdin.flush(); + const stdout = remainingOutput(proc.stdout); + expect(await proc.exited).toBe(1); + const diagnostic = await stderr; + expect(diagnostic).toContain("OPENCODEX_HOME"); + expect(diagnostic).toContain("ocx init"); + expect(diagnostic).not.toMatch(/fixture-init-key|private (permission|link|cleanup) detail/); + if (failure === "permissions") { + expect(diagnostic).toContain("permissions could not be secured"); + expect(diagnostic).not.toContain("Config may already exist"); + } else { + expect(diagnostic).toContain("hard-link publication"); + expect(diagnostic).toContain("Config may already exist; inspect it before retrying"); + } + expect(diagnostic.includes("A temporary file could not be removed")).toBe(failure === "link-residue"); + expect(await stdout).not.toMatch(/Inject into|autostart shim|Setup complete/); + expect(existsSync(join(home, "config.json"))).toBe(false); + expect(readFileSync(backup, "utf8")).toBe("preserve backup on publication failure"); + } finally { await stop(proc); } + }, 30_000); + // Windows process.kill does not deliver a POSIX SIGINT to readline. test.skipIf(process.platform === "win32")("SIGINT settles a pending prompt without creating config", async () => { const home = makeHome();