Skip to content

refactor(review-enrichment): dedupe githubHeaders() across 17 analyzers - #4675

Merged
JSONbored merged 1 commit into
mainfrom
fix/rees-shared-github-headers-4609
Jul 10, 2026
Merged

refactor(review-enrichment): dedupe githubHeaders() across 17 analyzers#4675
JSONbored merged 1 commit into
mainfrom
fix/rees-shared-github-headers-4609

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • 17 of 63 review-enrichment/src/analyzers/*.ts files hand-copied a private githubHeaders() helper that had drifted into 4 shapes: 11 files sent no User-Agent, 3 added a User-Agent plus a local GITHUB_API_VERSION constant, 1 (exhaustiveness-drift.ts) hardcoded Accept to raw-only (unable to ever request JSON), and 2 reinvented a raw-toggle param a 4th way.
  • Added one exported githubHeaders(token: string, opts?: { raw?: boolean }): Record<string, string> in a new review-enrichment/src/github-headers.ts, using the most complete existing shape (Accept toggle + GITHUB_API_VERSION + User-Agent) as the target shape.
  • Replaced all 17 call sites with an import of the shared helper and deleted each file's own private copy (plus the now-dead local GITHUB_API_VERSION const in the 5 files that had one).
  • Call sites that previously passed a positional true for raw content (caller-impact.ts, unused-export.ts) now pass { raw: true }. exhaustiveness-drift.ts's one call site now explicitly passes { raw: true } too, preserving its existing runtime behavior (it fetches raw file content) while closing the structural gap called out in the issue — it can now request JSON like every other analyzer, it just doesn't need to today.
  • All 17 files updated: approval-integrity.ts, blame-link.ts, churn-hotspot.ts, commit-hygiene.ts, commit-lint.ts, commit-signature.ts, coverage-delta.ts, flaky-test.ts, pending-review-requests.ts, revert-recurrence.ts, stale-branch.ts, asset-weight.ts, duplication-scan.ts, history.ts, exhaustiveness-drift.ts, caller-impact.ts, unused-export.ts.
  • Entirely self-contained within review-enrichment/ (own tsconfig.json, own build, own Railway deploy) — no cross-service-boundary risk, no version-bump ceremony.

Fixes #4609.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • This change touches only review-enrichment/**, which has its own tsconfig.json (not included by the root tsconfig.json) and its own node --test suite (not matched by the root vitest test/**/*.test.ts include), so the root actionlint / typecheck / test:coverage / test:workers / build:mcp / test:mcp-pack / ui:* checks are structurally unaffected by this diff and were not re-run. The actual CI check for this change is rees:test, which I ran directly: npm run rees:test (build + validate:sourcemaps + generate-analyzer-metadata.mjs --check + the full node:test suite) — 1223 passing, 0 failing, including a new test/github-headers.test.ts covering the shared helper's default/raw/empty-opts branches. Ran once before the final rebase and again immediately after rebasing onto latest origin/main, both green.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section 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.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

The four unchecked Safety boxes are not applicable: this PR touches no auth/cookie/CORS/GitHub App/Cloudflare/session code, no API/OpenAPI/MCP surface, no UI, and no docs/changelog — it is a pure internal header-builder consolidation inside review-enrichment/src/analyzers/.

UI Evidence

Not applicable — no UI/frontend/docs changes.

Notes

17 of 63 review-enrichment analyzer files hand-copied a private githubHeaders()
helper that had drifted into 4 shapes: 11 sent no User-Agent, 3 added a
User-Agent plus a local GITHUB_API_VERSION constant, 1 hardcoded Accept to
raw-only (unable to ever request JSON), and 2 reinvented a raw-toggle param a
4th way.

Add one exported githubHeaders(token, opts?: { raw?: boolean }) in a new
review-enrichment/src/github-headers.ts, using the most complete existing
shape (Accept toggle + GITHUB_API_VERSION + User-Agent) as the target, and
replace all 17 call sites with an import. Positional-boolean callers
(caller-impact.ts, unused-export.ts) move to the opts-object form; the
raw-only exhaustiveness-drift.ts call now explicitly passes { raw: true },
preserving its existing behavior while making the file capable of requesting
JSON like every other analyzer.

Entirely self-contained within review-enrichment (own tsconfig, own build,
own Railway deploy) -- no cross-service-boundary risk. Validated with
npm run rees:test (build, sourcemap check, analyzer-metadata drift check,
and the full node:test suite -- 1223 passing, 0 failing) plus a dedicated
test/github-headers.test.ts covering the new shared helper's default,
raw, and empty-opts branches.

Fixes #4609.
@JSONbored JSONbored self-assigned this Jul 10, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored
JSONbored merged commit 347ada8 into main Jul 10, 2026
8 checks passed
@JSONbored
JSONbored deleted the fix/rees-shared-github-headers-4609 branch July 10, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate review-enrichment's 17-file githubHeaders() duplication

1 participant