fix(0.2.1): surface npm install failures via marker file#133
Open
fix(0.2.1): surface npm install failures via marker file#133
Conversation
Closes the silent-install path flagged in the launch-readiness
review. Pre-fix:
$ npm install -g mapterrain
> postinstall fails (cosign missing)
> [warn] one-line stderr message
> exit 0
$ terrain analyze
> silently retries the same fetch
> fails with the same error
> user is confused about why "install" reported success
Two acceptable resolutions were on the table; the plan chose Option
B (lazy-with-loud-signal) over Option A (hard-fail npm install).
Hard-failing every cosign-missing host would be more disruptive than
the failure mode itself; CI pipelines that wrap `npm install` would
break for legitimate reasons. The right user experience is: install
"succeeds" with a loud, framed warning, and the *first* `terrain`
invocation refuses to retry silently — printing the original error
verbatim with concrete remediation.
Implementation:
* `bin/postinstall.js` writes ~/.terrain/install-failure.log with
the captured error (timestamp, platform, version, message,
stack) when ensureTerrainBinary throws. The stderr warning is
upgraded from one line to a multi-line framed banner so it's
hard to miss in a CI log.
* `bin/terrain-installer.js` exports `writeInstallFailureMarker`
and `clearInstallFailureMarker`. `runTerrainCli` (the trampoline
for `terrain ...`) consults the marker before retrying the
fetch; if a marker exists AND no installed binary is present,
it throws the recorded error with a structured remediation
block instead of attempting another silent retry.
* On a successful install or successful first run, the marker is
cleared so future invocations don't see stale state.
Tests added (`scripts/test-installer-marker.mjs`, run via
`node --test`):
* writeInstallFailureMarker captures the error fields
* clearInstallFailureMarker removes the marker
* clearInstallFailureMarker is idempotent
Wired into `npm test` as a `test:unit` step so it runs ahead of
`scripts/verify-pack.js` in the existing release-verify chain.
CHANGELOG note in PR #131 already calls this out as 0.2.1 work; a
follow-up PR can promote the entry from "known issue" to "fixed".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
[RISK] Terrain — Merge blocked
Coverage gaps in changed code
Owners: PMCLSF Limitations
Generated by Terrain · Targeted Test ResultsNo tests selected — change affects only non-code files. |
Terrain AI Risk Review
Decision: PASS — AI surfaces are covered. |
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
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.
Summary
Closes the silent-install path flagged in the 0.2.0 launch-readiness review (Item 5: postinstall.js swallows install failure). Pre-fix,
npm install -g mapterraincould exit 0 with the binary missing; the user only discovered it minutes later with a confusing retry loop.Approach
Option B (lazy with loud signal) per the implementation plan. Hard-failing every cosign-missing host would be more disruptive than the failure mode itself.
bin/postinstall.jswrites~/.terrain/install-failure.logwith the captured error and prints a multi-line framed warning instead of a single line.bin/terrain-installer.jsexportswriteInstallFailureMarker/clearInstallFailureMarker.runTerrainClichecks the marker before retrying the fetch and throws a structured remediation block if a failed install was recorded.User-visible flow after this PR
Test plan
scripts/test-installer-marker.mjs(Nodenode:test) — write/clear/idempotencynpm testvia newtest:unitstepnpm run release:verifygreen (format + lint + tests + verify-pack)Plan link
/Users/pzachary/.claude/plans/kind-mapping-turing.md(Phase 3.5).🤖 Generated with Claude Code