Skip to content

fix(mcp): complete tab-completion for cache list - #6298

Merged
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
luciferlive112116:fix/mcp-cache-list-completion
Jul 16, 2026
Merged

fix(mcp): complete tab-completion for cache list#6298
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
luciferlive112116:fix/mcp-cache-list-completion

Conversation

@luciferlive112116

Copy link
Copy Markdown
Contributor

Summary

Closes #6260

CLI_COMMAND_SPEC's cache entry read ["status", "clear"], but runCacheCli (bin/loopover-mcp.js:2340) has always accepted list/ls, and both printCacheHelp and the README document loopover-mcp cache list.

That entry is the single source for buildBashCompletion / buildZshCompletion / buildFishCompletion / buildPowershellCompletion and for suggestCommand's typo-suggester — so tab-completion for cache list silently did nothing across all four shells, while status/clear completed fine. That asymmetry is exactly why it went unnoticed.

Fix: add "list" — one word.

Only the canonical name, and that's an evidence-based call rather than a guess: the spec deliberably lists canonical subcommands only. profileCommand accepts ls/use/rm/delete and maintainCli accepts pending, yet none of those aliases appear in their spec entries. Completing an alias isn't the contract; completing every real subcommand is.

The audit the issue asks for

Verify no other command in CLI_COMMAND_SPEC has a similar stale/incomplete subcommand list by cross-checking each against its actual run*Cli implementation.

Done — cache was the only stale entry:

Command Handler accepts Spec Verdict
cache status, clear, list(ls) status, clear stale — fixed
agent plan, status, explain, packet same
profile list(ls), create, switch(use), remove(rm/delete) canonical 4
maintain status, queue(pending), approve, reject, pause, resume, set-level, precision canonical 8

Scope

