Skip to content

fix(config): allow configuring topo dcs via map, fix pg ssl mode config#4456

Open
MasterPtato wants to merge 1 commit into03-17-fix_api_dont_500_on_cross_dc_responsefrom
03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config
Open

fix(config): allow configuring topo dcs via map, fix pg ssl mode config#4456
MasterPtato wants to merge 1 commit into03-17-fix_api_dont_500_on_cross_dc_responsefrom
03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config

Conversation

@MasterPtato
Copy link
Copy Markdown
Contributor

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app bot commented Mar 18, 2026

🚅 Deployed to the rivet-pr-4456 environment in rivet-frontend

Service Status Web Updated (UTC)
frontend-cloud 😴 Sleeping (View Logs) Web Mar 27, 2026 at 10:08 pm
frontend-inspector 😴 Sleeping (View Logs) Web Mar 24, 2026 at 8:09 am
mcp-hub ✅ Success (View Logs) Web Mar 18, 2026 at 10:18 pm
website ❌ Build Failed (View Logs) Web Mar 18, 2026 at 10:16 pm
ladle ❌ Build Failed (View Logs) Web Mar 18, 2026 at 10:15 pm

Copy link
Copy Markdown
Contributor Author

MasterPtato commented Mar 18, 2026

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 18, 2026

More templates

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4456

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4456

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4456

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4456

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4456

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4456

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4456

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4456

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4456

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4456

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4456

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4456

commit: f6c3742

@claude
Copy link
Copy Markdown

claude bot commented Mar 18, 2026

CLAUDE.md

Important Domain Information

ALWAYS use rivet.dev - NEVER use rivet.gg

  • API endpoint: https://api.rivet.dev
  • Cloud API endpoint: https://cloud-api.rivet.dev
  • Dashboard: https://hub.rivet.dev
  • Documentation: https://rivet.dev/docs

The rivet.gg domain is deprecated and should never be used in this codebase.

ALWAYS use github.com/rivet-dev/rivet - NEVER use rivet-dev/rivetkit or rivet-gg/*

Never modify an existing published *.bare runner protocol version unless explicitly asked to do so.

  • Add a new versioned schema instead, then migrate versioned.rs and related compatibility code to bridge old versions forward.
  • When bumping the protocol version, update PROTOCOL_MK2_VERSION in engine/sdks/rust/runner-protocol/src/lib.rs and PROTOCOL_VERSION in engine/sdks/typescript/runner/src/mod.ts together. Both must match the latest schema version.

Commands

Build Commands

# Build all packages in the workspace
cargo build

# Build a specific package
cargo build -p package-name

# Build with release optimizations
cargo build --release

Test Commands

# Run all tests in the workspace
cargo test

# Run tests for a specific package
cargo test -p package-name

# Run a specific test
cargo test test_name

# Run tests with output displayed
cargo test -- --nocapture

Development Commands

# Format code (enforced by pre-commit hooks)
# cargo fmt
# DO NOT RUN CARGO FMT AUTOMATICALLY (note for humans: we need to run cargo fmt when everything is merged together and make sure lefthook is working)

# Run linter and fix issues
./scripts/cargo/fix.sh

# Check for linting issues
cargo clippy -- -W warnings

Docker Development Environment

# Start the development environment with all services
cd docker/dev
docker-compose up -d

Git Commands

# Use conventional commits with a single-line commit message, no co-author
git commit -m "chore(my-pkg): foo bar"

Never push to main unless explicitly specified by the user.

Dependency Management

pnpm Workspace

  • Use pnpm for all npm-related commands. We're using a pnpm workspace.

SQLite Package

  • Use @rivetkit/sqlite for SQLite WebAssembly support.
  • Do not use the legacy upstream package directly. @rivetkit/sqlite is the maintained fork used in this repository and is sourced from rivet-dev/wa-sqlite.

RivetKit Package Resolutions

The root /package.json contains resolutions that map RivetKit packages to their local workspace versions:

{
  "resolutions": {
    "rivetkit": "workspace:*",
    "@rivetkit/react": "workspace:*",
    "@rivetkit/workflow-engine": "workspace:*",
    // ... other @rivetkit/* packages
  }
}

When adding RivetKit dependencies to examples in /examples/, use * as the version. The root resolutions will automatically resolve these to the local workspace packages:

{
  "dependencies": {
    "rivetkit": "*",
    "@rivetkit/react": "*"
  }
}

If you need to add a new @rivetkit/* package that isn't already in the root resolutions, add it to the resolutions object in /package.json with "workspace:*" as the value. Internal packages like @rivetkit/workflow-engine should be re-exported from rivetkit subpaths (e.g., rivetkit/workflow) rather than added as direct dependencies.

Rust Dependencies

Documentation

  • If you need to look at the documentation for a package, visit https://docs.rs/{package-name}. For example, serde docs live at https://docs.rs/serde/
  • When adding new docs pages, update website/src/sitemap/mod.ts so the page appears in the sidebar.
  • When changing actor/runtime limits or behavior that affects documented limits (for example KV, queue, SQLite, WebSocket, HTTP, or timeouts), update website/src/content/docs/actors/limits.mdx in the same change.

Code Blocks in Docs

  • All TypeScript code blocks in docs are typechecked during the website build. They must be valid, compilable TypeScript.
  • Use <CodeGroup workspace> only when showing multiple related files together (e.g., actors.ts + client.ts). For a single file, use a standalone fenced code block.
  • Code blocks are extracted and typechecked via website/src/integrations/typecheck-code-blocks.ts. Add @nocheck to the code fence to skip typechecking for a block.

Content Frontmatter

Docs (website/src/content/docs/**/*.mdx)

