Skip to content

fix(test): stop repair diagnostics test racing a live manifest fetch - #6483

Closed
JSONbored wants to merge 1 commit into
mainfrom
claude/sad-cori-6b3cd6
Closed

fix(test): stop repair diagnostics test racing a live manifest fetch#6483
JSONbored wants to merge 1 commit into
mainfrom
claude/sad-cori-6b3cd6

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • The installation-repair integration test's first /repair request didn't stub fetch, so loadRepoFocusManifest could hit the real github.com/ghraw endpoint for JSONbored/gittensory.
  • That repo name also matches this test env's default LOOPOVER_DRIFT_ISSUE_REPO, so on any fetcher miss the loader's self-repo fallback injected the live repo's .loopover.yml (which carries autonomy: auto) regardless of network outcome — making the test fail deterministically in any sandbox with outbound network access, independent of whether the live fetch itself succeeded or failed.
  • Stub fetch to a deterministic 404 and point LOOPOVER_DRIFT_ISSUE_REPO at an unrelated repo name so neither the real network path nor the self-repo fallback fires.

Test plan

  • npx vitest run test/integration/api.test.ts -t "serves installation repair diagnostics" — green, 5 consecutive runs
  • npx vitest run test/integration/api.test.ts — 47/47 passing
  • npm run typecheck — clean

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 16, 2026
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
17286 4 17282 13
View the top 2 failed test(s) by shortest run time
test/unit/backfill.test.ts > GitHub backfill > marks comment, label, and check repair impacts disabled by repo settings
Stack Traces | 0.731s run time
AssertionError: expected { metadata: 'read', …(3) } to not have property "contents"

- Expected:
undefined

+ Received:
"write"

 ❯ test/unit/backfill.test.ts:1203:44
test/unit/check-migrations-script.test.ts > check-migrations script > reports every grandfathered duplicate migration number in the success summary
Stack Traces | 1.01s run time
Error: Command failed: .../loopover/node_modules/.bin/tsx scripts/check-migrations.mjs
check-migrations: duplicate migration number 0156: "0156_draft_pr_close_policy.sql", "0156_pull_request_screenshot_table_presence_satisfied.sql". Two PRs grabbed the same number — renumber the newest to the next free number (0157).

 ❯ test/unit/check-migrations-script.test.ts:38:20

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { status: 1, signal: null, output: [ null, '', 'check-migrations: duplicate migration number 0156: "0156_draft_pr_close_policy.sql", "0156_pull_request_screenshot_table_presence_satisfied.sql". Two PRs grabbed the same number — renumber the newest to the next free number (0157).\n' ], pid: 3635, stdout: '', stderr: 'check-migrations: duplicate migration number 0156: "0156_draft_pr_close_policy.sql", "0156_pull_request_screenshot_table_presence_satisfied.sql". Two PRs grabbed the same number — renumber the newest to the next free number (0157).\n' }
View the full list of 2 ❄️ flaky test(s)
test/unit/backfill.test.ts > GitHub backfill > repair diagnostics require contents:write for merge autonomy (#audit-install-health display)

Flake rate in main: 32.35% (Passed 23 times, Failed 11 times)

Stack Traces | 0.629s run time
AssertionError: expected 'write' to be 'read' // Object.is equality

Expected: "read"
Received: "write"

 ❯ test/unit/backfill.test.ts:1267:54
test/unit/queue-5.test.ts > queue processors > REGRESSION (#slop-commit-messages): the live slop gate fetches the PR's own commit messages, so low_quality_commit_message can actually fire

Flake rate in main: 50.00% (Passed 5 times, Failed 5 times)

Stack Traces | 0.554s run time
AssertionError: expected null to be 15 // Object.is equality

- Expected:
15

+ Received:
null

 ❯ test/unit/queue-5.test.ts:2708:31

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
The installation-repair test's first request had no fetch stub, so
loadRepoFocusManifest hit the real github.com/ghraw endpoint.
Worse, its target repo matches the default LOOPOVER_DRIFT_ISSUE_REPO
test env value, so on any fetcher miss the loader's self-repo fallback
injected the live repo's autonomy:auto block anyway, making the test
fail deterministically regardless of network outcome. Stub fetch and
point the env at an unrelated repo name so neither path fires.
@JSONbored
JSONbored force-pushed the claude/sad-cori-6b3cd6 branch from 8ad5199 to 96f8a7a Compare July 16, 2026 10:54
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-16 11:20:52 UTC

1 file · 1 AI reviewer · 2 blockers · CI failing · blocked

🛑 Suggested Action - Manual Review

Review summary
This is a narrow test-only fix that stubs global fetch to a deterministic 404 and swaps LOOPOVER_DRIFT_ISSUE_REPO to an unrelated repo name so the installation-repair test no longer depends on either a live network call or the self-repo fallback in loadRepoFocusManifest. The change is well-targeted, matches the stated root cause, and is scoped to a single test's setup lines with no production code touched. Test plan shows 5 consecutive green runs and full suite passing, consistent with the fix eliminating the nondeterminism described.

Nits — 4 non-blocking
  • The comment on line ~2149 explaining the self-repo fallback risk is a bit dense; consider linking directly to loadRepoFocusManifest's implementation for future readers.
  • No assertion was added to confirm the stubbed fetch is actually invoked/hit exactly once, which would make the test's intent more explicit.
  • Consider extracting the fetch-stub-to-404 pattern into a shared test helper if other integration tests hit the same manifest-fetch race, per test/helpers conventions.
  • Verify no other tests in api.test.ts share this same racing pattern with the default LOOPOVER_DRIFT_ISSUE_REPO and JSONbored/gittensory self-repo fallback.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.

CI checks failing

  • validate
  • validate-tests (2)
  • validate-tests (5)
  • validate-code

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 33 registered-repo PR(s), 26 merged, 326 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 33 PR(s), 326 issue(s).
Improvement ℹ️ Insufficient signal risk: clean · value: insufficient-signal
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 33 PR(s), 326 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 3 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 16, 2026
@JSONbored

Copy link
Copy Markdown
Owner Author

Closing as superseded by #6493 — same root cause, broader fix. See the note on #6491 for the full explanation: this test's failure and #6491's 3-test fix are both symptoms of createTestEnv()'s shared default leaking the bundled self-repo manifest into any fixture named "JSONbored/gittensory"; #6493 fixes the default itself (plus 10 affected files total) rather than patching this one call site.

@JSONbored JSONbored closed this Jul 16, 2026
@JSONbored
JSONbored deleted the claude/sad-cori-6b3cd6 branch July 19, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant