Skip to content

Commit 3c9f6fd

Browse files
committed
fix: migrate renamed channels in place
Change-Id: I7fb187c56bf95b3c7b83ba5975377be139767f69
1 parent 85cd4b6 commit 3c9f6fd

8 files changed

Lines changed: 434 additions & 6 deletions

File tree

apps/server/openapi.json

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,6 +1300,33 @@
13001300
},
13011301
"required": ["type", "name", "provider"]
13021302
},
1303+
"previousAddress": {
1304+
"type": "object",
1305+
"properties": {
1306+
"type": {
1307+
"type": "string",
1308+
"enum": [
1309+
"environment",
1310+
"vault",
1311+
"memory_store",
1312+
"skill",
1313+
"agent",
1314+
"template",
1315+
"deployment",
1316+
"file",
1317+
"identity",
1318+
"channel"
1319+
]
1320+
},
1321+
"name": {
1322+
"type": "string"
1323+
},
1324+
"provider": {
1325+
"type": "string"
1326+
}
1327+
},
1328+
"required": ["type", "name", "provider"]
1329+
},
13031330
"reason": {
13041331
"type": "string"
13051332
},

packages/sdk/src/internal/executor/executor.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { dirname, resolve } from "node:path";
22
import { UserError } from "../errors.ts";
33
import { computeComparableDesiredHash } from "../planner/comparable.ts";
44
import { getResourceDeclaration } from "../planner/declaration.ts";
5-
import { computeResourceHash } from "../planner/hasher.ts";
5+
import { computeReplacementFingerprint, computeResourceHash } from "../planner/hasher.ts";
66
import { buildReadinessBaseline } from "../planner/plan-semantics.ts";
77
import { ApiError, ConflictError } from "../providers/base-client.ts";
88
import { readComparableIfSupported } from "../providers/drift-support.ts";
@@ -229,8 +229,8 @@ async function executeAction(action: PlannedAction, provider: ResourceExecAdapte
229229
resource: action.address,
230230
message: `update ${action.address.type}.${action.address.name} (${action.address.provider}) — not found remotely, recreating`,
231231
});
232-
ctx.state.removeResource(action.address);
233-
return executeActionInner({ ...action, action: "create" }, provider, ctx);
232+
ctx.state.removeResource(action.previousAddress ?? action.address);
233+
return executeActionInner({ ...action, action: "create", previousAddress: undefined }, provider, ctx);
234234
}
235235
}
236236

@@ -317,7 +317,8 @@ async function executeActionInner(
317317
}
318318

319319
const isUpdate = action.action === "update";
320-
const existingId = isUpdate ? ctx.state.getResource(address)?.remote_id : undefined;
320+
const priorAddress = action.previousAddress ?? address;
321+
const existingId = isUpdate ? ctx.state.getResource(priorAddress)?.remote_id : undefined;
321322

322323
let result: RemoteResource;
323324

