[pull] main from czlonkowski:main

CHANGELOG.md

@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [2.68.2] - 2026-08-06
+
+### Changed
+
+- **Telemetry publishable key rotated.** The backend key moves from the legacy Supabase anon JWT to the new publishable key format (`sb_publishable_…`). `SUPABASE_ANON_KEY` still overrides the bundled default, so anyone pointing telemetry at their own project is unaffected.
+- **Telemetry batch flush interval raised from 5s to 60s.** Fewer, larger inserts for the same data.
+
+### Fixed
+
+- **Test runs no longer reach the telemetry backend.** Nothing disabled telemetry in the test environment, so with no config file present the first-run default left it enabled: test-generated rows could be delivered to the production project, and every server teardown blocked on a real round-trip (measured at 20 shutdowns costing 30s against an unreachable host). `vitest.config.ts` now sets `N8N_MCP_TELEMETRY_DISABLED`, and the config-manager suite manages the opt-out variables itself rather than inheriting them.
+- **Queued telemetry is no longer lost on shutdown.** Every shutdown path ends in `process.exit()`, which does not emit `beforeExit`, so the batch processor's own exit handler never ran: whatever was queued since the last interval flush was dropped. Raising the flush interval to 60s widened that window enough to lose most short sessions, including single-mutation ones (mutations auto-flush only from the second queued record onward). The MCP server's `shutdown()` and the single-session HTTP server's now await `telemetry.flushBeforeExit()`, a bounded, non-throwing final flush that cannot delay or fail an exit if the backend is unreachable.
+- **Mutation telemetry no longer sends the pre-mutation workflow.** `workflow_before` was a full second copy of a workflow on every partial or full update, roughly doubling each mutation row for a snapshot nothing queried. The record now carries `workflow_after` only; the before snapshot is still built locally to drive deduplication, the meaningful-change check, and `workflow_hash_before` / `workflow_structure_hash_before`, which continue to identify the prior state.
+
 ## [2.68.1] - 2026-08-04
 
 ### Changed

package.json

