Skip to content

miner(contribution-profile): GitHub API URLs are built without escaping or forge/repoPathPrefix awareness #10333

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/opportunity-fanout.ts builds every GitHub API request URL through a
ForgeConfig-aware helper:

function repoPath(forge: ForgeConfig, target: Target, suffix: string): string {
  return `${forge.repoPathPrefix}/${encodeURIComponent(target.owner)}/${encodeURIComponent(target.repo)}${suffix}`;
}

— it both (a) escapes owner/repo with encodeURIComponent, and (b) uses forge.repoPathPrefix
(not a hardcoded "repos"), so a tenant configured against a non-default forge host (a custom
repoPathPrefix, e.g. GitHub Enterprise) still resolves correctly.

packages/loopover-miner/lib/contribution-profile-extract.ts does neither. Its three GitHub API
URL-building call sites interpolate target.owner/target.repo raw, with a hardcoded /repos/
prefix, and no ForgeConfig/repoPathPrefix involved at all:

// fetchRepoLabels, line ~158
`${base}/repos/${target.owner}/${target.repo}/labels?per_page=100&page=${page}`,
// fetchContributing (two call sites), lines ~246 and ~269
`${base}/repos/${target.owner}/${target.repo}/contents/${path}`,

Two distinct, independently real gaps follow from this:

  1. No escaping. Unlike opportunity-fanout.ts's repoPath(), none of these three URLs pass
    target.owner/target.repo through encodeURIComponent. A repo segment containing a URL-special
    character that nonetheless passes this file's own parseRepoFullName (see the companion
    isValidRepoSegment issue for that function's separate gap) can alter the resolved request path.
  2. No forge-awareness. extractContributionProfile's public options
    (ExtractContributionProfileOptions) accept apiBaseUrl (used for the API host) but never a
    repoPathPrefix/ForgeConfig, and every URL hardcodes the literal /repos/ segment. Its only
    caller, resolveContributionProfilesForDiscover in discover-cli.ts, only threads
    ctx.apiBaseUrl through — never a forge/path-prefix value, even though the same file's own
    fan-out path (opportunity-fanout.ts) resolves a full ForgeConfig including repoPathPrefix
    for the exact same discover run. For any tenant whose forge uses a non-default path prefix, every
    fetchRepoLabels/fetchContributing request 404s. Because
    extractContributionProfile/fetchRepoLabels/fetchContributing are written to fail open on a
    failed/malformed response (returning an empty result rather than throwing — see the "Fail-open"
    doc comments already in this file), this failure is silent: the contribution profile for that
    repo comes back empty instead of erroring, which downstream feasibility/eligibility logic then
    treats as "no signal" rather than "this request was misrouted."

Requirements

  • Add encodeURIComponent(target.owner) / encodeURIComponent(target.repo) to all three URL
    template literals listed above in contribution-profile-extract.ts (fetchRepoLabels and both
    fetchContributing call sites), matching opportunity-fanout.ts's repoPath() escaping.
  • Add a repoPathPrefix (or an accepted Partial<ForgeConfig>, matching
    opportunity-fanout.ts's own forge?: Partial<ForgeConfig> option shape) to
    ExtractContributionProfileOptions, resolve it the same way opportunity-fanout.ts resolves
    ForgeConfig via resolveForgeConfig, and use the resolved prefix in place of the hardcoded
    "repos" segment in all three URL-building call sites.
  • Thread the resolved forge/repoPathPrefix value through
    resolveContributionProfilesForDiscover in discover-cli.ts, so a discover run that resolves a
    non-default ForgeConfig for its fan-out also passes the matching repoPathPrefix into
    extractContributionProfile instead of leaving it implicitly defaulted.
  • Do not change DEFAULT_API_BASE_URL or the existing apiBaseUrl handling — this issue is scoped
    to the path-segment construction (prefix + escaping), not the host resolution, which already
    works correctly.

Deliverables

  • All three URL-building call sites in contribution-profile-extract.ts escape owner/repo
    with encodeURIComponent.
  • All three URL-building call sites use a resolved repoPathPrefix (defaulting to "repos"
    when unset, matching ForgeConfig's own default) instead of the hardcoded literal "repos"
    segment.
  • resolveContributionProfilesForDiscover in discover-cli.ts passes its resolved forge
    configuration's repoPathPrefix through to extractContributionProfile.
  • A new regression test asserting that a repo/owner segment requiring escaping (e.g. containing
    a character encodeURIComponent changes) produces a correctly-escaped request URL.
  • A new regression test asserting that a non-default repoPathPrefix is honored in the request
    URL for all three call sites (or a shared helper if you factor the three call sites into one,
    in which case one test covering the shared helper is sufficient — but the fix in all three
    call sites is still required).

All deliverables are required in this one PR — escaping without forge-awareness (or vice versa)
leaves half the bug in place and does not resolve this issue.

Test Coverage Requirements

This repo's Codecov patch gate is 99%+ (branch-counted) on packages/loopover-miner/lib/**. Add
tests under test/unit/** (this package's tests live in the shared root test/ directory; there
is no packages/loopover-miner/test/** directory — verify against the existing sibling test file
test/unit/miner-contribution-profile-extract.test.ts-equivalent before adding). Cover both the
escaping branch and the non-default-prefix branch explicitly, plus the pre-existing default-prefix
behavior continuing to pass unchanged.

Expected Outcome

contribution-profile-extract.ts's GitHub API requests are correctly escaped and forge-aware,
matching the same package's opportunity-fanout.ts convention. A tenant configured with a
non-default repoPathPrefix gets real contribution-profile data instead of a silent, fail-open
empty profile for every repo.

Links & Resources

  • packages/loopover-miner/lib/opportunity-fanout.ts's repoPath() (around line 206) — the
    pattern to mirror for both escaping and forge-awareness.
  • packages/loopover-miner/lib/forge-config.tsForgeConfig type and resolveForgeConfig.
  • packages/loopover-miner/lib/contribution-profile-extract.tsfetchRepoLabels (~line 158),
    fetchContributing (~lines 246, 269), extractContributionProfile (~line 309).
  • packages/loopover-miner/lib/discover-cli.tsresolveContributionProfilesForDiscover (~line
    477), the caller that needs the new option threaded through.

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