fix(github): log discarded review-files fetch warnings, clear npm audit findings#7608
Conversation
fetchAndStorePullRequestFilesForReview built a warnings array from
fetchPullRequestFiles but never read it, so a REST+GraphQL double
failure during the live-review inline file fetch degraded to an
empty file list with zero observability. Log it (matching the
console.error(JSON.stringify({level, event, ...})) convention used
elsewhere in src/) without changing the fail-safe return-[] behavior.
adm-zip <0.6.0 (GHSA-xcpc-8h2w-3j85, high) is a transitive dependency of github-actionlint (adm-zip: ^0.5.16), used via extractAllTo in its binary download/extract step. adm-zip@0.6.0 is the first patched version; force it with a package.json override (the same mechanism already used here for tar/esbuild/ws/js-yaml). Verified against a cleared ~/.github-actionlint cache -- forcing a real re-extraction through the new adm-zip code path, not a cached hit -- that actionlint still passes. body-parser 2.0.0-2.2.2 (GHSA-v422-hmwv-36x6, low) is transitive via @modelcontextprotocol/sdk -> express. body-parser@2.3.0 satisfies the existing declared range, so `npm update body-parser` resolves it with a lockfile-only change (same shape as #7569's brace-expansion fix). npm audit --audit-level=moderate: 0 vulnerabilities.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 0cfc746 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 01:11 AM |
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7608 +/- ##
=======================================
Coverage 88.50% 88.50%
=======================================
Files 724 724
Lines 75959 75961 +2
Branches 22609 22610 +1
=======================================
+ Hits 67230 67232 +2
Misses 7681 7681
Partials 1048 1048
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 01:16:54 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
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.
|
Summary
fetchAndStorePullRequestFilesForReview(src/github/backfill.ts) built awarningsarray fromfetchPullRequestFilesbut never read it, so a REST+GraphQL double failure during the live-reviewinline file fetch degraded to an empty file list with zero observability — no log, no metric, no
audit row. Now logs a structured
review_files_fetch_failedwarning (matching theconsole.error(JSON.stringify({level, event, ...}))convention used elsewhere insrc/, e.g.review_files_inline_fetch_failedinsrc/queue/processors.ts) whenwarnings.length > 0, withoutchanging the fail-safe return-
[]behavior itself. Rebased on top of fix(review): retry an empty inline files fetch before trusting it #7601's just-merged retry logic;the log now fires after both fetch attempts, capturing whichever attempt(s) hit the double-failure path.
npm audit --audit-level=moderatefindings left after fix(deps): brace-expansion <1.1.16 has a high-severity ReDoS advisory (GHSA-3jxr-9vmj-r5cp) #7568/fix(deps): update brace-expansion to 1.1.16 to clear GHSA-3jxr-9vmj-r5cp #7569fixed
brace-expansion:adm-zip <0.6.0(high, GHSA-xcpc-8h2w-3j85) via apackage.jsonoverridesentry forcing
^0.6.0(same mechanism already used here fortar/esbuild/ws/js-yaml; verifiedagainst a cleared
~/.github-actionlintbinary cache sonpm run actionlintgenuinely re-extractsthrough the new
adm-zipcode path rather than reusing a stale cache hit), andbody-parser 2.0.0-2.2.2(low, GHSA-v422-hmwv-36x6) vianpm update body-parser(lockfile-only, same shape asfix(deps): update brace-expansion to 1.1.16 to clear GHSA-3jxr-9vmj-r5cp #7569) — the same bump Dependabot PR chore(deps): bump body-parser from 2.2.2 to 2.3.0 #7605 already carries, whose only failing check is the
Codecov-upload step that fails on every Dependabot PR here because Dependabot runs don't get repo
secrets, unrelated to the dependency itself.
Closes #7602, Closes #7607
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlint(re-verified against a cleared~/.github-actionlintcache after the adm-zip override, forcing a real re-extraction)npm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. — the two new lines/branches infetchAndStorePullRequestFilesForRevieware 100% branch-covered (verified via the raw v8 coverage JSON: both sides of the newif (warnings.length > 0)branch are exercised).npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate— 0 vulnerabilities (was 2: adm-zip high, body-parser low)If any required check was skipped, explain why:
npm run test:ci(the full aggregate gate) was run locally end-to-end before pushing; see commit history for the two logically-separate changes (observability log, then dependency audit fixes) resolved together after a rebase ontomainpicked up fix(review): retry an empty inline files fetch before trusting it #7601's just-merged retry logic for the same function.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — backend/dependency-only change (
src/github/backfill.ts,test/unit/backfill-2.test.ts,package.json,package-lock.json), no visible UI/frontend/docs surface.Notes
fix) into one PR at the maintainer's explicit request, to land both together rather than as two
round-trips.