Skip to content

Commit 944fe92

Browse files
committed
fix(check): time run directories in unix ms, give the preserve marker its own keepUntil, and create every log up front
- owner.startedAt is unix milliseconds. A well-formed owner whose start time is not a number counts as past the day-old backstop; a half-written one still counts as absent, so a starting run keeps its grace. Nothing an owned directory's age depends on reads a filesystem timestamp any more. - The preserve marker records keepUntil (start + 24h). A kept directory's lifetime depends only on that file: raising it keeps the directory past the backstop, and a marker that is deleted, expired, or unreadable holds nothing. - All four logs are created when a run opens, so a kept directory always holds runtime.log, as the spec says, even when no runtime rule ran.
1 parent d813ec7 commit 944fe92

4 files changed

Lines changed: 231 additions & 77 deletions

File tree

‎openspec/specs/cli-check/spec.md‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -622,9 +622,11 @@ instead of removing it: the snapshot that ran, the assembled configs, the `owner
622622
the logs. Human output SHALL name the kept directory on stderr. Under `--json`, the output
623623
SHALL carry an additive, optional `runDirectory` field, the directory's path relative to the
624624
project root, present only when the flag is set. A kept directory SHALL hold a `preserve` marker
625-
beside its `owner` record, and SHALL survive later runs while the marker is there, until it is
626-
swept for age (see "Abandoned run directories are swept"). Deleting the marker SHALL release the
627-
directory to the next run's sweep.
625+
beside its `owner` record, recording `keepUntil` in unix milliseconds, set to the run's start plus
626+
24 hours. The directory SHALL survive later runs while the current time is before `keepUntil`,
627+
whatever its owner or age, so raising `keepUntil` keeps it longer. A marker that is deleted,
628+
past its `keepUntil`, or unreadable SHALL hold nothing, and the directory SHALL fall to the
629+
ordinary sweep (see "Abandoned run directories are swept").
628630

629631
#### Scenario: A preserved run is named and complete
630632

@@ -642,6 +644,18 @@ directory to the next run's sweep.
642644
- **WHEN** the `preserve` marker is deleted from a kept run directory whose run has ended
643645
- **THEN** the next run SHALL remove the directory
644646

647+
#### Scenario: A raised keepUntil outlasts the day
648+
649+
- **WHEN** a kept run directory's run started more than 24 hours ago
650+
- **AND** its marker's `keepUntil` is still in the future
651+
- **THEN** the next run SHALL NOT remove it
652+
653+
#### Scenario: An unreadable marker holds nothing
654+
655+
- **WHEN** a kept run directory's `preserve` marker does not parse, or its `keepUntil` is not a number
656+
- **AND** its run has ended
657+
- **THEN** the next run SHALL remove the directory
658+
645659
#### Scenario: A preserved authenticated run holds no credential
646660

647661
- **WHEN** an authenticated `check --preserve-logs` reconciles
@@ -653,10 +667,14 @@ At the start of every run, the CLI SHALL remove each directory under `.taskless/
653667
`owner` names a process on this host that is no longer alive, and each directory with no
654668
`owner` record (left by an earlier version). It SHALL NOT remove a directory whose owning
655669
process is alive, one owned by another host, since this host cannot tell whether that process
656-
lives, or one holding a `preserve` marker. Liveness SHALL be the test, not age, with one backstop:
657-
a directory whose run started more than 24 hours ago SHALL be removed whatever its owner. That
658-
covers a dead run's process id recycled by an unrelated process, a host that never returns, and
659-
a kept directory nobody went back to.
670+
lives, or one whose `preserve` marker still holds it. Liveness SHALL be the test, not age, with one
671+
backstop: a directory whose run started more than 24 hours ago SHALL be removed whatever its
672+
owner, unless its `preserve` marker still holds it. That covers a dead run's process id recycled
673+
by an unrelated process, and a host that never returns. A run's start time SHALL be read from its
674+
`owner` record, as unix milliseconds, and never from a filesystem timestamp; a well-formed
675+
`owner` record whose start time is not a number SHALL count as past the backstop. An `owner`
676+
record that does not parse SHALL count as absent, since a run writes it just after creating its
677+
directory.
660678

661679
#### Scenario: A killed run's directory is swept
662680

@@ -672,7 +690,8 @@ a kept directory nobody went back to.
672690
#### Scenario: A day-old directory is swept whatever its owner
673691

674692
- **WHEN** a run directory's run started more than 24 hours ago
675-
- **THEN** the next run SHALL remove it, even if its process id names a live process, it is owned by another host, or it was kept by `--preserve-logs`
693+
- **AND** its `preserve` marker, if any, no longer holds it
694+
- **THEN** the next run SHALL remove it, even if its process id names a live process or it is owned by another host
676695

677696
### Requirement: Check reports rule integrity under --json
678697

‎packages/cli/src/agent/check.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,9 @@ delete the rules to make `check` pass, and do not suggest
118118
Its path is printed on stderr, or returned as `runDirectory` under
119119
`--json`. Use it to debug a rule that behaves unexpectedly; the logs hold
120120
matched source, so treat the directory like any local build output. It
121-
survives later runs and is removed after a day; delete its `preserve`
122-
file to let the next run remove it sooner.
121+
survives later runs until the `keepUntil` in its `preserve` file, a
122+
day out in unix milliseconds. Raise it to keep the directory longer, or
123+
delete the file to let the next run remove it.
123124

124125
## Steps
125126

‎packages/cli/src/rules/run-directory.ts‎

Lines changed: 70 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { hostname } from "node:os";
1313
import { join, relative } from "node:path";
1414
import process from "node:process";
1515

16+
import { isRecord } from "../util/is-record";
17+
1618
/**
1719
* The transient verification space one `check` (or `rule restore`) works in:
1820
* `.taskless/.run/<runId>/`.
@@ -43,9 +45,12 @@ import process from "node:process";
4345
*
4446
* **Age is only a backstop.** Any owned directory whose run started more than
4547
* {@link ABANDONED_AFTER_MS} ago is swept whatever its owner. That covers what
46-
* liveness cannot: a dead run's pid recycled by an unrelated process, a
47-
* foreign host that never came back, and a preserved directory nobody went
48-
* back to. No `check` runs for a day. A directory
48+
* liveness cannot: a dead run's pid recycled by an unrelated process, and a
49+
* foreign host that never came back. No `check` runs for a day. Every time is
50+
* unix milliseconds recorded by the run itself, never a filesystem timestamp,
51+
* which copies, checkouts, and backups rewrite. A preserved directory is kept
52+
* by its marker's own `keepUntil`, which `--preserve-logs` sets to the same day
53+
* and a user may raise. A directory
4954
* whose name is not a run id predates run ids (0.11's `runtime-rules/`, the
5055
* first 0.12 `snapshot/`) and is swept too. A run-id directory with no `owner`
5156
* is swept only after a grace period, because that is also what a run looks
@@ -84,12 +89,24 @@ const SIGNAL_FLUSH_MS = 2000;
8489
interface Owner {
8590
pid: number;
8691
hostname: string;
87-
startedAt: string;
92+
/** Unix milliseconds. A number, not a date string, so reading it cannot fail softly. */
93+
startedAt: number;
8894
}
8995

9096
/** The marker `--preserve-logs` writes beside `owner`. */
9197
const PRESERVE_MARKER = "preserve";
9298

99+
/**
100+
* What the marker holds. `keepUntil` is the directory's own deadline, in unix
101+
* milliseconds, so a kept directory's lifetime depends on nothing but this
102+
* file: not on `owner`, and never on a filesystem timestamp. Raising it keeps
103+
* the directory longer; deleting the file releases it.
104+
*/
105+
interface PreserveMarker {
106+
keepUntil: number;
107+
note: string;
108+
}
109+
93110
/** One append-only log file in a run directory. */
94111
export class RunLog {
95112
private pending: Promise<void> = Promise.resolve();
@@ -158,32 +175,47 @@ function isAlive(pid: number): boolean {
158175
}
159176
}
160177

