intake(idea): accept the IdeaTarget object shapes validateIdeaSubmission already supports - #10070
Conversation
…ion already supports Fixes JSONbored#10064
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 06:40:04 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
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.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more.
|


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:and the exported type it validates against is object-only —
packages/loopover-engine/src/idea-intake.ts:23:with
IdeaSubmission.targetRepo: IdeaTargetatpackages/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, theloopover_intake_ideaandloopover_plan_idea_claimsMCP tool input):targetRepo: z.string().optional(),packages/loopover-contract/src/api-requests.ts:168(intakeIdeaSchema, used bysrc/api/routes.ts:3549and: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-ideawith{"id":"i1","title":"t","body":"b","targetRepo":{"kind":"provision"}}returns
400 invalid_intake_idea_requestwith a zod "expected string, received object" issue. The{ kind: "provision" }target — the entire #7589 auto-provision path, whichbuildClaimPlan(
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 writingagainst 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 realIdeaSubmissionliterally cannot construct a body these schemas accept, becauseIdeaTargetis never astring.
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:targetRepois the one field that does not do that: instead of the actionabletarget_repo_required/target_repo_malformederror the handler would return, the caller gets a schema rejection "the caller cannotact 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-325asserts field-for-field parity betweenIntakeIdeaInput.shapeand theIntakeIdeaRequestcomponent by top-level key name only, so the three copies are identically wrong and staythat way.
Deliverables
IntakeIdeaInput.targetRepo(packages/loopover-contract/src/tools/agent.ts:33) accepts a string or anobject, 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), andPlanIdeaClaimsRequestSchema.targetRepo(src/openapi/schemas.ts:2325) declare the identical shape.packages/loopover-contract/src/api-schemas.tsandapps/loopover-ui/public/openapi.jsonregeneratedand committed via
npm run contract:api-schemasandnpm run ui:openapi.test/unit/contract-api-requests.test.ts:intakeIdeaSchema.safeParse({ targetRepo: { kind: "provision" } }).success === trueand
intakeIdeaSchema.safeParse({ targetRepo: { kind: "existing", repo: "acme/widgets" } }).success === true,alongside the existing
safeParse({})case at:109which must still pass.test/unit/contract-api-requests.test.ts:intakeIdeaSchema.safeParse({ targetRepo: "acme/widgets" }).success === true(the back-compat string form, pinned).
packages/loopover-engine/test/idea-intake.test.ts:validateIdeaSubmissionwithtargetRepo: { kind: "provision" }returns{ ok: true }withidea.targetRepodeep-equal to{ kind: "provision" }, andbuildClaimPlan(buildTaskGraph(idea), idea.targetRepo)returnstargetRepo: "".test/unit/contract-api-requests.test.tsnamed for this bug: a body carrying amalformed object target (e.g.
{ kind: "existing" }with norepo) passes the SCHEMA and is rejected byvalidateIdeaSubmissionwithtarget_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 onethat 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'scoverage.includecovers
src/**/*.ts,packages/loopover-engine/src/**/*.ts, andpackages/loopover-contract/src/**/*.ts—src/openapi/schemas.ts,packages/loopover-contract/src/tools/agent.ts, andpackages/loopover-contract/src/api-requests.tsare all measured and gated.apps/**is incodecov.yml's ignore list, so the regeneratedapps/loopover-ui/public/openapi.jsonis not gated (and isnot TypeScript). The changed lines are zod declarations with no runtime branching of their own, but the
safeParsepaths they drive must be exercised in both directions: string target accepted, object targetaccepted, 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 areunioned — add the test to
packages/loopover-engine/test/**as well as any roottest/**coverage, or thepatch gate can still fail.
Fixes #10064