Skip to content

fix(orb): actuation atomicity, close-critical concurrency, and reward-label integrity#9235

Merged
JSONbored merged 6 commits into
mainfrom
fix/9158-9159-9160-9161-actuation-integrity
Jul 27, 2026
Merged

fix(orb): actuation atomicity, close-critical concurrency, and reward-label integrity#9235
JSONbored merged 6 commits into
mainfrom
fix/9158-9159-9160-9161-actuation-integrity

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Four related integrity gaps in the ORB review engine, bundled into one PR since they all touch the same actuation/label-correlation surface:

  • Closes orb(actuation): the label-vs-close correlation guard is batch-scoped — a rejected or breaker-downgraded close still brands the PR with its enforcement label #9158 — the #label-close-split-brain label-vs-close correlation guard only inspected the same-batch planned array, so a rejected or breaker-downgraded close could leave its paired enforcement label applied to a PR that stayed open. Two paths never carried the pair through:

    • downgradeCloseToHold (src/settings/agent-actions.ts) now also drops any label action whose closeKind matches a close it just downgraded (the breaker-downgrade half).
    • The approval-queue accept path (src/services/agent-approval-queue.ts) now re-derives pairing from the durable audit trail (the same agent.action.close record every execution entry point writes through via latestCompletedAuditEventDetail) before applying a close-coupled label, since the queue stages one action per row and an independently rejected close has nothing correlating it to the label in the same batch.
  • Closes orb(race): the contributor-cap mutex is released before the merge it guards, so the #7284 TOCTOU is still open — a legitimate PR is closed for a cap already relieved #9159 — the per-author contributor-cap mutex was released before the merge it guards, reopening the feat(miner): add tenant create/list/destroy control-plane admin CLI #7284 TOCTOU. src/services/agent-action-executor.ts now holds the lock across the actual merge mutation (a finally wraps performMutation, not just the re-check), matching the sibling cap-close path. The approval-queue accept path (src/services/agent-approval-queue.ts) now also builds its own contributorCapMergeRecheck using the same resolvePerRepoContributorCapMatch the live webhook path uses — previously an accepted staged merge skipped the pre-merge re-check entirely.

  • Closes orb(gate): overlap-preserving claim timestamps let an old PR backdate a claim on any issue and steal the duplicate-winner slot #9160linkedIssueClaimedAt was a single PR-level timestamp that blends every currently-linked issue into one value, so an old placeholder PR could add a valuable issue later and inherit the placeholder's backdated claim time, stealing the duplicate-cluster winner slot. Added a durable per-(repo, PR, issue) claim ledger (linked_issue_claims, migration 0191) written once, immutably, the first time each issue is observed linked to a PR. resolveScopedLinkedIssueClaimedAt (src/queue/duplicate-detection.ts) reads this ledger scoped to only the issue(s) actually contested with an open sibling, and every duplicate-winner call site (src/queue/processors.ts, src/rules/advisory.ts) now threads that scoped value through instead of the blended PR-level column.

  • Closes orb(rewards): a contributor who authored the linked issue inherits every label on it, bypassing the reward-label trust gate and its opt-in flag #9161 — a contributor who authored (or was assigned) the linked issue inherited every label on it unconditionally, including reward-semantic labels from an additive mapping, bypassing the trustMaintainerAuthoredIssueForReward opt-in and the maintainer-authorship check every other path already requires. src/review/linked-issue-label-propagation-fetch.ts now splits the unlock: author/assignee identity still unlocks a plain type label unconditionally, but a reward-semantic label (named by any mapping with removeOtherTypeLabels !== true) always falls through to the same opt-in + maintainer-check gate regardless of the author match.

Judgment calls

Test plan

  • npx tsc --noEmit -p tsconfig.json --incremental false — clean (after rebuilding @loopover/engine)
  • npm run db:migrations:check — 195 migrations OK, contiguous 0001..0191, next free 0192
  • npm run db:schema-drift:checksrc/db/schema.ts matches migrations/
  • Targeted test files green: agent-action-executor.test.ts, agent-approval-queue.test.ts, agent-actions.test.ts, linked-issue-label-propagation-fetch.test.ts, db-parsers.test.ts, duplicate-winner.test.ts, resolve-scoped-linked-issue-claimed-at.test.ts (new), rules.test.ts, reconcile-live-duplicate-siblings.test.ts, transient-locks.test.ts — 950 passed, 0 failed

