refactor(review): encode resource measurement invariants in the descriptor type

.changeset/review-resource-measurement-type.md

@@ -0,0 +1,2 @@
+---
+---

packages/hunk/src/app/review/resourceStore.ts

@@ -305,8 +305,7 @@ export class ReviewResourceStore {
     const digest = this.digest(bytes);
     if (
       isMaterializedReviewResource(descriptor) &&
-      (descriptor.byteLength !== bytes.byteLength ||
-        !reviewDigestsEqual(descriptor.digest!, digest))
+      (descriptor.byteLength !== bytes.byteLength || !reviewDigestsEqual(descriptor.digest, digest))
     ) {
       return reviewResourceFailure(
         "resource-integrity",

packages/hunk/src/core/review/resources.test.ts

@@ -6,6 +6,7 @@ import {
   REVIEW_PATCH_CONTENT_TYPE,
   REVIEW_RESOURCE_CHUNK_BYTES,
   reviewResourceId,
+  type ReviewResourceDescriptorV1,
 } from "./resources";
 
 const fileKey = "file:0123456789abcdef";
@@ -50,12 +51,30 @@ describe("resource materialization state", () => {
 
   test("is only complete when both measurements are present", () => {
     expect(isMaterializedReviewResource(descriptor)).toBe(false);
+    // @ts-expect-error A length without a digest is not a descriptor the type admits.
     expect(isMaterializedReviewResource({ ...descriptor, byteLength: 10 })).toBe(false);
+    // @ts-expect-error A digest without a length is not a descriptor the type admits.
     expect(isMaterializedReviewResource({ ...descriptor, digest: "a".repeat(64) })).toBe(false);
     expect(
       isMaterializedReviewResource({ ...descriptor, byteLength: 10, digest: "a".repeat(64) }),
     ).toBe(true);
   });
+
+  /** Reads measurements through the predicate alone, so only its narrowing can typecheck them. */
+  function readTestMeasurements(resource: ReviewResourceDescriptorV1) {
+    if (!isMaterializedReviewResource(resource)) {
+      throw new Error("expected a materialized descriptor");
+    }
+
+    const byteLength: number = resource.byteLength;
+    const digest: string = resource.digest;
+    return { byteLength, digest };
+  }
+
+  test("narrows a materialized descriptor to concrete measurements", () => {
+    const measured = { ...descriptor, byteLength: 10, digest: "a".repeat(64) };
+    expect(readTestMeasurements(measured)).toEqual({ byteLength: 10, digest: "a".repeat(64) });
+  });
 });
 
 describe("isReviewResourceRange", () => {

packages/hunk/src/core/review/resources.ts

@@ -52,43 +52,50 @@ export const REVIEW_CANONICAL_FILE_CONTENT_TYPE =
 export const REVIEW_PATCH_CONTENT_TYPE = "text/x-diff; charset=utf-8" as const;
 export const REVIEW_SOURCE_CONTENT_TYPE = "text/plain; charset=utf-8" as const;
 
+/**
+ * Measurements are set together once the producer has materialized the content, and never
+ * apart: a descriptor that declares one without the other cannot be verified and is a bug.
+ */
+export type ReviewResourceMeasurement =
+  | { byteLength: number; digest: string }
+  | { byteLength?: never; digest?: never };
+
 interface ReviewResourceDescriptorBase {
   id: string;
   generation: string;
   fileKey: string;
-  /**
-   * Set together once the producer has materialized the content, and never apart: a
-   * descriptor that declares one without the other cannot be verified and is a bug.
-   */
-  byteLength?: number;
-  digest?: string;
 }
 
-export interface ReviewCanonicalFileResourceV1 extends ReviewResourceDescriptorBase {
-  kind: "canonical-file";
-  contentType: typeof REVIEW_CANONICAL_FILE_CONTENT_TYPE;
-}
+export type ReviewCanonicalFileResourceV1 = ReviewResourceDescriptorBase &
+  ReviewResourceMeasurement & {
+    kind: "canonical-file";
+    contentType: typeof REVIEW_CANONICAL_FILE_CONTENT_TYPE;
+  };
 
-export interface ReviewPatchResourceV1 extends ReviewResourceDescriptorBase {
-  kind: "patch";
-  contentType: typeof REVIEW_PATCH_CONTENT_TYPE;
-}
+export type ReviewPatchResourceV1 = ReviewResourceDescriptorBase &
+  ReviewResourceMeasurement & {
+    kind: "patch";
+    contentType: typeof REVIEW_PATCH_CONTENT_TYPE;
+  };
 
-export interface ReviewSourceResourceV1 extends ReviewResourceDescriptorBase {
-  kind: "source";
-  contentType: typeof REVIEW_SOURCE_CONTENT_TYPE;
-  side: ReviewSide;
-  /** Identity of the content behind the reader, so a stale read is detectable. */
-  sourceIdentity: string;
-}
+export type ReviewSourceResourceV1 = ReviewResourceDescriptorBase &
+  ReviewResourceMeasurement & {
+    kind: "source";
+    contentType: typeof REVIEW_SOURCE_CONTENT_TYPE;
+    side: ReviewSide;
+    /** Identity of the content behind the reader, so a stale read is detectable. */
+    sourceIdentity: string;
+  };
 
 export type ReviewResourceDescriptorV1 =
   | ReviewCanonicalFileResourceV1
   | ReviewPatchResourceV1
   | ReviewSourceResourceV1;
 
 /** Whether one descriptor has been measured, and can therefore be verified when read. */
-export function isMaterializedReviewResource(descriptor: ReviewResourceDescriptorV1) {
+export function isMaterializedReviewResource(
+  descriptor: ReviewResourceDescriptorV1,
+): descriptor is Extract<ReviewResourceDescriptorV1, { byteLength: number }> {
   return descriptor.byteLength !== undefined && descriptor.digest !== undefined;
 }
 

packages/hunk/src/session/broker/state.ts

@@ -502,11 +502,11 @@ export class HunkSessionBrokerState extends SessionBrokerState<
     key: { sessionId: string; generation: string; resourceId: string },
     descriptor: ReviewResourceDescriptorV1,
   ): Promise<Uint8Array> {
-    const measured = isMaterializedReviewResource(descriptor);
+    const measured = isMaterializedReviewResource(descriptor) ? descriptor : undefined;
     const reservation: ReviewResourceReservation = this.resources.reserve(
       key,
       measured
-        ? descriptor.byteLength!
+        ? measured.byteLength
         : Math.min(REVIEW_RESOURCE_CHUNK_BYTES, reviewResourceCeiling(descriptor.kind)),
     );
     try {
@@ -516,7 +516,7 @@ export class HunkSessionBrokerState extends SessionBrokerState<
         digest: nodeReviewDigest,
         maxBytes: reviewResourceCeiling(descriptor.kind),
         ...(measured
-          ? { expected: { byteLength: descriptor.byteLength!, digest: descriptor.digest! } }
+          ? { expected: { byteLength: measured.byteLength, digest: measured.digest } }
           : {}),
       });