Add type field to ActiveBuild record

[edolstra]

Motivation

In the future, we may want to add other types of active operations (like substitutions or waits for locks). So let's make this an internally tagged enum (in serde terminology) that can be extended later.

Context

Summary by CodeRabbit

• Bug Fixes

  • Enhanced validation to enforce proper data format requirements for incoming data.

• Chores

  • Improved data format consistency for system components.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes add serialization validation to ActiveBuild by introducing a "type" field in JSON. During deserialization, the code verifies the field contains "build" and raises an error if absent or incorrect. During serialization, the field is emitted with value "build". No public API modifications occur.

Changes

Cohort / File(s)	Summary
JSON Serialization Validation src/libstore/active-builds.cc	Added "type" field validation in ActiveBuild::from_json() to enforce discriminant check; throws error if missing or not "build". Added "type" field emission in ActiveBuild::to_json() with constant value "build".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Straightforward validation logic with a single new field check in deserialization
• Simple constant field addition in serialization path
• Verify error handling behavior and error message clarity on type mismatch

Poem

🐰 A type field hops into the stream,
Building blocks now know their dream.
JSON whispers "build" so true,
Validation guards both old and new!
✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add type field to ActiveBuild record' directly and clearly summarizes the main change: adding a type field to the ActiveBuild record, which matches the core modifications to input/output validation in the implementation.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch nix-ps-user

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/libstore/active-builds.cc (1)

120-120: LGTM – Type field pairs with validation.

The addition of the "type" field to the JSON output correctly pairs with the deserialization validation at lines 102-104. This is forward-compatible as older readers will simply ignore the additional field.

Optional: Consider extracting the "build" string literal (used here and at line 103) into a named constant to ensure consistency and ease future maintenance.

// At file or namespace scope
static constexpr const char* ACTIVE_BUILD_TYPE = "build";

Then use ACTIVE_BUILD_TYPE at lines 103 and 120.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7bcc712 and 8f739c9.

📒 Files selected for processing (1)

• src/libstore/active-builds.cc (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build_aarch64-darwin / build
• GitHub Check: build_x86_64-linux / build

🔇 Additional comments (1)

src/libstore/active-builds.cc (1)

102-104: No action needed—this is not a breaking change.

The "type" field addition does not create a backward compatibility issue. ActiveBuild JSON is used exclusively in the worker protocol's QueryActiveBuilds feature, which is behind a feature flag (featureQueryActiveBuilds). Old clients and daemons that lack this feature will not attempt to deserialize this JSON; they fail earlier with "remote store does not support querying active builds". Since both client and daemon are updated together in the same release, there is no persistent storage or version mismatch concern. The j.at("type") call is appropriate for a required field in a versioned protocol.

Likely an incorrect or invalid review comment.

[github-actions]

🚀 Deployed on https://6936e69fdda1071b907b8fbb--determinate-nix.netlify.app

[cole-h]

LGTM.

[cole-h]

This could also be used to version (e.g. build-v2 type with different fields) (paraphrased from a convo with Eelco)