fix(cache): implement schema-typed protobuf CacheService (main)

Merged
BN
2mo ago
+387-194

Summary

Implements prost-based CacheService v2 for sccache protobuf while preserving actions/cache@v4 JSON. Fixes cubic review on #147 (P1 blocker + P2 concern).

Changes

  • Replace hand-rolled pb_varint/pb_strings with prost::Message using official field numbers:
    • Create: metadata=1 key=2 version=3
    • Finalize: metadata=1 key=2 size_bytes=3 version=4
    • Get: metadata=1 key=2 restore_keys=3 version=4
    • CacheMetadata: repository_id=1 scope=2 / CacheScope: scope=1 permission=2
  • metadata_fields → Vec<String> scopes (preserve wire order, filter empty) + Option<String> repository; CachePbFields = (String,String,Vec<String>,Vec<String>,Option<String>)
  • GetDownloadURL iterates each scope's scoped_cache_key(scope, repo) until hit — authorized via 2nd scope no longer missed (was find(first)). Create/Finalize keep scopes.first() as write namespace.
  • pb_or_json<M:Message> now encodes via M::encode_to_vec() — ignores unknown fields (forward-compat, 0x28 0x01 test), rejects malformed.
  • Correct wire fixtures fixtures/wire/cache-{create(49B),get(62B)+unknown, multi-scope(55B), finalize(17B)}.pb generated via prost — not self-roundtrip.
  • Cargo.toml + Cargo.lock add prost 0.14.4 (+ prost-derive/either/itertools)

Fixes review comments (#147)

  • P1 blocker: added golden include_bytes!("../../../fixtures/wire/...") — pb_golden_sccache_create/get + pb_multi_scope_preserves_all_scopes_in_wire_order — hand-rolled key=1 would fail.
  • P2 concern: multi-scope preservation + Get loop.

Verification

  • cargo test -p preloop-runner-server cache_pb_tests -- --nocapture — 7 passed (pb_golden_sccache_*, pb_multi_scope, pb_request_schemas_use_their_distinct_version_fields, pb_request_rejects_missing_identity, pb_roundtrip..., pb_or_json...)
  • cargo test -p preloop-runner-server --lib on fix/cache-protobuf-fidelity:011649bc — 507 passed 3 ignored (main branch pending just test-ci after you approve)

Summary by cubic

Implements a schema-typed protobuf CacheService using prost while preserving the actions/cache@v4 JSON API. This restores compatibility with sccache’s official field numbers and fixes multi-scope cache restores.

  • Protobuf decoding now uses the official schemas (metadata=1, key=2, restore_keys=3, version=4), ignores unknown fields, and rejects malformed wire data. Previously a hand-rolled decoder could mis-parse sccache messages.
  • GetDownloadURL tries scopes in wire order until a hit; previously only the first scope was considered, which could miss valid, authorized hits. Create/Finalize continue to write under the first scope.
  • Requests must include non-empty key and version; JSON behavior is unchanged.

Review notes

  • Adds prost and replaces manual varint/string code. pb_or_json<M> encodes via prost::Message and sets the protobuf content type.
  • Internal request tuple is now (key, version, restore_keys, scopes, repository) to preserve all scopes and wire order; repository id is parsed from metadata when present.
  • Adds golden protobuf fixtures and tests for sccache create/get, multi-scope order, distinct version fields, and unknown-field forward compatibility.

[!NOTE]

Replace hand-rolled protobuf helpers with Prost Message types in CacheService

  • Replaces manual protobuf encoding helpers (pb_varint, pb_string_field, etc.) with Prost-annotated message structs (PbCacheMetadata, PbCreateCacheEntryRequest, etc.) in results_twirp.rs, adding prost = "0.14.3" as a dependency
  • Introduces CacheRequestKind enum to dispatch protobuf decoding to the correct message type, and validate_cache_identity to reject empty key/version with 400
  • Changes CachePbFields from a single optional scope to Vec<String>, preserving all scopes in wire order; twirp_cache_v2_get_dl_url now tries each scope until a hit
  • Extracts repository_id from PbCacheMetadata to enable repository scoping for protobuf clients
  • Risk: CachePbFields type alias changed from (String, String, Vec<String>, Option<String>, Option<String>) to (String, String, Vec<String>, Vec<String>, Option<String>); any in-tree callers expecting the old tuple shape must be updated. Unknown protobuf fields are now silently ignored per spec rather than causing errors.
📊 Macroscope summarized 0229491. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

Summary by CodeRabbit

  • New Features

    • Improved cache API compatibility with standard protobuf request and response formats.
    • Added support for cache lookups across multiple scopes, including restore keys and unscoped fallback behavior.
    • Cache operations now support multiple repository references and scope combinations.

  • Bug Fixes

    • Improved validation for missing cache keys, versions, and repository identity information.
    • Unknown protobuf fields are now safely ignored for greater compatibility.

  • Tests

    • Added coverage for protobuf decoding, multi-scope lookups, validation, and cache response generation.