Required frontmatter fields:

  • title (string)
  • description (string)
  • skill (boolean)

Blog + Changelog (website/src/content/posts/**/page.mdx)

Required frontmatter fields:

  • title (string)
  • description (string)
  • author (enum: nathan-flurry, nicholas-kissel, forest-anderson)
  • published (date string)
  • category (enum: changelog, monthly-update, launch-week, technical, guide, frogs)

Optional frontmatter fields:

  • keywords (string array)

Examples

All example READMEs in /examples/ should follow the format defined in .claude/resources/EXAMPLE_TEMPLATE.md.

Notes Tracking

  • When the user asks to track something in a note, store it in .agent/notes/ by default.

Architecture

Monorepo Structure

This is a Rust workspace-based monorepo for Rivet. Key packages and components:

  • Core Engine (packages/core/engine/) - Main orchestration service that coordinates all operations
  • Workflow Engine (packages/common/gasoline/) - Handles complex multi-step operations with reliability and observability
  • Pegboard (packages/core/pegboard/) - Actor/server lifecycle management system
  • Common Packages (/packages/common/) - Foundation utilities, database connections, caching, metrics, logging, health checks, workflow engine core
  • Core Packages (/packages/core/) - Main engine executable, Pegboard actor orchestration, workflow workers
  • Shared Libraries (shared/{language}/{package}/) - Libraries shared between the engine and rivetkit (e.g., shared/typescript/virtual-websocket/)
  • Service Infrastructure - Distributed services communicate via NATS messaging with service discovery

Important Patterns

Error Handling

  • Custom error system at packages/common/error/
  • Uses derive macros with struct-based error definitions

To use custom errors:

use rivet_error::*;
use serde::{Serialize, Deserialize};

// Simple error without metadata
#[derive(RivetError)]
#[error("auth", "invalid_token", "The provided authentication token is invalid")]
struct AuthInvalidToken;

// Error with metadata
#[derive(RivetError, Serialize, Deserialize)]
#[error(
    "api",
    "rate_limited",
    "Rate limit exceeded",
    "Rate limit exceeded. Limit: {limit}, resets at: {reset_at}"
)]
struct ApiRateLimited {
    limit: u32,
    reset_at: i64,
}

