fix(kimi): key transport on the resolved binary, not ~/.kimi-code existence#101
Merged
Merged
Conversation
…stence Follow-up to v0.8.61 (#99). chooseKimiTransport decided "drive kimi directly vs route to opencode-go" on `fs.existsSync(~/.kimi-code)`, but dispatch resolves the binary by name and detection resolves a concrete path — these disagree when BOTH kimi builds are installed. If a ~/.kimi-code dir existed but PATH resolved `kimi` to an unconfigured legacy Python build, v0.8.61 drove it directly → "LLM not set", instead of falling back to opencode-go. - New `isNativeKimiBinary(homeDir, binPath)` realpaths BOTH the binary and the ~/.kimi-code dir before a lexical containment check, so a symlinked binary (/usr/local/bin/kimi → ~/.kimi-code/bin/kimi) OR a symlinked ~/.kimi-code dir both compare in the same namespace. Returns undefined when the path is unknown so the caller falls back to dir existence. - chooseKimiTransport now takes a tri-state `isNativeBuild?: boolean`, staying fs-light/unit-testable. `false` (resolved, non-native) skips the dir shortcut and falls to the config probe — the dual-install fix. - isPathUnder: `..` guard now matches a real parent ref only, not a child segment that merely starts with dots (e.g. `..helpers/bin`). Tests: +15 (isPathUnder lexical, isNativeKimiBinary incl. symlinked-binary and symlinked-dir cases, tri-state precedence). Two chorus self-review rounds: round 1 request_changes (realpath asymmetry) → fixed → round 2 approved by all 7 reviewers. Note: a fallback-detected kimi whose dir isn't on the spawn PATH still ENOENTs (runHeadless spawns bare `kimi`) — pre-existing, tracked separately.
This was referenced May 31, 2026
Merged
chorus-codes
added a commit
that referenced
this pull request
Jun 1, 2026
…104) (#105) Detection (cli-detect) resolves a concrete absolute path (PATH lookup → fallback-dir scan → manual override), but headless spawns re-resolved by bare name against the daemon PATH. The two could disagree: - ENOENT: a CLI found only via the fallback-dir scan (its dir not on the spawn PATH) couldn't be launched by bare name. - shadowing: two same-named binaries (e.g. ~/.kimi/bin/kimi vs ~/.kimi-code/bin/kimi) — bare-name spawn could run a different build than detection (and the #101 transport decision) resolved. Add resolveCliBinaryPath(command): bare known CLI name → detection's verified absolute path, else passthrough (already-a-path, unknown name, or not-found → preserves the existing PATH/`where` fallback). - headless.ts:resolveBinaryPath calls it first — covers the 6 direct shims (claude/codex/gemini/kimi-cli/grok/agy) and Windows opencode. - opencode.ts + kimi.ts resolve opencode before wrapWithPty — the PTY wrapper embeds the binary in argv, so the spawn core only sees `script` and can't resolve it (covers the POSIX PTY path). No behaviour change when detection has no path: the bare name flows to the unchanged Unix-PATH / Windows-`where` resolution. Tests: resolver unit tests against real staged detection; headless spawn integration tests for the abs-path case (Linux + Windows) plus the detection-not-found fallback. Full suite 1033 passing. Co-authored-by: chorus-codes <280607145+chorus-codes@users.noreply.github.com>
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.
Follow-up hardening to v0.8.61 (#99) for the two-kimi situation (#98).
The gap
chooseKimiTransportdecided "drivekimidirectly vs route through opencode-go" purely onfs.existsSync(~/.kimi-code). But dispatch resolves the binary by name (runHeadlessspawns barekimi) and detection resolves a concrete path — these disagree when both kimi builds are installed. If a~/.kimi-codedir existed but PATH actually resolvedkimito an unconfigured legacy Python build, v0.8.61 drove that binary directly →kimi --printexits 1 "LLM not set", instead of falling back to opencode-go.The fix
isNativeKimiBinary(homeDir, binPath)— realpaths both the binary and the~/.kimi-codedir before a lexical containment check, so it's correct whether the symlink is on the binary (/usr/local/bin/kimi→~/.kimi-code/bin/kimi) or on the directory (~/.kimi-code→/opt/kimi-code). Returnsundefinedwhen the path is unknown → caller falls back to dir existence.chooseKimiTransportnow takes a tri-stateisNativeBuild?: boolean(instead of a raw path), staying fs-light and unit-testable.false(resolved, non-native) skips the dir shortcut and falls to the config probe — the dual-install bug fix.isPathUnder—..guard now matches a true parent ref only (../../…), not a child segment that merely starts with dots (..helpers/bin).Tests (+15)
isPathUnderlexical cases;isNativeKimiBinaryfilesystem cases incl. a symlinked binary on PATH and a symlinked~/.kimi-codedir (the asymmetry case); tri-state precedence. Full suite 1020 passed, typecheck clean, no new lint.Review
Two chorus
review-onlyrounds. Round 1 →request_changes(3 reviewers caught a realpath asymmetry: the binary was realpathed but the dir stayed lexical, false-negativing a symlinked~/.kimi-code). Fixed by resolving both sides inisNativeKimiBinary. Round 2 → approved by all 7 reviewers.Known, scoped out
A fallback-detected
kimiwhose install dir isn't on the daemon's spawn PATH still ENOENTs, becauserunHeadlessspawns barekimi. This is pre-existing (true on v0.8.61, and for opencode/grok with non-standard install dirs) and not regressed here. The deeper fix — spawn the detected path — is a separate follow-up.