feat(miner): wire chat action-dispatch to the existing portfolio release/requeue routes - #6850
Conversation
…ase/requeue routes (JSONbored#6838) Registers portfolio_release / portfolio_requeue into the chat-action registry the scaffolding (JSONbored#6519) ships empty. Handlers call the miner-ui clients releasePortfolioQueueItem / requeuePortfolioQueueItem, so chat POSTs the same /api/portfolio-queue/{release,requeue} routes the dashboard buttons already use -- no new route, no parallel write path, no direct store access. Release/requeue is local queue administration, not a chokepoint content-write: the route is a thin bridge to the same store methods the CLI's queue release/requeue use and invokes no chokepoint itself. Gating chat more strictly than the button beside it would change the button-triggered flow JSONbored#6838 freezes, so this mirrors chat-governor-actions.js exactly and satisfies the registry brand with an allow-stage evaluateGate. Execution stays behind the shared LOOPOVER_MINER_CHAT_ACTIONS flag. Closes JSONbored#6838
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6850 +/- ##
=======================================
Coverage 93.63% 93.64%
=======================================
Files 683 684 +1
Lines 68163 68191 +28
Branches 18706 18706
=======================================
+ Hits 63827 63855 +28
Misses 3350 3350
Partials 986 986
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-17 08:31:55 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver 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://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
The chat action-dispatch scaffolding (#6519) ships its registry deliberately empty —
chat-action-registry.js:98says so explicitly, and names this issue's family as one of the three that fill it. This registers the portfolio pair:portfolio_release→ the miner-ui clientreleasePortfolioQueueItemportfolio_requeue→ the miner-ui clientrequeuePortfolioQueueItemBoth clients POST the existing
/api/portfolio-queue/{release,requeue}routes the dashboard buttons already call. No new route, no parallel write path, no direct store access — the clients are injected by the miner-ui wire module, so this module can't reachportfolio-queue.jsor hand-roll a fetch even by accident. It owns only the registration contract + params validators, exactly like itschat-governor-actions.jssibling.The one judgement call — why an allow-stage gate, not the chokepoint
This is the part worth reviewing, because "chat action doesn't route through the Governor chokepoint" reads like a bypass. It isn't, and I checked rather than assumed:
vite-portfolio-queue-actions-api.ts:4-6describes itself as "a thin bridge to the EXISTING store methods the CLI'squeue release/queue requeuesubcommands already use (reclaimStuckItem/requeueItem) — no new queue semantics are invented here." I grepped it: it invokes no chokepoint at all. Release/requeue is local queue administration — it writes no content to GitHub.So routing the chat path through
evaluateGovernorChokepointGatewould gate chat more strictly than the button sitting beside it, which is a change to the button-triggered flow's contract that #6838 explicitly freezes ("No changes to the existing route or button-triggered flow"). Instead this mirrorschat-governor-actions.jsexactly, which faced the identical situation for administrative pause/resume and resolved it the same way: satisfy the registry'sgovernorGatedHandlerbrand with an allow-stageevaluateGate, and keep execution behind the sharedLOOPOVER_MINER_CHAT_ACTIONSflag viadispatchChatAction.The safety properties that matter are all still structural, not by convention:
register()refuses any handler not produced bygovernorGatedHandler()(privateSymbolbrand, unforgeable), so a raw function can never be registered.evaluateGatestays injectable, so a future decision to route these through the real chokepoint is a one-argument change here, not a rewrite.Params validation
Both actions share
isPortfolioItemChatParams. Two decisions differ from the governor sibling, deliberately:{...item, status: "done"}and{...item, repo_full_name: …}are both pinned as rejected.apiBaseUrlstays optional (the client's own type isPick<…, "repoFullName" | "identifier" | "apiBaseUrl">and the CLI path omits it), and when absent it is omitted from the forwarded object rather than passed as an explicitundefined— otherwise it would serialize into the POST body and change the request the buttons already send. A test pins the exact key list, becausetoHaveBeenCalledWithusestoEqualsemantics and would treat{a, b: undefined}as equal to{a}— it cannot catch this on its own.Validation
codecov/patchwall onpackages/loopover-miner/lib/**(rootvitest.config.tsincludespackages/loopover-miner/lib/**/*.js, so this file is gated).options.registry ?? chatActionRegistry— the shared-registry default that production actually uses, which every other test skips by injecting a registry. It now has its own test.registry,dispatch,governor-actions) stay green at 35/35.npm run typecheck— 0 errors, matching cleanmainexactly. (tsccaught two real errors in my own test that vitest didn't: untypedvi.fn(async () => …)infers a zero-length tuple, somock.calls[0][0]wouldn't type-check. Mocks are now parameter-typed.)npm run build:miner— the new file is added to the miner package's explicitnode --checklist, beside itschat-governor-actions.jssibling;package.jsonre-validated as parseable JSON.eslint— 0 errors/0 warnings ·git diff --checkclean · rebased on latestmain, no base conflict.One pre-existing failure, not mine — verified, not assumed:
check-miner-package.test.tsfails 1/13 identically on cleanmainwith my work stashed.Scope
.d.ts, the package's build list, and the root test suite. Wanted paths (packages/,test/).chat-action-registry.js,chat-action-dispatch.js,chat-governor-actions.js, the portfolio route, and the miner-ui clients are all untouched.site/,CNAME, orlovablechanges.Safety
LOOPOVER_MINER_CHAT_ACTIONSgates execution, anddispatchChatActionchecks it before anything else. With the flag unset, behavior is byte-identical to today.registry.hasguarded), so double-wiring can't throw on an already-registered name.Closes #6838