Skip to content

Docs polish: fix install.sh repo, add badges, quick-install and doc-site link - #2

Merged
gregnazario merged 5 commits into
mainfrom
docs/readme-polish
Aug 26, 2026
Merged

gregnazario merged 5 commits into
mainfrom
docs/readme-polish

Conversation

@gregnazario

Copy link
Copy Markdown
Owner

Targeted documentation fixes — no rewrite; existing reference content (quick start, command/recipe tables, caching, cross-compilation, Lua API, examples) is untouched.

install.sh

  • Repo correction: REPO="anomalyco/mustfile" → REPO="gregnazario/must", so the installer resolves latest-release metadata, archives, and SHA256SUMS from this repo instead of a nonexistent one.
  • Asset names verified: there are no published releases yet, so naming was verified against .github/workflows/release.yml. The workflow uploads must-<target>.tar.gz (x86_64/aarch64-unknown-linux-gnu, x86_64/aarch64-apple-darwin) plus SHA256SUMS — exactly what the script constructs (must-${TARGET}.tar.gz, checksum grep matches sha256sum output format). No asset-name changes were needed.
  • Smallest extra adjustment (flagged per plan): checksum verification used sha256sum, which does not exist on stock macOS. Added a tiny sha256_file helper that prefers sha256sum and falls back to shasum -a 256. Without this the script would abort on every Mac even after the repo fix. Passes sh -n and shellcheck -S warning.

