Skip to content

fix(signals): make blockers explain every not-ready registration report (#5946) - #5992

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/registration-readiness-blockers
Jul 15, 2026
Merged

fix(signals): make blockers explain every not-ready registration report (#5946)#5992
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/registration-readiness-blockers

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

Summary

Closes #5946.

buildRegistrationReadiness (src/signals/registration-readiness.ts) computed
ready = blockers.length === 0 && !configFragile && !configNeedsAttention, but configNeedsAttention only ever
appeared in warnings, never in blockers. So when configQuality.level === "needs_attention" and every other
input (isRegistered, configFragile, intakeBlocked) is healthy, the report returned ready: false while
blockers was [] — an empty list that contradicts the field's implied contract as "the reasons registration
isn't ready." A maintainer-facing caller rendering "why can't I register?" from blockers alone would see nothing,
and would have to separately cross-reference warnings and re-derive that one of them is actually gating ready.

The fix follows the approach the issue prefers, mirroring how the sibling "fragile" tier of the same
configQuality.level is already handled (fragile is a blocker, not a warning, and is not duplicated in warnings):

  • Add configNeedsAttention to blockers (right after configFragile, keeping the two tiers of the same field
    adjacent).
  • Remove its now-duplicate warnings entry, so the same fact isn't flagged as both — matching the configFragile
    precedent, which appears only in blockers.
  • Simplify ready to blockers.length === 0, since configFragile and configNeedsAttention are now both
    blockers. blockers is now the single source of truth for every ready === false reason.

directPrReadiness, issueDiscoveryReadiness, and every other field's logic are unchanged — the fix is scoped
strictly to the blockers/ready consistency for the configNeedsAttention condition.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥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:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Ran the full npm run test:ci gate (exit 0) plus npm audit --audit-level=moderate (0 vulnerabilities). The new configNeedsAttention blocker branch is covered on both arms (needs-attention present and absent) via the unsharded npm run test:coverage; the ready line has no branch.

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.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section 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.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes on the Safety boxes: this is a pure, private/API-first advisory report builder with no UI, API/OpenAPI, or auth/CORS/session surface. RegistrationReadinessReport is explicitly advisory and never emitted to public GitHub. The policy-sanitizer.test.ts invariant that asserts these report fields contain no private terms is preserved (updated to check the config-attention message in its new blockers home while keeping both private-terms assertions).

UI Evidence

Not applicable — backend-only change to a deterministic report builder; no visible UI, frontend, docs, or extension surface.

Notes

  • Tests: test/unit/registration-readiness.test.ts — the existing "config attention and strained intake" test now asserts the config-attention message is in blockers (and not in warnings), and a new regression for #5946 test covers the config-needs-attention-only scenario, asserting non-empty blockers explains ready === false. test/unit/policy-sanitizer.test.ts — the "config-attention warnings free of private terms" invariant is updated to assert the message in blockers and the strained-intake note in warnings, keeping both PRIVATE_TERMS_PATTERN checks.
  • test/unit/registration-workspace-ui.test.ts already placed this message in blockers in its fixtures — consistent with this change and unmodified.

…rt (JSONbored#5946)

buildRegistrationReadiness computed `ready = blockers.length === 0 &&
!configFragile && !configNeedsAttention`, but `configNeedsAttention` only ever
appeared in `warnings`, never in `blockers`. So a repo whose config quality is
"needs attention" — with every other input healthy — returned `ready: false`
alongside an empty `blockers` array, contradicting the field's contract that
`blockers` lists the reasons registration is not ready. A caller could not
render "why isn't this ready?" from `blockers` alone.

Add `configNeedsAttention` to `blockers` (mirroring how the sibling "fragile"
tier of the same configQuality.level is already handled), remove its now-
duplicate `warnings` entry, and simplify `ready` to `blockers.length === 0`.
`blockers` is now the single source of truth for every `ready === false` case.
directPrReadiness/issueDiscoveryReadiness logic is unchanged.

Extends the existing "config attention" test to assert `blockers` contents (the
untested gap), adds a regression test for the config-needs-attention-only
scenario, and updates the policy-sanitizer invariant test to check the message
in its new `blockers` home while keeping its private-terms assertions intact.
@shin-core
shin-core requested a review from JSONbored as a code owner July 15, 2026 06:01
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 15, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 15, 2026
@loopover-orb

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-15 06:17:32 UTC

3 files · 1 AI reviewer · no blockers · readiness 80/100 · CI pending · unknown

⏸️ Suggested Action - Manual Review

Review summary
This fix correctly moves `configNeedsAttention` from `warnings` into `blockers` in `buildRegistrationReadiness` (src/signals/registration-readiness.ts), mirroring the existing `configFragile` precedent, removes the duplicate warnings entry, and simplifies `ready` to `blockers.length === 0` — closing the gap where `ready: false` could be reported with an empty `blockers` array. The change is scoped tightly to this one field's logic, is well-tested with a targeted regression test isolating `configNeedsAttention` as the sole gating condition, and updates the two other test files that asserted the old warnings-based behavior. Logic traces cleanly: `configNeedsAttention` is derived once, used consistently in the new blockers array, removed from warnings, and `ready` now only depends on `blockers.length`.

Nits — 3 non-blocking
  • The inline comments above the `blockers`/`warnings`/`ready` lines in registration-readiness.ts are fairly verbose for what the code already states clearly; could be trimmed to a single line each.
  • test/unit/registration-readiness.test.ts's new regression test hardcodes `configQuality.level` to `"needs_attention"` via override — consider a shared helper if this pattern recurs elsewhere to avoid drift with the source's tier definitions.
  • Confirm no other caller (e.g., dashboards or comment renderers) special-cased `configNeedsAttention` warnings text specifically, since it's now absent from `warnings` for any repo relying on that field.
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5946
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High 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: 53 registered-repo PR(s), 25 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 53 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor
Review context
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 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://gittensory.aethereal.dev/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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@JSONbored JSONbored added contributor:verified Contributor passed trust analysis. and removed contributor:flagged Contributor flagged for review by trust analysis. labels Jul 15, 2026
@JSONbored
JSONbored merged commit 601d7dd into JSONbored:main Jul 15, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:verified Contributor passed trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(signals): buildRegistrationReadiness can return ready:false with an empty blockers array

2 participants