fix(ship): auto-fallback to --agent-mode on unusable LLM provider; fix agent-mode wrong-feature resume - #43
Merged
Conversation
…x agent-mode bare-continuation resuming wrong feature
- forge ship now probes the configured LLM provider (mirrors `forge doctor
--llm`) before a non-agent-mode run. When a provider is configured but a
live call fails permanently (invalid/expired key, or Anthropic's
invalid_request_error shape for "credit balance too low"), the run
automatically switches to --agent-mode instead of hard-failing every
checkpoint. FORGE_NO_AGENT_FALLBACK=1 opts out. No provider configured at
all is left untouched (existing stub/hint UX, not this failure).
- Bridge.SetFeature("", "") — the bare continuation forge itself prints as
the next-step hint after a submit — was blanking the session's recorded
feature identity on every call, so the next resolution of "which feature
is this session driving" had nothing to resume against and could fall
through to an unrelated feature's incomplete checkpoint (ISSUE 4 in
docs/plans/FORGE_SHIP_ISSUES_2026-09-04.md, ai-marketing-platfrom repo).
SetFeature("", "") is now a no-op when an identity already exists, and the
agent-mode bare-continuation path resolves the missing --name/description
from the session's own Bridge.Feature() first.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…in --dry-run
Addresses ISSUE 5 and ISSUE 3 in docs/plans/FORGE_SHIP_ISSUES_2026-09-04.md
(ai-marketing-platfrom repo), on top of the ISSUE 1/4 fixes already on this
branch.
- CRITICAL: Bridge was documented "not safe for concurrent use", but nothing
enforced it. checkArch's runParallelArchDebate fires one goroutine per
reviewer role (6 by default), and every one calls Lookup concurrently
against the same Bridge in agent mode — an unsynchronized data race on the
seen/byHash/byOrdinal maps and the pending/paused fields, which the Go
runtime can surface as a panic or, plausibly, as the multi-minute hang with
zero output actually observed mid-arch-debate. Bridge now serializes every
exported method touching shared state behind a mutex (SetFeature calls a
new unexported resetLocked to avoid re-entering it from Reset). New
regression test TestLookup_ConcurrentCallsAreSafe reproduces the exact
six-goroutines-one-operation shape; go test -race already runs nightly.
- --dry-run made real LLM calls and wrote files to disk despite its own help
text ("without making LLM calls or git operations"). newLLMPipeInteractive
returned a live, billable pipe whenever a provider was configured, dry-run
or not — its own doc comment already (incorrectly) claimed otherwise.
checkSpec/checkArch had no dryRun parameter at all, so previewing a
not-yet-generated feature unconditionally created .forge/specs/<slug>/ and
wrote workspace-context.md plus a spec.md/arch.md stub (or real LLM
output, if credentials worked) — the stray-directory behavior the doc
observed running exploratory --dry-run probes. Dry-run now always gets a
nil pipe, and checkSpec/checkArch report what they would generate without
touching disk when the target artefact doesn't exist yet.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ck-and-issue4 # Conflicts: # CHANGELOG.md # internal/agentbridge/agentbridge_test.go
teragrid
pushed a commit
that referenced
this pull request
Sep 5, 2026
…ed 2026-08-26 The requested release version 1.10.3 turned out to already be tagged and published (GitHub release + npm @forgeone/cli@1.10.3, both dated 2026-08-26, from PR #42). This session's earlier merge of PR #43 into main incorrectly folded PR #43's new fixes into that same already-released "## [1.10.3]" CHANGELOG section instead of treating it as immutable history. Split them back apart: the original 1.10.3 entry (Test/Code fabrication past a pause, undetected stale pending turn, ambiguous multi-spec resolution, .gitignore coverage, CI gates) is restored verbatim with its original date, and PR #43's fixes (LLM-pipeline agent-mode fallback, the critical wrong-feature resume, the arch-debate data race, and dry-run leaks) now form their own [1.10.4] section, which is the version actually being tagged and released next. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
forge shipwas documented to have--agent-modeas the escape hatch for a broken/out-of-credit LLM provider (ISSUE 1 indocs/plans/FORGE_SHIP_ISSUES_2026-09-04.md, ai-marketing-platfrom repo), but it required the operator to notice the failure and manually re-invoke with the flag.forge shipnow probes the configured provider (same live-call check asforge doctor --llm) and auto-switches to agent mode when a configured provider fails for a permanent reason (invalid/expired key, or theinvalid_request_errorshape Anthropic uses for "credit balance too low").FORGE_NO_AGENT_FALLBACK=1opts out. No-provider-at-all is untouched — that's a different, existing UX path.forge ship --agent-moderun bare (no--name, no description — exactly the hint forge itself prints after a submit) calledBridge.SetFeature("", ""), which blanked the session's recorded feature identity every time. The next resolution of "which feature is this session driving" then had nothing to resume against, and could fall through to an unrelated, pre-existing feature's incomplete checkpoint — silently corrupting its pipeline state if the wrong turn's answer got submitted.SetFeature("", "")is now a no-op when an identity already exists, and the bare-continuation path resolves the missing name/description from the session's ownBridge.Feature()before doing anything else.Test plan
go build ./...,go vet ./...go test ./...(full suite, all packages green)TestSetFeature_BareContinuationPreservesIdentity(agentbridge), existingTestSetFeature_SwitchingFeatureResetsStaleSessionstill passesforge shippre-push gate (gofmt, vet, golangci-lint, build, test, govulncheck, go mod verify, forge scan security, forge lint, forge check, 21 QA real-command tests, 12 ship dry-run scenarios) — all green🤖 Generated with Claude Code