feat: autodetect installed client tools in the npx install wizard#76
Merged
Conversation
The interactive `npx hypaware` picker now inspects the system and pre-selects sources whose tool is present, instead of opening with everything unchecked. It also defaults the export to local-parquet, aligning the interactive path with the documented `--yes` default. Autodetect covers the two client sources only (claude, codex) — the raw proxy sources and otel are manual integration modes with nothing installed to detect, so they are never auto-checked. Detection is a pre-check the user can freely undo; it only seeds initial checkbox state. It runs only on the interactive branch — when picks are supplied (`--yes` / `--dry-run` / presets) selection stays explicit and deterministic. - detect.js: detectClientSources stats each client's config-home dir (~/.claude, $CODEX_HOME ?? ~/.codex), reusing resolveClientSettingsPath for env-home overrides. Stats the directory, not settings.json, so the adapter writing settings.json on attach never self-triggers. - client_settings_path.js: extracted the pure resolver so the detector reuses it without status.js's heavier import graph; status.js re-exports it to keep existing import sites stable. - walkthrough.js: pre-check detected sources (with a "· detected" label), pre-check local-parquet, thread `checked` through the TUI multiselect, and record sources_detected on the walkthrough.start span. - CONTEXT.md: glossary for client source vs raw proxy source, autodetect vs default. Tests: detector unit tests (present/absent/both/empty, $CODEX_HOME, file-not-dir) and picker preselection tests (labels + export default, nothing-detected, picks-skip-detection). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Dual-agent review —
|
| Source | Finding (severity, evidence) | Intersects |
|---|---|---|
| Codex | Error Handling & Resilience (minor) — detection failures silently treated as "nothing detected"; detect.js:61-66, walkthrough.js:646-650 |
New feature surface (detect.js); best-effort by design; low blast radius |
| Codex | Concurrency/Ordering (minor) — parallel Set writes make detected_sources order nondeterministic; detect.js:57-64 |
Telemetry attribute only; no functional blast radius |
| Codex | Fix-validation INCOMPLETE — local-parquet export default applied only in TUI path; legacy HYP_NO_TUI=1/non-TUI still defaults keep-local; walkthrough.js:696-704,475-477 |
Partial delivery of a stated PR goal; non-TUI path only |
Notes
- Change is additive and well-tested (detect.test.js, walkthrough-detect.test.js, walkthrough-tui-happy.test.js); detection is failure-safe; non-interactive/preset flows explicitly unchanged; status.js is a verified extract+re-export keeping existing import sites stable.
- No reviewer finding lands on a high-blast-radius shared surface; all sit on the feature's own new code or on telemetry attributes.
Codex review
Fix Validations
Autodetect is skipped for explicit/non-interactive picks
- Status: correct
- Evidence:
src/core/cli/walkthrough.js:641,src/core/cli/walkthrough.js:644-651,src/core/cli/walkthrough.js:670-672,test/core/walkthrough-detect.test.js:92-105 - Assessment: Detection only runs when
opts.picksis absent, and the dedicated test confirms the injected detector is not called when picks are provided.
Detected client sources are pre-checked and labeled in TUI flow
- Status: correct
- Evidence:
src/core/cli/walkthrough.js:682-687,src/core/cli/walkthrough.js:418-427,test/core/walkthrough-detect.test.js:44-52 - Assessment: The picker options are annotated with
checkedand· detected, and tests assert both UI label/state and resulting selected sources.
Interactive export default alignment to local-parquet
- Status: incomplete
- Evidence:
src/core/cli/walkthrough.js:696-704,src/core/cli/walkthrough.js:475-477,src/core/cli/walkthrough.js:377,src/core/cli/walkthrough.js:707,test/core/walkthrough-tui-happy.test.js:175-203 - Assessment: Alignment is true for TUI multiselect, but legacy interactive mode (
HYP_NO_TUI=1/ non-TUI path) still defaults tokeep-localon empty input.
Findings
Error Handling & Resilience
- Severity: minor
- Confidence: high
- Evidence:
src/core/cli/detect.js:61-66,src/core/cli/walkthrough.js:646-650,src/core/cli/walkthrough.js:662 - Why it matters: Filesystem/detector failures are silently treated as “nothing detected,” which masks real environment problems (permissions, IO errors) as normal absence.
- Suggested fix: Log detection failures at debug level (error code/message) and emit a span attribute indicating detection errors occurred.
Concurrency, Ordering & State Safety
- Severity: minor
- Confidence: medium
- Evidence:
src/core/cli/detect.js:57-64,src/core/cli/walkthrough.js:661 - Why it matters: Parallel stats race when adding to
Set, sodetected_sourcesordering can vary (claude,codexvscodex,claude), creating telemetry noise and brittle downstream assertions. - Suggested fix: Normalize before emit, e.g.
detected_sources: [...detected].sort().join(',').
No Finding
- Behavioral Correctness
- Change Impact / Blast Radius
- Security Surface
- Resource Lifecycle & Cleanup
- Release Safety
- Test Evidence Quality
- Architectural Consistency
- Debuggability & Operability
Evidence Bundle
- Changed hot paths:
src/core/cli/walkthrough.js(interactive flow/prompt defaults),src/core/cli/detect.js(filesystem autodetect),src/core/daemon/client_settings_path.js+src/core/daemon/status.js(settings-path contract reuse/re-export) - Impacted callers:
src/core/cli/walkthrough.js:645-647(detector invocation),src/core/cli/walkthrough.js:475-477(TUI vs legacy routing),src/core/daemon/status.js:574(continued export surface forresolveClientSettingsPath) - Impacted tests:
test/core/detect.test.js:18-75,test/core/walkthrough-detect.test.js:24-107,test/core/walkthrough-tui-happy.test.js:89-112,test/core/walkthrough-tui-happy.test.js:175-203 - Unresolved uncertainty: I did not execute the test suite in this review pass; behavior claims are based on static diff/caller tracing only.
Claude review
Code review
No issues found. Checked for bugs and CLAUDE.md compliance.
(3 candidate findings were raised by the parallel scan but all scored below the
0.80 confidence threshold and were filtered: inline import('...') types in
test/core/walkthrough-detect.test.js [75 — verbatim CLAUDE.md violation but
style-only], walkthrough.finish span missing sources_detected [50],
detect.js JSDoc parenthetical omitting $CLAUDE_HOME [50].)
🤖 Generated with Claude Code
Reports: /Users/phil/testcity/.gc/pr-pipeline/reviews/pr-76 · manual dual-review (faithful to mol-pr-dual-review)
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.
What
The interactive
npx hypawarepicker now autodetects installed client tools and pre-selects them, instead of opening with every box unchecked. It also defaults the export to local-parquet.claude/codex· detectedkeep-locallocal-parquet(matches--yes)Scope & rules
claude,codex).raw-anthropic,raw-openai, andotelare manual integration modes with nothing installed to detect, so they're never auto-checked (but remain in the list).~/.claude, and$CODEX_HOME??~/.codex. Stats the directory, notsettings.json, so HypAware writingsettings.jsonon attach never self-triggers.--yes/--dry-run/ presets) selection stays explicit and deterministic — detection is skipped entirely (structurally, not by convention).local-parquetdefault is a plain default, not autodetect; it aligns the interactive picker with the already-documented--yesdefault.Changes
src/core/cli/detect.js(new) —detectClientSources({ env }).src/core/daemon/client_settings_path.js(new) — extracted the pureresolveClientSettingsPath(handles$CLAUDE_HOME/$CODEX_HOME) so the detector reuses it withoutstatus.js's heavy import graph;status.jsre-exports it so existing import sites are unchanged.src/core/cli/walkthrough.js— run detector on the interactive branch, pre-check detected sources +local-parquet, threadcheckedthroughtuiPromptFactory, recordsources_detected/detected_sourceson thewalkthrough.startspan. Detector injectable for tests.src/core/cli/types.d.ts—checked?onWalkthroughOption,detect?override onRunPickerWalkthroughOptions.CONTEXT.md(new) — glossary: client source vs raw proxy source, autodetect vs default.A code comment notes the future descriptor-driven migration path (read
attach_probe.settings_filefromclientDescriptors) for if/when the picker becomes plugin-driven — deliberately not done now, since at first run only bundled plugins exist andPICKER_SOURCESis itself hardcoded.Tests
test/core/detect.test.js— present/absent/both/empty,$CODEX_HOMEoverride, file-not-a-dir.test/core/walkthrough-detect.test.js— preselection +· detectedlabels + export default, nothing-detected, picks-skip-detection.local-parquetdefault.Verification
npm run typecheck— cleannpm run lint— 245 files OKnpm test— 665 pass, 0 fail🤖 Generated with Claude Code