Skip to content

docs: address cli-skills review findings#24

Merged
charlesrhoward merged 1 commit intomainfrom
feat/cli-skills-review-fixes
Apr 24, 2026
Merged

docs: address cli-skills review findings#24
charlesrhoward merged 1 commit intomainfrom
feat/cli-skills-review-fixes

Conversation

@charlesrhoward
Copy link
Copy Markdown
Contributor

Follow-up to #23. Addresses the Mogplex PR Review and Codex findings.

Fixes applied

  • Preflight gating (skills/using-mogplex-cli/SKILL.md + mirror). The snippet used command -v mogplex >/dev/null || echo "…" which prints a warning and then runs mogplex login status anyway, producing command not found instead of a clean stop. Replaced with a hard || { echo …; exit 1; } guard plus prose pointing at Installation.
  • mkdir -p before install (skills/README.md + content/docs/cli/skills/index.mdx). cp -R skills/* ~/.claude/skills/ fails on a clean machine when the directory doesn't exist. Now prepends mkdir -p ~/.claude/skills to match the per-project command.

False positives (no change needed)

  • Truncation in mogplex-slash.mdx / using-mogplex-cli.mdx. The reviewer saw clipped GitHub diff hunks; the on-branch files end cleanly with complete ## See also sections (wc -l confirms 108 and 90 lines respectively).
  • Missing Hard rules / Authentication-guide link in mogplex-auth.mdx. Both are present on-branch; the diff hunk window cut them off.

Test plan

  • pnpm lint
  • pnpm types:check
  • pnpm build (165 static pages, all skills routes still render)
  • Spot-check the updated preflight block renders on /cli/skills/using-mogplex-cli

- preflight: gate `mogplex login status` behind a hard `exit 1` when
  `mogplex` is not on PATH so agents following the snippet literally do
  not fall through to a confusing `command not found` (skills/
  using-mogplex-cli/SKILL.md + docs mirror).
- install: prepend `mkdir -p ~/.claude/skills` to the global install
  command so first-time users on a clean machine do not hit
  "No such file or directory" (skills/README.md + docs/skills/index.mdx).

The two truncation warnings from the reviewer were false positives
caused by clipped GitHub diff hunks; all `.mdx` files end cleanly and
already contain the `Hard rules` section and the Authentication-guide
See-also link the review flagged as missing.
@charlesrhoward charlesrhoward enabled auto-merge (squash) April 24, 2026 00:54
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mogplex-docs Ready Ready Preview, Comment Apr 24, 2026 0:54am

Request Review

@mogplex
Copy link
Copy Markdown

mogplex Bot commented Apr 24, 2026

Mogplex PR Review

Status: No material issues found

Summary

All four changes are correct, consistent, and directly address the prior review findings. No issues found — this PR is approve-ready.

Preflight fix (using-mogplex-cli.mdx + SKILL.md)

The original command -v mogplex >/dev/null || echo "…" was a silent no-op: it printed a warning and then continued to execute mogplex login status, which would produce a confusing command not found error instead of a clean stop. The replacement || { echo "…"; exit 1; } correctly hard-gates execution. The addition of 2>&1 is also correct — it suppresses any stderr from command -v itself. The surrounding prose is clear and updated consistently in both files.

mkdir -p fix (index.mdx + README.md)

Prepending mkdir -p ~/.claude/skills && to the global install command is the right fix. The per-project variant already had this guard and is untouched. Both files now have symmetric install patterns.

Consistency check

The behavior-critical code block is identical in both SKILL.md and the .mdx mirror. The prose wording differs slightly (absolute URL in SKILL.md vs. relative path in .mdx), which is appropriate given the different rendering contexts.

Verdict

APPROVE — Targeted, correct fixes with no regressions or new issues introduced.

Affected files:

  • content/docs/cli/skills/using-mogplex-cli.mdx
  • skills/using-mogplex-cli/SKILL.md
  • skills/README.md
  • content/docs/cli/skills/index.mdx

View check run

@charlesrhoward charlesrhoward merged commit 0ab9c6b into main Apr 24, 2026
6 checks passed
@charlesrhoward charlesrhoward deleted the feat/cli-skills-review-fixes branch April 24, 2026 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant