Merge pull request #242 from suraj-markup/feat/issue-238

fix(core): atomic metadata writes, restoredAt persistence, and session count/sorting bugs
This commit is contained in:
suraj_markup 2026-03-02 02:24:51 +05:30 committed by GitHub
commit 2d785beac5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 212 additions and 9 deletions

View File

@ -323,6 +323,116 @@ describe("readArchivedMetadataRaw", () => {
});
});
describe("atomic writes", () => {
it("writeMetadata leaves no .tmp files behind", () => {
writeMetadata(dataDir, "atomic-1", {
worktree: "/tmp/w",
branch: "main",
status: "working",
});
const files = readdirSync(dataDir);
const tmpFiles = files.filter((f) => f.includes(".tmp."));
expect(tmpFiles).toHaveLength(0);
// Verify the actual file was written correctly
const meta = readMetadata(dataDir, "atomic-1");
expect(meta!.status).toBe("working");
});
it("updateMetadata leaves no .tmp files behind", () => {
writeMetadata(dataDir, "atomic-2", {
worktree: "/tmp/w",
branch: "main",
status: "spawning",
});
updateMetadata(dataDir, "atomic-2", { status: "working" });
const files = readdirSync(dataDir);
const tmpFiles = files.filter((f) => f.includes(".tmp."));
expect(tmpFiles).toHaveLength(0);
const meta = readMetadata(dataDir, "atomic-2");
expect(meta!.status).toBe("working");
});
it("concurrent writeMetadata calls do not produce corrupt files", () => {
// Simulate rapid sequential writes (synchronous, so they serialize naturally,
// but each individual write must be atomic — no partial content)
for (let i = 0; i < 20; i++) {
writeMetadata(dataDir, "atomic-3", {
worktree: "/tmp/w",
branch: `branch-${i}`,
status: "working",
summary: `iteration ${i}`,
});
}
const meta = readMetadata(dataDir, "atomic-3");
expect(meta).not.toBeNull();
expect(meta!.branch).toBe("branch-19");
expect(meta!.summary).toBe("iteration 19");
// No leftover temp files
const files = readdirSync(dataDir);
const tmpFiles = files.filter((f) => f.includes(".tmp."));
expect(tmpFiles).toHaveLength(0);
});
});
describe("restoredAt persistence", () => {
it("roundtrips restoredAt through writeMetadata and readMetadata", () => {
const now = new Date().toISOString();
writeMetadata(dataDir, "restore-1", {
worktree: "/tmp/w",
branch: "main",
status: "working",
restoredAt: now,
});
const meta = readMetadata(dataDir, "restore-1");
expect(meta).not.toBeNull();
expect(meta!.restoredAt).toBe(now);
});
it("restoredAt is persisted in the key=value file", () => {
const now = "2026-03-01T12:00:00.000Z";
writeMetadata(dataDir, "restore-2", {
worktree: "/tmp/w",
branch: "main",
status: "working",
restoredAt: now,
});
const content = readFileSync(join(dataDir, "restore-2"), "utf-8");
expect(content).toContain(`restoredAt=${now}`);
});
it("restoredAt is undefined when not set", () => {
writeMetadata(dataDir, "restore-3", {
worktree: "/tmp/w",
branch: "main",
status: "working",
});
const meta = readMetadata(dataDir, "restore-3");
expect(meta!.restoredAt).toBeUndefined();
});
it("updateMetadata can set restoredAt on an existing session", () => {
writeMetadata(dataDir, "restore-4", {
worktree: "/tmp/w",
branch: "main",
status: "working",
});
const now = new Date().toISOString();
updateMetadata(dataDir, "restore-4", { restoredAt: now });
const meta = readMetadata(dataDir, "restore-4");
expect(meta!.restoredAt).toBe(now);
});
});
describe("listMetadata", () => {
it("lists all session IDs", () => {
writeMetadata(dataDir, "app-1", { worktree: "/tmp", branch: "a", status: "s" });

View File

@ -22,6 +22,7 @@
import {
readFileSync,
writeFileSync,
renameSync,
existsSync,
mkdirSync,
unlinkSync,
@ -63,6 +64,16 @@ function serializeMetadata(data: Record<string, string>): string {
);
}
/**
* Atomically write a file by writing to a temp file then renaming.
* rename() is atomic on POSIX, so concurrent writers never produce torn data.
*/
function atomicWriteFileSync(filePath: string, content: string): void {
const tmpPath = `${filePath}.tmp.${process.pid}.${Date.now()}`;
writeFileSync(tmpPath, content, "utf-8");
renameSync(tmpPath, filePath);
}
/** Validate sessionId to prevent path traversal. */
const VALID_SESSION_ID = /^[a-zA-Z0-9_-]+$/;
@ -100,6 +111,7 @@ export function readMetadata(dataDir: string, sessionId: SessionId): SessionMeta
agent: raw["agent"],
createdAt: raw["createdAt"],
runtimeHandle: raw["runtimeHandle"],
restoredAt: raw["restoredAt"],
role: raw["role"],
dashboardPort: raw["dashboardPort"] ? Number(raw["dashboardPort"]) : undefined,
terminalWsPort: raw["terminalWsPort"] ? Number(raw["terminalWsPort"]) : undefined,
@ -144,6 +156,7 @@ export function writeMetadata(
if (metadata.agent) data["agent"] = metadata.agent;
if (metadata.createdAt) data["createdAt"] = metadata.createdAt;
if (metadata.runtimeHandle) data["runtimeHandle"] = metadata.runtimeHandle;
if (metadata.restoredAt) data["restoredAt"] = metadata.restoredAt;
if (metadata.role) data["role"] = metadata.role;
if (metadata.dashboardPort !== undefined)
data["dashboardPort"] = String(metadata.dashboardPort);
@ -152,7 +165,7 @@ export function writeMetadata(
if (metadata.directTerminalWsPort !== undefined)
data["directTerminalWsPort"] = String(metadata.directTerminalWsPort);
writeFileSync(path, serializeMetadata(data), "utf-8");
atomicWriteFileSync(path, serializeMetadata(data));
}
/**
@ -183,7 +196,7 @@ export function updateMetadata(
}
mkdirSync(dirname(path), { recursive: true });
writeFileSync(path, serializeMetadata(existing), "utf-8");
atomicWriteFileSync(path, serializeMetadata(existing));
}
/**

View File

@ -152,6 +152,7 @@ function metadataToSession(
agentInfo: meta["summary"] ? { summary: meta["summary"], agentSessionId: null } : null,
createdAt: meta["createdAt"] ? new Date(meta["createdAt"]) : (createdAt ?? new Date()),
lastActivityAt: modifiedAt ?? new Date(),
restoredAt: meta["restoredAt"] ? new Date(meta["restoredAt"]) : undefined,
metadata: meta,
};
}

View File

@ -68,6 +68,21 @@ describe("getAttentionLevel", () => {
expect(getAttentionLevel(session)).toBe("respond");
});
it("returns respond when status is errored even if activity is active", () => {
const session = makeSession({ status: "errored", activity: "active" });
expect(getAttentionLevel(session)).toBe("respond");
});
it("returns respond when status is needs_input even if activity is active", () => {
const session = makeSession({ status: "needs_input", activity: "active" });
expect(getAttentionLevel(session)).toBe("respond");
});
it("returns respond when status is stuck even if activity is active", () => {
const session = makeSession({ status: "stuck", activity: "active" });
expect(getAttentionLevel(session)).toBe("respond");
});
it("merge takes priority over respond (mergeable PR + blocked agent)", () => {
const pr = makePR({
mergeability: {

View File

@ -229,7 +229,7 @@ function StatusLine({ stats }: { stats: DashboardStats }) {
const parts: Array<{ value: number; label: string; color?: string }> = [
{ value: stats.totalSessions, label: "sessions" },
...(stats.workingSessions > 0
? [{ value: stats.workingSessions, label: "active", color: "var(--color-status-working)" }]
? [{ value: stats.workingSessions, label: "working", color: "var(--color-status-working)" }]
: []),
...(stats.openPRs > 0 ? [{ value: stats.openPRs, label: "PRs" }] : []),
...(stats.needsReview > 0

View File

@ -20,6 +20,7 @@ import {
enrichSessionAgentSummary,
enrichSessionIssueTitle,
enrichSessionsMetadata,
computeStats,
} from "../serialize";
import { prCache, prCacheKey } from "../cache";
import type { DashboardSession } from "../types";
@ -947,6 +948,67 @@ describe("enrichSessionsMetadata", () => {
});
});
describe("computeStats", () => {
function makeDashboard(overrides: Partial<DashboardSession> = {}): DashboardSession {
return {
id: "test-1",
projectId: "test",
status: "working",
activity: "active",
branch: "feat/test",
issueId: null,
issueUrl: null,
issueLabel: null,
issueTitle: null,
summary: null,
summaryIsFallback: false,
createdAt: new Date().toISOString(),
lastActivityAt: new Date().toISOString(),
pr: null,
metadata: {},
...overrides,
};
}
it("counts active sessions as working", () => {
const sessions = [makeDashboard({ activity: "active" })];
expect(computeStats(sessions).workingSessions).toBe(1);
});
it("counts idle sessions as working", () => {
const sessions = [makeDashboard({ activity: "idle" })];
expect(computeStats(sessions).workingSessions).toBe(1);
});
it("counts ready sessions as working", () => {
const sessions = [makeDashboard({ activity: "ready" })];
expect(computeStats(sessions).workingSessions).toBe(1);
});
it("excludes exited sessions from working count", () => {
const sessions = [makeDashboard({ activity: "exited" })];
expect(computeStats(sessions).workingSessions).toBe(0);
});
it("excludes sessions with null activity from working count", () => {
const sessions = [makeDashboard({ activity: null })];
expect(computeStats(sessions).workingSessions).toBe(0);
});
it("counts mixed activity states correctly", () => {
const sessions = [
makeDashboard({ id: "s1", activity: "active" }),
makeDashboard({ id: "s2", activity: "idle" }),
makeDashboard({ id: "s3", activity: "ready" }),
makeDashboard({ id: "s4", activity: "exited" }),
makeDashboard({ id: "s5", activity: null }),
];
const stats = computeStats(sessions);
expect(stats.totalSessions).toBe(5);
expect(stats.workingSessions).toBe(3); // active + idle + ready
});
});
describe("basicPRToDashboard defaults", () => {
it("should not look like failing CI", () => {
const pr = createPRInfo();

View File

@ -380,7 +380,7 @@ export async function enrichSessionsMetadata(
export function computeStats(sessions: DashboardSession[]): DashboardStats {
return {
totalSessions: sessions.length,
workingSessions: sessions.filter((s) => s.activity === "active").length,
workingSessions: sessions.filter((s) => s.activity !== null && s.activity !== "exited").length,
openPRs: sessions.filter((s) => s.pr?.state === "open").length,
needsReview: sessions.filter((s) => s.pr && !s.pr.isDraft && s.pr.reviewDecision === "pending")
.length,

View File

@ -187,16 +187,18 @@ export function getAttentionLevel(session: DashboardSession): AttentionLevel {
}
// ── Respond: agent is waiting for human input ─────────────────────
// Check status-based error conditions first — these are authoritative
// and should not be masked by a stale activity value.
if (
session.activity === ACTIVITY_STATE.WAITING_INPUT ||
session.activity === ACTIVITY_STATE.BLOCKED
session.status === SESSION_STATUS.ERRORED ||
session.status === SESSION_STATUS.NEEDS_INPUT ||
session.status === SESSION_STATUS.STUCK
) {
return "respond";
}
if (
session.status === SESSION_STATUS.NEEDS_INPUT ||
session.status === SESSION_STATUS.STUCK ||
session.status === SESSION_STATUS.ERRORED
session.activity === ACTIVITY_STATE.WAITING_INPUT ||
session.activity === ACTIVITY_STATE.BLOCKED
) {
return "respond";
}