Mjp/include more in grant hash

[mj-palanker]

grantContentHash64 previously hashed only the grant's identity tuple
plus the set of source-entitlement ids, deliberately excluding all
annotations and source-map values as sync-transient noise. But
GrantImmutable and per-source is_direct are stable per-grant facts
(not bookkeeping that churns every sync) that the SDK already uses
elsewhere to distinguish otherwise-identical grants (rollback_expansion's
suspect-grant check, topological_merge's direct-wins-over-indirect
upgrade) — so two grants differing only in immutability or provenance
were silently digesting as identical. Bumps GrantDigestABIVersion to 2
since this changes what "the same grant" hashes to.

Also adds real support and detection for the version upgrade mechanism, forcing us to re-build all indexes if the SDK version is newer than the hashes written to the file, avoiding a silent, incorrect multi-version rollup digest

[github-actions]

🟡 Suggestion: a current stamp certifies the whole node keyspace, but only wholesale destroyers erase the stamp — the partial invalidation paths (InvalidateGrantDigestPartitions at grant_digest_repair.go:105, stageGrantDigestInvalidation at rawdb/records.go:239) delete partition nodes plus the global root and deliberately leave the stamp. So a pre-stamp SDK that partially invalidates + repairs a file this SDK stamped v2 rebuilds only the touched partitions at v1 framing, recomputes the global root, and leaves the v2 stamp intact — this check then trusts a mixed v1/v2 rollup, which is the silent multi-version digest the PR body says it avoids. Consider tying the certificate to something a partial invalidation already destroys (e.g. carrying the ABI version in the global-root value, which every invalidation path deletes), or gating pre-stamp writers out via the engine schema version.

[github-actions]

🟡 Suggestion: every immutable case in this file makes GrantImmutable the only annotation, so the multi-entry branch of scanGrantContentFactsRawBytes's case 8 loop is untested. Add a case where the annotation list carries another annotation (e.g. GrantMetadata) before/after GrantImmutable, plus one where a GrantExpandable was stripped by V2GrantToV3 — a raw-scan bug that stopped at the first Any would silently regress isImmutable to false for real connector grants, and the seal path has no oracle that would notice.

[github-actions]

General PR Review: Mjp/include more in grant hash

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 4bd2b38695ad.
Review mode: full
View review run

Review Summary

Scanned the full PR diff for security and correctness. The change folds GrantImmutable and per-source is_direct into grantContentHash64, bumps GrantDigestABIVersion to 2, and adds a durable ABI stamp under a new DigestMetaIndexID (0xFF) sub-range so a writable Open drops non-current digest state and a read-only Open reports the roots as "never built". I verified the framing stays injective (AppendTupleBool 0x26/0x27 are disjoint from the 0x00 separator / 0x01 escape, and the immutability slot is fixed after the six identity segments), that the raw protobuf field numbers match records.proto (annotations=8, sources=9, GrantSourceRecord.is_direct=4), that the raw-scan and from-record paths agree including empty source keys, that the stamp key is excluded from DigestNodeKeyspaceBounds yet included in every full-range digest delete (dropAllGrantDigestStateLocked, the fold's opening DeleteRange, ResetForNewSync, the compactor's digest bucket plan), that all three global-root write sites now stamp first, that isGrantDigestRootKey filters the stamp out of recomputeGrantDigestGlobalRootLocked's fold, and that the read-only stale flag covers every consumer (dirtyPartitionBuckets and ComputeEntitlementBucketDigest both funnel through the guarded root getters, and no production code reads hash-index rows directly). No blocking issues found.

