Skip to content

Commit a4d20d9

Browse files
committed
Drop only the colliding error record on a duplicate flush
A stale-seq assembly flushing [seq0/dup, seq1/fresh] discarded the whole batch, losing fresh diagnostics. The flush now maps the colliding key back to its record, drops only that record, and retries the rest in the same flush.
1 parent bc81652 commit a4d20d9

3 files changed

Lines changed: 136 additions & 13 deletions

File tree

‎vendor/intx-agent/PATCHES.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,12 @@ entry and its marker.
2323
## agent-ts-duplicate-error-flush
2424

2525
`agent.ts` — `flushErrors` treats `Duplicate error record:` from
26-
`commitErrors` as already-durable and drops the colliding batch instead
27-
of failing `afterCheckpoint`.
26+
`commitErrors` as already-durable instead of failing `afterCheckpoint`.
27+
Only the record named by the colliding `<sessionId>/<seq>-<category>`
28+
key is dropped; the rest of the batch is retried in the same flush, so
29+
a stale-seq assembly flushing `[seq0/dup, seq1/fresh]` still persists
30+
the fresh record. An unparseable key falls back to dropping the batch.
31+
Identical-bytes exact retries still commit normally upstream.
2832

2933
**Disposition:** Promotion candidate. **Removal path:** Upstream PR with
3034
the same duplicate-flush handling; then drop this entry and its marker.