// Use errors in code
let error = AuthInvalidToken.build();
let error_with_meta = ApiRateLimited { limit: 100, reset_at: 1234567890 }.build();

Key points:

  • Use #[derive(RivetError)] on struct definitions
  • Use #[error(group, code, description)] or #[error(group, code, description, formatted_message)] attribute
  • Group errors by module/domain (e.g., "auth", "actor", "namespace")
  • Add Serialize, Deserialize derives for errors with metadata fields
  • Always return anyhow errors from failable functions
    • For example: fn foo() -> Result<i64> { /* ... */ }
  • Do not glob import (::*) from anyhow. Instead, import individual types and traits

Rust Dependency Management

  • When adding a dependency, check for a workspace dependency in Cargo.toml
  • If available, use the workspace dependency (e.g., anyhow.workspace = true)
  • If you need to add a dependency and can't find it in the Cargo.toml of the workspace, add it to the workspace dependencies in Cargo.toml ([workspace.dependencies]) and then add it to the package you need with {dependency}.workspace = true

Inspector HTTP API

  • When updating the WebSocket inspector (rivetkit-typescript/packages/rivetkit/src/inspector/), also update the HTTP inspector endpoints in rivetkit-typescript/packages/rivetkit/src/actor/router.ts. The HTTP API mirrors the WebSocket inspector for agent-based debugging.
  • When adding or modifying inspector endpoints, also update the driver test at rivetkit-typescript/packages/rivetkit/src/driver-test-suite/tests/actor-inspector.ts to cover all inspector HTTP endpoints.
  • When adding or modifying inspector endpoints, also update the documentation in website/src/metadata/skill-base-rivetkit.md and website/src/content/docs/actors/debugging.mdx to keep them in sync.

Database Usage

  • UniversalDB for distributed state storage
  • ClickHouse for analytics and time-series data
  • Connection pooling through packages/common/pools/

Code Style

  • Hard tabs for Rust formatting (see rustfmt.toml)
  • Follow existing patterns in neighboring files
  • Always check existing imports and dependencies before adding new ones
  • Always add imports at the top of the file inside of inline within the function.

Naming Conventions

Data structures often include:

  • id (uuid)
  • name (machine-readable name, must be valid DNS subdomain, convention is using kebab case)
  • description (human-readable, if applicable)

Implementation Details

Data Storage Conventions

  • Use UUID (v4) for generating unique identifiers
  • Store dates as i64 epoch timestamps in milliseconds for precise time tracking

Timestamp Naming Conventions

  • When storing timestamps, name them *_at with past tense verb. For example, created_at, destroyed_at.

Logging Patterns

Structured Logging

  • Use tracing for logging. Do not format parameters into the main message, instead use tracing's structured logging.
    • For example, instead of tracing::info!("foo {x}"), do tracing::info!(?x, "foo")
  • Log messages should be lowercase unless mentioning specific code symbols. For example, tracing::info!("inserted UserRow") instead of tracing::info!("Inserted UserRow")

Configuration Management

Docker Development Configuration

  • Do not make changes to docker/dev* configs. Instead, edit the template in docker/template/ and rerun (cd docker/template && pnpm start). This will regenerate the docker compose config for you.

Development Warnings

  • Do not run ./scripts/cargo/fix.sh. Do not format the code yourself.
  • When adding or changing any version value in the repo, verify scripts/release/update_version.ts updates that location so release bumps cannot leave stale versions behind.

Testing Guidelines

  • When running tests, always pipe the test to a file in /tmp/ then grep it in a second step. You can grep test logs multiple times to search for different log lines.
  • For RivetKit TypeScript tests, run from rivetkit-typescript/packages/rivetkit and use pnpm test <filter> with -t to narrow to specific suites. For example: pnpm test driver-file-system -t ".*Actor KV.*".
  • For frontend testing, use the agent-browser skill to interact with and test web UIs in examples. This allows automated browser-based testing of frontend applications.

