fix(engine): validate repoFullName and prNumber before composing the customer-facing results payload

[tryeverything24]

Closes #9611

Bug

buildResultsPayload (packages/loopover-engine/src/results-payload.ts) composes the customer-facing prLink and summary by raw interpolation of unvalidated input:

• repoFullName was interpolated as-is, so "acme/widgets/../../evil" produced prLink: "https://github.com/acme/widgets/../../evil/pull/1", which a browser resolves to https://github.com/evil/pull/1 — a customer-facing link to an unrelated GitHub path. The same raw value also reached the documented "public-safe" summary.
• hasPr only checked !== null && !== undefined, so prNumber: 0, -3, or a non-integer took the has-PR branch and rendered .../pull/0, .../pull/-3, and "Opened PR #-3 in ...".
• Per-file additions/deletions were folded with ?? 0 only, so caller-supplied negative, fractional, or non-finite counts flowed straight into diffPreview and totals.

The asymmetry was inside this one function: result.title was already scrubbed with redactSecrets, while the two values that become a clickable link got no treatment.

Root cause

The composer trusted its IterationResult input to already be well-formed, but its only guarantees at both entry points (BuildResultsPayloadRequestSchema in src/openapi/schemas.ts and BuildResultsPayloadInput in packages/loopover-contract/src/tools/agent.ts) are z.string().min(1) for repoFullName and z.number().int().nullable().optional() for prNumber — neither traversal-safe nor positive.

Fix (composer-local, per the required pattern)

• Restated the isValidRepoSegment guard locally (the same deliberate duplication governor-ledger.ts and miner/deny-hook-synthesis.ts already use, since the engine package must not import the miner package): repoFullName is valid only when it splits on / into exactly two segments, each matching [A-Za-z0-9._-]+ and neither a bare ./... When invalid, prLink is null and the summary renders the literal unknown repository; the payload is still returned (the function never throws).
• hasPr now additionally requires Number.isInteger(result.prNumber) && result.prNumber > 0; 0, negatives, and non-integers take the existing no-PR branch with the existing "No pull request was opened for ..." phrasing.
• additions/deletions are each normalized with the exact finiteNonNegativeInt body the sibling Rent-a-Loop modules use (loop-consumption.ts / tenant-quota.ts) before entering diffPreview and totals.
• The cross cases hold: valid repo + invalid prNumber still renders the repo name; invalid repo + valid prNumber still yields prLink === null.
• Unchanged: totals.files, diffPreview ordering, the MAX_DIFF_PREVIEW_FILES cap and slice, the status default, and the redactSecrets(title) scrub. No new exported symbol, no zod-schema change.

RED → GREEN

With the fix stashed (current main behavior), the 8 new tests in test/unit/results-payload.test.ts fail; all 8 pre-existing tests pass:

Tests  8 failed | 9 passed (17)
  × regression: a traversal-shaped repoFullName never becomes a customer-facing prLink
  × renders 'unknown repository' for a malformed repoFullName while still returning the payload
  × regression: prNumber 0 takes the no-PR branch
  × treats a negative or non-integer prNumber as no PR
  × an invalid repoFullName with a valid prNumber still yields prLink === null
  × a valid repoFullName with an invalid prNumber still renders the repo name in the summary
  × normalizes negative and fractional per-file counts to non-negative integers
  × normalizes non-finite per-file counts to zero and passes finite non-negative integers through

With the fix applied:

Test Files  1 passed (1)
     Tests  17 passed (17)

The traversal case (prLink === null, summary contains unknown repository and no ../), the prNumber: 0 and -3 cases, the { path: "a", additions: -5, deletions: 2.7 } → { path: "a", additions: 0, deletions: 2 } / totals { files: 1, additions: 0, deletions: 2 } case, and the canonical acme/widgets + prNumber: 42 → https://github.com/acme/widgets/pull/42 case are all asserted by name.

Gates