‎vendor/intx-agent/src/agent.ts‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,24 @@ export async function createAgent<EnvReq extends BaseEnv>(
526526
let flushInProgress: Promise<void> | undefined;
527527
let pendingFollowUp: Promise<void> | undefined;
528528

529+
// File key mirror of the isogit store's error filename scheme
530+
// (`state/errors/<sessionId>/<seq>-<category>.json`, seq padded to
531+
// 8, unsafe category chars replaced). Maps a
532+
// `Duplicate error record: <key>` collision back to the batch member
533+
// that caused it so only that record is dropped.
534+
function errorFileKey(record: ErrorRecord): string {
535+
const seq = String(record.seq).padStart(8, "0");
536+
const category = record.category.replace(/[^a-zA-Z0-9_-]/g, "_");
537+
return `${record.sessionId}/${seq}-${category}`;
538+
}
539+
540+
function dropFromAccumulator(records: readonly ErrorRecord[]): void {
541+
for (const record of records) {
542+
const index = accumulatedErrors.indexOf(record);
543+
if (index !== -1) accumulatedErrors.splice(index, 1);
544+
}
545+
}
546+
529547
function flushErrors(): Promise<void> {
530548
if (flushInProgress !== undefined) {
531549
// If another caller already arranged a follow-up flush after
@@ -559,19 +577,39 @@ export async function createAgent<EnvReq extends BaseEnv>(
559577
// expectation is that commitErrors failures are transient.
560578
flushInProgress = (async () => {
561579
try {
562-
await auditStore.commitErrors(batch);
563-
accumulatedErrors.splice(0, count);
564-
} catch (cause) {
565-
// Locally patched — see vendor/intx-agent/PATCHES.md#agent-ts-duplicate-error-flush
566-
if (
567-
cause instanceof Error &&
568-
cause.message.startsWith("Duplicate error record:")
569-
) {
570-
logger.warn`duplicate error record already stored; dropping the colliding batch`;
571-
accumulatedErrors.splice(0, count);
580+
let remaining = batch;
581+
for (;;) {
582+
try {
583+
await auditStore.commitErrors(remaining);
584+
} catch (cause) {
585+
// Locally patched — see vendor/intx-agent/PATCHES.md#agent-ts-duplicate-error-flush
586+
if (
587+
cause instanceof Error &&
588+
cause.message.startsWith("Duplicate error record:")
589+
) {
590+
const key = cause.message
591+
.slice("Duplicate error record:".length)
592+
.trim();
593+
const index = remaining.findIndex(
594+
(record) => errorFileKey(record) === key,
595+
);
596+
if (index === -1) {
597+
logger.warn`duplicate error record already stored; dropping the colliding batch`;
598+
dropFromAccumulator(remaining);
599+
return;
600+
}
601+
const [colliding] = remaining.splice(index, 1);
602+
if (colliding !== undefined)
603+
dropFromAccumulator([colliding]);
604+
logger.warn`duplicate error record already stored; dropping the colliding record`;
605+
if (remaining.length === 0) return;
606+
continue;
607+
}
608+
throw cause;
609+
}
610+
dropFromAccumulator(remaining);
572611
return;
573612
}
574-
throw cause;
575613
} finally {
576614
flushInProgress = undefined;
577615
}

‎vendor/intx-agent/src/flush-errors.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,44 @@ function makeDuplicateErrorAuditStore(): FailingAuditStore {
126126
};
127127
}
128128

129+
// Audit store that collides on the first batch's leading record only,
130+
// simulating a stale-seq assembly flushing [seq0/dup, seq1/fresh]: the
131+
// first `commitErrors` throws `Duplicate error record` naming the
132+
// colliding record's file key, and the retry succeeds.
133+
function makePartialDuplicateAuditStore(): FailingAuditStore {
134+
const committedErrors: ErrorRecord[][] = [];
135+
let firstAttempt = true;
136+
return {
137+
async commitAudit(_records: AuditRecord[]): Promise<void> {
138+
// No-op.
139+
},
140+
async commitErrors(records: ErrorRecord[]): Promise<void> {
141+
if (firstAttempt) {
142+
firstAttempt = false;
143+
const colliding = records[0];
144+
const seq = String(colliding?.seq ?? 0).padStart(8, "0");
145+
const category = (colliding?.category ?? "").replace(
146+
/[^a-zA-Z0-9_-]/g,
147+
"_",
148+
);
149+
throw new Error(
150+
`Duplicate error record: ${colliding?.sessionId ?? "session"}/${seq}-${category}`,
151+
);
152+
}
153+
committedErrors.push([...records]);
154+
},
155+
async loadAudit(_sessionId: string): Promise<AuditRecord[]> {
156+
return [];
157+
},
158+
async loadErrors(_sessionId: string): Promise<ErrorRecord[]> {
159+
return [];
160+
},
161+
getCommittedErrors() {
162+
return committedErrors;
163+
},
164+
};
165+
}
166+
129167
// Director factory that closes over a caller-supplied `decide` to drive
130168
// the reactor through targeted event shapes. The factory shape requires
131169
// a configSchema (arktype) and returns a ReactorDirector; this helper
@@ -677,6 +715,49 @@ describe("agent error flushing", () => {
677715
expect(events.some((event) => event.type === "reactor.done")).toBe(true);
678716
});
679717

718+
test("a partial duplicate collision drops only the colliding record", async () => {
719+
// A stale-seq assembly flushing [seq0/dup, seq1/fresh] must persist
720+
// the fresh record: the first commit names only the colliding key,
721+
// so the flush drops that record and retries the rest.
722+
const audit = makePartialDuplicateAuditStore();
723+
let inferenceErrors = 0;
724+
const directors = makeDirectorRegistry(
725+
async (
726+
event: ReactorInboundEvent,
727+
_state: ReactorState,
728+
caps: ReactorCapabilities,
729+
) => {
730+
if (event.type === "message.received") return caps.infer();
731+
if (event.type === "inference.error") {
732+
inferenceErrors += 1;
733+
if (inferenceErrors === 1) return caps.infer();
734+
return [caps.checkpoint("after-second"), caps.done()];
735+
}
736+
return caps.done();
737+
},
738+
);
739+
const def = forbiddenAgentDef("cred-flush-partial-duplicate");
740+
const env = await buildAgentEnv({ workdir: workDir, audit, directors });
741+
const agent = await createAgent(def, { ...env, deps: FORBIDDEN_DEPS });
742+
const events: Array<{ type: string; data?: unknown }> = [];
743+
const stream = agent.stream();
744+
try {
745+
agent.deliver(inboundConversation());
746+
for await (const event of stream) {
747+
events.push(event);
748+
if (event.type === "reactor.done") break;
749+
}
750+
} finally {
751+
await agent.close();
752+
}
753+
754+
expect(duplicateFlushFailures(events)).toEqual([]);
755+
expect(events.some((event) => event.type === "reactor.done")).toBe(true);
756+
const persisted = audit.getCommittedErrors().flat();
757+
expect(persisted).toHaveLength(1);
758+
expect(persisted[0]?.seq).toBe(1);
759+
});
760+
680761
test("createAgent still assembles when loadErrors throws", async () => {
681762
const audit = makeRecordingAuditStore();
682763
audit.loadErrors = async () => {

0 commit comments

Comments
 (0)