Skip to content

refactor(server): simplify resource ownership and package boundaries - #11

Merged
ziflex merged 1 commit into
mainfrom
feat/boundary-cleanup
Sep 6, 2026
Merged

ziflex merged 1 commit into
mainfrom
feat/boundary-cleanup

Conversation

@ziflex

@ziflex ziflex commented Sep 6, 2026

Copy link
Copy Markdown
Member

The server previously coordinated each resource through global registries, owner indexes, operation managers, and a cross-resource lifecycle manager. This cleanup scopes resource storage to each logical Connect connection and puts creation and teardown on the resource owners. Multiple logical connections may continue sharing one transport.

Ownership and removed abstractions

  • Keep ConnectionRegistry for connection indexing, capacity, and shutdown admission. Connection owns its identity and cancellation lifetime plus one ResourceStore.
  • ResourceStore owns typed resource maps, reservations, registration, lookup, removal, and parent links. Plans retain descendants; normal sessions retain their active execution. Remove PlanRegistry, SessionRegistry, ExecutionRegistry, and DebugSessionRegistry, including global owner indexes and redundant owner IDs.
  • Replace Compiler, Executor, and Debugger with root CompilePlan/Run functions taking the borrowed api.Runtime explicitly and creation methods on Plan/Session.
  • Remove Lifecycle. Resources release their descendants before closing their hosted handle; the store reclaims membership and capacity. Pending, active, and closing resources remain charged through settlement. Concurrent release callers share the retained result; completed IDs become stale without permanent tombstones.
  • Remove DebugController. DebugSession directly owns the hosted debugger, poisoning, and exactly-once close. Retain the bounded event stream, breakpoint bookkeeping, cohesive debugger state, and server-internal close primitive.
  • The store mutex protects membership, reservations, admission, and parent links. Resource-state locks never acquire it. Hosted calls, cleanup waits, and recursive release run outside it.

Adapter and failure boundaries

Keep all five gRPC service adapters. Replace core.Context/operationContextFactory with ordinary contexts and prepareOperation; preserve values, deadlines, cancellation causes, and callback detachment. Pass canonical sources and API options into core, remove input envelopes and identity-bearing snapshots, and group conversions by responsibility within the adapter.

Resource owners construct terminal failures once. Share canonical diagnostic extraction and category serialization while retaining typed immediate errors, joined cleanup errors, cancellation/deadline distinctions, and host-error redaction. panicboundary remains under server/internal and wraps only hosted API calls; option preparation and Wire bookkeeping stay outside it. RPC recovery and detached cleanup retain distinct policies. Constructor panics preserve reusable parents; stateful session/debugger panics prevent reuse.

The shared packages remain pkg/execution (Wire asynchronous states, snapshots, events), pkg/debugger (Wire lifecycle snapshots and ordered events), and pkg/failure (sanitized failures and categories). Client production code and its four exports, cleanup escalation, and borrowed transport policies are unchanged.

Intentional Go API migrations

func NewServer(runtime api.Runtime, options ...Option) (*Server, error)

type RuntimeIdentity struct {
    Name       string
    Version    string
    InstanceID string
}

func WithRuntimeIdentity(identity RuntimeIdentity) Option

Replace server.Runtime with api.Runtime and execution.Identity with server.RuntimeIdentity. Both old types are deleted without aliases. Nil and typed-nil runtime rejection remains covered. Handshake-only metadata now belongs to transport configuration. Runtime/listener ownership, protobuf, Universal API behavior, limits, cancellation, and failure semantics are preserved.

Tests and review

Preserve integration/protocol assertions and migrate lifecycle coverage to the new owners. Remove obsolete topology/reflection and controller tests; retain their behavioral coverage in owner, protocol, and public integration tests. Added tests cover shared-transport connection isolation and independent limits, closing connection capacity, parent release racing session publication and abandoned cleanup, every hosted debugger panic boundary, concurrent retained debugger close, detached inspection slices, and operation-context causes/callback cleanup.

Updated README migration examples, architecture/protocol documentation, AGENTS.md, and API comments. Completed the full-diff self-review. It removed leftover duplicate normal-session/execution close coordination, placed registration/reclamation in the store, preserved Execute's existing snapshot behavior, and centralized repeated adapter conversions. Affected validation was rerun after corrections.

Validation

Passed locally on macOS arm64 with Go 1.26.5 (the module's Go 1.25 minimum is unchanged):

go test ./server/internal/core ./server/internal/grpcserver ./server
make fmt
make check-fmt
make check-tidy
make vet
make test
make test-race
make build
make proto-lint
make check-generate
make proto-breaking BUF_BREAKING_AGAINST=.git#branch=main

Fetched origin/main and verified local main matched 3388848 before the breaking check. Buf required network-enabled execution for its pinned Go tools. Protobuf sources, generated bindings, dependencies, and client implementation are unchanged. No required local gate was skipped. Hosted CI remains responsible for the configured Go 1.25 and Linux/Windows coverage; those environments were not run locally.

Benchmarks

Apple M2 Max, darwin/arm64; medians of three samples, original captured baseline to final code:

Benchmark ns/op before → after B/op before → after allocs/op before → after
Event publication, no watchers 24.93 → 25.22 0 → 0 0 → 0
Event publication, one watcher 67.59 → 67.94 0 → 0 0 → 0
Cancellation lookup/snapshot 63.02 → 65.77 48 → 48 1 → 1
Core durable session 5,423 → 3,868 2,632 → 1,600 43 → 33
Public durable-session round trip 84,974 → 82,377 36,508 → 35,449 597 → 586

Core durable-session samples reported 32–33 allocations. Cancellation's final median is 2.75 ns higher (+4.4%); lookup now checks retained release state. Earlier runs showed a 5–7 ns increase, with unchanged allocation cost. An initial public round-trip slowdown was investigated with an unchanged baseline checkout and five-sample back-to-back runs: median 86,835 → 79,675 ns/op. It did not reproduce in the final run. These small samples are local measurements, not cross-platform performance guarantees.

go test ./server/internal/core ./server -run '^$'   -bench '^Benchmark(CancelExecution|RunDurableSession|RuntimeAdapterDurableSession|ExecutionEventPublication)$'   -benchmem -count=3

The baseline's cancellation and core durable-session benchmarks used the old BenchmarkExecutorCancelExecution and BenchmarkExecutorRunDurableSession names. No protocol features, dependency upgrades, compatibility shims, native-runtime integration, or new framework machinery are included.

Copilot AI lite review requested due to automatic review settings September 6, 2026 03:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ziflex
ziflex merged commit 6f754e2 into main Sep 6, 2026
11 checks passed
@ziflex
ziflex deleted the feat/boundary-cleanup branch September 6, 2026 03:09
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.

2 participants