Skip to content

refactor(queue): extract plan-input builder from runAgentMaintenancePlanAndExecute - #4688

Merged
JSONbored merged 1 commit into
mainfrom
refactor/processors-runagentmaintenanceplan-4607
Jul 10, 2026
Merged

refactor(queue): extract plan-input builder from runAgentMaintenancePlanAndExecute#4688
JSONbored merged 1 commit into
mainfrom
refactor/processors-runagentmaintenanceplan-4607

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Part of Break up processors.ts mega-functions #4607 (the processors.ts mega-function breakup). This PR does only the
    runAgentMaintenancePlanAndExecute slice of that issue — maybePublishPrPublicSurface and
    processGitHubWebhook are separate, much larger follow-ups, not touched here.
  • The issue's own finding for this function: it "threads ~30 locally-computed values into one giant
    planAgentMaintenanceActions(...) call." This PR extracts that ~79-line inline object literal into a
    new, named, independently-typed buildAgentMaintenancePlanInput helper (colocated in
    src/queue/processors.ts, right above the orchestrator, matching the existing pattern already used
    there for applyPrecisionBreakers / agentHoldAuditDetail / agentDispositionLabels /
    resolveLiveMigrationCollisionHold).
  • Pure code motion — zero behavior change. Every field, every conditional spread
    (...(x !== undefined ? { x } : {})), every comment explaining a non-obvious decision, and the exact
    set of already-resolved local values feeding the call are preserved verbatim; only the shape of how
    they reach planAgentMaintenanceActions changed. Control flow, short-circuit ordering, error handling,
    and every computed value are unchanged.
  • Net effect: runAgentMaintenancePlanAndExecute shrinks from 717 to 665 lines (-52), with the extracted
    79-line inline object replaced by a 27-line call passing the same already-in-scope local variables by
    name to the new helper.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Note on the issue link: this is Part of #4607, not Closes/Fixes, since the issue explicitly covers
two more functions (maybePublishPrPublicSurface, processGitHubWebhook) as separate follow-up PRs — see
the issue body: "this is expected to land as multiple sequential PRs (one per function)."

