Skip to content

Commit c74a99c

Browse files
fix(scripts): stop pruning run-log entries with non-ISO run_id, fix Windows test path (#519)
append-run-log.mjs pruned entries with new Date(obj.run_id).getTime() and dropped any that came back NaN-or-too-old. JS's Date constructor is lenient: a non-ISO run_id (a numeric GitHub run id, a custom slug like "run-1") doesn't reliably parse to NaN, it can parse into a spurious in-range-looking date instead ("run-1" -> 2001-01-01 in local time), which then reads as 30+ days old and gets silently deleted on the very next append -- even though the entry was just written. loop-metrics already handles this same run_id shape correctly (keep on unparseable rather than drop); this script did the opposite. Now only strings shaped like an ISO date are parsed as timestamps at all, so a non-ISO run_id is always kept. Also fixes append-run-log.test.mjs's own SCRIPT path, which used new URL(...).pathname -- on Windows that yields a leading-slash path ("/D:/...") that node's CLI mis-resolves relative to the current drive instead of as absolute, the same class of bug already fixed for tools/loop/test/files.test.mjs. Switched to fileURLToPath(), which is what let this fix's own regression test actually run and confirm the behavior locally. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 50eb9d0 commit c74a99c

2 files changed

Lines changed: 35 additions & 3 deletions

File tree

scripts/append-run-log.mjs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ import { readFile, writeFile } from 'node:fs/promises';
77

88
const MARKER = '<!-- Loop appends below this line -->';
99
const MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000;
10+
// Only strings shaped like an ISO date are treated as timestamps for pruning.
11+
// `new Date(...)` is a lenient parser: a non-ISO run_id (a numeric GitHub run
12+
// id, a custom slug like "run-1") doesn't reliably yield NaN, it can parse
13+
// into a spurious in-range-looking date instead (e.g. "run-1" -> 2001-01-01),
14+
// which would then read as "older than 30 days" and get silently pruned even
15+
// though the entry was just written. Gate the parse on this pattern first so
16+
// non-ISO run_ids are always kept, matching loop-metrics' handling of the
17+
// same run_id shapes.
18+
const ISO_DATE_RE = /^\d{4}-\d{2}-\d{2}/;
1019

1120
const entryJson = process.argv[2];
1221
const logPath = process.argv[3] || 'loop-run-log.md';
@@ -40,8 +49,8 @@ for (const line of after.split('\n')) {
4049
if (!trimmed.startsWith('{')) continue;
4150
try {
4251
const obj = JSON.parse(trimmed);
43-
const t = new Date(obj.run_id).getTime();
44-
if (!Number.isNaN(t) && now - t <= MAX_AGE_MS) {
52+
const t = ISO_DATE_RE.test(obj.run_id) ? new Date(obj.run_id).getTime() : NaN;
53+
if (Number.isNaN(t) || now - t <= MAX_AGE_MS) {
4554
kept.push(trimmed);
4655
}
4756
} catch {

scripts/append-run-log.test.mjs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,15 @@ import { promisify } from 'node:util';
1010
import { mkdtemp, writeFile, rm, readFile } from 'node:fs/promises';
1111
import { tmpdir } from 'node:os';
1212
import path from 'node:path';
13+
import { fileURLToPath } from 'node:url';
1314

1415
const exec = promisify(execFile);
15-
const SCRIPT = new URL('./append-run-log.mjs', import.meta.url).pathname;
16+
// `new URL(...).pathname` yields a leading-slash path (e.g. "/D:/...") that
17+
// Windows' node CLI mis-resolves relative to the current drive instead of
18+
// treating as absolute (same class of bug already fixed for
19+
// tools/loop/test/files.test.mjs) -- fileURLToPath() gives the correct
20+
// platform-native path on every OS.
21+
const SCRIPT = fileURLToPath(new URL('./append-run-log.mjs', import.meta.url));
1622

1723
test('invalid JSON second arg exits 1 with a Usage / valid JSON message', async () => {
1824
await assert.rejects(
@@ -40,3 +46,20 @@ test('valid minimal JSON entry does not throw on parse', async () => {
4046
await rm(dir, { recursive: true, force: true });
4147
}
4248
});
49+
50+
test('a non-ISO run_id (e.g. a custom slug) survives the next append instead of being pruned', async () => {
51+
const dir = await mkdtemp(path.join(tmpdir(), 'append-run-log-'));
52+
const logPath = path.join(dir, 'loop-run-log.md');
53+
await writeFile(logPath, '<!-- Loop appends below this line -->\n');
54+
try {
55+
// "run-1" parses under JS's lenient Date constructor into 2001-01-01,
56+
// which looks 30+ days old -- it must not be pruned on the next append.
57+
await exec('node', [SCRIPT, JSON.stringify({ run_id: 'run-1', outcome: 'ok' }), logPath]);
58+
await exec('node', [SCRIPT, JSON.stringify({ run_id: 'run-2', outcome: 'ok' }), logPath]);
59+
const written = await readFile(logPath, 'utf8');
60+
assert.ok(written.includes('"run_id":"run-1"'), 'run-1 entry should still be present');
61+
assert.ok(written.includes('"run_id":"run-2"'), 'run-2 entry should still be present');
62+
} finally {
63+
await rm(dir, { recursive: true, force: true });
64+
}
65+
});

0 commit comments

Comments
 (0)