@@ -652,7 +653,7 @@ async function executeActionInner(
652653

653654
// The externally-managed marker is sticky: it survives applies and is only
654655
// cleared by removing the resource from state (`agents state rm` / destroy).
655-
const priorResource = ctx.state.getResource(address);
656+
const priorResource = ctx.state.getResource(priorAddress);
656657
ctx.state.setResource({
657658
address,
658659
remote_id: result.id,
@@ -669,9 +670,11 @@ async function executeActionInner(
669670
desired_readiness_baseline: buildReadinessBaseline(getResourceDeclaration(address, ctx.config)),
670671
remote_hash: remoteHash,
671672
remote_snapshot: remoteSnapshot,
673+
replacement_fingerprint: computeReplacementFingerprint(address, ctx.config),
672674
drift_paths: [],
673675
drift_status: remoteHash ? "in_sync" : undefined,
674676
});
677+
if (action.previousAddress) ctx.state.removeResource(action.previousAddress);
675678
return adopted;
676679
}
677680

@@ -719,6 +722,16 @@ async function adoptOnConflict(
719722
// command, so fail with actionable guidance instead of the raw wire error.
720723
function nameReservedError(err: unknown, address: ResourceAddress, searchName: string): UserError {
721724
const detail = err instanceof ApiError ? err.message : String(err);
725+
if (
726+
address.type === "channel" &&
727+
err instanceof ApiError &&
728+
err.responseBody.includes("CHANNEL_CREDENTIAL_CONFLICT")
729+
) {
730+
return new UserError(
731+
`${address.provider} rejected channel.${address.name} because its credentials are already used by another Channel. ` +
732+
`Keep the existing Channel address so it can be updated in place, remove the old Channel first, or use a different credential set. (${detail})`,
733+
);
734+
}
722735
return new UserError(
723736
`${address.provider} reported ${address.type} "${searchName}" already exists, but it could not be found remotely to adopt. ` +
724737
`This usually means it was recently deleted and the provider still reserves the name. ` +

packages/sdk/src/internal/planner/hasher.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ export async function computeResourceHash(
5151
return contentHash(decl);
5252
}
5353

54+
/** Stable, non-reversible identity hint for resources whose YAML key may change. */
55+
export function computeReplacementFingerprint(address: ResourceAddress, config: ProjectConfig): string | undefined {
56+
if (address.type !== "channel") return undefined;
57+
const decl = config.channels?.[address.name];
58+
if (!decl) return undefined;
59+
return contentHash({ channel_type: decl.type, credentials: decl.credentials ?? {} });
60+
}
61+
5462
function resolveChannelReferenceIds(
5563
decl: { agent: string; identity?: string },
5664
config: ProjectConfig,

packages/sdk/src/internal/planner/planner.ts

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type { ExecutionPlan, PlannedAction } from "../types/plan.ts";
1010
import type { ResourceAddress, StateFile } from "../types/state.ts";
1111
import { addressKey } from "../types/state.ts";
1212
import { getResourceDeclaration } from "./declaration.ts";
13-
import { computeResourceHash } from "./hasher.ts";
13+
import { computeReplacementFingerprint, computeResourceHash } from "./hasher.ts";
1414
import { buildReadinessBaseline, classifyReadinessImpact, diffReadinessBaseline } from "./plan-semantics.ts";
1515

1616
export interface PlanOptions {
@@ -218,9 +218,92 @@ export async function buildPlan(
218218
});
219219
}
220220

221+
coalesceChannelRenames(actions, config, state);
221222
return { actions, diagnostics: diagnostics.getAll() };
222223
}
223224

225+
/**
226+
* A YAML key is a resource address, but changing that key should not force a
227+
* remote Channel replacement when the old and new declarations form one
228+
* unambiguous same-type pair. Retaining the remote id is especially important
229+
* for messaging providers that allow a credential set to belong to only one
230+
* Channel at a time.
231+
*/
232+
function coalesceChannelRenames(actions: PlannedAction[], config: ProjectConfig, state: StateFile): void {
233+
const creates = actions.filter((action) => action.action === "create" && action.address.type === "channel");
234+
const deletes = actions.filter((action) => action.action === "delete" && action.address.type === "channel");
235+
const stateByAddress = new Map(state.resources.map((resource) => [addressKey(resource.address), resource]));
236+
const matchedDeletes = new Set<PlannedAction>();
237+
238+
for (const create of creates) {
239+
const desiredType = config.channels?.[create.address.name]?.type;
240+
if (!desiredType) continue;
241+
const desiredFingerprint = computeReplacementFingerprint(create.address, config);
242+
const candidates = deletes.filter((deletion) => {
243+
if (matchedDeletes.has(deletion) || deletion.address.provider !== create.address.provider) return false;
244+
const prior = stateByAddress.get(addressKey(deletion.address));
245+
const snapshot = prior?.remote_snapshot as { channel_type?: unknown } | undefined;
246+
if (snapshot?.channel_type !== desiredType) return false;
247+
return !prior?.replacement_fingerprint || prior.replacement_fingerprint === desiredFingerprint;
248+
});
249+
if (candidates.length !== 1) continue;
250+
251+
const deletion = candidates[0]!;
252+
const prior = stateByAddress.get(addressKey(deletion.address));
253+
const competingCreates = creates.filter(
254+
(candidate) =>
255+
candidate !== create &&
256+
candidate.address.provider === create.address.provider &&
257+
config.channels?.[candidate.address.name]?.type === desiredType &&
258+
(!prior?.replacement_fingerprint ||
259+
computeReplacementFingerprint(candidate.address, config) === prior.replacement_fingerprint),
260+
);
261+
if (competingCreates.length > 0) continue;
262+
263+
create.action = "update";
264+
create.previousAddress = deletion.address;
265+
create.before = deletion.before;
266+
create.driftKind = "local";
267+
create.reason = `Channel key renamed from '${deletion.address.name}' (remote resource retained)`;
268+
protectRenamedChannelDependencies(actions, stateByAddress, deletion, create);
269+
matchedDeletes.add(deletion);
270+
}
271+
272+
for (let index = actions.length - 1; index >= 0; index--) {
273+
if (matchedDeletes.has(actions[index]!)) actions.splice(index, 1);
274+
}
275+
}
276+
277+
/** Do not delete the old Identity/Template when the Channel migration that releases it fails. */
278+
function protectRenamedChannelDependencies(
279+
actions: PlannedAction[],
280+
stateByAddress: Map<string, StateFile["resources"][number]>,
281+
deletion: PlannedAction,
282+
replacement: PlannedAction,
283+
): void {
284+
const prior = stateByAddress.get(addressKey(deletion.address));
285+
const snapshot = prior?.remote_snapshot as { identity_id?: unknown; template_id?: unknown } | undefined;
286+
const referencedIds = new Set(
287+
[snapshot?.identity_id, snapshot?.template_id].filter((id): id is string => typeof id === "string"),
288+
);
289+
if (referencedIds.size === 0) return;
290+
291+
for (const action of actions) {
292+
if (
293+
action.action !== "delete" ||
294+
(action.address.type !== "identity" && action.address.type !== "template") ||
295+
action.address.provider !== replacement.address.provider
296+
) {
297+
continue;
298+
}
299+
const dependency = stateByAddress.get(addressKey(action.address));
300+
if (!dependency?.remote_id || !referencedIds.has(dependency.remote_id)) continue;
301+
if (!action.dependencies.some((address) => addressKey(address) === addressKey(replacement.address))) {
302+
action.dependencies.push(replacement.address);
303+
}
304+
}
305+
}
306+
224307
/** Keep the old delivery resource alive when creating its new materialization fails. */
225308
function deliveryReplacementAddress(address: ResourceAddress, graph: DependencyGraph): ResourceAddress | undefined {
226309
if (address.type !== "agent" && address.type !== "template") return undefined;

packages/sdk/src/internal/state/state-manager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export class StateManager implements IStateManager {
4747
desired_readiness_baseline: r.desired_readiness_baseline as ResourceState["desired_readiness_baseline"],
4848
remote_hash: r.remote_hash as string | undefined,
4949
remote_snapshot: r.remote_snapshot,
50+
replacement_fingerprint: r.replacement_fingerprint as string | undefined,
5051
drift_paths: r.drift_paths as string[] | undefined,
5152
drift_status: r.drift_status as ResourceState["drift_status"],
5253
}));

packages/sdk/src/internal/types/dto.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ export type PlanReadinessImpact = z.infer<typeof PlanReadinessImpactSchema>;
4444
export const PlannedActionSchema = z.object({
4545
action: ActionTypeSchema,
4646
address: ResourceAddressSchema,
47+
/** Existing state address to retain when this action is an inferred logical rename. */
48+
previousAddress: ResourceAddressSchema.optional(),
4749
reason: z.string(),
4850
driftKind: DriftKindSchema.optional(),
4951
readinessImpact: PlanReadinessImpactSchema.optional(),

packages/sdk/src/internal/types/state.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ export interface ResourceState {
2121
desired_readiness_baseline?: ResourceReadinessBaseline;
2222
remote_hash?: string;
2323
remote_snapshot?: unknown;
24+
/** Non-reversible declaration fingerprint used to infer safe logical renames. */
25+
replacement_fingerprint?: string;
2426
drift_paths?: string[];
2527
drift_status?: "in_sync" | "drifted" | "missing" | "unchecked";
2628
}

0 commit comments

Comments
 (0)