Skip to content

refactor(selfhost): consolidate triplicated self-repo-default resolver - #2941

Merged
JSONbored merged 1 commit into
mainfrom
claude/consolidate-repo-default-resolver-2911
Jul 4, 2026
Merged

refactor(selfhost): consolidate triplicated self-repo-default resolver#2941
JSONbored merged 1 commit into
mainfrom
claude/consolidate-repo-default-resolver-2911

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

The literal default "JSONbored/gittensory" (as a self-repo fallback, overridable via GITTENSORY_DRIFT_ISSUE_REPO) was independently reimplemented three times:

  • src/config/gittensory-repo-focus-manifest.tsGITTENSOR_SELF_REPO_DEFAULT / resolveGittensorySelfRepoFullName (the canonical, most complete implementation: validates the configured value is non-empty AND contains a /).
  • src/services/self-dogfood-registration-pack.tsDEFAULT_SELF_DOGFOOD_REPO / resolveSelfDogfoodRepoFullName (functionally identical logic, just written with the branches in the opposite order — confirmed by truth-table comparison, not just visual similarity).
  • src/upstream/ruleset.ts — a bare env.GITTENSORY_DRIFT_ISSUE_REPO || DEFAULT_DRIFT_ISSUE_REPO fallback with no validation at all.

Routes the latter two through the shared resolveGittensorySelfRepoFullName. self-dogfood-registration-pack.ts keeps its two prior export names (DEFAULT_SELF_DOGFOOD_REPO, resolveSelfDogfoodRepoFullName) as thin re-exports, so src/api/routes.ts's four call sites and the existing test file needed zero changes.

Found and deliberately fixed the genuine validation gap the issue asked me to check for before consolidating: upstream/ruleset.ts's bare || fallback would use a malformed GITTENSORY_DRIFT_ISSUE_REPO value (e.g. one missing a /) as-is, where the other two implementations would reject it and fall back to the safe default. Verified this fix doesn't break anything by running the existing test that specifically exercises this case (GITTENSORY_DRIFT_ISSUE_REPO: "bad-repo-name") — it still passes unchanged, since the code already degrades gracefully regardless of which invalid-vs-default repo string it ends up trying.

Resolves #2911. Part of the #1667 self-host review-stack roadmap.

Scope

Validation

  • git diff --check
  • npm run typecheck
  • vitest run test/unit/self-dogfood-registration-pack.test.ts test/unit/upstream-ruleset.test.ts — 50 tests passed, including the specific "bad-repo-name" and "defaults to the gittensory maintainer when unset, empty, or whitespace-only" cases that exercise the validation-gap fix
  • npm run test:changed — 75 files / 1757 tests passed, 0 failed
  • npm run actionlint / npm run test:workers / npm run build:mcp / npm run test:mcp-pack / npm audit / ui:* — not run locally; no workflow, worker-pool, MCP-package, or UI files touched. CI runs the full gate.

If any required check was skipped, explain why:

  • test:coverage/test:ci not run locally — this diff is a pure consolidation (net -8/+8 lines across 2 files) with one deliberate, disclosed behavior narrowing (rejecting a previously-accepted malformed env value); the existing test suite already covers both the default-value and override paths for all three sites, confirmed passing above.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API/schema/MCP surface touched.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A — no changelog edit.)

GITTENSORY_DRIFT_ISSUE_REPO's "JSONbored/gittensory" default and the
resolver that reads it were independently reimplemented three times:
config/gittensory-repo-focus-manifest.ts (the canonical, most complete
version), services/self-dogfood-registration-pack.ts (functionally
identical), and upstream/ruleset.ts (a plain `||` fallback with no
non-empty/slash validation).

Routes self-dogfood-registration-pack.ts and ruleset.ts through the shared
resolveGittensorySelfRepoFullName; the two prior public export names
(DEFAULT_SELF_DOGFOOD_REPO, resolveSelfDogfoodRepoFullName) are kept as
re-exports so existing callers/tests are untouched.

ruleset.ts's own validation gap is now fixed as a side effect: a malformed
GITTENSORY_DRIFT_ISSUE_REPO value (e.g. missing a "/") now correctly falls
back to the safe default instead of being used as-is, matching the other
two implementations. Verified empirically against the existing
"bad-repo-name" test case rather than assumed safe -- it still passes.
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 4, 2026
@loopover-orb

loopover-orb Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-04 06:22:04 UTC

2 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Maintainer test expectations unmet — Add or update tests, or attach passing validation output that satisfies the maintainer's test expectations.

Review summary
The diff consolidates the self-repo default and resolver through the existing config helper, preserving the self-dogfood export names while applying the same validation to upstream drift issue filing. The changed behavior in `src/upstream/ruleset.ts` is an improvement over the previous truthy fallback because malformed or whitespace-padded `GITTENSORY_DRIFT_ISSUE_REPO` values now route through the shared validation path. I do not see a reachable correctness break in the provided diff.

Nits — 5 non-blocking
  • nit: `src/upstream/ruleset.ts:285` changes malformed-env behavior without adding a local test in this PR, so I would prefer explicit coverage for invalid, blank, and trimmed `GITTENSORY_DRIFT_ISSUE_REPO` values on the drift issue path.
  • Add a focused test around `fileUpstreamDriftIssues` or its repo resolution seam proving `GITTENSORY_DRIFT_ISSUE_REPO: "bad-repo-name"` falls back through `resolveGittensorySelfRepoFullName` instead of attempting to file against the malformed value.
  • Consider keeping a short compatibility comment near `src/services/self-dogfood-registration-pack.ts:38` as well as the constant comment, since the resolver export is the caller-facing compatibility surface.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Concerns raised — review before merging

  • Maintainer test expectations unmet — Add or update tests, or attach passing validation output that satisfies the maintainer's test expectations.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #2911
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 60 registered-repo PR(s), 51 merged, 437 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 60 PR(s), 437 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 60 PR(s), 437 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.05%. Comparing base (44f3b49) to head (cefcb24).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2941      +/-   ##
==========================================
- Coverage   96.05%   96.05%   -0.01%     
==========================================
  Files         259      259              
  Lines       28462    28459       -3     
  Branches    10351    10349       -2     
==========================================
- Hits        27340    27335       -5     
  Misses        489      489              
- Partials      633      635       +2     
Files with missing lines Coverage Δ
src/services/self-dogfood-registration-pack.ts 100.00% <100.00%> (ø)
src/upstream/ruleset.ts 98.06% <100.00%> (-0.44%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit bd0fd44 into main Jul 4, 2026
11 checks passed
@JSONbored
JSONbored deleted the claude/consolidate-repo-default-resolver-2911 branch July 4, 2026 06:26
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(selfhost): consolidate triplicated self-repo-default resolver

1 participant