@@ -1,6 +1,6 @@
 {
   "name": "n8n-mcp",
-  "version": "2.68.1",
+  "version": "2.68.2",
   "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",

scripts/test-telemetry-direct.ts

@@ -4,16 +4,17 @@
  */
 
 import { createClient } from '@supabase/supabase-js';
+import { TELEMETRY_BACKEND } from '../src/telemetry/telemetry-types';
 
-const TELEMETRY_BACKEND = {
-  URL: 'https://ydyufsohxdfpopqbubwk.supabase.co',
-  ANON_KEY: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6InlkeXVmc29oeGRmcG9wcWJ1YndrIiwicm9sZSI6ImFub24iLCJpYXQiOjE3Mzc2MzAxMDgsImV4cCI6MjA1MzIwNjEwOH0.LsUTx9OsNtnqg-jxXaJPc84aBHVDehHiMaFoF2Ir8s0'
-};
+// Resolved the same way the runtime does (telemetry-manager.ts), so this script
+// always probes the credentials the package actually ships with.
+const url = process.env.SUPABASE_URL || TELEMETRY_BACKEND.URL;
+const key = process.env.SUPABASE_ANON_KEY || TELEMETRY_BACKEND.ANON_KEY;
 
 async function testDirect() {
   console.log('🧪 Direct Telemetry Test\n');
 
-  const supabase = createClient(TELEMETRY_BACKEND.URL, TELEMETRY_BACKEND.ANON_KEY, {
+  const supabase = createClient(url, key, {
     auth: {
       persistSession: false,
       autoRefreshToken: false,

scripts/test-workflow-insert.ts

@@ -4,14 +4,15 @@
  */
 
 import { createClient } from '@supabase/supabase-js';
+import { TELEMETRY_BACKEND } from '../src/telemetry/telemetry-types';
 
-const TELEMETRY_BACKEND = {
-  URL: 'https://ydyufsohxdfpopqbubwk.supabase.co',
-  ANON_KEY: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6InlkeXVmc29oeGRmcG9wcWJ1YndrIiwicm9sZSI6ImFub24iLCJpYXQiOjE3NTg3OTYyMDAsImV4cCI6MjA3NDM3MjIwMH0.xESphg6h5ozaDsm4Vla3QnDJGc6Nc_cpfoqTHRynkCk'
-};
+// Resolved the same way the runtime does (telemetry-manager.ts), so this script
+// always probes the credentials the package actually ships with.
+const url = process.env.SUPABASE_URL || TELEMETRY_BACKEND.URL;
+const key = process.env.SUPABASE_ANON_KEY || TELEMETRY_BACKEND.ANON_KEY;
 
 async function testWorkflowInsert() {
-  const supabase = createClient(TELEMETRY_BACKEND.URL, TELEMETRY_BACKEND.ANON_KEY, {
+  const supabase = createClient(url, key, {
     auth: {
       persistSession: false,
       autoRefreshToken: false,

src/http-server-single-session.ts

@@ -1547,6 +1547,18 @@ export class SingleSessionHTTPServer {
       });
     }
 
+    // Ship queued telemetry before the process exits. This server closes each
+    // session's MCP server directly rather than calling its shutdown(), so the
+    // flush there does not cover this path. Lazy-required so telemetry stays off
+    // the module load path. Bounded and non-throwing.
+    try {
+      // eslint-disable-next-line @typescript-eslint/no-var-requires
+      const { telemetry } = require('./telemetry');
+      await telemetry.flushBeforeExit();
+    } catch (error) {
+      logger.debug('Telemetry flush during shutdown failed:', error);
+    }
+
     // Close the shared database connection (only during process shutdown)
     // This must happen after all sessions are closed
     try {

src/mcp/server.ts

@@ -4749,6 +4749,22 @@ Full documentation is being prepared. For now, use get_node_essentials for confi
 
     logger.info('Shutting down MCP server...');
 
+    // Ship queued telemetry first. Callers exit via process.exit() right after
+    // this method returns, which never emits 'beforeExit', so the batch
+    // processor's own exit handler does not get to run; without this a short
+    // session loses everything it queued since the last interval flush.
+    // Bounded and non-throwing, and deliberately ahead of the initialization
+    // await below: telemetry needs no database, so an initialization that never
+    // settles must not also cost us the queued events. Guarded like every other
+    // cleanup step here — telemetry must never change a shutdown's outcome,
+    // which for src/mcp/index.ts would mean exit code 1 and skipped stdin
+    // teardown.
+    try {
+      await telemetry.flushBeforeExit();
+    } catch (error) {
+      logger.debug('Telemetry flush during shutdown failed:', error);
+    }
+
     // Wait for initialization to complete (or fail) before cleanup
     // This prevents race conditions where shutdown runs while init is in progress
     try {

src/telemetry/batch-processor.ts

@@ -18,13 +18,11 @@ function keyToSnakeCase(key: string): string {
 /**
  * Convert WorkflowMutationRecord to Supabase-compatible format.
  *
- * IMPORTANT: Only converts top-level field names to snake_case.
- * Nested workflow data (workflowBefore, workflowAfter, operations, etc.)
- * is preserved EXACTLY as-is to maintain n8n API compatibility.
- *
- * The Supabase workflow_mutations table stores workflow_before and
- * workflow_after as JSONB columns, which preserve the original structure.
- * Only the top-level columns (user_id, session_id, etc.) require snake_case.
+ * IMPORTANT: Only converts top-level field names to snake_case, because only the
+ * top-level columns (user_id, session_id, etc.) are named that way. Nested workflow
+ * data (workflowAfter, operations, etc.) is preserved EXACTLY as-is to maintain n8n
+ * API compatibility — the workflow_mutations table stores it in JSONB columns, which
+ * keep the original structure.
  *
  * Issue #517: Previously this used recursive conversion which mangled:
  * - Connection keys (node names like "Webhook" → "_webhook")

src/telemetry/mutation-tracker.ts

@@ -40,7 +40,10 @@ export class MutationTracker {
         return null;
       }
 
-      // Sanitize workflows to remove credentials and sensitive data
+      // Sanitize workflows to remove credentials and sensitive data. The before
+      // snapshot never leaves this method — it drives deduplication and the
+      // before-hashes, which are computed over the sanitized form so they stay
+      // comparable with previously recorded mutations.
       const workflowBefore = WorkflowSanitizer.sanitizeWorkflowRaw(data.workflowBefore);
       const workflowAfter = WorkflowSanitizer.sanitizeWorkflowRaw(data.workflowAfter);
 
@@ -103,7 +106,6 @@ export class MutationTracker {
       const record: WorkflowMutationRecord = {
         userId,
         sessionId: data.sessionId,
-        workflowBefore,
         workflowAfter,
         workflowHashBefore: hashBefore,
         workflowHashAfter: hashAfter,

src/telemetry/mutation-types.ts

@@ -87,7 +87,11 @@ export interface WorkflowMutationRecord {
   id?: string;
   userId: string;
   sessionId: string;
-  workflowBefore: any;
+  /**
+   * Post-mutation workflow only. The pre-mutation snapshot is used locally for
+   * hashing, deduplication, and change detection, then discarded — the before
+   * hashes below are what identify the prior state.
+   */
   workflowAfter: any;
   workflowHashBefore: string;
   workflowHashAfter: string;

src/telemetry/telemetry-manager.ts

@@ -8,7 +8,7 @@ import { TelemetryConfigManager } from './config-manager';
 import { TelemetryEventTracker } from './event-tracker';
 import { TelemetryBatchProcessor } from './batch-processor';
 import { TelemetryPerformanceMonitor } from './performance-monitor';
-import { TELEMETRY_BACKEND } from './telemetry-types';
+import { TELEMETRY_BACKEND, TELEMETRY_CONFIG } from './telemetry-types';
 import { TelemetryError, TelemetryErrorType, TelemetryErrorAggregator } from './telemetry-error';
 import { telemetryFetch } from './telemetry-fetch';
 import { logger } from '../utils/logger';
@@ -301,6 +301,44 @@ export class TelemetryManager {
     }
   }
 
+  /**
+   * Final flush for shutdown paths.
+   *
+   * Every shutdown path ends in process.exit(), which does not emit
+   * 'beforeExit', so the batch processor's own exit handler cannot be relied on
+   * to ship what is still queued — and with a 60s flush interval most of a
+   * short session's events are still queued. Shutdown callers await this
+   * instead. Bounded and non-throwing: an unreachable backend must not delay or
+   * fail an exit.
+   *
+   * The deadline stops this method awaiting; it does not cancel the flush. A
+   * batch sends events, workflows and mutations as separate sequential
+   * requests, so work already under way can continue past the deadline —
+   * bounded per request by telemetryFetch's FETCH_TIMEOUT_MS abort, not in
+   * total. That is acceptable only because every caller exits immediately
+   * after this returns, ending the process before the remainder matters.
+   */
+  async flushBeforeExit(timeoutMs: number = TELEMETRY_CONFIG.SHUTDOWN_FLUSH_TIMEOUT_MS): Promise<void> {
+    if (!this.isInitialized || !this.configManager.isEnabled()) return;
+
+    let timer: NodeJS.Timeout | undefined;
+    try {
+      const deadline = new Promise<void>(resolve => {
+        timer = setTimeout(resolve, timeoutMs);
+        // Never let the deadline itself hold the event loop open
+        timer.unref?.();
+      });
+
+      await Promise.race([this.flush(), deadline]);
+    } catch (error) {
+      logger.debug('Telemetry flush before exit failed:', error);
+    } finally {
+      // Clear on every path, so a rejecting flush cannot leave the deadline
+      // pending — harmless while unref'd, but it would surface under fake timers.
+      if (timer) clearTimeout(timer);
+    }
+  }
+
   /**
    * Flush queued mutations only
    */

src/telemetry/telemetry-types.ts

@@ -82,13 +82,21 @@ export interface SanitizedWorkflow {
 
 export const TELEMETRY_CONFIG = {
   // Batch processing
-  BATCH_FLUSH_INTERVAL: 5000, // 5 seconds
+  BATCH_FLUSH_INTERVAL: 60000, // 60 seconds
   EVENT_QUEUE_THRESHOLD: 10, // Batch events for efficiency
   WORKFLOW_QUEUE_THRESHOLD: 5, // Batch workflows
 
   // Network timeouts
   OPERATION_TIMEOUT: 5000, // 5 seconds
   FETCH_TIMEOUT_MS: 2000, // Hard deadline for each telemetry request
+  // Cap on the final flush so it cannot delay exit. Bounded on both sides:
+  // above FETCH_TIMEOUT_MS, because a batch sends events, workflows and
+  // mutations as separate sequential requests and a budget equal to the
+  // per-request cap would guarantee only the first one lands (mutations go
+  // last, and they are the rarest records); and below the shutdown budgets
+  // callers allow themselves (the integration test helper's is 3000ms), so the
+  // flush can never be the thing that ties or overruns them.
+  SHUTDOWN_FLUSH_TIMEOUT_MS: 2500,
 
   // Rate limiting
   RATE_LIMIT_WINDOW: 60000, // 1 minute
@@ -101,7 +109,13 @@ export const TELEMETRY_CONFIG = {
 
 export const TELEMETRY_BACKEND = {
   URL: 'https://ydyufsohxdfpopqbubwk.supabase.co',
-  ANON_KEY: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6InlkeXVmc29oeGRmcG9wcWJ1YndrIiwicm9sZSI6ImFub24iLCJpYXQiOjE3NTg3OTYyMDAsImV4cCI6MjA3NDM3MjIwMH0.xESphg6h5ozaDsm4Vla3QnDJGc6Nc_cpfoqTHRynkCk'
+  /**
+   * Supabase publishable key (`sb_publishable_…`), the successor to the legacy
+   * anon JWT. The field keeps the ANON_KEY name to match the SUPABASE_ANON_KEY
+   * environment variable that overrides it — a documented public contract.
+   * Insert-only by design; row access is governed by RLS policies.
+   */
+  ANON_KEY: 'sb_publishable_UbVUTyXgIyvemM9b15auQg_YzGa47Gq'
 } as const;
 
 export interface TelemetryMetrics {

tests/unit/mcp/server-shutdown-telemetry.test.ts

@@ -0,0 +1,91 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+
+vi.mock('../../../src/database/database-adapter');
+vi.mock('../../../src/database/node-repository');
+vi.mock('../../../src/templates/template-service');
+vi.mock('../../../src/utils/logger', () => ({
+  logger: {
+    info: vi.fn(),
+    warn: vi.fn(),
+    debug: vi.fn(),
+    error: vi.fn(),
+  },
+  Logger: class {},
+  LogLevel: { ERROR: 0, WARN: 1, INFO: 2, DEBUG: 3 },
+}));
+
+const { flushBeforeExit, trackSessionStart } = vi.hoisted(() => ({
+  flushBeforeExit: vi.fn().mockResolvedValue(undefined),
+  trackSessionStart: vi.fn(),
+}));
+
+// Covers every telemetry method reached through this barrel — handlers-n8n-manager
+// imports the same one, so a partial stub would fail later tests in this file with
+// "is not a function" rather than a meaningful assertion.
+vi.mock('../../../src/telemetry', () => ({
+  telemetry: {
+    flushBeforeExit,
+    trackSessionStart,
+    trackToolUsage: vi.fn(),
+    trackError: vi.fn(),
+    trackEvent: vi.fn(),
+    trackSearchQuery: vi.fn(),
+    trackValidationDetails: vi.fn(),
+    trackToolSequence: vi.fn(),
+    trackWorkflowCreation: vi.fn(),
+    trackWorkflowMutation: vi.fn(),
+  },
+}));
+
+import { N8NDocumentationMCPServer } from '../../../src/mcp/server';
+
+describe('MCP server shutdown flushes telemetry', () => {
+  let server: N8NDocumentationMCPServer;
+
+  beforeEach(() => {
+    process.env.NODE_DB_PATH = ':memory:';
+    vi.clearAllMocks();
+    flushBeforeExit.mockResolvedValue(undefined);
+    server = new N8NDocumentationMCPServer();
+  });
+
+  afterEach(() => {
+    delete process.env.NODE_DB_PATH;
+  });
+
+  // Every shutdown path exits via process.exit(), which never emits
+  // 'beforeExit', so this call is the only thing that ships a short session's
+  // queued telemetry. Deleting it would otherwise fail nothing.
+  it('awaits the bounded telemetry flush', async () => {
+    await server.shutdown();
+
+    expect(flushBeforeExit).toHaveBeenCalledTimes(1);
+  });
+
+  it('flushes before waiting on database initialization', async () => {
+    // Telemetry needs no database, so a never-settling init must not also cost
+    // the queued events: the flush is ordered ahead of that await.
+    (server as any).initialized = new Promise(() => {});
+
+    let flushed = false;
+    flushBeforeExit.mockImplementation(async () => {
+      flushed = true;
+    });
+
+    // shutdown() itself never settles here, which is the point — assert the
+    // flush already happened rather than awaiting the call.
+    void server.shutdown();
+    await vi.waitFor(() => expect(flushed).toBe(true));
+  });
+
+  it('still shuts down cleanly when the flush rejects', async () => {
+    // Telemetry must never change a shutdown's outcome: src/mcp/index.ts turns a
+    // throwing shutdown into exit code 1 and skips stdin teardown.
+    flushBeforeExit.mockRejectedValue(new Error('backend unreachable'));
+
+    await expect(server.shutdown()).resolves.toBeUndefined();
+    // Resolving is not enough — assert the cleanup past the flush actually ran.
+    expect((server as any).db).toBeNull();
+    expect((server as any).repository).toBeNull();
+  });
+});