Skip to content

fix(test): stop 3 tests from bleeding real autonomy overrides via self-repo fallback - #6491

Closed
JSONbored wants to merge 1 commit into
mainfrom
fix/self-repo-manifest-autonomy-bleed-773
Closed

fix(test): stop 3 tests from bleeding real autonomy overrides via self-repo fallback#6491
JSONbored wants to merge 1 commit into
mainfrom
fix/self-repo-manifest-autonomy-bleed-773

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Currently on `main`, 3 tests fail: `test/unit/backfill.test.ts` (x2) and `test/integration/api.test.ts` (x1), all in the installation-repair-diagnostics area.
  • Root cause: these tests use `"JSONbored/gittensory"` (the old pre-rename name) as their fixture repo, set up a DB-only `autonomy` override, and assert on the computed required GitHub permissions -- but don't stub `fetch`. `github.com/ghraw` transparently follows the gittensory->loopover repo rename, so the real, unstubbed `fetch` call succeeds and returns the ACTUAL live `.loopover.yml` content. That file gained a real `settings.autonomy` block in fix(settings): give loopover its own autonomy config-as-code #6468 (fixing the "reviews approved but never merged" production bug), so these 3 tests started silently inheriting real production autonomy config instead of their own deterministic DB-only setup, flipping their expected `pull_requests`/`contents` permission assertions.
  • Fix: stub `fetch` to a deterministic 404 in each (removing the live-network dependency), and override `LOOPOVER_DRIFT_ISSUE_REPO` so a 404'd fetch doesn't then fall back to the bundled self-repo manifest constant (which carries the same `autonomy` block).

Scope

  • Test-only change (`test/unit/backfill.test.ts`, `test/integration/api.test.ts`, plus a similarly-affected test in `test/unit/queue-5.test.ts` found during the same investigation)
  • No production code changed

Validation

  • All 3 previously-failing tests pass
  • Full `test/unit` + `test/integration` run is green except for one pre-existing, unrelated migration-number collision (0156, filed separately)

…f-repo fallback

repository_settings-driven tests using "JSONbored/gittensory" as a fixture repo
name, with no fetch mock of their own, made a REAL network call to
github.com/ghraw -- which transparently follows the gittensory->loopover
rename and serves the actual live .loopover.yml. That file gained a real
settings.autonomy block (#773, loopover#6468), so these tests silently inherited
autonomy overrides they never configured, instead of the DB-only setup they
actually assert against.

Fixes 3 tests: stubs fetch to a deterministic 404 (no live network dependency)
and overrides LOOPOVER_DRIFT_ISSUE_REPO so a 404 doesn't fall through to the
bundled self-repo manifest fallback (which carries the same autonomy block)
either.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
17286 1 17285 13
View the top 1 failed test(s) by shortest run time
test/unit/check-migrations-script.test.ts > check-migrations script > reports every grandfathered duplicate migration number in the success summary
Stack Traces | 0.992s 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: 3718, 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' }

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
@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:21:03 UTC

3 files · 1 AI reviewer · 1 blocker · CI failing · blocked

🛑 Suggested Action - Manual Review

Review summary
This is a test-only fix that stubs a previously-unstubbed real network fetch and overrides LOOPOVER_DRIFT_ISSUE_REPO across three tests, preventing them from silently picking up the live .loopover.yml autonomy config introduced in #6468 or the bundled self-repo manifest's own autonomy block. The root-cause analysis is sound and traceable: github.com/ghraw follows the repo rename, so unstubbed fetch calls in these DB-only-settings tests were returning real production config that clobbered the intended deterministic assertions. The fix operates at the correct layer (test isolation) rather than patching symptoms, and the queue-5.test.ts change is a reasonable defensive addition even though that test wasn't in the original failing set.

Blockers

  • The failing CI checks (validate, validate-tests (5), validate-code) are not explained or addressed in the PR description, which claims all target tests pass and only one unrelated pre-existing migration collision remains — this discrepancy between the stated validation and actual CI results should be clarified before merge.
Nits — 4 non-blocking
  • The repeated multi-line comment block explaining the fetch-stub rationale is duplicated near-verbatim across three test cases in two files; consider extracting it to a shared helper or referencing it once (file: test/unit/backfill.test.ts, test/integration/api.test.ts).
  • No assertion verifies that the 404 stub is actually exercised (e.g., no check that resolveRepositorySettings falls back to DB-only settings rather than throwing) — the fix relies on the code under test tolerating a 404 gracefully, which is plausible but unverified in the diff shown.
  • Consider a small shared test helper like `stubFetch404WithIsolatedDriftRepo(overrides)` to DRY up the repeated stub+env pattern across the three files.
  • Investigate and note in the PR description why validate/validate-tests (5)/validate-code are failing on this commit, since the PR claims a clean validation run.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
📋 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.

CI checks failing

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

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
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.
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, which fixes the same 3 tests via the same technique (stub fetch + explicit self-repo override) but also traces the issue to its actual root cause: test/helpers/d1.ts's createTestEnv() default LOOPOVER_DRIFT_ISSUE_REPO was "JSONbored/gittensory" — the same literal string ~100+ generic test fixtures use as an arbitrary placeholder — so ANY bare createTestEnv() call using that fixture name was silently vulnerable to the bundled self-repo manifest leaking in, not just these 3.

Running the full suite after fixing only these 3 surfaces 7 more affected files (ops-wire.test.ts, draft.test.ts, public-stats.test.ts, maintainer-recap-wire.test.ts, upstream-ruleset.test.ts, index.test.ts, queue.test.ts, routes-self-dogfood-registration-pack.test.ts, public-stats-route.test.ts, routes-errors.test.ts) that share the identical root cause and would still be broken after this PR alone. #6493 fixes the default itself plus all of these.

@JSONbored JSONbored closed this Jul 16, 2026
@JSONbored
JSONbored deleted the fix/self-repo-manifest-autonomy-bleed-773 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