fix(miner): replace never-registered run with loop in fleet-mode manifests - #9524
Conversation
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-28 06:38:45 UTC
Review summary Nits — 5 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
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. Decision record
🟩 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.
|
|
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 #9524 +/- ##
=======================================
Coverage 89.56% 89.56%
=======================================
Files 843 843
Lines 110201 110213 +12
Branches 26230 26233 +3
=======================================
+ Hits 98705 98717 +12
Misses 10233 10233
Partials 1263 1263
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…manifests docker-compose.miner.yml and k8s/miner-deployment.yaml both invoked `run` since their first commits (#5299, #5258) -- a subcommand the miner CLI has never dispatched. A fleet worker built from either manifest sets up the egress firewall, then immediately exits 1 with "Unknown command: run" instead of ever mining. `loop` is the real continuous fleet-worker daemon. Also extends the DEPLOYMENT.md docs-accuracy audit (scripts/check-miner-deployment-docs.ts) to cross-check these container manifests' commands against the CLI's registered-command table, the same way it already does for markdown prose, so this drift class fails CI going forward instead of only being caught by inspection.
647f322 to
5bd0846
Compare
Summary
docker-compose.miner.ymlandk8s/miner-deployment.yamlboth invoke the miner CLI withrun— a subcommand that has never been registered inpackages/loopover-miner/bin/loopover-miner.ts's dispatch table, confirmed all the way back to each manifest's first commit (feat(miner-deployment): add docker-compose.miner.yml for AMS fleet mode #5299, feat(miner-deployment): add Kubernetes StatefulSet + Secret example for AMS fleet-mode #5258) viagit log -S. A fleet worker built from either manifest sets up the egress firewall, then immediately exits 1 withUnknown command: runinstead of ever mining.loopis the real continuous fleet-worker daemon (seeloop-cli.ts,DEPLOYMENT.md,systemd/loopover-miner.service.example).loopwith placeholder<owner/repo>/<login>args (the daemon requires a real target and--miner-login, with no env-var fallback), updates their comments, and fixes the two test files that pinned the brokenrunvalue (test/unit/miner-docker-compose.test.ts,test/unit/miner-k8s-manifests.test.ts), adding a regression assertion to each.DEPLOYMENT.mddocs-accuracy audit (scripts/check-miner-deployment-docs.ts,packages/loopover-miner/lib/deployment-docs-audit.ts) to also cross-check these two container manifests'command:/args:against the CLI's registered-command table — the same drift class the audit already catches for markdown prose, now covering deploy YAML too, so this exact bug fails CI going forward instead of only being caught by inspection.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.runwas never registered, in any commit, viagit log -S), not a contributor PR against the issue queue.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally — not run in full (unsharded whole-repo run); instead ran targetedvitest run+--coverageon the four affected test files (miner-docker-compose.test.ts,miner-k8s-manifests.test.ts,miner-deployment-docs-audit.test.ts,check-miner-deployment-docs.test.ts, 55/55 passing) and confirmed the one Codecov-measured touched file,packages/loopover-miner/lib/deployment-docs-audit.ts, hits 100% lines/branches/functions via the resultingcoverage/lcov.info.scripts/**(the other touched source file) is explicitly excluded from Codecov's patch gate (codecov.yml'signore:list, mirroringvitest.config.ts'scoverage.include).npm run test:workers— not run; no Cloudflare Worker code touched.npm run build:mcp— not run; no MCP package touched.npm run test:mcp-pack— not run; no MCP package touched.npm run ui:openapi:check— not run; no API/OpenAPI surface touched.npm run ui:lint/ui:typecheck/ui:build— not run; no UI code touched.npm audit --audit-level=moderate— ran; reports 11 pre-existing high-severity findings (sharp/libvips, minimatch chain) unrelated to this diff — this PR makes nopackage.json/package-lock.jsonchanges (verified viagit diff main -- package.json package-lock.json, empty).extractContainerCommandClaim,auditContainerCommands,assertContainerCommandsInSync), plus arun-must-not-appear regression test in each manifest's own test file and a live-tree integration test proving the new container-manifest audit path is genuinely wired (assertsclaimCounts.containerCommands === 2against real files, not a stub).Also directly ran the actual CI entrypoint this PR changes:
npm run test:miner-deployment-docs-audit→Miner deployment docs audit ok: 30 env vars, 16 paths, 5 subcommands, 2 container commands.Safety
UI Evidencesection — not applicable, no visible UI change (deploy manifests + a Node CI script).DEPLOYMENT.mditself already documentedloopcorrectly (only the two manifests had drifted), so no further doc edit was needed.Notes
<owner/repo>,<login>) in both manifests are deliberate:looprequires a real repository target (or--search <query>) plus--miner-login <login>, and there is no environment-variable fallback for either inloop-cli.ts's argument parser, so an un-edited manifest fails loudly on a clearly-named placeholder rather than silently misbehaving.