Skip to content

Simplify the public client API around api.Runtime - #10

Merged
ziflex merged 2 commits into
mainfrom
feat/api-stabilization
Sep 6, 2026
Merged

ziflex merged 2 commits into
mainfrom
feat/api-stabilization

Conversation

@ziflex

@ziflex ziflex commented Sep 5, 2026

Copy link
Copy Markdown
Member

The handwritten client exposed both RPC resource handles and a remote Universal API adapter. This change makes client.New(ctx, conn) return api.Runtime, so consumers use canonical compile, session, run, debugger, and close operations after configuring their transport.

This is an intentional alpha API break. It adds no compatibility shims or version bump.

Export migration

Previous export Result
New(ctx, conn) (*Client, error) New(ctx, conn) (api.Runtime, error); failure returns a nil interface
NewRuntime Removed; use New
Runtime, Session, Output aliases Removed; use canonical api types
Client, Plan, Execution, DebugSession and their methods Removed from the public surface; necessary resource owners remain private
ExecutionEvents, DebugEvents and their methods Private stream implementation details; debugger commands return canonical events
Parameters, CompileOptions, ExecuteOptions, DebugSessionOptions, RunOptions Removed; use canonical option callbacks and parameter maps
RuntimeInfo, Capabilities Removed with metadata access/storage; handshake validation and protocol metadata remain
Error, ErrClosed, ErrExecutionCancelled Preserved with fields, methods, identity, diagnostics, and error-chain behavior

The only package-level exports are now New, Error, ErrClosed, and ErrExecutionCancelled. An exact constructor-signature assertion and public-surface test guard this contract. The surface check reads sources from the package test working directory, so it also works with -trimpath.

Ownership and remaining API gaps

Private connection, plan, session, execution, debugger, and stream owners stay in client. Allocation methods consume the existing private option collectors; unused convenience operations and duplicate option types are deleted. Direct hosted Runtime.Run, reusable plans, durable sessions, sequential reuse, overlapping-run rejection, option ordering/aggregation, omitted optimization levels, encoded output, defensive copies, and debugger event mapping are preserved.

Public Close() error retains bounded detached cleanup, retained results, nearest-owner reclamation, and sibling preservation. Runtime closure never closes the supplied transport. Cancellation, sanitization, portable-input validation, and resource limits retain their existing contracts.

The Universal API has no equivalent structured remote-error taxonomy: Error still exposes a Wire failure category, canonical diagnostics, and an unwrap-able transport cause. Terminal errors remain *failure.Failure; remote execution cancellation remains distinct from caller-context cancellation. The API also does not define a portable parameter subset, so Wire continues validating its transportable values locally. Private execution snapshots and watches serve protocol bookkeeping without adding public semantic types. grpc.ClientConnInterface remains the construction boundary because endpoint, credentials, TLS, and transport limits belong to the caller.

Production server architecture, shared domain packages, API dependency, protobuf sources, and generated bindings are unchanged.

Tests and documentation

Migrated legacy client/server tests to canonical operations while retaining private conversion, stream, ownership, and lifecycle coverage. Added malformed/failed constructor, nil-interface, construction-context, and borrowed-transport checks. Generated RPC coverage preserves Execute, CancelExecution, Terminate, and handshake metadata assertions after removing their facade equivalents. Operation/cleanup errors remain discoverable.

All round-trip integration assertions are unchanged; their harness only renames NewRuntime to New. README examples, package documentation, and client/architecture/protocol documentation describe the canonical resource model and correct the deferred-client-cleanup wording.

Successful local validation (Go 1.26.5, darwin/arm64):

go test ./client ./test/integration/...
make fmt check-fmt check-tidy vet test test-race build
make proto-lint check-generate proto-breaking BUF_BREAKING_AGAINST=.git#branch=main
go test -trimpath ./client -run '^TestPublicSurface

The full test and race targets include the round-trip integration suite. Formatting, vet, full tests/race, and build were rerun after the ownership self-review fix. Buf checks passed with network access for pinned tooling. Generation consistency produced no generated diff; `main` and fetched `origin/main` were both `e2eac36`. Hosted CI supplies the repository's Go 1.25 and Linux/macOS/Windows matrix; those results are separate from this local validation.

