Skip to content

[pull] main from czlonkowski:main - #108

Merged
pull[bot] merged 1 commit into
5474312:mainfrom
czlonkowski:main
Aug 7, 2026
Merged

[pull] main from czlonkowski:main#108
pull[bot] merged 1 commit into
5474312:mainfrom
czlonkowski:main

Conversation

@pull

@pull pull Bot commented Aug 7, 2026

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

…fore, fix shutdown flush (#980)

* chore(telemetry): rotate publishable key, 60s flush, drop workflow_before

Rotate the bundled telemetry key to the new Supabase publishable key
format (sb_publishable_...). SUPABASE_ANON_KEY still overrides it.

Raise BATCH_FLUSH_INTERVAL from 5s to 60s. Queue thresholds and the
beforeExit/SIGINT/SIGTERM flush are unchanged, so short stdio sessions
still ship their batch on exit.

Stop sending workflow_before on mutation records. It was a full second
copy of the workflow on every update for a snapshot nothing queried.
The before snapshot is still built locally for deduplication, the
meaningful-change check, and the before hashes, which continue to
identify the prior state.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* chore(telemetry): tighten mutation-record comments and test assertion

Comment polish on the removed workflow_before path: state that the
before snapshot never leaves processMutation and that its hashes are
computed over the sanitized form, so they stay comparable with rows
recorded earlier. The new test now asserts workflowAfter survives
intact and the structural before-hash is present, rather than just
checking the field exists.

No behavior change.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* fix(telemetry): flush queued data on shutdown instead of relying on beforeExit

Every shutdown path ends in process.exit(), which does not emit
'beforeExit', so the batch processor's exit handler never ran and
anything queued since the last interval flush was dropped. At a 5s
interval that was a small window; at 60s it loses most short sessions,
and single-mutation sessions entirely, since mutations auto-flush only
from the second queued record onward.

Add telemetry.flushBeforeExit(): a bounded, non-throwing final flush
capped by SHUTDOWN_FLUSH_TIMEOUT_MS so an unreachable backend cannot
delay or fail an exit. Awaited from the MCP server's shutdown() (which
covers both stdio entry points) and from the single-session HTTP
server's shutdown(), which closes per-session servers directly rather
than going through that path.

Corrects the CHANGELOG claim that short stdio sessions already shipped
their batch on exit — they did not.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* chore(telemetry): address Copilot review on the shutdown flush

Document that ANON_KEY now holds a publishable key and why the field
keeps its name: it mirrors the SUPABASE_ANON_KEY environment variable
that overrides it, which is a documented public contract.

Note in flushBeforeExit that its deadline stops the await rather than
cancelling the flush, and that request-level bounding already comes
from telemetryFetch's FETCH_TIMEOUT_MS abort.

Replace the wall-clock assertion in the deadline test with fake timers,
so it proves the bound (not settled at 4999ms, settled at 5000ms)
instead of depending on CI scheduling.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* fix(telemetry): keep test runs offline and harden the shutdown flush

Awaiting the flush in shutdown() exposed that nothing disabled telemetry
in tests: with no config file present the first-run default is enabled,
so integration tests that construct a server both shipped rows to the
production project and blocked each teardown on a real round-trip.
Measured on tool-invocation.test.ts (20 shutdowns) against an
unreachable host: 1.12s before the flush, 31.39s after, 1.61s with
N8N_MCP_TELEMETRY_DISABLED set in vitest.config.ts. The config-manager
suite now saves, clears, and restores the three opt-out variables so it
still exercises the enabled-by-default path deterministically.

Lower SHUTDOWN_FLUSH_TIMEOUT_MS to 2000 so it cannot tie with a caller's
shutdown budget (the integration helper races close against 3000ms, and
two equal timeouts could abandon its transport cleanup).

Move the flush ahead of the initialization await in shutdown(): telemetry
needs no database, so an initialization that never settles should not
also cost the queued events.

Cover the two test gaps: the reject case now rejects from flush() itself
rather than the batch processor, which swallows its own errors and left
the catch block unexercised, and a new case pins that a shutdown never
initializes telemetry when it was never initialized.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* fix(telemetry): widen the shutdown flush budget and pin the call site

SHUTDOWN_FLUSH_TIMEOUT_MS at 2000 exactly equalled FETCH_TIMEOUT_MS,
and a batch sends events, workflows and mutations as three sequential
requests — so on a slow link only the first was guaranteed to land, and
mutations, which go last and are the rarest records, were the first
casualty. 2500 keeps the budget above the per-request cap while staying
below the 3000ms shutdown budget callers allow themselves.

Guard the flush call in shutdown() like every other cleanup step there:
telemetry must never change a shutdown's outcome, which for
src/mcp/index.ts would mean exit code 1 and skipped stdin teardown.

Add tests pinning the call site itself — that shutdown() awaits the
flush, that it does so ahead of the database initialization await, and
that a rejecting flush still shuts down cleanly. Nothing failed before
if the call was simply deleted.

Also stop two tests from being able to go vacuous later: the opt-out
restore in config-manager.test.ts no longer assigns an undefined value
(which Node coerces to the string "undefined"), and the
never-initialized case asserts the mocked config manager still reports
enabled, since that is what gives it teeth.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* test: complete the telemetry mock and assert shutdown cleanup ran

The shutdown test file mocked the telemetry barrel with 7 of the 10
methods reached through it — handlers-n8n-manager imports the same
barrel, so trackEvent, trackWorkflowCreation and trackWorkflowMutation
were missing. The three existing tests never reach them, but the next
test added to this file that exercises a workflow tool would have died
with "is not a function" instead of failing meaningfully.

The rejecting-flush case now also asserts the cleanup past the flush
ran, so "shuts down cleanly" is checked rather than inferred from the
promise resolving.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* chore(telemetry): clear the shutdown deadline timer on every path

Move clearTimeout into a finally block so a rejecting flush cannot leave
the deadline pending. Harmless while the timer is unref'd, but it would
surface under fake timers.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* chore(telemetry): stop the probe scripts duplicating the backend constants

Both scripts carried their own copy of the URL and key, which is why this
rotation had to touch three files and why one of them still held a key
from an earlier rotation. They now import TELEMETRY_BACKEND and resolve
the same env overrides the runtime does, so they always probe the
credentials the package actually ships with. The key now exists in
exactly one place in source.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

* docs(telemetry): state the shutdown flush bound accurately

The comment claimed nothing outlives the deadline by more than one
request. A batch sends events, workflows and mutations as separate
sequential requests, so a deadline that fires during the first still
leaves the rest to be issued: the bound is per request, not total. What
makes that acceptable is that every caller exits immediately after this
returns, which the comment now says instead.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en
@pull pull Bot locked and limited conversation to collaborators Aug 7, 2026
@pull pull Bot added the ⤵️ pull label Aug 7, 2026
@pull
pull Bot merged commit 9be5080 into 5474312:main Aug 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant