Skip to content

fix(miner-governor): wire buildHouseRulesPreToolUseHook into a real driver-construction call site#5082

Merged
JSONbored merged 2 commits into
mainfrom
fix/wire-pretooluse-hook-2343-followup
Jul 11, 2026
Merged

fix(miner-governor): wire buildHouseRulesPreToolUseHook into a real driver-construction call site#5082
JSONbored merged 2 commits into
mainfrom
fix/wire-pretooluse-hook-2343-followup

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Follow-up to #2343 (merged as #5031). That PR built buildHouseRulesPreToolUseHook (the live Agent SDK PreToolUse interception point) but nothing ever called it — confirmed via grep that createCodingAgentDriver/runCodingAgentAttempt (driver-factory.ts) had zero real callers anywhere in packages/gittensory-miner. The engine package cannot import the miner package's hook builder directly (dependency only flows miner → engine, never the reverse), so the wiring has to live miner-side.

  • buildHouseRulesAgentSdkHooks wraps the hook callback into the Agent SDK's documented hooks.PreToolUse registration shape (an array of matcher groups, each holding an array of callbacks).
  • runHouseRulesEnforcedCodingAgentAttempt is a drop-in replacement for the engine's runCodingAgentAttempt that defaults hooks to house-rule enforcement for the agent-sdk provider whenever the caller omits it — an explicit hooks override still wins.

This does not build a CLI entrypoint — nothing constructs a coding-agent driver in production yet, which is separate, larger follow-up work. What this guarantees: whenever such an entrypoint exists, it only has to call runHouseRulesEnforcedCodingAgentAttempt instead of the engine's own runCodingAgentAttempt to get real, unbypassable house-rule enforcement automatically, rather than depending on that future author to remember to wire hooks by hand.

Verification

Tested end-to-end, not just the shape: the defaulted hooks are captured at the exact point they'd reach the SDK's query() call (same query-capturing test seam agent-sdk-driver.test.ts already uses), and the resulting callback is invoked directly to confirm it genuinely denies a house-rule violation (.env read) and allows a clean one — proving the wiring reaches a real, enforcing hook, not a placeholder.

Test plan

  • npm run typecheck clean
  • packages/gittensory-miner's build script (node --check) passes on the new file
  • Engine package rebuilt fresh; its full node:test suite passes (505 tests)
  • New root vitest suite (8 tests): hook-shape wrapping, real enforcement through the wrapped callback, config/options threading, hooks defaulting when omitted, caller-supplied hooks NOT overridden, inert for non-agent-sdk providers, paused/dry-run never constructs a driver so hooks are never even consulted
  • Measured coverage on the new file directly: 100% statements/branches/functions/lines

Advances #2343 (does not close it — the "wire a real CLI entrypoint that actually invokes a coding-agent attempt in production" work is still open, tracked separately)

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 11, 2026

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
gittensory-ui 768913e Commit Preview URL

Branch Preview URL
Jul 11 2026, 02:06 PM

@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 11, 2026
@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 11, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 14:33:35 UTC

4 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
Adds a miner-side wrapper module that composes the already-merged buildHouseRulesPreToolUseHook into the SDK's PreToolUse registration shape and defaults it into runCodingAgentAttempt for the agent-sdk provider, plus a matching .d.ts and a test file that exercises the wrapping, defaulting, override, and inert-provider behavior against the real (non-mocked) deny-hook logic. The wiring is correct and narrowly scoped: hooks default only when the caller omits them, non-agent-sdk providers pass hooks through inertly, and the tests genuinely invoke the callback to prove a `.env` read is denied rather than just asserting shape. The PR is explicit that no production call site is created yet, so real-world enforcement still depends on a future entrypoint calling this function — the description itself flags this as intentional scope-limiting rather than a gap being hidden.

Nits — 5 non-blocking
  • packages/gittensory-miner/lib/coding-agent-house-rules.js:36 embeds the issue number `2343` as a bare literal in the JSDoc rather than a named constant, per the external brief — purely cosmetic, not worth blocking on.
  • The module doc comments are unusually long and repeat the PR description almost verbatim (coding-agent-house-rules.js:1-14); consider trimming to what a future reader needs at the call site rather than re-narrating the PR history.
  • CI shows `validate` and `validate-code` as FAILED on this commit — worth confirming those failures are unrelated lint/format issues rather than something in this new file before merge, though that's a CI-status note, not a code defect I can independently verify from the diff.
  • Consider adding a one-line CHANGELOG or README pointer in coding-agent-driver.md noting `runHouseRulesEnforcedCodingAgentAttempt` exists as the house-rules-enforcing entrypoint, so a future author building the real CLI call site discovers it instead of reaching for the engine's raw `runCodingAgentAttempt`.
  • Since this PR explicitly defers building any real call site, consider linking the follow-up issue that will wire an actual entrypoint to this function, so the dangling guarantee doesn't go unnoticed.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 46 registered-repo PR(s), 38 merged, 416 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 46 PR(s), 416 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
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: not available
  • Official Gittensor activity: 46 PR(s), 416 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

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

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.20%. Comparing base (0800f23) to head (2561b4d).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5082   +/-   ##
=======================================
  Coverage   94.20%   94.20%           