### Benchmark and self-review

Repeated `go test ./server -run '^$' -bench '^BenchmarkRuntimeAdapterDurableSession$' -benchmem -count=5` against an isolated source snapshot of `e2eac36` and the final implementation, sequentially on an Apple M2 Max:

| Median of five runs | Baseline | Final |
| --- | ---: | ---: |
| ns/op | 96,405 | 90,078 |
| B/op | 36,472 | 36,476 |
| allocs/op | 597 | 597 |

No material regression. An earlier run overlapped tool compilation and had substantial timing variance, so both sides were repeated in isolation; the observed latency difference is not a claim of a statistically established speedup.

Mandatory complete-diff self-review covered correctness, lifecycle/concurrency, architecture/API, security, compatibility, and scope. It removed an obsolete direct-plan ownership branch, corrected unique session IDs in migrated test fixtures, corrected stale documentation/assertion wording, and corrected public-surface source discovery under `-trimpath`. Affected validation passed after the fixes. No remaining findings.
 -count=1
(cd client && go test -trimpath -run '^TestPublicSurface

The full test and race targets include the round-trip integration suite. Formatting, vet, full tests/race, and build were rerun after the ownership self-review fix. Buf checks passed with network access for pinned tooling. Generation consistency produced no generated diff; `main` and fetched `origin/main` were both `e2eac36`. Hosted CI supplies the repository's Go 1.25 and Linux/macOS/Windows matrix; those results are separate from this local validation.

### Benchmark and self-review

Repeated `go test ./server -run '^$' -bench '^BenchmarkRuntimeAdapterDurableSession$' -benchmem -count=5` against an isolated source snapshot of `e2eac36` and the final implementation, sequentially on an Apple M2 Max:

| Median of five runs | Baseline | Final |
| --- | ---: | ---: |
| ns/op | 96,405 | 90,078 |
| B/op | 36,472 | 36,476 |
| allocs/op | 597 | 597 |

No material regression. An earlier run overlapped tool compilation and had substantial timing variance, so both sides were repeated in isolation; the observed latency difference is not a claim of a statistically established speedup.

Mandatory complete-diff self-review covered correctness, lifecycle/concurrency, architecture/API, security, compatibility, and scope. It removed an obsolete direct-plan ownership branch, corrected unique session IDs in migrated test fixtures, and corrected stale documentation/assertion wording. Affected validation passed after the fixes. No remaining findings.
 -count=1)
make check-fmt test
git diff --check

The full test and race targets include the round-trip integration suite. Formatting, vet, full tests/race, and build were rerun after the ownership self-review fix. Buf checks passed with network access for pinned tooling. Generation consistency produced no generated diff; main and fetched origin/main were both e2eac36. Hosted CI supplies the repository's Go 1.25 and Linux/macOS/Windows matrix; those results are separate from this local validation.

Benchmark and self-review

Repeated go test ./server -run '^$' -bench '^BenchmarkRuntimeAdapterDurableSession$' -benchmem -count=5 against an isolated source snapshot of e2eac36 and the final implementation, sequentially on an Apple M2 Max:

Median of five runs Baseline Final
ns/op 96,405 90,078
B/op 36,472 36,476
allocs/op 597 597

No material regression. An earlier run overlapped tool compilation and had substantial timing variance, so both sides were repeated in isolation; the observed latency difference is not a claim of a statistically established speedup.

Mandatory complete-diff self-review covered correctness, lifecycle/concurrency, architecture/API, security, compatibility, and scope. It removed an obsolete direct-plan ownership branch, corrected unique session IDs in migrated test fixtures, and corrected stale documentation/assertion wording. Affected validation passed after the fixes. No remaining findings.

Copilot AI lite review requested due to automatic review settings September 5, 2026 19:00

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.

🟡 Changes recommended

server/protocol_resource_test.go contains a non-compiling composite literal for map[string]*wirev1.Value (missing &wirev1.Value{...} for the map value).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR intentionally breaks the handwritten Go client alpha API by making client.New(ctx, conn) return the canonical api.Runtime interface and by removing the previously exported lower-level resource handles, option structs, and type aliases so consumers use the Unified API programming model directly over a borrowed gRPC transport.

