refactor(adk): make the foreground handoff API usable outside the module - #1233
Merged
Merged
Conversation
Two related fixes to the public surface around foreground/background handoff. ShouldAutoBackground was unusable outside this module. Its parameter type, foreground.CandidateInfo, lives in adk/internal/foreground: packages under adk/ can import it, so the build stayed green, but an external caller cannot. There is also no workaround — Go does not infer function-literal parameter types from assignment context, and a defined type is not identical to a structurally equal struct literal, so no assignable closure can be written. The field was dead weight for every user of the framework. Expose backgroundtask.ForegroundCandidate as an alias for the internal type, plus backgroundtask.DefaultForegroundTimeoutMs, which the Config docs already referenced through an unreachable symbol. This follows the existing pattern in callbacks.RunInfo and schema.GobSerializer. foreground.Policy and ProjectionDetached stay internal: they are wiring, not user API. Rename LocalBackgroundConfig.AlwaysForeground to BackendOwnsCommandTimeout. The option's only effect is selecting what the tool's timeout argument bounds: the caller's foreground wait, or the command's own execution limit passed to filesystem.ExecuteRequest.Timeout. "Always foreground" named a derived consequence rather than the thing being configured, and hid the cost — the mode disables the Manager's foreground timer, leaving context cancellation as the only bound on a backend that ignores Timeout. Note that a nil ShouldAutoBackground is not equivalent: on timer expiry it terminates the run with a *backgroundtask.ForegroundTimeoutError rather than staying attached. The tool description variants are renamed to match (BackendTimeoutManagedExecuteToolDesc), and the internal helper becomes backendOwnsTimeoutForRun so it no longer collides with the struct field. Breaking change to unreleased API; no behavior change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mrh997
approved these changes
Sep 1, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha/10 #1233 +/- ##
===========================================
Coverage ? 81.92%
===========================================
Files ? 219
Lines ? 37230
Branches ? 0
===========================================
Hits ? 30501
Misses ? 4527
Partials ? 2202 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Why
Two related problems on the public surface around foreground/background handoff.
1.
ShouldAutoBackgroundcannot be used outside this module.Its parameter type is
foreground.CandidateInfo, defined inadk/internal/foreground. Packages underadk/may import it, so CI stayed green — but an external caller gets:There is no workaround: Go does not infer function-literal parameter types from assignment context, and a defined type is not identical to a structurally equal struct literal, so no assignable closure can be written. The field was unusable for every user of the framework. It appeared on five public configs (
backgroundtask/local.Config,backgroundtask/tool.ManagedToolConfig,middlewares/subagent,middlewares/filesystem,prebuilt/deep). TheConfigdocs also referencedforeground.DefaultTimeoutMs, equally unreachable.2.
LocalBackgroundConfig.AlwaysForegroundnames the wrong thing.Its only effect is selecting what the tool's
timeoutargument bounds — the caller's foreground wait, or the command's own execution limit passed tofilesystem.ExecuteRequest.Timeout. "Always foreground" named a derived consequence rather than the thing being configured, and hid the cost: the mode disables the Manager's foreground timer, so context cancellation becomes the only bound on a backend that ignoresTimeout.Worth noting because it is easy to assume otherwise: a nil
ShouldAutoBackgroundis not equivalent to this mode. On timer expiry it callscancel()and returns a*backgroundtask.ForegroundTimeoutErrorrather than staying attached.What
backgroundtask.ForegroundCandidate(alias for the internal type) andbackgroundtask.DefaultForegroundTimeoutMs. Same pattern ascallbacks.RunInfoandschema.GobSerializer.foreground.PolicyandProjectionDetachedstay internal — they are wiring, not user API.ShouldAutoBackgroundfields at the public name, and the internal construction sites too, so no file carries two names for one type.AlwaysForeground→BackendOwnsCommandTimeout, with the doc comment restructured to state the timeout ownership, the cost, and the never-auto-backgrounded implication separately.BackendTimeoutManagedExecuteToolDesc/...Chinese.backendOwnsTimeoutForRunso it no longer collides with the struct field it reads.No behavior change. Breaking change to API that has only shipped in
v0.10.0-alpha.*.Verification
go build ./...go test -race ./...— full suite passesgolangci-lint run ./adk/...— 0 issuesgofmt -s -l ./adk— cleanreplaceto this tree) that both renamed APIs are now reachable: constructing the callback, reading everyForegroundCandidatefield, and referencingDefaultForegroundTimeoutMsandBackendOwnsCommandTimeoutall compile and run.🤖 Generated with Claude Code