Skip to content

refactor(rpc): one typed contract for every RPC call - #917

Open
omartelo wants to merge 11 commits into
mainfrom
refactor/rpc-typed-contract
Open

omartelo wants to merge 11 commits into
mainfrom
refactor/rpc-typed-contract

Conversation

@omartelo

Copy link
Copy Markdown
Collaborator

Summary

The RPC had no contract a compiler could check. A client named a method by string and passed ...any, the backend matched the arguments by position through reflection, and the window's types were hand-written mirrors of the backend's. Two strings swapped in a call compiled, and a JSON tag renamed on one side broke the other at run time.

Every method is now declared once, in a new public package, api:

var ThemesSelect = define[SelectThemeOptions, None]("themes.Select")
  • Backend: rpc.Handle(d, api.ThemesSelect, fn) only compiles when fn takes and returns those types. The routes live in internal/routes, one function per service. Reflection, Register and the denyInternal list are gone: a method nothing routes cannot be reached, so the methods kept out for Go callers stay out by construction (the package doc names them).
  • Clients: lich, lich mcp and the window call callMethod(ctx, c, api.X, opts). It is the only way either client posts, so a call by name no longer exists to get wrong.
  • One options object per call: the window's calls were positional; every request is now one struct, as the CLI's already were. On the wire the envelope is the same ([options] or []), so a lich of another release still talks to this backend, and a route reads an empty list as the zero options.
  • No more mirrors: native/backend aliases api's types. Only the /events payloads are still hand-owned.
  • Golden: api/testdata/contract.golden records every method's name, options and result with each JSON field. A change to the contract shows up there in review (go test ./api -update), and TestEveryRequestIsOneObject keeps positional arguments out.
  • native/lichclient is renamed to native/backend.

Test plan

  • api: the golden, one struct per request, the envelope Args writes, a duplicate declaration refused.
  • internal/rpc: options decoded by name, an empty list as zero options, None answered as null, anything but one object refused with 400, the service's error as 500, the request's context handed over, an unknown method as 404.
  • internal/routes: every method api declares has a route, per service, and every xdgprofile provider has its profile methods declared.
  • internal/cli/wire_test.go posts as a client of another release through the production routes.
  • native/: the window's tests and fakes read the options objects.
  • Rig on its own backend and display: the window loads the workspace, runs a shell and takes typed input, opens a project, lists and previews files, searches text, shows the review, lists pull requests with their overview and files, and opens Settings (providers, profiles and plan usage, appearance, sandbox, version control, updates). lich version, sessions, theme list, theme switch, open, worktrees and lich mcp (list_sessions, list_worktrees) answer; the backend log shows no refused call.
  • Local gate: gofmt, go vet ./..., go test ./... on the backend and in native/; backend coverage 90.5%.

This branch has not been deployed

No deployments
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