Optimizations

  • Never build a new reqwest client from scratch. Use rivet_pools::reqwest::client().await? to access an existing reqwest client instance.

Documentation

  • When talking about "Rivet Actors" make sure to capitalize "Rivet Actor" as a proper noun and lowercase "actor" as a generic noun

Documentation Sync

When making changes to the engine or RivetKit, ensure the corresponding documentation is updated:

  • Limits changes (e.g., max message sizes, timeouts): Update website/src/content/docs/actors/limits.mdx
  • Config changes (e.g., new config options in engine/packages/config/): Update website/src/content/docs/self-hosting/configuration.mdx
  • RivetKit config changes (e.g., rivetkit-typescript/packages/rivetkit/src/registry/config/index.ts or rivetkit-typescript/packages/rivetkit/src/actor/config.ts): Update website/src/content/docs/actors/limits.mdx if they affect limits/timeouts
  • Actor error changes: When adding, removing, or modifying variants in ActorError (engine/packages/types/src/actor/error.rs) or RunnerPoolError, update website/src/content/docs/actors/troubleshooting.mdx to keep the Error Reference in sync. Each error should document the dashboard message (from frontend/src/components/actors/actor-status-label.tsx) and the API JSON shape.
  • Actor status changes: When modifying status derivation logic in frontend/src/components/actors/queries/index.ts or adding new statuses, update website/src/content/docs/actors/statuses.mdx and the corresponding tests in frontend/src/components/actors/queries/index.test.ts.
  • Kubernetes manifest changes: When modifying k8s manifests in self-host/k8s/engine/, update website/src/content/docs/self-hosting/kubernetes.mdx, self-host/k8s/README.md, and scripts/run/k8s/engine.sh if file names or deployment steps change.
  • Landing page changes: When updating the landing page (website/src/pages/index.astro and its section components in website/src/components/marketing/sections/), update README.md to reflect the same headlines, features, benchmarks, and talking points where applicable.
  • Sandbox provider changes: When adding, removing, or modifying sandbox providers in rivetkit-typescript/packages/rivetkit/src/sandbox/providers/, update website/src/content/docs/actors/sandbox.mdx to keep provider documentation, option tables, and custom provider guidance in sync.

Comments

  • Write comments as normal, complete sentences. Avoid fragmented structures with parentheticals and dashes like // Spawn engine (if configured) - regardless of start kind. Instead, write // Spawn the engine if configured. Especially avoid dashes (hyphens are OK).
  • Do not use em dashes (—). Use periods to separate sentences instead.
  • Documenting deltas is not important or useful. A developer who has never worked on the project will not gain extra information if you add a comment stating that something was removed or changed because they don't know what was there before. The only time you would be adding a comment for something NOT being there is if its unintuitive for why its not there in the first place.

Examples

  • When adding new examples, or updating existing ones, ensure that the user also modified the vercel equivalent, if applicable. This ensures parity between local and vercel examples. In order to generate vercel example, run ./scripts/vercel-examples/generate-vercel-examples.ts after making changes to examples.
  • To skip Vercel generation for a specific example, add "skipVercel": true to the template object in the example's package.json.

Common Vercel Example Errors

After regenerating Vercel examples, you may see type check errors like:

error TS2688: Cannot find type definition file for 'vite/client'.

with warnings about node_modules missing. This happens because the regenerated examples need their dependencies reinstalled. Fix by running pnpm install before running type checks.

@claude
Copy link
Copy Markdown

claude bot commented Mar 18, 2026

PR Review

Summary: Two focused fixes -- allow configuring topology datacenters via a hashmap (with backward-compatible list support) and fix the PostgreSQL SSL mode configuration. The DatacentersRepr enum + custom iterator pattern is clean, and the ActorName typed struct is a good improvement. A few issues worth addressing:


Bug: Inverted SSL behavior (breaking change)

File: engine/packages/universaldb/src/driver/postgres/database.rs

The SSL logic has been inverted in a breaking way.

