Skip to content

Commit 53a9684

Browse files
committed
fix(sdk): match Qoder managed resources by declared identity during drift detection
- Pass the resource declaration to readComparableResource so adapters can use declared display names and metadata for identity. - Qoder adapter now identifies agents/environments by the protected agents.project/agents.resource metadata instead of only by name. - Treat empty agent descriptions as omitted to avoid phantom drift. - Update drift-detection and slim-state tests for managed identity behavior.
1 parent 818b39b commit 53a9684

11 files changed

Lines changed: 245 additions & 24 deletions

File tree

packages/sdk/src/internal/core/resource-runtime.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,15 @@ export async function importResource(
157157
// comparable and report a one-time phantom "Remote drift detected" — most
158158
// visibly for external-reference environments OpenCMA never configured.
159159
const provider = ctx.providers.get(address.provider);
160-
const remote = provider ? await readComparableIfSupported(provider, address.type, remoteId, address.name) : null;
160+
const remote = provider
161+
? await readComparableIfSupported(
162+
provider,
163+
address.type,
164+
remoteId,
165+
address.name,
166+
getResourceDeclaration(address, ctx.config) ?? undefined,
167+
)
168+
: null;
161169
const remoteHash = remote ? stableContentHash(remote.comparable) : undefined;
162170

163171
const resource: ResourceState = {

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,13 @@ async function executeActionInner(
772772
// hash avoids false "Remote drift detected" on the next plan.
773773
let remoteHash = comparableHash;
774774
let remoteSnapshot: unknown;
775-
const remote = await readComparableIfSupported(provider, type, result.id, name);
775+
const remote = await readComparableIfSupported(
776+
provider,
777+
type,
778+
result.id,
779+
name,
780+
getResourceDeclaration(address, ctx.config) ?? undefined,
781+
);
776782
if (remote) {
777783
remoteHash = contentHash(remote.comparable);
778784
remoteSnapshot = remote.snapshot ?? remote.comparable;

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@ export async function refreshState(
5757
const support = provider.getDriftSupport?.(res.address.type) ?? "existence";
5858

5959
if (supportsFullDrift(provider, res.address.type) && provider.normalizeDesiredResource) {
60-
const remote = await provider.readComparableResource?.(res.address.type, res.remote_id, res.address.name);
60+
const decl = options.config ? getResourceDeclaration(res.address, options.config) : null;
61+
const remote = await provider.readComparableResource?.(
62+
res.address.type,
63+
res.remote_id,
64+
res.address.name,
65+
decl ?? undefined,
66+
);
6167
if (!remote) {
6268
if (!options.quiet) {
6369
emitRuntimeFeedback(options.onFeedback, {
@@ -74,7 +80,6 @@ export async function refreshState(
7480
}
7581

7682
const remoteHash = contentHash(remote.comparable);
77-
const decl = options.config ? getResourceDeclaration(res.address, options.config) : null;
7883
const desiredComparable = decl
7984
? provider.normalizeDesiredResource(res.address.type, res.address.name, decl)
8085
: null;

packages/sdk/src/internal/providers/drift-support.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ export async function readComparableIfSupported(
2121
type: ResourceType,
2222
id: string | null,
2323
name: string,
24+
decl?: unknown,
2425
): Promise<ComparableRemoteResource | null> {
2526
if (!supportsFullDrift(adapter, type)) return null;
2627
// Invoke as a method on the adapter — extracting it into a local first would
2728
// drop the `this` binding and silently fail for class-based adapters.
2829
if (typeof adapter.readComparableResource !== "function") return null;
2930
try {
30-
return await adapter.readComparableResource(type, id, name);
31+
return await adapter.readComparableResource(type, id, name, decl);
3132
} catch {
3233
return null;
3334
}

packages/sdk/src/internal/providers/interface.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ export interface ProviderAdapter {
225225
type: ResourceType,
226226
id: string | null,
227227
name: string,
228+
decl?: unknown,
228229
): Promise<ComparableRemoteResource | null>;
229230
normalizeDesiredResource?(type: ResourceType, name: string, decl: unknown): unknown | null;
230231

packages/sdk/src/internal/providers/qoder/adapter.ts

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ export class QoderAdapter implements ProviderAdapter {
292292
type: ResourceType,
293293
id: string | null,
294294
name: string,
295+
decl?: unknown,
295296
): Promise<ComparableRemoteResource | null> {
296297
if (type !== "agent" && type !== "environment" && type !== "template" && type !== "identity" && type !== "channel")
297298
return null;
@@ -304,15 +305,23 @@ export class QoderAdapter implements ProviderAdapter {
304305
const comparable = this.normalizeRemote(type, raw);
305306
return { id: remote.id, type, comparable, snapshot: comparable };
306307
}
307-
const isTemplate = type === "template";
308-
const endpoint = type === "agent" ? "/agents" : type === "environment" ? "/environments" : "/templates";
309-
const raw = await locateRemote(
310-
isTemplate ? this.forwardClient : this.client,
311-
endpoint,
312-
name,
313-
id,
314-
isTemplate ? (item) => item.status !== "archived" : notArchived,
315-
);
308+
309+
let raw: Record<string, unknown> | null;
310+
if ((type === "agent" || type === "environment") && decl !== undefined && this.projectName) {
311+
const declaredName = (decl as { name?: unknown }).name;
312+
const displayName = typeof declaredName === "string" ? declaredName : name;
313+
raw = await this.readManagedResourceByIdentity(type, id, displayName);
314+
} else {
315+
const isTemplate = type === "template";
316+
const endpoint = type === "agent" ? "/agents" : type === "environment" ? "/environments" : "/templates";
317+
raw = await locateRemote(
318+
isTemplate ? this.forwardClient : this.client,
319+
endpoint,
320+
name,
321+
id,
322+
isTemplate ? (item) => item.status !== "archived" : notArchived,
323+
);
324+
}
316325
if (!raw) return null;
317326

318327
const comparable = this.normalizeRemote(type, raw);
@@ -325,6 +334,52 @@ export class QoderAdapter implements ProviderAdapter {
325334
};
326335
}
327336

337+
private async readManagedResourceByIdentity(
338+
type: "agent" | "environment",
339+
id: string | null,
340+
displayName: string,
341+
): Promise<Record<string, unknown> | null> {
342+
const endpoint = type === "agent" ? "/agents" : "/environments";
343+
if (id) {
344+
try {
345+
const raw = (await this.client.get(`${endpoint}/${id}`)) as Record<string, unknown>;
346+
return this.matchesManagedResourceIdentity(type, raw, displayName) ? raw : null;
347+
} catch (error) {
348+
if (ApiError.isNotFound(error)) return null;
349+
throw error;
350+
}
351+
}
352+
353+
const matches = (await this.client.getAllPaged(endpoint)).filter((raw) =>
354+
this.matchesManagedResourceIdentity(type, raw, displayName),
355+
);
356+
if (matches.length !== 1) return null;
357+
const matchedId = matches[0]?.id;
358+
if (typeof matchedId !== "string") return null;
359+
try {
360+
const raw = (await this.client.get(`${endpoint}/${matchedId}`)) as Record<string, unknown>;
361+
return this.matchesManagedResourceIdentity(type, raw, displayName) ? raw : null;
362+
} catch (error) {
363+
if (ApiError.isNotFound(error)) return null;
364+
throw error;
365+
}
366+
}
367+
368+
private matchesManagedResourceIdentity(
369+
type: "agent" | "environment",
370+
raw: Record<string, unknown>,
371+
displayName: string,
372+
): boolean {
373+
if (!notArchived(raw) || raw.name !== displayName) return false;
374+
if (typeof raw.type === "string" && raw.type !== type) return false;
375+
const metadata = raw.metadata;
376+
if (!metadata || typeof metadata !== "object") return false;
377+
return (
378+
(metadata as Record<string, unknown>)["agents.project"] === this.projectName &&
379+
(metadata as Record<string, unknown>)["agents.resource"] === displayName
380+
);
381+
}
382+
328383
normalizeDesiredResource(type: ResourceType, name: string, decl: unknown): unknown | null {
329384
if (type === "environment") {
330385
return this.normalizeRemote(
@@ -410,7 +465,7 @@ export class QoderAdapter implements ProviderAdapter {
410465

411466
return compactDeep({
412467
name: raw.name,
413-
description: raw.description,
468+
description: raw.description === "" ? undefined : raw.description,
414469
model: normalizeModel(raw.model),
415470
instructions: raw.system,
416471
tools: normalizeQoderTools(raw.tools),

packages/sdk/src/internal/providers/qoder/mapper.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { resolveSandboxMountPath } from "../../utils/sandbox-mount.ts";
1717
import { permissionOverridesFromWire, resolveBuiltinTools, toPermissionPolicy } from "../../utils/tool-permissions.ts";
1818
import type { ResolvedAgentRefs, ResolvedDeploymentRefs, ResolvedTemplateRefs } from "../interface.ts";
1919
import { mapGithubRepositorySessionResource, resolveGithubRepositoryMountPath } from "../session-resource-mapper.ts";
20-
import { injectMetadata, secretPlaceholder, slug } from "../sync-mapping.ts";
20+
import { injectManagedResourceMetadata, injectMetadata, secretPlaceholder, slug } from "../sync-mapping.ts";
2121

2222
// Qoder's API expects builtin tool names in PascalCase. The configuration layer
2323
// (agents.yaml / playbook JSON) uses snake_case or lowercase aliases and is
@@ -54,6 +54,7 @@ function normalizeEnvironmentPackages(value: unknown): Record<string, string[]>
5454
}
5555

5656
export function mapEnvironment(name: string, decl: EnvironmentDecl, projectName: string): unknown {
57+
const displayName = decl.name ?? name;
5758
const envType = decl.config.type ?? "cloud";
5859
const config: Record<string, unknown> = { type: envType };
5960
if (decl.config.networking) config.networking = decl.config.networking;
@@ -62,10 +63,10 @@ export function mapEnvironment(name: string, decl: EnvironmentDecl, projectName:
6263
if (packages) config.packages = packages;
6364
if (decl.config.setup_script !== undefined) config.setup_script = decl.config.setup_script;
6465
return {
65-
name,
66+
name: displayName,
6667
description: decl.description ?? "",
6768
config,
68-
metadata: injectMetadata(decl.metadata, projectName, name),
69+
metadata: injectManagedResourceMetadata(decl.metadata, projectName, displayName),
6970
};
7071
}
7172

@@ -410,16 +411,17 @@ export function mapAgent(
410411
model = typeof qoderModel === "string" ? qoderModel : qoderModel.id;
411412
}
412413

414+
const displayName = decl.name ?? name;
413415
const body: Record<string, unknown> = {
414-
name: decl.name ?? name,
416+
name: displayName,
415417
model,
416418
system: decl.instructions,
417419
};
418420

419421
if (version !== undefined) body.version = version;
420422
if (decl.description) body.description = decl.description;
421423
if (projectName) {
422-
body.metadata = injectMetadata(decl.metadata, projectName, name);
424+
body.metadata = injectManagedResourceMetadata(decl.metadata, projectName, displayName);
423425
} else if (decl.metadata) {
424426
body.metadata = decl.metadata;
425427
}

packages/sdk/src/internal/providers/resource-workflow.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ export interface DriftReadAdapter {
159159
type: ResourceType,
160160
id: string | null,
161161
name: string,
162+
decl?: unknown,
162163
): Promise<ComparableRemoteResource | null>;
163164
normalizeDesiredResource?(type: ResourceType, name: string, decl: unknown): unknown | null;
164165
}

packages/sdk/src/internal/providers/sync-mapping.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@ export function injectMetadata(
1717
return { ...injected, ...userMetadata };
1818
}
1919

20+
export function injectManagedResourceMetadata(
21+
userMetadata: Record<string, string> | undefined,
22+
projectName: string,
23+
resourceName: string,
24+
): Record<string, string> {
25+
return {
26+
...userMetadata,
27+
"agents.project": projectName,
28+
"agents.resource": resourceName,
29+
};
30+
}
31+
2032
/** Lowercase slug suitable for a yaml key. Falls back to `fallback` when empty. */
2133
export function slug(value: string | undefined, fallback: string): string {
2234
const out = (value ?? "")

packages/sdk/tests/unit/drift-detection.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,27 @@ describe("drift-aware refresh", () => {
9090

9191
expect(state.getResource({ type: "agent", name: "assistant", provider: "qoder" })).toBeDefined();
9292
});
93+
94+
test("passes the declared resource to comparable reads", async () => {
95+
const state = StateManager.initialize(tmpPath());
96+
state.setResource({
97+
address: { type: "agent", name: "assistant", provider: "qoder" },
98+
remote_id: "agent_1",
99+
content_hash: "h",
100+
desired_hash: "h",
101+
});
102+
const provider = fakeProvider(config.agents!.assistant!);
103+
const readComparableResource = provider.readComparableResource!;
104+
let receivedDeclaration: unknown;
105+
provider.readComparableResource = async (type, id, name, declaration) => {
106+
receivedDeclaration = declaration;
107+
return readComparableResource(type, id, name, declaration);
108+
};
109+
110+
await refreshState(state, new Map([["qoder", provider]]), { config });
111+
112+
expect(receivedDeclaration).toEqual(config.agents!.assistant!);
113+
});
93114
});
94115

95116
describe("Qoder comparable fixtures", () => {
@@ -144,6 +165,24 @@ describe("Qoder comparable fixtures", () => {
144165
expect(changed).not.toEqual(desired);
145166
expect(removed).not.toEqual(matching);
146167
});
168+
169+
test("treats an empty Agent description as omitted while preserving non-empty description drift", () => {
170+
const adapter = new QoderAdapter("pt-test", undefined, "tmp") as any;
171+
const desired = adapter.normalizeDesiredResource("agent", "assistant", {
172+
model: "auto",
173+
instructions: "Reply only with the marker.",
174+
});
175+
const remote = {
176+
name: "assistant",
177+
model: "auto",
178+
system: "Reply only with the marker.",
179+
tools: [{ type: "agent_toolset_20260401" }],
180+
metadata: { "agents.project": "tmp", "agents.resource": "assistant" },
181+
};
182+
183+
expect(adapter.normalizeRemote("agent", { ...remote, description: "" })).toEqual(desired);
184+
expect(adapter.normalizeRemote("agent", { ...remote, description: "remote description" })).not.toEqual(desired);
185+
});
147186
});
148187

149188
describe("planner drift classification", () => {
@@ -300,3 +339,79 @@ describe("Qoder archived resources are treated as gone", () => {
300339
expect((await adapter.findResource("agent", "a"))?.id).toBe("agent_active");
301340
});
302341
});
342+
343+
describe("Qoder managed comparable identity", () => {
344+
const declaration = { name: "display-name", model: "auto", instructions: "test" };
345+
const identity = {
346+
id: "agent_1",
347+
type: "agent",
348+
name: "display-name",
349+
metadata: { "agents.project": "tmp", "agents.resource": "display-name" },
350+
model: "auto",
351+
system: "test",
352+
};
353+
354+
test("accepts a matching detail resource when the logical key differs from its display name", async () => {
355+
const adapter = new QoderAdapter("pt-test", undefined, "tmp") as any;
356+
adapter.client = {
357+
get: async (path: string) => {
358+
expect(path).toBe("/agents/agent_1");
359+
return identity;
360+
},
361+
};
362+
363+
const remote = await adapter.readComparableResource("agent", "agent_1", "logical-key", declaration);
364+
365+
expect(remote?.id).toBe("agent_1");
366+
});
367+
368+
test("rejects an ID detail response whose type or managed metadata is not the declared identity", async () => {
369+
const adapter = new QoderAdapter("pt-test", undefined, "tmp") as any;
370+
adapter.client = {
371+
get: async () => ({
372+
...identity,
373+
type: "environment",
374+
metadata: { "agents.project": "tmp", "agents.resource": "other" },
375+
}),
376+
};
377+
378+
expect(await adapter.readComparableResource("agent", "agent_1", "logical-key", declaration)).toBeNull();
379+
});
380+
381+
test("claims only one full-identity match from paginated discovery and verifies its detail", async () => {
382+
const calls: string[] = [];
383+
const adapter = new QoderAdapter("pt-test", undefined, "tmp") as any;
384+
adapter.client = {
385+
getAllPaged: async (path: string) => {
386+
calls.push(path);
387+
return [{ ...identity, id: "agent_1" }];
388+
},
389+
get: async (path: string) => {
390+
calls.push(path);
391+
return identity;
392+
},
393+
};
394+
395+
const remote = await adapter.readComparableResource("agent", null, "logical-key", declaration);
396+
397+
expect(remote?.id).toBe("agent_1");
398+
expect(calls).toEqual(["/agents", "/agents/agent_1"]);
399+
});
400+
401+
test("fails closed for zero or multiple full-identity matches", async () => {
402+
for (const candidates of [[], [{ ...identity }, { ...identity, id: "agent_2" }]]) {
403+
const adapter = new QoderAdapter("pt-test", undefined, "tmp") as any;
404+
let detailRead = false;
405+
adapter.client = {
406+
getAllPaged: async () => candidates,
407+
get: async () => {
408+
detailRead = true;
409+
return identity;
410+
},
411+
};
412+
413+
expect(await adapter.readComparableResource("agent", null, "logical-key", declaration)).toBeNull();
414+
expect(detailRead).toBe(false);
415+
}
416+
});
417+
});

0 commit comments

Comments
 (0)