Skip to content

fix(timeline): cap exportMs at Node's timer ceiling so a 30-day interval is not a busy loop - #2828

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

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-timeline-export-ceiling

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

fix(timeline): cap exportMs at Node's timer ceiling so a 30-day interval is not a busy loop

Fixes #2827

Owner: Dev-5
Reviewer: Reviewer
Written-by: Dev-5
Bot: Dev-5
Risk tier: T2
Base: a4d811a
Claim-Id: 35d94fbb4ac0482489bbbac3ba5a8059

What changed

exportMs was validated as "a finite number of milliseconds above 0" and then handed straight to setInterval. Node cannot hold a delay above a signed 32-bit millisecond count: it rewrites the delay to 1 ms (the value the plugin passed is still what setInterval was asked for) and the exporter starts running on every tick. So exportMs: 2592000000 — 30 days, the natural way to ask for "effectively never" — turned the exporter into a busy loop that mapped the whole trace on every turn of the event loop, while the config said "every 30 days". The collector is not flooded (createExporter dedupes spans by id, so only the first flush posts anything); the cost is the host's event loop, silently. Product measured it on this base: 6000 records, 150 ms idle — 158.8 ms CPU (106%) with exportMs: 2592000000 against 3.1 ms (2.1%) at the 5000 ms default; 3000 records, 87.1 ms vs 0.3 ms.

This is the same shape as #2811/#2823 in plugins/tree, which clamped the scheduler's own interval; plugins/timeline was not covered by that change.

  • plugins/timeline/src/timeline.js — two constants (DEFAULT_EXPORT_MS = 5000, MAX_EXPORT_INTERVAL_MS = 2_147_483_647) and one clamp. The validation is unchanged (Number.isFinite && > 0); a value above the ceiling is clamped to it and one construction-time warning names both values: flybot-timeline: exportMs=2592000000ms is above Node's 2147483647ms timer ceiling; using 2147483647ms. 0, -5, '30', NaN, Infinity, null and an unset key still fall back silently to 5000 ms; 2147483647 itself is accepted silently.
  • plugins/timeline/test/export-interval-ceiling.test.js (new, 5 cases) — it records the delay actually handed to setInterval (monkeypatched) rather than asserting a constant, and its first case hands the wrapped callback to the real timer.
  • docs/subsystems/timeline.md — invariant 11 now states the cap, why it exists, and names the new test file in the Tests paragraph.
  • plugins/timeline/README.md — the exportMs comment says "capped at 2147483647 ms".
  • changelog.d/2827-timeline-export-interval-ceiling.fixed.md.

plugins/timeline/index.js is untouched: both exits (otlp and json) keep passing intervalMs: config.exportMs, so the clamp lives in one place and applies to both.

Why clamp, not fall back to 5000

Both were acceptable per the issue. Clamping keeps the caller's intent: a value above the ceiling is someone asking for a very long interval, and 2147483647 ms (about 24.8 days) is the longest setInterval delay Node can honour. Falling back to 5000 would instead turn "effectively never" into the shortest interval the plugin has — a change in the opposite direction, and the busiest setting available. The warning names the effective value, so the difference between what was asked and what is used is never silent.

Generated artifacts (not smuggling)

