Skip to content

refactor: read and write config toml in TypeScript#3662

Open
christierney wants to merge 26 commits intomainfrom
chris/configurations
Open

refactor: read and write config toml in TypeScript#3662
christierney wants to merge 26 commits intomainfrom
chris/configurations

Conversation

@christierney
Copy link
Collaborator

@christierney christierney commented Mar 6, 2026

Intent

Migrate configuration reading and writing from Go API calls to a pure TypeScript toml module in the VSCode extension. This removes three HTTP round-trips to the Go backend for operations that only need local file I/O, and moves us toward reducing the extension's dependency on the Go subprocess.

Type of Change

  • Refactor
  • Bug fix (schema misspelling)

Approach

Updated type definitions and the schema to match current behavior:

  • Consolidated TS types to match Go and schema (remove dead types/fields, add missing ones)
  • Removed dead Go types (Schedule, AccessType, ConnectAccessControl) and unreachable code in content.go
  • Fixed schema: auth_type in integration_requests (was camelCase), add additionalProperties: false to integration_requests items

Added a new src/toml/ module with seven files:

  • errors.tsConfigurationLoadError class and error factory functions
  • convertKeys.ts — Bidirectional snake_case ↔ camelCase key conversion, preserving user-defined keys in environment and integration_requests[].config
  • compliance.ts — Port of Go's ForceProductTypeCompliance() for Connect Cloud
  • loader.ts — Reads TOML, validates against the JSON schema (using ajv), applies defaults
  • writer.ts — Validates, converts to snake_case, strips empty leaf values, writes TOML
  • discovery.ts — File discovery (listConfigFiles), single/batch/recursive loading, path resolution
  • validate.ts — Shared setup for TOML validator function

The public API surface is five exports: loadConfiguration, loadAllConfigurations, loadAllConfigurationsRecursive, writeConfigToFile, and ConfigurationLoadError. Internal helpers stay exported on their source files for intra-module use and tests, but are not re-exported from the barrel.

All functions that produce Configuration objects accept (projectDir, rootDir) where projectDir is relative (e.g., "." or "subdir") and rootDir is the absolute workspace root. Path resolution to absolute paths for file I/O happens inside the module. This matches the Go convention where ProjectDirFromRequest resolves relative dir parameters against the workspace base.

After replacing all callsites, the three Go API handlers (GET /configurations, GET /configurations/{name}, PUT /configurations/{name}) and their tests were deleted. The shared configDTO/configLocation types remain as they're used by other endpoints.

A copy of the JSON schema is bundled at src/toml/schemas/posit-publishing-schema-v3.json, identical to the Go source of truth. A comment in the schema test notes the need to keep them in sync.

Key design decisions:

  • No intermediate store — callsites call toml functions directly
  • Writer returns the written Configuration — no write-then-read round-trip
  • Loader throws on error; discovery functions catch and collect errors for the UI
  • src/toml/ has no vscode imports — it's pure Node.js
  • stripEmpty only removes leaf values (undefined, null, empty strings), never parent objects, because the schema conditionally requires section headers like [r] even when all fields are empty

User Impact

No user-facing behavior change. Configuration reading and writing now happens directly in the extension process instead of through HTTP calls to the Go backend.

Automated Tests

[ed: there's probably more now, I've made some improvements since ]

a bunch of new tests in src/toml/:

  • Key conversion (19 tests) — snake/camel round-trip, preservation of user-defined keys
  • Schema validation (32 tests) — validates the bundled JSON schema against known valid/invalid configs
  • Loader (19 tests) — happy path, TOML parse errors, schema errors, defaults, comments, business rules
  • Writer (14 tests) — round-trip fidelity, snake_case output, comments, stripEmpty, compliance, unknown type handling, empty section preservation
  • Discovery (22 tests) — file listing, single/batch/recursive loading, relative projectDir, error collection, skip rules
  • Compliance (13 tests) — cleanup and field modifications based on target product (connect or cloud)
  • Errors (13 tests) — error factories and formatting of validation error messages

Existing state.test.ts tests were rewritten to mock the toml module instead of the API client.

Directions for Reviewers

I recommend reading through the new toml/ directory first and then reviewing the updates to the extension code.

I am working my way through these manual tests:

  • Open workspace with existing config — sidebar loads, no errors
  • Open workspace with configs in a subdirectory — configs discovered and selectable
  • Open workspace with no configs — loads cleanly
  • Open workspace with malformed .toml — config shows error state, no crash
  • Select a deployment — config loads, files/packages/secrets populate (no 404s)
  • Open config dropdown — shows configs matching current entrypoint
  • New deployment for Python app — config created with [python] section, snake_case keys
  • New deployment for R Shiny app — config created with [r] section, no schema error
  • Re-associate: select existing config — config list loads
  • Re-associate: create new config — file written, deployment uses it
  • Subdirectory project — all above work, no absolute paths in debug console

