Fix activity-feed test flake: updateJob honors explicit patch.updatedAt; scoped test cleanup
Two-part root cause (found by the lh-ui stream + master together): (1) src/store/jobs.ts updateJob unconditionally overwrote updatedAt with now(), silently clobbering any explicit patch timestamp — JobPatch types it as patchable but the adapter ignored it, so order-sensitive tests rode on same-millisecond tie-breaking (failed ~4/5 in isolation). The store now honors patch.updatedAt ?? now() — semantics-preserving for every existing caller, which never passed updatedAt explicitly. (2) the activity-feed test's finally wiped the ENTIRE shared data/jobs dir — with node:test running test files as concurrent processes, that deletes other processes' live fixtures; cleanup is now scoped to the test's own created ids.
This commit is contained in:
Vendored
+2
-2
@@ -1,7 +1,7 @@
|
|||||||
/// <reference types="next" />
|
/// <reference types="next" />
|
||||||
/// <reference types="next/image-types/global" />
|
/// <reference types="next/image-types/global" />
|
||||||
import "./.next/dev/types/routes.d.ts";
|
import "./.next/types/routes.d.ts";
|
||||||
import "./.next/dev/types/root-params.d.ts";
|
import "./.next/types/root-params.d.ts";
|
||||||
|
|
||||||
// NOTE: This file should not be edited
|
// NOTE: This file should not be edited
|
||||||
// see https://nextjs.org/docs/app/api-reference/config/typescript for more information.
|
// see https://nextjs.org/docs/app/api-reference/config/typescript for more information.
|
||||||
|
|||||||
+4
-2
@@ -48,11 +48,13 @@ function getJob(id: string): JobRecord | null {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Merge a partial patch into the record (identity fields are immutable). */
|
/** Merge a partial patch into the record (identity fields are immutable).
|
||||||
|
* An explicit patch.updatedAt is honored (tests pin deterministic order);
|
||||||
|
* otherwise the merge bumps it to now. */
|
||||||
function updateJob(id: string, patch: JobPatch): JobRecord | null {
|
function updateJob(id: string, patch: JobPatch): JobRecord | null {
|
||||||
const record = getJob(id);
|
const record = getJob(id);
|
||||||
if (!record) return null;
|
if (!record) return null;
|
||||||
const next: JobRecord = { ...record, ...patch, updatedAt: new Date().toISOString() };
|
const next: JobRecord = { ...record, ...patch, updatedAt: patch.updatedAt ?? new Date().toISOString() };
|
||||||
fs.writeFileSync(jobFile(id), JSON.stringify(next, null, "\t"), "utf-8");
|
fs.writeFileSync(jobFile(id), JSON.stringify(next, null, "\t"), "utf-8");
|
||||||
return next;
|
return next;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,12 +18,19 @@ import { activityFeed } from "../src/server/observability.ts";
|
|||||||
test("activityFeed: running jobs across ALL chats, running-only, title-joined, oldest first", () => {
|
test("activityFeed: running jobs across ALL chats, running-only, title-joined, oldest first", () => {
|
||||||
const chatA = chatStore.createChat("feed chat A");
|
const chatA = chatStore.createChat("feed chat A");
|
||||||
const chatB = chatStore.createChat("feed chat B");
|
const chatB = chatStore.createChat("feed chat B");
|
||||||
|
// job ids tracked as created — the finally cleanup stays scoped to them even
|
||||||
|
// if the test throws mid-way (test files run as concurrent processes over
|
||||||
|
// the shared data/ scratch; wiping the whole jobs dir would delete other
|
||||||
|
// processes' live fixtures — the wave-4 flake lesson)
|
||||||
|
const createdJobs: string[] = [];
|
||||||
try {
|
try {
|
||||||
const jobA = jobStore.createJob({ kind: "research", chatId: chatA.id, toolCallId: "tc_a" });
|
const jobA = jobStore.createJob({ kind: "research", chatId: chatA.id, toolCallId: "tc_a" });
|
||||||
const jobB = jobStore.createJob({ kind: "demo", chatId: chatB.id, toolCallId: "tc_b" });
|
const jobB = jobStore.createJob({ kind: "demo", chatId: chatB.id, toolCallId: "tc_b" });
|
||||||
const jobDone = jobStore.createJob({ kind: "research", chatId: chatA.id, toolCallId: "tc_done" });
|
const jobDone = jobStore.createJob({ kind: "research", chatId: chatA.id, toolCallId: "tc_done" });
|
||||||
|
createdJobs.push(jobA.id, jobB.id, jobDone.id);
|
||||||
jobStore.updateJob(jobDone.id, { status: "done" });
|
jobStore.updateJob(jobDone.id, { status: "done" });
|
||||||
const jobB2 = jobStore.createJob({ kind: "research", chatId: chatB.id, toolCallId: "tc_b2" });
|
const jobB2 = jobStore.createJob({ kind: "research", chatId: chatB.id, toolCallId: "tc_b2" });
|
||||||
|
createdJobs.push(jobB2.id);
|
||||||
// make jobB2 the MOST recently active → should still sort AFTER jobA/jobB (oldest-first).
|
// make jobB2 the MOST recently active → should still sort AFTER jobA/jobB (oldest-first).
|
||||||
// Explicit later timestamp: the real bump can land in the same ms as creation.
|
// Explicit later timestamp: the real bump can land in the same ms as creation.
|
||||||
jobStore.updateJob(jobB2.id, { activity: "second step", updatedAt: new Date(Date.now() + 10).toISOString() });
|
jobStore.updateJob(jobB2.id, { activity: "second step", updatedAt: new Date(Date.now() + 10).toISOString() });
|
||||||
@@ -47,6 +54,8 @@ test("activityFeed: running jobs across ALL chats, running-only, title-joined, o
|
|||||||
fs.unlinkSync(`${PATHS.chats}/${chatA.id}.json`);
|
fs.unlinkSync(`${PATHS.chats}/${chatA.id}.json`);
|
||||||
fs.unlinkSync(`${PATHS.chats}/${chatB.id}.json`);
|
fs.unlinkSync(`${PATHS.chats}/${chatB.id}.json`);
|
||||||
const jobsDir = path.join(PATHS.data, "jobs");
|
const jobsDir = path.join(PATHS.data, "jobs");
|
||||||
for (const f of fs.readdirSync(jobsDir)) fs.unlinkSync(path.join(jobsDir, f));
|
for (const id of createdJobs) {
|
||||||
|
fs.unlinkSync(path.join(jobsDir, `${id}.json`));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user