fix(install): build before replacing the working install - #118
Merged
Conversation
A failed install left the machine with no CLI at all. do_install ran `rm -rf "$SRC_DIR"` and only then built; if the build failed, or the run was interrupted, what remained was an unbuilt tree, a wrapper still pointing at the dist/ that was never produced, and the previous run's install.json still claiming success. Every later `logicsrc` invocation died with MODULE_NOT_FOUND, and nothing said why. That is what happened here: install.json dated 01:57, src/ replaced at 02:59 by a second run that did not finish. Now the download, npm install and build all happen in a staging directory, and $SRC_DIR is only touched once packages/cli/dist/index.js actually exists -- the file the wrapper execs, so its absence is exactly the failure the user would otherwise hit on their next command. Staging sits inside $LOGICSRC_HOME so the swap is a rename on one filesystem rather than a cross-device copy of node_modules, and the previous tree is kept until the swap succeeds so a failed move can be undone. Build output was going to /dev/null, so "build failed" carried no reason at all. It is captured now, with the last 25 lines printed on failure and the full log left on disk. Also validates the commit id from the GitHub API before recording it: anything that is not 40 hex characters is dropped rather than written into install.json, which `logicsrc update` compares against. Verified against a stubbed npm/curl in all three paths: a failing build leaves the existing install running, a build that produces no artifact is caught, and a clean install still swaps in and writes a correct manifest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A failed install left the machine with no CLI at all.
What happened
do_installdestroyed the existing install before building:If the build failed — or the run was interrupted, or the machine slept — what remained was an unbuilt tree, a wrapper still pointing at the
dist/that was never produced, and the previous run'sinstall.jsonstill claiming success (it lives besidesrc/, sorm -rf "$SRC_DIR"doesn't clear it).Every later invocation died with no explanation:
That's exactly what occurred on this machine:
install.jsondated 01:57,src/replaced at 02:59 by a second run that never finished.Fix
Download,
npm installand build all happen in a staging directory.$SRC_DIRis only touched oncepackages/cli/dist/index.jsexists — the file the wrapper execs, so its absence is precisely the failure the user would otherwise discover on their next command.$LOGICSRC_HOME, so the swap is a rename on one filesystem rather than a cross-device copy ofnode_modules.mvis undone.INT/TERM/HUPclean up staging.Build output is no longer discarded.
"build failed"with no reason isn't a diagnosis — the last 25 lines are printed and the full log is left on disk.The commit id from the GitHub API is validated before being recorded. Anything that isn't 40 hex characters is dropped rather than written into
install.json, whichlogicsrc updatecompares against. (An error page or proxy HTML was previously stored verbatim as the commit.)Verification
Stubbed
npm/curl, all three paths:npmsucceeds but emits no artifactdist/index.jscheck; install survivessh -nanddash -nboth clean.🤖 Generated with Claude Code