diff --git a/apps/server/openapi.json b/apps/server/openapi.json index a888f0b..1bc1a56 100644 --- a/apps/server/openapi.json +++ b/apps/server/openapi.json @@ -1300,6 +1300,33 @@ }, "required": ["type", "name", "provider"] }, + "previousAddress": { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "environment", + "vault", + "memory_store", + "skill", + "agent", + "template", + "deployment", + "file", + "identity", + "channel" + ] + }, + "name": { + "type": "string" + }, + "provider": { + "type": "string" + } + }, + "required": ["type", "name", "provider"] + }, "reason": { "type": "string" }, diff --git a/apps/webui/src/lib/api/generated/schema.d.ts b/apps/webui/src/lib/api/generated/schema.d.ts index f2461b5..373a037 100644 --- a/apps/webui/src/lib/api/generated/schema.d.ts +++ b/apps/webui/src/lib/api/generated/schema.d.ts @@ -731,6 +731,22 @@ export interface paths { name: string; provider: string; }; + previousAddress?: { + /** @enum {string} */ + type: + | "environment" + | "vault" + | "memory_store" + | "skill" + | "agent" + | "template" + | "deployment" + | "file" + | "identity" + | "channel"; + name: string; + provider: string; + }; reason: string; /** @enum {string} */ driftKind?: "none" | "local" | "remote" | "both"; diff --git a/packages/sdk/src/internal/executor/executor.ts b/packages/sdk/src/internal/executor/executor.ts index 95d3d56..208c76a 100644 --- a/packages/sdk/src/internal/executor/executor.ts +++ b/packages/sdk/src/internal/executor/executor.ts @@ -2,7 +2,7 @@ import { dirname, resolve } from "node:path"; import { UserError } from "../errors.ts"; import { computeComparableDesiredHash } from "../planner/comparable.ts"; import { getResourceDeclaration } from "../planner/declaration.ts"; -import { computeResourceHash } from "../planner/hasher.ts"; +import { computeReplacementFingerprint, computeResourceHash } from "../planner/hasher.ts"; import { buildReadinessBaseline } from "../planner/plan-semantics.ts"; import { ApiError, ConflictError } from "../providers/base-client.ts"; import { readComparableIfSupported } from "../providers/drift-support.ts"; @@ -229,8 +229,8 @@ async function executeAction(action: PlannedAction, provider: ResourceExecAdapte resource: action.address, message: `update ${action.address.type}.${action.address.name} (${action.address.provider}) — not found remotely, recreating`, }); - ctx.state.removeResource(action.address); - return executeActionInner({ ...action, action: "create" }, provider, ctx); + ctx.state.removeResource(action.previousAddress ?? action.address); + return executeActionInner({ ...action, action: "create", previousAddress: undefined }, provider, ctx); } } @@ -317,7 +317,8 @@ async function executeActionInner( } const isUpdate = action.action === "update"; - const existingId = isUpdate ? ctx.state.getResource(address)?.remote_id : undefined; + const priorAddress = action.previousAddress ?? address; + const existingId = isUpdate ? ctx.state.getResource(priorAddress)?.remote_id : undefined; let result: RemoteResource; @@ -652,7 +653,7 @@ async function executeActionInner( // The externally-managed marker is sticky: it survives applies and is only // cleared by removing the resource from state (`agents state rm` / destroy). - const priorResource = ctx.state.getResource(address); + const priorResource = ctx.state.getResource(priorAddress); ctx.state.setResource({ address, remote_id: result.id, @@ -669,9 +670,11 @@ async function executeActionInner( desired_readiness_baseline: buildReadinessBaseline(getResourceDeclaration(address, ctx.config)), remote_hash: remoteHash, remote_snapshot: remoteSnapshot, + replacement_fingerprint: computeReplacementFingerprint(address, ctx.config), drift_paths: [], drift_status: remoteHash ? "in_sync" : undefined, }); + if (action.previousAddress) ctx.state.removeResource(action.previousAddress); return adopted; } @@ -719,6 +722,16 @@ async function adoptOnConflict( // command, so fail with actionable guidance instead of the raw wire error. function nameReservedError(err: unknown, address: ResourceAddress, searchName: string): UserError { const detail = err instanceof ApiError ? err.message : String(err); + if ( + address.type === "channel" && + err instanceof ApiError && + err.responseBody.includes("CHANNEL_CREDENTIAL_CONFLICT") + ) { + return new UserError( + `${address.provider} rejected channel.${address.name} because its credentials are already used by another Channel. ` + + `Keep the existing Channel address so it can be updated in place, remove the old Channel first, or use a different credential set. (${detail})`, + ); + } return new UserError( `${address.provider} reported ${address.type} "${searchName}" already exists, but it could not be found remotely to adopt. ` + `This usually means it was recently deleted and the provider still reserves the name. ` + diff --git a/packages/sdk/src/internal/planner/hasher.ts b/packages/sdk/src/internal/planner/hasher.ts index 9a2781e..c9f107a 100644 --- a/packages/sdk/src/internal/planner/hasher.ts +++ b/packages/sdk/src/internal/planner/hasher.ts @@ -51,6 +51,14 @@ export async function computeResourceHash( return contentHash(decl); } +/** Stable, non-reversible identity hint for resources whose YAML key may change. */ +export function computeReplacementFingerprint(address: ResourceAddress, config: ProjectConfig): string | undefined { + if (address.type !== "channel") return undefined; + const decl = config.channels?.[address.name]; + if (!decl) return undefined; + return contentHash({ channel_type: decl.type, credentials: decl.credentials ?? {} }); +} + function resolveChannelReferenceIds( decl: { agent: string; identity?: string }, config: ProjectConfig, diff --git a/packages/sdk/src/internal/planner/planner.ts b/packages/sdk/src/internal/planner/planner.ts index 065a21c..4fa4e3d 100644 --- a/packages/sdk/src/internal/planner/planner.ts +++ b/packages/sdk/src/internal/planner/planner.ts @@ -10,7 +10,7 @@ import type { ExecutionPlan, PlannedAction } from "../types/plan.ts"; import type { ResourceAddress, StateFile } from "../types/state.ts"; import { addressKey } from "../types/state.ts"; import { getResourceDeclaration } from "./declaration.ts"; -import { computeResourceHash } from "./hasher.ts"; +import { computeReplacementFingerprint, computeResourceHash } from "./hasher.ts"; import { buildReadinessBaseline, classifyReadinessImpact, diffReadinessBaseline } from "./plan-semantics.ts"; export interface PlanOptions { @@ -218,9 +218,92 @@ export async function buildPlan( }); } + coalesceChannelRenames(actions, config, state); return { actions, diagnostics: diagnostics.getAll() }; } +/** + * A YAML key is a resource address, but changing that key should not force a + * remote Channel replacement when the old and new declarations form one + * unambiguous same-type pair. Retaining the remote id is especially important + * for messaging providers that allow a credential set to belong to only one + * Channel at a time. + */ +function coalesceChannelRenames(actions: PlannedAction[], config: ProjectConfig, state: StateFile): void { + const creates = actions.filter((action) => action.action === "create" && action.address.type === "channel"); + const deletes = actions.filter((action) => action.action === "delete" && action.address.type === "channel"); + const stateByAddress = new Map(state.resources.map((resource) => [addressKey(resource.address), resource])); + const matchedDeletes = new Set(); + + for (const create of creates) { + const desiredType = config.channels?.[create.address.name]?.type; + if (!desiredType) continue; + const desiredFingerprint = computeReplacementFingerprint(create.address, config); + const candidates = deletes.filter((deletion) => { + if (matchedDeletes.has(deletion) || deletion.address.provider !== create.address.provider) return false; + const prior = stateByAddress.get(addressKey(deletion.address)); + const snapshot = prior?.remote_snapshot as { channel_type?: unknown } | undefined; + if (snapshot?.channel_type !== desiredType) return false; + return !prior?.replacement_fingerprint || prior.replacement_fingerprint === desiredFingerprint; + }); + if (candidates.length !== 1) continue; + + const deletion = candidates[0]!; + const prior = stateByAddress.get(addressKey(deletion.address)); + const competingCreates = creates.filter( + (candidate) => + candidate !== create && + candidate.address.provider === create.address.provider && + config.channels?.[candidate.address.name]?.type === desiredType && + (!prior?.replacement_fingerprint || + computeReplacementFingerprint(candidate.address, config) === prior.replacement_fingerprint), + ); + if (competingCreates.length > 0) continue; + + create.action = "update"; + create.previousAddress = deletion.address; + create.before = deletion.before; + create.driftKind = "local"; + create.reason = `Channel key renamed from '${deletion.address.name}' (remote resource retained)`; + protectRenamedChannelDependencies(actions, stateByAddress, deletion, create); + matchedDeletes.add(deletion); + } + + for (let index = actions.length - 1; index >= 0; index--) { + if (matchedDeletes.has(actions[index]!)) actions.splice(index, 1); + } +} + +/** Do not delete the old Identity/Template when the Channel migration that releases it fails. */ +function protectRenamedChannelDependencies( + actions: PlannedAction[], + stateByAddress: Map, + deletion: PlannedAction, + replacement: PlannedAction, +): void { + const prior = stateByAddress.get(addressKey(deletion.address)); + const snapshot = prior?.remote_snapshot as { identity_id?: unknown; template_id?: unknown } | undefined; + const referencedIds = new Set( + [snapshot?.identity_id, snapshot?.template_id].filter((id): id is string => typeof id === "string"), + ); + if (referencedIds.size === 0) return; + + for (const action of actions) { + if ( + action.action !== "delete" || + (action.address.type !== "identity" && action.address.type !== "template") || + action.address.provider !== replacement.address.provider + ) { + continue; + } + const dependency = stateByAddress.get(addressKey(action.address)); + if (!dependency?.remote_id || !referencedIds.has(dependency.remote_id)) continue; + if (!action.dependencies.some((address) => addressKey(address) === addressKey(replacement.address))) { + action.dependencies.push(replacement.address); + } + } +} + /** Keep the old delivery resource alive when creating its new materialization fails. */ function deliveryReplacementAddress(address: ResourceAddress, graph: DependencyGraph): ResourceAddress | undefined { if (address.type !== "agent" && address.type !== "template") return undefined; diff --git a/packages/sdk/src/internal/state/state-manager.ts b/packages/sdk/src/internal/state/state-manager.ts index d4b404c..5666549 100644 --- a/packages/sdk/src/internal/state/state-manager.ts +++ b/packages/sdk/src/internal/state/state-manager.ts @@ -47,6 +47,7 @@ export class StateManager implements IStateManager { desired_readiness_baseline: r.desired_readiness_baseline as ResourceState["desired_readiness_baseline"], remote_hash: r.remote_hash as string | undefined, remote_snapshot: r.remote_snapshot, + replacement_fingerprint: r.replacement_fingerprint as string | undefined, drift_paths: r.drift_paths as string[] | undefined, drift_status: r.drift_status as ResourceState["drift_status"], })); diff --git a/packages/sdk/src/internal/types/dto.ts b/packages/sdk/src/internal/types/dto.ts index d550307..58ef38d 100644 --- a/packages/sdk/src/internal/types/dto.ts +++ b/packages/sdk/src/internal/types/dto.ts @@ -44,6 +44,8 @@ export type PlanReadinessImpact = z.infer; export const PlannedActionSchema = z.object({ action: ActionTypeSchema, address: ResourceAddressSchema, + /** Existing state address to retain when this action is an inferred logical rename. */ + previousAddress: ResourceAddressSchema.optional(), reason: z.string(), driftKind: DriftKindSchema.optional(), readinessImpact: PlanReadinessImpactSchema.optional(), diff --git a/packages/sdk/src/internal/types/state.ts b/packages/sdk/src/internal/types/state.ts index ab5a5a9..1395754 100644 --- a/packages/sdk/src/internal/types/state.ts +++ b/packages/sdk/src/internal/types/state.ts @@ -21,6 +21,8 @@ export interface ResourceState { desired_readiness_baseline?: ResourceReadinessBaseline; remote_hash?: string; remote_snapshot?: unknown; + /** Non-reversible declaration fingerprint used to infer safe logical renames. */ + replacement_fingerprint?: string; drift_paths?: string[]; drift_status?: "in_sync" | "drifted" | "missing" | "unchecked"; } diff --git a/packages/sdk/tests/unit/qoder-identity-channel.test.ts b/packages/sdk/tests/unit/qoder-identity-channel.test.ts index 6663221..1f69873 100644 --- a/packages/sdk/tests/unit/qoder-identity-channel.test.ts +++ b/packages/sdk/tests/unit/qoder-identity-channel.test.ts @@ -4,10 +4,13 @@ import { join } from "node:path"; import { executePlan } from "../../src/internal/executor/executor.ts"; import { buildDependencyGraph } from "../../src/internal/graph/dependency.ts"; import { buildPlan } from "../../src/internal/planner/planner.ts"; +import { ConflictError } from "../../src/internal/providers/base-client.ts"; import type { ProviderAdapter } from "../../src/internal/providers/interface.ts"; import { QoderAdapter } from "../../src/internal/providers/qoder/adapter.ts"; import { StateManager } from "../../src/internal/state/state-manager.ts"; import type { ProjectConfig } from "../../src/internal/types/config.ts"; +import type { ExecutionPlan } from "../../src/internal/types/plan.ts"; +import { contentHash } from "../../src/internal/utils/hash.ts"; import "../../src/internal/providers/all.ts"; function config(): ProjectConfig { @@ -59,6 +62,113 @@ describe("Identity and Channel declarations", () => { expect(actions.indexOf("template.assistant")).toBeLessThan(actions.indexOf("channel.dingtalk")); }); + test("plans an unambiguous Channel key rename as an in-place update", async () => { + const desired = config(); + desired.channels = { "chimp-dingtalk": desired.channels!.dingtalk! }; + const plan = await buildPlan(desired, { + resources: [ + { + address: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + remote_id: "channel_existing", + content_hash: "old-hash", + remote_snapshot: { channel_type: "dingtalk" }, + }, + ], + }); + + const channelActions = plan.actions.filter((action) => action.address.type === "channel"); + expect(channelActions).toEqual([ + expect.objectContaining({ + action: "update", + address: { type: "channel", name: "chimp-dingtalk", provider: "qoder" }, + previousAddress: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + }), + ]); + }); + + test("does not guess a Channel rename when more than one same-type destination exists", async () => { + const desired = config(); + desired.channels = { + "chimp-dingtalk": desired.channels!.dingtalk!, + "ops-dingtalk": { ...desired.channels!.dingtalk!, name: "Ops DingTalk" }, + }; + const plan = await buildPlan(desired, { + resources: [ + { + address: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + remote_id: "channel_existing", + content_hash: "old-hash", + remote_snapshot: { channel_type: "dingtalk" }, + }, + ], + }); + + const channelActions = plan.actions.filter((action) => action.address.type === "channel"); + expect(channelActions.map((action) => action.action)).toEqual(["create", "create", "delete"]); + expect(channelActions.every((action) => action.previousAddress === undefined)).toBe(true); + }); + + test("uses a stored credential fingerprint to identify a Channel rename among multiple destinations", async () => { + const desired = config(); + desired.channels = { + "chimp-dingtalk": desired.channels!.dingtalk!, + "ops-dingtalk": { + ...desired.channels!.dingtalk!, + credentials: { client_id: "ops-client", client_secret: "ops-secret" }, + }, + }; + const plan = await buildPlan(desired, { + resources: [ + { + address: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + remote_id: "channel_existing", + content_hash: "old-hash", + remote_snapshot: { channel_type: "dingtalk" }, + replacement_fingerprint: contentHash({ + channel_type: "dingtalk", + credentials: { client_id: "client", client_secret: "secret" }, + }), + }, + ], + }); + + const renamed = plan.actions.find((action) => action.address.name === "chimp-dingtalk"); + expect(renamed).toMatchObject({ + action: "update", + previousAddress: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + }); + expect(plan.actions.find((action) => action.address.name === "ops-dingtalk")?.action).toBe("create"); + }); + + test("keeps the old Channel dependencies when its in-place rename fails", async () => { + const desired = config(); + desired.channels = { "chimp-dingtalk": desired.channels!.dingtalk! }; + const plan = await buildPlan(desired, { + resources: [ + { + address: { type: "identity", name: "byoc", provider: "qoder" }, + remote_id: "idn_old", + content_hash: "old-identity-hash", + }, + { + address: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + remote_id: "channel_existing", + content_hash: "old-channel-hash", + remote_snapshot: { channel_type: "dingtalk", identity_id: "idn_old" }, + }, + ], + }); + + const identityDelete = plan.actions.find( + (action) => action.action === "delete" && action.address.type === "identity" && action.address.name === "byoc", + ); + expect(identityDelete?.dependencies).toContainEqual({ + type: "channel", + name: "chimp-dingtalk", + provider: "qoder", + }); + }); + test("keeps unsupported Provider capabilities isolated", async () => { const desired = config(); desired.providers = { claude: {} }; @@ -112,6 +222,188 @@ describe("Identity and Channel declarations", () => { await executePlan(deletePlan, ctx); expect(calls).toEqual([]); }); + + test("executes an inferred Channel rename against the existing remote id and migrates state", async () => { + const desired = config(); + desired.channels = { "chimp-dingtalk": desired.channels!.dingtalk! }; + const state = StateManager.initialize(join(tmpdir(), `channel-rename-${crypto.randomUUID()}.json`)); + state.setResource({ + address: { type: "identity", name: "chen", provider: "qoder" }, + remote_id: "idn_existing", + content_hash: "identity-hash", + }); + state.setResource({ + address: { type: "template", name: "assistant", provider: "qoder" }, + remote_id: "tmpl_existing", + content_hash: "template-hash", + }); + state.setResource({ + address: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + remote_id: "channel_existing", + content_hash: "channel-hash", + }); + const calls: string[] = []; + const provider = { + name: "qoder", + findResource: async () => null, + updateChannel: async (id: string) => { + calls.push(`update:${id}`); + return { id, type: "channel" }; + }, + createChannel: async () => { + calls.push("create"); + return { id: "unexpected", type: "channel" }; + }, + deleteChannel: async () => calls.push("delete"), + } as unknown as ProviderAdapter; + const plan: ExecutionPlan = { + actions: [ + { + action: "update", + address: { type: "channel", name: "chimp-dingtalk", provider: "qoder" }, + previousAddress: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + reason: "Channel key renamed", + dependencies: [], + }, + ], + diagnostics: [], + }; + + const result = await executePlan(plan, { + config: desired, + configPath: "/tmp/agents.yaml", + providers: new Map([["qoder", provider]]), + state, + }); + + expect(result.partial).toBe(false); + expect(calls).toEqual(["update:channel_existing"]); + expect(state.getResource({ type: "channel", name: "byoc-dingtalk", provider: "qoder" })).toBeUndefined(); + expect(state.getResource({ type: "channel", name: "chimp-dingtalk", provider: "qoder" })).toMatchObject({ + remote_id: "channel_existing", + replacement_fingerprint: expect.any(String), + }); + }); + + test("skips deleting the old Identity when an inferred Channel rename fails", async () => { + const desired = config(); + desired.channels = { "chimp-dingtalk": desired.channels!.dingtalk! }; + const state = StateManager.initialize(join(tmpdir(), `channel-rename-failure-${crypto.randomUUID()}.json`)); + for (const resource of [ + { + address: { type: "identity" as const, name: "chen", provider: "qoder" }, + remote_id: "idn_new", + content_hash: "new-identity-hash", + }, + { + address: { type: "template" as const, name: "assistant", provider: "qoder" }, + remote_id: "tmpl_existing", + content_hash: "template-hash", + }, + { + address: { type: "identity" as const, name: "byoc", provider: "qoder" }, + remote_id: "idn_old", + content_hash: "old-identity-hash", + }, + { + address: { type: "channel" as const, name: "byoc-dingtalk", provider: "qoder" }, + remote_id: "channel_existing", + content_hash: "channel-hash", + }, + ]) { + state.setResource(resource); + } + const calls: string[] = []; + const provider = { + name: "qoder", + updateChannel: async () => { + calls.push("update-channel"); + throw new Error("update failed"); + }, + createChannel: async () => ({ id: "unexpected", type: "channel" }), + deleteIdentity: async () => calls.push("delete-identity"), + } as unknown as ProviderAdapter; + const channelAddress = { type: "channel" as const, name: "chimp-dingtalk", provider: "qoder" }; + const plan: ExecutionPlan = { + actions: [ + { + action: "update", + address: channelAddress, + previousAddress: { type: "channel", name: "byoc-dingtalk", provider: "qoder" }, + reason: "Channel key renamed", + dependencies: [], + }, + { + action: "delete", + address: { type: "identity", name: "byoc", provider: "qoder" }, + reason: "removed", + dependencies: [channelAddress], + }, + ], + diagnostics: [], + }; + + const result = await executePlan(plan, { + config: desired, + configPath: "/tmp/agents.yaml", + providers: new Map([["qoder", provider]]), + state, + }); + + expect(result.results.map((item) => item.status)).toEqual(["failed", "skipped"]); + expect(calls).toEqual(["update-channel"]); + expect(state.getResource({ type: "identity", name: "byoc", provider: "qoder" })?.remote_id).toBe("idn_old"); + }); + + test("reports a Qoder credential conflict as credential ownership instead of a reserved name", async () => { + const desired = config(); + const state = StateManager.initialize(join(tmpdir(), `channel-conflict-${crypto.randomUUID()}.json`)); + state.setResource({ + address: { type: "identity", name: "chen", provider: "qoder" }, + remote_id: "idn_existing", + content_hash: "identity-hash", + }); + state.setResource({ + address: { type: "template", name: "assistant", provider: "qoder" }, + remote_id: "tmpl_existing", + content_hash: "template-hash", + }); + const provider = { + name: "qoder", + findResource: async () => null, + createChannel: async () => { + throw new ConflictError( + 409, + JSON.stringify({ + error: { code: "CHANNEL_CREDENTIAL_CONFLICT", message: "Credential is already in use." }, + }), + "Qoder API", + ); + }, + updateChannel: async () => ({ id: "unexpected", type: "channel" }), + } as unknown as ProviderAdapter; + const plan: ExecutionPlan = { + actions: [ + { + action: "create", + address: { type: "channel", name: "dingtalk", provider: "qoder" }, + reason: "missing", + dependencies: [], + }, + ], + diagnostics: [], + }; + + const result = await executePlan(plan, { + config: desired, + configPath: "/tmp/agents.yaml", + providers: new Map([["qoder", provider]]), + state, + }); + + expect(result.results[0]?.error?.message).toContain("credentials are already used by another Channel"); + expect(result.results[0]?.error?.message).not.toContain("recently deleted"); + }); }); describe("Qoder Identity and Channel adapter", () => {