Skip to content

test(review): cover the markAiReviewPublished write-failure fail-open path#3477

Merged
JSONbored merged 2 commits into
mainfrom
fix/ai-review-published-mark-failure-coverage
Jul 5, 2026
Merged

test(review): cover the markAiReviewPublished write-failure fail-open path#3477
JSONbored merged 2 commits into
mainfrom
fix/ai-review-published-mark-failure-coverage

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Follow-up to #3461. That PR added markAiReviewPublished (src/db/repositories.ts) alongside the pre-existing markPullRequestSurfacePublished stamp in the finalize step of maybePublishPrPublicSurface, but only carried a regression test for the sibling call's write-failure path — codecov/patch flagged the new call's own .catch()/console.error branch as uncovered after merge (96.77% vs the 99% target, 1 missing line). This adds the matching test, mirroring the existing "#4 over-publish dedup" test for markPullRequestSurfacePublished.

Test-only change; no production code touched.

No linked issue — direct coverage follow-up to #3461's post-merge Codecov finding.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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 an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage — confirmed the previously-uncovered line (src/queue/processors.ts's markAiReviewPublished catch handler) is now exercised; full unsharded suite green
  • npm run actionlint / test:workers / build:mcp / test:mcp-pack / ui:* (not re-run — test-only change under test/unit/, none of these are affected)
  • npm audit --audit-level=moderate — 0 vulnerabilities (no dependency changes)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — this PR is the missing fallback-path test

If any required check was skipped, explain why:

  • Test-only diff under test/unit/queue.test.ts; no source, workflow, MCP, or UI files touched.

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. (N/A)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — test-only.)
  • UI changes use live API data or real empty/error/loading states. (N/A)
  • Visible UI changes include a UI Evidence section. (N/A — no UI change.)
  • Public docs/changelogs are updated where needed. (N/A)

… path

#3461 added markAiReviewPublished alongside the existing
markPullRequestSurfacePublished stamp but only tested the latter's
failure path, leaving the new call's own catch/console.error branch
without a regression test (codecov/patch flagged it after merge).
Mirrors the existing "over-publish dedup" test for the sibling stamp.
@superagent-security

Copy link
Copy Markdown

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

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 5, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 09:03:13 UTC

1 file · 1 AI reviewer · 2 blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This is a narrow test-only follow-up that exercises the existing fail-open path when `markAiReviewPublished` rejects during public-surface publishing. The test drives `processJob` through the webhook path, stubs the GitHub calls needed for a successful comment publish, and asserts the publish still completes while the stamp write was attempted. I do not see a production correctness issue introduced by this diff.

Nits — 4 non-blocking
  • nit: `test/unit/queue.test.ts:2954` only asserts `markSpy` was called, so the test would still pass if the stamp were invoked with the wrong repository/PR/cache identity; assert the expected arguments to keep this regression tied to the real write path.
  • nit: `test/unit/queue.test.ts:2955` uses a broad fetch fallback of `Response.json({})`, which can hide an unexpected extra GitHub request in this scenario; prefer throwing on unmatched URLs after the known stubs.
  • At `test/unit/queue.test.ts:2954`, tighten the spy assertion to `toHaveBeenCalledWith(...)` using the expected repo/PR identifiers for this fixture.
  • At `test/unit/queue.test.ts:2967`, replace the default `return Response.json({})` with an explicit failure so future wiring changes do not silently pass through this test.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 586 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 586 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 56 PR(s), 586 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.03%. Comparing base (a979552) to head (85a201c).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3477   +/-   ##
=======================================
  Coverage   93.03%   93.03%           
=======================================
  Files         298      298           
  Lines       31203    31203           
  Branches    11386    11386           
=======================================
+ Hits        29029    29030    +1     
+ Misses       1518     1517    -1     
  Partials      656      656           

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 5, 2026
…sion

Addresses the gate review's nit on #3477: the markAiReviewPublished
spy assertion only checked that it was called, not with which
repo/PR/head, so a wrong-identity call would have passed silently.
@JSONbored JSONbored merged commit b5d2956 into main Jul 5, 2026
10 checks passed
@JSONbored JSONbored deleted the fix/ai-review-published-mark-failure-coverage branch July 5, 2026 09:06
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.5x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant