Skip to content

fix(review): replay linked issue one-shot blockers - #4732

Merged
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-linked-issue-vulnerability
Jul 11, 2026
Merged

fix(review): replay linked issue one-shot blockers#4732
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-linked-issue-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • One-shot review cadence could skip re-running linked-issue satisfaction checks by only testing for a cached row, which suppressed replaying prior linked_issue_scope_mismatch findings and allowed a previously-blocked PR to become unblocked on later automatic triggers.
  • The change must preserve the one-shot cost behavior (no fresh model call) while ensuring block-mode blockers are replayed into the advisory so gate evaluation remains correct.

Description

  • Add getLatestPublishedLinkedIssueSatisfaction to src/db/repositories.ts to fetch the latest stored linked-issue satisfaction row for a given (repo, pull, linked issue) regardless of head SHA or fingerprint.
  • In src/queue/processors.ts update the one-shot linked-issue skip path to call getLatestPublishedLinkedIssueSatisfaction and, when a prior ok result exists, reuse its verdict and push a linked_issue_scope_mismatch finding into advisory.findings when the prior verdict was unaddressed and the repo is in block mode.
  • Add unit coverage for the new cache helper in test/unit/linked-issue-satisfaction-cache.test.ts and a regression test in test/unit/queue.test.ts asserting a repeat one-shot trigger does not spend a fresh LLM call but still produces a gate failure via the cached linked_issue_scope_mismatch blocker.
  • Wire the new helper into the processors import list and keep existing audit-event behaviour for one-shot skips.

Testing

  • Ran the targeted Vitest suites: npm exec vitest run test/unit/linked-issue-satisfaction-cache.test.ts test/unit/queue.test.ts -- -t "linked-issue satisfaction cache|hasPublishedLinkedIssueSatisfaction|getLatestPublishedLinkedIssueSatisfaction|one-shot AI review cadence", and the tests passed.
  • Ran type checking with npm run typecheck, which completed successfully.
  • The change includes the new unit tests that exercise both the cache helper and the regression scenario and they pass under the local test run.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.22%. Comparing base (3ae7ece) to head (c3f9813).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4732   +/-   ##
=======================================
  Coverage   94.22%   94.22%           
=======================================
  Files         441      441           
  Lines       38741    38749    +8     
  Branches    14117    14120    +3     
=======================================
+ Hits        36504    36512    +8     
  Misses       1577     1577           
  Partials      660      660           
Files with missing lines Coverage Δ
src/db/repositories.ts 96.72% <100.00%> (+<0.01%) ⬆️
src/queue/processors.ts 95.71% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 00:27:06 UTC

4 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ 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 fix correctly closes a real gap: the one-shot review-cadence skip path only checked for the *existence* of a cached linked-issue-satisfaction row (hasPublishedLinkedIssueSatisfaction) and never replayed the stored verdict, so a PR previously blocked by a `linked_issue_scope_mismatch` finding could silently unblock on a later automatic re-evaluation. The new getLatestPublishedLinkedIssueSatisfaction (src/db/repositories.ts) and its wiring into the one-shot skip branch (src/queue/processors.ts) correctly reuse the frozen first-pass verdict without spending a fresh AI call, and only push the blocker finding when gate mode is block and the stored result is unaddressed. The regression test in test/unit/queue.test.ts exercises the real production path (seeds a cached unaddressed row, drives a real agent-regate-pr job, asserts zero AI calls and a failure gate outcome containing linked_issue_scope_mismatch), which is genuine coverage rather than a fabricated scenario.

Nits — 5 non-blocking
  • codecov/patch is 87.50% against a 99% target, and the visible gaps are real branches: no test for priorLinkedIssueSatisfaction being null/non-"ok" (the .catch(() => null) or non-ok status arm), no test for result.status === "addressed" in block mode (finding must not be pushed), and no test for gateMode !== "block" with an unaddressed prior verdict (verdict reused for display but no finding pushed) — all three arms are visibly branched on in src/queue/processors.ts and should be covered.
  • The ORDER BY created_at DESC, head_sha DESC tiebreak in getLatestPublishedLinkedIssueSatisfaction (src/db/repositories.ts) is arbitrary lexical ordering on ties; an autoincrement rowid/id DESC tiebreak would be less surprising if two rows share a created_at timestamp.
  • The PR description does not name or link an eligible open JSONbored/gittensory issue that this fix closes — worth confirming this is tied to a maintainer-authorized issue per repo convention before treating it as merge-ready.
  • Add the three missing-branch test cases called out above to close the codecov/patch gap.
  • Consider an explicit id/rowid tiebreaker in the ORDER BY clause in src/db/repositories.ts for determinism on same-timestamp rows.

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 ⚠️ Missing No linked issue or no-issue rationale found.
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: 48 registered-repo PR(s), 40 merged, 275 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 275 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 is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 48 PR(s), 275 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • 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

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 10, 2026
Closes the codecov/patch gap: a failed replay read (fail-safe fall
through), an "addressed" prior verdict in block mode (no blocker
pushed), and an unaddressed prior verdict outside block mode (reused
for display, no hard blocker).
@JSONbored
JSONbored force-pushed the codex/propose-fix-for-linked-issue-vulnerability branch from 24d51b6 to c3f9813 Compare July 11, 2026 00:10
@JSONbored
JSONbored merged commit d9b7be0 into main Jul 11, 2026
11 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-linked-issue-vulnerability branch July 11, 2026 00:29
JSONbored added a commit that referenced this pull request Jul 11, 2026
…es (#4814)

* test(unit): split queue.test.ts and backfill.test.ts into smaller files

queue.test.ts (33,461 lines / 810 tests) and backfill.test.ts (6,874 lines
/ 236 tests) each ran as one atomic unit under a single vitest worker --
queue.test.ts alone accounted for ~282s (roughly half the wall-clock) of
the full coverage run's ~565s duration, since vitest schedules whole test
files to workers and can't parallelize within one file.

Split each into several files along safe, verified boundaries (no shared
mutable state crosses a file boundary; hoisted helper functions used
across sections were promoted into each file's shared header). Test
count and pass/fail results are identical before and after (810 and 236
tests respectively, all passing) -- verified via `vitest run` on the full
old-vs-new file sets.

Also fixes two stale comments in src/queue/processors.ts that named the
old monolithic queue.test.ts file for a test that moved.

* fixup: port #4757's e2e-test-gen changes into split queue-5.test.ts

Rebasing onto main picked up #4757 (feat(review): decouple e2e-test-gen
auto-trigger and widen checkbox auth), which touched the monolithic
queue.test.ts before it was split. That content now lives in
queue-5.test.ts, so port the same test changes there: the new
autoTrigger opt-in on seedAutoTriggerPr, its three new/updated tests,
and the [BETA] badge text update. Verified against main's actual source
change (src/queue/processors.ts already carries the [BETA] label) and a
full run of all 6 split queue files: 813 tests passing (810 + 3 new).

* fixup: port #4732, #4659, #4816 test changes into split queue files

* fixup: port #4732 and #4816 test changes into split queue files

* fixup: port latest main's queue.test.ts changes into split queue-4.test.ts
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant