Repository navigation
fix(waits): monotonic judge deadlines, early messages, hidden-hold count, pin with the first write, settings follow-ups - #119
scuttletoby wants to merge 1 commit into
Conversation
…unt, pin with the first write, settings follow-ups - Waits (#95): judge deadlines (late, busy, register limit) use the monotonic clock, so a wall clock set back cannot stall a pass or keep a stuck check from turning late. A met wait's message goes out while slower checks in the same pass are still running, once. - Slots (#117): while a re-ask is not kept yet, the old hold still counts against its own account, and only once against the machine. - PR waits (#118): a pin made while a new wait is being written is saved with it, so a changed remote is never read. - Settings (#105): account rows list every selected environment's accounts; an administrator lock on waitChecks makes Edit and Remove inert; the check dialog keeps the draft until the write is saved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
lens: concurrency-and-abuse · reviewer: opus high (t3lens119a) · read at: b04ade9 covered: checks:
finding: minor (test gap, does not block) · notes (no finding):
findings: 1 (minor) |
|
lens: correctness · reviewer: opus high (t3lens119c) · read at: b04ade9 covered: #95 (monotonic deadlines in checks:
finding 1 — minor (outside the diff, same symptom as #95 item 1), finding 2 — minor (outside the diff, same pattern), note (not a finding): while a re-ask is not kept yet, Since this PR says "Fixes #95", I suggest either fixing findings 1-2 here (small) or keeping #95 open / opening a follow-up for them. Failing test for finding 1 (put at apps/server/src/scheduling/DueHint.reviewClock.test.ts)// Review (PR #119, correctness lens): after the wall clock is set back, the waits worker's
// DueHint keeps reporting "idle" until the wall clock reaches the recorded due time, so on a
// quiet server no waits sweep runs (no judging, no messages) for as long as the clock was set
// back. The 10-minute backstop never fires because the age is negative.
import { assert, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as DueHint from "./DueHint.ts";
const NOW = Date.parse("2026-11-14T12:00:00.000Z");
const HOUR = 3_600_000;
it.effect("a due hint recorded before the wall clock was set back an hour does not keep the poller idle", () =>
Effect.gen(function* () {
const due = DueHint.make();
const version = yield* due.version;
// The next sweep has work in 30 s.
yield* due.record(version, NOW + 30_000, NOW);
// The wall clock is set back an hour; a scheduler tick 5 s of real time later.
assert.isFalse(yield* due.isIdle(NOW - HOUR + 5_000));
}),
);Failing test for finding 2 (put at apps/server/src/waits/WaitService.reviewClock.test.ts)// Review (PR #119, correctness lens): the wall clock set back still holds a retried wake
// message for as long as the clock was set back, because the in-memory retry time
// (`retryAt`) is on the wall clock, like the deadlines #95 moved to the monotonic clock.
import * as NodeServices from "@effect/platform-node/NodeServices";
import { assert, it } from "@effect/vitest";
import { ProjectId, ThreadId } from "@t3tools/contracts";
import * as Clock from "effect/Clock";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import { TestClock } from "effect/testing";
import { ServerSettingsService } from "../serverSettings.ts";
import type { WaitDelivery, WaitDeliveryResult } from "./WaitDelivery.ts";
import * as WaitService from "./WaitService.ts";
import { memoryFileSystem } from "./WaitService.testkit.ts";
import { WAIT_STORE_FILE } from "./WaitStore.ts";
const NOW = Date.parse("2026-11-14T12:00:00.000Z");
const HOUR = 3_600_000;
const ON = { waitFor: { enabled: true } } as const;
const shiftedClock = Effect.gen(function* () {
const test = yield* TestClock.testClockWith(Effect.succeed);
const shift = { wall: 0 };
const wall = () => test.currentTimeMillisUnsafe() + shift.wall;
const clock = {
...test,
currentTimeMillisUnsafe: wall,
currentTimeMillis: Effect.sync(wall),
currentTimeNanosUnsafe: () => BigInt(wall()) * 1_000_000n,
currentTimeNanos: Effect.sync(() => BigInt(wall()) * 1_000_000n),
} as unknown as Clock.Clock;
return { clock, shift };
});
it.layer(NodeServices.layer)("review #119: wall clock set back (retry timers)", (it) => {
it.effect(
"a wake message that got 'try again later' is retried after its delay even when the wall clock was set back an hour",
() =>
Effect.gen(function* () {
const { clock, shift } = yield* shiftedClock;
yield* Effect.gen(function* () {
yield* TestClock.setTime(NOW);
const filePath = `/waits/state/${WAIT_STORE_FILE}`;
const fs = yield* FileSystem.FileSystem;
yield* fs.makeDirectory("/waits/state", { recursive: true });
yield* fs.writeFileString(
filePath,
JSON.stringify({
version: 1,
waits: [
{
id: "w_aaaaaaaaaa",
threadId: ThreadId.make("thread-a"),
projectId: ProjectId.make("project-a"),
key: "0123456789abcdef",
createdAt: "2026-11-14T11:00:00.000Z",
deadlineAt: "2026-11-14T15:00:00.000Z",
state: "delivering",
result: { outcome: "exists", detail: "Exists.", at: "2026-11-14T11:30:00.000Z" },
attempt: 0,
kind: "file",
spec: { path: "/nowhere/ready", until: "exists" },
},
],
}),
);
let busy = true;
let sent = 0;
const delivery: WaitDelivery = {
receiver: () => Effect.succeed({ type: "ok" }),
deliver: () =>
Effect.sync((): WaitDeliveryResult => {
if (busy) return { type: "retry", detail: "busy" };
sent += 1;
return { type: "delivered" };
}),
};
const service = yield* WaitService.make({ filePath, delivery, fileChildJudge: null });
assert.strictEqual((yield* service.deliverDue()).sent, 0);
busy = false;
// A time sync sets the wall clock back an hour; real time moves on two minutes,
// far past the retry delay (10 s).
shift.wall = -HOUR;
yield* TestClock.adjust(2 * 60_000);
yield* service.deliverDue();
assert.strictEqual(sent, 1, "the retried message is held until the wall clock catches up");
}).pipe(Effect.provideService(Clock.Clock, clock));
}).pipe(
Effect.provideService(FileSystem.FileSystem, memoryFileSystem().fs),
Effect.provide(ServerSettingsService.layerTest(ON)),
),
);
});findings: 0 blocker, 0 major, 2 minor (both outside the diff, for a follow-up), 1 note. |
Four small waits and slots follow-ups. Each fix has its own test that failed on the code without the fix.
Fixes #95
Fixes #105
Fixes #117
Fixes #118
What changes
#95: judge deadlines use the monotonic clock, and a met wait's message does not wait for slow checks. The judge's own deadlines (the late time of a running check, the busy wake time, the 3 s register limit, and the start and answer limits of a pass) now come from
Clock.monotonicTimeNanos, not the wall clock. They are kept in memory only, so a server restart starts them from nothing. Stored times (nextCheckAt, the wait record) stay on the wall clock as before. A wall clock set back no longer stalls a pass or keeps a hung check from turning late. During the judge step, each wait that ends wakes a small deliverer, which runs the normal deliver step at once. So a met wait's message goes out while slower checks in the same pass are still running. The deliver step after the judge step is kept.judgeDue(used by the tests) is unchanged.#105: settings screen.
settingKeys={["waitChecks"]}, so an administrator lock onwaitChecksmakes them inert, like "Add check". They are notserverScoped, so they do not repeat the section's scope chip.useUpdateScopedSettingsgains an optionalonSaved(called when every target saved the patch), next to the existingonFailed. While the write is out, the Save button shows "Saving…" and is disabled. On a refusal, the toast shows as before, and the dialog keeps the typed draft with a short note. Only the dialog that sent the write closes on success.#117: a hidden old hold still counts against its own account. While a re-ask is not kept yet, the old hold is hidden (from #114). It now still counts against its own account (new
accountOnlyinput to the shared slot decision), but not again against the machine. So a re-ask moved to another account (or to none) cannot let a third thread take that account's last slot, and the machine count is not doubled.#118: a pin made while a new wait is being written is saved with it. When the pr judge pins the repository and host after register has started writing the new wait, register now writes that pin itself, under the store lock, right after the wait is adopted (shared
pinInStore, the same compare-and-set aspinStored). Before, that pin could be lost, and the next read used the project's current remote.What happens with stored or old state
waitingwith the spec it was built with; a pin equal to what was built is skippedhost: pinned at its next read, as in #116onFailedruns and the toast lists which saved; the draft staysWith waits off (the default), none of this runs: no judge or deliver step starts, and nothing is read or written.
Tests
WaitService.test.ts: 4 in "monotonic clock and early messages" (waits: monotonic clock for check timing; send met file waits without waiting for the sweep #95). 3 were red; the restart test passes on both and guards the restart case.SlotGate.test.ts: 1 (Slot waits: an account can be booked twice while a re-ask on another account is unwritten #117). Old code granted a slot on an account that was already full.WaitPrJudge.test.ts: 1 (Waits: a pr wait's repository pin made during the wait's first write is lost until the next check #118). Old code stored{number: 7, until: "merged"}with no pin.WaitsAndSlotsSettings.browser.test.tsx: 3 (Wait settings screen: three small UI follow-ups from #104 review #105), one per finding, all 3 red on the old component.waitFor/agentSlots: 14 files, 308/308 pass. Web settings (unit + browser): 490/490 pass (17 browser tests timed out once under high machine load and all passed on a rerun with 2 workers).knip:checkpass.General vs project
All general: this is T3's own waits, slots and settings code, with no project facts. The
onSavedoption onuseUpdateScopedSettingsis a general hook next to upstream'sonFailed, and the account rows use upstream'suseEnvironmentProviders. Nothing project-only.🤖 Generated with Claude Code