fix: forge bugfix exits non-zero on LLM failure (closes #18)#19
Merged
Conversation
P1: model tier routing (tierrouter), 3-layer KB (knowledge/layered.go) P2: OTEL checkpoint spans, Prometheus metrics (forge metrics), forge undo wiring feat: forge companion — zero-setup AI pairing (install/update/status/guide) feat: forge init companion hint feat: 7 command groups in forge --help (50 commands organised) feat: vibe-coding daily workflows in all skill templates fix: errcode ranges 6600-6649 (metrics), 6650-6699 (companion) test: companion_test.go (12), prometheus_test.go (6), ship_undo_test.go (10), llmpipe_tier_test.go (4) All tests: EXIT 0
SetUsageTemplate was comparing .GroupID (string) against \$.Groups (slice),
which always evaluates false in Go templates. Capture the current group's ID
with {{\\ := .ID}} before the inner {{range \$.Commands}} loop.
Add TestHelpCommandGroups regression test that verifies each group header
is followed by at least one expected command in the help output.
Fixes: forge help showing 7 empty group sections
Before this fix, three code paths in llmBugfix/Run silently returned (result, nil) when the LLM provider was unavailable or its response was invalid. This caused forge bugfix to exit 0 with an empty result, bypassing all quality gates and leading AI tools to apply direct file edits instead. Changes: - Add FORGE-6553 (ErrLLMCallFailed) and FORGE-6554 (ErrNoLLMProvider) - Run(): replace silent no-LLM placeholder with errcode.New(ErrNoLLMProvider) - llmBugfix(): replace silent provider.Complete failure with errcode.New(ErrLLMCallFailed) - llmBugfix(): replace silent JSON parse failure with errcode.New(ErrLLMCallFailed) - Add unexported RunContext.testProvider and package-level testProviderHook as clean test-injection seams (zero-value nil, invisible to external callers) Tests: - TestRun_LLMCallFailed_ExitsNonZero: regression guard for issue #18 - TestRun_LLMResponse_ValidJSON_HappyPath: happy path with mock provider - TestRun_LLMResponse_InvalidJSON_ExitsNonZero: boundary (corrupt LLM output) - TestRun_LLMCallFailed_FalsePositiveGuard: valid LLM must NOT produce error - All 13 previously broken tests updated to use mock provider or accept errors - 23/23 tests pass
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
2 participants
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.
Closes #18.
Problem
Three code paths in
internal/cli/cmdbugfixsilently returned(result, nil)when the LLM provider was unavailable or its response was unparseable. This causedforge bugfixto exit 0 with an empty result, bypassing all quality gates and leading AI tools to fall through to direct file edits instead of failing loudly.Changes
internal/cli/cmdbugfix/bugfix.goErrNoLLMProvider(FORGE-6554) whenllmprovider.Detectfails; returnErrLLMCallFailed(FORGE-6553) whenprovider.Completefails or response is not valid JSON; addtestProviderinjection seam toRunContextinternal/cli/cmdbugfix/bugfix_test.gomockProvider; add 4 new regression/boundary tests; update 13 existing tests to use mock or accept correct error behaviordocs/ERROR_CODES.mdNew error codes
LLM call failed — forge bugfix cannot proceed without a working LLMno LLM provider configuredTests (23/23 pass)
TestRun_LLMCallFailed_ExitsNonZeroTestRun_LLMResponse_ValidJSON_HappyPathTestRun_LLMResponse_InvalidJSON_ExitsNonZeroTestRun_LLMCallFailed_FalsePositiveGuardPre-push: all 13/13 quality gates passed (gofmt, go vet, golangci-lint, go build, go test, govulncheck, go mod verify, forge scan, forge lint, forge check, forge qa 33/33).