-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add fastify + connectrpc for SDK clients #1328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Greptile OverviewGreptile SummaryIntroduced a Fastify server with ConnectRPC integration and Buf protobuf codegen to enable a typed RPC layer for future client/server split. Added a minimal Ping service as the foundation.
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client
participant Fastify Server
participant ConnectRPC Router
participant Validate Interceptor
participant Ping Handler
participant Zod Schema
Client->>Fastify Server: POST /stagehand.v1.StagehandPingService/Ping
Fastify Server->>ConnectRPC Router: Route request
ConnectRPC Router->>Ping Handler: Call ping(req)
Ping Handler->>Zod Schema: ensureValidRequest(req)
Zod Schema-->>Ping Handler: Validated request
Ping Handler->>Ping Handler: Build response message
Ping Handler->>Zod Schema: ensureValidResponse(response)
Zod Schema-->>Ping Handler: Validated response
Ping Handler-->>Validate Interceptor: Return response
Validate Interceptor->>Validate Interceptor: Proto validation (min_len, max_len)
Validate Interceptor-->>ConnectRPC Router: Validated response
ConnectRPC Router-->>Fastify Server: Response
Fastify Server-->>Client: JSON response
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
12 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 issues found across 14 files
Prompt for AI agents (all 5 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/proto/stagehand/v1/ping.proto">
<violation number="1" location="packages/core/proto/stagehand/v1/ping.proto:7">
P2: PingRequest is defined without a min-length constraint even though the server rejects empty messages, so the proto no longer matches the actual API behavior.</violation>
<violation number="2" location="packages/core/proto/stagehand/v1/ping.proto:11">
P2: PingResponse advertises a 250-character maximum, but the server never enforces that limit and can emit longer strings, so the new constraint makes the API contract incorrect.</violation>
</file>
<file name="packages/core/server/server.ts">
<violation number="1" location="packages/core/server/server.ts:20">
P2: Handle rejections from `main()` so server startup failures (such as port conflicts) don’t crash with an unhandled promise rejection.</violation>
</file>
<file name="packages/core/package.json">
<violation number="1" location="packages/core/package.json:55">
P2: Move @bufbuild/buf to devDependencies; it is only needed for the rpc:* build scripts, not by library consumers at runtime.</violation>
<violation number="2" location="packages/core/package.json:57">
P2: Move @bufbuild/protoc-gen-es to devDependencies; it is only used during buf code generation and should not be shipped to consumers.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
…ork due to serviceDesc changing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 12 files (reviewed changes from recent commits).
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/server/schema/ping.ts">
<violation number="1" location="packages/core/server/schema/ping.ts:9">
P2: `timestampShape` never caps `nanos`, so values ≥ 1 000 000 000 slip through even though protobuf timestamps forbid them. This lets invalid ping payloads pass validation and silently alters their time when rebuilding a Timestamp.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
- Removed unnecessary client.ts example - moved logic into test
# Conflicts: # packages/core/server/schema/ping.ts
why
We want a fastify server & connectrpc set up so that we can create a client/server split
what changed
Added fastify server + connectrpc + buf and an example "ping" handler.
pnpm devwas added - this starts the fastify server.pnpm rpc:generatewas added - this runs the codegen when we change the schema (we'll want to havetest plan
pnpm rpc:checkcommand which is run in CI, so if someone changes the proto but forgets to runrpc:generate, the mismatch will be caught.Summary by cubic
Sets up a Fastify server with ConnectRPC and Buf codegen, and adds a typed Ping RPC using google.protobuf.Timestamp. Adds validation via ConnectRPC and tests for RTT, latency, and clock offset.
Why:
What:
Test Plan:
Written for commit 159b27f. Summary will update automatically on new commits.