diff --git a/src/providers/devin-provider-merge-migration.ts b/src/providers/devin-provider-merge-migration.ts index 746e346143..647763e657 100644 --- a/src/providers/devin-provider-merge-migration.ts +++ b/src/providers/devin-provider-merge-migration.ts @@ -195,33 +195,54 @@ const DEFAULT_DEPS: DevinProviderMergeStartupDeps = { * the snapshot is taken strictly before the save, and a backup failure throws * rather than writing without a rollback point. * - * The auth half is deliberately detached. `startServer` is synchronous — an - * `await` in the boot window would suspend the composition root — and + * Both destination slots are inspected before either account-bound file is + * changed. The auth write itself is deliberately detached. `startServer` is + * synchronous — an `await` in the boot window would suspend the composition root — and * `mutateStore` is async-only, so the rekey is fired after its snapshot and - * its outcome is logged when it lands. That is safe here: the credential is - * valid under either slot name while the `devin-cli` alias exists, a conflict - * refuses by design, and a failed rekey simply retries on the next boot. + * its outcome is logged when it lands. A late concurrent conflict refuses by + * design, and a failed rekey simply retries on the next boot. */ export function runDevinProviderMergeStartupMigration( config: OcxConfig, deps: DevinProviderMergeStartupDeps = DEFAULT_DEPS, ): OcxConfig { const projection = deps.project(config); - // Warnings are emitted even on a no-op: the collision case IS the warning. + const hasSourceConfig = config.providers?.[FROM_ID] !== undefined; + const hasSourceAuth = deps.hasAuthSlot(FROM_ID); + const hasDestinationAuth = deps.hasAuthSlot(TO_ID); + + // A configured provider and its credentials are one account-bound unit. Do + // not move either half if the config projection refused, or if the target + // credential slot could belong to another account. + if (hasSourceConfig && (!projection.changed || hasDestinationAuth)) { + // Projection warnings still matter on a no-op: a config collision is the warning. + for (const warning of projection.warnings) console.warn(`[devin-provider-merge] ${warning}`); + if (projection.changed && hasDestinationAuth) { + console.warn( + `[devin-provider-merge] auth.json already has a "${TO_ID}" credential slot; ` + + `provider "${FROM_ID}" and both credential slots were left untouched. Remove the ` + + "unused destination credential manually, then restart.", + ); + } + return config; + } + for (const warning of projection.warnings) console.warn(`[devin-provider-merge] ${warning}`); + let result = config; if (projection.changed) { + // Snapshot both account-bound files before changing either one. + if (hasSourceAuth) deps.backupAuth(); deps.backupConfig(); deps.save(projection.config); result = projection.config; } - // The auth rekey runs even when the config half refused or had nothing to - // do: a `devin-cli` credential slot is orphaned state regardless of whether - // a provider row still points at it, and the conflict check inside the - // rekey is the same refuse-on-occupied rule the config half applies. - if (!deps.hasAuthSlot(FROM_ID)) return result; - deps.backupAuth(); + // With no legacy config row, a `devin-cli` credential slot is orphaned and + // can still be rekeyed under the helper's refuse-on-occupied rule. A refused + // config migration returned above so its account-bound slot stays put. + if (!hasSourceAuth) return result; + if (!projection.changed) deps.backupAuth(); void deps.rekey(FROM_ID, TO_ID).then(outcome => { if (outcome === "conflict") { console.warn( diff --git a/structure/providers/xai-grok.md b/structure/providers/xai-grok.md index 6213ce1449..30742c36b4 100644 --- a/structure/providers/xai-grok.md +++ b/structure/providers/xai-grok.md @@ -136,3 +136,5 @@ Pool quota producers and account commands follow the [bounded raw-observation co Account quota surfaces use [safe probe diagnostics](../transports/inventory.md#account-quota-failure-diagnostics) separately from quota validity, credential health and routing authority. Live sideband admission and its bounded upstream handshake follow the [runtime contract](../runtime.md#live-sideband-handshake); the ordinary Responses WebSocket exchange remains separate. + +Shared startup provider-id migration preserves the account binding between configuration and OAuth credentials; see the [runtime contract](../runtime.md). diff --git a/structure/runtime.md b/structure/runtime.md index a09d007d7b..4daa0ba59d 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -332,6 +332,8 @@ The lightweight top-level CLI help counts Cline CLI among the fifteen registered Devin CLI credential path composition in `src/oauth/devin/cli-import.ts` follows the selected platform: Windows uses Win32 APPDATA paths, other platforms use POSIX XDG-data paths. The explicit absolute override remains verbatim; credential parsing and login behavior are unchanged. +The `src/providers/devin-provider-merge-migration.ts` startup migration treats the legacy provider row and OAuth slot as one account-bound unit: an occupied destination or a refused config projection leaves both unchanged, and both backups complete before either file changes. + Native Chat applies qualifying effort ceilings independently of model pins; pin selection precedes the cap and only pins or cap rewrites enter wire mapping. The [catalog effort contract](catalog.md#ultra-reasoning-level) records the V1/compaction exemptions and caller-preservation boundary. Pool quota producers and account commands follow the [bounded raw-observation contract](providers/openai-tiers.md#bounded-pool-quota-observations), separate from the latest display snapshot and capacity estimates. diff --git a/structure/subagents.md b/structure/subagents.md index 69047b076b..5aa8295601 100644 --- a/structure/subagents.md +++ b/structure/subagents.md @@ -371,3 +371,5 @@ Exact [model input declarations](config.md#explicit-per-model-capability-declara Provider-scoped approval reviewer settings are projected by the [catalog owner](catalog.md#provider-scoped-approval-reviewer); this surface retains its existing routing, transport and account-selection behavior. Renamed fixed-key providers receive [missing reasoning metadata](catalog.md#renamed-destination-reasoning-metadata) during derivation; explicit per-model entries and provider defaults retain precedence. + +Startup provider-id migration preserves the account binding between configuration and OAuth credentials; see the [runtime contract](runtime.md). diff --git a/structure/transports/inventory.md b/structure/transports/inventory.md index 10b5be6a6b..f649dd6646 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -142,3 +142,5 @@ The [explicit model-capability contract](../config.md#explicit-per-model-capabil Provider-scoped approval reviewer settings are projected by the [catalog owner](../catalog.md#provider-scoped-approval-reviewer); this surface retains its existing routing, transport and account-selection behavior. Renamed fixed-key providers receive [missing reasoning metadata](../catalog.md#renamed-destination-reasoning-metadata) during derivation; explicit per-model entries and provider defaults retain precedence. + +Startup provider-id migration preserves the account binding between configuration and OAuth credentials; see the [runtime contract](../runtime.md). diff --git a/tests/providers/devin-provider-merge-migration.test.ts b/tests/providers/devin-provider-merge-migration.test.ts index 9f89f2405a..a9caf50470 100644 --- a/tests/providers/devin-provider-merge-migration.test.ts +++ b/tests/providers/devin-provider-merge-migration.test.ts @@ -129,7 +129,7 @@ describe("devin provider merge startup runner", () => { backupConfig: () => { order.push("backupConfig"); }, backupAuth: () => { order.push("backupAuth"); }, save: () => { order.push("save"); }, - hasAuthSlot: () => opts.hasAuthSlot ?? false, + hasAuthSlot: (provider: string) => provider === "devin-cli" && (opts.hasAuthSlot ?? false), rekey: async (from: string, to: string) => { order.push(`rekey:${from}->${to}`); return opts.rekey ? opts.rekey() : "moved" as const; }, }; } @@ -137,7 +137,7 @@ describe("devin provider merge startup runner", () => { test("snapshots config strictly before saving, and rekeys the auth slot", async () => { const order: string[] = []; const result = runDevinProviderMergeStartupMigration(migratableConfig(), depsWith(order, { hasAuthSlot: true })); - expect(order.slice(0, 2)).toEqual(["backupConfig", "save"]); + expect(order.slice(0, 3)).toEqual(["backupAuth", "backupConfig", "save"]); expect(order).toContain("backupAuth"); expect(order).toContain("rekey:devin-cli->devin"); expect(result.providers!['devin']).toBeDefined(); @@ -178,13 +178,41 @@ describe("devin provider merge startup runner", () => { expect(warnings.join(" ")).toContain("[devin-provider-merge]"); }); - test("a rekey conflict warns rather than throwing out of startup", async () => { + test("an auth destination collision refuses both halves of the migration", () => { const order: string[] = []; const warnings: string[] = []; const originalWarn = console.warn; console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; try { - runDevinProviderMergeStartupMigration(migratableConfig(), depsWith(order, { hasAuthSlot: true, rekey: async () => "conflict" })); + const deps = depsWith(order, { hasAuthSlot: true }); + deps.hasAuthSlot = provider => provider === "devin-cli" || provider === "devin"; + const config = migratableConfig(); + const result = runDevinProviderMergeStartupMigration(config, deps); + expect(result).toBe(config); + } finally { + console.warn = originalWarn; + } + expect(order).toEqual([]); + expect(warnings.join(" ")).toContain('auth.json already has a "devin" credential slot'); + }); + + test("a config collision never independently rekeys credentials", () => { + const order: string[] = []; + const config = migratableConfig(); + config.providers!["devin"] = { adapter: "devin" } as never; + runDevinProviderMergeStartupMigration(config, depsWith(order, { hasAuthSlot: true })); + expect(order).toEqual([]); + }); + + test("a late rekey conflict warns rather than throwing out of startup", async () => { + const order: string[] = []; + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { warnings.push(args.map(String).join(" ")); }; + try { + const deps = depsWith(order, { hasAuthSlot: true, rekey: async () => "conflict" }); + deps.hasAuthSlot = provider => provider === "devin-cli"; + runDevinProviderMergeStartupMigration(migratableConfig(), deps); // The detached promise needs a real tick, not one microtask. await new Promise(resolve => setTimeout(resolve, 0)); } finally {