Risk triage (per docs/BUG_CATCHING.md §2): silence — yes (a wrong content hash produces a well-formed wrong digest, never a panic); durability — yes (digest nodes, hash-index values, and the manifest abi_version outlive the process); uncontrolled dimensions — yes (correctness depends on which SDK version wrote the state being read); consumer distance — the c1 platform and future SDK versions. Verdict: HIGH, remediation rung 3–4 (re-derive digest state fleet-wide, or coordinate across SDK versions). The PR does bring real instruments — a positive-evidence oracle validated against a tampered row (TestGrantDigestABIOracle), stamp-present/stale/missing/orphan/read-only/pending-marker permutations, and reseal-then-recheck assertions. The one instrument it lacks is a two-artifact cross-version harness: nothing in the diff exercises a file whose digest state was written by more than one ABI, which is exactly where the remaining gap is (suggestion 1). Recommend the full pass-set review per §6 before merge rather than treating this single-shot CI pass as coverage.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• pkg/dotc1z/engine/pebble/grant_digest.go:114 — the stamp is only erased by wholesale digest deletes; partial invalidation (InvalidateGrantDigestPartitions, stageGrantDigestInvalidation) leaves it, so a pre-stamp SDK that partially repairs a v2-stamped file leaves v1-hashed partitions under a v2 stamp that this check trusts.
• pkg/sdk/version.go:3 — still v0.24.1; per the repo criteria a durable-format change plus a behavior change to the exported GrantContentHash / GrantDigestABIVersion needs the 0.x minor bump as the compatibility signal, and the PR body should carry the rollout note for mixed-version fleets.
• pkg/dotc1z/engine/pebble/grant_digest_hash_test.go:233 — no case exercises GrantImmutable alongside other annotations, so the multi-entry branch of the raw annotation scan is untested.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/dotc1z/engine/pebble/grant_digest.go`:
- Around line 106-127 (verifyGrantDigestABI): the ABI stamp is only erased by
  wholesale digest destroyers (dropAllGrantDigestStateLocked, the fold's opening
  DeleteRange over DigestLowerBound..DigestUpperBound, ResetForNewSync). The
  PARTIAL invalidation paths — Engine.InvalidateGrantDigestPartitions in
  grant_digest_repair.go and RecordBatch.stageGrantDigestInvalidation in
  internal/rawdb/records.go — delete only the touched partitions' nodes, their
  hash-index ranges, and the global root; they leave the stamp in place. That is
  correct within one SDK version, but it means an already-shipped pre-stamp SDK
  can partially invalidate + repair a file this SDK stamped as ABI 2, rebuild the
  touched partitions at v1 framing, recompute the global root, and leave the v2
  stamp untouched. The next current-SDK Open then reads a current stamp and
  trusts a mixed v1/v2 rollup — the silent multi-version digest the PR body says
  it prevents. Fix by tying the ABI certificate to state that a partial
  invalidation already destroys (for example encode the ABI version inside the
  global-root value written by packDigestLeaf, since every invalidation path
  deletes rawdb.GlobalGrantDigestNodeKey(), and treat a global root without a
  matching ABI tag as stale), or by bumping the engine schema version
  (SDKPebbleFormat) so pre-stamp SDKs refuse the file outright. Whichever way you
  go, add a two-artifact cross-version test: build digest state, mutate one
  partition through the pre-stamp code path (leaving the stamp), and assert the
  next Open does not trust the result.

In `pkg/sdk/version.go`:
- Around line 3: Version is still "v0.24.1". This PR changes durable on-disk
  digest state semantics and changes the output of the exported
  GrantContentHash for the same input, and bumps the exported
  GrantDigestABIVersion from 1 to 2. Per the repo review criteria a 0.x minor
  bump is the compatibility signal for serialized-state/default-behavior changes;
  bump it to v0.25.0 and add a short migration/rollout note to the PR body
  covering what happens to files sealed by older SDKs (dropped and rebuilt on the
  next writable open + EndSync) and to files this SDK seals that older SDKs may
  later write to.

In `pkg/dotc1z/engine/pebble/grant_digest_hash_test.go`:
- Around line 233 and the table cases above it: every immutable case sets
  GrantImmutable as the sole annotation, so the loop in
  scanGrantContentFactsRawBytes's `case 8` that walks multiple field-8
  occurrences is never exercised with more than one entry. Add table cases where
  the annotation list carries another annotation (e.g. GrantMetadata) both before
  and after GrantImmutable, and one where a GrantExpandable annotation was
  stripped by V2GrantToV3, asserting the raw-scanned isImmutable still matches
  the from-record path.

[github-actions]

No blocking issues found.