diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 458bb67e0a..261aece1d1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -67,6 +67,14 @@ jobs: bump-dev-version: needs: publish if: ${{ inputs.dry-run != true }} + # A reusable-workflow CALL cannot grant the callee more than the calling job holds, + # and GitHub refuses the whole run at startup when the called workflow's own job + # declares permissions the caller did not pass down ("startup_failure", runs + # 33615174183 / 33615177849 — the first dispatches since #3129 wired this call). + # The callee's job declares exactly these two; nothing else in this file gains them. + permissions: + contents: write + pull-requests: write uses: ./.github/workflows/dev-version-bump.yml with: released-version: v${{ inputs.version }} diff --git a/src/client/state.ts b/src/client/state.ts index 4711586d09..073016a205 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -5,7 +5,6 @@ import { getDefaultConfig, mutatePersistedConfig, readConfigDiagnostics, - saveConfig, } from "../config"; import type { OcxClientConnectionConfig } from "../types"; import { @@ -127,10 +126,7 @@ export function inspectClientRotationRecoveryGate( return { kind: "clean" }; } -export function commitClientConnection( - - state: OcxClientConnectionConfig, -): "committed" | "unchanged" { +export function commitClientConnection(state: OcxClientConnectionConfig): "committed" | "unchanged" { const outcome = mutatePersistedConfig(config => { const unchanged = config.runtimeRole === "client" && JSON.stringify(config.client) === JSON.stringify(state); @@ -139,20 +135,8 @@ export function commitClientConnection( config.client = structuredClone(state); } return { changed: !unchanged, value: undefined }; - }); + }, { createIfMissing: getDefaultConfig }); if (outcome.status === "committed" || outcome.status === "unchanged") return outcome.status; - if (outcome.status === "unavailable" && outcome.reason === "missing") { - // First ocx run on a fresh machine: ocx connect is the expected first command in - // client mode, so there is no config.json yet. mutatePersistedConfig correctly - // refuses to invent one (a lost config must fail closed), but a genuinely absent - // file is the bootstrap case, not corruption — seed defaults plus the client - // block atomically. Found on the first MacBook↔oracle dogfood connect. - const seeded = getDefaultConfig(); - seeded.runtimeRole = "client"; - seeded.client = structuredClone(state); - saveConfig(seeded); - return "committed"; - } throw new Error(`client state commit unavailable: ${"reason" in outcome ? outcome.reason : "unknown"}`); } diff --git a/src/config.ts b/src/config.ts index 0641f772eb..589dc0c29d 100644 --- a/src/config.ts +++ b/src/config.ts @@ -3000,8 +3000,19 @@ export type PersistedConfigMutationOutcome = | { status: "committed" | "unchanged"; value: T } | { status: "unavailable"; reason: "missing" | "invalid" | "conflict" }; +export type PersistedConfigMutationOptions = { + /** Seed a genuinely absent config while holding the shared mutation lock. */ + createIfMissing?: () => OcxConfig; +}; + const CONFIG_MUTATION_MAX_REBASE_ATTEMPTS = 3; let persistedConfigMutationBeforeCommitForTests: (() => void) | null = null; +let persistedConfigMutationBeforeLockForTests: (() => void) | null = null; + +/** Test-only one-shot seam: inject a competing creation after observation, before lock acquisition. */ +export function setPersistedConfigMutationBeforeLockForTests(hook: (() => void) | null): void { + persistedConfigMutationBeforeLockForTests = hook; +} /** Test-only one-shot seam: inject a competing mutation after the first decision, before freshness revalidation. */ export function setPersistedConfigMutationBeforeCommitForTests(hook: (() => void) | null): void { @@ -3017,22 +3028,35 @@ function unavailableConfigMutationReason(snapshot: ConfigFileSnapshot): "missing * serialized; the callback is rerun on the newest snapshot so observed direct byte changes rebase * and credential predicates are re-evaluated immediately before the atomic commit. A writer that * ignores the coordinator can still change bytes after the final check because the filesystem has - * no portable conditional rename. Missing or malformed config always fails closed and is never - * recreated from a prior snapshot. + * no portable conditional rename. Malformed config always fails closed. Callers may explicitly + * seed a missing config; the factory and mutation then run under this same transaction. */ export function mutatePersistedConfig( mutate: (config: OcxConfig) => PersistedConfigMutation, + options?: PersistedConfigMutationOptions, ): PersistedConfigMutationOutcome { // Avoid creating/opening the coordinator database for a read-path update that already knows // there is no valid config. The same check runs again under the transaction for authority. const observed = readConfigFileSnapshot(); if (observed.diagnostics.source !== "file" || observed.raw === undefined) { - return { status: "unavailable", reason: unavailableConfigMutationReason(observed) }; + const reason = unavailableConfigMutationReason(observed); + if (reason !== "missing" || !options?.createIfMissing) { + return { status: "unavailable", reason }; + } } + const beforeLockHook = persistedConfigMutationBeforeLockForTests; + persistedConfigMutationBeforeLockForTests = null; + beforeLockHook?.(); return withConfigMutationLockSync(() => { let base = readConfigFileSnapshot(); for (let attempt = 0; attempt < CONFIG_MUTATION_MAX_REBASE_ATTEMPTS; attempt += 1) { if (base.diagnostics.source !== "file" || base.raw === undefined) { + if (unavailableConfigMutationReason(base) === "missing" && options?.createIfMissing) { + const seeded = options.createIfMissing(); + const result = mutate(seeded); + if (persistConfigUnlocked(seeded)) bumpGenerationForCooperatingConfigWrite(); + return { status: "committed", value: result.value }; + } return { status: "unavailable", reason: unavailableConfigMutationReason(base) }; } diff --git a/tests/client-connect.test.ts b/tests/client-connect.test.ts index 313e2d1d1b..2beac9792d 100644 --- a/tests/client-connect.test.ts +++ b/tests/client-connect.test.ts @@ -12,7 +12,13 @@ import { issueClientKey, normalizeHubOrigin, } from "../src/client/hub-client"; +import { commitClientConnection } from "../src/client/state"; import { handleConnectCommand } from "../src/cli/connect"; +import { + getDefaultConfig, + saveConfig, + setPersistedConfigMutationBeforeLockForTests, +} from "../src/config"; import { removeTreeWithRetry } from "./helpers/remove-tree"; const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); @@ -285,6 +291,51 @@ function runTransactionScenario(stage: "success" | "catalog" | "preflight" | "co } describe("connect transaction and offline disconnect", () => { + test("fresh connect rebases onto a config created before bootstrap lock acquisition", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-client-bootstrap-race-")); + const previous = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + try { + const winner = getDefaultConfig(); + winner.port = 24444; + winner.hostname = "127.0.0.2"; + const admissionKey = `ocx_data_${"a".repeat(40)}`; + winner.apiKeys = [{ + id: "preserved-key", + name: "Preserved admission key", + key: admissionKey, + createdAt: "2026-08-28T00:00:00.000Z", + }]; + setPersistedConfigMutationBeforeLockForTests(() => saveConfig(winner)); + + expect(commitClientConnection({ + serverUrl: "https://hub.example.test", + managementUrl: "https://hub.example.test", + managementTransport: "direct", + selectedClients: ["claude"], + tokenEnv: "OPENCODEX_API_AUTH_TOKEN", + apiKeyId: "client-key-1", + tokenFingerprint: "fingerprint", + protocolVersion: 1, + connectedAt: "2026-08-28T00:00:00.000Z", + })).toBe("committed"); + + const persisted = JSON.parse(readFileSync(join(home, "config.json"), "utf8")); + expect(persisted).toMatchObject({ + port: 24444, + hostname: "127.0.0.2", + apiKeys: [{ id: "preserved-key", key: admissionKey }], + runtimeRole: "client", + client: { apiKeyId: "client-key-1" }, + }); + } finally { + setPersistedConfigMutationBeforeLockForTests(null); + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + removeTreeWithRetry(home); + } + }); + test("commits key id/state last, zeroes authority, and disconnects with the hub offline", () => { const run = runTransactionScenario("success"); try {