test(unit): isolate loopover-mcp/miner tests from real machine state#7515
Conversation
Three test/unit/ files pass or fail depending on the host machine rather than the working tree: local-branch.test.ts compared against the raw os.tmpdir() path instead of safeResolvedPath's realpathSync-resolved form (diverges on macOS, where tmpdir lives under a /var -> /private/var symlink), and miner-init-verify-token.test.ts / miner-self-review- context.test.ts didn't isolate env/loopoverAuth, so resolveGitHubToken / resolveLoopoverBackendSession fell through to a real ~/.config/loopover/config.json on any machine that has run `loopover-mcp login`, firing an extra concurrent fetch that raced with and corrupted other captured calls/headers/timeouts. Closes #7512
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7515 +/- ##
==========================================
- Coverage 91.43% 91.41% -0.02%
==========================================
Files 718 718
Lines 73214 73214
Branches 21692 21692
==========================================
- Hits 66944 66932 -12
- Misses 5227 5234 +7
- Partials 1043 1048 +5
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-20 19:40:27 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
test/unit/local-branch.test.ts,test/unit/miner-init-verify-token.test.ts, andtest/unit/miner-self-review-context.test.tsfail non-deterministically depending on thehost machine, independent of any working-tree change (reproduces on a clean
maincheckout). Root-caused and fixed both distinct bugs; see
#7512for the full writeup.local-branch.test.ts: two assertions compared against the rawos.tmpdir()path insteadof the
realpathSync-resolved formsafeResolvedPath(the code under test) actuallyproduces — diverges on macOS, where
os.tmpdir()lives under a/var→/private/varsymlink. Fixed by resolving the expected path through
realpathSynctoo.miner-init-verify-token.test.ts/miner-self-review-context.test.ts: several callsdidn't isolate
env/loopoverAuth, soresolveGitHubToken/resolveLoopoverBackendSessionfell through to a real
~/.config/loopover/config.jsonon any machine that has runloopover-mcp login, firing an extra concurrent fetch that raced with and corrupted othercaptured fetch calls/headers/timeouts in the same
Promise.all(confirmed by temporarilymoving the real config file aside — all 9 originally-reported failures vanished). Fixed by
isolating
makeTempEnv()'sLOOPOVER_CONFIG_DIR/LOOPOVER_CONFIG_PATH/XDG_CONFIG_HOME/GITHUB_TOKEN, and by passingloopoverAuth: nullto everyfetchSelfReviewContext(...)call site that isn't itself testing the live-gate-thresholds probe (matching that file's own
existing convention for the tests that already do this).
Closes #7512
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:ciran end-to-end and every one of its 35 chained steps passed exceptui:test(which blocked the finalui:buildstep behind it via&&).ui:testfails inthis sandbox because it is running Node v26.5.0, not the repo's pinned Node 22
(
.nvmrc) — under v26, jsdom 25.0.1'swindow.localStoragedoes not initialize correctlyinside vitest's jsdom environment (
Cannot read properties of undefined (reading 'clear')),affecting all 5 pre-existing
apps/loopover-uitest files that touchlocalStorage, notjust ones near this diff. Confirmed unrelated to this PR: this diff touches zero files under
apps/loopover-ui/**; a cleannpm cireproduces the same failure; and the same failurereproduces on unrelated files (
src/lib/use-local-storage.test.ts,src/lib/analytics-window.test.ts) that share nothing with this change. Real CI installsNode 22 per
.nvmrc, so this is a local sandbox environment mismatch, not a code defect —not filed as a tracked issue since it isn't expected to reproduce there.
Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — this PR touches only
test/unit/**files; no UI/frontend/docs/extension change.Notes
coverage.include(src/**only), so thisPR carries no
codecov/patchobligation.