Skip to content

feat: migrate configuration get to TypeScript#3614

Draft
christierney wants to merge 1 commit intomainfrom
chris/ts-refactor-outside-in
Draft

feat: migrate configuration get to TypeScript#3614
christierney wants to merge 1 commit intomainfrom
chris/ts-refactor-outside-in

Conversation

@christierney
Copy link
Collaborator

Replace the HTTP round-trip to the Go backend for loading a single configuration with an in-process TypeScript implementation. The new FSConfigurationStore reads TOML from disk, validates against the JSON schema, and converts snake_case keys to camelCase — matching the Go API's behavior without the HTTP overhead.

New modules:

  • core/ports.ts: ConfigurationStore interface
  • core/errors.ts: error factory functions for parse/validation errors
  • core/conversion.ts: recursive snake_case→camelCase key converter
  • adapters/fs-configuration-store.ts: filesystem implementation

Replace the HTTP round-trip to the Go backend for loading a single
configuration with an in-process TypeScript implementation. The new
FSConfigurationStore reads TOML from disk, validates against the JSON
schema, and converts snake_case keys to camelCase — matching the Go
API's behavior without the HTTP overhead.

New modules:
- core/ports.ts: ConfigurationStore interface
- core/errors.ts: error factory functions for parse/validation errors
- core/conversion.ts: recursive snake_case→camelCase key converter
- adapters/fs-configuration-store.ts: filesystem implementation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@christierney
Copy link
Collaborator Author

Claude's self review:

⏺ Here's my review. Three things I'd flag:

  1. Schema duplication is a maintenance risk. We copied posit-publishing-schema-v3.json into src/core/schemas/. Now there are two copies that can drift silently. This should either be a symlink, a build-step copy (e.g., in the justfile),
    or at minimum a comment in both locations pointing at the other. As-is, someone updating the Go schema won't know they need to update the TS copy.

  2. errors.ts manufactures Go-backend error types. The createInvalidTOMLError and createSchemaValidationError functions produce AgentError objects — types designed to represent errors from the Go backend (code, msg, operation, data). This
    works today because the rest of the extension already has type guards and display logic for these shapes. But it's conceptually odd: pure TypeScript code forging objects that look like they came from a Go HTTP response. For a proof of
    concept migrating one call, this is pragmatic — it avoids touching downstream error handling. But if more calls migrate, this coupling becomes a problem. Worth noting as tech debt.

  3. The convertKeysToCamelCase environment exemption is implicit knowledge. The function hardcodes that keys inside environment should not be converted. This mirrors Go's behavior (the Go struct has Environment as map[string]string, so
    its keys pass through unchanged). But there's nothing connecting this rule to its source — if Go adds another user-defined-key map, someone would need to know to update the TS converter too. A comment referencing the Go struct would
    help, or an explicit allowlist parameter instead of a hardcoded check.

Everything else looks solid for the stated scope. The core/adapters split is clean, the workspace root injection we fixed is the right pattern, and the test coverage is thorough. The change to state.ts is minimal — it swaps one call and
adjusts error handling without touching surrounding code.

@christierney
Copy link
Collaborator Author

An interesting wrinkle that this caught: the Go subprocess has knowledge of the workspace baseDir because it is passed in at startup:

const path = workspaces.path();
// Create command to send to terminal stdin
const [command, args] = await commands.create(
context,
path!,

That base dir is passed along to lots of the Go API handlers:

func RouterHandlerFunc(base util.AbsolutePath, lister accounts.AccountList, log logging.Logger, eventServer *sse.Server, emitter events.Emitter) http.HandlerFunc {

So that will be a common requirement for a lot of our TypeScript replacements.

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