Skip to content

feat(miner-governor): late-binding freshness check before open_pr fires (#3007)#5112

Merged
JSONbored merged 2 commits into
mainfrom
feat/miner-submission-freshness-check-3007
Jul 11, 2026
Merged

feat(miner-governor): late-binding freshness check before open_pr fires (#3007)#5112
JSONbored merged 2 commits into
mainfrom
feat/miner-submission-freshness-check-3007

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Closes #3007.

Adds checkSubmissionFreshness (packages/gittensory-miner/lib/submission-freshness-check.js): a final, read-only check immediately before open_pr spec construction, complementing (not replacing) the existing claim-time check. A soft-claim made at the start of a long create/iterate loop can go stale — the issue may have been closed, fixed by another contributor, or the claim itself superseded/expired in the interim.

Note on this PR's history: this issue was previously closed by #3754, an unrelated PR (PHP gRPC stub classification) that cited this issue number to satisfy the linked-issue gate requirement without implementing the actual feature. Reopened and implemented for real here.

  • Checks the miner's own claim-ledger status first (local, free) before spending a network round-trip on the live issue/PR snapshot.
  • Staleness signals: claim_superseded (local claim row missing or not active), issue_closed, already_addressed (another author already has an open or merged PR referencing the issue — the miner's own referencing PRs don't count), live_state_unavailable (the injected fetch failed or threw — fails closed, never treated as "no evidence of staleness, so proceed").
  • Aborts before the Governor chokepoint would ever be invoked for a stale candidate, per the issue's own requirement.
  • Logs the abort reason to the event ledger — only on abort, not on every check, matching the issue's own "log the abort reason" wording (not a per-decision audit trail like the harness-submission-trigger's).
  • Standalone, composable unit: the freshness check, prepareOpenPrSubmission (maintainer: wire the submission-gate trigger into the harness driving loop #2337), and the Governor chokepoint are separate, sequentially-composed steps a future real call site (tracked separately) wires together — not nested calls.

Scope

Validation

  • npm run typecheck — clean.
  • node --check on the new file + full packages/gittensory-miner build script (56 files) — all pass.
  • npx vitest run test/unit/miner-submission-freshness-check.test.ts — 13/13 passing, covering every deliverable: fresh-claim-proceeds, closed-issue abort, duplicate-PR-exists abort (both open and merged, from another author), self-authored referencing PRs not counted, closed referencing PRs not counted, claim-superseded (missing row and non-active row, both without ever calling the live-state fetch), live-state-unavailable on both a null return and a thrown error, a defensively-missing referencingPrs key, and fail-closed validation on every malformed candidate/dependency field.
  • Scoped coverage: npx vitest run test/unit/miner-submission-freshness-check.test.ts --coverage --coverage.include="packages/gittensory-miner/lib/submission-freshness-check.js" → 100% stmts/branches/funcs/lines (39/39, 47/47, 4/4, 30/30).
  • npm audit --audit-level=moderate — no dependency changes.

Safety

  • No secrets/wallets/hotkeys/trust-scores exposed.
  • Read-only by contract — the only write this module performs is its own abort-reason audit event; the fresh path writes nothing.
  • Fail-closed on every malformed-input and live-state-fetch-failure path (tested).
  • Not an API/OpenAPI/MCP/UI surface change.

…es (#3007)

Adds checkSubmissionFreshness: a final, read-only check immediately
before open_pr spec construction, complementing the claim-time check.
Checks the miner's own claim-ledger status first (local, free) before
spending a network round-trip on the live issue/PR snapshot, aborting
before the Governor chokepoint is ever invoked for a stale candidate:

- claim_superseded: the local claim row is missing or not active
- issue_closed: the live issue snapshot shows it closed
- already_addressed: another author already has an open or merged PR
  referencing the issue (the miner's own referencing PRs don't count)
- live_state_unavailable: the injected fetch failed or threw -- fails
  closed, never treated as "no evidence of staleness, so proceed"

Logs the abort reason to the event ledger (not every check, matching
the issue's own "log the abort reason" wording); the fresh path never
writes. This is a standalone, composable unit -- the freshness check,
prepareOpenPrSubmission (#2337), and the Governor chokepoint are
separate, sequentially-composed steps a future real call site wires
together, not nested calls.

Closes #3007.
@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 b52a53a Commit Preview URL

Branch Preview URL
Jul 11 2026, 03:55 PM

@superagent-security

Copy link
Copy Markdown
Contributor

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

@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.21%. Comparing base (86370f2) to head (b52a53a).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5112   +/-   ##
=======================================
  Coverage   94.21%   94.21%           
=======================================
  Files         470      470           
  Lines       39743    39743           
  Branches    14506    14506           
=======================================
  Hits        37442    37442           
  Misses       1645     1645           
  Partials      656      656           
Flag Coverage Δ
shard-1 46.42% <ø> (-0.01%) ⬇️
shard-2 33.60% <ø> (-0.26%) ⬇️
shard-3 30.90% <ø> (-1.25%) ⬇️
shard-4 33.14% <ø> (+1.32%) ⬆️
shard-5 33.35% <ø> (-0.18%) ⬇️
shard-6 45.20% <ø> (+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.

@JSONbored JSONbored self-assigned this Jul 11, 2026
@gittensory-orb gittensory-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 11, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-11 15:57:36 UTC

4 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Adds a small, well-tested standalone freshness-check module (`checkSubmissionFreshness`) that gates a stale claim/issue/already-addressed candidate before `open_pr` construction, mirroring the fail-closed and validate-then-act conventions already used in `harness-submission-trigger.js`. The logic is correct: claim-ledger check runs first and short-circuits the network fetch, live-state fetch failures/nulls fail closed, closed issues abort, and the already-addressed check correctly excludes the miner's own referencing PRs while catching both merged and open PRs from other authors. As with the earlier `harness-submission-trigger.js`/`prepareOpenPrSubmission` PR, this ships the check as a standalone unit with no real call site wiring it before an actual `open_pr` invocation yet — the PR states that wiring is tracked separately, following established precedent in this package.

Blockers

  • packages/gittensory-miner/lib/submission-freshness-check.js:68 treats GitHub author logins as case-sensitive, so an own PR authored as `Miner-Bot` while `candidate.minerLogin` is `miner-bot` is incorrectly classified as `already_addressed`; normalize both sides before comparing, e.g. `const minerLoginKey = minerLogin.toLowerCase();` and `(pr) => typeof pr.authorLogin === "string" && pr.authorLogin.trim().toLowerCase() !== minerLoginKey && (pr.state === "merged" || pr.state === "open")`.
  • packages/gittensory-miner/lib/submission-freshness-check.js:38 adds the freshness check as an unused standalone helper, but no changed call site invokes it before `prepareOpenPrSubmission` or any `open_pr` handoff, so stale candidates can still reach the existing submission path exactly as before; wire `checkSubmissionFreshness` into the actual pre-submission sequence or narrow the PR/title to a non-shipping helper only.
Nits — 6 non-blocking
  • packages/gittensory-miner/lib/submission-freshness-check.js:38 — `checkSubmissionFreshness` is flagged at ~23 cyclomatic complexity / depth 5 by static analysis; most of this is validation boilerplate consistent with sibling files in this package, but consider extracting the four `deps`/candidate validation blocks into a small shared helper if this pattern keeps repeating across new lib files.
  • No real call site invokes `checkSubmissionFreshness` yet (same deferred-wiring pattern as `harness-submission-trigger.js`'s `prepareOpenPrSubmission`) — worth confirming the tracked follow-up issue for wiring actually exists so this doesn't become an orphaned unused export.
  • test/unit/miner-submission-freshness-check.test.ts has no test for `eventLedger.appendEvent` itself throwing during an abort path — minor, since nothing in the function currently catches it, but worth a comment noting that's intentional (propagate) if that's the desired behavior.
  • Consider a short JSDoc note on why `already_addressed` checks `pr.state === "merged" || pr.state === "open"` but not `"closed"` inline in the code itself (the tests document it, but a one-line code comment would save a future reader the trip to the test file).
  • If a follow-up PR wires this into the real submission path, make sure the ordering (claim check → freshness check → `prepareOpenPrSubmission` → Governor) documented in the file header is enforced by an integration test, not just unit tests of each piece in isolation.
  • 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.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3007
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High 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: 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 ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Check active issues and PRs before submitting.
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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; 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

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@gittensory-orb

Copy link
Copy Markdown

An AI reviewer flagged a likely defect, but its confidence was below this repository's configured close-confidence floor, so this is held for a maintainer to confirm instead of closing automatically. Resolve the flagged defect (see the review notes), or ask a maintainer to override.

…itively

pr.authorLogin !== minerLogin was a strict comparison, so a miner's own
referencing PR authored as e.g. "Miner-Bot" while candidate.minerLogin
is configured as "miner-bot" was misclassified as already_addressed by
another author -- GitHub logins are case-insensitive for identity, but
different API responses can echo back different casing. Normalize both
sides before comparing, and guard against a non-string authorLogin
(ignored rather than crashing or false-flagging).

Found by AI review on the original PR.
@JSONbored

Copy link
Copy Markdown
Owner Author

Fixed the case-sensitivity bug (author-login comparison now normalizes both sides before comparing, plus a non-string guard) — pushed in b52a53a.

On the second blocker (no call site wires this in yet): that's now addressed by #5118, which is open and imports `checkSubmissionFreshness` directly as the first stage of the real `runMinerAttempt` pipeline (freshness → submission-gate → Governor chokepoint → open_pr construction/execution). #5118 depends on this PR's commit (currently carries it via cherry-pick until this merges), so the wiring genuinely exists — it's just in the next PR rather than this one, matching the same "standalone unit now, real call site next" sequencing this package's other PRs in this batch have used (#2336#2337, #2340's chokepoint→its own callers).

@JSONbored JSONbored merged commit 7d260f1 into main Jul 11, 2026
20 checks passed
@JSONbored JSONbored deleted the feat/miner-submission-freshness-check-3007 branch July 11, 2026 16:13
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. manual-review Gittensor contributor context

Projects

Development

Successfully merging this pull request may close these issues.

feat(miner): add a late-binding freshness check against live repo state before open_pr fires

1 participant