Validation

  • git diff --check
  • npm run actionlint — not run; no .github/workflows/** files touched.
  • npm run typecheck — clean, both before and after rebasing onto latest main.
  • npm run test:coverage — not run as the literal full-suite command (see explanation below); a
    scoped coverage run proved the diff itself is fully covered.
  • npm run test:workers — not run; no test/workers/**-relevant code touched.
  • npm run build:mcp / npm run test:mcp-pack — not run; no MCP package changes.
  • npm run ui:openapi:check / npm run ui:lint / npm run ui:typecheck / npm run ui:build — not
    run; no apps/gittensory-ui/** or API/schema changes.
  • npm audit --audit-level=moderate — not run; no dependency changes.
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — there is no new behavior (pure extraction); see below for how the moved code's existing coverage was confirmed.

If any required check was skipped, explain why:

  • This is a single-file, behavior-preserving refactor of src/queue/processors.ts with no UI, MCP,
    workers, schema, OpenAPI, or dependency surface touched, so those gates are left to CI rather than
    duplicated locally (they path-filter out for this diff anyway). The two checks that matter for a pure
    refactor — typecheck and the exercising test suite — were run directly, in full, in the foreground,
    twice (once pre-rebase, once immediately after rebasing onto the latest main):
    • npm run typecheck: clean both times.
    • npx vitest run test/unit/queue.test.ts: 807/807 tests passed, unmodified, both times. This is
      the primary test file exercising runAgentMaintenancePlanAndExecute (via the higher-level webhook
      processing / auto-action-convergence suites) and planAgentMaintenanceActions. No existing assertion
      was touched — an unmodified green run of the exact pre-existing suite is the correctness signal for a
      zero-behavior-change extraction.
    • Coverage: npx vitest run test/unit/queue.test.ts --coverage, then cross-referenced the resulting
      coverage/lcov.info against the exact added-line ranges from git diff --unified=0 (the new
      buildAgentMaintenancePlanInput function and the rewritten call site). Zero uncovered lines and zero
      uncovered branches within those ranges — the extraction is fully exercised by the existing suite
      transitively, so no new dedicated test file was added for it (a hand-built RepositorySettings /
      PullRequestRecord / gate fixture for a new test file would be disproportionate — RepositorySettings
      alone is a 432-line type with no existing minimal-fixture helper in the codebase).

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no auth/cookie/CORS/session code touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no API/OpenAPI/MCP surface touched.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
  • Visible UI changes include a UI Evidence section below... — N/A, no visible/UI changes (backend-only refactor).
  • Public docs/changelogs are updated where needed... — N/A, no doc-affecting behavior change; CHANGELOG.md intentionally not touched.

UI Evidence

Not applicable — this PR has no visible/UI/frontend/docs surface; it is a backend-only, behavior-preserving
extraction inside src/queue/processors.ts.

Notes

  • This is a maintainer/owner PR (issue Break up processors.ts mega-functions #4607 is labeled maintainer-only).
  • Follow-ups (separate PRs, per the issue): extracting maybePublishPrPublicSurface into named steps, and
    splitting processGitHubWebhook into per-event-type handlers.

…lanAndExecute

Move the ~30-value planAgentMaintenanceActions object literal into a
named, independently-typed buildAgentMaintenancePlanInput helper so the
orchestrator threads fewer inline values. Pure code motion, no behavior
change (#4607).
@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 10, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 10, 2026
@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-10 12:35:11 UTC

1 file · 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
This is a pure code-motion refactor: the ~79-line inline object literal built for the `planAgentMaintenanceActions(...)` call inside `runAgentMaintenancePlanAndExecute` is extracted into a new `buildAgentMaintenancePlanInput` helper co-located above the orchestrator, matching existing patterns in this file (`applyPrecisionBreakers`, `agentHoldAuditDetail`, etc.). I compared the removed inline object against the new function's return block field-by-field and every key, conditional spread, and comment is preserved verbatim in the same order, and the 23 named properties passed at the new call site match the 23 destructured parameters exactly — this is a faithful, behavior-preserving extraction, not a symptom fix or scope-creep addition.

Nits — 5 non-blocking
  • The new function's doc comment claims this extraction lets 'the shaping itself is unit-tested directly instead of only through the orchestrator (Break up processors.ts mega-functions #4607)', but no direct unit test for `buildAgentMaintenancePlanInput` is added in this diff (0 test-file changes) and the function isn't exported, so that stated benefit isn't actually realized yet — either add the test now or soften the comment to avoid overclaiming.
  • Since `buildAgentMaintenancePlanInput` is a pure function with no side effects, consider exporting it (if not already) so a follow-up can add the promised direct unit test without reaching into a private orchestrator internal.
  • Add a focused unit test for `buildAgentMaintenancePlanInput` covering at least one conditional-spread arm (e.g. `blacklistEntry !== null` vs `null`) now that it's independently typed via `AgentActionPlanInput`, since coverage on this path currently only comes indirectly through orchestrator-level tests.
  • Given this is explicitly framed as the first of several `processors.ts` breakup slices (Break up processors.ts mega-functions #4607), a one-line note in the PR description on the plan for the remaining, larger slices (`maybePublishPrPublicSurface`, `processGitHubWebhook`) would help reviewers track the sequencing.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

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: 48 registered-repo PR(s), 40 merged, 316 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 316 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
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: 48 PR(s), 316 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.

🟩 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 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.14%. Comparing base (a2e5d3e) to head (354915b).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4688   +/-   ##
=======================================
  Coverage   94.14%   94.14%           
=======================================
  Files         436      436           
  Lines       38530    38532    +2     
  Branches    14049    14049           
=======================================
+ Hits        36273    36275    +2     
  Misses       1599     1599           
  Partials      658      658           
Files with missing lines Coverage Δ
src/queue/processors.ts 95.32% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 10, 2026
@JSONbored
JSONbored merged commit 8af214c into main Jul 10, 2026
11 checks passed
@JSONbored
JSONbored deleted the refactor/processors-runagentmaintenanceplan-4607 branch July 10, 2026 13:24
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

None yet

Development

Successfully merging this pull request may close these issues.

1 participant