feat(review): config-as-code manifest override for the loop-escalation sweep cron - #8059
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…n sweep cron (JSONbored#8018) loop-escalation-sweep was the only flag-gated cron job in job-dispatch's switch without a manifest-override re-check: six siblings (ops-alerts, sweep-liveness-watchdog, reconcile-open-prs, reconcile-active-review-tracking, generate-maintainer-recap, rag-index-repo) resolve a resolveXManifestOverride(env) and pass it into isXEnabled(env, override) so a stale in-flight job that lands after a .loopover.yml-based flag flip still no-ops -- but an operator disabling Rent-a-Loop escalation via .loopover.yml (rather than LOOPOVER_LOOP_ESCALATION) could not stop an already-enqueued sweep job. The capability was never built when JSONbored#6349 added the sweep. Mirror the siblings end to end: a top-level `loopEscalation:` manifest block ({present, enabled}, parse/serialize/allowlist shaped exactly like prReconciliation's), resolveLoopEscalationManifestOverride in loop-escalation-wire.ts (60s single-slot TTL cache + fail-safe degrade to present:false, mirroring pr-reconciliation.ts), isLoopEscalationSweepEnabled honoring the override (present wins outright, else env fallback), and the dispatch case resolving + passing it like its six siblings. Tests mirror each sibling's: the full parse/round-trip suite for the new manifest block, resolver present/absent/failure/TTL cases, override-precedence on isLoopEscalationSweepEnabled, and both dispatch directions (manifest disables despite env ON; manifest enables despite env OFF).
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-22 17:45:23 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8059 +/- ##
==========================================
+ Coverage 91.93% 93.04% +1.10%
==========================================
Files 742 648 -94
Lines 76071 54219 -21852
Branches 23093 19337 -3756
==========================================
- Hits 69939 50448 -19491
+ Misses 5037 2850 -2187
+ Partials 1095 921 -174
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Summary
loop-escalation-sweepwas the only flag-gated cron job inprocessJob's switch without a config-as-code manifest-override re-check: its six siblings resolve aresolveXManifestOverride(env)and pass it intoisXEnabled(env, override)so a stale in-flight job that lands after a.loopover.yml-based flag flip still no-ops — but an operator disabling Rent-a-Loop escalation via.loopover.yml(rather thanLOOPOVER_LOOP_ESCALATION) could not stop an already-enqueued sweep job. The capability was never built when Wire a live, scheduled trigger for evaluateEscalation so an escalation reaches a human automatically #6349 added the sweep.loopEscalation:manifest block ({present, enabled}, parse/serialize/known-field allowlist shaped exactly likeprReconciliation's, wired through the engine parser, validation normalizer, loader snapshot, and the empty-manifest/presentcomputation),resolveLoopEscalationManifestOverrideinloop-escalation-wire.ts(60s single-slot TTL cache + fail-safe degrade topresent: false, mirroringpr-reconciliation.ts),isLoopEscalationSweepEnabled(env, override)honoring the override (a present block wins outright, else env fallback), and theloop-escalation-sweepdispatch case resolving + passing it like its six siblings.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 actionlintnpm 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.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=moderateIf any required check was skipped, explain why:
focus-manifest(full parse/round-trip suite for the new block, mirroringactiveReviewReconciliation's),focus-manifest-validation,selfhost-config-lint,loop-escalation-wire(resolver present/absent/failure/TTL cases + override-precedence), andqueue-5(both dispatch directions: manifest disables despite env ON; manifest enables despite env OFF) — 1,046 tests green — plusnpm run build --workspace @loopover/engineand the full rootnpm run typecheck. Verified per-diff-line coverage via lcov intersection: every changed line and branch across all 6 instrumented source files is covered, and simulated the scoped-CI shard condition with the exact CI invocation (--changed=origin/main --coverage.all=false): the per-shardlcov.infois non-empty and contains exactly the changed instrumented files.actionlint/workers/mcp/ui checks are untouched surfaces; CI runs them all.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/engine-only change (no UI, docs, or extension surface touched).
Notes
.loopover.yml/ bundled fallback manifest are intentionally untouched: an absentloopEscalation:block parses to{present: false}and falls through to the env var, byte-identical to today — the manifest-drift check stays green and no operator behavior changes until a block is actually configured.pickIssueTimestamp-style duplication was avoided entirely: the parser, serializer, and known-top-level-field allowlist additions are single-definition mirrors of theprReconciliationblock, and the wire resolver mirrorsresolvePrReconciliationManifestOverrideincluding its test-only cache-clear hook (wired intoqueue-5's sharedbeforeEachalongsideclearOpsManifestOverrideCacheForTest).