Skip to content

Commit 3fb99b4

Browse files
committed
fix(examples-chat): retry checkpoint push only on 409, capped (review)
Address final-review findings: the retry now discriminates — only a mid-run 409 (conflict) is retried, and only up to MAX_PUSH_RETRIES, so a persistently failing thread (404/auth/500) can't spin an unbounded background loop. Track the retry timer, and correct the now-stale run-gated/500ms comment. Adds isConflict unit tests.
1 parent f3f29ed commit 3fb99b4

2 files changed

Lines changed: 43 additions & 11 deletions

File tree

examples/chat/angular/src/app/shell/demo-shell.component.spec.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
FakeStreamTransport,
1010
type AgentTransport,
1111
} from '@threadplane/langgraph';
12-
import { DemoShell, shouldSyncCheckpoint, extractItinerary } from './demo-shell.component';
12+
import { DemoShell, shouldSyncCheckpoint, extractItinerary, isConflict } from './demo-shell.component';
1313
import { DEMO_AGENT_REF } from './agent-ref';
1414
import { ItineraryStore, type ItineraryStop } from '../itinerary-store';
1515

@@ -543,6 +543,22 @@ describe('shouldSyncCheckpoint — push-gate predicate', () => {
543543
});
544544
});
545545

546+
describe('isConflict — retry only the mid-run 409', () => {
547+
it('is true for a 409 status', () => {
548+
expect(isConflict({ status: 409 })).toBe(true);
549+
});
550+
551+
it('is true when the message mentions 409/conflict', () => {
552+
expect(isConflict(new Error('Request failed: 409 Conflict'))).toBe(true);
553+
});
554+
555+
it('is false for other errors (terminal — do not retry)', () => {
556+
expect(isConflict({ status: 404 })).toBe(false);
557+
expect(isConflict(new Error('Unauthorized'))).toBe(false);
558+
expect(isConflict(null)).toBe(false);
559+
});
560+
});
561+
546562
describe('extractItinerary — value → stops', () => {
547563
it('returns the itinerary array from a state value', () => {
548564
const stops: ItineraryStop[] = [{ id: 'x', day: 1, place: 'Louvre' }];

examples/chat/angular/src/app/shell/demo-shell.component.ts

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,19 @@ export function extractItinerary(value: unknown): ItineraryStop[] | null {
112112
return null;
113113
}
114114

115+
/** Cap on checkpoint-push retries — a backstop so a persistently-failing thread
116+
* can't spin an unbounded background retry loop. */
117+
export const MAX_PUSH_RETRIES = 20;
118+
119+
/** A mid-run `updateState` returns HTTP 409 (the client-tool resume loop is
120+
* streaming); that is the ONLY error worth retrying. Any other failure
121+
* (404/auth/500) is terminal for this best-effort sync. */
122+
export function isConflict(err: unknown): boolean {
123+
const status = (err as { status?: number })?.status;
124+
if (status === 409) return true;
125+
return /\b409\b|conflict/i.test(String((err as { message?: string })?.message ?? err));
126+
}
127+
115128
@Component({
116129
selector: 'demo-shell',
117130
standalone: true,
@@ -272,31 +285,34 @@ export class DemoShell {
272285
this.itinerary.hydrate(incoming);
273286
});
274287

275-
// (3) Push the working copy to the checkpoint at run-settle and on user
276-
// edits between runs. Run-gated (never mid-run) + debounced (~500ms).
277-
// Depends on stops(), isLoading() (so it re-fires when a run SETTLES), and
278-
// threadIdState(). Errors are swallowed — this is a best-effort sync.
288+
// (3) Push the working copy to the durable checkpoint on every change.
289+
// NOT run-gated: the client-tool resume loop keeps the agent loading for
290+
// the whole plan, so a mid-run write returns 409 — retry ONLY that (bounded
291+
// by MAX_PUSH_RETRIES) until the run settles so the final itinerary always
292+
// lands. Debounced ~1200ms; onCleanup cancels a superseded attempt. Any
293+
// non-conflict error is terminal — this is a best-effort sync, not a queue.
279294
effect((onCleanup) => {
280295
const stops = this.itinerary.stops();
281296
const tid = threadIdState();
282297
const json = JSON.stringify(stops);
283298
if (!shouldSyncCheckpoint(tid, json, this.lastSyncedItinerary)) return;
284-
// Debounce, then push. A mid-run write returns 409 (the client-tool resume
285-
// loop is streaming); retry until the run settles so the FINAL itinerary
286-
// always lands in the checkpoint. onCleanup cancels a superseded attempt.
287299
let cancelled = false;
300+
let retries = 0;
301+
let timer: ReturnType<typeof setTimeout>;
288302
const attempt = async (): Promise<void> => {
289303
if (cancelled) return;
290304
try {
291305
await this.lgClient.threads.updateState(tid as string, {
292306
values: { itinerary: stops },
293307
});
294308
this.lastSyncedItinerary = json;
295-
} catch {
296-
if (!cancelled) setTimeout(() => void attempt(), 1500);
309+
} catch (err) {
310+
if (!cancelled && isConflict(err) && retries++ < MAX_PUSH_RETRIES) {
311+
timer = setTimeout(() => void attempt(), 1500);
312+
}
297313
}
298314
};
299-
const timer = setTimeout(() => void attempt(), 1200);
315+
timer = setTimeout(() => void attempt(), 1200);
300316
onCleanup(() => {
301317
cancelled = true;
302318
clearTimeout(timer);

0 commit comments

Comments
 (0)