README.md

  • Badge row (worktree-ui style): CI badge (real .github/workflows/ci.yml) + MIT License badge.
  • Quick-install one-liner added to Install: curl -fsSL https://github.com/gregnazario/must/releases/latest/download/install.sh | sh (install.sh is uploaded as a release asset by the release workflow), alongside the existing cargo install.
  • Release binaries spelled out: prebuilt must-<target>.tar.gz/.zip assets + SHA256SUMS attached to each GitHub Release.
  • Doc site link near top: "Documentation, guides, and recipe reference → https://must.ai". Confirmed must.ai is really treated as the docs site in-repo (site/mkdocs.yml sets site_url: https://must.ai; deploy-docs.yml writes a must.ai CNAME).

Notes / follow-ups (not touched here)

  • .github/workflows/release.yml's generated release body still hardcodes anomalyco/mustfile in its curl/brew snippets and references an anomalyco/tap Homebrew tap — left alone as out of scope for this PR, but it will publish wrong install instructions on the next tag.
  • No releases exist yet despite tag v1.0.0; the first tagged release is required for the quick-install URL to resolve.

@codecov

codecov Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.01%. Comparing base (6cc0ea7) to head (5078491).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #2   +/-   ##
=======================================
  Coverage   92.01%   92.01%           
=======================================
  Files          49       49           
  Lines       18129    18129           
=======================================
  Hits        16681    16681           
  Misses       1448     1448           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hedgehog-pr-bot

hedgehog-pr-bot Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Pi code review

Findings

Medium — README.md (~line 326): doc-site link is removed, not relocated

The PR title/body state that a https://must.ai link is added "near the top," but the diff contains no such addition — the only change involving must.ai is the deletion of the Doc Site bullet from the ## Documentation section:

 ## Documentation
 
-- [Doc Site](https://must.ai) — full guides, recipe reference, and migration docs
 - [Architecture](docs/DESIGN.md) — execution model and internals

Net effect: the README loses its only link to the docs site, the opposite of the stated intent. Either re-add the link near the top as described or restore the bullet.

Medium — README.md (line 4): license badge contradicts the PR description

The PR body says the badge row includes an "MIT License badge," but the added badge is Apache 2.0:

[![License: Apache 2.0](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE)

One of these is wrong. Verify the actual license and make the badge match; also confirm the badge's link target LICENSE matches the real filename (e.g., not LICENSE-MIT or COPYRIGHT), otherwise the badge link 404s.

Low — README.md (Install section): Windows .zip claim is unverified by this PR's own asset check

The new text advertises ".zip for Windows x86_64" release assets, but the PR body's asset verification (against .github/workflows/release.yml) enumerates only four must-<target>.tar.gz targets plus SHA256SUMS, with no Windows job mentioned. If the workflow doesn't build a Windows zip, the README documents assets that will never exist. Confirm against the workflow matrix or drop the Windows clause.

Low — install.sh (sha256_file, ~lines 57–72): checksum verification fails open

When neither sha256sum nor shasum is present, sha256_file prints a warning and returns 1, and verify_checksum then return 0, so the install proceeds with an unverified binary. This is a deliberate trade-off (and the README discloses that checksums only guard corruption), but for an installer it's safer to fail closed — abort with instructions to install coreutils/shasum — or at minimum make the skip opt-in. As written, a stripped-down minimal image silently gets no verification.

Low — install.sh (sha256_file): tool failure is masked and misreported

sha256sum "$1" | awk '{print $1}'

The function's exit status is awk's, not sha256sum's (POSIX sh has no pipefail). If sha256sum fails (unreadable file, I/O error), the branch still returns success with an empty digest, and the caller reports a misleading "SHA256 mismatch" instead of a tool error. Suggested fix — capture the sum separately and propagate status:

out=$(sha256sum "$1") || return 1
echo "${out%% *}"

(Same for the shasum branch.) It fails safe today, but the error message will confuse users debugging real failures.

Notes

  • The core fix (REPO="gregnazario/must") and the macOS shasum fallback are correct and needed; the fallback digest formats match what grep " ${BASENAME}$" / awk '{print $1}' expect.
  • The acknowledged follow-up (release.yml still hardcoding anomalyco/mustfile and an anomalyco/tap in the generated release body) is outside this diff, but since this PR's purpose is correcting the repo rename in install paths, folding that fix in would prevent the next tag from publishing broken instructions.
  • The README's temp-file-then-run one-liner is safer than the curl | sh form quoted in the PR body; no issue there. The rm only running on success is trivial and not worth changing.

Reviewed 5078491 with Pi 0.84.2 using zai/glm-5.3:high.

- Use a fetch-then-run install form so a missing release fails
  visibly instead of curl|sh exiting 0 on empty stdin
- Note the release prerequisite and that SHA256SUMS comes from the
  same channel (corruption guard, not tamper guard)
- Warn clearly and skip when no sha256sum/shasum exists instead of
  aborting with an obscure not-found error under set -eu
@gregnazario

Copy link
Copy Markdown
Owner Author

Addressed the review:

  • Windows asset (Medium) — not a finding: release.yml matrix includes x86_64-pc-windows-msvc (release.yml:35-36), builds must-<target>.zip via the Windows prepare step (lines 56-61), uploads it alongside the tarballs, and sha256sum *.tar.gz *.zip covers it in SHA256SUMS. README sentence stands.
  • curl | sh silent success (Medium) — valid; switched to a fetch-then-run form that propagates failure (curl -o /tmp/must-install.sh && sh ... && rm ...) and added a note that a published release is required.
  • sha256_file without tools (Low) — added an explicit guard: warns and skips verification instead of aborting obscurely under set -eu.
  • same-channel integrity note (Low) — added one line under the snippet stating checksums guard against corruption, not a tampered release.

The release-body hardcoding anomalyco/mustfile remains flagged as follow-up before the next tag.

The domain predates this repo (upstream mustfile artifacts) and is
not ours; in-repo docs under docs/ cover architecture and migration.
@gregnazario

Copy link
Copy Markdown
Owner Author

Correction to an earlier claim: I described adding "a prominent docs-site link near top" — that linked must.ai, which came in with upstream anomalyco/mustfile artifacts and is not owned by us. Removed the README links to it in 5e23e8d...; related repo debris that needs owner decisions instead of unilateral edits:

  • .github/CODEOWNERS → @anomalyco/mustfile-maintainers (review requests route there)
  • .github/SECURITY.md → security contact security@must.ai (vuln reports go off-domain)
  • issue template contact link (must.ai/docs)
  • site/mkdocs.yml site_url + deploy-docs.yml CNAME → must.ai

Repointing these needs real values (your handle, a security email, or removal), so flagging rather than editing.

@gregnazario

Copy link
Copy Markdown
Owner Author

License pieces settled: this branch's badge now reads Apache-2.0; the actual LICENSE file + corrected License section arrive via #4 (repo had none, and main's License section claimed MIT against a dangling link).

@gregnazario
gregnazario merged commit afba165 into main Aug 26, 2026
8 checks passed
@gregnazario
gregnazario deleted the docs/readme-polish branch August 26, 2026 20:39
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