=======================================
  Files         470      470           
  Lines       39722    39722           
  Branches    14493    14493           
=======================================
  Hits        37421    37421           
  Misses       1645     1645           
  Partials      656      656           
Flag Coverage Δ
shard-1 46.40% <ø> (-0.02%) ⬇️
shard-2 33.47% <ø> (-0.12%) ⬇️
shard-3 31.16% <ø> (-0.18%) ⬇️
shard-4 33.12% <ø> (+0.31%) ⬆️
shard-5 33.36% <ø> (-0.18%) ⬇️
shard-6 45.21% <ø> (+0.30%) ⬆️

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
…river-construction call site (#2343 follow-up)

The merged #2343 PR built buildHouseRulesPreToolUseHook (pretooluse-hook.js) but
nothing ever called it -- confirmed via grep that createCodingAgentDriver/
runCodingAgentAttempt (driver-factory.ts) had zero real callers anywhere in
packages/gittensory-miner. The engine package cannot import the miner
package's hook builder directly (dependency only flows miner -> engine), so
the wiring has to live miner-side.

Adds coding-agent-house-rules.js: buildHouseRulesAgentSdkHooks wraps the hook
callback into the Agent SDK's documented hooks.PreToolUse registration shape,
and runHouseRulesEnforcedCodingAgentAttempt is a drop-in replacement for
runCodingAgentAttempt that defaults hooks to house-rule enforcement for the
agent-sdk provider whenever the caller omits it -- an explicit hooks override
still wins. This doesn't build a CLI entrypoint (nothing constructs a coding-
agent driver in production yet, a separate and larger piece of work); it
guarantees that whenever such an entrypoint calls this function instead of the
engine's own runCodingAgentAttempt, house-rule enforcement is on by default
rather than something that has to be remembered.

Verified end-to-end: the defaulted hooks reach the SDK's query() call
(captured via the same query-capturing test seam agent-sdk-driver.test.ts
uses) and the resulting callback genuinely denies a house-rule violation, not
just a matching shape.
…age specifier

test/unit/miner-coding-agent-house-rules.test.ts imported AgentSdkQueryFn/
CodingAgentDriverTask via the "@jsonbored/gittensory-engine" package
specifier, which resolves through the workspace package's dist/ (git-
ignored). CI's Typecheck step runs before the "Build engine package"
step (#ci-engine-build-order fixed this ordering for the coverage step
but not typecheck), so dist/ does not exist yet when tsc runs, failing
TS2307 in CI while passing locally against a leftover dist/. Import the
types from the same source path the file's own vi.mock already redirects
the runtime import to -- needs no build step, and keeps both resolutions
consistent.
@JSONbored JSONbored merged commit e04e847 into main Jul 11, 2026
19 checks passed
@JSONbored JSONbored deleted the fix/wire-pretooluse-hook-2343-followup branch July 11, 2026 14:33
JSONbored added a commit that referenced this pull request Jul 11, 2026
…overage (#5093)

validate-code's "Build engine package" step ran after Typecheck, so
packages/gittensory-engine/dist/ (git-ignored, only materializes once
that step runs) didn't exist yet when tsc ran. This was latent until a
root test file added the first real (non-vi.mock) `import type` from
the "@jsonbored/gittensory-engine" package specifier -- needing its
dist/-backed types, not just its vi.mock-redirected runtime behavior --
which failed TS2307 in CI while passing locally against a stale
leftover dist/ (PR #5082). The exact same ordering class was already
fixed once for "Test with coverage" (#ci-engine-build-order); this
moves the build step ahead of both consumers instead of just one.

Verified by removing packages/gittensory-engine/dist locally, running
the build step, then typecheck in that order -- passes clean, matching
the new CI sequence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

Development

Successfully merging this pull request may close these issues.

maintainer: PreToolUse-hook-enforced house rules (deny even under bypassPermissions)

1 participant