Skip to content

feat(miner): resolve GitHub token from loopover-mcp session for AMS git operations - #6132

Merged
loopover-orb[bot] merged 2 commits into
mainfrom
feat/ams-github-token-resolution
Jul 15, 2026
Merged

feat(miner): resolve GitHub token from loopover-mcp session for AMS git operations#6132
loopover-orb[bot] merged 2 commits into
mainfrom
feat/ams-github-token-resolution

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • AMS's git-operation entrypoints (loop, attempt, init --verify-token, manage poll) now resolve a GitHub token via a new resolveGitHubToken helper instead of reading env.GITHUB_TOKEN directly: an explicit GITHUB_TOKEN override wins outright (existing self-host PAT setups keep working unchanged), else a live token is fetched from the authenticated loopover-mcp login session (POST /v1/auth/github/token, Persist + expose the GitHub user-to-server token from login instead of discarding it #6114/Decide + implement GitHub token refresh/expiration handling for long-running AMS sessions #6115) and cached in memory for the process's lifetime.
  • doctor's github-token presence check now recognizes a recorded loopover-mcp login session (offline, no network call) in addition to GITHUB_TOKEN, so a user who only ran the new login flow no longer sees a spurious "not set" warning.
  • Docs: packages/loopover-miner/README.md now documents loopover-mcp login as the primary auth path with the PAT as an explicit fallback; packages/loopover-miner/docs/config-precedence.md gets a new precedence section for this resolution order.
  • Deliberately reimplements loopover-mcp's config-file read standalone in github-token-resolution.js rather than adding @loopover/mcp as a runtime dependency of @loopover/miner — the two are separately-installable CLIs, and this milestone's whole point is that installing one doesn't require the other.

Closes #6116

Test plan

  • test/unit/miner-github-token-resolution.test.ts (25 tests, 100% stmt/line, 98.21% branch on the new module — the one uncovered branch is the real-homedir() fallback, same accepted-gap pattern as the module this mirrors)
  • test/unit/miner-cli-doctor-checks.test.ts extended for checkGitHubTokenPresent's new dual-source behavior (env-only, session-only, neither, empty-string)
  • Confirmed (via branch-level coverage inspection) all 5 wired call sites already reach full branch coverage through existing test suites' env.GITHUB_TOKEN fixtures
  • npm run typecheck, npm run test:coverage (unsharded), npm run docs:drift-check, npm run command-reference:check all pass
  • Full local npm run test:ci gate green

…it operations

Closes #6116

