Skip to content

Conversation

@monadoid
Copy link
Contributor

@monadoid monadoid commented Dec 1, 2025

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 dev was added - this starts the fastify server.
pnpm rpc:generate was added - this runs the codegen when we change the schema (we'll want to have

test plan

  • Added vitest tests to spin up a test server and test the basic "ping" handler works and will reject if invalid data is passed
  • Added an pnpm rpc:check command which is run in CI, so if someone changes the proto but forgets to run rpc: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:

  • Kick off a clean client/server split with a typed RPC layer.
  • Provide a simple Ping endpoint for time sync.

What:

  • Fastify server with ConnectRPC and validate interceptor.
  • Proto: StagehandPingService.Ping using Timestamp; TS generated via protoc-gen-es.
  • Handler echoes client_send_time and sets server_send_time with timestampNow().
  • Scripts: dev, rpc:deps, rpc:lint, rpc:generate.
  • Ignore generated code in packages/core/gen for Prettier/ESLint.

Test Plan:

  • Vitest spins a Fastify + ConnectRPC server on an ephemeral port; asserts client_send_time echo and server_send_time >= client time.
  • Invalid timestamp structure is rejected with Code.Internal via validate interceptor.
  • Manual: pnpm dev to start server; call Ping with any Connect client and verify RTT/latency/offset.

Written for commit 159b27f. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 1, 2025

⚠️ No Changeset found

Latest commit: 159b27f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 1, 2025

Greptile Overview

Greptile Summary

Introduced 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.

  • Added Fastify server on port 8080 with ConnectRPC plugin and protovalidate interceptor
  • Defined stagehand.v1.StagehandPingService proto with buf.validate constraints on response
  • Implemented ping handler with dual validation: Zod for request, proto-based for response
  • Added CI check (rpc:check) to ensure proto definitions and generated TypeScript stay in sync
  • Added vitest tests covering happy path and invalid payload rejection

Confidence Score: 5/5

  • This PR is safe to merge - it adds new server infrastructure without modifying existing functionality
  • The changes are additive, introducing a new Fastify server and ConnectRPC setup. No existing code is modified. The implementation follows standard patterns for ConnectRPC with proper validation at both Zod and proto levels. Tests cover basic functionality.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
packages/core/server/server.ts 5/5 Added Fastify server with ConnectRPC plugin, validate interceptor, and a simple "Hello World" root route on port 8080
packages/core/server/connect.ts 5/5 Added ConnectRPC route handler for StagehandPingService with Zod-based request/response validation
packages/core/server/schema/ping.ts 4/5 Added Zod schemas for ping request/response; request validates message min length, response has no max length constraint unlike proto
packages/core/proto/stagehand/v1/ping.proto 5/5 Added proto definition for StagehandPingService with buf.validate constraints on response message
packages/core/tests/server/ping.test.ts 4/5 Added vitest tests for ping handler; tests basic functionality and validation rejection
packages/core/package.json 5/5 Added new scripts (dev, client, rpc:*) and dependencies for Fastify, ConnectRPC, and Buf
.github/workflows/ci.yml 5/5 Added rpc:check step to verify proto schemas and generated code stay in sync

Sequence Diagram

sequenceDiagram
    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
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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

Edit Code Review Agent Settings | Greptile

@pirate pirate changed the title Add fastify Add fastify + connectrpc for SDK clients Dec 1, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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
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.

3 participants