fix(fm-primary): resolve installed-shim symlinks to the tracked repo root before launch - #5
Merged
Conversation
4 tasks
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.
Intent
Fix the supported installed 'firstmate ' launcher so invocation through the generated symlink resolves and launches from this tracked Firstmate repository instead of treating the shim directory as the repository root. The exact captain-facing acceptance path is 'firstmate kimi': it must expand to the primary-only Kimi K3 profile and reach its normal primary launch path, without adding Kimi worker support. Preserve unchanged direct bin/fm-primary.sh behavior, safe refusal of unrelated existing shim files or symlinks, and exact-link idempotent installation. Keep root-resolution mechanics owned by the launcher header/help, cover actual generated-shim invocation with isolated temporary state and fake CLI seams, test a relative chained symlink as disconfirming evidence, review all supported primary profiles, and validate with tests/fm-primary.test.sh plus bin/fm-lint.sh.
What Changed
bin/fm-primary.shnow dereferences up to 40 absolute or relative symlink hops from the invoked command (via a newresolve_script_pathhelper) before computingSCRIPT_DIR/FM_ROOT, so running the installedfirstmate <profile>shim resolves and launches from the tracked Firstmate repository root instead of the shim directory; directbin/fm-primary.shinvocation behaves as before, and the launcher header comment documents the new resolution step.tests/fm-primary.test.shextends the shim-safety group to invoke the actual generated shim:firstmate kimithrough the installed symlink must report the tracked root, expand to thekimi-k3profile, and reach the pinned Kimi primary argv; a relative chained symlink must also resolve the tracked root; and shim reinstall through the installed command must stay idempotent.readlink "$source"could add a--guard for stylistic consistency with the script's other hardened calls, though the case it guards is unreachable.Risk Assessment
✅ Low: A small, standard bounded symlink-resolution fix confined to one launcher script, preserving direct-invocation and shim-install behavior, with new tests that exercise the actual generated shim, the Kimi acceptance path, and a relative chained symlink exactly as the stated intent requires.
Testing
Ran the focused fm-primary suite (all green, including the new shim and chained-symlink tests) on top of the already-green full baseline, then manually demonstrated the captain-facing acceptance path: installed the shim into isolated temp state and ran
firstmate kimithrough it with fake CLI seams, showing it resolves the tracked repo root, expands to kimi-k3, and reaches the normal primary launch path — while the base-commit launcher fails through the same symlink. Also swept all six profiles plus aliases through the shim and re-verified installer safety refusals; bin/fm-lint.sh was skipped per the no-linters testing rule and is handled by the pipeline's lint stage.Evidence: firstmate kimi via installed shim (install, dry-run, real exec, idempotent reinstall)
$ cd $DEMO/somewhere-else && firstmate kimi root=/Users/larsmusic/.no-mistakes/worktrees/f569cc43ac96/01KXWZRJW50GZ3DPS7WT7AJT3Y profile=kimi-k3 role=FIRSTMATE KIMI_CODE_HOME=$DEMO/home/data/primary/kimi-k3 'kimi' '--model' 'kimi-code/k3' '--yolo' --- without dry-run seam --- FAKE-KIMI launched: pwd=<tracked repo root> KIMI_CODE_HOME=$DEMO/home/data/primary/kimi-k3 argv=--model kimi-code/k3 --yoloEvidence: Disconfirming evidence: base-commit launcher fails via symlink; fixed launcher resolves relative chained symlink
old launcher (6cf9e61) via symlink: fm-primary: could not inspect the Firstmate session lock: ... $DEMO/bin/fm-lock.sh: No such file or directory exit status: 1 fixed launcher via relative chained symlink (../bin/firstmate): root=<tracked repo root> profile=kimi-k3 'kimi' '--model' 'kimi-code/k3' '--yolo'Evidence: All supported primary profiles through the installed shim + installer safety refusals
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-primary.sh:64-readlink "$source"omits the--end-of-options guard that the surrounding code consistently uses (dirname --,cd -P --). Only the first iteration could ever see a path starting with-(later hops are always absolute), so this is unreachable in practice, but adding--would make the resolver internally consistent with the script's hardening style.✅ **Test** - passed
✅ No issues found.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline configured test command: alltests/*.test.sh(ran green before this session)bash tests/fm-primary.test.sh— all 10 test groups pass, including new installed-shimfirstmate kimi, relative chained symlink, and shim-reinstall idempotency coverageManual e2e:bin/fm-primary.sh --install-shiminto an isolated temp dir, thenfirstmate kimivia PATH from an unrelated cwd with fakekimiCLI seam — dry-run seam showed root=<repo>, profile=kimi-k3, pinned argv; non-dry-run pass exec'd the fake kimi CLI from the tracked repo root with the managed KIMI_CODE_HOMEManual disconfirming check: base-commit (6cf9e61) launcher invoked through the samefirstmatesymlink resolves the shim dir as root and fails with missing bin/fm-lock.sh; fixed launcher via relative chained symlink resolves the tracked rootManual sweep: all supported profiles (pi, claude-fable, codex, opencode, grok, kimi-k3) plus aliases (claude, kimi) launched through the installed shim, each resolving the tracked root with exact pinned argvManual safety check: installer refuses an unrelated existing file and an unrelated symlink at the shim path, and reinstall through the installed command stays idempotentbin/fm-lint.shwas NOT run (testing step forbids linters; covered by the pipeline's lint stage)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.