Skip to content

intake(idea): accept the IdeaTarget object shapes validateIdeaSubmission already supports - #10070

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
phamngocquy:miner/issue-10064
Jul 31, 2026
Merged

intake(idea): accept the IdeaTarget object shapes validateIdeaSubmission already supports#10070
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
phamngocquy:miner/issue-10064

Conversation

@phamngocquy

Copy link
Copy Markdown
Contributor

Summary

The idea-intake bridge's target-repo field accepts three wire forms. validateIdeaSubmission
(packages/loopover-engine/src/idea-intake.ts:112) implements all three:

  let resolvedTarget: IdeaTarget | undefined;
  if (isNonEmptyString(input.targetRepo)) {
    // Back-compat wire form: a bare "owner/name" string.
    resolvedTarget = resolveExistingTarget(input.targetRepo, errors);
  } else if (typeof input.targetRepo === "object" && input.targetRepo !== null) {
    const target = input.targetRepo as Record<string, unknown>;
    if (target.kind === "provision") {
      resolvedTarget = { kind: "provision" };
    } else if (target.kind === "existing" && isNonEmptyString(target.repo)) {
      resolvedTarget = resolveExistingTarget(target.repo, errors);
    } else errors.push("target_repo_required");
  } else errors.push("target_repo_required");

and the exported type it validates against is object-only —
packages/loopover-engine/src/idea-intake.ts:23:

/** Where an idea's work lands: an existing repo (BYOR) or a not-yet-created one to auto-provision (#7589). */
export type IdeaTarget =
  | { kind: "existing"; repo: string }
  | { kind: "provision" };

with IdeaSubmission.targetRepo: IdeaTarget at packages/loopover-engine/src/idea-intake.ts:32.

Every surface that reaches that validator declares the field as a plain string, so two of the three forms —
including both members of the type the validator returns — are rejected at the schema boundary and never
arrive. There are four copies of the declaration, all identical:

  • packages/loopover-contract/src/tools/agent.ts:33 (IntakeIdeaInput, the loopover_intake_idea and
    loopover_plan_idea_claims MCP tool input): targetRepo: z.string().optional(),
  • packages/loopover-contract/src/api-requests.ts:168 (intakeIdeaSchema, used by
    src/api/routes.ts:3549 and :3564): targetRepo: z.string().optional(),
  • src/openapi/schemas.ts:2290 (IntakeIdeaRequestSchema, the published component):
    targetRepo: z.string().optional(),
  • src/openapi/schemas.ts:2325 (PlanIdeaClaimsRequestSchema, the sibling published component):
    targetRepo: z.string().optional(),

Concretely, POST /v1/loop/intake-idea with {"id":"i1","title":"t","body":"b","targetRepo":{"kind":"provision"}}
returns 400 invalid_intake_idea_request with a zod "expected string, received object" issue. The
{ kind: "provision" } target — the entire #7589 auto-provision path, which buildClaimPlan
(packages/loopover-engine/src/idea-intake.ts:305) has a dedicated branch for
(target.kind === "existing" ? target.repo : "") — is unreachable from every surface in the repo.
{ kind: "existing", repo }, added by #9609 specifically "so a value it produced (or any TS caller writing
against the exported IdeaSubmission type) round-trips back through it"
(packages/loopover-engine/src/idea-intake.ts:121), is unreachable too. A TS consumer holding a real
IdeaSubmission literally cannot construct a body these schemas accept, because IdeaTarget is never a
string.

This also breaks the stated design of all three schemas, which each carry the same comment. From
packages/loopover-contract/src/api-requests.ts:160:

// #6755: mirrors `IntakeIdeaInput` in @loopover/contract VERBATIM. Fields are deliberately LOOSE here for the same
// reason they are on the tool: the engine's validateIdeaSubmission owns the real bounds/format checks and returns
// the actionable error list, so an empty/malformed submission must reach the handler rather than be rejected
// upstream by the schema.

targetRepo is the one field that does not do that: instead of the actionable target_repo_required /
target_repo_malformed error the handler would return, the caller gets a schema rejection "the caller cannot
act on" — the exact outcome IntakeIdeaInput's own doc comment
(packages/loopover-contract/src/tools/agent.ts:43) says the loose typing exists to prevent.

test/unit/openapi.test.ts:320-325 asserts field-for-field parity between IntakeIdeaInput.shape and the
IntakeIdeaRequest component by top-level key name only, so the three copies are identically wrong and stay
that way.

