fix(dev-fleet): actionable error when no trusted git resolves (#2530) - #2553
Conversation
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThis is a small, well-contained refactor. The logic is coherent: the No semantic defects survive falsification. No findings. [OPUS-REVIEWED] feb58b5 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — ✅ PASSAdvisory design-level review of Design-Verdict: PASS Targeted reclassification at the right seam, with the sentinel-collision case tested; proportionate to the reported harm. Suggestions
[DESIGN-REVIEWED] feb58b5 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- src/kiro_crew/apps/builtins/dev_fleet/server.py:3031 -- No trusted git makes the initial False positive or not applicable? A repository writer can comment: |
When _trusted_bin('git') resolves nothing, worktree discovery and /api/sync
used to blame the checkout ('git worktree discovery failed in <repo>: no
trusted executable...') and echo the whole trusted PATH into the UI, while
never naming KIROCREW_DEVFLEET_BIN_GIT — the operator override that is the
actual remedy.
- _UNRESOLVED_TOOL_PREFIX gives the failure an identity callers can branch
on instead of re-matching prose; _run_cmd uses it when synthesizing the
unresolved-tool stderr.
- _bin_override_var() is the single source of truth for the override env
var name, shared by _trusted_bin (reads it) and the remedy messages
(advertise it) — derived from the tool name, not hardcoded for git.
- _discover_worktrees checks the unresolved-tool case BEFORE the .git probe
and raises a message that names the tool, states the checkout is not the
problem, and gives the override remedy; the trusted-PATH detail moves to
the log line.
- /api/sync's git_bin-None branch returns the same remedy-first message.
- Genuine git failures still surface git's own redacted, bounded message;
a companion test proves mid-string sentinel text is not misclassified.
Closes #2530
72d1426 to
feb58b5
Compare
…tdev#2530) (kirodotdev#2553) When _trusted_bin('git') resolves nothing, worktree discovery and /api/sync used to blame the checkout ('git worktree discovery failed in <repo>: no trusted executable...') and echo the whole trusted PATH into the UI, while never naming KIROCREW_DEVFLEET_BIN_GIT — the operator override that is the actual remedy. - _UNRESOLVED_TOOL_PREFIX gives the failure an identity callers can branch on instead of re-matching prose; _run_cmd uses it when synthesizing the unresolved-tool stderr. - _bin_override_var() is the single source of truth for the override env var name, shared by _trusted_bin (reads it) and the remedy messages (advertise it) — derived from the tool name, not hardcoded for git. - _discover_worktrees checks the unresolved-tool case BEFORE the .git probe and raises a message that names the tool, states the checkout is not the problem, and gives the override remedy; the trusted-PATH detail moves to the log line. - /api/sync's git_bin-None branch returns the same remedy-first message. - Genuine git failures still surface git's own redacted, bounded message; a companion test proves mid-string sentinel text is not misclassified. Closes kirodotdev#2530
Summary
When Dev Fleet's trusted-binary resolver cannot find a
gitit is willing to run, the failure surfaced as a repository problem:git worktree discovery failed in <repo>: no trusted executable for 'git' in <PATH>— blaming a healthy checkout, echoing the whole trusted PATH into the UI as unactionable noise, and never namingKIROCREW_DEVFLEET_BIN_GIT, the operator override that is the actual remedy (already read at_trusted_bin).Changes
_UNRESOLVED_TOOL_PREFIX— module constant giving the unresolved-tool failure an identity callers can branch on (startswith) instead of re-matching prose._run_cmduses it when synthesizing the stderr; the text itself is unchanged, so no existing consumer breaks._bin_override_var(name)— single source of truth for the override env-var name, shared by_trusted_bin(reads it) and the remedy messages (advertise it). Derived from the tool name (KIROCREW_DEVFLEET_BIN_{NAME}, dash→underscore), not hardcoded for git._discover_worktrees— checks the unresolved-tool case BEFORE the.gitprobe and the generic wrapper; raises a message that names the tool, states the checkout is not the problem, and gives the override remedy in the same voice as the missing-checkout branch. The trusted-PATH detail moves to alogger.warningline./api/sync(_sync_start_locked) — same remedy-first message at thegit_bin is Nonebranch; consistent with discovery._redact(raw)[:_GIT_ERR_MAX]; the sandbox-unavailable branch is unaffected (checked first, and unresolved-tool returns before the sandbox layer runs).Tests
4 new tests, each verified to FAIL against unmodified code:
test_discover_worktrees_unresolved_git_blames_host_not_repo— message names git +KIROCREW_DEVFLEET_BIN_GIT, contains neither the trusted PATH, nor "worktree discovery failed", nor the repo pathtest_discover_worktrees_real_git_error_not_misclassified— mid-string sentinel text still takes the genuine-git-error pathtest_sync_unresolved_git_names_override_not_path—/api/syncreturns the same remedy-first messagetest_bin_override_var_derivation— env-var derivation incl. dash mappingLocal gates: isort / flake8 / mypy clean; full pytest green in the dev_fleet area (host-env-only failures in unrelated modules).
Backend-only — no dashboard/API/i18n surface.
Closes #2530