From 991f3c36503ffe21c9390f4d75d13d396240108a Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Wed, 29 Jul 2026 10:21:28 +0800 Subject: [PATCH] fix(engine): accept the { kind: 'existing', repo } IdeaTarget shape in 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. --- packages/loopover-engine/src/idea-intake.ts | 36 +++++++++++----- .../loopover-engine/test/idea-intake.test.ts | 42 +++++++++++++++++++ test/unit/idea-intake-bridge.test.ts | 17 ++++++++ 3 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 packages/loopover-engine/test/idea-intake.test.ts diff --git a/packages/loopover-engine/src/idea-intake.ts b/packages/loopover-engine/src/idea-intake.ts index 2b6013dd69..0f673ec225 100644 --- a/packages/loopover-engine/src/idea-intake.ts +++ b/packages/loopover-engine/src/idea-intake.ts @@ -86,6 +86,19 @@ function isNonEmptyString(value: unknown): value is string { return typeof value === "string" && value.trim().length > 0; } +/** Resolve an existing-repo target from its "owner/name" string, applying the same split-and-guard as + * repo-clone.ts's normalizeRepoFullName (exactly two valid segments, so a "."/".." traversal is rejected). + * Pushes `target_repo_malformed` and returns undefined on a bad slug. Shared by the bare-string wire form + * and the canonical `{ kind: "existing", repo }` object shape (#9609). */ +function resolveExistingTarget(repo: string, errors: string[]): IdeaTarget | undefined { + const [owner, name, extra] = repo.split("/"); + if (!owner || !name || extra !== undefined || !isValidRepoSegment(owner) || !isValidRepoSegment(name)) { + errors.push("target_repo_malformed"); + return undefined; + } + return { kind: "existing", repo }; +} + /** Validate + normalize a raw renter submission (spec §1). Returns every failure at once (never folds with * `??`/`||`) so a caller can surface all problems in one pass rather than one-at-a-time. */ export function validateIdeaSubmission(raw: unknown): IdeaValidationResult { @@ -102,16 +115,19 @@ export function validateIdeaSubmission(raw: unknown): IdeaValidationResult { // `go`). A `{ kind: "provision" }` object requests a not-yet-created repo (#7589). Anything else is missing. let resolvedTarget: IdeaTarget | undefined; if (isNonEmptyString(input.targetRepo)) { - // Same split-and-guard shape as repo-clone.ts's normalizeRepoFullName: exactly two segments, each a - // valid repo segment, so a bare "." / ".." traversal segment is rejected at intake too. - const [owner, repo, extra] = input.targetRepo.split("/"); - if (!owner || !repo || extra !== undefined || !isValidRepoSegment(owner) || !isValidRepoSegment(repo)) { - errors.push("target_repo_malformed"); - } else { - resolvedTarget = { kind: "existing", repo: input.targetRepo }; - } - } else if (typeof input.targetRepo === "object" && input.targetRepo !== null && (input.targetRepo as Record).kind === "provision") { - resolvedTarget = { kind: "provision" }; + // Back-compat wire form: a bare "owner/name" string. + resolvedTarget = resolveExistingTarget(input.targetRepo, errors); + } else if (typeof input.targetRepo === "object" && input.targetRepo !== null) { + // The canonical IdeaTarget object shapes -- including `{ kind: "existing", repo }`, the exact shape this + // validator itself returns, so a value it produced (or any TS caller writing against the exported + // IdeaSubmission type) round-trips back through it (#9609). `{ kind: "provision" }` requests a + // not-yet-created repo (#7589). + const target = input.targetRepo as Record; + 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"); const constraints = input.constraints; diff --git a/packages/loopover-engine/test/idea-intake.test.ts b/packages/loopover-engine/test/idea-intake.test.ts new file mode 100644 index 0000000000..480def28bf --- /dev/null +++ b/packages/loopover-engine/test/idea-intake.test.ts @@ -0,0 +1,42 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { validateIdeaSubmission } from "../dist/index.js"; + +// Engine-suite (node:test) coverage for validateIdeaSubmission's targetRepo resolution (#9609) so the +// `engine` Codecov flag credits the changed lines, mirroring test/unit/idea-intake-bridge.test.ts. +function rawIdea(targetRepo: unknown) { + return { id: "idea-1", title: "One-line intent", body: "A description.", targetRepo }; +} + +test("resolves a bare owner/name string to an existing target", () => { + const r = validateIdeaSubmission(rawIdea("owner/name")); + assert.equal(r.ok, true); + if (r.ok) assert.deepEqual(r.idea.targetRepo, { kind: "existing", repo: "owner/name" }); +}); + +test("accepts the canonical { kind: 'existing', repo } object it returns (round-trip)", () => { + const r = validateIdeaSubmission(rawIdea({ kind: "existing", repo: "acme/widgets" })); + assert.equal(r.ok, true); + if (r.ok) assert.deepEqual(r.idea.targetRepo, { kind: "existing", repo: "acme/widgets" }); +}); + +test("accepts a provision object", () => { + const r = validateIdeaSubmission(rawIdea({ kind: "provision" })); + assert.equal(r.ok, true); + if (r.ok) assert.deepEqual(r.idea.targetRepo, { kind: "provision" }); +}); + +test("rejects a malformed slug in both the string and the existing-object form", () => { + for (const bad of ["no-slash", "a/b/c", { kind: "existing", repo: "no-slash" }, { kind: "existing", repo: "a/b/c" }]) { + assert.equal(validateIdeaSubmission(rawIdea(bad)).ok, false); + } +}); + +test("requires a target for null, a non-object, a non-string repo, a missing repo, and an unknown kind", () => { + for (const bad of [null, 42, { kind: "existing" }, { kind: "existing", repo: 5 }, { kind: "banana" }, {}]) { + const r = validateIdeaSubmission(rawIdea(bad)); + assert.equal(r.ok, false); + if (!r.ok) assert.ok(r.errors.includes("target_repo_required")); + } +}); diff --git a/test/unit/idea-intake-bridge.test.ts b/test/unit/idea-intake-bridge.test.ts index 86609c8df6..ba0095cf01 100644 --- a/test/unit/idea-intake-bridge.test.ts +++ b/test/unit/idea-intake-bridge.test.ts @@ -82,6 +82,23 @@ describe("validateIdeaSubmission", () => { } }); + it("accepts the canonical { kind: 'existing', repo } object it returns — round-trips — and still validates the slug (#9609)", () => { + const r = validateIdeaSubmission(rawIdea({ targetRepo: { kind: "existing", repo: "acme/widgets" } })); + expect(r.ok).toBe(true); + if (r.ok) expect(r.idea.targetRepo).toEqual({ kind: "existing", repo: "acme/widgets" }); + // A malformed slug inside the object shape is rejected the same as the bare-string wire form. + expect(validateIdeaSubmission(rawIdea({ targetRepo: { kind: "existing", repo: "no-slash" } })).ok).toBe(false); + expect(validateIdeaSubmission(rawIdea({ targetRepo: { kind: "existing", repo: "a/b/c" } })).ok).toBe(false); + }); + + it("still requires a target for null, a non-object, a non-string repo, and an unknown kind (#9609)", () => { + for (const bad of [null, 42, { kind: "existing", repo: 5 }, { kind: "banana" }]) { + const r = validateIdeaSubmission(rawIdea({ targetRepo: bad as unknown as string })); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.errors).toContain("target_repo_required"); + } + }); + it("flags invalid constraints (non-array, non-string element, over-length entry)", () => { expect((validateIdeaSubmission(rawIdea({ constraints: "x" as unknown as string[] }))).ok).toBe(false); expect((validateIdeaSubmission(rawIdea({ constraints: [1] as unknown as string[] }))).ok).toBe(false);