feat: implement TypeScript Connect API client#3673
Conversation
Replace the stub TypeScriptDirectClient with a full implementation that makes HTTP requests directly to the Connect server, mirroring the Go client's behavior. All 15 API methods are implemented and pass the same 68 contract tests that validate the Go client. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the TypeScript Connect API client from the contract test file into a standalone @posit-dev/connect-client package with proper types, error classes, and barrel exports. The contract test adapter becomes a thin wrapper that imports from the new package. All 68 contract tests pass with both Go and TypeScript backends. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a matrix strategy to the contract-tests job so it runs once with API_BACKEND=go (existing behavior) and once with API_BACKEND=typescript (new ConnectClient from @posit-dev/connect-client). The TypeScript backend skips Go setup since it doesn't need the harness binary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| "dependencies": { | ||
| "@posit-dev/connect-client": "file:../../packages/connect-client" | ||
| }, |
There was a problem hiding this comment.
If we continue down this path (which I think we should) we should consider doing a few things:
- create a shared
tsconfigby creating a TypeScript configuration package. The Turborepo docs have some ideas on how to do this. This will prevent us from duplicatingtsconfig.jsonsetups everywhere.- We actually have an old issue for this: Setup shared TypeScript configurations for monorepo packages #1295
- we should also consider using
npm workspaces: Setup npm workspaces for shared code #1291- We can wait until we ditch Go, and can raise the extension to the top level of the repo
I think its a good idea to have separate packages. It makes the way we import and deal with some of the code around the homeView much easier to deal with. Right now it imports types from extension/vscode by importing with ../../.. which is not great.
There was a problem hiding this comment.
@dotNomad would you say creating a shared configuration package should be part of this PR, or part of follow-up work?
| // HTTP helpers | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| private async request( |
There was a problem hiding this comment.
I would prefer to keep using axios despite some of the problems we've had so we don't have to write our own error handling logic, or change how we do it in a lot of spots. That would simplify what we have here quite a bit.
packages/connect-api/src/client.ts
Outdated
| return { | ||
| id: dto.guid, | ||
| username: dto.username, | ||
| first_name: dto.first_name, | ||
| last_name: dto.last_name, | ||
| email: dto.email, | ||
| }; |
There was a problem hiding this comment.
This is a preference in design, but in my opinion it has a lot of benefits:
I would prefer to return the exact thing the API call responds with, including all data, and not creating new objects.
This will keep this code extremely single use and the only thing we will need to adjust overtime are the types. We won't have to adjust the functions since they just pass the response right through. We can always extract the data from the response.
It is much more flexible.
You can read more in our README.md for the Go API client we wrote: https://github.com/posit-dev/publisher/blob/183abd7d744fb8f6d8bcffc323717547521eca57/extensions/vscode/src/api/README.md
There was a problem hiding this comment.
I see this is mapping a UserDTO to a User, but I think we should avoid this. It would make the initial migration easier, but once it is complete it will be much more difficult to understand why the attributes are different.
Down to discuss temporary trade-offs.
| export type ContentID = string & { readonly __brand: "ContentID" }; | ||
| export type BundleID = string & { readonly __brand: "BundleID" }; | ||
| export type TaskID = string & { readonly __brand: "TaskID" }; | ||
| export type UserID = string & { readonly __brand: "UserID" }; | ||
| export type GUID = string & { readonly __brand: "GUID" }; |
There was a problem hiding this comment.
Branded Types are a nice addition to avoid ID overlap 👍
Rename the package from @posit-dev/connect-client to @posit-dev/connect-api and add comprehensive unit tests for the ConnectClient class and error types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Also renames ConnectClientOptions to ConnectAPIOptions and ConnectClientError to ConnectAPIError for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace native fetch with axios for HTTP requests in the ConnectAPI class. Uses axios.create() with baseURL, default Authorization header, and validateStatus: () => true to preserve existing error handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Stop constructing new objects in client methods — return the exact response data from the Connect API instead. Removes the User type (a client-side invention) and updates the contract test adapter to extract fields from full DTOs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| }; | ||
| } | ||
|
|
||
| private async dispatch( |
There was a problem hiding this comment.
once we get rid of the go API client, we can clean up the shape of this stub, or just use the ConnectAPI client directly in test.
Right now the test shape is coupled to the go client's shape so we can have an apples-to-apples comparison in the contract tests, but we won't always need to have the apples-to-apples comparison we're running here.
| }); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // getSettings |
There was a problem hiding this comment.
I could be convinced that the right approach will end up being splitting these out into atomic calls - maybe something we can feel out as we go along.
| * Validates credentials and checks user state (locked, confirmed, role). | ||
| * Returns the full UserDTO on success; throws AuthenticationError otherwise. | ||
| */ | ||
| async testAuthentication(): Promise<UserDTO> { |
There was a problem hiding this comment.
not strictly a light wrapper around an API call, but this does seem pretty useful to have on the API client.
|
|
||
| /** | ||
| * Fetches composite server settings from 7 separate endpoints, | ||
| * mirroring the Go client's GetSettings behavior. |
There was a problem hiding this comment.
Flagging again - not sure if we want to mimic this or not
| // Integration type | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| export interface Integration { |
There was a problem hiding this comment.
@dotNomad is there any convention around calling something vs DTO?
Intent
Implement a TypeScript Connect API client package (
@posit-dev/connect-api) and validate it against contract tests alongside the existing Go backend.Type of Change
Approach
Created a standalone
packages/connect-api/package with aConnectAPIclass that wraps the Posit Connect REST API. All 15 API methods are implemented:TestAuthentication,GetCurrentUserContentDetails,CreateDeployment,UpdateDeploymentGetEnvVars,SetEnvVarsUploadBundle,LatestBundleID,DownloadBundleDeployBundle,WaitForTask,ValidateDeploymentGetIntegrations,GetSettingsKey design decisions:
axios.create()with a pre-configuredbaseURL, defaultAuthorizationheader, andvalidateStatus: () => trueso the client controls error handling rather than axios auto-throwing on non-2xxContentID,BundleID,TaskID) for type safety on parametersUser Impact
None — this is preparatory code.
Automated Tests
packages/connect-api/(vitest) covering all methods, error paths, and URL constructionnpx vitest runandAPI_BACKEND=typescript npx vitest run)🤖 Generated with Claude Code