Checklist

  • I have updated the root CHANGELOG.md to cover notable changes.

yes, only for the auth_type schema fix. The rest should not be user-facing.

christierney and others added 13 commits March 6, 2026 17:20
Add a TypeScript-native TOML configuration loader that matches Go's
config.FromFile behavior, as a building block for migrating away from
the Go backend.

- Add smol-toml, ajv, ajv-formats dependencies for TOML parsing and
  JSON Schema validation
- Create src/toml/ module: loader, key converter, error factories,
  and JSON schema copy
- Consolidate TS types to match Go and schema (remove dead types/fields,
  add missing ones)
- Remove dead Go types (Schedule, AccessType, ConnectAccessControl) and
  unreachable code in content.go
- Fix schema: auth_type in integration_requests (was camelCase),
  add additionalProperties: false to integration_requests items

Fixes #3651
Fixes #3652

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Jordan Jensen <jordan.jensen@posit.co>
Complete the toml module with:
- convertKeysToSnakeCase: reverse of camelCase converter for writing TOML
- forceProductTypeCompliance: port of Go's compliance logic for Connect Cloud
- writeConfigToFile: validates, transforms, and writes config TOML files
- discovery functions: listConfigFiles, loadConfiguration, loadAllConfigurations,
  loadAllConfigurationsRecursive for finding and loading configs from disk
- barrel exports in index.ts

Also removes unnecessary type assertions across test files, replacing
`as ConfigurationLoadError` with `instanceof` checks per project conventions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace all callsites of api.configurations.get, getAll, and
createOrUpdate with direct calls to the TypeScript toml module:

- state.ts: getSelectedConfiguration uses loadConfiguration,
  refreshConfigurations uses loadAllConfigurationsRecursive
- newDeployment.ts: uses loadAllConfigurations + writeConfigToFile
- selectNewOrExistingConfig.ts: uses loadAllConfigurations + writeConfigToFile
- homeView.ts: uses loadAllConfigurations with entrypoint filtering

Remove get, getAll, and createOrUpdate from Configurations API class
(inspect is kept as it still requires the Go backend).

Update state.test.ts to mock toml module instead of API client.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The toml module was storing absolute paths in Configuration.projectDir,
but the Go backend's ProjectDirFromRequest resolves dir query parameters
relative to the workspace root. This caused 404 errors from Go API
endpoints (files, secrets, packages, etc.) that still receive projectDir.

Discovery functions now accept a rootDir parameter and compute relative
paths (e.g., "." or "subdir") for Configuration metadata, while using
absolute paths internally for file I/O.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… + rootDir

writeConfigToFile now takes (configName, projectDir, rootDir, config) instead
of (configPath, projectDir, config), matching the signature pattern of
loadConfiguration and loadAllConfigurations. This eliminates the need for
callers to compute absolute paths and call getConfigPath separately, removing
a class of path-semantic mismatches. getConfigPath is no longer exported.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove internal-only symbols (loadConfigFromFile, forceProductTypeCompliance,
convertKeysToCamelCase, convertKeysToSnakeCase, getConfigDir, listConfigFiles)
from the barrel file. These remain exported on their source files for
intra-module use and direct test imports, but are no longer part of the
module's public API.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
stripEmpty was removing parent objects (like `r: {}`) after stripping
their empty-string children. The JSON schema conditionally requires
these sections (e.g., `r` for R content types), so removing them caused
validation failures when creating new R Shiny configurations.

Go's TOML encoder writes section headers like `[r]` even when all
fields are omitted via omitempty. Match that behavior by only stripping
leaf values (undefined, null, empty strings), never parent objects.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tions

Error location strings in getSummaryStringFromError still referenced the
removed Go API methods (configurations.getAll, configurations.createOrUpdate).
Updated to reference the toml module functions that replaced them.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These endpoints are no longer called by the extension — configuration
reading and writing is now handled by the TypeScript toml module.

Deletes:
- GET /api/configurations (GetConfigurationsHandlerFunc)
- GET /api/configurations/{name} (GetConfigurationHandlerFunc)
- PUT /api/configurations/{name} (PutConfigurationHandlerFunc)

