From 12dc07beb010bbfc7f64086a4d504fdc807c6673 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 18 Sep 2026 05:11:09 +0900 Subject: [PATCH 1/2] fix(oauth): keep Devin migration account-bound Inspect both the config and auth source/destination slots before persisting either half of the devin-cli -> devin startup migration. A refused config projection or an occupied destination credential slot now leaves the provider row and both credential slots untouched, closing the cross-account credential-binding window. Both account-bound files are snapshotted before either is written. --- .../devin-provider-merge-migration.ts | 45 ++++++++++++++----- structure/providers/xai-grok.md | 2 + structure/runtime.md | 2 + structure/subagents.md | 2 + structure/transports/inventory.md | 2 + .../devin-provider-merge-migration.test.ts | 36 +++++++++++++-- 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/src/providers/devin-provider-merge-migration.ts b/src/providers/devin-provider-merge-migration.ts index 746e346143e..647763e6571 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 10439313118..00eaeef4e51 100644 --- a/structure/providers/xai-grok.md +++ b/structure/providers/xai-grok.md @@ -159,3 +159,5 @@ Shared response-log retention and native SSE inspection pacing follow the [bound Native steering retains fixed phase deadlines and reconciled replay output; see the [steering stability contract](../transports/streaming-health.md#steering-deadlines-and-replay-completeness). Native steering generation overrides, explicit public-API eligibility and the consent-gated wire probe follow the [shared control contract](../transports/streaming-health.md#steering-settings-public-api-and-diagnostic-probe); this owner does not change routing or execute diagnostic tools. + +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 186b4549811..7e7708f3cc7 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -473,6 +473,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 3ee59b82c67..1796bec2313 100644 --- a/structure/subagents.md +++ b/structure/subagents.md @@ -394,3 +394,5 @@ Shared response-log retention and native SSE inspection pacing follow the [bound Native steering retains fixed phase deadlines and reconciled replay output; see the [steering stability contract](transports/streaming-health.md#steering-deadlines-and-replay-completeness). Native steering generation overrides, explicit public-API eligibility and the consent-gated wire probe follow the [shared control contract](transports/streaming-health.md#steering-settings-public-api-and-diagnostic-probe); this owner does not change routing or execute diagnostic tools. + +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 10670c2eb1d..0d8884bc190 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -160,3 +160,5 @@ Shared response-log retention and native SSE inspection pacing follow the [bound Native steering retains fixed phase deadlines and reconciled replay output; see the [steering stability contract](../transports/streaming-health.md#steering-deadlines-and-replay-completeness). Native steering generation overrides, explicit public-API eligibility and the consent-gated wire probe follow the [shared control contract](streaming-health.md#steering-settings-public-api-and-diagnostic-probe); this owner does not change routing or execute diagnostic tools. + +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 9f89f2405aa..a9caf504703 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 { From fff0cca9ef23f4db7db9d7cfe548e0562563e806 Mon Sep 17 00:00:00 2001 From: lidge-jun Date: Fri, 18 Sep 2026 10:02:36 +0900 Subject: [PATCH 2/2] docs(structure): keep the Devin migration contract inside the runtime.md budget structure/runtime.md is exactly at the 600-line structure-ssot budget on dev and is not in grace.oversizeDocs, so the two added lines fail that gate. CI confirmed it: test 4/4 reported "structure/runtime.md is 602 lines, over the 600-line budget". Fold the new contract into the adjacent Devin paragraph, which is the same subject, instead of adding a paragraph. --- structure/runtime.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/structure/runtime.md b/structure/runtime.md index 900915449da..ffa48154496 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -479,9 +479,7 @@ Cline CLI joins the existing export/client integration registries. Explicit CLI Config JSON preserves the boolean; only literal true activates the role-changing transform. The lightweight top-level CLI help counts Cline CLI among the fifteen registered export clients; registry parity remains covered by the client help and integration tests. -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. +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 its 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.