• npx vitest run test/unit/results-payload.test.ts — 17/17 passed.
• npx vitest run --coverage test/unit/results-payload.test.ts — packages/loopover-engine/src/results-payload.ts: 100% lines (19/19), 100% branches (31/31), 100% functions (6/6) (lcov LF:19 LH:19 BRF:31 BRH:31 FNF:6 FNH:6), so every changed line/branch of the patch is covered, including both arms of every new conditional (valid/invalid repo, valid/invalid prNumber, the four-way cross, and finite/non-finite/negative/fractional counts).
• npm run build --workspace @loopover/engine — clean.
• npm run typecheck:root (tsc --noEmit) — clean.
• git diff --check — clean.

[loopover-orb]

Caution

🛑 LoopOver review result - fixes required

_{Review updated: 2026-07-29 00:55:54 UTC}

2 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This fixes a real bug: buildResultsPayload previously interpolated raw repoFullName into a customer-facing prLink and summary without validating segment shape, allowing traversal-shaped values to produce misleading GitHub URLs, and treated any non-null prNumber (including 0/negative/fractional) as a valid PR. The fix adds a local two-segment validator, a positive-integer check for prNumber, and normalizes per-file additions/deletions to non-negative integers, with a pure fail-open fallback (null prLink, 'unknown repository' text) rather than throwing. The change is well-scoped, closes issue #9611 with a linked issue, and includes thorough regression tests covering the traversal case, malformed repos, and non-positive/non-integer prNumber values.

Nits — 4 non-blocking

• The github.com host is still hardcoded at results-payload.ts:87 as before this diff — not introduced here, but worth flagging as future hardening if the codebase moves toward config-driven URLs.
• codecov/patch shows only 37.83% of the diff hit despite the extensive new test file — worth double-checking that the coverage tool is picking up the right diff base, since the added tests appear to exercise nearly every new branch.
• Consider adding a test where repoFullName contains a valid two-segment shape but with URL-unsafe characters requiring encoding (e.g. spaces) to confirm the regex correctly rejects them (already partially covered by 'acme/wid gets').
• If `isValidRepoSegment`/`isValidRepoFullName` duplication across governor-ledger.ts and miner/deny-hook-synthesis.ts continues to grow, consider hoisting to a small shared validation-only package to avoid drift between the three copies.

CI checks failing

• codecov/patch — 67.56% of diff hit (target 99.00%)

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	#9611
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	❌ 5/25	Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload	✅ 10/10	Author activity: 72 registered-repo PR(s), 38 merged, 4 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	tryeverything24; Gittensor profile; 72 PR(s), 4 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: moderate

Linked issue satisfaction

Addressed
The diff adds validation for repoFullName (two-segment guard rendering "unknown repository" and null prLink when invalid), requires prNumber to be a positive integer for the has-PR branch, and normalizes additions/deletions via finiteNonNegativeInt, matching all stated requirements; new regression tests cover the traversal case, prNumber 0/-3, mixed valid/invalid combos, and the negative/fractiona

Review context

• Author: tryeverything24
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is registered but has no active allocation in the current snapshot.
• Public profile languages: not available
• Official Gittensor activity: 72 PR(s), 4 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Await review-lane availability.
• Then work through the remaining 2 steps 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 <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> 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.

_{🟩 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]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

❌ Patch coverage is 67.56757% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.01%. Comparing base (bd139a5) to head (681d7d9).

Files with missing lines	Patch %	Lines
packages/loopover-engine/src/results-payload.ts	67.56%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9619      +/-   ##
==========================================
- Coverage   90.02%   90.01%   -0.02%     
==========================================
  Files         888      888              
  Lines      111983   112015      +32     
  Branches    26570    26574       +4     
==========================================
+ Hits       100810   100826      +16     
- Misses       9843     9855      +12     
- Partials     1330     1334       +4     

Flag	Coverage Δ
backend	95.56% <100.00%> (-0.01%)	⬇️
engine	66.53% <37.83%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/loopover-engine/src/results-payload.ts	75.49% <67.56%> (-5.94%)	⬇️

... and 1 file with indirect coverage changes

[loopover-orb]

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.