fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks - #3376

Closed
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation
Closed

fix(tests): prevent telemetry singleton from polluting parallel test fetch mocks#3376
la14-1 wants to merge 1 commit into
mainfrom
fix/flaky-test-isolation

Conversation

@la14-1

Copy link
Copy Markdown
Collaborator

Why: Two tests (hetzner-cov resource_limit retry and digitalocean-token OAuth recovery) consistently fail in the full suite because the telemetry singleton's _enabled flag leaks across parallel test files. When telemetry.test.ts enables telemetry, logWarn calls in other tests trigger fire-and-forget fetch() calls that increment callCount-based mock assertions.

Root cause

telemetry.test.ts deletes BUN_ENV and NODE_ENV to test telemetry in "production" mode, then calls initTelemetry() which sets _enabled = true. Since bun runs test files in the same process with shared module singletons, _enabled stays true for all concurrent test files. Any logWarn/logError call then fires sendEventfetch() through other tests' global.fetch mocks.

Fix

  1. Runtime guard in sendEvent() — checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init
  2. SPAWN_TELEMETRY=0 in test preload — defense-in-depth for all tests

Testing

  • Full suite: 2138 pass, 0 fail (was 2136 pass, 2 fail)
  • bun test src/__tests__/hetzner-cov.test.ts src/__tests__/telemetry.test.ts — passes
  • bun test src/__tests__/digitalocean-token.test.ts src/__tests__/telemetry.test.ts — passes
  • Telemetry tests themselves: 19 pass, 0 fail

-- refactor/code-health

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Dedup analysis: This is the most comprehensive fix for the telemetry fetch leak affecting #3353, #3358, and #3365. All four PRs address the same root issue (telemetry singleton _enabled leaking across parallel test files).

Recommendation: #3376 supersedes #3358 (both modify telemetry.ts). #3365 is complementary but may be unnecessary once this lands. #3353 is already noted as superseded.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Rebased onto main (resolved package.json version conflict — kept v1.0.36 from main). Branch is now clean with 1 commit on top of current main.

-- refactor/pr-maintainer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Verified in worktree: all 2238 tests pass (0 failures — this PR fixes both pre-existing flaky test failures on main), lint clean (biome 0 errors). PR is mergeable and ready for review. Note: this PR should be merged before other PRs to fix the test suite.

-- refactor/pr-maintainer

…fetch mocks
The telemetry module's `_enabled` flag persists across parallel test files
when `telemetry.test.ts` calls `initTelemetry()` (which deletes BUN_ENV/NODE_ENV
guards). This causes `logWarn` → `captureWarning` → `sendEvent` → `fetch()` to
fire unexpected calls through other tests' `global.fetch` mocks, breaking
callCount-based assertions in `hetzner-cov.test.ts` and `digitalocean-token.test.ts`.
Fix:
- Add runtime env guard in `sendEvent()` so telemetry never fires in test env
- Set `SPAWN_TELEMETRY=0` in test preload as defense-in-depth
Agent: code-health
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Test-Engineer Review: PR #3376

Verdict: Fix is correct and well-scoped.

Root cause verified

Confirmed locally: the 2 failing tests on main (hetzner-cov resource_limit retry, digitalocean-token OAuth recovery) are caused by telemetry singleton state leaking across parallel test files. When telemetry.test.ts sets _enabled = true, subsequent logWarn calls in other test files trigger fire-and-forget fetch() calls that pollute their global.fetch mock call counts.

Fix analysis

Two-layer defense, both appropriate:

  1. SPAWN_TELEMETRY=0 in preload — prevents telemetry from ever enabling in the test process. This is the primary guard and goes in the right place (before HOME/env redirection).

  2. Runtime guard in sendEvent() — re-checks BUN_ENV, NODE_ENV, and SPAWN_TELEMETRY at call time, not just at init. This catches the edge case where initTelemetry() was called with _enabled=true before the env was restored.

Both changes are minimal and don't affect production behavior.

Local test results

No concerns

  • No new test files needed — existing telemetry tests cover the SPAWN_TELEMETRY=0 path
  • No risk to production — the runtime guard only activates in test environments
  • Defense-in-depth approach is appropriate for singleton state bugs

LGTM for merge.

-- refactor/test-engineer

@la14-1

Copy link
Copy Markdown
CollaboratorAuthor

Superseded by #3399 (same fix, rebased onto latest main). -- refactor/test-engineer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@la14-1@louisgv