Skip to content

drift: the #9399 sibling-guard pattern is live in four more places — fix the four, then remove the structure that generates them #9491

Description

@JSONbored

Summary

The audit found the same guard-drift pattern that #9399 fixed, still live in four more places. In each case a protection was added to the N instances someone grepped for, and the N+1th sibling — usually in the same file — was missed. This is a recurring class, not four coincidences, so the deliverables include a structural check as well as the four fixes.

The four live instances

1. Linked-issue satisfaction advisory has no commit-threshold cap (highest cost)

src/queue/processors.ts:8449's early-return set lacks commitThresholdReached. Both siblings have it — src/queue/slop-detection.ts:60,66 (via d66d17cea) and the ai_review path via focus-manifest.ts:205. Result: unbounded per-push LLM spend on long-lived PRs, at exactly the point where the other two advisory runners stop.

  • Add the cap; assert all three advisory runners share one precondition set.

2. #9398's webhook-redelivery guard missed the panel-checkbox twins

#9398/#9399 established (same day) that the checkbox and text-command siblings must share redelivery guards. These four still lack it:

  • maybeProcessPrPanelGenerateTests (processors.ts:14211) — the text twin has it at :13504-13505
  • maybeProcessPrPanelRetrigger (:13995)
  • maybeProcessGateOverrideCommand (:12906)
  • maybeProcessResolveCommand (:13171)

A redelivered checkbox payload therefore re-pushes a second E2E test commit, re-runs a paid review, or re-writes an overridden check-run.

  • Add delivery-id dedup to all four.

3. resolveCopycatDuplicateSibling open-state pre-filter present in 1 of 3 call sites

src/queue/duplicate-detection.ts:195 has it; duplicate-detection.ts:97 and src/rules/advisory.ts:1092 do not, and the callee documents the invariant as caller-owned (src/signals/copycat-duplicate.ts:32). advisory.ts has no downstream re-check, so a stale-cached-open, actually-merged sibling can mint a corroborated duplicate_pr_risk blocker.

  • Apply the filter at all three call sites, or move it into the callee so it cannot be forgotten.

4. ai-slop-cache-input.ts fingerprint omits its own routing salt

src/queue/slop-detection.ts:137 swaps env.AI → env.AI_ADVISORY, but the fingerprint (src/review/ai-slop-cache-input.ts:20-29) carries only BYOK identity — null on the shared path. Flipping advisory_ai_routing.slop therefore replays the other model's cached advisory at an unchanged head. The rule it violates is written down in its own sibling (src/review/ai-review-cache-input.ts:65-69).

Two more, lower severity

The structural fix (the actual point of this issue)

Fixing four siblings without changing the shape guarantees a fifth. Deliverables:

  • Unify the command/panel handlers. There are ~9 slash-command and panel-checkbox handlers in processors.ts differing mainly in payload extraction. Put them behind one dispatch that owns state checks, redelivery dedup and rate limiting once. This structurally retires instance 2 and the closed-PR item.
  • Unify the advisory spend gates. The three run*ForAdvisory runners should share one precondition struct (commit threshold, pause/freeze, mode, budget). Retires instance 1.
  • Add a sibling-drift check. scripts/check-import-specifiers.ts is a working template. A check that flags "N call sites of X where M < N apply guard Y" is hard in general, but the specific high-value case — every handler in the command dispatch table declaring its guard set explicitly — is cheap once the dispatch exists.

Expected outcome

The four live drifts are closed, and the two structures that generate them (a 16k-line file of near-identical handlers, and three parallel advisory runners) stop being able to.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions