test(adapters/detect): cover config-directory branch and env-var priority chain#287
Merged
mksglu merged 7 commits intomksglu:nextfrom Apr 16, 2026
Merged
Conversation
* feat: add Pi package manifest * docs: update Pi install instructions --------- Co-authored-by: Harvey Guo <harveyguo@HarveydeMac-mini.local>
…tes (mksglu#285) test(executor): cover background mode, cleanupBackgrounded, and hardCapBytes Three untested paths in PolyglotExecutor: - background: true returns partial output with backgrounded flag - cleanupBackgrounded() kills detached processes (zombie prevention) - hardCapBytes kills process when stdout+stderr exceeds byte limit
…rity chain The existing detect.test.ts covers env vars, clientInfo, and the CONTEXT_MODE_PLATFORM override, but leaves the medium-confidence config-dir branch (detect.ts:128-210, ~80 lines) completely untested. Adds a new file that mocks `node:fs.existsSync` via `vi.mock` and exercises each branch: - 7 direct-home dirs: ~/.claude, ~/.gemini, ~/.codex, ~/.cursor, ~/.kiro, ~/.pi, ~/.openclaw - 3 XDG dirs: ~/.config/kilo, ~/.config/opencode, ~/.config/zed - Fallback to low-confidence claude-code when no dirs exist - Priority locks: env var > config dir, CONTEXT_MODE_PLATFORM > config dir, ~/.claude > ~/.gemini ordering - Env-var priority chain: CLAUDE > GEMINI > OPENCLAW > KILO, CODEX > CURSOR Pi platform detection (now in-scope per mksglu#279, mksglu#283) gets its first dedicated test. 18 new assertions, 0 runtime changes, 40 existing detect tests unaffected.
Owner
|
Hi Thanks, are we sure PLATFORM_ENV_VARS variables is correct and exist in code-base? If it's, that's seems duplicate Strings in here. If not, how we even sure these keys are valid? @sebastianbreguel |
…rrogates, marker boundaries (mksglu#286) Adds 10 focused assertions to tests/truncate.test.ts covering gaps left by the mksglu#273 surrogate-pair fix: - 3-byte UTF-8 (CJK) cap sweep — fills the gap between ASCII and 4-byte emoji - ZWJ emoji sequence cap sweep — modern emoji with chained surrogate pairs - Lone low surrogate in input — round-trips safely through UTF-8 - Mixed-width input unchanged when cap >= input byte length - capBytes marker-boundary arithmetic (cap == markerBytes, cap == markerBytes+1) - escapeXML consecutive reserved runs + non-idempotent re-escape lock - truncateJSON exact-size and cap == size-1 boundaries All 29 tests in the file pass. Typecheck clean. Pure additions — no runtime behavior changes.
… of truth Addresses PR review feedback: the test file hardcoded a literal list of env-var names that duplicated the strings used inside detectPlatform(). If detect.ts renamed a var, the test's cleanup list would silently drift. - Export PLATFORM_ENV_VARS from src/adapters/detect.ts as the canonical (platform, envVars) priority table. - Replace the 8 per-platform if-blocks with a single loop over the table. Same priority order, same reason strings, same behavior. - Test imports PLATFORM_ENV_VARS and derives its cleanup list from it, so any future rename in detect.ts is automatically picked up.
Contributor
Author
|
Good catch — it was a local hardcoded array. Pushed c10aa6a: exports |
Owner
|
Good coverage work. The PLATFORM_ENV_VARS extraction is a clean DRY refactor and the config-dir mock pattern is solid. Pi detection having zero test coverage was a real gap. Merging. |
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
3 participants
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.
What
Adds
tests/adapters/detect-config-dir.test.ts— 18 new assertions covering the medium-confidence config-directory branch of `detectPlatform()` and the env-var priority chain.Why
`src/adapters/detect.ts` has three detection tiers: env vars (high), config dirs (medium), fallback (low). The existing `tests/adapters/detect.test.ts` covers high-confidence detection thoroughly (env vars, clientInfo, CONTEXT_MODE_PLATFORM override, adapter dispatch) but leaves the medium-confidence branch at lines 128-210 — roughly 80 source lines — completely untested. Any reorder or removal of a config-dir check could regress platform detection for users who rely on dir-based auto-detect.
This is particularly relevant now that Pi is in-scope (per #279, #283 merged today) — its only detection path is `~/.pi/`, and until this PR it had zero test coverage.
Coverage added
Config directory branches (11 tests)
Mocks `node:fs.existsSync` via `vi.mock` to force each path.
/.claude`, `/.gemini`, `/.codex`, `/.cursor`, `/.kiro`, `/.pi`, `~/.openclaw`/.config/kilo`, `/.config/opencode`, `~/.config/zed`/.claude` > `/.gemini`, env var > config dir, `CONTEXT_MODE_PLATFORM` > config dirEnv-var priority chain (4 tests)
Locks the ordering in `detect.ts`:
The existing file only tested cursor > vscode-copilot priority.
Test plan
Pure additions, no runtime behavior changes.