Give api.NewHandler one explicit Dependencies value - #302
Merged
Merged
Conversation
NewHandler(ResourceStore, auth, engine, options...) and its 26 With* options become NewHandler(Dependencies). Each application area is one interface field declared beside its handlers, and NewHandler rejects a missing required field. Execution (the Worker, its executor URL and the optional native installer) and Sandboxes (the managed deployment, which requires Execution) are the only optional groups; nil means disabled. The handler no longer discovers capabilities through type assertions and no longer falls back to GetAgent when a store lacks GetAgentForSession. The 503 and 409 answers that only a partially wired test handler could produce are gone. cmd/server builds the one Dependencies value. API tests use strict per-area fakes that fail on any unexpected call.
Core no longer emits skill_storage_unavailable, file_storage_unavailable or diagnostics_unavailable, so Web and the agents-client tests stop handling them. cmd/server drops a managed-node check that is always true and the unreachable type-asserted observation source behind it, plus a repeated installation ID check. CoreErrorBoolean had no caller, so Core error details no longer list booleans.
unexpectedCall now records the failure with t.Errorf and panics. net/http recovers the panic on an httptest server goroutine, where t.Fatalf cannot stop the test, and a direct ServeHTTP call fails loudly. The routing fixture's trapTB overrides Errorf, so a trapped call still reports "handler reached" without failing the test. newTestAuthenticator takes the test and fails it on an invalid fixture key instead of returning an error every caller only passed to t.Fatal.
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
1 participant
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.
Part of the Core layering plan (issue #1):
api.NewHandlertakes one explicitapi.Dependenciesvalue, so later domain cutovers can replace one area interface at a time.What changes
api.NewHandler(Dependencies) (http.Handler, error)replaces the handler options. Each application area is a field typed as an interface declared inapi, listing exactly the methods its handlers call.validate()rejects any missing required field at construction.Execution{ExecutorURL, Admission, SessionArchive, Workspaces, NativeInstaller}Sandboxes{Deployment, DeploymentChanges, ConfigurationDiscovery}(requiresExecution)api.cmd/servercomposesDependenciesexplicitly (executor_connections.go).AuthenticatorandNewDatabaseAuthenticator. The key rules live inauth.go(projectBearerDigest,resolveCaller) andsandbox_manager_auth.go.Removed
Error codes that only reported a missing optional dependency no longer exist, because every such dependency is now required:
skill_storage_unavailable,file_storage_unavailable,diagnostics_unavailable,artifact_storage_unavailable,subagent_storage_unavailable,stream_unavailable,execution_configuration_unavailable. Web andpackages/agents-clientdrop them too, andCoreErrorBooleanis deleted.Tests
api/fakes_test.go: one strictfake<Area>per interface. An unset func fails the test with "unexpected call to ".store/public_handler_fixture_test.gobuilds the full router for the store-backed integration tests.Checks
go build ./...,go vet(api, store, cmd/server, execution) cleango test: api 310, cmd/server 33, sandbox/providers 36, execution 92 passedpython3 scripts/check-names.py,make openapi(no diff), Web typecheck and affected testsNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.