Before: SSL only when ssl_config is explicitly set; otherwise NoTls.
After: SSL always, unless sslmode=disable is in the URL.

Any existing deployment without ssl_config set was using NoTls. After this PR those same deployments attempt SSL (with no certs), which will fail against a self-signed or unverified server cert. The old opt-in behavior should be preserved -- consider keeping the ssl_config-based check and using sslmode=disable only as an additional override:

let pool = if ssl_disabled {
    pool_config.create_pool(Some(Runtime::Tokio1), tokio_postgres::NoTls)...
} else if let Some(config) = &config.ssl_config {
    // build TLS config from explicit cert paths
    ...
} else {
    pool_config.create_pool(Some(Runtime::Tokio1), tokio_postgres::NoTls)...
};

Bug: DSN-format connection strings not handled for SSL check

File: engine/packages/universaldb/src/driver/postgres/database.rs

Url::parse only succeeds for URL-format strings (postgres://host/db?sslmode=disable). If the connection string is in PostgreSQL DSN format (host=localhost dbname=foo sslmode=disable), the parse fails, ssl_disabled defaults to false, and SSL is unexpectedly applied. Either document that DSN format is unsupported, or add a fallback DSN parser for the sslmode key.


Bug: Test helper sets name in Map mode -- will fail validate()

File: engine/packages/test-deps/src/lib.rs

The name field is set in a map entry where validate() expects it to be empty. validate() in config/mod.rs bails if dc.name is non-empty for map entries (name is auto-derived from the key). The name field should be String::new() here.

Same issue exists in Topology::default() which sets name: "default" inside a map entry.


Minor: DatacentersRepr missing is_empty()

File: engine/packages/config/src/config/topology.rs

Clippy will warn about a type that has len() but no is_empty(). Add is_empty() returning self.len() == 0.


Positives

  • Clean DatacentersRepr enum + DatacentersIter pattern for backward-compatible migration from list to map.
  • Good removal of use anyhow::* glob imports in favor of explicit use anyhow::{Context, Result}.
  • Typed ActorName struct instead of raw serde_json::Value is a clear improvement.
  • The .iter().cloned().collect() pattern is consistent across all callers.
  • Schema JSON is kept in sync with the Rust types.

Generated with Claude Code

@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch from f301f2d to f07072d Compare March 19, 2026 00:51
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4456 March 19, 2026 00:51 Destroyed
@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch from f07072d to ea5642d Compare March 21, 2026 01:55
@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from 8ee527a to 5c55237 Compare March 21, 2026 01:55
@MasterPtato MasterPtato mentioned this pull request Mar 21, 2026
11 tasks
@claude
Copy link
Copy Markdown

claude bot commented Mar 21, 2026

This PR makes two main changes:

  1. Allows topology datacenters to be configured via a hashmap (key = datacenter name) in addition to the existing list format (now deprecated).
  2. Fixes PostgreSQL SSL mode configuration to respect sslmode=disable in connection strings.

Bug: Programmatic Datacenter creation in tests sets name on map entries

In engine/packages/test-deps/src/lib.rs, Datacenter is inserted into a HashMap with the name field explicitly set. But the validation in engine/packages/config/src/config/mod.rs rejects map entries where name is non-empty. Both the test-deps setup and Default::default() for Topology create Datacenter values in the map with name already populated, which would fail this check if validate() is called on them. Programmatic map construction should use name: String::new() to let validation populate it, or validation should only run on deserialized user configs, not programmatically constructed configs.

Behavioral Change: SSL Logic Inversion

The SSL logic has been inverted in engine/packages/universaldb/src/driver/postgres/database.rs:

  • Before: No ssl_config = no TLS; with ssl_config = TLS
  • After: sslmode=disable in connection string = no TLS; otherwise always TLS (with optional certs)

This is a silent breaking change for existing deployments that do not have ssl_config configured and also do not have sslmode=disable in their connection strings. They would now unexpectedly attempt TLS. This should be explicitly called out with migration notes.

Non-deterministic HashMap Ordering

DatacentersRepr::Map uses HashMap which has non-deterministic iteration order. Fine for the concurrent health fanout, but callers that need stable ordering should consider IndexMap or BTreeMap.

Missing Runtime Deprecation Warning

The list format is marked Deprecated in doc comments and JSON schema, but no runtime warning is emitted when a config using the list format is loaded. A tracing::warn! in the validation branch for List would help operators discover the deprecation.

Minor: Error Message

The validation error shows dc.name (the value) but not the map key. Including the key would make the error more actionable.

Positive Observations

  • Clean deprecation path for the old list format with backward compatibility.
  • DatacentersIter abstraction avoids forcing callers to match on the enum.
  • ActorName typed struct in fetch.rs is a good improvement over raw serde_json::Value navigation.
  • Removing name from the required JSON schema fields is correct given the new default.

Generated by Claude Code

@claude
Copy link
Copy Markdown

claude bot commented Mar 21, 2026

Code Review: PR #4456 — fix(config): allow configuring topo dcs via map, fix pg ssl mode config

Summary

This PR makes two independent improvements:

  1. Allows topology datacenters to be configured via a HashMap map form in addition to the deprecated Vec list form, deriving the name field from the map key.
  2. Fixes PostgreSQL SSL mode configuration so that sslmode=disable in the connection string correctly bypasses TLS setup instead of always requiring TLS.

Overall the changes are well-scoped and solve a real usability problem with a clean approach. A few issues are worth addressing before merge.


Issues

Bug: Validation order in validate_and_set_defaults — names checked before they are set

File: engine/packages/config/src/config/mod.rs

The valid_hosts validation loop runs before the map-name-injection loop. When the map form is used, dc.name is still empty string at the point of the valid_hosts error message, so users would see datacenter must have valid_hosts configured rather than the helpful name. Move the name-injection block above the valid_hosts validation, or use the map key directly in the error message.


Design concern: HashMap iteration order is non-deterministic

File: engine/packages/config/src/config/topology.rs

HashMap has no defined iteration order. Any code that depends on a consistent ordering of datacenters (e.g., determining which DC is the first for default/leader selection, or producing deterministic health-check output) will behave non-deterministically across process restarts. Consider using IndexMap (from the indexmap crate) or BTreeMap if ordering needs to be stable.


Missing is_empty companion method

File: engine/packages/config/src/config/topology.rs

DatacentersRepr implements len() but not is_empty(). Clippy will flag any x.len() == 0 check. Consider adding an is_empty method that returns self.len() == 0.


SSL mode detection — silent fallback on non-URL connection strings

File: engine/packages/universaldb/src/driver/postgres/database.rs

If Url::parse fails (e.g. DSN format host=... dbname=...), ssl_disabled silently defaults to false, meaning TLS will be attempted. This is safe but could be confusing. A debug-level log when the URL parse fails would help. Also worth adding a comment explaining that only disable bypasses TLS — all other modes (including prefer) will use Rustls.


Unrelated lockfile churn: removal of @visx/* packages

File: pnpm-lock.yaml

The diff removes all @visx/* specifiers from the frontend package's pnpm-lock.yaml importers section. This appears unrelated to the stated purpose of the PR. Please confirm this is intentional and explain in the PR description, or revert.


Minor / Style

  • Default Datacenter name in map default impl: The default map entry sets name: default.into() even though it will be overwritten by the map key during validate_and_set_defaults. Setting it to String::new() would be more consistent.

  • DatacentersIter visibility: It is pub but only an internal implementation detail. Consider pub(crate) or pub(super) to avoid leaking the iterator type.

  • ssl_config repeated access: The three successive config.ssl_config.as_ref().and_then(...) calls could be simplified by binding let ssl = config.ssl_config.as_ref(); once. Minor readability nit.


Checklist

Concern Status
Code quality Generally good; minor style issues noted
Correctness Validation order bug: names checked before set in map form
Performance No concerns
Security SSL disable detection is safe; could benefit from a clarifying comment
Test coverage No new tests for the map config path or SSL mode fix
CLAUDE.md conventions Structured logging used; workspace deps used
Documentation config-schema.json updated appropriately

@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch from ea5642d to f6c3742 Compare March 24, 2026 00:30
@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from 5c55237 to 876713f Compare March 24, 2026 00:30
@MasterPtato MasterPtato mentioned this pull request Mar 24, 2026
11 tasks
@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from 876713f to 3bf4304 Compare March 24, 2026 00:36
@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch from f6c3742 to 10c9ce3 Compare March 24, 2026 00:36
@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from 3bf4304 to 9282590 Compare March 25, 2026 00:05
@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch from 10c9ce3 to 42a11eb Compare March 25, 2026 00:05
@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from 9282590 to bc5319a Compare March 26, 2026 01:18
@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch 2 times, most recently from 38080c6 to 875b002 Compare March 26, 2026 20:50
@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from bc5319a to 72b27cf Compare March 26, 2026 20:50
@claude
Copy link
Copy Markdown

claude bot commented Mar 26, 2026

PR Review: fix(config): allow configuring topo dcs via map, fix pg ssl mode config

This PR makes two main changes: (1) allows topology.datacenters config to be specified as a map keyed by name in addition to the existing list format, and (2) fixes PostgreSQL SSL mode configuration so that sslmode=disable in the connection string correctly disables TLS. A secondary cleanup converts actor_names deserialization to a typed ActorName struct.

Critical Issue

Default Topology will fail validate_and_set_defaults()

In Topology::default() (topology.rs), the Datacenter inside the map entry has name set to default string explicitly. But the new validation in validate_and_set_defaults() bails with an error if dc.name is non-empty for map-format datacenters, since the name is auto-derived from the map key.

Calling validate_and_set_defaults() on a Root::default() config will fail. Fix: change the default Datacenter to use name: String::new().

Other Issues

Validation error shows empty string for map-format datacenter names (mod.rs): The valid_hosts validation block runs before the map name-auto-derivation block. When datacenters are configured as a map, dc.name is still empty string when the error is constructed. The name-assignment block should be moved before the valid_hosts check.

DatacentersRepr has len() without is_empty() (topology.rs): Clippy will warn (len_without_is_empty). An is_empty() method should be added alongside len().

SSL mode detection only handles disable (postgres/database.rs): The fix correctly handles sslmode=disable, but sslmode=allow / sslmode=prefer still enforce TLS via Rustls. A comment explaining why only disable maps to NoTls would help future maintainers.

One caller not updated to the new iterator pattern: engine/packages/pegboard/src/ops/runner/list_runner_config_enabled_dcs.rs still calls .clone() on DatacentersRepr directly rather than using the .iter().cloned().collect() pattern that the three other updated callers now use.

Positive Changes

  • Typed ActorName struct in serverless_metadata/fetch.rs is cleaner than raw serde_json::Value.
  • The map format for datacenters improves config readability with backward-compatible list support.
  • The SSL mode fix properly handles the common local dev case of sslmode=disable.
  • The custom DatacentersIter / IntoIterator impl is a sound approach for the enum-backed union type.
  • Import style in test-deps improved from anyhow glob imports to specific imports, consistent with project style.

Generated with Claude Code (https://claude.ai/claude-code)

@MasterPtato MasterPtato force-pushed the 03-17-fix_api_dont_500_on_cross_dc_response branch from 72b27cf to a9b074b Compare March 28, 2026 00:20
@MasterPtato MasterPtato force-pushed the 03-18-fix_config_allow_configuring_topo_dcs_via_map_fix_pg_ssl_mode_config branch from 875b002 to 14a6bb1 Compare March 28, 2026 00:20
@NathanFlurry NathanFlurry mentioned this pull request Mar 28, 2026
11 tasks
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