feat(miner-governor): dry-run-by-default enforcement + fail-closed chokepoint (#2342, #2340) - #5014
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | aa82231 | Commit Preview URL Branch Preview URL |
Jul 11 2026, 01:17 PM |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5014 +/- ##
==========================================
+ Coverage 94.18% 94.20% +0.01%
==========================================
Files 468 470 +2
Lines 39616 39693 +77
Branches 14456 14486 +30
==========================================
+ Hits 37314 37392 +78
+ Misses 1646 1645 -1
Partials 656 656
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - fixes requiredReview updated: 2026-07-11 13:20:36 UTC
🛑 Suggested Action - Manual Review Review summary Nits — 6 non-blocking
CI checks failing
Review context
Contributor next steps
Signal definitions
🟩 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.
|
b418a79 to
820d62c
Compare
85f1e72 to
84b7f83
Compare
8138bc9 to
b0b2d69
Compare
|
Pushed a coverage-only follow-up commit (00b20d6): measured coverage via `node --experimental-test-coverage` found `chokepoint.js` at 84.44% branch despite the fail-closed catch blocks already being exercised by the existing `null as unknown as X` tests -- those all throw a real `TypeError` (an `Error` instance), so `error instanceof Error ? error.message : String(error)` never took its `String(error)` arm. Added a `throwingProxy` helper that throws a plain string to exercise that arm for all five calculator stages, plus the `rateLimitRandomFn` conditional-spread branch and the self-plagiarism `similarity ?? null` fallback. `chokepoint.js` now measures 100.00% lines / 100.00% branch / 100.00% funcs. 409/409 tests pass. Test-only, no production code changes. |
00b20d6 to
23599a3
Compare
Resolves the miner's overall action mode with "safest wins" precedence mirroring resolveAgentActionMode (src/settings/agent-execution.ts): paused > dry_run > live. A freshly-configured miner (no opt-in anywhere) always defaults to dry_run, never live. Live mode requires an explicit, hard-to-fat-finger opt-in -- the exact string literal "live", never a boolean -- from either the operator's own global env var (GITTENSORY_MINER_LIVE_MODE) or the target repo's own .gittensory-miner.yml (MinerGoalSpec.execution.liveModeOptIn). The #2341 kill-switch always wins over any live-mode opt-in. Dry-run mode's would-be action is recorded to the governor ledger with a distinct dry_run decision, without ever executing. Stacked on #2341 (imports isMinerKillSwitchActive from kill-switch.ts).
Two branches were untested: MinerGoalSpec's execution.liveModeOptIn normalizer's "mapping present, key absent" path (distinct from the field being omitted entirely), and buildMinerDryRunGovernorLedgerEvent's repoFullName ?? null arm. Also narrows MinerExecutionPolicy.liveModeOptIn from `string | null` to the literal `typeof MINER_LIVE_MODE_OPT_IN | null`, so a typo in code constructing a MinerGoalSpec is caught at compile time rather than relying solely on the runtime exact-match check.
…lculator (#2340) (#5018) The single decision point every miner write action (open_pr, file_issue, apply_labels, post_eligibility_comment, create_branch, delete_branch, generate_tests) must pass through. Composes the kill-switch (#2341) and dry-run-default (#2342) primitives with the previously-shipped pure calculators -- rate-limit (#2344), budget/turn/termination caps, non-convergence detection, self-reputation throttle, and self-plagiarism -- into one precedence ladder: global kill-switch > per-repo pause > dry-run > rate-limit > budget cap > non-convergence > reputation throttle > self-plagiarism > allow. Reputation-throttle and self-plagiarism extend beyond the issue's three explicitly-named calculators, per those two modules' own doc comments forward-referencing this exact chokepoint; both reuse their own already-reviewed boolean gate semantic (throttled/allowed) rather than inventing new policy. Both apply only to actionClass "open_pr" (their own ledger builders are submission-scoped). Any calculator that throws denies immediately with stage "internal_error", never falls through to allow. Pure engine module (no IO); the miner-lib wrapper owns persisting the ledger event and advancing rate-limit bucket state only when the rate-limit stage actually ran. Stacked on #2341 + #2342 (imports from both).
…int tests Measured coverage showed chokepoint.js at 84.44% branch despite every stage's fail-closed catch block already being exercised -- every existing error test threw a genuine TypeError (a real Error instance) via a null-cast input, so error instanceof Error was always true and the String(error) fallback arm never ran. Add a throwingProxy helper that throws a plain string instead, exercising that arm for all five calculator stages. Also covers the rateLimitRandomFn conditional-spread branch and the self-plagiarism similarity ?? null fallback (via a whitespace-only fingerprint, which denies with no computed similarity). chokepoint.js now measures 100/100/100 (lines/branch/funcs), up from 84.44% branch. No production code changes -- test-only.
test/unit/miner-governor-chokepoint.test.ts is the root-vitest file Codecov actually measures (packages/gittensory-engine's own node:test suite is invisible to it), and it only covered allow/kill_switch/dry_run/rate_limit before this change -- budget_cap, non_convergence, reputation_throttle, and self_plagiarism denies, their omitted-stage skips, the non-open_pr actionClass skip, and every stage's fail-closed internal_error catch (including the error-instanceof-Error vs String(error) fallback arms) were all untested. Add 21 tests covering: each deny path, each stage running-but-not-tripping, both single-stage-omitted skips plus the combined non-open_pr skip, the rateLimitRandomFn conditional-spread, the self-plagiarism similarity ?? null fallback (whitespace-only fingerprint), and both throw shapes (null-deref TypeError and a non-Error thrown value via a throwing Proxy) for all five calculator call sites. chokepoint.ts now measures 100/100/100/100 (statements/branches/functions/ lines), up from 44.64% patch coverage.
23599a3 to
aa82231
Compare
Summary
packages/gittensory-engine/src/governor/action-mode.ts(maintainer: dry-run-by-default enforcement #2342): resolves the miner's overall action mode with "safest wins" precedence mirroringresolveAgentActionMode(src/settings/agent-execution.ts) —paused>dry_run>live. A freshly-configured miner always defaults todry_run, neverlive. Live mode requires the exact string literal"live"(never a boolean or truthy coercion) from either the operator's global env var (GITTENSORY_MINER_LIVE_MODE=live) or the target repo's own.gittensory-miner.yml(MinerGoalSpec.execution.liveModeOptIn: live). The maintainer: global + per-repo kill-switch #2341 kill-switch always wins over any live-mode opt-in.packages/gittensory-engine/src/governor/chokepoint.ts(maintainer: wire the fail-closed Governor chokepoint before every write action #2340): the single fail-closed decision point every miner write action must pass through, composing the kill-switch + this action-mode resolver + the already-shipped pure calculators (rate-limit, budget/turn/termination caps, non-convergence, self-reputation throttle, self-plagiarism) into one precedence ladder. Any calculator that throws denies immediately (stage: "internal_error") rather than falling through to allow — this fail-closed behavior is directly tested (5 dedicated tests forcing a real exception into each composable calculator).packages/gittensory-miner/lib/governor-*.js(+ hand-written.d.ts) wrapper. The chokepoint's wrapper persists the ledger event and advances rate-limit bucket state only when the rate-limit stage actually ran.MinerGoalSpec.execution.liveModeOptInfield, tolerantly parsed exactly likekillSwitch.paused— doc/schema/example surface kept in sync.Closes #2342, Closes #2340
Scope
CONTRIBUTING.md.Validation
git diff --checknpm run typecheck— clean.npm run build+npm run testinpackages/gittensory-engine— 402/402 passing.npm run buildinpackages/gittensory-miner(node --check, includinggovernor-action-mode.jsandgovernor-chokepoint.js) — clean.vitest runacross every touched/adjacent root-level suite — all passing.npm run test:coverage,test:workers,build:mcp,test:mcp-pack,ui:*,npm audit— not re-run locally; this PR touches onlypackages/gittensory-engine/**,packages/gittensory-miner/**, andtest/unit/**, and perCONTRIBUTING.mdonlysrc/**is Codecov-measured. CI runs them authoritatively.Safety
=== "live"), never truthy coercion — verified by dedicated tests.LocalWriteActionSpeccall site; that live actuation wiring is separate, later, maintainer-owned work (maintainer: wire the submission-gate trigger into the harness driving loop #2337).miner-goal-spec.md, JSON Schema,.gittensory-miner.yml.example); noCHANGELOG.mdchange.Notes
*LedgerEventbuilder keyed on its own boolean gate field — reusing an existing, already-reviewed semantic rather than inventing new policy. Both apply only toactionClass === "open_pr".nullthe same as an intentionally-omittedundefined— silently skipping the stage instead of failing closed. Two of the fail-closed tests caught this before the PR was finished; fixed by switching both guards to!== undefined.execution.liveModeOptInfield lets a target repo's own maintainer opt in to receiving live automated writes via a file they control, but that alone is deliberately insufficient — the miner operator must independently consent via their own global env var too. This reuses the kill-switch's "either side can only make it safer" pattern. Flagging in case OR-only semantics are preferred instead.