feat: record how and where install.sh installed, for rook update

[samyakLambda]

What

install.sh records how and where it installed, in
install-manifest.json inside the versioned install tree:

{
  "v": 1,
  "channel": "curl",
  "installDir": "/Users/x/.local/bin",
  "version": "0.1.0"
}

Plus scripts/test-install-manifest.sh, which runs the real, unmodified
install.sh against a synthetic release and asserts the record.

Nothing about installing changes. This is one file write at the end of a
successful install.

Why

rook update (LambdatestIncPrivate/rook#554) has to know how the running copy
was installed before it can update it, and it cannot work that out from the
binary's own path. The tarball's launcher resolves its own symlink chain and
execs the deep entry, so by the time any code runs, the directory this
script chose is already gone. An npm --prefix can point at ~/.local too,
so the inference fails in both directions.

installDir is the point, more than the channel name. This script links
$INSTALL_DIR/rook (install.sh:232), and --dir moves it. Re-running the
installer without the recorded directory writes a second symlink into the
default ~/.local/bin while the user's real one still points at the old
version — an update that reports success and changes nothing on PATH. Only
this script knows where it put the link, so only this script can record it.

Three details that are load-bearing

The path is resolved after mkdir, not at argument-parse time. The target
need not exist when --dir is parsed, and ~/.local/bin does not exist on a
fresh machine — so cd, realpath and readlink -f would all fail there and
set -euo pipefail would turn a working install into a non-zero exit that
installs nothing.

This is worth calling out because resolving at parse time passes every
existing harness. All of them pre-create the install directory and all of
them pass --dir (test-install-fixture.sh:168,222,308,335,379). A fresh
curl … | bash is exactly the case none of them cover, and it is the
documented install path. Case A of the new harness is that install.

It is written last, after ln -sf. Its presence then means an install that
actually reached PATH; written first, it would survive a failed symlink and
describe an install nobody can run. A failure writing it warns rather than
aborting — the tree and the symlink are already in place and working, and a
missing manifest degrades rook update to printing the command instead of
running it, which is the safe direction.

Control characters are refused, backslash and double-quote escaped. This is
bash with no JSON encoder. A legal --dir '/tmp/a"b' would otherwise write a
document that will not parse, which the reader treats as absent — leaving that
install silently never updating, with no error anywhere.

The harness, and how it differs from test-install-fixture.sh

Same seam (a stub curl earlier on PATH serves fixture files by basename,
so the real download → verify → extract → symlink pipeline runs), two
deliberate differences:

1. It does not always pass --dir, and does not pre-create the directory —
   see above.
2. Its fixture tarball carries lib/node_modules/@lambdatestincprivate/rook,
   the scope a real public tarball has. test-install-fixture.sh uses
   @testmuai/rook, which is fine for what that file asserts, but @testmuai
   is the published package's name — the npm rename applies only to the
   published copy, not to the tarball — and it is the wrong side of the
   discriminator rook update uses to tell a curl install from an npm one.
   It also writes VERSION at the tarball root, which the real one has and
   which rook update reads to confirm an update landed.

Six cases, 20 assertions: default --dir on a machine where it does not exist,
explicit --dir not pre-created, a relative --dir recorded absolute, a
double quote in the path, a control character refused, and the write ordered
after the symlink.

Evidence

Stash-verified: git stash push install.sh → 14 of the harness's assertions
fail; restored → all 20 pass.

All seven scripts/test-*.sh pass, including the existing
test-install-fixture.sh against the modified install.sh. shellcheck is
clean on both files.

One thing the harness found rather than me: expectations have to compare
against the physical path (pwd -P), because macOS puts $TMPDIR behind
/private. Comparing against the unresolved form is green on Linux CI and red
on a maintainer's laptop.

Not in this PR

• rook update itself, which lives in the private CLI repository.
• Pruning older ~/.testmuai/rook-<version> trees — pre-existing, filed as
  install.sh never prunes older ~/.testmuai/rook-<version> trees #14. An automated update path makes it faster rather than causing it.

[samyakLambda]

Review

Ran an independent review of this diff (install.sh manifest write + scripts/test-install-manifest.sh). The core safety property — a bash script with no JSON encoder guarding against control characters — is more porous than its own tests prove; two of the findings below were empirically reproduced against this branch, not just reasoned about.

Findings

1. [HIGH] The control-character guard runs before $INSTALL_DIR is resolved to a physical path; the manifest embeds the value after resolution — so a control character introduced by the resolution step bypasses the guard entirely.
install.sh:243 (INSTALL_DIR="$(cd "$INSTALL_DIR" && pwd -P)") runs after the guard at :59-62. Reproduced:

mkdir -p $'/tmp/x\ty'; cd $'/tmp/x\ty'; install.sh --dir mybin

mybin is clean and passes the guard. The resolved INSTALL_DIR then carries the tab from the CWD. Script exits 0, install-manifest.json (:285) contains a raw control byte and fails to parse (Invalid control character) — this defeats the exact property the existing Case E test was written for.

2. [HIGH] $version has no control-character guard at all.
$INSTALL_DIR is checked; $version (from --version "$2" at :40, through resolve_version(), into the same json_escape()-guarded heredoc at :285) is not. Reproduced:

install.sh --version $'9.9.9\tinjected'

Exits 0, writes a manifest with a raw tab in the version field — json.load fails (Invalid control character at: line 1 column 146). json_escape()'s own doc comment (:146-147) claims control characters are "refused up front (see the check above)" — that check only ever inspects $INSTALL_DIR.

3. [MED] Confirmed regression: reusing the resolved physical path for the (unchanged) $PATH-membership check produces a spurious "Add to your PATH" warning.
:295 reuses the newly physical-resolved $INSTALL_DIR for the pre-existing grep -qx "$INSTALL_DIR" check. On macOS (/tmp → /private/tmp), with /tmp/mybin already on $PATH: install.sh --dir /tmp/mybin resolves to /private/tmp/mybin, which doesn't match the literal /tmp/mybin in $PATH, so the script now prints "Add to your PATH" for a directory that's already reachable. Verified this warning did not print against the pre-PR script for the identical setup.

4. [MED] Case F's ordering check doesn't structurally verify what it claims.
scripts/test-install-manifest.sh:350 derives MANIFEST_LINE via grep -n 'install-manifest.json' | tail -1, which resolves to the warning-echo line (:288), not the actual write (:284). It only passes today because both happen to sit after ln -sf, not because it verifies the real write's position — a future refactor moving the actual write earlier while leaving any mention of the filename after ln -sf would make this test falsely pass on the exact ordering regression (a manifest surviving a failed symlink) it exists to catch.

5. [LOW] No UTF-8 validation — a third unguarded byte class.
Neither the control-character guard nor json_escape() validates UTF-8, but RFC 8259 requires JSON text be valid UTF-8. A stray non-UTF-8 byte in the install path (plausible on Linux, where filenames are arbitrary byte strings) passes both untouched and produces unparseable JSON via the same silent-degrade path as findings 1/2.

6. [LOW] Non-atomic manifest write. cat > file <<HEREDOC (:284) has no temp-file-plus-rename. A write interrupted mid-flight (disk full, signal, power loss) can leave a manifest that is present but not valid JSON — a third state beyond the absent/present binary the design's safety reasoning relies on. Whether the downstream reader (rook update, the private repo) treats a parse failure the same as absent isn't verifiable from this diff alone — worth confirming there.

7. [LOW] Sourcing install.sh for detect_platform() unconditionally runs the control-character guard against the ambient $HOME.
The guard (:59) is unguarded top-level code, not gated behind the BASH_SOURCE main-guard at the bottom — all three test harnesses source this file purely to reuse detect_platform(). HOME=$'/tmp/fakehome\tx' bash -c 'set --; source install.sh' exits 1 before detect_platform is even defined; inside the harnesses' source ... >/dev/null 2>&1 this is swallowed, producing a misleading "FATAL: ... is this host unsupported?" instead of the real cause.

8. [LOW, doc accuracy] Comment overclaim. :276's "its presence means an install that actually reached PATH" is contradicted by the script's own next lines (:295-297, unchanged), which explicitly handle the case where $INSTALL_DIR is not on $PATH. Worth a wording fix.

Test-harness quality notes (not correctness bugs)

• The stub-curl heredoc and checksum-tool detection in scripts/test-install-manifest.sh:98 are copied verbatim from scripts/test-install-fixture.sh (a third near-identical copy exists in scripts/test-runtime-version-poll.sh) — worth extracting to a shared helper so a future fix doesn't need applying in 2-3 places.
• Six check() call sites (e.g. :213-214, :217-218, :221-222) call manifest_field() twice per assertion (once for the message, once for the predicate) instead of caching the value in a local the way the file itself does correctly two cases later (:267/269, :278/281) — an extra python3 spawn per assertion on the normal green-CI path.
• The header comment calls out the @lambdatestincprivate npm scope and the root-level VERSION file as load-bearing fixture details, but no check() actually asserts either survives install.sh's extract/copy step — a future filter/copy change could silently drop them with the harness staying green.

---

Findings 1, 2, and 3 were empirically reproduced against this branch in a scratch worktree; happy to share the exact repro commands/output if useful.

[samyakLambda]

Pushed 08a583c addressing every finding from the review above.

#	Finding	Fix
1 (HIGH)	control-char guard only checked --dir pre-resolution; pwd -P could fold one in	added a second check on the resolved physical path
2 (HIGH)	--version had no control-char guard at all	now goes through the same guard as --dir
3 (MED, regression)	PATH-membership check compared the resolved path against $PATH, producing false "add to your PATH" warnings for dirs on PATH via an unresolved/symlinked form	now compares the raw, pre-resolution value
4 (MED)	Case F's grep matched the warning-echo line, not the actual manifest write	now matches the write (temp-file rename) specifically
5 (LOW)	no UTF-8 validation	folded into the same guard as the control-char check, via iconv (skipped gracefully if absent)
6 (LOW)	manifest write wasn't atomic	now temp-file-then-mv in the same directory
7 (LOW)	sourcing install.sh for detect_platform() (all 3 test harnesses do this) unconditionally ran arg-parsing + the guards against the ambient environment	arg parsing and both guards moved inside main()
8 (LOW)	doc comment overclaimed the manifest's presence means PATH was reached	reworded

Also added Cases G/H/I to scripts/test-install-manifest.sh — each positively proves its guard fires, including G, which only reproduces post-fix (a control character introduced by resolving a relative --dir, invisible to the old pre-resolution-only check).

Verified: test-install-manifest.sh (29/29), test-install-fixture.sh (32/32), test-platform-detect.sh, test-runtime-version-poll.sh all green; shellcheck clean on all three touched files. Independently re-ran all of the above myself rather than trusting the fix pass's own report, plus reproduced #3 and #7 directly (HOME=$'/tmp/fakehome\tx' bash -c 'source install.sh' now sources cleanly instead of erroring).