Retains configDTO and configLocation types in get_configurations.go
as they are still used by other handlers (files, secrets, integration
requests).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@christierney christierney changed the title Chris/configurations refactor: read and write config toml in TypeScript Mar 9, 2026
@christierney christierney marked this pull request as ready for review March 9, 2026 15:04
@christierney christierney requested a review from a team as a code owner March 9, 2026 15:04
christierney and others added 2 commits March 9, 2026 12:02
These directories can be large in Python/R projects and will never
contain .posit/publish/ configuration files. Skipping them avoids
unnecessary filesystem traversal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Format ajv validation errors with "key: problem" style matching Go's
jsonschema library (e.g., "invalidParam: not allowed." instead of
"must NOT have unevaluated properties"). Also drop the file path
prefix from the error msg field, matching Go's AgentError format.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
*
* Skips:
* - Dot-directories (except .posit itself)
* - node_modules, __pycache__, renv, packrat
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Go code has a more complicated setup for excluding files: see https://github.com/posit-dev/publisher/blob/main/internal/bundles/matcher/walker.go

That code is also used for excluding files from bundles, etc. It didn't seem worth implementing all that for just loading configs and the above skiplist seemed sufficient to me, but I'm interested in feedback on this point.

@christierney christierney marked this pull request as draft March 9, 2026 18:08
christierney and others added 2 commits March 9, 2026 14:36
…ionErrors

Port Go's entrypointObjectRef handling to TypeScript compliance:
- For Connect, copy entrypointObjectRef to entrypoint (object-reference style)
- Always clear entrypointObjectRef before validation (non-TOML field)
- Delete entrypointObjectRef in writer alongside comments/alternatives

Move formatValidationErrors from loader.ts to errors.ts so both loader
and writer produce Go-compatible schema error messages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The redundancy filter was using pathKey (the parent object path from
ajv's instancePath) for prefix comparison. For root-level errors,
pathKey is "" which matches everything, incorrectly dropping
unevaluatedProperties errors whenever any other error existed.

Use fullKey (including the property name) to match Go's behavior,
where InstanceLocation points to the property itself. Now only a
deeper error at "python.garbage.something" filters out the
"python.garbage: not allowed." error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Filters redundant unevaluatedProperties errors when a more specific
* error exists at the same or deeper path (matching Go's behavior).
*/
export function formatValidationErrors(errors: ErrorObject[]): string {
Copy link
Collaborator Author

@christierney christierney Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is attempting to produce useful error messages from TOML validation failures, and to make them match what the Go validator returns (e.g. <key>: not allowed or <key>: missing property). The e2e tests look for these specific error messages. We could decide to let the messages from ajv through if we want, but this keeps the behavior and test changes smaller. In any case we need to do some processing to extract actionable info from the array of validation errors, even if we don't coerce the messages into the Go form.

const rawConfigs = response.data;
// remove the errors
configurations = configurations.filter(
(cfg): cfg is Configuration => !isConfigurationError(cfg),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was redundant as filterConfigurationToValidAndType already filters out errors

christierney and others added 5 commits March 9, 2026 16:17
…checks

workspaces.path() returns string | undefined. Four call sites used
the non-null assertion (!) assuming a workspace must be open. Replace
with explicit undefined checks and early returns, matching the
existing pattern in state.ts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… depth limit

Parallelize config file loading with Promise.allSettled, eliminate an
extra fs.stat per directory by checking readdir entries, remove the
abs→rel→abs path round-trip in walkForConfigs, and add a depth limit
(20) to prevent runaway walks in pathological directory trees.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… last

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a local loadError closure that captures location, replacing 4
repeated new ConfigurationLoadError(createConfigurationError(..., location))
patterns with single-line throw statements.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
christierney and others added 2 commits March 9, 2026 19:01
Extract shared convertKeys() helper that both convertKeysToCamelCase and
convertKeysToSnakeCase delegate to, eliminating near-identical code.
In writer.ts, reuse getConfigPath from discovery and add loadError
closure matching the loader.ts pattern.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@christierney christierney marked this pull request as ready for review March 9, 2026 23:21
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@christierney christierney force-pushed the chris/configurations branch from 56943cb to 745207f Compare March 10, 2026 00:42
The "Create New Configuration For Destination" command was writing
configs with product_type = '' because the Go inspect API doesn't
populate productType. The newDeployment flow already derived it from
the credential's serverType via getProductType — apply the same
pattern here using activeDeployment.serverType.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +465 to +467
selectedInspectionResult.configuration.productType = getProductType(
activeDeployment.serverType,
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a fix for a pre-existing bug I found while testing. This path was not setting a product type, so you'd end up with an invalid config TOML (with 'product_type = '').

The new deployment flow already does this:

newDeploymentData.entrypoint.inspectionResult.configuration.productType =
getProductType(newOrSelectedCredential.serverType);

on:
pull_request:
workflow_dispatch:
inputs:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixing this!

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.

2 participants