Skip to content

feat(miner): wire deny-hook synthesis to a live consumer and give it an operator CLI#8817

Merged
JSONbored merged 1 commit into
mainfrom
feat/wire-deny-hook-synthesis
Jul 26, 2026
Merged

feat(miner): wire deny-hook synthesis to a live consumer and give it an operator CLI#8817
JSONbored merged 1 commit into
mainfrom
feat/wire-deny-hook-synthesis

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

AMS audit fix (#8806): deny-hook synthesis stored and displayed maintainer-approved guardrails that no code path ever enforcedrefreshProposals had zero callers (no CLI either) and both driver-construction sites passed no houseRulesConfig, so resolveEffectiveRules was never consulted and every attempt ran on DEFAULT_DENY_RULES alone.

Changes

  • Enforce halfbuildAttemptDeps gains the target repoFullName (threaded from the attempt) and resolves the repo's effective rules into the coding-agent driver's PreToolUse hooks via a new exported resolveAttemptHouseRulesConfig (injectable store seam). Fail-open to undefined → the pre-fix(miner): deny-hook synthesis has zero live consumers — approved guardrail rules never reach a coding agent #8806 defaults on any store failure: a guardrail read hiccup must never block an attempt, and the defaults remain the floor.
  • Operate halfloopover-miner deny-hooks list | refresh --history <file.json> | approve | reject, strictly local/offline like purge. refresh deliberately requires an explicit history file ({ blockerCodes, changedPaths }[]): no local ledger records both fields today (prediction-ledger has codes, no paths) — the module header documents auto-sourcing as the tracked follow-up instead of inventing an implicit source that hides the gap. (Contrast the correctly-wired min-rank loop (Epic: AMS-side calibration loop over the miner's own event ledger (reusing the #8082 engine primitives) #8172), which this now matches in end-to-end operability.)

Test plan

  • npm run typecheck clean; @loopover/miner builds
  • END-TO-END test pinning the previously-severed loop: refreshapprove → the attempt-side resolver includes the approved rule (+1 over the defaults baseline)
  • Resolver tests: rules+repo returned with the store closed; all three fail-open arms (no repo, open-throw, resolve-throw-still-closes)
  • CLI tests: list (json + human + empty), reject-never-enforces, every usage-error exit, bad-history parse error
  • 157 tests green across the four miner suites; whole-new-file + changed-line coverage: 0 uncovered statements/branches

Closes #8806

@JSONbored JSONbored self-assigned this Jul 26, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 26, 2026
@loopover-orb

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-26 07:55:23 UTC

5 files · 1 AI reviewer · no blockers · CI pending · dirty

⏸️ Suggested Action - Manual Review

Review summary
This closes the exact gap #8806 flagged: refreshProposals had no caller and buildAttemptDeps never threaded houseRulesConfig into the driver, so approved deny-hook proposals were dead weight. The PR wires both halves — resolveAttemptHouseRulesConfig (attempt-cli.ts:277) resolves the repo's effective rules with a documented fail-open-to-undefined on any store error, and buildAttemptDeps now passes repoFullName through so the driver actually consults the synthesis store — plus a new `deny-hooks` operator CLI (list/refresh/approve/reject) mirroring the existing `purge` command's local-only convention. The end-to-end test (miner-deny-hooks-cli.test.ts) genuinely exercises refresh→approve→resolver-sees-the-rule, which is the real severed loop, not a fabricated scenario.

Nits — 5 non-blocking
  • deny-hook-synthesis.ts's setProposalStatus (unchanged by this diff) does a plain UPDATE with no existence/rows-affected check, so `deny-hooks approve <bad-id>` prints "Approved ... it enforces on the next attempt" even though nothing was updated — worth a follow-up now that this path is operator-facing.
  • deny-hooks-cli.ts's refresh case nests to depth 5 (subcommand switch → historyFlag lookup → historyPath undefined-check); consider extracting the --history parsing into a small helper for readability.
  • attempt-cli.ts imports `DenyRule` from `@​loopover/engine` (attempt-cli.ts:48) while deny-hook-synthesis.ts's store returns `DenyRule` from `./deny-hooks.js` — confirm these are the same type (typecheck reportedly passed, but worth a comment noting the two are meant to be identical) rather than two structurally-similar but separately-maintained types.
  • attempt-cli.ts is now ~510 lines, over this repo's apparent 400-line file-size guideline; not this diff's fault alone, but the new resolver function is a natural candidate to extract into its own module alongside deny-hooks-cli.ts.
  • Add a rows-affected check (or a listProposals-based existence check) to setProposalStatus so `deny-hooks approve/reject` can report a clear 'no such proposal' error instead of a false-success message.

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 #8806
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 13 registered-repo PR(s), 13 merged, 246 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 13 PR(s), 246 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The PR wires resolveEffectiveRules into buildAttemptDeps via a new resolveAttemptHouseRulesConfig (attempt-cli.ts), threads repoFullName from runAttempt, adds a deny-hooks CLI subcommand (list/refresh/approve/reject) wired into bin/loopover-miner.ts, and includes an end-to-end test showing refresh→approve→resolver produces an approved rule beyond the defaults baseline — matching every element the

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), 246 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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.

🟩 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

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
21298 3 21295 21
View the top 3 failed test(s) by shortest run time
test/unit/queue-2.test.ts > queue processors > #4603: the SAME sub-floor defect one-shot-closes when aiReviewLowConfidenceDisposition is explicitly one_shot
Stack Traces | 0.243s run time
AssertionError: expected 'error' to be 'completed' // Object.is equality

Expected: "completed"
Received: "error"

 ❯ test/unit/queue-2.test.ts:361:28
test/unit/queue.test.ts > queue processors > linked-issue hard-rule violation persistence (#linked-issue-hard-rule-persistence) > REGRESSION (live-issue-state-change-before-re-evaluation): Pass 1 flags a real violation; Pass 2's live re-parse re-evaluates the SAME issue as clean (assignee removed) but the persisted violation still closes the PR
Stack Traces | 0.275s run time
AssertionError: expected 'error' to be 'completed' // Object.is equality

Expected: "completed"
Received: "error"

 ❯ test/unit/queue.test.ts:2843:32
test/unit/queue.test.ts > queue processors > linked-issue hard-rule violation persistence (#linked-issue-hard-rule-persistence) > REGRESSION (body-edit-during-grace-window): Pass 1 flags a real owner-assigned violation; Pass 2's live re-parse finds NO linked issues (body edited) but the persisted violation still closes the PR
Stack Traces | 0.362s run time
AssertionError: expected 'error' to be 'completed' // Object.is equality

Expected: "completed"
Received: "error"

 ❯ test/unit/queue.test.ts:2798:32

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

…an operator CLI (#8806)

The synthesis store (#5667) was written and reviewable but NEVER read:
refreshProposals had no caller anywhere (not even a CLI), and both driver
construction sites passed no houseRulesConfig — so every attempt fell back
to DEFAULT_DENY_RULES and a maintainer-approved synthesized guardrail
never enforced anything.

- Enforce half: buildAttemptDeps threads the target repo and resolves its
  effective rules (approved proposals merged over defaults) into the
  driver's PreToolUse hooks via the new exported
  resolveAttemptHouseRulesConfig. FAIL-OPEN to the pre-#8806 defaults on
  any store failure — a guardrail read hiccup never blocks an attempt, and
  the defaults are the historical floor, never nothing.
- Operate half: `loopover-miner deny-hooks list|refresh|approve|reject`
  (strictly local + offline, mirroring `purge`). refresh takes an explicit
  --history <file.json> — deliberately not auto-sourced: no local ledger
  carries blockerCodes AND changedPaths together today (prediction-ledger
  lacks paths); the implicit-source gap is documented at the module header
  as the tracked follow-up rather than hidden behind an invented source.
- End-to-end test pins the previously-severed loop: refresh → approve →
  the attempt-side resolver includes the approved rule.
@JSONbored
JSONbored force-pushed the feat/wire-deny-hook-synthesis branch from 9375b80 to bf35c85 Compare July 26, 2026 08:03
@JSONbored
JSONbored merged commit 33bacda into main Jul 26, 2026
3 of 4 checks passed
@JSONbored
JSONbored deleted the feat/wire-deny-hook-synthesis branch July 26, 2026 08:10
@github-actions github-actions Bot mentioned this pull request Jul 26, 2026
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(miner): deny-hook synthesis has zero live consumers — approved guardrail rules never reach a coding agent

1 participant