Validation

  • git diff --check clean.
  • npm run build --workspace @loopover/mcp (the package's node --check gate) — exit 0.
  • Every affected suite, not just the new onemcp-cli-completion-spec, mcp-cli-basics, mcp-tool-rename-aliases (the 42-tool count canary), command-suggest, mcp-cli-packets: 71 tests passed.
  • Proved the tests catch the bug: reverted the one-line fix and confirmed 3 tests fail (REGRESSION: cache declares list…, the cache parity case, and the typo-suggester case), then pass again with it restored.
  • Checked the drift-check surface: scripts/gen-command-reference.mjs does not read CLI_COMMAND_SPEC, so command-reference:check is unaffected and no generated artifact needs regenerating.
  • Lint: CI's only lint step is npm run ui:lint (@loopover/ui + @loopover/ui-miner), which covers apps/** — this PR touches neither. bin/loopover-mcp.js is not prettier-formatted on main either, so no formatting rule applies to it.
  • Rebased onto current main.

The test pins the audit as an invariant, in both directions, rather than just this one miss:

Test What it locks
REGRESSION: cache declares list the reported bug
every canonical subcommand a handler accepts is declared (×4 commands) the next entry to rot fails CI
nothing declared is unhandled (×4) a spec entry for a removed subcommand would complete to a guaranteed error
all four shells offer cache list one stale entry breaks every shell, so the fix must reach all of them

It reads the spec out of the committed source rather than importing it, because bin/loopover-mcp.js starts a server on import — parsing is how a test can inspect the spec without launching one.

If any required check was skipped, explain why:

  • Full test:ci not run end-to-end locally (Linux-only steps on Windows). The change-relevant gates — the package's syntax check and every suite touching this file or the CLI spec — were run directly and are green.
  • No Codecov patch obligation: packages/loopover-mcp/** is outside Codecov's coverage.include; test/** is ignored.

Safety

  • No secrets, wallets, hotkeys, trust scores, rewards, private rankings, or private maintainer evidence.
  • No auth/cookie/CORS/GitHub App/session change; no network or API surface touched.
  • Strictly additive to completion: it offers one more subcommand the CLI already supports. No existing completion, command, or handler behavior changes — cache status/cache clear are untouched, and the added value routes to an already-implemented, already-documented path.
  • No API/OpenAPI/MCP tool change; no schema change; no generated artifact affected (verified against the command-reference generator).
  • No UI changes; no changelog edit.

@superagent-security

Copy link
Copy Markdown
Contributor

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

CLI_COMMAND_SPEC's cache entry read ["status", "clear"], but runCacheCli
has always accepted list/ls too, and both printCacheHelp and the README
document `loopover-mcp cache list`. That entry is the single source for
buildBash/Zsh/Fish/PowershellCompletion and for suggestCommand's
typo-suggester, so tab-completion for `cache list` silently did nothing
across all four shells -- while status/clear completed fine, which is
why it went unnoticed.

Add "list". Only the canonical name: the spec deliberately lists
canonical subcommands only, since profile accepts ls/use/rm/delete and
maintain accepts pending, yet none of those aliases appear in their
entries.

Audited every other entry against its handler, as the issue asks, and
cache was the only stale one: agent (plan/status/explain/packet),
profile (list/create/switch/remove) and maintain (status/queue/approve/
reject/pause/resume/set-level/precision) each already declare every
canonical subcommand their handler runs.

The test pins that audit as an invariant rather than just this one miss,
in both directions: every canonical subcommand a handler accepts must be
declared, and nothing declared may be unhandled (which would complete to
a guaranteed error). So the next entry to rot fails CI instead of
silently degrading completion. It reads the spec out of the committed
source because bin/loopover-mcp.js starts a server on import, and it
checks all four shells, since one stale entry breaks every one of them.

Closes JSONbored#6260
matchAll's capture groups are string | undefined under this repo's strict
TS config, so indexing spec[rawName] and calling rawSubs.matchAll tripped
TS2538/TS18048. vitest transpiles without typechecking, which is why the
suite passed locally while validate-code failed. Bind both groups
explicitly instead.
@luciferlive112116
luciferlive112116 force-pushed the fix/mcp-cache-list-completion branch from e3eed00 to 2f1792d Compare July 16, 2026 00:49
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 01:05:35 UTC

2 files · 1 AI reviewer · no blockers · readiness 88/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a genuine one-line fix: `cache` was missing `list` from `CLI_COMMAND_SPEC`, which is the single source feeding all four shell completion builders and the typo-suggester, so `cache list` silently failed to complete while `status`/`clear` worked. The added regression suite is well-designed — it asserts the general invariant (every canonical subcommand a handler accepts must appear in its spec entry, and vice versa) across all four commands and all four shells rather than merely pinning `cache list`, which would let the next entry rot the same way undetected. The audit table matches what's shown in the diff (profile/agent/maintain already canonical-only), and the fix is scoped tightly to the stale spec entry with no unrelated changes.

Nits — 3 non-blocking
  • The parity test regex-parses `bin/loopover-mcp.js` as text (test/unit/mcp-cli-completion-spec.test.ts) rather than importing it, which is reasonable given the entrypoint starts a server on import, but it's brittle against reformatting (e.g. multi-line array literals) — worth a short comment noting the fragility trade-off is intentional (already partially present).
  • acceptedBy()'s fallback of slicing 5000 chars when no next `function` is found is an arbitrary bound that could silently truncate a handler body if one grows past that in the future.
  • Consider asserting `declaredSpec()` returns non-empty for all top-level keys once, to catch a totally malformed CLI_COMMAND_SPEC block instead of only per-command misses.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6260
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
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: 142 registered-repo PR(s), 77 merged, 32 issue(s).
Contributor context ✅ Confirmed Gittensor contributor luciferlive112116; Gittensor profile; 142 PR(s), 32 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff adds "list" to CLI_COMMAND_SPEC.cache exactly as requested, and includes tests plus a documented cross-check of the other spec entries (agent, profile, maintain) confirming no other stale entries were found.

Review context
  • Author: luciferlive112116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 142 PR(s), 32 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step in the Signals table above.
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://gittensory.aethereal.dev/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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 0d722ca into JSONbored:main Jul 16, 2026
14 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 16, 2026
12 tasks
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.

fix(mcp): shell-completion spec is stale for "cache list", silently breaking tab-completion across all 4 shells

1 participant