Decouple config validation from I/O; check is the sole completeness gate

What changed

bench check accepted inbound endpoints missing the field that makes them addressable: an HTTP endpoint with no route, a gRPC one with no service_name, a NATS one with no subject. Each parses as a well-formed message but describes nothing that could serve a request, which is exactly the mistake check exists to catch.

Fixing that surfaced a deeper problem. Validation was baked into every read and write, ran a full validate(), and rejected any incomplete spec. The issue is that bench set builds a spec one field at a time, and each call is a read-modify-write, so a strict completeness rule made the intermediate file unreadable for the next set. This was exhibited by the need to do bench set assemblies.foo-module.listeners.0 '{"port":8080,"protocol":"PROTOCOL_HTTP"}' in the test.sh since both fields were required on the listener object.

Scope

The MR now separates the two questions that coupling conflated:

  • Can the config be read or written at all? Read only parses; write only serializes. An incomplete spec round-trips freely.
  • Is the spec a complete, runnable service? That's bench check's job, run explicitly (and by scaffold/build), never on config I/O.

Validation severity

Findings carry a severity, and there are two:

Level Fails check Fails check --strict
error yes yes
warning no yes

An error is a missing required field or an impossible value (a port out of range, a duplicate handler identity). A warning is an advisory that doesn't block a build. The only warning today is the shared-handler smell from #46: one handler func bound to several distinct routes. --strict (which CI runs) promotes warnings to failures.

An earlier revision of this MR carried a third incomplete level to let set write unfinished specs. Once validation came off the I/O path that level had no job left to do, so it's gone.

The listener composite, fixed

Before this change, building a listener needed a single composite value because both fields were required at write time:

$ bench set assemblies.foo-module.listeners.0 '{"port":8080,"protocol":"PROTOCOL_HTTP"}'

Now each field writes on its own, like any other object, and check enforces both are present:

$ bench set assemblies.foo-module.listeners.0.port 8080        # writes; protocol still missing
$ bench check
  error: ...listeners[0].protocol: protocol is required (PROTOCOL_HTTP or PROTOCOL_GRPC)   # exit 1
$ bench set assemblies.foo-module.listeners.0.protocol PROTOCOL_HTTP
$ bench check                                                  # exit 0

tests/test.sh splits the listener accordingly and wires the handlers that check now requires on every endpoint.

API notes for reviewers

Click to expand
  • validate() returns ValidationErrors directly (empty means valid) instead of a Result. Err used to mean "blocks I/O"; nothing blocks I/O on completeness anymore.
  • Removed Severity::Incomplete, ValidationErrors::blocks_io, blocking(), and push_incomplete. Added has_errors().
  • read_*/write_* helpers no longer validate; their doc comments say so.
  • set no longer validates. check and scaffold call validate() explicitly; scaffold fails on errors but tolerates warnings.

Closes #42 (closed)

Edited by Lucas Charles

Merge request reports

Loading
Loading