From ea820a528079c064336153f34d64bcc0c267d88e Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 00:55:15 +0800 Subject: [PATCH 1/4] test(runtime-host): give the owned settle assertion a CI-safe bound The owned Host test closed its only connection and required the process to settle within 500 ms. Shutdown takes about 30 ms on an idle machine, but under a full CI suite on a 4-vCPU runner the Host is starved for CPU and the same clean exit lands past 500 ms, so the assertion reported false without any product regression. PR #3221 widened the connection half of this test for the same reason. The promptness claim is that an owned launch exits on its idleGraceMs of 0 instead of the 30 s default grace. A 5 s bound still separates those by six times while covering the starvation the CI suite produces, and it matches the settle budget the rest of the file already uses. Measuring from a close acknowledgement would not help: the Client close is a local abort with no reply, and the slow part is the shutdown work that follows it inside the Host. Refs #3190 Generated-by: Claude Code --- packages/runtime-host/src/__tests__/owned-candidate.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/runtime-host/src/__tests__/owned-candidate.test.ts b/packages/runtime-host/src/__tests__/owned-candidate.test.ts index 83f9e212e7..3c264f145d 100644 --- a/packages/runtime-host/src/__tests__/owned-candidate.test.ts +++ b/packages/runtime-host/src/__tests__/owned-candidate.test.ts @@ -258,7 +258,11 @@ test('owned Host exits promptly after its first connection closes', async () => assert.equal(result.kind, 'connected', connectFailure(result)); if (result.kind !== 'connected') return; await result.connection.close(); - assert.equal(await result.host.settle(500), true); + // Prompt means the owned launch's idleGraceMs of 0, as opposed to the 30 s + // default grace, so the bound only has to sit well below that. Shutdown takes + // about 30 ms on an idle machine and stretches past 500 ms under a full CI + // suite while still exiting cleanly: the Host is starved, not stuck. + assert.equal(await result.host.settle(5_000), true); }); test('an exited owned Candidate permits one real successor in the same election', { From 1a1fc79842c8f4c7e3dafdff48fae717af2eac0a Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 01:03:39 +0800 Subject: [PATCH 2/4] fix(runtime-host): claim a Daily Review archive before its reads Two Clients running the same Daily Review at once could each get their own archive. The coordinator consulted its in-flight map only after awaiting the summary, the existing-archive read, and the config read, so under CI load the first request finished and published while the second sat between its existence check and its in-flight check, then generated again. The two-client UDS test caught this on main as generatedAt values 6 ms apart. The claim now happens before the first await. The archive id derives synchronously from the clock and the requested day, so a second request for the same archive joins the running generation or conflicts on different options. Joining compares the requested modelKeyOverride and trigger instead of the resolved model key: the resolved key is only known after the config read, and reading it later is the same race. The regression test holds the second request's session read until the first has published; without the fix the model runs twice. Generated-by: Claude Code --- .../daily-review-coordinator.test.ts | 72 ++++++++++++++++++- .../src/server/daily-review-coordinator.ts | 56 +++++++++------ 2 files changed, 106 insertions(+), 22 deletions(-) diff --git a/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts b/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts index c781604f41..eec17b3eaf 100644 --- a/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts @@ -153,6 +153,73 @@ test('Daily Review conflicts rather than coalescing different generation options ); }); +test('Daily Review joins an in-flight generation even when its own reads land later', async () => { + let releaseLaggingRead: (() => void) | undefined; + const laggingRead = new Promise((resolve) => { + releaseLaggingRead = resolve; + }); + let listCalls = 0; + let modelCalls = 0; + + await withCoordinator( + async ({ coordinator, usage }) => { + const now = Date.now(); + await usage.telemetry.recordLlmCall({ + id: 'daily-review-join-source', + callKind: 'main', + callId: 'daily-review-join-source', + connectionSlug: 'test', + providerId: 'test', + modelId: 'test', + inputTokens: 1, + outputTokens: 1, + cacheHitInputTokens: 0, + cacheMissInputTokens: 1, + cachedInputTokens: 0, + cacheWriteInputTokens: 0, + reasoningTokens: 0, + totalTokens: 2, + costUsd: 0, + latencyMs: 1, + status: 'success', + startedAt: now, + date: new Date(now).toISOString().slice(0, 10), + ts: now, + }); + const run = { + kind: 'run' as const, + range: 1 as const, + offsetDays: 0, + modelKeyOverride: 'provider::model-a', + replaceExisting: true, + }; + // Two Clients ask for the same archive at once. Whichever finishes first + // releases the other's lagging read, so the laggard only reaches its own + // bookkeeping after the leader has already published. + const first = coordinator.handlers['daily-review.mutate'](run, CONTEXT); + const second = coordinator.handlers['daily-review.mutate'](run, CONTEXT); + void Promise.race([first, second]).then(() => releaseLaggingRead?.()); + const [firstResult, secondResult] = await Promise.all([first, second]); + assert.equal(modelCalls, 1); + assert.deepEqual(secondResult, firstResult); + }, + { + generate: async () => { + modelCalls += 1; + return { ok: false, errorClass: 'configuration' }; + }, + }, + true, + { + list: async () => { + listCalls += 1; + if (listCalls === 2) await laggingRead; + return []; + }, + }, + ); +}); + test('Daily Review does not coalesce cron and manual archive provenance', async () => { let releaseModel: (() => void) | undefined; let notifyModelStarted: (() => void) | undefined; @@ -275,6 +342,9 @@ async function withCoordinator( generate: async () => ({ ok: false, errorClass: 'configuration' }), }, recoverBeforeRun = true, + sessions: ConstructorParameters[0]['sessions'] = { + list: async () => [], + }, ): Promise { const base = await mkdtemp(join(tmpdir(), 'maka-daily-review-coordinator-')); const root = join(base, 'interactive'); @@ -291,7 +361,7 @@ async function withCoordinator( const coordinator = new HostDailyReviewCoordinator({ store, usage, - sessions: { list: async () => [] }, + sessions, model, acquireResidency: () => ({ release: () => undefined }), requestDrain: () => { diff --git a/packages/runtime-host/src/server/daily-review-coordinator.ts b/packages/runtime-host/src/server/daily-review-coordinator.ts index 864e507b01..fa6dd55189 100644 --- a/packages/runtime-host/src/server/daily-review-coordinator.ts +++ b/packages/runtime-host/src/server/daily-review-coordinator.ts @@ -30,6 +30,7 @@ import { type DailyReviewArchiveSectionContent, type DailyReviewRange, type DailyReviewSummary, + type DayRangeMs, } from '@maka/core/daily-review'; import { collapseSessionRevisions } from '@maka/core/session-revisions'; import { mergeUsageBuckets, mergeUsageSummary } from '@maka/core/usage-ledger-merge'; @@ -90,7 +91,7 @@ export class HostDailyReviewCoordinator { readonly #inFlight = new Map< string, { - readonly modelKey: string; + readonly modelKeyOverride: string; readonly trigger: 'cron' | 'manual'; readonly promise: Promise; } @@ -170,11 +171,13 @@ export class HostDailyReviewCoordinator { const snapshot = await this.#store.readConfig(); return querySuccess({ kind: 'config', ...snapshot }); } - case 'summary': + case 'summary': { + const now = this.#now(); return querySuccess({ kind: 'summary', - summary: await this.#buildSummary(input.offsetDays, input.daySpan), + summary: await this.#buildSummary(dayRange(now, input.offsetDays, input.daySpan), now), }); + } case 'archives': { const beforeArchiveId = input.beforeArchiveId; const page = await this.#store.listArchivePage(beforeArchiveId, input.limit); @@ -247,13 +250,7 @@ export class HostDailyReviewCoordinator { } } - async #buildSummary(offsetDays: number, daySpan: number): Promise { - const offset = Math.trunc(offsetDays); - const span = Math.max(1, Math.min(30, Math.trunc(daySpan))); - const now = this.#now(); - const endDay = offset === 0 ? localDayBoundsForInstant(now) : localDayBoundsAt(now, offset); - const startDay = localDayBoundsAt(endDay.fromMs, -(span - 1)); - const range = { fromMs: startDay.fromMs, toMs: endDay.toMs }; + async #buildSummary(range: DayRangeMs, now: number): Promise { const query = dailyUsageQuery(range); const canonical = await readCompleteCanonicalUsage(this.#usage, query, now); const [usageSummary, toolBuckets, modelBuckets, sessions] = await Promise.all([ @@ -285,21 +282,23 @@ export class HostDailyReviewCoordinator { readonly trigger: 'cron' | 'manual'; readonly replaceExisting: boolean; }): Promise { - const summary = await this.#buildSummary(input.offsetDays, input.range); - const archiveId = dailyReviewArchiveId(summary.day, input.range); - const existing = await this.#store.getArchive(archiveId); - if (existing && !input.replaceExisting) return existing; - const config = await this.#store.readConfig(); - const modelKey = input.modelKeyOverride.trim() || config.config.modelKey; + const now = this.#now(); + const day = dayRange(now, input.offsetDays, input.range); + const archiveId = dailyReviewArchiveId(day, input.range); + const modelKeyOverride = input.modelKeyOverride.trim(); + // Claim the archive before the first await. Two Clients asking for the + // same archive at once share one generation; a claim taken only after the + // reads let the second request slip past a first that had already + // published, and each Client then saw its own archive. const inFlight = this.#inFlight.get(archiveId); if (inFlight) { - if (inFlight.modelKey === modelKey && inFlight.trigger === input.trigger) { + if (inFlight.modelKeyOverride === modelKeyOverride && inFlight.trigger === input.trigger) { return inFlight.promise; } throw new DailyReviewRunConflictError(archiveId); } - const pending = this.#generateArchive(archiveId, summary, modelKey, input); - const entry = { modelKey, trigger: input.trigger, promise: pending }; + const pending = this.#generateArchive(archiveId, day, now, modelKeyOverride, input); + const entry = { modelKeyOverride, trigger: input.trigger, promise: pending }; this.#inFlight.set(archiveId, entry); try { return await pending; @@ -310,13 +309,20 @@ export class HostDailyReviewCoordinator { async #generateArchive( archiveId: string, - summary: DailyReviewSummary, - modelKey: string, + day: DayRangeMs, + now: number, + modelKeyOverride: string, input: { readonly range: DailyReviewRange; readonly trigger: 'cron' | 'manual'; + readonly replaceExisting: boolean; }, ): Promise { + const summary = await this.#buildSummary(day, now); + const existing = await this.#store.getArchive(archiveId); + if (existing && !input.replaceExisting) return existing; + const config = await this.#store.readConfig(); + const modelKey = modelKeyOverride || config.config.modelKey; const base = { id: archiveId, day: summary.day, @@ -457,6 +463,14 @@ export class HostDailyReviewCoordinator { } } +function dayRange(nowMs: number, offsetDays: number, daySpan: number): DayRangeMs { + const offset = Math.trunc(offsetDays); + const span = Math.max(1, Math.min(30, Math.trunc(daySpan))); + const endDay = offset === 0 ? localDayBoundsForInstant(nowMs) : localDayBoundsAt(nowMs, offset); + const startDay = localDayBoundsAt(endDay.fromMs, -(span - 1)); + return { fromMs: startDay.fromMs, toMs: endDay.toMs }; +} + function scheduledTimeHasPassed(nowMs: number, executeTime: string): boolean { const now = new Date(nowMs); const [hours, minutes] = executeTime.split(':').map(Number); From cc7cac9ca9f842df477b2bc889cc1f1965fab51b Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 01:22:57 +0800 Subject: [PATCH 3/4] fix(runtime-host): let a Daily Review replace outlive a non-replacing leader The early claim treated replaceExisting true and false as one operation, so a replace that arrived while a non-replacing run held the claim joined it and received the archive that run merely found, with no regeneration. Before the claim moved ahead of the reads, the replace regenerated after the other run returned. A replace now waits for a non-replacing leader to finish and then claims the archive itself. Non-replacing runs still join anything compatible, and a replace joins a replace. Reporting a conflict instead would have turned a flow that used to work into an error. Generated-by: Claude Code --- .../daily-review-coordinator.test.ts | 56 ++++++++++++++++++- .../src/server/daily-review-coordinator.ts | 19 +++++-- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts b/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts index eec17b3eaf..f6b9bbbb8a 100644 --- a/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts @@ -22,7 +22,12 @@ import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { test } from 'node:test'; -import { localDayBoundsAt, type DailyReviewArchive } from '@maka/core/daily-review'; +import { + dailyReviewArchiveId, + localDayBoundsAt, + localDayBoundsForInstant, + type DailyReviewArchive, +} from '@maka/core/daily-review'; import { openInteractiveDailyReviewAuthorityForWrite } from '@maka/storage/daily-review-authority'; import { acquireOperationalStateDatabase } from '@maka/storage/operational-state-store'; import { resolveStorageRoot, tryAcquireInteractiveRootOwner } from '@maka/storage/root-authority'; @@ -220,6 +225,55 @@ test('Daily Review joins an in-flight generation even when its own reads land la ); }); +test('Daily Review regenerates for a replace that arrives during a non-replacing run', async () => { + await withCoordinator(async ({ coordinator, store }) => { + const archiveId = dailyReviewArchiveId(localDayBoundsForInstant(Date.now()), 1); + const existing = await store.publishArchive(archive(archiveId), 180); + const run = { + kind: 'run' as const, + range: 1 as const, + offsetDays: 0, + modelKeyOverride: '', + replaceExisting: false, + }; + const keep = coordinator.handlers['daily-review.mutate'](run, CONTEXT); + const replace = coordinator.handlers['daily-review.mutate']( + { ...run, replaceExisting: true }, + CONTEXT, + ); + const [kept, replaced] = await Promise.all([keep, replace]); + assert.deepEqual(kept, { ok: true, result: { kind: 'archive', archive: existing } }); + assert.equal(replaced.ok, true); + if (!replaced.ok || replaced.result.kind !== 'archive') return; + assert.equal(replaced.result.archive.status, 'no_data'); + assert.deepEqual(await store.getArchive(archiveId), replaced.result.archive); + }); +}); + +test('Daily Review lets a non-replacing run join a replace already in flight', async () => { + await withCoordinator(async ({ coordinator, store }) => { + const archiveId = dailyReviewArchiveId(localDayBoundsForInstant(Date.now()), 1); + await store.publishArchive(archive(archiveId), 180); + const run = { + kind: 'run' as const, + range: 1 as const, + offsetDays: 0, + modelKeyOverride: '', + replaceExisting: true, + }; + const replace = coordinator.handlers['daily-review.mutate'](run, CONTEXT); + const keep = coordinator.handlers['daily-review.mutate']( + { ...run, replaceExisting: false }, + CONTEXT, + ); + const [replaced, kept] = await Promise.all([replace, keep]); + assert.equal(replaced.ok, true); + if (!replaced.ok || replaced.result.kind !== 'archive') return; + assert.equal(replaced.result.archive.status, 'no_data'); + assert.deepEqual(kept, replaced); + }); +}); + test('Daily Review does not coalesce cron and manual archive provenance', async () => { let releaseModel: (() => void) | undefined; let notifyModelStarted: (() => void) | undefined; diff --git a/packages/runtime-host/src/server/daily-review-coordinator.ts b/packages/runtime-host/src/server/daily-review-coordinator.ts index fa6dd55189..db798cc3a6 100644 --- a/packages/runtime-host/src/server/daily-review-coordinator.ts +++ b/packages/runtime-host/src/server/daily-review-coordinator.ts @@ -93,6 +93,7 @@ export class HostDailyReviewCoordinator { { readonly modelKeyOverride: string; readonly trigger: 'cron' | 'manual'; + readonly replaceExisting: boolean; readonly promise: Promise; } >(); @@ -292,13 +293,23 @@ export class HostDailyReviewCoordinator { // published, and each Client then saw its own archive. const inFlight = this.#inFlight.get(archiveId); if (inFlight) { - if (inFlight.modelKeyOverride === modelKeyOverride && inFlight.trigger === input.trigger) { - return inFlight.promise; + if (inFlight.modelKeyOverride !== modelKeyOverride || inFlight.trigger !== input.trigger) { + throw new DailyReviewRunConflictError(archiveId); } - throw new DailyReviewRunConflictError(archiveId); + if (!input.replaceExisting || inFlight.replaceExisting) return inFlight.promise; + // A non-replacing leader may hand back an archive it merely found. A + // replace must not inherit that, so it waits its turn and claims for + // itself. + await inFlight.promise.catch(() => undefined); + return this.#run(input); } const pending = this.#generateArchive(archiveId, day, now, modelKeyOverride, input); - const entry = { modelKeyOverride, trigger: input.trigger, promise: pending }; + const entry = { + modelKeyOverride, + trigger: input.trigger, + replaceExisting: input.replaceExisting, + promise: pending, + }; this.#inFlight.set(archiveId, entry); try { return await pending; From ba702bed6caa2ba6da8e00cd8b7c4e20c74d3f0f Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 01:35:19 +0800 Subject: [PATCH 4/4] chore(runtime-host): state the Daily Review join key and release a rejected leader's laggard Review nits on #4672: the code said the claim happens before the first await but not why requests match on the requested override rather than the resolved model key, and the lagging-read test only released its gate when the leader resolved, so a rejected leader would leave close() waiting on the laggard for the whole suite. Generated-by: Claude Code --- .../src/__tests__/daily-review-coordinator.test.ts | 8 +++++--- .../runtime-host/src/server/daily-review-coordinator.ts | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts b/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts index f6b9bbbb8a..9df8437004 100644 --- a/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/daily-review-coordinator.test.ts @@ -198,12 +198,14 @@ test('Daily Review joins an in-flight generation even when its own reads land la modelKeyOverride: 'provider::model-a', replaceExisting: true, }; - // Two Clients ask for the same archive at once. Whichever finishes first + // Two Clients ask for the same archive at once. Whichever settles first // releases the other's lagging read, so the laggard only reaches its own - // bookkeeping after the leader has already published. + // bookkeeping after the leader has already published. A rejected leader + // releases it too; otherwise close() would wait on the laggard forever. const first = coordinator.handlers['daily-review.mutate'](run, CONTEXT); const second = coordinator.handlers['daily-review.mutate'](run, CONTEXT); - void Promise.race([first, second]).then(() => releaseLaggingRead?.()); + const release = () => releaseLaggingRead?.(); + void Promise.race([first, second]).then(release, release); const [firstResult, secondResult] = await Promise.all([first, second]); assert.equal(modelCalls, 1); assert.deepEqual(secondResult, firstResult); diff --git a/packages/runtime-host/src/server/daily-review-coordinator.ts b/packages/runtime-host/src/server/daily-review-coordinator.ts index db798cc3a6..60f0564664 100644 --- a/packages/runtime-host/src/server/daily-review-coordinator.ts +++ b/packages/runtime-host/src/server/daily-review-coordinator.ts @@ -290,7 +290,9 @@ export class HostDailyReviewCoordinator { // Claim the archive before the first await. Two Clients asking for the // same archive at once share one generation; a claim taken only after the // reads let the second request slip past a first that had already - // published, and each Client then saw its own archive. + // published, and each Client then saw its own archive. Requests match on + // the override they asked for, not the resolved model key: resolving it + // needs the config read, which would put the claim back after an await. const inFlight = this.#inFlight.get(archiveId); if (inFlight) { if (inFlight.modelKeyOverride !== modelKeyOverride || inFlight.trigger !== input.trigger) {