Deliverables

  • IntakeIdeaInput.targetRepo (packages/loopover-contract/src/tools/agent.ts:33) accepts a string or an
    object, still optional, with no discrimination logic in the schema.
  • intakeIdeaSchema.targetRepo (packages/loopover-contract/src/api-requests.ts:168),
    IntakeIdeaRequestSchema.targetRepo (src/openapi/schemas.ts:2290), and
    PlanIdeaClaimsRequestSchema.targetRepo (src/openapi/schemas.ts:2325) declare the identical shape.
  • packages/loopover-contract/src/api-schemas.ts and apps/loopover-ui/public/openapi.json regenerated
    and committed via npm run contract:api-schemas and npm run ui:openapi.
  • Test in test/unit/contract-api-requests.test.ts: intakeIdeaSchema.safeParse({ targetRepo: { kind: "provision" } }).success === true
    and intakeIdeaSchema.safeParse({ targetRepo: { kind: "existing", repo: "acme/widgets" } }).success === true,
    alongside the existing safeParse({}) case at :109 which must still pass.
  • Test in test/unit/contract-api-requests.test.ts: intakeIdeaSchema.safeParse({ targetRepo: "acme/widgets" }).success === true
    (the back-compat string form, pinned).
  • Test in packages/loopover-engine/test/idea-intake.test.ts: validateIdeaSubmission with
    targetRepo: { kind: "provision" } returns { ok: true } with idea.targetRepo deep-equal to
    { kind: "provision" }, and buildClaimPlan(buildTaskGraph(idea), idea.targetRepo) returns
    targetRepo: "".
  • Regression test in test/unit/contract-api-requests.test.ts named for this bug: a body carrying a
    malformed object target (e.g. { kind: "existing" } with no repo) passes the SCHEMA and is rejected by
    validateIdeaSubmission with target_repo_required — proving the error surface is the handler, not zod.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that widens the three schemas but does not commit the regenerated api-schemas.ts/openapi.json, or one
that adds the schema tests without the engine-side test proving the provision target flows all the way
through buildClaimPlan — does not resolve this issue.

Test plan

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, packages/loopover-engine/src/**/*.ts, and packages/loopover-contract/src/**/*.ts
src/openapi/schemas.ts, packages/loopover-contract/src/tools/agent.ts, and
packages/loopover-contract/src/api-requests.ts are all measured and gated. apps/** is in
codecov.yml's ignore list, so the regenerated apps/loopover-ui/public/openapi.json is not gated (and is
not TypeScript). The changed lines are zod declarations with no runtime branching of their own, but the
safeParse paths they drive must be exercised in both directions: string target accepted, object target
accepted, omitted target accepted, and a non-string non-object target (e.g. a number) rejected. The engine
test lands in packages/loopover-engine/test/**; engine lines are credited by two uploads whose hits are
unioned — add the test to packages/loopover-engine/test/** as well as any root test/** coverage, or the
patch gate can still fail.

Fixes #10064

@phamngocquy
phamngocquy requested a review from JSONbored as a code owner July 31, 2026 06:16
@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 06:40:04 UTC

6 files · 1 AI reviewer · no blockers · readiness 88/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR widens targetRepo's wire schema (four zod copies plus the generated openapi.json) from a bare string to string-or-object, unblocking the two object forms validateIdeaSubmission already accepts (kind:'provision' and kind:'existing'). The change is narrowly scoped, mechanical, and consistent with the description's trace through all four schema copies; new tests directly exercise both object shapes and a malformed-object rejection path through validateIdeaSubmission. openapi.json is a generated artifact reflecting the schema change and src/openapi/schemas.ts's long-file size is pre-existing, not introduced by this diff.

Nits — 3 non-blocking
  • z.looseObject({}) accepts any object shape at the schema layer including garbage like {foo:1}, relying entirely on validateIdeaSubmission to reject it downstream — worth a comment noting that's intentional (per the existing intakeIdeaSchema file comment already explains this pattern for other fields).
  • The PR title/description references issue intake(idea): accept the IdeaTarget object shapes validateIdeaSubmission already supports #10064 but the linked-issue check only found partial coverage — worth confirming the issue link is explicit in the PR body.
  • Consider tightening the object arm to z.object({kind: z.string()}).passthrough() or similar if you want the schema layer to reject non-idea-shaped objects earlier, though current behavior (defer to validateIdeaSubmission) seems intentional given the file's existing comment about loose fields.

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 #10064
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
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: 68 registered-repo PR(s), 17 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor phamngocquy; Gittensor profile; 68 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
All four schema declarations now accept targetRepo as a string or loose object via z.union([z.string(), z.looseObject({})]), staying byte-identical and not re-implementing kind/repo discrimination, and the openapi.json artifact plus new tests confirm both object shapes pass the schema while malformed objects still get validateIdeaSubmission's actionable error rather than a zod rejection.

Review context
  • Author: phamngocquy
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 68 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • 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.

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 91.92%. Comparing base (32e3886) to head (558857b).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10070      +/-   ##
==========================================
+ Coverage   91.88%   91.92%   +0.04%     
==========================================
  Files         930      930              
  Lines      113827   113827              
  Branches    27466    27473       +7     
==========================================
+ Hits       104588   104637      +49     
+ Misses       7940     7887      -53     
- Partials     1299     1303       +4     
Flag Coverage Δ
backend 95.66% <ø> (-0.01%) ⬇️
engine 72.59% <ø> (+0.23%) ⬆️

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-requests.ts 98.63% <ø> (ø)
packages/loopover-contract/src/tools/agent.ts 100.00% <ø> (ø)
src/openapi/schemas.ts 100.00% <ø> (ø)

... and 3 files 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.

@loopover-orb
loopover-orb Bot merged commit 55d1e4b into JSONbored:main Jul 31, 2026
10 checks passed
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.

intake(idea): accept the IdeaTarget object shapes validateIdeaSubmission already supports

1 participant