feat(wiki): support local Claude and Codex providers#1769
Conversation
|
Someone is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
|
@lianliankan007 can you please rebase your changes on the tip of the main branch? 🙏 |
|
There's no need to make changes in the Express routes, as it's been fixed recently. |
|
@lianliankan007 Can you please resolve the merge conflicets? |
07e11f0 to
349950c
Compare
|
@magyargergo |
|
Claude finished @magyargergo's task in 5m 48s —— View job PR #1769 — Production-Readiness ReviewTasks:
Review bar for this PRApplied against: DoD.md v2.0.0, GUARDRAILS.md, and the PR-specific DoD in the trigger comment. Required for merge: TypeScript correctness, unit+integration tests on the real changed path, CI (quality+tests gate), no subprocess hang risk, no injection surfaces, no contract drift between providers, and branch hygiene. Problem being solvedExtend Current PR state
Merge status and mergeabilityCI did not run. This is a fork PR ( Repository history considered
Branch hygiene assessmentClean. One feature commit. Only Understanding of the changeThe PR adds
Config changes add The changes to FindingsFinding 1: Local CLI subprocess has no timeout — Risk: Evidence: Recommended fix: Add a Blocks merge: yes — local providers can hang the wiki process indefinitely with no user-visible error. Finding 2: Risk: On Windows, every LLM call to Claude or Codex during wiki generation flashes a console window. Evidence: Recommended fix: Add Blocks merge: maybe — Windows platform is supported; this causes visible UX regression on every wiki generation. Finding 3: Risk: In Evidence: model:
overrides?.model ||
process.env.GITNEXUS_MODEL ||
savedLocalModel ||
(localProvider ? '' : savedConfig.model || 'minimax/minimax-m2.5'),Recommended fix: For local providers, skip Blocks merge: maybe — requires a common mixed-use environment to trigger, but when it does, the error message doesn't explain why. Finding 4: No subprocess-contract tests — tests don't validate argv, stdin, or exit behavior Risk: All tests in
The two Evidence: Recommended fix: Add at least one test per provider that spawns a fake binary (a script that asserts its argv and stdin and exits 0 with known output), placed in Blocks merge: yes — the runtime contract for Claude and Codex invocation is entirely unverified by tests. A broken flag or wrong stdin behavior would ship undetected. Finding 5: Codex CLI command contract is unverified Risk: The constructed Codex command Evidence: Recommended fix: Verify the current Blocks merge: yes if contract is wrong — unverifiable from source alone; requires real CLI validation or a stub that enforces the contract. Finding 6: Claude CLI The Claude implementation uses Evidence: Recommended fix: Covered by the subprocess-level tests from Finding 4. Blocks merge: no (flags appear correct) — but should be locked by tests before merge. Finding 7: CI gate did not run The required CI gate ( Evidence: PR Checks tab shows 0 checks. Fork PR authorization was not granted. Confirming from workflow: Action required: Maintainer must either authorize the workflow run in the PR UI or provide evidence of local validation ( Blocks merge: yes. Finding 8: Empty stdout from Claude produces silent empty-content response Risk: In Evidence: Recommended fix: Add a guard in Blocks merge: maybe — produces corrupted (silent, empty) wiki pages with no actionable error. PR-specific assessment sections1. Provider contract and scope: Correctly scoped to CLI wiki. Issue #97 (web bridge, Gemini) should remain open. 2. CLI parsing and help text: 3. Config resolution and persistence: 4. Local CLI detection: Cache behavior is correct and consistent with Cursor. 5. Claude invocation: Prompt via stdin is correct. Flags look correct. Gap: No subprocess-contract test (Finding 4); no empty-stdout guard (Finding 8). 6. Codex invocation: Read-only sandbox won't block tmpdir writes. Output file path correctly uses 7. Security and trust boundaries: Prompt content flows via stdin, not command args (good). 8. Runtime reliability: Missing binary throws fast with an actionable message. Non-zero exit rejects with stderr+stdout context. 9. Timeout, retry, rate-limit, progress: 10. Existing provider regression: 11. Test quality: See Finding 4. Tests are too mocked to validate runtime subprocess behavior. 12. CI and branch protection: See Finding 7. CI did not run. Back-and-forth avoided by verifying
Open questionsNone that can't be answered by inspecting source or running the CLI. Final verdictNot production-ready. Five confirmed blockers: (1) Local CLI subprocesses have no timeout, causing The implementation approach is well-structured and follows the Cursor precedent closely, but the missing timeout, the absence of real subprocess-level tests, and the empty-output silent failure are runtime risks that block merge under the project's own DoD bar. |
|
@lianliankan007 could you please look into claude's findings above? |
|
/autofix |
|
✅ Applied autofix and pushed a commit. (apply run) |
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 9765 tests passed 2 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
- Add subprocess timeout: LocalCLIConfig gains requestTimeoutMs, runLocalCLI sets a kill timer that rejects with an actionable error matching the HTTP timeout message format. --timeout is no longer silently ignored for claude/codex providers. - Add windowsHide: true to spawn() to prevent console window flash on Windows, matching cursor-client.ts behavior. - Skip GITNEXUS_MODEL env var for local providers so a user's OpenAI model name doesn't cross-contaminate claude/codex CLI invocations. Precedence for local providers: --model → savedLocalModel → ''. - Guard against empty stdout: reject with actionable error when CLI exits 0 but produces no output, preventing silent empty wiki pages.
- Move empty-output guard from runLocalCLI to per-provider callers so Codex can read --output-last-message file even when stdout is empty - Merge existing config in interactive setup (local + Azure paths) to prevent saveCLIConfig from erasing previously saved API keys - Use StringDecoder for stdout/stderr to handle multi-byte UTF-8 chars split across pipe chunk boundaries - Distinguish ENOENT from non-zero exit in detectLocalCLI so users see auth guidance instead of misleading "CLI not found" when the binary exists but is not authenticated
Add 21 integration-level tests covering the Claude and Codex subprocess contracts that wiki-flags.test.ts mocks out: - Claude argv: -p, --output-format text, --no-session-persistence, --model conditional, stdin prompt content, CI=1, windowsHide:true - Codex argv: exec subcommand, --sandbox read-only, -c approval_policy, --output-last-message temp path, --cd, stdin marker, --model - Timeout: kill timer fires and rejects, no timer when unset - Codex file fallback: stdout used when file missing, error when both empty - detectLocalCLI: warn on non-ENOENT, silent on ENOENT - onChunk: cumulative byte count forwarded Also register the test in cross-platform-tests.ts SPAWN_CLI section and fix detectLocalCLI ENOENT detection logic (invert the check so non-ENOENT errors produce a warning).
- Add killChildTree helper that uses taskkill /T /F /PID on Windows to terminate the entire process tree (including cmd.exe grandchildren), with fallback to child.kill() if taskkill fails or on non-Windows - Add Codex CLI flag contract snapshot test that locks the exact spawn args — any flag rename, reorder, or removal is caught immediately - Add Windows taskkill tests: success path asserts taskkill called with correct PID and /T /F flags, failure path verifies child.kill() fallback
Summary
Motivation / context
Areas touched
gitnexus/(CLI / core / MCP server)gitnexus-web/(Vite / React UI).github/(workflows, actions)eval/or other toolingAGENTS.md,CLAUDE.md,.cursor/,llms.txt, etc.)Scope & constraints
In scope
Explicitly out of scope / not done here
Implementation notes
Testing & verification
cd gitnexus && npm testcd gitnexus && npm run test:integration(if core/indexing/MCP paths changed)cd gitnexus && npx tsc --noEmitcd gitnexus-web && npm test(if web changed)cd gitnexus-web && npx tsc -b --noEmit(if web changed)gitnexus-web/e2e/)Risk & rollout
Checklist
AGENTS.md/ overlays changed: headers, scope block, and changelog updated per project conventionscommand