AMS's git-operation call sites (loop, attempt, init --verify-token, manage
poll) previously read GITHUB_TOKEN from the environment directly, requiring
a separately-configured PAT even after a user completed `loopover-mcp
login`. resolveGitHubToken() now resolves, at each CLI entrypoint: an
explicit GITHUB_TOKEN override first, else a live token fetched from the
authenticated loopover-mcp session (#6114/#6115), threaded down explicitly
to every real GitHub caller. doctor's github-token check now recognizes a
recorded login session too, so it no longer reports a spurious "not set"
warning for a user who only ran the new login flow.
The #6116 changes make loopover-mcp login sufficient on its own for AMS's
git operations, but the miner README's setup section still only described
the GITHUB_TOKEN PAT flow. Add the one-command login path as primary, with
the PAT kept as the documented explicit fallback.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.33%. Comparing base (e9ecca2) to head (39de876).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6132   +/-   ##
=======================================
  Coverage   95.32%   95.33%           
=======================================
  Files         597      598    +1     
  Lines       47133    47178   +45     
  Branches    15026    15026           
=======================================
+ Hits        44930    44975   +45     
  Misses       1477     1477           
  Partials      726      726           
Flag Coverage Δ
shard-1 43.93% <8.16%> (-0.05%) ⬇️
shard-2 36.66% <46.93%> (+0.01%) ⬆️
shard-3 32.12% <93.87%> (-0.12%) ⬇️
shard-4 34.05% <55.10%> (+0.08%) ⬆️
shard-5 31.45% <38.77%> (-0.19%) ⬇️
shard-6 45.23% <51.02%> (+0.34%) ⬆️

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/attempt-cli.js 98.05% <100.00%> (ø)
...ages/loopover-miner/lib/github-token-resolution.js 100.00% <100.00%> (ø)
packages/loopover-miner/lib/laptop-init.js 98.26% <100.00%> (ø)
packages/loopover-miner/lib/loop-cli.js 98.17% <100.00%> (ø)
packages/loopover-miner/lib/manage-poll.js 97.89% <ø> (ø)
packages/loopover-miner/lib/status.js 94.53% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 15, 2026
@loopover-orb

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-15 12:12:42 UTC

11 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a resolveGitHubToken helper that prefers an explicit GITHUB_TOKEN env override, falls back to a live token fetched from an authenticated loopover-mcp session, and caches only successful resolutions in-process, then wires this into the four AMS git-operation entrypoints (loop, attempt, init --verify-token, manage poll) and doctor's presence check. The precedence logic, config-path resolution, and caching semantics are implemented carefully and match the description; the new module is thoroughly tested (25 tests) including edge cases like malformed JSON, legacy API URLs, and invalid profile names. One real gap: none of the new call sites appear to be exercised by tests for the actual session-fallback path (only existing GITHUB_TOKEN fixtures are claimed to cover them per the test plan), so the wiring itself — as opposed to the underlying resolveGitHubToken unit — is largely unverified.

Nits — 7 non-blocking
  • The PR's own test plan states the 5 call sites reach coverage only via existing GITHUB_TOKEN env fixtures, meaning the new session-fallback branch (`resolveGitHubToken` returning a session token, not env) at each call site in loop-cli.js:274, attempt-cli.js:141/324, manage-poll.js:191/231, and laptop-init.js:310 has no direct integration test — consider adding at least one test per entrypoint that stubs a session-only resolution.
  • github-token-resolution.js's in-memory `cachedToken` is a module-level singleton with no per-env keying, so if a process legitimately switches LOOPOVER_PROFILE or env between calls (unlikely in AMS's actual usage but not structurally prevented), a stale token from a different profile could be served; worth a one-line comment noting this is intentionally scoped to 'one profile per process'.
  • status.js's checkGitHubTokenPresent detail message conflates 'present' with 'valid' (a stale/expired session still reports ok:true) — this is called out well in the code comment, but worth surfacing in the CLI-facing detail string itself for user clarity.
  • Add an integration-level test (in miner-attempt-cli.test.ts / miner-loop-cli.test.ts / etc.) that injects a session-only env (no GITHUB_TOKEN) and a stubbed resolveGitHubToken-style fetch, asserting the token actually reaches fetchLiveIssueSnapshot/fetchSelfReviewContext/verifyGithubToken/pollPrDisposition, closing the gap noted above.
  • Consider whether `fetchGitHubTokenFromSession`'s 10s timeout is appropriate for `manage poll`, which may run in a tight polling loop — a slow/hanging session endpoint could add latency to every poll iteration until the token is cached.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Possible secret-shaped assignment in the diff (generic_secret_assignment) — Verify the value is not a real credential.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #6116
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 43 registered-repo PR(s), 35 merged, 364 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 43 PR(s), 364 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The diff adds a resolveGitHubToken helper implementing exactly the required precedence (GITHUB_TOKEN override, then live session-token fetch, cached in-memory), wires it into all the named call sites (attempt-cli.js, loop-cli.js, manage-poll.js, laptop-init.js), updates doctor's presence check, adds docs describing the new primary flow with PAT as fallback, and includes precedence/fallback tests a

Review context
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 3 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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://gittensory.aethereal.dev/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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit e94ba61 into main Jul 15, 2026
18 checks passed
@loopover-orb
loopover-orb Bot deleted the feat/ams-github-token-resolution branch July 15, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AMS git operations resolve a GitHub token from the loopover-mcp session (one login for both ORB and AMS)

1 participant