Skip to content

fix(openapi): declare the real statuses for POST /v1/loop/request-apr-transfer - #9816

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/apr-transfer-spec-status-9711
Jul 29, 2026
Merged

fix(openapi): declare the real statuses for POST /v1/loop/request-apr-transfer#9816
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/apr-transfer-spec-status-9711

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

POST /v1/loop/request-apr-transfer's spec entry (requestAprTransfer) declared 200: "Transfer requested". The handler has exactly four exits and none of them is 200:

  • 400 — failed parse
  • 409 — the transfer gate rejected the request (GitHub was not touched)
  • 502 — the transfer attempt failed downstream
  • 202 — initiation accepted, pending acceptance (the success path)

So every generated client documented a status the route cannot return and omitted the three it actually produces.

The fix

Replace 200 with 202 and add 409 + 502 (keeping 400/401), mirroring the concrete-per-status description style of the neighbouring MISC_ROUTES entries. The handler's statuses are unchanged — this only makes the published contract match what it already returns. apps/loopover-ui/public/openapi.json regenerated.

Tests

  • test/unit/openapi.test.ts: buildOpenApiSpec()'s declared statuses for POST /v1/loop/request-apr-transfer are exactly {202, 400, 401, 409, 502}fails on main, which returns 200.
  • The route's real exits are already pinned by test/unit/routes-request-apr-transfer.test.ts (existing 202/409/502/400 cases), which the spec now matches.

Validation

  • npm run typecheck green; npm run ui:openapi:check clean (regenerated spec matches source); both suites green.
  • Diff coverage on the changed spec file is 100%.
  • git diff --check <base> HEAD clean; diff is the spec source + its test + the regenerated openapi.json; no handler or lockfile change.

Closes #9711

…-transfer

The `requestAprTransfer` spec entry declared `200: "Transfer requested"`, but the
handler has exactly four exits — `400` on a parse failure, `409` when the transfer
gate rejects (GitHub untouched), `502` when the attempt fails downstream, and
`202` (pending-acceptance) otherwise. It never returns `200`, so every generated
client documented a status the route cannot produce and omitted the three it can.

Replace `200` with `202` and add `409` + `502` (keeping `400`/`401`), mirroring the
concrete-outcome description style of the neighbouring MISC_ROUTES entries. The
handler's statuses are unchanged — this only makes the published contract match
what it already returns. openapi.json regenerated.

Closes JSONbored#9711
@shin-core
shin-core requested a review from JSONbored as a code owner July 29, 2026 11:09
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 29, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-29 11:22:36 UTC

3 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This PR corrects the declared OpenAPI responses for POST /v1/loop/request-apr-transfer from an unreachable 200 to the handler's actual four exits (202/400/401/409/502), matching the source-of-truth entry in internal-and-public-route-specs.ts to the regenerated openapi.json and adding a spec-shape test. The change is narrow, well-scoped, and the description accurately matches the diff — the regenerated JSON mirrors the TS source change exactly. It closes #9711 as claimed.

Nits — 3 non-blocking
  • The PR doesn't include or update `test/unit/routes-request-apr-transfer.test.ts` in the diff, though the description says it already pins the 202/409/502/400 behavior — worth double-checking that file wasn't touched unnecessarily or that it still exists as claimed.
  • The magic-number status codes (202/400/401/409/502) are inline literals consistent with the rest of the file's style (e.g. MISC_ROUTES entries), so this is a pure style nit, not something this PR should change on its own.
  • Consider whether other MISC_ROUTES entries with similarly stale 200-only responses (if any) should get a follow-up sweep, since this PR shows the same drift pattern could recur elsewhere in this hand-maintained table.
Flagged checks (non-blocking)
  • Contributor trust — Contributor flagged for review

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9711
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 ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 63 registered-repo PR(s), 46 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 63 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Partially addressed
The spec entry, regenerated openapi.json, and the exact-status-set assertion in openapi.test.ts fully satisfy the spec-side deliverables, but the PR adds no new tests to test/unit/routes-request-apr-transfer.test.ts asserting the handler returns 202/409/502 for the corresponding result statuses—it only claims those cases are 'already pinned' by existing tests rather than adding the new tests the i

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 63 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Await review-lane availability.
  • Then work through the remaining 1 step 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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.

Decision record
  • action: hold · clause: success
  • config: 0357f06920bcdbf26ebfb09c040e4d4be1d6bdf77c20519a160e4a7b613b804e · pack: oss-anti-slop · ci: passed
  • record: 6a01cb4d46d8f98f6f79957b0317419d2da43c74107c5cac8f61a604c029cdf6 (schema v5, head 237dff1)
Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
diff /
diff /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy · Diff highlights exactly what changed.

