Skip to content

fix(api): validate discovery routes against the schemas their spec publishes - #10225

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10040
Jul 31, 2026
Merged

fix(api): validate discovery routes against the schemas their spec publishes#10225
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10040

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

fix(api): validate discovery routes against the schemas their spec publishes

FindOpportunitiesRequestSchema and IssueRagRetrieveRequestSchema were published
in the OpenAPI document but never wired into the handlers, which validated with
independent hand-rolled checks instead — so a generated client accepted bodies
the server rejected and vice versa, and the REST route disagreed with its own
MCP tool on the same input.

Both request schemas now re-derive from @loopover/contract's FindOpportunitiesInput
and RetrieveIssueContextInput (single source instead of three restatements), the
routes parse against them before the hand-rolled cross-field/trim pass, and that
pass no longer silently truncates over-long labels/body or accepts non-integer
limit/topK. RetrieveIssueContextInput's owner/repo/title also gain .min(1), since
the catalog published empty strings as valid when every handler always rejected
them.

gen-contract-api-schemas.ts only scanned packages/loopover-contract/src/*.ts, so
it never resolved the contract-tool imports these two schemas rely on; extended
it to walk one level of subdirectories the same way the top-level scan already
does.

Closes #10040

Note: the checked-in openapi.json under apps/loopover-ui/public/ is a generated artifact — it is regenerated from the spec source by the repo's own script, not hand-edited, and stays in sync via the spec parity test in this PR. No rendered UI changes, so there is no visual diff to screenshot.

…blishes

FindOpportunitiesRequestSchema and IssueRagRetrieveRequestSchema were published
in the OpenAPI document but never wired into the handlers, which validated with
independent hand-rolled checks instead — so a generated client accepted bodies
the server rejected and vice versa, and the REST route disagreed with its own
MCP tool on the same input.

Both request schemas now re-derive from @loopover/contract's FindOpportunitiesInput
and RetrieveIssueContextInput (single source instead of three restatements), the
routes parse against them before the hand-rolled cross-field/trim pass, and that
pass no longer silently truncates over-long labels/body or accepts non-integer
limit/topK. RetrieveIssueContextInput's owner/repo/title also gain .min(1), since
the catalog published empty strings as valid when every handler always rejected
them.

gen-contract-api-schemas.ts only scanned packages/loopover-contract/src/*.ts, so
it never resolved the contract-tool imports these two schemas rely on; extended
it to walk one level of subdirectories the same way the top-level scan already
does.
@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 31, 2026 13:05
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 13:34:04 UTC

16 files · 1 AI reviewer · no blockers · readiness 95/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This closes #10040 by making both discovery routes (`/v1/opportunities/find`, `/v1/issue-rag/retrieve`) parse against the exact zod schemas (`FindOpportunitiesInput`/`RetrieveIssueContextInput`) the OpenAPI document publishes, instead of drifting hand-rolled checks — and trims the MCP-side helpers down to only the cross-field/trim logic zod can't express (targets-or-searchQuery, whitespace-only fields, no more silent truncation of over-long body/labels). The `gen-contract-api-schemas.ts` fix to walk one level of subdirectories is the real root-cause fix that makes the contract-side import of `tools/local-branch.js` resolvable, and it's exercised by CI regenerating `openapi.json`. Test coverage is thorough and traces the intended REST/MCP parity claim end-to-end.

Nits — 5 non-blocking
  • src/mcp/issue-rag.ts's `cleanLabels` still rejects a label whose *trimmed* length exceeds `PREFLIGHT_LIMITS.labelChars` (reason `invalid_labels`), but since the REST route and (per the updated MCP test comment) the MCP tool both already reject any label exceeding `labelChars` at the schema layer before trimming, and trimming can only shrink length, this branch looks unreachable on both real entry points — the new unit test in test/unit/mcp-issue-rag.test.ts exercises it only by calling `validateIssueRagInput` directly, bypassing the schema.
  • packages/loopover-contract/src/api-schemas.ts gains an unrelated `linkedIssueMaintainerExempt` field with no corresponding source-schema change visible in this diff — worth confirming this is just the generated file re-syncing to an already-merged field on `src/openapi/schemas.ts`'s `RepositorySettingsSchema` rather than stray scope creep pulled in by regeneration.
  • The new magic numbers noted by the brief (e.g. `39` in local-branch.ts:532, `12` in issue-rag.ts:58) are pre-existing bound values being relocated/duplicated rather than newly invented, so naming them is optional polish, not required for this PR.
  • If `validateIssueRagInput`'s post-trim label-length branch really is unreachable now that the schema enforces the bound first, consider dropping it (or explicitly documenting why the helper must remain safe to call without prior schema validation) rather than covering it with a test that manufactures a state the real callers can't produce.
  • Double check `linkedIssueMaintainerExempt` in packages/loopover-contract/src/api-schemas.ts:437 didn't get pulled into this PR's diff by accident during regeneration — if it belongs to a different in-flight PR, regenerate against a clean base to avoid drift.

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 #10040
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: 130 registered-repo PR(s), 92 merged, 3 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 130 PR(s), 3 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The PR single-sources FindOpportunitiesRequestSchema/IssueRagRetrieveRequestSchema from the contract's FindOpportunitiesInput/RetrieveIssueContextInput, wires schema.safeParse into both routes before the hand-rolled validators (which now only keep cross-field/trim/normalisation logic), preserves the existing 400 response shapes, stops silently truncating over-long labels/body, adds .min(1) to owne

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript
  • Official Gittensor activity: 130 PR(s), 3 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ 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.

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

@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 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.22%. Comparing base (f26e656) to head (7575aa2).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10225      +/-   ##
==========================================
- Coverage   92.22%   92.22%   -0.01%     
==========================================
  Files         937      937              
  Lines      114379   114370       -9     
  Branches    27655    27647       -8     
==========================================
- Hits       105486   105476      -10     
- Misses       7588     7589       +1     
  Partials     1305     1305              
Flag Coverage Δ
backend 95.67% <100.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
packages/loopover-contract/src/api-schemas.ts 100.00% <100.00%> (ø)
...ckages/loopover-contract/src/tools/local-branch.ts 100.00% <ø> (ø)
src/api/routes.ts 95.67% <100.00%> (+0.05%) ⬆️
src/mcp/find-opportunities.ts 100.00% <ø> (ø)
src/mcp/issue-rag.ts 100.00% <100.00%> (ø)
src/openapi/schemas.ts 100.00% <100.00%> (ø)
src/openapi/spec.ts 99.27% <ø> (ø)

... and 1 file with indirect coverage changes

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

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.

api(discovery): validate /v1/opportunities/find and /v1/issue-rag/retrieve against the schemas their spec publishes

1 participant