@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 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
22782 4 22778 21
View the top 3 failed test(s) by shortest run time
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > runEngineParityMain returns 0 for the real monorepo workspace
Stack Traces | 0.0605s run time
AssertionError: expected 1 to be +0 // Object.is equality

- Expected
+ Received

- 0
+ 1

 ❯ test/unit/check-engine-parity-script.test.ts:798:48
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > engine version skew > uses default version readers against the real monorepo workspace
Stack Traces | 0.0943s run time
AssertionError: expected [ Array(1) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   "Gate-decision logic change in src/rules/advisory.ts requires either:
+   • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or
+   • a version bump in packages/loopover-engine/package.json (currently 3.15.3 vs base 3.15.3).",
+ ]

 ❯ test/unit/check-engine-parity-script.test.ts:785:31
test/unit/queue-5.test.ts > queue processors > type label decoupling (#label-decoupling) > REGRESSION (#4528, PR #4494 shape): keeps the propagated labels on the PR's own merge-closed webhook, instead of falling back to the title guess
Stack Traces | 0.24s run time
AssertionError: expected [ 'gittensor:feature' ] to deeply equal [ 'gittensor:feature', …(1) ]

- Expected
+ Received

  [
    "gittensor:feature",
-   "gittensor:priority",
  ]

 ❯ test/unit/queue-5.test.ts:6660:34
test/unit/check-engine-parity-script.test.ts > check-engine-parity script > prints a clean summary and exits 0 for the real repo state when run as a subprocess
Stack Traces | 0.824s run time
Error: Command failed: .../loopover/node_modules/.bin/tsx scripts/check-engine-parity.ts
Engine-parity check found 1 issue(s):
Gate-decision logic change in src/rules/advisory.ts requires either:
  • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or
  • a version bump in packages/loopover-engine/package.json (currently 3.15.3 vs base 3.15.3).

 ❯ test/unit/check-engine-parity-script.test.ts:803:20

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { status: 1, signal: null, output: [ null, '', 'Engine-parity check found 1 issue(s):\nGate-decision logic change in src/rules/advisory.ts requires either:\n  • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or\n  • a version bump in packages/loopover-engine/package.json (currently 3.15.3 vs base 3.15.3).\n' ], pid: 6983, stdout: '', stderr: 'Engine-parity check found 1 issue(s):\nGate-decision logic change in src/rules/advisory.ts requires either:\n  • a matching edit to the other twin (.../src/advisory/gate-advisory.ts), or\n  • a version bump in packages/loopover-engine/package.json (currently 3.15.3 vs base 3.15.3).\n' }

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@JSONbored
JSONbored force-pushed the fix/9158-9159-9160-9161-actuation-integrity branch from 1919f22 to d51f5ea Compare July 27, 2026 08:25
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Logic backtest

Replayed 0 historical case(s) for linked_issue_scope_mismatch through the base (75b5498) and head (e8639e6) versions of its detection logic (corpus checksum 4f53cda18c2b).

Backtest comparison: linked_issue_scope_mismatch

Verdict: unchanged — no comparable axis moved.

Advisory only — this check never blocks merge (#8105).

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 27, 2026
@JSONbored
JSONbored force-pushed the fix/9158-9159-9160-9161-actuation-integrity branch from d51f5ea to 5194abb Compare July 27, 2026 08:33
@loopover-orb

loopover-orb Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-27 08:56:08 UTC

18 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Manual Review

Review summary
The AI review returned non-blocking notes for this change but did not include a separate narrative summary. Review the nits below before deciding this PR.

Nits — 5 non-blocking
  • The failed validate/validate-tests/validate-code checks came back with no detail, and this branch is 1 commit behind the default branch — per the base-branch status, that's a plausible cause worth ruling out with a rebase before assuming a content defect.
  • src/services/agent-approval-queue.ts now imports resolvePerRepoContributorCapMatch from ../queue/processors — worth confirming this doesn't introduce a circular import given processors.ts is a 14k-line module that may itself depend on approval-queue machinery.
  • Non-inclusive terminology: `blacklist`/`blacklisted` appear in several new lines (agent-action-executor.ts, agent-approval-queue.ts, agent-actions.ts, and the new test) — consider `denylist`/`denylisted` per the repo's existing conventions.
  • src/db/repositories.ts:604/622 introduce a new magic number `200` for boundedString(repoFullName, 200) — matches existing convention elsewhere in the file but a named constant would self-document intent.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

CI checks failing

  • validate
  • validate-code
  • validate-tests

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9158, #9159, #9160, #9161
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 (4 linked issues).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 13 registered-repo PR(s), 13 merged, 331 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 13 PR(s), 331 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The PR directly implements both fixes the issue requested for #9158: downgradeCloseToHold now drops any label whose closeKind matches a downgraded close (breaker-downgrade half), and the approval-queue accept path re-derives close/label pairing from the durable audit trail before applying a coupled label, rejecting it if the paired close never completed (approval-queue-split half). It also adds an

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: Python, TypeScript, Ruby, Go, MDX, Shell, Solidity, JavaScript
  • Official Gittensor activity: 13 PR(s), 331 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
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.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: success
  • config: 03a7f8b529a9 · pack: oss-anti-slop
  • record: ab7d840fcab9 (schema v3, head 5194abb)

🟩 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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 27, 2026
…er PR

The blended pull_requests.linked_issue_claimed_at column preserves an old
timestamp whenever the new linked-issue set overlaps the old one, so a PR
that claimed a throwaway issue on day one and later adds a valuable issue
inherits the day-one timestamp for the new issue too -- letting an old
placeholder PR backdate a claim and steal the duplicate-cluster winner slot.

Add a durable per-(repo, PR, issue) claim ledger (linked_issue_claims,
migration 0191) written once, immutably, the first time each issue is
observed linked to a PR. The duplicate-winner election now reads this
ledger, scoped to only the issue(s) actually contested with an open
sibling, instead of the PR-level blended value.

Closes #9160
Wire the per-(PR, issue) claim ledger (previous commit) into every
duplicate-winner call site: resolveScopedLinkedIssueClaimedAt reads the
ledger's earliest claim among only the issue(s) actually contested with an
open sibling, and buildPullRequestAdvisory's election now compares against
that scoped value instead of the PR's blended linkedIssueClaimedAt column.
Falls back to the blended column when the caller doesn't resolve a scoped
value, so every unwired caller keeps prior behavior exactly.

Closes #9160
…ue authors

A contributor who authored (or was assigned) the linked issue previously
inherited every label on it unconditionally, including reward-semantic
labels from an additive mapping -- bypassing the trustMaintainerAuthoredIssueForReward
opt-in and the maintainer-authorship check every other path already
requires. Split the unlock: author/assignee identity still unlocks a plain
type label unconditionally, but a reward-semantic label (named by any
mapping with removeOtherTypeLabels !== true) now always falls through to
the same opt-in + maintainer-check gate regardless of the author match.

Closes #9161
…try points

The #label-close-split-brain guard only inspected the same-batch planned
array, so a rejected or breaker-downgraded close could leave its paired
enforcement label applied to a PR that stayed open. Two paths never carried
the pair through to the executor's own correlation check:

- downgradeCloseToHold now also drops any label whose closeKind matches a
  close it just downgraded (the breaker-downgrade half).
- The approval-queue accept path now re-derives pairing from the durable
  audit trail (the same agent.action.close record every execution entry
  point writes through) before applying a close-coupled label, since the
  queue stages one action per row and an independently rejected close has
  nothing correlating it to the label in the same batch.

Closes #9158
…t just the re-check

The per-author contributor-cap mutex was released before the merge it
guards, reopening the #7284 TOCTOU: a concurrent cap-close could claim the
just-released lock, live-verify the merging PR as still open, and
wrong-close it for a cap the merge was about to relieve milliseconds
later. The lock now spans the actual merge mutation via a finally that
wraps performMutation, matching the sibling cap-close path.

The approval-queue accept path also builds its own contributorCapMergeRecheck
now, using the same resolvePerRepoContributorCapMatch the live webhook path
uses -- previously an accepted staged merge skipped the pre-merge re-check
entirely.

Closes #9159
@JSONbored
JSONbored force-pushed the fix/9158-9159-9160-9161-actuation-integrity branch from 5194abb to e8639e6 Compare July 27, 2026 09:03
@JSONbored
JSONbored merged commit 2206537 into main Jul 27, 2026
5 of 6 checks passed
@JSONbored
JSONbored deleted the fix/9158-9159-9160-9161-actuation-integrity branch July 27, 2026 09:06
JSONbored added a commit that referenced this pull request Jul 27, 2026
#9235 and #9237 merged 36 seconds apart, each independently claiming
migration number 0191 (0191_linked_issue_claims.sql and
0191_retention_column_indexes.sql). Renumbers the latter (the one that
merged second) to the next free number, 0193.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment