Add CustomBuild config schema and builds config download

[shiv-tyagi]

Should be merged after #248 and #250

Adds a shared config schema and build_config helpers, packs custombuild.yaml into each build archive, and exposes GET /api/v1/builds/{id}/config so a build can be downloaded as rebuildable YAML.

Made with Cursor

[peterbarker]

We probably don't care about the redis thing. But the version thing is worth looking at.

  Review: PR #249 — Add CustomBuild config schema and builds config download

  Author: shiv-tyagi · feat/build-config-endpoint → main · +606/−9 across 13
  files

  Overview

  Adds a shared build_config/ package that serializes build metadata to a
  versioned, JSON-Schema-validated custombuild.yaml. The builder packs that file
  into each archive; the web app exposes it at GET /api/v1/builds/{id}/config.
  To support this, BuildInfo gains four display-name fields captured at submit
  time, and BuildVersionInfo gains name/type.

  The direction is good — a versioned schema with a shared producer used by both
  processes is the right shape for a future "upload config to rebuild" flow.
  Flake8 is clean and all 253 tests pass on the branch. But there are two
  defects that will fire on the first deploy.

  Blocking

  1. Unpickling pre-existing BuildInfo from Redis raises AttributeError

  BuildInfo is dill-serialized into Redis with a 24h TTL, and compose mounts a
  persistent volume (./redis_data:/data). Dill restores __dict__ directly
  without calling __init__, so every build submitted before the deploy comes
  back missing the four new attributes for up to 24 hours after rollout.

  Worse, the diff removes the guard that was added for exactly this reason in
  commit 75a412a:

  -            'time_started': getattr(self, 'time_started', None),
  +            'time_started': self.time_started,

  Confirmed by round-tripping an old-shape object through dill:

  FAIL  to_dict() [time_started]:     AttributeError: 'BuildInfo' object has no
  attribute 'vehicle_name'
  FAIL  builder archive config:       AttributeError: 'BuildInfo' object has no
  attribute 'vehicle_name'
  FAIL  web _build_info_to_output:    AttributeError: 'BuildInfo' object has no
  attribute 'board_name'

  That's a 500 on GET /builds and GET /builds/{id} for every in-flight build,
  plus a builder-side failure (see #3). Suggest defaulting the new params to
  None and reading them via getattr(build_info, 'vehicle_name', None) — or
  adding a __setstate__ that backfills defaults — and restoring the getattr for
  time_started.

  2. Literal on BuildVersionInfo.type rejects real-world release_type values

  web/schemas/builds.py:50 constrains type to Literal["beta", "stable", 
  "latest", "tag"], but release_type is free-form text read straight from
  remotes.json (providers.py:271: release.get("release_type")). The repo's own
  examples/remotes.json.sample ships "release_type": "Custom":

  OK    release_type='stable' / 'latest' / 'tag' / 'beta'
  FAIL  release_type='Custom': ValidationError

  Any build against a custom remote 500s on read. Note
  _release_type_dedup_priority already handles unknown types with a case _: 
  return 99 fallback — the codebase treats this as open-ended. Use str, or an
  enum with an explicit fallback.

  Important

  3. A config-write failure kills the builder process

  write_config_yaml validates against the schema and raises on any mismatch
  (including a None name, or the AttributeError from #1). It's called in
  __generate_archive, and there is no try/except anywhere in the chain —
  __generate_archive → __process_build → run() → __main__.py:75. An exception
  terminates the builder daemon, and does so after a completed 15-minute build
  whose artifacts are then never archived.

  The config file is a nice-to-have relative to the firmware; wrap it so it
  degrades instead:

  try:
      write_config_yaml(config_path, config_dict_from_build_info(build_info))
      files_to_include.append(str(config_path.resolve()))
  except Exception:
      self.logger.exception(f"Could not write {CONFIG_FILENAME} for {build_id}")

  4. selected_features ordering is nondeterministic across processes

  build_config_dict does list(selected_features) on a set. With hash
  randomization, the builder process and the web process emit different
  orderings for the same build:

  seed=0 -> ['GPS_UBLOX', 'AHRS_EKF3', 'BATTERY_SMBUS', 'OSD_PARAM',
  'MOUNT_GIMBAL']
  seed=1 -> ['MOUNT_GIMBAL', 'OSD_PARAM', 'BATTERY_SMBUS', 'GPS_UBLOX',
  'AHRS_EKF3']

  So the YAML in the archive won't match the YAML from the endpoint, and configs
  won't diff cleanly. For a file whose purpose is reproducible rebuilds,
  sorted(selected_features) is worth the one-word change.

  Minor

  - web/services/builds.py:274 — the from build_config import ... is
  function-local while every other import in the file is top-level. If that's
  dodging a circular import, a comment would help; otherwise hoist it.
  - _build_info_to_output now calls get_version_info per build, and list_builds
  calls it for every build — get_version_info is a linear scan over a vehicle's
  versions, so the list endpoint becomes O(builds × versions). More to the
  point: now that name/type are stored on BuildInfo, the live
  lookup-with-fallback is doing work the stored fields already cover. Reading
  the stored values directly would be simpler and drop the N+1.
  So the YAML in the archive won't match the YAML from the endpoint, and configs won't diff cleanly. For a file whose purpose is reproducible
  rebuilds, sorted(selected_features) is worth the one-word change.

  Minor

  - web/services/builds.py:274 — the from build_config import ... is function-local while every other import in the file is top-level. If that's      self.logger.exception(f"Could not write {CONFIG_FILENAME} for {build_id}")

  4. selected_features ordering is nondeterministic across processes

  build_config_dict does list(selected_features) on a set. With hash randomization, the builder process and the web process emit different orderings for the same build:

  seed=0 -> ['GPS_UBLOX', 'AHRS_EKF3', 'BATTERY_SMBUS', 'OSD_PARAM', 'MOUNT_GIMBAL']
  seed=1 -> ['MOUNT_GIMBAL', 'OSD_PARAM', 'BATTERY_SMBUS', 'GPS_UBLOX', 'AHRS_EKF3']

  So the YAML in the archive won't match the YAML from the endpoint, and configs won't diff cleanly. For a file whose purpose is reproducible rebuilds, sorted(selected_features) is worth the
  one-word change.

  Minor

  - web/services/builds.py:274 — the from build_config import ... is function-local while every other import in the file is top-level. If that's dodging a circular import, a comment would
  help; otherwise hoist it.
  - _build_info_to_output now calls get_version_info per build, and list_builds calls it for every build — get_version_info is a linear scan over a vehicle's versions, so the list endpoint
  becomes O(builds × versions). More to the point: now that name/type are stored on BuildInfo, the live lookup-with-fallback is doing work the stored fields already cover. Reading the stored
  values directly would be simpler and drop the N+1.
  - schemas/config/0.0.1.json — use_default_features is defined but no producer ever emits it. Either wire it up or drop it until it's used.
  - schema_path_for_version interpolates config_version into a filesystem path. Not reachable today (nothing ingests untrusted config), but this module is clearly aimed at an
  upload-to-rebuild flow — worth gating on a known-versions whitelist before that lands.
  - tests/build_config/test_build_config.py:38 — pytest.raises(Exception) will pass on a typo or an import error just as happily as on a real validation failure. Use
  jsonschema.ValidationError.

  Test coverage

  Good coverage of the new helpers, service method, and endpoint, including the fallback paths. One real gap: test_packaged_yaml_inside_tar re-implements the tar logic rather than exercising
  Builder.__generate_archive, so the actual integration point — the thing changed in builder/builder.py — is untested. Nothing covers deserializing an old-shape BuildInfo, which is why #1
  slipped through; a test that pickles an object without the new attrs and asserts to_dict() survives would lock that down.

  Summary

  Blocking on #1 and #2 — both are confirmed runtime failures on deploy, and #1 cascades into #3. #4 undercuts the reproducibility goal for a one-word fix. Everything else is polish. The
  build_config package itself is well-factored and the schema versioning is the right call.