Skip to content

fix(miner-ui): register chat-action vite plugins for the preview server too - #7293

Closed
luciferlive112116 wants to merge 1 commit into
JSONbored:mainfrom
luciferlive112116:fix/vite-preview-chat-action-plugins-7228
Closed

fix(miner-ui): register chat-action vite plugins for the preview server too#7293
luciferlive112116 wants to merge 1 commit into
JSONbored:mainfrom
luciferlive112116:fix/vite-preview-chat-action-plugins-7228

Conversation

@luciferlive112116

Copy link
Copy Markdown
Contributor

Closes #7228

Summary

  • apps/loopover-miner-ui/vite-chat-governor-actions.ts (chatGovernorActionsPlugin) and vite-chat-discover-attempt-actions.ts (chatDiscoverAttemptActionsPlugin) implemented only configureServer().
  • The app's documented persistent-service deployment path (apps/loopover-miner-ui/README.md's "Running as a persistent service" + systemd/loopover-miner-ui.service.example) runs npm run build && npm run preview, and vite preview fires only configurePreviewServer, never configureServer. So the governor pause/resume and discover/attempt chat actions were silently unregistered under vite preview in production — unlike every sibling vite-*-api.ts plugin, all of which register for both hooks.
  • Fix: register into the shared registry on both configureServer and configurePreviewServer via a shared register helper in each plugin (the exact both-hooks shape the sibling plugins already use). No new /api/* route is added; only the existing plugin hook is wired for preview. Adds a parity test asserting both plugins now implement both hooks.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused: the two named plugin files plus one test, scoped exactly to the configurePreviewServer gap called out in the issue. No other vite-*-api.ts file and no new route are touched.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves — Closes #7228 (bare, above).

Validation

  • git diff --check — clean.
  • npm run ui:typecheck (@loopover/ui-miner tsc --noEmit) — 0 errors.
  • npm run ui:test (@loopover/ui-miner vitest run --coverage) — 322 tests pass, coverage thresholds met; the new vite-chat-action-plugins.test.ts (2 tests) passes.
  • npm --workspace @loopover/ui-miner run build (vite build) — built successfully.
  • ESLint clean on all changed files.

If any required check was skipped, explain why:

  • Backend checks (test:coverage, test:workers, build:mcp/test:mcp-pack, ui:openapi:check, engine/miner suites, npm audit) are not applicable: this PR touches only apps/loopover-miner-ui/** (two Vite plugin config files + one test) — no src/**, no MCP/API/OpenAPI surface, no engine, and no dependency changes. apps/** is outside the Codecov codecov/patch scope; the miner-ui workspace's own vitest coverage thresholds (verified above) are the relevant signal.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, 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/route logic changes; the plugins register the same actions on an additional lifecycle hook. The plugins already sit under /api/* behind authPlugin() in vite.config.ts, unchanged.
  • API/OpenAPI/MCP behavior is updated and tested where needed — N/A (no new/changed route or tool).
  • UI changes use live API data or real empty/error/loading states — N/A: this is dev/preview-server plugin wiring, not a rendered UI change.
  • Visible UI changes include a UI Evidence section — N/A: no visible/visual change. The fix only makes existing chat actions register when the app is served via vite preview; there is no new or altered rendered surface to screenshot.
  • Public docs/changelogs are updated where needed; no changelog edited. The README already documents the both-hooks contract this fix makes true.

UI Evidence

Not applicable — no visible/visual change. This PR wires two Vite dev/preview-server plugins to register on the preview hook in addition to the dev hook; nothing rendered changes.

Notes

  • The two hooks are mutually exclusive at runtime (vite dev fires configureServer, vite preview fires configurePreviewServer), so registering on both is safe — only one runs per process.

…er too

vite-chat-governor-actions.ts and vite-chat-discover-attempt-actions.ts
implemented only configureServer, but the documented persistent-service path
(README + systemd/loopover-miner-ui.service.example) runs npm run build && npm
run preview, and vite preview fires only configurePreviewServer. So the governor
pause/resume and discover/attempt chat actions were silently unregistered in
production, unlike every sibling vite-*-api.ts plugin. Register on both hooks via
a shared helper, mirroring the siblings, and add a parity test.

Closes JSONbored#7228
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 19, 2026
@loopover-orb

loopover-orb Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-19 13:14:42 UTC

3 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This PR adds a `configurePreviewServer` hook alongside the existing `configureServer` hook in both chat-action vite plugins, using a shared `register` closure so the dynamic-import registration logic isn't duplicated. This directly closes #7228's gap where `vite preview` (the documented persistent-service path) never fired `configureServer` and thus silently dropped governor/discover/attempt chat action registration in production. The fix matches the existing pattern used by sibling `vite-*-api.ts` plugins, and the new test asserts both hooks are present on both plugin objects; CI (typecheck, tests, build) passed on this exact commit.

Nits — 5 non-blocking
  • The new test only checks `typeof plugin.configureServer === 'function'` and doesn't invoke `configurePreviewServer` to assert it actually calls `register()` end-to-end and populates the registry, unlike the existing `chatDiscoverAttemptActionsPlugin` configureServer test shown in the reference file — consider adding that same live-registration assertion for the preview hook for full parity coverage.
  • Both plugins now duplicate the identical `register` closure + dual-hook wiring pattern; a small shared helper (e.g. `registerOnServerAndPreview(fn)`) could remove the near-duplicate code between the two files, though at this size it's a minor DRY nit, not a blocker.
  • apps/loopover-miner-ui/src/vite-chat-action-plugins.test.ts: extend the test to actually call `plugin.configurePreviewServer()` and `vi.waitFor` on the shared registry the same way the existing configureServer test does, so the new hook's behavior is verified, not just its presence.
  • Consider factoring the repeated register-on-both-hooks Plugin shape into a tiny shared utility used by both vite-chat-*-actions.ts files to avoid future drift if a third such plugin is added.
  • Pull request duplicates other open work — Check for an existing pull request or issue covering this change and coordinate or consolidate before continuing.

Why this is blocked

  • Linked issue overlaps another open PR — Review the related PRs before spending reviewer time on duplicate work.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Linked issue overlaps another open PR — Review the related PRs before spending reviewer time on duplicate work.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7228
Related work ⚠️ Same linked issue: #7292, #7295 Another open PR references the same linked issue.
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: 181 registered-repo PR(s), 111 merged, 28 issue(s).
Contributor context ✅ Confirmed Gittensor contributor luciferlive112116; Gittensor profile; 181 PR(s), 28 issue(s).
Improvement ✅ Minor risk: low · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
Both vite-chat-governor-actions.ts and vite-chat-discover-attempt-actions.ts now implement configurePreviewServer alongside configureServer, calling the same registration function via a shared register() helper, exactly mirroring the vite-chat-api.ts pattern the issue points to. A regression test is added asserting both hooks exist on both plugins, though it only checks hook presence rather than d

Review context
Contributor next steps
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.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 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.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Linked issue overlaps another open PR; duplicate of open PR #7295). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 19, 2026
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chat governor and discover/attempt action registration is missing from configurePreviewServer, unlike every sibling vite-*-api.ts plugin

1 participant