feat(miner): wire chat action-dispatch to the existing discover/attempt routes - #6855
Conversation
…pt routes (JSONbored#6837) Registers discover / attempt into the chat-action registry the scaffolding (JSONbored#6519) ships empty -- the third and last family its own docstring names. Handlers call the miner-ui clients requestDiscover / requestAttempt, so chat POSTs the same /api/discover and /api/attempt routes that already exist (JSONbored#6522, registered at vite.config.ts:36-37). The issue's first deliverable ('add new API routes') is already satisfied: JSONbored#6522 shipped both routes and their plugins. Adding a second pair would duplicate live routes, so this delivers the only unbuilt part -- the chat wiring. The gate lives at the endpoint, deliberately: attempt inherits the real chokepoint via runAttempt -> attempt-runner.js, and discover performs no gated write so neither the CLI nor the route has one. Re-evaluating it here would be a second, competing gate. Mirrors chat-governor-actions.js / chat-portfolio-actions.js exactly. Closes JSONbored#6837
|
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 #6855 +/- ##
=======================================
Coverage 93.64% 93.64%
=======================================
Files 684 685 +1
Lines 68192 68237 +45
Branches 18706 18706
=======================================
+ Hits 63856 63901 +45
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:52:00 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed 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 last of the three chat action families.
chat-action-registry.js:4-5names them all — "portfolio release/requeue, governor pause/resume, discover/attempt" — and the first two now ship. This registers the pair:discover→ the miner-ui clientrequestDiscoverattempt→ the miner-ui clientrequestAttemptBoth POST the existing
/api/discoverand/api/attemptroutes — neverdiscover-cli.js/attempt-cli.jsdirectly, and never a hand-rolled fetch. The clients are injected by the wire module, so this module structurally cannot reach the CLI even by accident. It owns only the registration contract + params validators, exactly like its two siblings.Deliverable 1 is already shipped — please check this claim first
The issue's first deliverable asks for "new
discover/attemptAPI routes", on the premise that they're "CLI-only today ... with no HTTP route". That premise is stale. #6522 already shipped both:apps/loopover-miner-ui/vite-discover-api.tsandvite-attempt-api.tsvite.config.ts:36-37(discoverApiPlugin(),attemptApiPlugin())src/discover-api.test.tsandsrc/attempt-api.test.tsSo adding a second pair would duplicate live routes. That is not a hypothetical concern:
mainwas red earlier today because #6706 and #6709 — two PRs for two duplicate issues — each added the same import pair and both merged 72 seconds apart, leavingroutes.tsunparseable. I'd rather flag a stale deliverable than recreate that.This PR therefore delivers the only unbuilt part: the chat wiring (deliverable 2) and its tests (deliverable 3).
The gate lives at the endpoint — deliberately, and I verified it
The issue also asks that the routes "route through the same Governor chokepoint /
PreToolUsedeny-hook path". The house architecture is inherit, don't re-implement, and the existing code says so directly:vite-attempt-api.ts:7-9— "Because it calls the real, unmodifiedrunAttempt, it inherits that command's Governor chokepoint gate for free" (attempt-runner.js gates every write at:203before executing at:223).vite-discover-api.ts:13-14— "discoverhas no Governor chokepoint of its own (it only fans out + ranks + enqueues, none of the gated write actions), so — matching the CLI exactly — this route adds none either."vite-governor-api.ts:10— "this file never touches governor-chokepoint.js".No miner-ui route invokes the chokepoint. Re-evaluating it in this module would be a second, competing gate on a path that already has one (attempt) or needs none (discover) — and would gate chat more strictly than the identical CLI invocation. So this mirrors
chat-governor-actions.jsandchat-portfolio-actions.js: satisfy the registry'sgovernorGatedHandlerbrand with an allow-stageevaluateGate, keep it injectable.Worth noting
PreToolUsecan't be wired here even in principle:buildHouseRulesPreToolUseHookis a factory for a Claude Agent SDK hook callback, not an HTTP-callable gate.The safety properties remain structural, not conventional:
register()refuses any handler not produced bygovernorGatedHandler()(private unforgeableSymbol), and the flag is checked first, before the registry is touched.Params validation
discoverandattemptdiffer deliberately, matching their clients' real contracts:discover— everyDiscoverActionInputfield is optional (the CLI defaults them all), so nullish/{}is a valid "discover with defaults" and is forwarded as{}rather thanundefined, since the client always POSTs a JSON body.targets: []is accepted as an explicit empty list.attempt—repoFullName/issueNumber/minerLoginare required: there is no default issue to attempt.issueNumbermust be a positive integer, so0,-3,1.5,NaNand"12"all reject rather than reaching the CLI as a nonsense issue reference.Validation
codecov/patchwall onpackages/loopover-miner/lib/**(rootvitest.config.tsincludespackages/loopover-miner/lib/**/*.js, so this file is gated).portfolio,registry,dispatch,governor-actions— 73/73.npm run typecheck— 0 errors, matching cleanmainexactly.npm run build:miner— added to the miner package's explicitnode --checklist, alphabetically beside its three siblings;package.jsonre-validated as parseable JSON.eslint— 0 errors/0 warnings ·git diff --checkclean · rebased on latestmain, no base conflict.Rebasing surfaced a real conflict worth mentioning: #6850 (the portfolio sibling) merged mid-work and edits the same
node --checklist. Resolved so both entries survive —chat-discover-attempt-actions.js,chat-governor-actions.js, andchat-portfolio-actions.jsare all present and alphabetical, and the full chat suite passes against the merged base.Scope
.d.ts, the package build list, and the root test suite. Wanted paths (packages/,test/).site/,CNAME, orlovablechanges.Safety
LOOPOVER_MINER_CHAT_ACTIONSgates execution and is checked before anything else. With the flag unset, behavior is byte-identical to today.attempt— the one family that performs real writes — reaches them only throughrunAttempt, which carries the persisted chokepoint gate. Chat gets no privilege the CLI doesn't already have.registry.hasguarded), so double-wiring can't throw.Closes #6837