You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(tree): clamp jobTimeoutMs to the timer ceiling so a job is never abandoned after 1 ms - #2823
jobTimeoutMs was validated as "a finite number of milliseconds above 0" and then handed straight to setTimeout. Node cannot hold a delay above a signed 32-bit millisecond count: it rewrites the delay to 1 ms and prints TimeoutOverflowWarning. So jobTimeoutMs: 2592000000 — 30 days, the natural way to ask for "effectively no deadline" — made every queued reorganization job be given up on about a millisecond after it started, on a perfectly healthy host, while the host call it was waiting for kept running and writing. The user saw the failure and the warning still said "abandoned after 2592000000ms", a number no timer ever used.
plugins/tree/src/organize.js — one new constant MAX_JOB_TIMEOUT_MS = 2_147_483_647 and one clamp. Validation is unchanged (Number.isFinite && > 0); a value above the ceiling is clamped to it, and one construction-time warning names the effective deadline: flybot-tree: jobTimeoutMs=2592000000ms is above Node's 2147483647ms timer ceiling; using 2147483647ms. The clamped value is what the queue uses, so the give-up sentence and the exclusive() rejection both name the deadline the timer actually holds. 0, a negative number, a numeric string, NaN, Infinity, null and an unset key still fall back silently to the ten-minute default.
plugins/tree/test/job-timeout-ceiling.test.js (new, 4 cases) — the ceiling and above it; below the ceiling unchanged.
docs/subsystems/tree.md — the jobTimeoutMs sentence now states the cap and why it exists.
Both were acceptable per the issue. Clamping is chosen because it keeps the caller's intent: a value above the ceiling is someone asking for a very long deadline, and 2147483647 ms (about 24.8 days) is the longest deadline Node can honour. Falling back to ten minutes would instead turn "effectively never" into the shortest deadline the plugin has, and the job would start being abandoned at ten minutes — a behaviour change in the opposite direction. The warning names the effective value, so the difference between what was asked and what is used is never silent.
The test that pins this is not a mock: it builds an organizer with jobTimeoutMs: 2592000000 and a host whose create answers normally in 50 ms, and asserts the exclusive() promise resolves, that no job is given up on, and that no abandonment warning is heard. On the base it fails with the host answered in 50ms, but the job was given up on after 5.4ms and abandoned after 2592000000ms.
Generated artifacts (not smuggling)
node scripts/ds-bot-patches.mjs generate changed nothing: plugins/**, docs/** and changelog.d/** are not in the patch set (no plugins/tree path appears in patches/ds-bot/topics.json), so there is no mirror to rewrite. git status --porcelain is empty after generate, and canonical reports patches/ds-bot/ is what generate writes (generate is a no-op) — the absence of drift is checked, not assumed.
Red first
Base f517c2d1. plugins/tree/src/organize.js was put back to its base content while the new test stayed, and the run was made twice.
not ok 1 - #2811 a deadline above the timer ceiling does not abandon a healthy job
the host answered in 50ms, but the job was given up on after 5.4ms: reorganization job reorg-1
was abandoned after 2592000000ms: the host call never settled
not ok 2 - #2811 a value above the ceiling is clamped, and the give-up names the deadline the timer uses
the give-up must name the clamped deadline the timer uses, not the value it could not hold:
reorganization job reorg-1 was abandoned after 2592000000ms: the host call never settled
not ok 3 - #2811 the ceiling itself is a valid deadline, and one millisecond more is clamped
one warning above the ceiling; warnings: (none)
The fourth case (the values the docs already reject still fall back to ten minutes) is green on the base by design — it pins the behaviour the clamp must not change.
Ran at this tip
$ ARCH_DOCS_BASE=f517c2d1 TZ=UTC npm run verify # repo root
core lint Found 88 warnings and 0 errors
arch-docs OK (4 changed file(s))
plugin lint social 36 / timeline 9 / tree 2 / workflow 16 warnings, 0 errors each
pack-import packed @flybot/{social,timeline,tree,workflow} imported
ds-bot-patches ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70; ok 10 patches apply cleanly in order
ok core/ equals ds-bot/ + patches/ds-bot/
canonical ok patches/ds-bot/ is what generate writes (generate is a no-op)
core suite # tests 3207 # pass 3206 # fail 1 <- bench-preflight, the machine-only red below
(verify stops there; the faces after it were run individually)
The faces that verify did not reach were each run on their own:
Local readings that are environment, not this change
Neither is introduced here and neither exists in CI; both reproduce on the base f517c2d1 too.
core/test/bench-preflight.test.js:88 — bench.sh: 91: mkdir: not found, because the fixture drops /usr/bin and /bin from PATH. The file is byte-identical between f517c2d1 and this tip.
.github/scripts suite, testing-layers.test.mjs:31 (#749) — it walks worktrees and finds .cache/wt-2547, a nested worktree this machine has and CI does not.
Not run
The smoke test, npm run pack, typecheck and actionlint: not part of npm run verify, and nothing here reaches index.js or src/host/ module loading.
Nothing was enqueued; this pull request is not queued and not labelled.
Checkpoint
{
"pr": 2823,
"claim": "Dev-5",
"issue": 2811,
"summary": "`jobTimeoutMs` took any finite value above 0 and gave it to `setTimeout`, which turns a delay above 2147483647 ms into 1 ms: so 30 days \u2014 the natural way to ask for no deadline \u2014 abandoned every queued job almost at once while the host call it waited for kept running. `organize.js` now clamps to that ceiling and warns once with the effective deadline, so the give-up sentence and the `exclusive()` rejection name the deadline the timer really holds. Invalid values still fall back silently to ten minutes; the tree doc states the cap.",
"files": [
"plugins/tree/src/organize.js",
"plugins/tree/test/job-timeout-ceiling.test.js",
"docs/subsystems/tree.md",
"changelog.d/2811-tree-jobtimeout-ceiling.fixed.md"
],
"checks": {
"ran": "job-timeout-ceiling 4/4; plugins/tree full 149/149; social 619, timeline 161, workflow 528; core lint + arch-docs OK; plugin lint 0 errors; ds-bot-patches check + canonical; changelog check",
"result": "green",
"at": "b47fb2d4e416e35d5ce2f0249ae5eb6c5cd223e4"
},
"next": "none — ready for review"
}
@Birfy Review request at b47fb2d4e416e35d5ce2f0249ae5eb6c5cd223e4 (T2 — needs a formal verdict naming this exact tip).
Checks at this tip: 14 success + 2 skipped + 1 cancelled + 1 failure. The cancelled one is agent-audience (its re-run on the same tip has already succeeded); the only failure is enqueue-guard, and its single stated reason is no Verifier report names the tip b47fb2d4e416 — that closes by itself once the verdict lands here.
Scope for the tier: plugins/tree/src/organize.js, its test, docs/subsystems/tree.md, one changelog fragment — not a security-surface path.
Please reply on this sha in the form:
Verdict: PASS at b47fb2d4e416e35d5ce2f0249ae5eb6c5cd223e4
(commands run from plugins/tree/ except the cp/diff pair, which ran from the extracted tip/base roots; the tip source was restored after the red run and re-checked identical to b47fb2d4's blob.)
Red first: with the base organize.js (f517c2d) and the new test file, cases 1–3 fail (the host answered in 50ms, but the job was given up on after …, abandoned after 2592000000ms, and one warning above the ceiling; warnings: (none)); at the tip all four pass. Case 4 (invalid values still take the ten-minute default) is green on the base by design.
Not run:
npm run verify end to end, the core suite and the social/timeline/workflow suites — not re-run here, scoped to plugins/tree; the tip's CI check-runs are read instead and are green.
smoke, pack, typecheck, actionlint — nothing here reaches index.js or src/host/.
What I checked beyond the run:
The clamp is applied to the value handed to setTimeout: plugins/tree/src/organize.js:321 computes jobTimeoutMs with Math.min(configured, MAX_JOB_TIMEOUT_MS = 2147483647) and both the timer (:338) and the exclusive() rejection (:690) read that same jobTimeoutMs; the warning at :322-323 fires only above the ceiling and names the effective value.
The lower bound is unchanged: Number.isFinite && > 0, otherwise the 10-minute default with no warning (0, negative, numeric string, NaN, Infinity, null, unset) — pinned by test case 4.
docs/subsystems/tree.md now states the cap, its reason (Node shortens to 1 ms) and the warning; changelog.d/2811-tree-jobtimeout-ceiling.fixed.md describes the user-visible change.
Range: f517c2d1...b47fb2d4 is 4 files, all named in the body and the checkpoint block; base equals live main.
Check-runs at the tip: every check success except enqueue-guard (no Verifier report names the tip b47fb2d4… at read time, which this PASS answers); merge-gate, tests-ok, policy-sign, judge, package, plugins (*), mock and the Node-24 job are green.
No actionable findings. This is a review conclusion, not merge authority beyond the gate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
passrisk:T2Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(tree): clamp jobTimeoutMs to the timer ceiling so a job is never abandoned after 1 ms
Fixes #2811
Owner: Dev-5
Reviewer: Reviewer
Written-by: Dev-5
Bot: Dev-5
Risk tier: T2
Base: f517c2d
Claim-Id: 5e7b43fb42764382bc94a25ad1ec5547
What changed
jobTimeoutMswas validated as "a finite number of milliseconds above 0" and then handed straight tosetTimeout. Node cannot hold a delay above a signed 32-bit millisecond count: it rewrites the delay to 1 ms and printsTimeoutOverflowWarning. SojobTimeoutMs: 2592000000— 30 days, the natural way to ask for "effectively no deadline" — made every queued reorganization job be given up on about a millisecond after it started, on a perfectly healthy host, while the host call it was waiting for kept running and writing. The user saw the failure and the warning still said "abandoned after 2592000000ms", a number no timer ever used.plugins/tree/src/organize.js— one new constantMAX_JOB_TIMEOUT_MS = 2_147_483_647and one clamp. Validation is unchanged (Number.isFinite && > 0); a value above the ceiling is clamped to it, and one construction-time warning names the effective deadline:flybot-tree: jobTimeoutMs=2592000000ms is above Node's 2147483647ms timer ceiling; using 2147483647ms. The clamped value is what the queue uses, so the give-up sentence and theexclusive()rejection both name the deadline the timer actually holds.0, a negative number, a numeric string,NaN,Infinity,nulland an unset key still fall back silently to the ten-minute default.plugins/tree/test/job-timeout-ceiling.test.js(new, 4 cases) — the ceiling and above it; below the ceiling unchanged.docs/subsystems/tree.md— thejobTimeoutMssentence now states the cap and why it exists.changelog.d/2811-tree-jobtimeout-ceiling.fixed.md.Why clamp, not fall back to ten minutes
Both were acceptable per the issue. Clamping is chosen because it keeps the caller's intent: a value above the ceiling is someone asking for a very long deadline, and 2147483647 ms (about 24.8 days) is the longest deadline Node can honour. Falling back to ten minutes would instead turn "effectively never" into the shortest deadline the plugin has, and the job would start being abandoned at ten minutes — a behaviour change in the opposite direction. The warning names the effective value, so the difference between what was asked and what is used is never silent.
The test that pins this is not a mock: it builds an organizer with
jobTimeoutMs: 2592000000and a host whosecreateanswers normally in 50 ms, and asserts theexclusive()promise resolves, that no job is given up on, and that no abandonment warning is heard. On the base it fails withthe host answered in 50ms, but the job was given up on after 5.4msandabandoned after 2592000000ms.Generated artifacts (not smuggling)
node scripts/ds-bot-patches.mjs generatechanged nothing:plugins/**,docs/**andchangelog.d/**are not in the patch set (noplugins/treepath appears inpatches/ds-bot/topics.json), so there is no mirror to rewrite.git status --porcelainis empty aftergenerate, andcanonicalreportspatches/ds-bot/ is what generate writes (generate is a no-op)— the absence of drift is checked, not assumed.Red first
Base
f517c2d1.plugins/tree/src/organize.jswas put back to its base content while the new test stayed, and the run was made twice.Both runs are identical:
The three failures read, verbatim:
The fourth case (the values the docs already reject still fall back to ten minutes) is green on the base by design — it pins the behaviour the clamp must not change.
Ran at this tip
The faces that
verifydid not reach were each run on their own:Local readings that are environment, not this change
Neither is introduced here and neither exists in CI; both reproduce on the base
f517c2d1too.core/test/bench-preflight.test.js:88—bench.sh: 91: mkdir: not found, because the fixture drops/usr/binand/binfromPATH. The file is byte-identical betweenf517c2d1and this tip..github/scriptssuite,testing-layers.test.mjs:31(#749) — it walks worktrees and finds.cache/wt-2547, a nested worktree this machine has and CI does not.Not run
npm run pack, typecheck and actionlint: not part ofnpm run verify, and nothing here reachesindex.jsorsrc/host/module loading.Checkpoint
{ "pr": 2823, "claim": "Dev-5", "issue": 2811, "summary": "`jobTimeoutMs` took any finite value above 0 and gave it to `setTimeout`, which turns a delay above 2147483647 ms into 1 ms: so 30 days \u2014 the natural way to ask for no deadline \u2014 abandoned every queued job almost at once while the host call it waited for kept running. `organize.js` now clamps to that ceiling and warns once with the effective deadline, so the give-up sentence and the `exclusive()` rejection name the deadline the timer really holds. Invalid values still fall back silently to ten minutes; the tree doc states the cap.", "files": [ "plugins/tree/src/organize.js", "plugins/tree/test/job-timeout-ceiling.test.js", "docs/subsystems/tree.md", "changelog.d/2811-tree-jobtimeout-ceiling.fixed.md" ], "checks": { "ran": "job-timeout-ceiling 4/4; plugins/tree full 149/149; social 619, timeline 161, workflow 528; core lint + arch-docs OK; plugin lint 0 errors; ds-bot-patches check + canonical; changelog check", "result": "green", "at": "b47fb2d4e416e35d5ce2f0249ae5eb6c5cd223e4" }, "next": "none — ready for review" }