Skip to content

Add the core-component registry table (#826) - #856

Merged
AcoPiper merged 3 commits into
mainfrom
AcoPiper/issue-826
Aug 18, 2026
Merged

Add the core-component registry table (#826)#856
AcoPiper merged 3 commits into
mainfrom
AcoPiper/issue-826

Conversation

@AcoPiper

@AcoPiper AcoPiper commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Adds src/tables/core_component.rs: the registry of the platform's own host-fixed infrastructure — review, aice-web-next, roxyd and bootroot — which is neither an agent nor an external service. One row per (component, host) records the installed build and its install/run state.

  • Record type CoreComponent with the six fields the issue names, component as a String (the reason is recorded in a doc comment on the type, so it is not re-argued in review). Re-exported from src/tables.rs and src/lib.rs, and sealed in src/types.rs.
  • Persisted value keeps the lifecycle as a raw u8 variant index in the private Value struct, calling the dependency's Lifecycle::to_stored_index / Lifecycle::from_stored_index rather than adding a second mapping. An unrecognized stored number reads back as Lifecycle::Unknown instead of failing the whole row.
  • Collision-safe key: a single Key::to_bytes / Key::from_bytes pair encodes the pair as a length-prefixed serialized tuple through the crate's existing serialize / deserialize helpers, so ("ab", "c") and ("a", "bc") land on distinct keys. Every read and write path — get, delete, unique_key — goes through it, and Key stays private to the module.
  • Table block with open, get, delete, update, and iteration via the blanket Iterable impl. Inserts go through Table::insert, so a second row for an occupied pair is refused by Map::insert rather than overwriting; update onto an occupied pair is likewise refused.
  • Column family is deliberately not registered: CORE_COMPONENTS is defined next to the other name constants but left out of MAP_NAMES, with a comment stating that registration and the Store accessor land with the format bump. open therefore carries an #[allow(dead_code)] explaining the same.
  • Tests live in #[cfg(test)] mod tests inside the module and open their own database over a tempfile::tempdir() with a raw OptimisticTransactionDB::open_cf, holding a crate::test::acquire_db_permit() guard for as long as the database is open.

Closes #826

Test plan

  • Value round-trip covers every field, with installed_version / installed_commit both None and Some, and both installer_managed values
  • ("ab", "c") and ("a", "bc") produce distinct keys, are stored simultaneously, and each get returns its own row
  • Key encode-then-decode returns the original pair for ("roxyd", "host-01.example.com") and for the adversarial pairs
  • Three roxyd rows on three hosts coexist, each is independently gettable, and deleting one leaves the other two intact
  • A bootroot row stores and reads back installer_managed = true; a review row reads back false
  • Inserting a second row for an existing (component, host) returns an error and leaves the stored row unchanged
  • lifecycle round-trips through every variant
  • Overwriting a stored lifecycle byte with 9 still reads back: the row yields Lifecycle::Unknown with component, host, installed_version, installed_commit and installer_managed intact
  • CRUD and iteration work end to end: insert, get, update, delete, and iterate returns every row
  • CHANGELOG.md gains an entry under ## [Unreleased]
  • cargo fmt -- --check --config group_imports=StdExternalCrate passes
  • cargo clippy --bins --tests --all-features -- -D warnings passes
  • cargo test --all-features is green
  • MAP_NAMES, COMPATIBLE_VERSION_REQ, migrations, Agent and ExternalService are untouched

REView, aice-web-next, roxyd and bootroot are host-fixed
infrastructure: they are neither agents nor external services, so
neither of the existing per-node tables fits them. Register them in
their own table, keyed by (component, host), recording the build
installed there and the install or run state it is in.

Both halves of the key are variable-length strings, so the key is a
length-prefixed serialized tuple rather than a byte concatenation,
which would map ("ab", "c") and ("a", "bc") onto the same bytes. One
encoder and its exact inverse serve every read and write path.

The persisted value holds the lifecycle as a raw variant index, as the
agent and external-service tables do, so a number written by a newer
build reads back as Unknown instead of failing the whole row.

The column family is deliberately left out of MAP_NAMES: registering
it would silently add a column family to a data dir already at a
compatible version, which migrate_data_dir returns early for.
Registration and the Store accessor land with the format bump.

Closes #826
Insert is not the only path that could put a second row on a pair that
already has one: update also writes, and renaming a row moves it onto a
pair the caller may not have read. Pin that it is refused and that both
rows survive, so the single-row invariant cannot be lost to a later
change in the update path.

Part of #826
Iteration was the one read path never exercised over more than one row:
multi-row cases only counted the iterator, and only a single-row case
decoded it. That left the path that hands `from_key_value` raw keys —
rather than keys `get` had just encoded — untested for the case where
the decoder has to tell several rows apart.

Part of #826
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.83%. Comparing base (fe40f5c) to head (cc234cd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #856      +/-   ##
==========================================
+ Coverage   83.67%   83.83%   +0.16%     
==========================================
  Files          91       92       +1     
  Lines       35064    35419     +355     
==========================================
+ Hits        29340    29695     +355     
  Misses       5724     5724              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AcoPiper

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 1]

No findings. The implementation matches the issue’s deliberately deferred-registration design: CORE_COMPONENTS is defined but excluded from MAP_NAMES, and the test-only raw database setup covers the otherwise-unreachable table opening path. The serialized (component, host) tuple is used consistently for record keys, lookups, deletes, and decoding, with collision and round-trip tests. The persisted lifecycle is correctly stored as the dependency-provided raw index and maps unrecognized values to Lifecycle::Unknown without losing the other fields.

The PR body correctly closes #826 and includes a complete test plan; the review thread contains no implementation-created issue.

@AcoPiper

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 1: APPROVED]

@AcoPiper

Copy link
Copy Markdown
Contributor Author

Suggested squash commit

Title

Add the core-component registry table

Body

The platform's own host-fixed infrastructure — review, aice-web-next,
roxyd and bootroot — is neither an agent nor an external service, so
neither existing table can hold it. This adds its registry: one row
per (component, host) recording which build is installed there and
what install or run state it is in.

component is a String rather than an enum because the package-id
registry is owned by the packaging layer, not by this crate. An enum
would fork that registry into a persisted schema, and adding a fifth
core component would then mean a data migration and a format bump for
what is really just a new row.

Both halves of the key are variable-length, so their bytes are never
concatenated: a single encoder writes them as a length-prefixed tuple
through the crate's existing serialize helpers, and its exact inverse
lives beside it. Every read and write path goes through that pair, so
get, delete and unique_key cannot drift apart.

The persisted value holds the lifecycle as a raw variant index rather
than as a Lifecycle. A stored number this build does not recognize
then resolves to Unknown instead of failing the whole row, and the
bytes are identical either way. The conversions are the ones that ship
next to the enum, so there is no second mapping to keep in step.

Core components are single-instance, so v1 refuses a second row for an
occupied pair rather than allocating another number: inserts go
through Table::insert, and update likewise refuses a move onto a pair
that already has a row.

The column family is deliberately left out of MAP_NAMES. StateDb::open
creates every name listed there, while migrate_data_dir returns early
for a data dir already at a compatible version, so registering it now
would add a column family to an existing data dir with no version
change. Registration and the Store accessor land with the format bump.

Closes #826

@AcoPiper
AcoPiper merged commit 8091212 into main Aug 18, 2026
10 checks passed
@AcoPiper
AcoPiper deleted the AcoPiper/issue-826 branch August 18, 2026 07:19
@AcoPiper AcoPiper mentioned this pull request Aug 18, 2026
17 tasks
@sehkone sehkone mentioned this pull request Aug 22, 2026
6 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.

Add the core-component registry table

1 participant