Move the Runtime gateway into Core and check the wire contract against shared scenarios - #295
Merged
Merged
Conversation
Only Core uses internal/agentdaemon/gateway and internal/agentdaemon/device in production, so they now live at services/core/internal/runtimegateway and services/core/internal/runtimedevice. internal/agentdaemon/proto stays shared. The daemon's WebSocket contract test imported Core's gateway directly and cannot cross Go's internal-package boundary any more; it is removed here and replaced by shared-scenario conformance tests in the next commit. The gateway's swag annotations are removed: the moved handler is now inside the OpenAPI scan, and these routes are documented as having no generated schema.
internal/agentdaemon/proto/prototest/wire.go defines each exchange once: the ordered frames Core and the Runtime send, built from the proto types, plus the native settlement, connection loss, reconnection and silence between them, and the incompatible-version handshake. Each side replays the other side's frames from a scripted peer over a real WebSocket and asserts what it owns: - services/core/internal/runtimegateway/wire_test.go runs Core's gateway: the 426 rejection, delivery to preparation, Run and receipt waiters, no receipt before the Runtime's acknowledgement, no invented terminal events on disconnect and no inherited routes or replay after reconnect. - apps/daemon/internal/wireconformance runs the production transport and dispatcher with the controlled adapter: permanent stop on 426, the exact frames it sends, no input during preparation, cleanup on failure, no receipt before native settlement, and settled cleanup after connection loss. Runtime-generated values (Executor ID, handle, expiry) are placeholders that the Runtime side binds to the values its Runtime sends.
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.
internal/holds code that both Core and the daemon use. Ofinternal/agentdaemon, onlyproto, the Core–Runtime wire protocol, is shared.gatewayanddeviceare Core's, but they stayed there because the daemon's wire contract test ran Core's production gateway, and Go's internal-package rule stops the daemon importingservices/core/internal.This moves them into Core and replaces that cross-import with the method AGENTS.md prescribes: define each exchange once, and check each side against the shared definition.
Move (commit 1, mechanical):
internal/agentdaemon/gateway→services/core/internal/runtimegatewayinternal/agentdaemon/device→services/core/internal/runtimedeviceprotostays where it is.services/corethey would have put/agent-daemon/*into the public/v1OpenAPI.make openapishows no diff.Wire contract (commit 2):
internal/agentdaemon/proto/prototest/wire.golists each scenario once, as data: the ordered frames each side sends, built from the realprototypes, plus native settlement, connection loss and reconnection. The scenarios are version rejection, cancellation after settlement, preparation failure, and disconnect without replay.services/core/internal/runtimegateway/wire_test.go): the real gateway runs against a scripted Runtime.apps/daemon/internal/wireconformance/wire_test.go): the real transport and dispatcher run against a scripted Core, using the same controlled adapter as before.contracttest/wire_test.gonow sits on the side that owns the behavior. Both sides compare the same frames as JSON, so encoding compatibility is still covered.Makefile(check-runtime-contract),docs/runtime-protocol.md,services/core/IMPLEMENTATION.mdandscripts/build-core.share updated to match.Checks run (focused):
go build ./...go veton the moved packages and all importers, plus darwin and windows vet for the daemon packagesgo teston the moved packages,runtime,runtimeenrollment,execution,apiandcmd/...without a database, and the new wire tests on both sides, including 20 runs with-racemake check-runtime-contract,make openapi(no diff),make build-core,check-namesgo vet ./services/core/...still reports two copylocks warnings in store test files. They are the same on main.The implementer also injected eight regressions to confirm the tests fail. Each was caught by the side that owns it:
doneon disconnect, acks a cancellation before settlement, or keeps the Run route after close; Core accepts any version.ErrIncompatibleVersion;startedchanges the Executor ID.Review: no blind review. Commit 1 is a mechanical move. Commit 2 is test-only, and the coordinator read it and relied on the injected-regression evidence above.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.