Changes:

  • Replace the public client surface with a minimal export set (New, Error, ErrClosed, ErrExecutionCancelled) returning api.Runtime.
  • Rework the internal client implementation to keep Wire connection/resource IDs and watch streams private while preserving lifecycle/cleanup semantics.
  • Migrate tests and documentation to the canonical api.Runtime/api.Plan/api.Session operations and add contract guards for the new public surface.
File summaries
File Description
test/integration/harness/harness.go Switch harness runtime construction to client.New.
server/runtime_optimization_presence_test.go Update optimization tests to use canonical compile/debug methods and Plan.Close().
server/runtime_adapter_benchmark_test.go Update benchmark to construct runtime via client.New.
server/protocol_resource_test.go Add protocol-level regression test ensuring RPC operations remain available.
server/integration_test.go Migrate server integration tests to canonical runtime/plan/session APIs and handshake assertions.
README.md Update public usage examples and migration guidance for client.New returning api.Runtime.
docs/protocol.md Update protocol documentation to reflect canonical API projection and private Wire resources.
docs/client.md Rewrite client documentation around client.Newapi.Runtime and remove lower-level facade docs.
docs/architecture.md Update architecture docs to match new client surface and private resource model.
client/session_handle.go Update session handle wiring to new internal connection/plan/execution handle types.
client/runtime.go Make New the public constructor returning api.Runtime; update runtime adapter methods/types.
client/runtime_transport.go Update direct runtime run transport path to use configured session options.
client/runtime_session_options.go Replace public parameter model with canonical map[string]any collection for session options.
client/runtime_contract_test.go Add public-surface guard and constructor contract assertions.
client/runtime_allocation.go Move allocation reclaim to connectionHandle.
client/run_options.go Remove deprecated RunOptions.
client/remote_session.go Update remote session to return api.Output and use new allocation/reclaim plumbing.
client/remote_plan.go Update remote plan to return api.Session and use new session/debug session creation paths.
client/remote_debug_session.go Update remote debugger adapter to wrap new debugSessionHandle.
client/plan.go Replace exported Plan with private planHandle; migrate session/debug session creation APIs internally.
client/plan_test.go Migrate plan tests to canonical session-based execution and new close semantics.
client/params.go Remove exported Parameters type alias; keep encoder accepting map[string]any.
client/ownership_test.go Update ownership tests for new session/execution path and service registration.
client/metadata.go Remove runtime metadata/capabilities types and conversions.
client/metadata_test.go Remove legacy runtime metadata defensive-copy test.
client/lifecycle_test.go Update lifecycle tests to use internal newConnection/planHandle/executionHandle.
client/handle_lifecycle_test.go Update handle lifecycle tests to new session-based execution fixture and handle types.
client/execution.go Replace exported Execution with private executionHandle; remove explicit Cancel RPC and return api.Output.
client/execution_test.go Update execution tests to use new internal execution fixture types.
client/execution_events.go Make execution event receiver private (executionEvents).
client/execute_options.go Remove deprecated ExecuteOptions.
client/errors.go Update comments for new public runtime-centric surface while preserving error identities.
client/doc.go Update package docs to describe canonical api.Runtime surface and private Wire resource model.
client/debug.go Replace exported DebugSession with private debugSessionHandle; remove Stop.
client/debug_test.go Update debug tests for private debug event receiver types.
client/debug_session_options.go Remove deprecated DebugSessionOptions.
client/debug_inspection.go Remove convenience SetBreakpoint; keep canonical SetBreakpointAt on private handle.
client/debug_events.go Make debug event receiver private (debugEvents).
client/constructor_test.go Add constructor failure/borrowing/context-detachment regression tests.
client/compile.go Remove exported Client.Compile wrapper; keep internal compileConfigured on connectionHandle.
client/compile_options.go Remove deprecated CompileOptions struct.
client/client.go Replace exported Client with private connectionHandle and newConnection.
client/client_test.go Migrate “Run owns resources” tests to api.Runtime.Run over client.New and add session service support.
Review details
  • Files reviewed: 43/43 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread server/protocol_resource_test.go
@ziflex
ziflex merged commit 3388848 into main Sep 6, 2026
11 checks passed
@ziflex
ziflex deleted the feat/api-stabilization branch September 6, 2026 01: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