fix(cli): fail unresolved graph commands - #547
Conversation
Review — good consolidation, one wrong-code case to decideReplacing ten hand-rolled llm-only handlers with Confirmed the text path is unchanged: The one thing worth deciding first
if (result.ambiguous) {
printAmbiguous(symbol, result.result, opts); // stderr: 'Ambiguous symbol "X": ...'
}
return null;So an ambiguous symbol now emits, on stdout: { "error": "unresolved_target", "message": "No entity found matching \"X\"." }while stderr says the opposite — it resolved to several candidates and the user should pass This is not new to your PR —
Two ways to go, both fine by me:
Smaller notes
|
Blocking, on evidence — this has #559's problem, at ten times the surfaceI was clearing the batch and went to merge this. It should not merge yet, and the reason is the one that already made #559 a draft. #559 is held back because making The plugins discard stdout on a non-zero exitCounted across the six plugin repos, in code — not prompt prose:
Bun's And the MCP layer does the same, worse
if (!result.ok) {
const detail = result.stderr.trim() || result.stdout.trim() || `… failed without output`;
return textResult(JSON.stringify({ error: detail, tool }), true);
}
( What I would doPut this in the same queue as #559 — after ix-opencode-plugin #20, ix-openclaw-plugin #33 and ix-claude-plugin #37, which are the PRs that teach the plugins to read a payload from a failed run. The change itself is right and I am not asking for edits to it. Worth noting the two PRs also still disagree on the ambiguity slug (my earlier comment, and the comment in #559's diff). If the plugin work is landing anyway, fixing Merged from this batch, for context#549, #553, #555, #556, #558, #561, #562, #563, #564. This one, #543, #545, #551 and #559 stayed open — reasons on each. |
|
Correction to my comment above: I listed three plugin repos with runners. Running the check systematically, it is five of the six.
plus That does not change the conclusion, it widens it: the plugin-side work this needs is five repos, not three. The check itself is now written down in #565, along with why it is invisible from inside this repo. |
…565) Twice now a PR has added a structured error body plus a matching non-zero exit code, and the exit code is what stopped anyone from seeing the body (#539, and #547 which is still open for this reason). The mechanism is not visible from inside this repo, which is why it keeps recurring: - plugins run `ix` through wrappers that discard stdout on a failed run -- bun's `$` throws, the shell hooks branch on `||` under `set -euo pipefail` - `runCommand` in mcp/server.ts prefers stderr over stdout when the exit is non-zero, so a record written to stdout is replaced by whatever prose the resolver already put on stderr So the check has to happen in the plugin repos, and this documents it with the grep that answers it. Verified the three patterns are exhaustive: no plugin builds the command name from a variable, so a literal search finds every call site. Spot-checked against three commands with known answers -- `impact` (8 runners across 4 repos), `read` and `doctor` (none; both only appear in agent prompt prose, which is why they were safe to change).
|
Thanks! I left the CLI code unchanged and only documented the plugin compatibility findings. I’ll leave the plugin-side follow-ups with you. |
2 participants
Fixes #546
Summary
Make unresolved graph targets a structured command failure instead of an empty successful result.
Type
Changes
unresolved_targetJSON and LLM records and set exit status 1.overview,impact,contains,callers,callees,imports,imported-by,depends,trace, andhistory.Compatibility and merge blocker
Do not merge this PR until the shipped plugin call sites retain and interpret stdout from non-zero CLI runs.
The current plugin PRs are still open.
ix-cursor-plugin#26andix-gemini-plugin#29cover the affected fallback paths in those plugins. The currentix-opencode-plugin#20,ix-openclaw-plugin#33, andix-claude-plugin#37primarily cover locate/docs paths and do not yet cover every call site expanded by this PR:impact,callers, anddependspaths used byix-impact,ix-neighbors, andix-decide.hooks/ix-utils.tsstill rejectsoverviewandimpactbefore the hook can inspect stdout.ix impactpath still treats a non-zero run with a body as a generic command failure.The CLI change itself remains unchanged as requested in review; this section records why the PR must remain open.
Known behavior
No entity resolved fortoNo entity found matching.unresolved_targetreporter becauseresolveFileOrEntitycurrently represents both ambiguity and a miss asnull. The more accurateambiguous_resolutionsplit remains a separate decision noted in review.Validation
npm test(84 files, 1456 passed, 3 skipped; parser smoke passed)npm run typecheckmktemp --suffixdependency that does not run cleanly on macOSChecklist
Release checklist (if merging to main)
ix-cli/package.jsonversion bumpedgit tag vX.Y.Z && git push origin vX.Y.Z)ix-memory-layertagged and released firstdocker-compose.standalone.ymlchanged: verifiedcurl | shinstall works