Repository navigation
fix: honor explicit fallbacks and preserve finished shell calls - #2020
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
AbirAbbas
deleted the
salvage/1738-explicit-fallbacks-and-cancellation
branch
October 28, 2026 22:47
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
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.
A cancellation watcher that starts after a shell call has returned can find both completion and cancellation ready. If it picks cancellation, it can kill a background server or watcher that the completed call left running. This checks for completion before signalling the process group. Commands that are still running keep their existing cancellation behavior.
On dev, a fallback list that names only the failing model also falls through to catalog guesses. This respects any nonblank configured list before the catalog is consulted. The original blank-only-list control stays, so an effectively empty setting can still use catalog suggestions.
This carries forward santoshkumarradha's fixes and regressions for completed calls and fallback lists from #1738, originally
b11e3d7a1b523464490e6b5cf87ccbb00da695cdanda8039c5cf387fdb1a90cfa2e1273a254bf7b8ab3. The completion check is adapted to current dev, and both original regressions are kept. The manual and a change entry describe the result.Most of #1738's pair/continue work is already on dev through #1888, #1950, #1967 and #1990. This PR covers only these two independent defects. The rest of #1738 (task/checker pin fallback policy, a deadline for blocked skill imports, and harness question-publication ordering) needs scope decisions and is tracked separately.
app/v3 already has a narrower fix for the self-only list. The full original regression still fails there for a blank-only list, so a port to app/v3 is a possible follow-up.
Validation (two commits on dev
3bf6072fd):internal/exec/bare/promote.goreverted to dev and the regression kept,go test -count=1 -p 1 ./internal/exec/bare -run '^TestACallThatReturnedLeavesItsBackgroundProcessAlone$'fails with "the cancellation after a finished call killed the group it left running". With the fix restored, it passes.internal/provider/endpoints.goreverted to dev,TestAFallbackListNamingOnlyTheFailingModelIsAPinfails by returning catalog guesses for a self-only list. With the fix restored, it passes, including the blank-only control.internal/provider: the original regression, explicit-list precedence, no provider-layer model hop, and pacing;internal/exec/bare: the regression, cancellation before start, cancellation while running, a leaked pipe, and timeout;internal/manual.go veton all three packages, gofmt and whitespace checks are clean. The process-group regression is POSIX-only. Full suites and live-provider tests were not run locally; CI covers them.🤖 Generated with Claude Code