fix(engine): accept the { kind: 'existing', repo } IdeaTarget shape in validateIdeaSubmission (#9609) - #9634
Conversation
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 02:34:26 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed 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. 🟩 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. |
…n validateIdeaSubmission
validateIdeaSubmission returns an IdeaTarget (a { kind: 'existing', repo } |
{ kind: 'provision' } union), but its own targetRepo validation only accepted a
bare 'owner/name' string or a { kind: 'provision' } object -- so the canonical
{ kind: 'existing', repo } shape it produces (and that any TS caller writing
against the exported IdeaSubmission type constructs) fell through to
target_repo_required. The value it returns did not round-trip through it.
Accept the existing-target object shape, sharing the same owner/name
split-and-guard as the bare-string form via a resolveExistingTarget helper so a
'..'-traversal slug is rejected identically in both forms.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9634 +/- ##
==========================================
- Coverage 90.12% 90.12% -0.01%
==========================================
Files 891 891
Lines 112306 112322 +16
Branches 26629 26633 +4
==========================================
+ Hits 101216 101228 +12
Misses 9760 9760
- Partials 1330 1334 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Problem
Closes #9609.
packages/loopover-engine/src/idea-intake.ts'svalidateIdeaSubmissionreturns{ ok: true, idea }whosetargetRepois anIdeaTarget({ kind: "existing"; repo } | { kind: "provision" }). But its owntargetRepobranch only recognized a bare"owner/name"string (back-compat wire form) or a{ kind: "provision" }object — so the canonical{ kind: "existing", repo: "acme/widgets" }shape it itself produces (and that any TS caller writing against the exportedIdeaSubmissiontype constructs) fell through totarget_repo_required. The value it returns did not round-trip back through it.Fix
Accept the
{ kind: "existing", repo }object shape, resolving its slug through the same split-and-guard as the bare-string form via a sharedresolveExistingTargethelper (exactly-two valid segments, so a"."/".."traversal is rejected identically).{ kind: "provision" }, the bare string, and every rejection path are unchanged.Tests
test/unit/idea-intake-bridge.test.ts(root vitest): the object shape round-trips; a malformed slug inside it is rejected like the string form; andnull/non-object/non-string-repo/missing-repo/unknown-kind all still require a target.packages/loopover-engine/test/idea-intake.test.ts(engine node:test, new): mirrors the same cases so theengineCodecov flag credits the changed lines.100% line + branch coverage on the changed source (108/108 branches).