Scroll preview
Route Before (production) After (this PR's preview)
/ before / (scroll)
before / (scroll)
after / (scroll)
after / (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 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

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.03%. Comparing base (0c6b441) to head (237dff1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9816      +/-   ##
==========================================
+ Coverage   79.02%   79.03%   +0.01%     
==========================================
  Files         280      281       +1     
  Lines       58352    58382      +30     
  Branches     6697     6699       +2     
==========================================
+ Hits        46114    46144      +30     
  Misses      11955    11955              
  Partials      283      283              
Flag Coverage Δ
backend 100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/openapi/internal-and-public-route-specs.ts 100.00% <ø> (ø)

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

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Held for manual review: the gate and required CI are green, but GitHub reports this pull request's mergeable state as unstable because a non-required check or status — Contributor trust — is not passing, so LoopOver will not auto-merge. A maintainer can resolve the failing check or review and merge manually. This is an automated maintenance action.

JSONbored added a commit that referenced this pull request Jul 29, 2026
…ty scan (#9818)

* chore(superagent): disable contributor-trust scoring, keep the security scan

The contributor-trust check has been posting `action_required` on established
contributors' PRs — shin-core scored 10/100 "dangerous" on PR #9816 — and was a
major source of manual-review volume.

Two independent reasons to turn it off rather than tune it further:

1. The trustedAuthors exemption does not work. shin-core is listed in it and was
   still flagged. An exemption that does not exempt is worse than none: it looks
   handled while every affected PR still stalls.
2. Its verdict has no consumer. LoopOver now lists this check under
   gate.ignoredCheckRuns (#9813) on all three gate repos, so a non-passing result
   no longer gates, pends, or holds anything. Leaving it on produces a permanently
   red check on contributor PRs that nothing acts on — noise that makes good
   contributors think they have failed something.

prScan stays enabled and still gates CI normally: that is the protection worth
having, and it is a different check from the same app. trustedAuthors is kept
(inert) so the history survives if this is ever revisited.

* chore(superagent): drop the trustedAuthors allowlist too

It provably does not work (a listed author was still flagged action_required) and
is inert with contributorTrust disabled. Keeping it would only suggest the repo
has a working exemption mechanism that it does not.
@JSONbored
JSONbored merged commit 55b6a82 into JSONbored:main Jul 29, 2026
10 of 11 checks passed
JSONbored added a commit that referenced this pull request Jul 29, 2026
…either

gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI
aggregate, but mergeable_state is GITHUB's computation and stays "unstable"
while the check exists at all -- and derivePrDisposition holds unconditionally
on unstable. So the ignore was half-effective: the check no longer failed the
gate, and the PR was held anyway. Observed live on #9816
immediately after the config flip, reason "mergeable_state is unstable --
non-required check(s) not passing: Contributor trust".

Dismiss an unstable state ONLY when the ignore list fully explains it: at least
one ignored run concluded non-passing, our aggregate found no non-required
failure, and ciState is not failed. Any other unstable cause still holds, and
the flag never rescues a PR held for a different reason.

Also make the un-itemized unstable message actionable. It used to say only "a
non-required check or status is not passing" -- no check name, no next step.
GitHub never says why, and the aggregate can legitimately fail to itemize it (a
commit status rather than a check-run, an unreadable app page, a run that
appeared after CI was read). Name that ambiguity and point at the Checks tab.
JSONbored added a commit that referenced this pull request Jul 29, 2026
…either

gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI
aggregate, but mergeable_state is GITHUB's computation and stays "unstable"
while the check exists at all -- and derivePrDisposition holds unconditionally
on unstable. So the ignore was half-effective: the check no longer failed the
gate, and the PR was held anyway. Observed live on #9816
immediately after the config flip, reason "mergeable_state is unstable --
non-required check(s) not passing: Contributor trust".

Dismiss an unstable state ONLY when the ignore list fully explains it: at least
one ignored run concluded non-passing, our aggregate found no non-required
failure, and ciState is not failed. Any other unstable cause still holds, and
the flag never rescues a PR held for a different reason.

Also make the un-itemized unstable message actionable. It used to say only "a
non-required check or status is not passing" -- no check name, no next step.
GitHub never says why, and the aggregate can legitimately fail to itemize it (a
commit status rather than a check-run, an unreadable app page, a run that
appeared after CI was read). Name that ambiguity and point at the Checks tab.
JSONbored added a commit that referenced this pull request Jul 29, 2026
…either (#9820)

* fix(gate): an ignored check must not hold the PR via mergeable_state either

gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI
aggregate, but mergeable_state is GITHUB's computation and stays "unstable"
while the check exists at all -- and derivePrDisposition holds unconditionally
on unstable. So the ignore was half-effective: the check no longer failed the
gate, and the PR was held anyway. Observed live on #9816
immediately after the config flip, reason "mergeable_state is unstable --
non-required check(s) not passing: Contributor trust".

Dismiss an unstable state ONLY when the ignore list fully explains it: at least
one ignored run concluded non-passing, our aggregate found no non-required
failure, and ciState is not failed. Any other unstable cause still holds, and
the flag never rescues a PR held for a different reason.

Also make the un-itemized unstable message actionable. It used to say only "a
non-required check or status is not passing" -- no check name, no next step.
GitHub never says why, and the aggregate can legitimately fail to itemize it (a
commit status rather than a check-run, an unreadable app page, a run that
appeared after CI was read). Name that ambiguity and point at the Checks tab.

* fix(gate): an ignored check must not hold the PR via mergeable_state either

gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI
aggregate, but mergeable_state is GITHUB's computation and stays "unstable"
while the check exists at all -- and derivePrDisposition holds unconditionally
on unstable. So the ignore was half-effective: the check no longer failed the
gate, and the PR was held anyway. Observed live on #9816
immediately after the config flip, reason "mergeable_state is unstable --
non-required check(s) not passing: Contributor trust".

Dismiss an unstable state ONLY when the ignore list fully explains it: at least
one ignored run concluded non-passing, our aggregate found no non-required
failure, and ciState is not failed. Any other unstable cause still holds, and
the flag never rescues a PR held for a different reason.

Also make the un-itemized unstable message actionable. It used to say only "a
non-required check or status is not passing" -- no check name, no next step.
GitHub never says why, and the aggregate can legitimately fail to itemize it (a
commit status rather than a check-run, an unreadable app page, a run that
appeared after CI was read). Name that ambiguity and point at the Checks tab.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. 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.

api(loop): /v1/loop/request-apr-transfer publishes a 200 it never returns

2 participants