fix(engine): accept the { kind: 'existing', repo } IdeaTarget shape in validateIdeaSubmission (#9634)

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.

Co-authored-by: RealDiligent <nft.gold.eth@gmail.com>

Author: RealDiligent

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<string, unknown>).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<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");
 
   const constraints = input.constraints;

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"));
+  }
+});

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);