Skip to content

fix(miner): queueItemId/parseQueueItemId corrupts round-trip when identifier contains "::" #8857

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

packages/loopover-miner/lib/portfolio-queue-manager.ts:12,27-49 joins apiBaseUrl::repoFullName::identifier in queueItemId, and parseQueueItemId splits from the last "::" for identifier, then the next-to-last for repoFullName. repoFullName is guaranteed "::"-free (isValidRepoSegment, repo-clone.ts:72), but identifier (normalizeIdentifier, portfolio-queue.ts:112-117) has no such restriction. Verified directly: queueItemId("https://api.github.com", "acme/widgets", "issue::5") parses back as { apiBaseUrl: 'https://api.github.com::acme/widgets', repoFullName: 'issue', identifier: '5' } -- silently corrupted. Not triggered today (the only real producer uses a single colon), but nothing enforces the invariant.

Requirements

Reject any identifier containing the literal substring "::" in normalizeIdentifier (portfolio-queue.ts:112-117), throwing a clear validation error at the point of construction rather than allowing silent corruption downstream. Do not attempt an encoding-based fix (e.g. escaping) unless you also add round-trip fuzz tests proving it's collision-proof -- rejecting the invariant violation outright is the simpler, safer default for this issue.

Deliverables

  • normalizeIdentifier throws a clear error when given an identifier containing "::"
  • A regression test asserting queueItemId/parseQueueItemId round-trips correctly for identifiers that do NOT contain "::", and a second test asserting normalizeIdentifier rejects one that does

All of the above Deliverables are required in the same PR unless the deliverable text itself states otherwise.

Test Coverage Requirements

packages/loopover-miner/** -- 99%+ patch coverage, both the rejection path and the existing valid-identifier round-trip.

Expected Outcome

An identifier containing the "::" separator sequence is rejected at construction time instead of silently corrupting the parsed apiBaseUrl/repoFullName/identifier triple.

Links & Resources

  • packages/loopover-miner/lib/portfolio-queue-manager.ts:12,27-49
  • packages/loopover-miner/lib/portfolio-queue.ts:112-117
  • packages/loopover-miner/lib/repo-clone.ts:72

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions