Repository navigation
test: hermetic notion thread-session mocks + drop duplicated usage-analytics suite - #8392
Merged
diegosouzapw merged 2 commits intoJul 24, 2026
Merged
Conversation
…ated usage-analytics file Root cause A (#8159): sendNotionInferenceRequest() in open-sse/executors/notion-web.ts was migrated from fetch() to tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native tls-client-node binary) to get past Notion's Cloudflare TLS fingerprinting. #8159 updated the mock in the sibling tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired through __setTlsFetchOverrideForTesting) but never touched tests/unit/executor-notion-web-thread-sessions.test.ts (split out earlier by #7900) — its 3 execute()-driven tests still mocked globalThis.fetch, which tlsFetchNotion() never calls once the native TLS client loads successfully. Confirmed live: all 3 tests hit real https://app.notion.com with a fake cookie and got a real 401 (~8.1-8.5s each here; on a network with blocked/slow egress this would instead hang up to the client's ~190s timeout+grace per test — a CI-hang risk). Fix: replicate installNotionTlsMock verbatim from the sibling file into executor-notion-web-thread-sessions.test.ts so the 3 tests mock the TLS override point instead of global fetch. Suite is now fully hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s. Root cause B (#7700): tests/unit/usage-analytics-route-extra.test.ts was created as a byte-for-byte duplicate of 10 of the 22 tests in tests/unit/usage-analytics-route.test.ts. #7300 later fixed a fixture bug in the retention-window boundary test ("does not double-count raw and aggregated rows") in the main file only — reading getUserDatabaseSettings().retention.usageHistory live instead of a hardcoded 30-day cutoff (default retention is 365 days) — leaving the duplicate copy on the stale hardcoded value, which now fails (1 !== 2). Fix: delete the duplicate file. All 10 of its test names exist verbatim in the main file (verified with comm -12) and that file passes 22/22: - does not double-count raw and aggregated rows - does not persist guessed API key attribution - does not throw Unknown named parameter on short range (needsAggregated=false) - does not throw Unknown named parameter with apiKey filter on long range - groups renamed API key usage by stable ID - includes activityMap for heatmap - includes cost by API key - omits global aggregates when filtering by API key - returns 500 on database errors - returns weeklyPattern for the costs dashboard No coverage loss — same production code, same assertions, one fewer redundant file. Validation: - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail (401 !== 200, real network hit), 31.0s - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail (1 !== 2), 18.7s - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s, hermetic (no network) - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37 pass, byte-identical diff - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass, byte-identical diff - npx eslint on the changed file: clean - npm run typecheck:core: clean (exit 0) Refs #8159 Refs #7300 Refs #7700
…-masking allowlist check:test-masking hard-flags any deleted test file without a _deletedWithReplacement entry. The deletion is legitimate (100% duplicate suite, coverage retained verbatim in tests/unit/usage-analytics-route.test.ts) -- same registration pattern as the video-dashscope entry. Refs #7700 Refs #7300
diegosouzapw
force-pushed
the
test/basered-notion-mock-analytics-dup
branch
from
July 24, 2026 13:00
64712e2 to
2788bef
Compare
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
…alytics suite (cheaper-inference#8392) * test: hermetic TLS mock for notion thread-session suite + drop duplicated usage-analytics file Root cause A (cheaper-inference#8159): sendNotionInferenceRequest() in open-sse/executors/notion-web.ts was migrated from fetch() to tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native tls-client-node binary) to get past Notion's Cloudflare TLS fingerprinting. cheaper-inference#8159 updated the mock in the sibling tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired through __setTlsFetchOverrideForTesting) but never touched tests/unit/executor-notion-web-thread-sessions.test.ts (split out earlier by cheaper-inference#7900) — its 3 execute()-driven tests still mocked globalThis.fetch, which tlsFetchNotion() never calls once the native TLS client loads successfully. Confirmed live: all 3 tests hit real https://app.notion.com with a fake cookie and got a real 401 (~8.1-8.5s each here; on a network with blocked/slow egress this would instead hang up to the client's ~190s timeout+grace per test — a CI-hang risk). Fix: replicate installNotionTlsMock verbatim from the sibling file into executor-notion-web-thread-sessions.test.ts so the 3 tests mock the TLS override point instead of global fetch. Suite is now fully hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s. Root cause B (cheaper-inference#7700): tests/unit/usage-analytics-route-extra.test.ts was created as a byte-for-byte duplicate of 10 of the 22 tests in tests/unit/usage-analytics-route.test.ts. cheaper-inference#7300 later fixed a fixture bug in the retention-window boundary test ("does not double-count raw and aggregated rows") in the main file only — reading getUserDatabaseSettings().retention.usageHistory live instead of a hardcoded 30-day cutoff (default retention is 365 days) — leaving the duplicate copy on the stale hardcoded value, which now fails (1 !== 2). Fix: delete the duplicate file. All 10 of its test names exist verbatim in the main file (verified with comm -12) and that file passes 22/22: - does not double-count raw and aggregated rows - does not persist guessed API key attribution - does not throw Unknown named parameter on short range (needsAggregated=false) - does not throw Unknown named parameter with apiKey filter on long range - groups renamed API key usage by stable ID - includes activityMap for heatmap - includes cost by API key - omits global aggregates when filtering by API key - returns 500 on database errors - returns weeklyPattern for the costs dashboard No coverage loss — same production code, same assertions, one fewer redundant file. Validation: - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail (401 !== 200, real network hit), 31.0s - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail (1 !== 2), 18.7s - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s, hermetic (no network) - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37 pass, byte-identical diff - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass, byte-identical diff - npx eslint on the changed file: clean - npm run typecheck:core: clean (exit 0) Refs cheaper-inference#8159 Refs cheaper-inference#7300 Refs cheaper-inference#7700 * chore(quality): register usage-analytics-route-extra deletion in test-masking allowlist check:test-masking hard-flags any deleted test file without a _deletedWithReplacement entry. The deletion is legitimate (100% duplicate suite, coverage retained verbatim in tests/unit/usage-analytics-route.test.ts) -- same registration pattern as the video-dashscope entry. Refs cheaper-inference#7700 Refs cheaper-inference#7300
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…alytics suite (cheaper-inference#8392) * test: hermetic TLS mock for notion thread-session suite + drop duplicated usage-analytics file Root cause A (cheaper-inference#8159): sendNotionInferenceRequest() in open-sse/executors/notion-web.ts was migrated from fetch() to tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native tls-client-node binary) to get past Notion's Cloudflare TLS fingerprinting. cheaper-inference#8159 updated the mock in the sibling tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired through __setTlsFetchOverrideForTesting) but never touched tests/unit/executor-notion-web-thread-sessions.test.ts (split out earlier by cheaper-inference#7900) — its 3 execute()-driven tests still mocked globalThis.fetch, which tlsFetchNotion() never calls once the native TLS client loads successfully. Confirmed live: all 3 tests hit real https://app.notion.com with a fake cookie and got a real 401 (~8.1-8.5s each here; on a network with blocked/slow egress this would instead hang up to the client's ~190s timeout+grace per test — a CI-hang risk). Fix: replicate installNotionTlsMock verbatim from the sibling file into executor-notion-web-thread-sessions.test.ts so the 3 tests mock the TLS override point instead of global fetch. Suite is now fully hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s. Root cause B (cheaper-inference#7700): tests/unit/usage-analytics-route-extra.test.ts was created as a byte-for-byte duplicate of 10 of the 22 tests in tests/unit/usage-analytics-route.test.ts. cheaper-inference#7300 later fixed a fixture bug in the retention-window boundary test ("does not double-count raw and aggregated rows") in the main file only — reading getUserDatabaseSettings().retention.usageHistory live instead of a hardcoded 30-day cutoff (default retention is 365 days) — leaving the duplicate copy on the stale hardcoded value, which now fails (1 !== 2). Fix: delete the duplicate file. All 10 of its test names exist verbatim in the main file (verified with comm -12) and that file passes 22/22: - does not double-count raw and aggregated rows - does not persist guessed API key attribution - does not throw Unknown named parameter on short range (needsAggregated=false) - does not throw Unknown named parameter with apiKey filter on long range - groups renamed API key usage by stable ID - includes activityMap for heatmap - includes cost by API key - omits global aggregates when filtering by API key - returns 500 on database errors - returns weeklyPattern for the costs dashboard No coverage loss — same production code, same assertions, one fewer redundant file. Validation: - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail (401 !== 200, real network hit), 31.0s - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail (1 !== 2), 18.7s - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s, hermetic (no network) - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37 pass, byte-identical diff - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass, byte-identical diff - npx eslint on the changed file: clean - npm run typecheck:core: clean (exit 0) Refs cheaper-inference#8159 Refs cheaper-inference#7300 Refs cheaper-inference#7700 * chore(quality): register usage-analytics-route-extra deletion in test-masking allowlist check:test-masking hard-flags any deleted test file without a _deletedWithReplacement entry. The deletion is legitimate (100% duplicate suite, coverage retained verbatim in tests/unit/usage-analytics-route.test.ts) -- same registration pattern as the video-dashscope entry. Refs cheaper-inference#7700 Refs cheaper-inference#7300
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test-only PR. No production code touched.
Root causes
A —
executor-notion-web-thread-sessions.test.tshits realapp.notion.com(#8159)sendNotionInferenceRequest()inopen-sse/executors/notion-web.tswas migrated fromfetch()totlsFetchNotion()(open-sse/services/notionTlsClient.ts, nativetls-client-nodebinary, Chrome-JA3 fingerprint) to get past Notion's Cloudflare TLS fingerprinting — plainfetch()is now only a fallback path used when the native TLS client fails to load.#8159 updated the mock in the sibling
tests/unit/executor-notion-web.test.ts(addedinstallNotionTlsMock, wired through the module's__setTlsFetchOverrideForTestingtest hook) but never touchedtests/unit/executor-notion-web-thread-sessions.test.ts(split out earlier by #7900 for the file-size gate). Its 3execute()-driven tests still mockedglobalThis.fetch, whichtlsFetchNotion()never calls once the native TLS client loads successfully — so the mock silently stopped intercepting anything.Confirmed live in this sandbox: all 3 tests hit real
https://app.notion.comwith a fake cookie and got a real401(AssertionError: 401 !== 200), ~8.1-8.5s each due to the real TLS handshake + round trip. On a CI runner with blocked or slow egress this would instead hang up to the client's timeout+grace window (default180_000ms+10_000msgrace ≈ 190s) per test instead of failing fast — a CI-hang risk, not just 3 red assertions.B —
usage-analytics-route-extra.test.tsis a stale duplicate of 10/22 tests (#7700 / #7300)tests/unit/usage-analytics-route-extra.test.ts(added by #7700) is a byte-for-byte duplicate of 10 of the 22 tests intests/unit/usage-analytics-route.test.ts. #7300 later fixed a fixture bug in the retention-window boundary test ("does not double-count raw and aggregated rows") only in the main file — readinggetUserDatabaseSettings().retention.usageHistorylive instead of hardcoding a 30-day cutoff (default retention is actually 365 days, so the hardcoded 30-day boundary put the "older" row inside the still-current retention window). The duplicate file kept the stale hardcoded 30-day value and now fails deterministically (1 !== 2).Fix
A: Replicated
installNotionTlsMockverbatim fromexecutor-notion-web.test.tsintoexecutor-notion-web-thread-sessions.test.ts(same handler shape:{status, text}→{status, headers: new Headers(), text, body: null}), and swapped all 3globalThis.fetch = .../ restore blocks forinstallNotionTlsMock(...)/restore(). No assertions changed — only the interception point.B: Deleted
tests/unit/usage-analytics-route-extra.test.ts. Confirmed first, viacomm -12of the extractedtest(...)names, that all 10 of its test names exist verbatim in the main file, and that the main file passes in full:does not double-count raw and aggregated rowsdoes not persist guessed API key attributiondoes not throw Unknown named parameter on short range (needsAggregated=false)does not throw Unknown named parameter with apiKey filter on long rangegroups renamed API key usage by stable IDincludes activityMap for heatmapincludes cost by API keyomits global aggregates when filtering by API keyreturns 500 on database errorsreturns weeklyPattern for the costs dashboardNo coverage loss: same production code (
src/app/api/usage/analytics/route.ts), same assertions, one fewer redundant file.Validation
All runs:
env -u OMNIROUTE_API_KEY node --import tsx/esm --test tests/unit/<file>.test.ts, sequential.executor-notion-web-thread-sessions.test.ts401 !== 200, real network) — 31.0sexecutor-notion-web.test.ts(sibling, untouched)usage-analytics-route-extra.test.ts1 !== 2) — 18.7susage-analytics-route.test.ts(untouched)Also ran clean:
npx eslint tests/unit/executor-notion-web-thread-sessions.test.ts— 0 errors/warningsnpm run typecheck:core— exit 0, no errorsRefs #8159
Refs #7300
Refs #7700