fix: stop the bleeding — install.sh installed the wrong package, deps unpinned, releases unverified - #7
Conversation
install.sh ran `pipx install mem-cli`, which resolves to a different project on PyPI (mem-cli 0.1.1, an unrelated Textual CLI by another author). Anyone following the documented curl|bash install got a stranger's code instead of mem. The script URL also pointed at branch `main`, which 404s — this repo uses `master` — so the published one-liner never worked at all. Also assert the resulting `mem` on PATH is ours before reporting success, so a name collision fails loudly instead of silently.
All four dependencies were unpinned. Click 9 removes `Context.protected_args`, which the implicit-search router in MemGroup.invoke depends on, so a routine `pip install -U` breaks the entire CLI. The 87 DeprecationWarnings in the test suite are the advance notice. apple-fm-sdk is capped below 0.2 because 0.2.x reworked the SDK around multimodal attachments and has not been validated against the guided-generation path in patterns.py.
pyproject declares >=3.10 but CI only ran 3.11 and 3.12, leaving the oldest and newest supported versions untested. Adds a build job that runs twine check --strict and installs the wheel into a clean venv to confirm the console script is ours — the same class of failure that shipped the mem-cli/cli-mem mixup, caught at PR time instead of after publication.
A PyPI release cannot be replaced once uploaded, only yanked, so every check that can fail now runs before twine upload: twine check --strict and a wheel smoke test asserting the installed binary reports the tagged version. The Homebrew tap step used `curl -sL` without -f, so an HTTP error page would be piped into shasum and published as the formula checksum. Now fails on HTTP errors, retries, and rejects any sha256 that isn't 64 characters.
.claude/ and .specify/ are listed in .gitignore but 20 files were committed before the rule existed, so the ignore never applied. Files stay on disk.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07de18aa27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| env: | ||
| TWINE_USERNAME: __token__ | ||
| TWINE_PASSWORD: ${{ secrets.PYPI_TOKEN }} | ||
| run: twine upload --skip-existing dist/* |
There was a problem hiding this comment.
When an already-published tag is force-moved—the exact scenario the new concurrency block claims to support—--skip-existing continues after finding an existing PyPI artifact rather than verifying identical content (Twine upload documentation), while the later gh release upload --clobber overwrites same-named GitHub assets (GitHub CLI manual) and Homebrew is updated to the moved tag's tarball. The rerun can therefore leave PyPI serving the old code while GitHub and Homebrew serve new code under the same version; abort or compare artifact hashes when that version already exists instead of silently skipping it.
Useful? React with 👍 / 👎.
CI went red on this very PR with 44 lint errors while the same commit was clean locally: ruff 0.16.0 shipped and enabled BLE001, S110 and PLW1510 by default. The dev extra said `ruff>=0.6`, so every CI run was resolving whatever had been released that day. This is the same defect this branch fixes for runtime dependencies, and it bites harder for a linter, where new releases routinely add rules that turn an untouched branch red. Adopting 0.16 is worth doing — its BLE001/S110 rules land squarely on the `except Exception: pass` in cli.py:262 that hid the dead background sync for four months — but it needs per-site judgement rather than a blanket fix, since the broad catch in storage.py:97 (skip a corrupted JSONL line) is correct as written. Tracked as a TODO for the test branch; not smuggled into a distribution-only PR.
checkout@v4 and setup-python@v5 target Node 20, which GitHub already forces onto Node 24 with a deprecation warning on every run and will eventually stop running altogether. Bumped to checkout@v7 and setup-python@v7 so the pipeline breaks on a deliberate upgrade rather than on GitHub's timetable.
|
CI is green across Two follow-up commits after the initial push, both worth calling out because
Adopting 0.16 is genuinely worth doing rather than ducking: its new
One latent trap found while verifying, not fixed here: |
The pull_request trigger was filtered to 'branches: [master]', so a PR targeting any other base got no CI whatsoever. The overhaul is delivered as a stack of sequential PRs, each based on the previous one, which means every PR after the first was silently unverified. Confirmed: the test-harness PR produced zero workflow runs.
1 participant
First branch of the v0.5.0 overhaul. Zero product code — everything here is
distribution and CI. Split out because these three problems are causing harm
right now, on
master, to anyone who installs mem today.What was wrong
install.shinstalled someone else's package. The script ranpipx install mem-cli. That name belongs to an unrelated project on PyPI(
mem-cli0.1.1, "Interface CLI interactive avec Textual et Click", byGuillaume Lefebvre). This project publishes as
cli-mem. Anyone followingthe documented
curl | bashinstall got a stranger's code.The script's own URL also pointed at branch
main, which returns 404 — thisrepo uses
master. Verified:Dependencies had no upper bounds. Click 9 removes
Context.protected_args, whichMemGroup.invokedepends on for the implicitsearch router — so
pip install -U clickbreaks the whole CLI. The 87DeprecationWarnings in the test suite are the advance notice for exactlythis.
Releases were validated after publishing.
twine uploadran with notwine check, no smoke test, and no--skip-existing. A PyPI release cannotbe replaced once uploaded, only yanked. Separately, the Homebrew tap step used
curl -sLwithout-f, so an HTTP error page would be piped intoshasumand published as the formula checksum.
What changed
install.sh: correct package name, correct branch, and a post-installassertion that the
memon PATH is actually ours — a name collision nowfails loudly instead of reporting success.
pyproject.toml:click>=8.1,<9,rich>=13.7,<15,pydantic>=2.5,<3.apple-fm-sdkcapped below 0.2 because 0.2.x reworked the SDK aroundmultimodal attachments and is unvalidated against the guided-generation path.
ci.yml: matrix widened to 3.10–3.13 (pyproject claims>=3.10but only3.11/3.12 were tested),
ruff format --check, and a build job that runstwine check --strictand installs the wheel into a clean venv to confirmthe console script.
release.yml: all validation moved ahead oftwine upload;-fand retrieson the tarball fetch; sha256 length guard; idempotent GitHub Release step;
concurrencygroup..claude/and.specify/that.gitignorealready covers but that predate the rule. Files stay on disk.
Verification
Noted, not fixed here
.specify/memory/constitution.md— the document that governs this project —is not tracked by git. Only the template is. It exists on one machine and
is gitignored. Flagged for the docs branch, since deciding whether the
constitution belongs in the repo is a governance call, not a cleanup.