161-
async function readOwner(directory: string): Promise<Owner | undefined> {
178+
async function readJson(path: string): Promise<Record<string, unknown>> {
162179
try {
163-
const parsed = JSON.parse(
164-
await readFile(join(directory, "owner"), "utf8")
165-
) as Partial<Owner>;
166-
return typeof parsed.pid === "number" && typeof parsed.hostname === "string"
167-
? (parsed as Owner)
168-
: undefined;
180+
const parsed: unknown = JSON.parse(await readFile(path, "utf8"));
181+
return isRecord(parsed) ? parsed : {};
169182
} catch {
183+
return {};
184+
}
185+
}
186+
187+
/**
188+
* The `owner` record, or `undefined` when there is none or it is not a record
189+
* yet. A run writes `owner` just after creating its directory, so a sweep can
190+
* read it half-written; that must look ownerless (and get the grace), never
191+
* old. A well-formed record whose `startedAt` is not a finite number reads as
192+
* starting at the epoch, past the day-old backstop: an age that cannot be
193+
* known is treated as old, never as new.
194+
*/
195+
async function readOwner(directory: string): Promise<Owner | undefined> {
196+
const parsed = await readJson(join(directory, "owner"));
197+
if (typeof parsed.pid !== "number" || typeof parsed.hostname !== "string") {
170198
return undefined;
171199
}
200+
return {
201+
pid: parsed.pid,
202+
hostname: parsed.hostname,
203+
startedAt: Number.isFinite(parsed.startedAt)
204+
? (parsed.startedAt as number)
205+
: 0,
206+
};
172207
}
173208

174209
/**
175-
* Whether the run that owns `directory` started more than `ms` ago, from the
176-
* owner's `startedAt`, or the directory's own mtime when that does not parse.
210+
* Whether the directory's `preserve` marker still holds it. A marker that is
211+
* missing, unreadable, or has no numeric `keepUntil` holds nothing, and the
212+
* directory falls back to the ordinary liveness rules.
177213
*/
178-
async function ownerOlderThan(
179-
owner: Owner,
180-
directory: string,
181-
ms: number
182-
): Promise<boolean> {
183-
const started = Date.parse(owner.startedAt);
184-
return Number.isNaN(started)
185-
? olderThan(directory, ms)
186-
: Date.now() - started > ms;
214+
async function isPreserved(directory: string): Promise<boolean> {
215+
const path = join(directory, PRESERVE_MARKER);
216+
if (!(await exists(path))) return false;
217+
const { keepUntil } = await readJson(path);
218+
return Number.isFinite(keepUntil) && Date.now() < (keepUntil as number);
187219
}
188220

189221
async function exists(path: string): Promise<boolean> {
@@ -221,15 +253,11 @@ export async function sweepAbandonedRuns(cwd: string): Promise<string[]> {
221253
for (const entry of entries) {
222254
if (!entry.isDirectory()) continue;
223255
const directory = join(root, entry.name);
256+
if (await isPreserved(directory)) continue;
224257
const owner = await readOwner(directory);
225258
if (owner !== undefined) {
226-
const expired = await ownerOlderThan(
227-
owner,
228-
directory,
229-
ABANDONED_AFTER_MS
230-
);
259+
const expired = Date.now() - owner.startedAt > ABANDONED_AFTER_MS;
231260
if (!expired) {
232-
if (await exists(join(directory, PRESERVE_MARKER))) continue;
233261
if (owner.hostname !== hostname()) continue;
234262
if (isAlive(owner.pid)) continue;
235263
}
@@ -275,13 +303,17 @@ export async function openRun(
275303
const owner: Owner = {
276304
pid: process.pid,
277305
hostname: hostname(),
278-
startedAt: now.toISOString(),
306+
startedAt: now.getTime(),
279307
};
280308
if (preserve) {
281309
// Before `owner`, so no sweep can see this run owned but unmarked.
310+
const marker: PreserveMarker = {
311+
keepUntil: now.getTime() + ABANDONED_AFTER_MS,
312+
note: "Kept by --preserve-logs until keepUntil (unix ms). Raise it to keep this directory longer; delete this file to let the next run remove it.",
313+
};
282314
await writeFile(
283315
join(path, PRESERVE_MARKER),
284-
"Kept by --preserve-logs. A run sweeps this directory once it is a day old; delete this file to let the next run sweep it sooner.\n",
316+
`${JSON.stringify(marker, undefined, 2)}\n`,
285317
"utf8"
286318
);
287319
}
@@ -293,6 +325,14 @@ export async function openRun(
293325
vale: new RunLog(join(path, "vale.log")),
294326
runtime: new RunLog(join(path, "runtime.log")),
295327
};
328+
// Every log exists from the start, so a kept directory always holds all
329+
// four, and an empty one says that engine had nothing to do rather than
330+
// leaving the reader to wonder whether it was ever logged.
331+
await Promise.all(
332+
[logs.engine, logs.sg, logs.vale, logs.runtime].map(async (log) =>
333+
writeFile(log.path, "", { flag: "a" })
334+
)
335+
);
296336
logs.engine.write(`run ${id} started (pid ${String(process.pid)})`);
297337
if (swept.length > 0) {
298338
logs.engine.write(`swept abandoned run directories: ${swept.join(", ")}`);

0 commit comments

Comments
 (0)