Skip to content

feat(selfhost): per-repo opt-out for the active-review reconciliation sweep - #7880

Merged
JSONbored merged 1 commit into
mainfrom
orb-review-reconciliation-per-repo-opt-out
Jul 21, 2026
Merged

feat(selfhost): per-repo opt-out for the active-review reconciliation sweep#7880
JSONbored merged 1 commit into
mainfrom
orb-review-reconciliation-per-repo-opt-out

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to fix(selfhost): stop delayed webhooks from resurrecting closed PRs' review tracking #7839: adds review.activeReviewReconciliation: false as an explicit per-repo FORCE-OFF for the active-review-tracking reconciliation sweep, mirroring review.prReconciliation/review.sweepWatchdog exactly (same shape, same deliberately FORCE-OFF-ONLY semantics, same fail-open-on-manifest-error behavior). The sweep's fleet-wide enable already supported config-as-code (fix(selfhost): stop delayed webhooks from resurrecting closed PRs' review tracking #7839); this closes the other half — per-repo fine-tuning — which the initial cut missed.
  • Wires the opt-out into runActiveReviewReconciliation: a row's own repo manifest is checked before any GitHub API call is spent, so an opted-out repo's stale rows are skipped cheaply.
  • Fixes a pre-existing bug in packages/loopover-engine's own test script found while re-verifying this change: rm -rf dist-test deleted the compiled test output but left .test.tsbuildinfo behind, so a second invocation could see a stale incremental cache, skip re-emitting entirely, and node --test would report "0 tests found" as a trivial success — silently no-op-ing the engine package's whole 594-test suite while test:ci still reported green. Clearing both together (rm -rf dist-test .test.tsbuildinfo) forces a genuine recompile every run.
  • Fixes a flaky timestamp-inequality assertion in the out-of-order-webhook regression test added in fix(selfhost): stop delayed webhooks from resurrecting closed PRs' review tracking #7839 (two upserts can legitimately land in the same test millisecond) — matches the same caveat already documented on the sibling review-latency-metric tests in that file.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused on one concern (the per-repo opt-out + the two bugs found while verifying it) and does not mix in unrelated backend, UI, MCP, docs, dependency, or deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • Linked issue — none, same as fix(selfhost): stop delayed webhooks from resurrecting closed PRs' review tracking #7839: a direct maintainer follow-up correcting that PR's own config-as-code coverage, not a separate contributor-facing issue.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — full unsharded run, 19,865 tests, 0 failures
  • npm run test:workers
  • npm run build:mcp (build:mcp:check in the full gate)
  • 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 — 0 vulnerabilities
  • New/changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — 3 new tests for the per-repo opt-out (false excludes, true is a no-op, fails open on manifest-load error), plus engine-level parser/round-trip coverage for the new review.activeReviewReconciliation field

Ran the full npm run test:ci gate twice while iterating this PR (once surfaced the flaky timestamp assertion above, fixed and re-ran clean).

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. (N/A — no public-facing GitHub text changed.)
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth/session surface changed by this PR specifically; the sweep's own GitHub-token negative paths were already covered in fix(selfhost): stop delayed webhooks from resurrecting closed PRs' review tracking #7839.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API/OpenAPI/MCP surface changed.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
  • Visible UI changes include a UI Evidence section. (N/A — no UI changes.)
  • Public docs/changelogs are updated where needed. config/examples/loopover.full.yml and .loopover.yml.example document the new per-repo activeReviewReconciliation: false knob; CHANGELOG.md itself is intentionally untouched (not a release-prep PR).

UI Evidence

N/A — no UI, frontend, docs-site, or extension changes.

Notes

  • No schema/migration change in this PR (the review.activeReviewReconciliation field lives in the existing .loopover.yml manifest layer, not a DB column).
  • Once merged, I'll cut a new beta image and redeploy the self-hosted box, then update the box's private config: the global default and all repo-scoped manifests can now use both the fleet-wide toggle and, where desired, the per-repo override.

… sweep

Add review.activeReviewReconciliation: false as an explicit per-repo
FORCE-OFF, mirroring review.prReconciliation/review.sweepWatchdog exactly --
config-as-code for this class of fleet-wide sweep is meant to be both
globally toggleable AND fine-tunable per repo, which the initial cut missed.
Wire the opt-out into runActiveReviewReconciliation so a repo's stale rows
are skipped before any GitHub call is spent, failing open on a manifest-load
error like every sibling check.

Also fixes a pre-existing bug in the engine package's own test script found
while re-verifying this change: `rm -rf dist-test` deleted the compiled test
output but left `.test.tsbuildinfo` behind, so a second `npm run test`
invocation could see the stale incremental cache, skip re-emitting entirely,
and `node --test` would then report 0 tests found as a trivial success --
silently no-op-ing the engine's whole test suite while the gate still passed.
Clearing both together forces a real recompile every run.

And fixes a flaky timestamp-inequality assertion in the out-of-order-webhook
regression test added in the prior commit (two upserts can land in the same
test millisecond), matching the same caveat already documented on the
sibling review-latency-metric tests in this file.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored JSONbored self-assigned this Jul 21, 2026
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
loopover-ui c70e811 Commit Preview URL

Branch Preview URL
Jul 21 2026, 03:56 PM

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7880      +/-   ##
==========================================
- Coverage   91.40%   91.39%   -0.01%     
==========================================
  Files         730      730              
  Lines       74763    74767       +4     
  Branches    22815    22817       +2     
==========================================
  Hits        68335    68335              
  Misses       5385     5385              
- Partials     1043     1047       +4     
Flag Coverage Δ
shard-1 58.56% <100.00%> (-0.01%) ⬇️
shard-2 51.80% <40.00%> (-0.05%) ⬇️
shard-3 50.21% <40.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
packages/loopover-engine/src/focus-manifest.ts 98.44% <100.00%> (+<0.01%) ⬆️
src/review/active-review-reconciliation.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@JSONbored
JSONbored merged commit b76c91d into main Jul 21, 2026
15 checks passed
@JSONbored
JSONbored deleted the orb-review-reconciliation-per-repo-opt-out branch July 21, 2026 16:07
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.

1 participant