Skip to content

refactor: make the v2 stack assemblable outside package main - #2543

Closed
jjamroga wants to merge 1 commit into
kagent-dev:mainfrom
jjamroga:export-v2-controller-building-blocks
Closed

refactor: make the v2 stack assemblable outside package main#2543
jjamroga wants to merge 1 commit into
kagent-dev:mainfrom
jjamroga:export-v2-controller-building-blocks

Conversation

@jjamroga

Copy link
Copy Markdown
Collaborator

Problem

core/cmd/controller-v2 is the only place the v2 stack is assembled, and it's package main. Two of its dependencies are internal, so nothing outside this module can reproduce it:

  • api/database exports the Client interface but no constructor — Connect/NewClient are in core/internal/database. Every v2 component takes a store, so there's nothing to hand them.
  • core/internal/grpcserver is internal, so the server that registers those services, including A2A, is unavailable.

The visible consequence: core/pkg/app is the only entry point available externally, and it never sets A2AHandler. grpcserver registers A2A only when that field is non-nil, so a controller built on app.Start can create AgentInstances but not talk to them — Unimplemented: unknown service lf.a2a.v1.A2AService.

Why restructure it this way

This is a restructuring, not a feature, and it doesn't add tests. What it could enable is testing the substrate-independent half of the v2 stack without the substrate-dependent machinery.

Today TestAgentInstanceInteraction can't run until CI has stood up a kind cluster, installed Substrate, bootstrapped the CA/JWT pools with kubectl-ate, brought up a gVisor WorkerPool and built a golden actor. Much of what it then asserts isn't substrate behaviour: AgentInstance lifecycle, the ownership check, A2A routing by instance headers, DefaultMethodPolicies on the A2A methods, task persistence.

Those are reachable with a Postgres and two fakes — a2agateway's dialer is one method, agentinstance's actor client is six — leaving the cluster e2e to cover what genuinely needs a cluster. That's also what a2agateway/gateway.go already anticipates: "a standalone gateway can register the same handler on its own server later."

Two honest caveats. This PR is only the first half: the composition still lives in func main(), so a test would have to duplicate that wiring. Extracting it into an exported constructor would finish the job — happy to do that here or as a follow-up. And tests inside this module are unaffected either way; go/core/test/e2e can already import core/internal/....

Change

No behaviour change.

  • core/pkg/database — thin re-export of Connect, NewClient, ResolveURL, PostgresConfig aliased. Implementation stays internal.
  • core/internal/grpcservercore/pkg/grpcserver — move plus import updates in core/pkg/app and core/cmd/controller-v2.

Config keeps fields typed on internal packages; that's fine, since omitting a field never requires importing its type and every such field is optional. SystemService and MethodPolicies self-default, so DefaultMethodPolicies() still applies and A2A authorization is unchanged.

Testing

go build ./... and go vet clean; go test ./core/pkg/grpcserver/ passes, with the existing tests moving with the package.

core/internal/service/{session,task} stay internal, so SessionService/TaskService still can't be registered externally — both optional; happy to move them here or in a follow-up.

@jjamroga
jjamroga requested a review from a team as a code owner August 24, 2026 18:16
… v2 stack

Nothing outside this module can currently assemble a controller on kagent's
own v2 components. api/database exports the Client interface but not a
constructor, and grpcserver is internal, so a downstream can only ever
receive a Client from core/pkg/app -- which never sets A2AHandler and so
never registers the A2A service.

Two changes, no behaviour change:

  * core/pkg/database re-exports Connect/NewClient/ResolveURL and aliases
    PostgresConfig. The implementation stays in core/internal/database.
  * core/internal/grpcserver moves to core/pkg/grpcserver.

Config keeps its fields typed on internal service packages. That is fine for
external callers: omitting a struct field never requires importing its type,
and the ones a downstream cannot name are all optional -- SystemService and
MethodPolicies self-default, so DefaultMethodPolicies still applies.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
@jjamroga

Copy link
Copy Markdown
Collaborator Author

Superseded by #2544, which is a much smaller change covering the same need: rather than exporting grpcserver and adding a public store constructor, ExtensionConfig gains an A2AHandler field that an extension can populate with a2agateway.New, using the DbClient it already receives in BootstrapConfig.

The integration-testing argument here still stands on its own — assembling the v2 stack outside package main would allow testing AgentInstance lifecycle, ownership checks and A2A routing with fakes for the dialer and actor client, no cluster or Substrate required. Happy to revive this separately if that is wanted; it just should not be bundled with the smaller fix.

@jjamroga jjamroga closed this Aug 24, 2026
EItanya pushed a commit that referenced this pull request Aug 25, 2026
## Problem

`grpcserver` registers the A2A service only when `A2AHandler` is
non-nil, and `core/pkg/app` never sets it. A controller built on
`app.Start` can create AgentInstances but cannot talk to them:

```
Unimplemented: unknown service lf.a2a.v1.A2AService
```

`core/cmd/controller-v2` does set it, but that is `package main`, so the
capability is unreachable to anything embedding `app.Start`.

## Change

`ExtensionConfig` gains an `A2AHandler` field, passed through to
`grpcserver`. That's it — one file, two lines of code.

Nothing else changes. The field defaults to nil and `grpcserver` skips
registration exactly as it does today, so this is inert unless an
extension opts in.

It also needs no reordering: `getExtensionConfig` is called with the
`DbClient` in `BootstrapConfig` well before the gRPC server is
constructed, so an extension can build a handler with `a2agateway.New`
and return it.

## Alternative considered

I first went at this by exporting `core/internal/grpcserver` and adding
a public constructor for the store, so the v2 stack could be assembled
outside `package main` (#2543). This is much smaller, keeps the
internals internal, and covers the actual need — so I've closed that one
in favour of this.

The larger export still has an independent argument: it would let an
external module stand up the gRPC surface with a real Postgres and fakes
for `a2agateway`'s dialer and `agentinstance`'s actor client, and test
AgentInstance lifecycle, ownership checks, A2A routing and
`DefaultMethodPolicies` without a cluster or Substrate. Happy to revive
it if that's wanted, but it shouldn't be bundled with this.

## Testing

`go build ./...`, `go vet`, and `go test ./core/pkg/app/
./core/internal/grpcserver/` all pass.

Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant