Skip to content

fix(tree): clamp jobTimeoutMs to the timer ceiling so a job is never abandoned after 1 ms - #2823

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-tree-jobtimeout-ceiling
Nov 11, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-tree-jobtimeout-ceiling

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

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

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.
  • 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: 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.

Both runs are identical:

$ TZ=UTC node --import ./test/resolve.mjs --test test/job-timeout-ceiling.test.js
# tests 4 / # pass 1 / # fail 3

The three failures read, verbatim:

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:

$ TZ=UTC node --import ./test/resolve.mjs --test test/job-timeout-ceiling.test.js   # plugins/tree
# tests 4  # pass 4  # fail 0
$ TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"                    # plugins/tree, full
# tests 149  # pass 149  # fail 0
$ TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"                    # plugins/social
# tests 619  # pass 619  # fail 0
$ TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"                    # plugins/timeline
# tests 161  # pass 161  # fail 0
$ TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"                    # plugins/workflow
# tests 528  # pass 528  # fail 0
$ TZ=UTC node --test ".github/scripts/*.test.mjs" "policy/*.test.mjs"
# tests 673  # pass 672  # fail 1   <- #749, the machine-only red below
$ TZ=UTC npm run test:interop     # 37/37      $ TZ=UTC npm run test:live-e2e   # 34/34
$ node .github/scripts/changelog.mjs check
changelog: 3 fragment(s) in changelog.d/ are well formed

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"
}

@HugoXK

HugoXK commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

@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

@Birfy

Birfy commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at b47fb2d
Written-by: Reviewer
Attestation: b47fb2d role=Reviewer

Ran at this tip:

$ DSH_HARNESS=/opt/flybot TZ=UTC node --import ./test/resolve.mjs --test test/job-timeout-ceiling.test.js
# tests 4  # pass 4  # fail 0
$ DSH_HARNESS=/opt/flybot TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 149  # pass 149  # fail 0
$ cp base/plugins/tree/src/organize.js plugins/tree/src/organize.js   # base blob over the tip source, new test kept
$ DSH_HARNESS=/opt/flybot TZ=UTC node --import ./test/resolve.mjs --test test/job-timeout-ceiling.test.js
# tests 4  # pass 1  # fail 3
$ diff -u base/plugins/tree/src/organize.js tip/plugins/tree/src/organize.js
$ diff -u base/docs/subsystems/tree.md tip/docs/subsystems/tree.md

(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.

@github-actions github-actions Bot added the pass label Nov 11, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 11, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 11, 2026
@github-actions github-actions Bot added the risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff label Nov 11, 2026
Merged via the queue into ThinkFlowLab:main with commit e7cffe1 Nov 11, 2026
18 of 19 checks passed
@Birfy
Birfy deleted the dev/5-tree-jobtimeout-ceiling branch November 11, 2026 01:51
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugins/tree: jobTimeoutMs above 2^31-1 ms makes every queued job abandon after 1 ms on a healthy host

2 participants