fix(miner-governor): wire buildHouseRulesPreToolUseHook into a real driver-construction call site - #5082
Conversation
Deploying with
|
| 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 didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-11 14:33:35 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 5 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk 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.
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.
|
4596ea4 to
28e06a1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…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.
28e06a1 to
768913e
Compare
…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.
…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.
…kage surface
Investigated swapping ci.yml's root "Typecheck" step to Turborepo (the
last remaining plain-npm build/lint/typecheck step) and found a latent
correctness bug in turbo.json's //#typecheck task instead: root
tsconfig.json's "include" (src, test) only seeds tsc's starting file
set, but test/** reaches via relative imports into six other workspace
packages/apps that //#typecheck's inputs never accounted for
(packages/loopover-miner/{lib,bin}, packages/loopover-mcp/package.json,
packages/discovery-index/src, apps/loopover-ui/src/lib,
apps/loopover-extension, apps/loopover-miner-extension). A stale cache
hit here would mask a real type error -- the exact failure mode PR
JSONbored#5082 already burned this repo on once.
The bug is currently inert: ci.yml's "Typecheck" step still calls plain
`npm run typecheck`, so nothing consults this task's cache yet. Fixed
anyway since the gap would otherwise resurface for whoever next tries
this swap without redoing the same audit.
Deliberately did NOT swap the ci.yml step itself. tsc's own
--incremental engine already tracks this exact transitive graph
precisely and with zero drift risk, and already delivers the real
measured speedup (~13.5s cold vs ~3.6s warm) -- wrapping it in
Turborepo would layer a cruder, hand-maintained approximation on top of
a mechanism that's already strictly more correct for this one task.
Documented the decision in both files so it isn't re-litigated blind.
…two runtime assumptions into tsc (#9853) turbo.json's //#typecheck `inputs` list is a hand-maintained approximation of tsc's real transitive surface. Its own comment says so -- "a snapshot ... not a structural guarantee" -- and asks a human to "re-run the same grep ... before trusting this list again". Nobody re-runs a grep on request, and it HAD drifted. Four paths that src/+test/ statically import were unhashed: packages/loopover-mcp/lib/** (mcp-cli-error, mcp-gateway tests) packages/loopover-mcp/bin/** (mcp-cli-completion-spec imports CLI_COMMAND_SPEC) packages/loopover-miner/scripts/** (miner-benchmark, miner-env-reference tests) packages/discovery-index/scripts/** (validate-posthog-release test) Editing any of them could leave a stale cache HIT on a typecheck a real `tsc --noEmit` would fail -- the #5082 class this list exists to prevent. The bin/** case also falsified the comment's own stated reason for excluding it ("never a static import"), which is now corrected rather than left misleading. Add scripts/check-turbo-typecheck-inputs.ts: compute the reach every CI run instead of remembering it. A path is covered by a glob OR by a dependsOn build (turbo hashes a dependency's own inputs), so engine/contract sources are not false-positived. Paths that do not exist on disk are ignored -- the checker-test fixtures embed import statements inside STRINGS naming a package renamed away, which a naive scan reports as real reach. Also enable isolatedModules + verbatimModuleSyntax: both cost ZERO errors today (measured) and turn assumptions the runtime already makes into compile-time guarantees -- per-file transpilability for `--experimental-strip-types` and esbuild, and type-only imports that can never be silently elided from (or dragged into) the self-host bundle. Deliberately NOT enabling noImplicitReturns: 3 errors, and two are documented, intentional implicit-undefined switch arms. Churn for marginal value.
Summary
Follow-up to #2343 (merged as #5031). That PR built
buildHouseRulesPreToolUseHook(the live Agent SDKPreToolUseinterception point) but nothing ever called it — confirmed via grep thatcreateCodingAgentDriver/runCodingAgentAttempt(driver-factory.ts) had zero real callers anywhere inpackages/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.buildHouseRulesAgentSdkHookswraps the hook callback into the Agent SDK's documentedhooks.PreToolUseregistration shape (an array of matcher groups, each holding an array of callbacks).runHouseRulesEnforcedCodingAgentAttemptis a drop-in replacement for the engine'srunCodingAgentAttemptthat defaultshooksto house-rule enforcement for theagent-sdkprovider whenever the caller omits it — an explicithooksoverride 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
runHouseRulesEnforcedCodingAgentAttemptinstead of the engine's ownrunCodingAgentAttemptto 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
hooksare captured at the exact point they'd reach the SDK'squery()call (same query-capturing test seamagent-sdk-driver.test.tsalready uses), and the resulting callback is invoked directly to confirm it genuinely denies a house-rule violation (.envread) and allows a clean one — proving the wiring reaches a real, enforcing hook, not a placeholder.Test plan
npm run typecheckcleanpackages/gittensory-miner's build script (node --check) passes on the new filenode:testsuite passes (505 tests)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)