feat(core): add schema versioning to pack.toml, manifests, and lockfile#237
Conversation
There was a problem hiding this comment.
Pull request overview
Adds forward-compatible schema versioning across weave’s pack manifest (pack.toml), adapter sidecar manifests, and lockfile so older weave versions can gracefully reject newer formats with a clear “please upgrade” error.
Changes:
- Introduces
schema_version: u32with serde defaults (back-compat) and “too new” guards (forward-compat) for pack/lockfile/sidecar manifests. - Updates
weave initandpack.schema.tomldocumentation to emit/documentschema_version = 1. - Adds unit tests covering pack parsing and adapter manifest rejection; extends lockfile tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/error.rs |
Adds SchemaVersionTooNew error variant for forward-compat rejection. |
src/core/pack.rs |
Adds pack schema version constant + guard when parsing pack.toml; adds pack schema tests. |
src/core/lockfile.rs |
Adds lockfile schema version field + guard in LockFile::load; updates/extends tests. |
src/cli/sync.rs |
Updates test construction of LockFile to include schema_version. |
src/cli/init.rs |
Makes weave init generate schema_version = 1 in new pack.toml. |
src/adapters/mod.rs |
Centralizes manifest schema constant + serde default helper for adapters. |
src/adapters/gemini_cli.rs |
Adds schema version to Gemini sidecar manifest + load-time “too new” guard + test. |
src/adapters/codex_cli.rs |
Adds schema version to Codex sidecar manifest + load-time “too new” guard + test. |
src/adapters/claude_code.rs |
Adds schema version to Claude sidecar manifest + load-time “too new” guard + test. |
pack.schema.toml |
Documents schema_version in the pack manifest reference. |
| format!( | ||
| r#"[pack] | ||
| r#"schema_version = 1 | ||
|
|
||
| [pack] | ||
| name = "{name}" |
There was a problem hiding this comment.
[low-priority] weave init hard-codes schema_version = 1 in the generated pack.toml. To avoid drift when the supported pack schema version changes, use the shared constant (e.g., CURRENT_PACK_SCHEMA_VERSION) when generating this content.
| pub const CURRENT_PACK_SCHEMA_VERSION: u32 = 1; | ||
|
|
||
| fn default_schema_version() -> u32 { | ||
| 1 |
There was a problem hiding this comment.
[low-priority] default_schema_version() returns a literal 1 even though CURRENT_PACK_SCHEMA_VERSION is the source of truth. Returning the constant (or referencing it inside the default fn) keeps the serde default and the supported version in sync when the schema version bumps.
| 1 | |
| CURRENT_PACK_SCHEMA_VERSION |
| pub const CURRENT_LOCKFILE_SCHEMA_VERSION: u32 = 1; | ||
|
|
||
| fn default_schema_version() -> u32 { | ||
| 1 |
There was a problem hiding this comment.
[low-priority] default_schema_version() returns a literal 1 while CURRENT_LOCKFILE_SCHEMA_VERSION is the source of truth. Consider returning the constant to prevent future mismatches when the schema version is bumped.
| 1 | |
| CURRENT_LOCKFILE_SCHEMA_VERSION |
|
|
||
| /// Default schema version for serde deserialization of adapter manifests. | ||
| pub(crate) fn default_manifest_schema_version() -> u32 { | ||
| 1 |
There was a problem hiding this comment.
[low-priority] default_manifest_schema_version() returns a literal 1. Returning CURRENT_MANIFEST_SCHEMA_VERSION would make it harder to forget updating the serde default when the manifest schema version changes.
| 1 | |
| CURRENT_MANIFEST_SCHEMA_VERSION |
| // The version check happens in LockFile::load() which reads from disk, | ||
| // so we verify the field deserializes correctly and test the guard directly. | ||
| assert!(parsed.schema_version > CURRENT_LOCKFILE_SCHEMA_VERSION); | ||
| } |
There was a problem hiding this comment.
[robustness] reject_lockfile_with_future_schema_version currently only deserializes TOML and asserts schema_version > CURRENT_LOCKFILE_SCHEMA_VERSION, but it doesn't exercise the actual guard in LockFile::load(). Since load() reads from disk and returns SchemaVersionTooNew, this test can pass even if the runtime check regresses—suggest writing the TOML into a temp WEAVE_TEST_STORE_DIR and asserting LockFile::load() returns the expected error variant/message.
| # Generate a real pack.toml with: weave init | ||
|
|
||
| # Schema version of this file format. Defaults to 1 if omitted. | ||
| # Older weave versions will reject manifests with a version they don't support. |
There was a problem hiding this comment.
[low-priority] This comment says older weave versions will reject "manifests", but this file documents pack.toml. Consider rewording to "pack.toml files" (or "pack manifests") to avoid confusion with the adapter sidecar manifests.
| # Older weave versions will reject manifests with a version they don't support. | |
| # Older weave versions will reject pack.toml files with a schema_version they don't support. |
Extend the schema versioning from PR #237 to cover registry responses: - Add schema_version field to PackMetadata (serde default = 1) - Parse index.json as versioned envelope or legacy flat format - Validate schema_version on both index and pack metadata responses - Reject registries with schema versions newer than this build supports - Add tests for envelope parsing, flat fallback, and deserialization
Add forward-compatible schema versioning to all generated registry files:
- index.json now uses envelope format: {"schema_version": 1, "packs": {…}}
- Each packs/{name}.json includes a top-level schema_version field
- generate.py emits REGISTRY_SCHEMA_VERSION (currently 1) in both formats
This enables older weave clients to detect and reject registry formats
they cannot safely parse, displaying a clear "please upgrade" message.
Companion to breferrari/weave#237.
|
Registry-side companion PR: PackWeave/registry#3 — adds |
- Replace "sidecar manifest" jargon with CLI-specific labels
("Claude Code tracking file", "Gemini CLI tracking file", etc.)
- Include current weave version and install hint in SchemaVersionTooNew
error message
- Extract check_manifest_schema_version() helper, eliminating 6
duplicate load-then-check blocks across adapters
- Fix u64-to-u32 truncation in parse_search_index (use try_from)
- Add doc comments to all default_schema_version() functions explaining
they intentionally return 1, not the current constant
- Add doc comments to schema_version fields on LockFile and PackManifest
- Improve doc comments on parse_search_index and SearchIndexEnvelope
- Add schema versioning section to ARCHITECTURE.md
- Use CURRENT_PACK_SCHEMA_VERSION constant in weave init template
- Add load_rejects_future_schema_version test for LockFile::load()
- Make weave diagnose degrade gracefully on future schema versions
instead of aborting
- Mark schema versioning done in ROADMAP.md
- Reject schema_version == 0 at all check sites (defense-in-depth: version 0 is invalid and should not bypass future version-gated validation) - Add unit tests for registry schema version rejection: - reject_pack_metadata_with_future_schema_version - reject_pack_metadata_with_schema_version_zero - reject_search_index_envelope_with_future_schema_version - u64_overflow_schema_version_treated_as_too_new - Fix pack.schema.toml comment wording (Copilot review)
Summary
schema_version(u32) field topack.toml, all three adapter sidecar manifests (Claude Code, Codex CLI, Gemini CLI), and the lockfile#[serde(default)], preserving backward compatibility with all existing filesSchemaVersionTooNewerror directing users to upgrade weaveweave initnow emitsschema_version = 1at the top of generatedpack.tomlpack.schema.tomlupdated to document the new fieldTest plan
parse_pack_with_explicit_schema_version_1— explicit version 1 worksreject_pack_with_future_schema_version— version 99 rejected with clear errorparse_pack_without_schema_version_defaults_to_1— existing packs still parsereject_manifest_with_future_schema_version— one test per adapter (Claude Code, Codex CLI, Gemini CLI)reject_lockfile_with_future_schema_version— lockfile version checkCloses #224