Skip to content

Commit 6562e87

Browse files
lidge-junHQ1995claude
committed
fix(devin): qualify child trajectories by supplied parent
Addresses post-merge review #6557 discussion_r4176028586. Preserve the supplied parent context with tagged standalone and child identities; keep session-only and lifecycle behavior unchanged. Co-authored-by: Hanqing Zhao <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
1 parent df3dd3f commit 6562e87

6 files changed

Lines changed: 200 additions & 8 deletions

File tree

‎docs-site/src/content/docs/reference/adapters.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,10 @@ configuration that names the old id is rewritten at startup.
579579
`parseStream` path is disabled.
580580
- Named conversations reuse a trajectory across sequential turns, scoped to the resolved credential
581581
and tenant host. Own-thread identity takes precedence over session markers (`session_id`,
582-
`session-id`, or `x-session-affinity`); a shared parent alone is not a conversation identity.
582+
`session-id`, or `x-session-affinity`). When an own-thread identity and a nonempty
583+
`x-codex-parent-thread-id` are both supplied, their pair identifies the conversation,
584+
so equal own IDs under different parents remain separate. Standalone own and session-only
585+
identities retain their existing precedence; a shared parent alone is not a conversation identity.
583586
Overlapping turns receive distinct IDs. The proxy retains at most 256 entries in memory,
584587
evicts only inactive entries, and releases claims after completion, failure, or cancellation.
585588
Restarting the proxy clears retention. Unnamed calls continue allocating an ID per request.

‎src/adapters/devin.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,11 +753,13 @@ export function createDevinAdapter(
753753
};
754754

