Skip to content

fix(locate): exit non-zero for an unresolved target - #559

Draft
KageBinary wants to merge 1 commit into
mainfrom
fix/539-locate-exit-code
Draft

fix(locate): exit non-zero for an unresolved target#559
KageBinary wants to merge 1 commit into
mainfrom
fix/539-locate-exit-code

Conversation

@KageBinary

Copy link
Copy Markdown
Collaborator

Closes #539. Draft on purpose — this is step 2 and must land after step 1.

Merge order

#539 is explicit that flipping this exit code first is what breaks things. The three consumers it names are now ready and green:

  • ix-opencode-plugin#20
  • ix-openclaw-plugin#33
  • ix-claude-plugin#37

Once those are in, this is ready to go. Marking it ready before then would degrade all three.

Change

ix locate <missing> reported the miss in every format but always exited 0, so a machine caller could not tell it from a successful command that intentionally returned nothing. context, explain, read, trace and subsystems all exit 1 for the same condition after #526/#532 — locate was the odd one out, and its llm output already emitted an error record while exiting 0.

Verified live against backend 1.0.27, all three formats:

$ ix locate DefinitelyMissing99 --format json   → body unchanged, exit=1
$ ix locate DefinitelyMissing99 --format llm    → error code=unresolved_target, exit=1
$ ix locate DefinitelyMissing99                 → No matches found., exit=1
$ ix locate registerLocateCommand --format json → resolves, exit=0

Two deliberate choices

The body stays as it is. I did not switch to reportUnresolvedTarget, whose {error, message} shape is what the sibling commands use — shipped plugins read diagnostics off locate's result, and #539's whole point is that the exit code becomes informative without the payload going away. Worth a second opinion if you would rather have the uniform shape and accept the consumer churn.

Ambiguity is untouched. It resolves to several candidates — an answer, not a miss — and returns on its own path before this branch. Making that non-zero too would be a separate policy call.

End-to-end proof of the sequence

Old vs new plugin safeRun, both against this build:

OLD (.text()):            null   ← diagnostics lost
NEW (.nothrow().text()):  [ "No graph entity found." ]

That is exactly the degradation #539 predicted, and exactly what step 1 prevents.

Checks

Both new tests fail without the change. Full gate: 1449 passed / 2 skipped, lint 0 errors, typecheck clean, knip clean.

`ix locate <missing>` reported the miss in every format but always exited 0,
so a machine caller could not distinguish it from a successful command that
intentionally returned nothing. `context`, `explain`, `read`, `trace` and
`subsystems` all exit 1 for the same condition after #526 and #532; locate was
the odd one out, and its llm output even emitted an `error` record while
exiting 0.

The result body is deliberately left alone rather than replaced with
`reportUnresolvedTarget`'s `{error, message}` shape. Shipped plugins read
`diagnostics` off it, and the point of #539 is that the exit code becomes
informative without the payload going away.

Ambiguity keeps its own path: it resolves to several candidates, which is an
answer rather than a miss, and it returns before this branch.

This is step 2 of the sequence in #539, and must land after step 1 --
ix-opencode-plugin#20, ix-openclaw-plugin#33 and ix-claude-plugin#37.

Closes #539
@KageBinary

Copy link
Copy Markdown
Collaborator Author

Status — reviewed, deliberately left as a draft

Re-read this as part of a sweep of every open PR. The change itself is correct and I am not asking for anything: process.exitCode = 1 on the miss branch, the result body left intact because shipped plugins read diagnostics off it, and ambiguity kept on its own path because it resolved to candidates rather than nothing.

Not converting it to ready. The merge order in #539 still stands and none of it has moved:

order PR repo
1 #20 ix-opencode-plugin
1 #33 ix-openclaw-plugin
1 #37 ix-claude-plugin
2 #559 Ix

Landing this first makes ix locate <missing> exit 1 against plugins that do not yet expect it.

One cross-reference worth having here: the comment in this diff draws the ambiguity distinction explicitly —

Ambiguity is a different condition and keeps its own path above — it resolved to several candidates, which is an answer, not a miss.

whereas #547, open right now, routes both conditions through reportUnresolvedTarget, so an ambiguous symbol reports unresolved_target on stdout while stderr says it matched several. That is pre-existing in context/explain/read, and #547 takes it from 3 commands to 13. Two open PRs disagreeing about the same question — flagged on #547 as well, so whichever you pick lands in one place.

@KageBinary

Copy link
Copy Markdown
Collaborator Author

Re-checked today (2026-09-01). Still correct, still must merge last, and the queue in front of it has not moved.

The change is small and I have no objection to it: exiting 1 on a genuine miss while keeping the {resolvedTarget, resolutionMode, systemPath, diagnostics} body is the right call, and leaving ambiguity on its own path is right too — several candidates is an answer, not a miss.

The reason it can't go in yet, verified against each repo's current main rather than assumed:

  • ix-cursor-plugin mcp/tools/locate.ts:52if (!result.ok) return wrapErr("IX_LOCATE_FAILED", …). This is the sharpest illustration: twelve lines further down, at :63, the same file already has if (!target) return wrapOk(… match: null …) — it already models a miss as an answer. This PR makes that branch unreachable and routes the miss into the error path instead.
  • ix-gemini-plugin mcp/tools/ix_locate.ts:55if (cliResult.ok) { … }, else falls through to runtime-unavailable.
  • ix-opencode-plugin tools/ix-query.ts:69 — bun $ throws → fallbackUnavailable(...).

Merge order is unchanged from 08-30: ix-opencode-plugin #20 → ix-openclaw-plugin #33 → ix-claude-plugin #37, then this. ix-claude-plugin #37 is titled "a locate miss is an answer, not a locate failure" — it is precisely this PR's counterpart. All three are green and all three still 405 on merge because the plugin repos' rulesets have empty bypass_actors.

Leaving it as a draft. Nothing to fix on this branch.

@KageBinary

Copy link
Copy Markdown
Collaborator Author

Second pass on the code. Re-verified against today's main (793c56b, after #543/#545/#551): merges with no conflicts, typecheck clean, ix-cli 1507 passed / 2 skipped.

Correction to what I said on 08-31

I flagged that this PR and #547 disagree about whether an ambiguous target should report unresolved_target. They don't — that was my error.

Your comment in locate.ts is exactly right, and I've now confirmed it holds on both sides:

  • locate is the only command using resolveWithAmbiguity. Its ambiguous branch emits its own ambiguous_target slug with the candidate list and exits 0 — untouched by this PR.
  • Every command fix(cli): fail unresolved graph commands #547 converts goes through resolveFileOrEntity, which never returns null for ambiguityresolve.ts:730 onward sorts, tie-breaks on path, and returns matches[0]. Its only null returns are a failed raw-UUID lookup, a Class::method phase-B miss, and matches.length === 0.

So both PRs already encode the same rule: ambiguity is an answer, zero matches is a failure. No decision needed, and I've withdrawn it from the list of things needing a call.

The rest

Keeping the {resolvedTarget, resolutionMode, systemPath, diagnostics} body rather than swapping in reportUnresolvedTarget's {error, message} shape is the right call, and the comment explains why well enough that nobody will "tidy" it later. Mutation-checked both new tests by dropping the process.exitCode = 1 line: 2 failed / 3 passed, and they fail for the right reason.

Still blocked on the same three, in order: ix-opencode-plugin #20 → ix-openclaw-plugin #33 → ix-claude-plugin #37, then this. Leaving it as a draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] ix locate exits 0 with a null result for a target that does not exist

1 participant