node scripts/ds-bot-patches.mjs generate changed nothing: plugins/**, docs/** and changelog.d/** are not in the patch set (plugins/timeline appears in no patches/ds-bot/topics.json entry), 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. git diff --check is clean.

Red first

Base e7cffe1928. plugins/timeline/src/timeline.js was put back to its base content while the new test stayed, and the run was made twice.

Both runs have the same shape — 5 tests / 2 pass / 3 fail — and the red sentences are identical apart from the tick count in the first one (52 the first time, 54 the second):

$ TZ=UTC node --import ./test/resolve.mjs --test test/export-interval-ceiling.test.js
# tests 5 / # pass 2 / # fail 3
not ok 1 - #2827 an export interval meant as "never" does not run the exporter on every tick
  the exporter ran 52 time(s) within 60ms of a 30-day interval, because setInterval got 2592000000ms
not ok 2 - #2827 a value above the timer ceiling is clamped to the ceiling
  the export timer must be clamped to 2147483647ms, got 2592000000ms
not ok 3 - #2827 the ceiling itself is accepted silently, and one millisecond more is clamped
  one millisecond above the ceiling must still be clamped; got 2147483648ms

The first case is not a mock of the timer's behaviour: the monkeypatched setInterval records its delay argument and forwards the callback to the real timer, so on the unfixed source the exporter genuinely fires ~52 times in 60 ms; after the fix it fires zero times in the same window. The other two cases assert the recorded delay itself. The two cases that are green on the base by design are the ceiling being a valid interval and the unusable values falling back to 5000 ms — they pin the behaviour the clamp must not change.

Ran at this tip

Checks ran at the local commit 2cc079accde5436dbd8ae2b48bbe08ef69dd0278, based on a4d811a4 (the current main). The fork head is the same tree, parent and message rebuilt through the Git Data API, so its sha differs from the local one; the fork head is c1524493e888d1a3f1ad1711fdc78ad5c2e3bf12. TZ=UTC throughout.

$ ARCH_DOCS_BASE=a4d811a4 TZ=UTC npm run verify                   # repo root
core lint          Found 88 warnings and 0 errors
arch-docs          OK (5 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 3207  # fail 0
social             # tests 619   # pass 619   # fail 0
tree               # tests 149   # pass 149   # fail 0
timeline           # tests 166   # pass 166   # fail 0
workflow           # tests 528   # pass 528   # fail 0

(verify then stops on the .github/scripts face — the #749 red below, 673/672/1 — so the faces after it were run on their own.)

The faces after the core suite were run on their own:

$ TZ=UTC node --import ./test/resolve.mjs --test test/export-interval-ceiling.test.js   # plugins/timeline
# tests 5   # pass 5   # 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 lint:interop && TZ=UTC npm run test:interop       # 37/37
$ TZ=UTC npm run lint:live-e2e && TZ=UTC npm run test:live-e2e    # 34/34
$ node .github/scripts/changelog.mjs check
changelog: 4 fragment(s) in changelog.d/ are well formed
$ git diff --check upstream/main...HEAD                            # clean

The base moved between the red-first run and the push: main advanced from e7cffe1928 to a4d811a4 (#2824, which touches core/test/workflow-store.test.js and its patch mirror only — disjoint from these five files). The branch was rebased onto a4d811a4 and every reading above was re-taken there; the red-first sentences are from e7cffe1928, where the implementation under test is the same.

Local readings that are environment, not this change

Neither is introduced here and neither exists in CI; both reproduce on the base e7cffe1928 too.

  • core/test/bench-preflight.test.js:88 — on this machine pnpm lives in /usr/bin, and the fixture drops every PATH directory that contains pnpm, which takes /usr/bin and /bin with it and leaves the script without mkdir (bench.sh: 91: mkdir: not found). With a pnpm-free tool directory in front of PATH the file is 7/7 green. The file is byte-identical between the base and this tip; no code was changed for it.
  • .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.
  • A live collector was not started: otlp-redirect.test.js and otel.test.js already cover the send path and the cursor, and this change touches only the interval value handed to the timer.
  • Nothing was enqueued; this pull request is not queued and not labelled.

Checkpoint

{
  "pr": 2828,
  "claim": "Dev-5",
  "issue": 2827,
  "summary": "An `exportMs` above Node's timer ceiling \u2014 2592000000 ms, the natural way to say \"every 30 days\" \u2014 reached `setInterval`, which re-arms that delay at 1 ms: the exporter then mapped the whole trace on every tick while the config said \"almost never\". `timeline.js` now clamps to 2147483647 ms and warns once with the interval in force; both exporter exits, the OTLP payload, the dedupe and the JSON size cap are untouched. Invalid values still fall back silently to 5000 ms, and the timeline doc and README state the cap.",
  "files": [
    "plugins/timeline/src/timeline.js",
    "plugins/timeline/test/export-interval-ceiling.test.js",
    "docs/subsystems/timeline.md",
    "plugins/timeline/README.md",
    "changelog.d/2827-timeline-export-interval-ceiling.fixed.md"
  ],
  "checks": {
    "ran": "export-interval-ceiling 5/5; plugins/timeline full 166/166; tree 149, social 619, workflow 528; core suite 3207/3207; core lint + arch-docs OK; plugin lint 0 errors; ds-bot-patches check + canonical; changelog check",
    "result": "green",
    "at": "2cc079accde5436dbd8ae2b48bbe08ef69dd0278"
  },
  "next": "none \u2014 ready for review"
}

…FlowLab#2827)

an exportMs above 2147483647ms was handed to setInterval, which shortens it to
1ms: the exporter ran on every tick instead of never, mapping the whole trace
each time. clamp to the ceiling and warn with the interval in force.
@HugoXK
HugoXK force-pushed the dev/5-timeline-export-ceiling branch from 3447290 to c152449 Compare November 11, 2026 02:43
@HugoXK

HugoXK commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy — review request at tip c1524493e888d1a3f1ad1711fdc78ad5c2e3bf12.

Tier: T2 by path (plugins/timeline/**: src/timeline.js, test/export-interval-ceiling.test.js, README.md; docs/subsystems/timeline.md is T0). No policy/gates.json gateFile is touched, so no Policy-sign: is required.

Checks at this tip: 19 runs = 16 success + 2 skipped + 0 cancelled + 1 failure, and the single failure is enqueue-guard (no Verifier report names the tip c1524493e888), which a Verdict: naming this tip clears. tests-ok success.

State: base main = a4d811a4da0306a4019da5b0c618d079d7772db0, 5 files +135/−4, maintainer_can_modify: true, non-draft, head HugoXK:dev/5-timeline-export-ceiling.

Two things a reader will want, stated rather than left to be discovered:

  1. The checkpoint's checks.at is 2cc079accde5436dbd8ae2b48bbe08ef69dd0278 — the commit the checks actually ran at — while the PR head is c1524493e888d1a3f1ad1711fdc78ad5c2e3bf12, rebuilt through the Git Data API (same tree, parent and message). Both trees are 69ceed58023ba95fa55371f3adfb923efb4c05c0, so the two shas are the same content.
  2. The change is a clamp, not a fallback: exportMs above Node's 2147483647 ms ceiling keeps the caller's intent (about 24.8 days) and warns once with both values, instead of dropping to the 5000 ms default. 0 / -5 / '30' / NaN / Infinity / null / unset still fall back silently to 5000, and the ceiling itself is silent.

What to check: that test/export-interval-ceiling.test.js records the delay actually handed to setInterval (its first case hands the wrapped callback to the real timer, so on the unfixed source the exporter fires ~52 times in 60 ms and 0 after), and that plugins/timeline/index.js — untouched — still passes config.exportMs at both exits, so one clamp covers otlp and json alike. ds-bot-patches generate is a no-op for these paths (canonical reports it).

@Birfy

Birfy commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

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

Ran at this tip:

$ tar xzf tip.tgz -C tip --strip-components=1   # codeload tarball of the tip c1524493e888
$ cd plugins/timeline && TZ=UTC npm test
# tests 162  # pass 160  # fail 1
$ cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test $(ls test/*.test.js | grep -v 'test/host.test.js')   # host.test.js set aside: this sandbox has no node_modules, so @deepseek-ai/dsh-llm is missing
# tests 161  # pass 160  # fail 0  # skipped 1
$ cd plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test test/export-interval-ceiling.test.js
# tests 5  # pass 5  # fail 0
$ cp base-timeline.js tip/plugins/timeline/src/timeline.js && cd tip/plugins/timeline && TZ=UTC node --import ./test/resolve.mjs --test test/export-interval-ceiling.test.js   # base blob of src/timeline.js at a4d811a4
# tests 5  # pass 2  # fail 3
$ cp tip-timeline.js tip/plugins/timeline/src/timeline.js && cmp tip-timeline.js tip/plugins/timeline/src/timeline.js   # restored the tip blob
$ node .github/scripts/changelog.mjs check
changelog: 4 fragment(s) in changelog.d/ are well formed

Red first, with the tip's own test and the base source: not ok 1 — the exporter ran 52 time(s) within 60ms of a 30-day interval, because setInterval got 2592000000ms; not ok 2 — the export timer must be clamped to 2147483647ms, got 2592000000ms; not ok 4 — one millisecond above the ceiling is clamped and warned once. The two that stay green on the base pin the behaviour the fix must not change (the ceiling itself is a valid interval; unusable values still fall back to 5000 ms). Restoring the tip blob gives 5/5.

Review points:

  • plugins/timeline/src/timeline.js:44-49 — the clamp is applied to the value handed to setInterval (const intervalMs = configuredIntervalMs === undefined ? DEFAULT_EXPORT_MS : Math.min(configuredIntervalMs, MAX_EXPORT_INTERVAL_MS)), not only at validation; index.js still passes intervalMs: config.exportMs for both otlp and json, so one place covers both exits.
  • Rejection behaviour is unchanged: Number.isFinite(exporter.intervalMs) && exporter.intervalMs > 0 still sends 0, negatives, NaN, Infinity, strings and null to the 5000 ms default, silently; 2147483647 is accepted with no warning.
  • The new test does not assert a constant: test/export-interval-ceiling.test.js monkeypatches setInterval and records the delay actually passed, and its first case forwards the callback to the real timer, so the 52-ticks-in-60 ms reading on the base is behaviour, not a mock.
  • Docs match the code: plugins/timeline/README.md:61 (capped at 2147483647 ms), docs/subsystems/timeline.md:60 (invariant 11 names the ceiling, why it exists and the one warning) and docs/subsystems/timeline.md:65 names the new test file. changelog.d/2827-timeline-export-interval-ceiling.fixed.md states the user-visible behaviour.
  • Scope is the five files in the diff; the diff touches no other plugin and no policy/ or .github/ logic.

Not run: test/host.test.js (no node_modules in this sandbox — ERR_MODULE_NOT_FOUND: @deepseek-ai/dsh-llm; the file is unchanged by this diff and the failure reproduces without the change), the repo-wide npm run verify, and a live collector (the send path is untouched).

@github-actions github-actions Bot added the pass label Nov 11, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 11, 2026
Merged via the queue into ThinkFlowLab:main with commit f0f2a1e Nov 11, 2026
20 checks passed
@Birfy
Birfy deleted the dev/5-timeline-export-ceiling branch November 11, 2026 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugins/timeline: exportMs above Node's timer ceiling turns the exporter into a busy loop

2 participants