755755
try {
756-
const conversation = incoming.headers.get("thread-id")?.trim() || parsed._codexOwnThreadId?.trim()
756+
const ownThreadId = incoming.headers.get("thread-id")?.trim() || parsed._codexOwnThreadId?.trim();
757+
const conversation = ownThreadId
757758
|| incoming.headers.get("session_id")?.trim() || incoming.headers.get("session-id")?.trim()
758759
|| incoming.headers.get("x-session-affinity")?.trim()
759760
|| (!incoming.headers.has("x-codex-parent-thread-id") ? parsed._clientThreadId : undefined);
760-
trajectory = claimDevinTrajectory(apiKey, host, conversation);
761+
trajectory = claimDevinTrajectory(apiKey, host, conversation,
762+
ownThreadId ? incoming.headers.get("x-codex-parent-thread-id")?.trim() || undefined : undefined);
761763
const trajectoryId = trajectory.trajectoryId;
762764
// Read the selected UID's catalog row, not the picker's collapsed base.
763765
contextWindow = resolveDevinContextWindow(provider, modelUid, catalog?.byUid.get(modelUid));

‎src/adapters/devin/trajectory.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@ export function claimDevinTrajectory(
1313
apiKey: string,
1414
host: string,
1515
conversation: string | null | undefined,
16+
parentThreadId?: string,
1617
): DevinTrajectoryClaim {
1718
if (!conversation) return { release() {} };
18-
// Structured framing separates host and conversation even if either contains a delimiter.
19+
// Tag both forms so a standalone JSON-like ID cannot alias a parent/own pair.
20+
const identity = parentThreadId
21+
? ["codex-child", parentThreadId, conversation]
22+
: ["standalone", conversation];
1923
// Only the fixed digest and UUID enter the retained map, never the token or conversation.
2024
const key = new Bun.CryptoHasher("sha256")
21-
.update(JSON.stringify([devinCacheIdentity(apiKey, host), conversation])).digest("hex");
25+
.update(JSON.stringify([devinCacheIdentity(apiKey, host), identity])).digest("hex");
2226
let entry = trajectories.get(key);
2327
if (entry?.active) return { trajectoryId: crypto.randomUUID(), release() {} };
2428
if (!entry) {

‎structure/providers-and-adapters.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ rewrite rules and the routed-id settlement.
196196
| `src/adapters/image.ts`, `src/adapters/anthropic-image-guard.ts`, `src/adapters/anthropic-image-normalize.ts`, `src/adapters/anthropic-image-codec.ts` | Image conversion for adapter ingress and Anthropic-specific normalization/limits. An image's ladder position is pinned to its own identity (content hash + media type), so appending a newer image cannot re-encode older ones and bust Anthropic's prompt prefix cache (#4532). |
197197
| `src/adapters/run-turn-queue.ts`, `src/adapters/tool-catalog-nudge.ts`, `src/adapters/identity.ts`, `src/adapters/upstream-http-error.ts` | Shared adapter execution support: turn queueing, tool-catalog nudging, client identity, upstream error normalization. |
198198

199-
`src/adapters/devin/trajectory.ts` owns up to 256 process-local conversation trajectories, keyed by a digest of resolved credential identity, tenant host and named conversation. Own-thread identity takes precedence over session aliases; a parent-only request does not borrow its parent's trajectory. Sequential turns reuse an inactive entry across request-scoped adapters; overlapping turns and a store full of active entries receive fresh unretained IDs. Only inactive entries are evicted, and a claim's idempotent release runs in the adapter's `finally` on success, error and cancellation. The optional trajectory travels through `src/adapters/devin/cloud-direct/chat.ts` to protobuf field #15.1; unnamed calls retain per-request allocation. A named claim remains held through the turn's existing reset/signature retries. This is identifier continuity, not proof of upstream cache hits.
199+
`src/adapters/devin/trajectory.ts` owns up to 256 process-local conversation trajectories, keyed by a digest of resolved credential identity, tenant host and named conversation. Own-thread identity takes precedence over session aliases. When a nonempty parent header accompanies an own-thread identity, the tagged parent/own pair identifies the conversation; standalone identities have a distinct tag, so a JSON-like caller string cannot alias a child tuple. Session-only keys retain their existing behavior, and a parent-only request does not borrow its parent's trajectory. Sequential turns reuse an inactive entry across request-scoped adapters; overlapping turns and a store full of active entries receive fresh unretained IDs. Only inactive entries are evicted, and a claim's idempotent release runs in the adapter's `finally` on success, error and cancellation. The optional trajectory travels through `src/adapters/devin/cloud-direct/chat.ts` to protobuf field #15.1; unnamed calls retain per-request allocation. A named claim remains held through the turn's existing reset/signature retries. This is identifier continuity, not proof of upstream cache hits.
200200

201201
Devin pairs a late signature only with immediately preceding unsigned thinking in one assistant message; a call between them breaks the pair. While a signed attempt is held for an optional unsigned retry, a timer emits plain heartbeats even when the upstream stalls. The held queue is capped at 1,024 events or approximately 1 MiB of UTF-16 reasoning, signature, and signature-type payload; signature types are individually capped at 4 KiB before UTF-8 decoding, and crossing either queue cap releases the events and disables that retry. Held usage frames merge per cumulative field, and the refused attempt's usage is added to the retry. A signed `invalid_argument` refusal is offered the unsigned retry before the history-overflow classifier sees any final refusal. If the send budget withholds that retry, the original refusal reaches the classifier and the recovery is recorded as withheld.
202202

‎tests/providers/devin-prompt-cache.test.ts‎

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,17 +356,136 @@ describe("one catalog read serves the cached chat path", () => {
356356
expect(repeat).toBe(a);
357357
});
358358

359+
test("the same child under different supplied parents has separate wire UUIDs and repeat continuity", async () => {
360+
seed([{ uid: "swe-2-high" }]);
361+
const own = crypto.randomUUID();
362+
const parents = ["parent-a-" + own, "parent-b-" + own];
363+
for (const parent of [...parents, ...parents]) {
364+
const events = await run("swe-2-high", {}, {}, undefined,
365+
new Headers({ "thread-id": own, "x-codex-parent-thread-id": parent }), { _clientThreadId: parent });
366+
expect(events.some(event => event.type === "error")).toBe(false);
367+
expect(events.at(-1)?.type).toBe("done");
368+
}
369+
const [a, b, aRepeat, bRepeat] = sentTrajectories();
370+
expect(aRepeat).toBe(a);
371+
expect(bRepeat).toBe(b);
372+
expect(b, "different supplied parents must not collide on one wire UUID").not.toBe(a);
373+
});
374+
375+
test("parent B retains its wire UUID across sequential turns while parent A holds the same child active", async () => {
376+
seed([{ uid: "swe-2-high" }]);
377+
const own = crypto.randomUUID();
378+
const aHeaders = new Headers({ "thread-id": own, "x-codex-parent-thread-id": "parent-a-" + own });
379+
const bHeaders = new Headers({ "thread-id": own, "x-codex-parent-thread-id": "parent-b-" + own });
380+
const recorded = globalThis.fetch;
381+
let release!: () => void;
382+
let sent!: () => void;
383+
const held = new Promise<void>(resolve => { release = resolve; });
384+
const firstSent = new Promise<"sent">(resolve => { sent = () => resolve("sent"); });
385+
globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
386+
const response = await recorded(input, init);
387+
if (String(input).endsWith("/GetChatMessage") && requests.length === 1) {
388+
sent();
389+
await held;
390+
}
391+
return response;
392+
}) as typeof fetch;
393+
const deadline = AbortSignal.timeout(3_000);
394+
const first = run("swe-2-high", {}, {}, deadline, aHeaders);
395+
try {
396+
const ended = first.then(() => "ended" as const, () => "ended" as const);
397+
const timedOut = new Promise<"timeout">(resolve => deadline.addEventListener("abort", () => resolve("timeout"), { once: true }));
398+
expect(await Promise.race([firstSent, ended, timedOut])).toBe("sent");
399+
for (const headers of [bHeaders, bHeaders, aHeaders, aHeaders]) {
400+
const events = await run("swe-2-high", {}, {}, undefined, headers);
401+
expect(events.some(event => event.type === "error")).toBe(false);
402+
expect(events.at(-1)?.type).toBe("done");
403+
}
404+
} finally {
405+
release();
406+
try { expect((await first).at(-1)?.type).toBe("done"); }
407+
finally { globalThis.fetch = recorded; }
408+
}
409+
await run("swe-2-high", {}, {}, undefined, aHeaders);
410+
await run("swe-2-high", {}, {}, undefined, bHeaders);
411+
const [a, b1, b2, aOverlap1, aOverlap2, aLater, bLater] = sentTrajectories();
412+
expect(new Set([a, aOverlap1, aOverlap2]).size).toBe(3);
413+
expect(aLater).toBe(a);
414+
expect(b1).not.toBe(a);
415+
expect(b2, "parent B must retain its own UUID while parent A is active").toBe(b1);
416+
expect(bLater).toBe(b1);
417+
});
418+
419+
test("internal own and header own select the same qualified trajectory and explicit own wins", async () => {
420+
seed([{ uid: "swe-2-high" }]);
421+
const own = crypto.randomUUID();
422+
const parent = "parent-" + own;
423+
await run("swe-2-high", {}, {}, undefined,
424+
new Headers({ "x-codex-parent-thread-id": " " + parent + " ", session_id: "session-a-" + own }),
425+
{ _codexOwnThreadId: " " + own + " ", _clientThreadId: "parsed-other-" + own });
426+
await run("swe-2-high", {}, {}, undefined,
427+
new Headers({ "thread-id": " " + own + " ", "x-codex-parent-thread-id": parent, session_id: "session-b-" + own }));
428+
await run("swe-2-high", {}, {}, undefined,
429+
new Headers({ "thread-id": "explicit-" + own, "x-codex-parent-thread-id": parent }), { _codexOwnThreadId: own });
430+
await run("swe-2-high", {}, {}, undefined,
431+
new Headers({ "x-codex-parent-thread-id": parent }), { _codexOwnThreadId: "explicit-" + own });
432+
const [internal, header, explicit, explicitRepeat] = sentTrajectories();
433+
expect(header).toBe(internal);
434+
expect(explicit).not.toBe(internal);
435+
expect(explicitRepeat).toBe(explicit);
436+
});
437+
438+
test.each(["session_id", "session-id", "x-session-affinity"])("session-only %s remains unqualified across changing parents", async alias => {
439+
seed([{ uid: "swe-2-high" }]);
440+
const name = crypto.randomUUID();
441+
for (const parent of [undefined, "parent-a-" + name, "parent-b-" + name, ""]) {
442+
const headers = new Headers({ [alias]: name });
443+
if (parent !== undefined) headers.set("x-codex-parent-thread-id", parent);
444+
const events = await run("swe-2-high", {}, {}, undefined, headers, { _clientThreadId: "parsed-" + parent });
445+
expect(events.at(-1)?.type).toBe("done");
446+
}
447+
const [first, ...repeats] = sentTrajectories();
448+
expect(repeats).toEqual([first, first, first]);
449+
});
450+
451+
test("missing or blank parent leaves own identity standalone with own precedence", async () => {
452+
seed([{ uid: "swe-2-high" }]);
453+
const own = crypto.randomUUID();
454+
for (const parent of [undefined, "", " \t "]) {
455+
const headers = new Headers({ "thread-id": " " + own + " ", session_id: "session-" + own });
456+
if (parent !== undefined) headers.set("x-codex-parent-thread-id", parent);
457+
await run("swe-2-high", {}, {}, undefined, headers,
458+
{ _codexOwnThreadId: "internal-other-" + own, _clientThreadId: "parsed-other-" + own });
459+
}
460+
await run("swe-2-high", {}, {}, undefined, new Headers(), { _codexOwnThreadId: own });
461+
const [first, ...repeats] = sentTrajectories();
462+
expect(repeats).toEqual([first, first, first]);
463+
});
464+
465+
test.each(["", " \t "])("blank parent header %p suppresses direct parsed fallback at the adapter boundary", async parent => {
466+
// This is the adapter contract; ingress header materialization may omit blank values.
467+
seed([{ uid: "swe-2-high" }]);
468+
const headers = new Headers({ "x-codex-parent-thread-id": parent });
469+
const identity = { _clientThreadId: crypto.randomUUID() };
470+
for (let i = 0; i < 2; i++) {
471+
expect((await run("swe-2-high", {}, {}, undefined, headers, identity)).at(-1)?.type).toBe("done");
472+
}
473+
expect(sentTrajectories()[1]).not.toBe(sentTrajectories()[0]);
474+
});
475+
359476
test("internal own identity survives headerless handoffs and outranks session aliases", async () => {
360477
seed([{ uid: "swe-2-high" }]);
361478
const own = crypto.randomUUID();
362479
await run("swe-2-high", {}, {}, undefined, new Headers({ session_id: "session-" + own }), { _codexOwnThreadId: own, _clientThreadId: "parent-" + own });
363480
await run("swe-2-high", {}, {}, undefined, new Headers({ "thread-id": own }));
364481
await run("swe-2-high", {}, {}, undefined, new Headers({ "thread-id": "explicit-" + own }), { _codexOwnThreadId: own });
365482
await run("swe-2-high", {}, {}, undefined, new Headers({ "x-codex-parent-thread-id": "parent-" + own }), { _codexOwnThreadId: own, _clientThreadId: "parent-" + own });
366-
const [internal, header, explicit, handedOff] = sentTrajectories();
483+
await run("swe-2-high", {}, {}, undefined, new Headers({ "x-codex-parent-thread-id": "parent-" + own }), { _codexOwnThreadId: own, _clientThreadId: "parent-" + own });
484+
const [internal, header, explicit, handedOff, handedOffRepeat] = sentTrajectories();
367485
expect(header).toBe(internal);
368486
expect(explicit).not.toBe(internal);
369-
expect(handedOff).toBe(internal);
487+
expect(handedOff).not.toBe(internal);
488+
expect(handedOffRepeat).toBe(handedOff);
370489
});
371490

372491
test("parent-only requests with parsed parent identity remain unnamed", async () => {

‎tests/providers/devin-trajectory.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ describe("bounded Devin trajectory claims", () => {
55
const host = "https://server.codeium.com";
66
const namespace = () => crypto.randomUUID();
77

8+
function releasedId(own: string, parent?: string): string | undefined {
9+
// Release sequential claims so overlap fallback cannot mask a key collision.
10+
const claim = claimDevinTrajectory("fixture", host, own, parent);
11+
try {
12+
expect(claim.trajectoryId).toMatch(/^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/);
13+
return claim.trajectoryId;
14+
} finally { claim.release(); }
15+
}
16+
817
test("unnamed claims leave allocation to the wire", () => {
918
for (const name of [undefined, null, ""]) {
1019
const claim = claimDevinTrajectory("fixture", host, name);
@@ -36,6 +45,61 @@ describe("bounded Devin trajectory claims", () => {
3645
try { expect(b.trajectoryId).not.toBe(a.trajectoryId); } finally { b.release(); }
3746
});
3847

48+
test("released claims separate supplied parents for the same child and retain each parent's UUID", () => {
49+
const own = namespace();
50+
const parentA = "parent-a-" + own;
51+
const parentB = "parent-b-" + own;
52+
const a = releasedId(own, parentA);
53+
const b = releasedId(own, parentB);
54+
expect(releasedId(own, parentA)).toBe(a);
55+
expect(releasedId(own, parentB)).toBe(b);
56+
expect(b, "same child in different supplied parents must not collide").not.toBe(a);
57+
});
58+
59+
test("parent/own delimiter pairs remain distinct after sequential release", () => {
60+
const prefix = namespace();
61+
// A colon join would make both identities `${prefix}:a:b:c`.
62+
const a = releasedId("c", prefix + ":a:b");
63+
const b = releasedId("b:c", prefix + ":a");
64+
expect(releasedId("c", prefix + ":a:b")).toBe(a);
65+
expect(releasedId("b:c", prefix + ":a")).toBe(b);
66+
expect(b).not.toBe(a);
67+
});
68+
69+
test("a standalone serialized tagged child tuple cannot impersonate the actual child identity", () => {
70+
const own = namespace();
71+
const parent = "parent-" + own;
72+
const literal = JSON.stringify(["codex-child", parent, own]);
73+
const standalone = releasedId(literal);
74+
const child = releasedId(own, parent);
75+
expect(releasedId(literal)).toBe(standalone);
76+
expect(releasedId(own, parent)).toBe(child);
77+
expect(child).not.toBe(standalone);
78+
});
79+
80+
test("JSON-like and delimiter-bearing parents remain distinct for one own ID", () => {
81+
const prefix = namespace();
82+
const own = '["own",":\\u001f",{"nested":true}]';
83+
const parentA = prefix + ':["parent",{"a":"b:c"}]';
84+
const parentB = prefix + ':["parent",{"a:b":"c"}]';
85+
const a = releasedId(own, parentA);
86+
const b = releasedId(own, parentB);
87+
expect(releasedId(own, parentA)).toBe(a);
88+
expect(releasedId(own, parentB)).toBe(b);
89+
expect(b).not.toBe(a);
90+
});
91+
92+
test("JSON-like own IDs remain distinct under one delimiter-bearing parent", () => {
93+
const parent = namespace() + ':parent\x1f["codex-child",{}]';
94+
const ownA = '["own",{"a":"b:c"}]';
95+
const ownB = '["own",{"a:b":"c"}]';
96+
const a = releasedId(ownA, parent);
97+
const b = releasedId(ownB, parent);
98+
expect(releasedId(ownA, parent)).toBe(a);
99+
expect(releasedId(ownB, parent)).toBe(b);
100+
expect(b).not.toBe(a);
101+
});
102+
39103
test("three overlapping claims are distinct and cannot release the retained owner", () => {
40104
const name = namespace();
41105
const owner = claimDevinTrajectory("fixture", host, name);

0 commit comments

Comments
 (0)