From caed46d6c3449405987df081e8a962b1992c5c28 Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 1 Aug 2026 02:02:01 +0200 Subject: [PATCH] fix(review): accept additive gate and projection surfaces capabilities.gates and capabilities.projections used assertExactSet, rejecting any provider release that advertises one extra gate or projection beyond the required floor. Decode both as plain string arrays instead of enumArray-against-the-known-enum, so an unknown addition reaches assertSupersetOf instead of being rejected earlier by the enum check, matching the superset-tolerant handling already used for operations and schemas. Unknown advertised gates/projections are still not leaked into internal use: the decoded ReviewCapabilitiesV2 narrows to the required members only, the same pattern already used for operations and schemas. Mandatory features keep assertExactSet unchanged, per the provider's own unknown_mandatory: "reject" contract. --- lib/review-integration-v2.ts | 17 +++++++---- runtime/review-integration-v2.mjs | 17 +++++++---- tests/review-integration-v2.test.ts | 44 +++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/lib/review-integration-v2.ts b/lib/review-integration-v2.ts index bab16f75..71b8c48e 100644 --- a/lib/review-integration-v2.ts +++ b/lib/review-integration-v2.ts @@ -712,12 +712,17 @@ export function decodeReviewCapabilitiesV2(value: unknown, verifiedExecutableDig if (selfReportedDigest !== normalizedVerifiedDigest) throw new TypeError("review provider executable digest mismatch"); const advertisedOperations = stringArray(body.operations, "capabilities.operations", { minimum: REQUIRED_OPERATIONS.length, unique: true }); - const gates = enumArray(body.gates, REQUIRED_GATES, "capabilities.gates", { minimum: 5, maximum: 5, unique: true }); - const projections = enumArray(body.projections, REQUIRED_PROJECTIONS, "capabilities.projections", { minimum: 2, maximum: 2, unique: true }); + // Gates and projections are, like operations and schemas, a superset promise + // rather than an exact manifest: a compatible provider release may advertise + // an additional gate or projection name beyond the required floor. Decode as + // a plain string array (not `enumArray` against the known enum) so an + // unknown addition is not rejected before assertSupersetOf can even run. + const advertisedGates = stringArray(body.gates, "capabilities.gates", { minimum: REQUIRED_GATES.length, unique: true }); + const advertisedProjections = stringArray(body.projections, "capabilities.projections", { minimum: REQUIRED_PROJECTIONS.length, unique: true }); const advertisedSchemas = stringArray(body.schemas, "capabilities.schemas", { minimum: REQUIRED_SCHEMAS.length, unique: true }); assertSupersetOf(advertisedOperations, REQUIRED_OPERATIONS, "capabilities operations"); - assertExactSet(gates, REQUIRED_GATES, "capabilities gates"); - assertExactSet(projections, REQUIRED_PROJECTIONS, "capabilities projections"); + assertSupersetOf(advertisedGates, REQUIRED_GATES, "capabilities gates"); + assertSupersetOf(advertisedProjections, REQUIRED_PROJECTIONS, "capabilities projections"); assertSupersetOf(advertisedSchemas, REQUIRED_SCHEMAS, "capabilities schemas"); const features = exactRecord(body.features, "capabilities.features", ["mandatory", "optional"]); @@ -764,8 +769,8 @@ export function decodeReviewCapabilitiesV2(value: unknown, verifiedExecutableDig buildId, executableDigest: selfReportedDigest, operations: new Set(REQUIRED_OPERATIONS), - gates: new Set(gates), - projections: new Set(projections), + gates: new Set(REQUIRED_GATES), + projections: new Set(REQUIRED_PROJECTIONS), schemas: new Set(REQUIRED_SCHEMAS), mandatoryFeatures: new Set(mandatoryNames), optionalFeatures: new Set(optional.filter((feature) => feature.supported && (FEATURE_NAMES as readonly string[]).includes(feature.name)).map((feature) => feature.name)), diff --git a/runtime/review-integration-v2.mjs b/runtime/review-integration-v2.mjs index a56f6423..b34fb3d5 100644 --- a/runtime/review-integration-v2.mjs +++ b/runtime/review-integration-v2.mjs @@ -713,12 +713,17 @@ export function decodeReviewCapabilitiesV2(value , verifiedExecutableDig if (selfReportedDigest !== normalizedVerifiedDigest) throw new TypeError("review provider executable digest mismatch"); const advertisedOperations = stringArray(body.operations, "capabilities.operations", { minimum: REQUIRED_OPERATIONS.length, unique: true }); - const gates = enumArray(body.gates, REQUIRED_GATES, "capabilities.gates", { minimum: 5, maximum: 5, unique: true }); - const projections = enumArray(body.projections, REQUIRED_PROJECTIONS, "capabilities.projections", { minimum: 2, maximum: 2, unique: true }); + // Gates and projections are, like operations and schemas, a superset promise + // rather than an exact manifest: a compatible provider release may advertise + // an additional gate or projection name beyond the required floor. Decode as + // a plain string array (not `enumArray` against the known enum) so an + // unknown addition is not rejected before assertSupersetOf can even run. + const advertisedGates = stringArray(body.gates, "capabilities.gates", { minimum: REQUIRED_GATES.length, unique: true }); + const advertisedProjections = stringArray(body.projections, "capabilities.projections", { minimum: REQUIRED_PROJECTIONS.length, unique: true }); const advertisedSchemas = stringArray(body.schemas, "capabilities.schemas", { minimum: REQUIRED_SCHEMAS.length, unique: true }); assertSupersetOf(advertisedOperations, REQUIRED_OPERATIONS, "capabilities operations"); - assertExactSet(gates, REQUIRED_GATES, "capabilities gates"); - assertExactSet(projections, REQUIRED_PROJECTIONS, "capabilities projections"); + assertSupersetOf(advertisedGates, REQUIRED_GATES, "capabilities gates"); + assertSupersetOf(advertisedProjections, REQUIRED_PROJECTIONS, "capabilities projections"); assertSupersetOf(advertisedSchemas, REQUIRED_SCHEMAS, "capabilities schemas"); const features = exactRecord(body.features, "capabilities.features", ["mandatory", "optional"]); @@ -765,8 +770,8 @@ export function decodeReviewCapabilitiesV2(value , verifiedExecutableDig buildId, executableDigest: selfReportedDigest, operations: new Set(REQUIRED_OPERATIONS), - gates: new Set(gates), - projections: new Set(projections), + gates: new Set(REQUIRED_GATES), + projections: new Set(REQUIRED_PROJECTIONS), schemas: new Set(REQUIRED_SCHEMAS), mandatoryFeatures: new Set(mandatoryNames), optionalFeatures: new Set(optional.filter((feature) => feature.supported && (FEATURE_NAMES ).includes(feature.name)).map((feature) => feature.name)), diff --git a/tests/review-integration-v2.test.ts b/tests/review-integration-v2.test.ts index 9276cafc..df3aa6e1 100644 --- a/tests/review-integration-v2.test.ts +++ b/tests/review-integration-v2.test.ts @@ -245,6 +245,50 @@ test("capabilities enforce the exact mandatory feature set while accepting a sup const extraOperation = clone(source); (extraOperation.operations as string[]).push("review.future_operation"); assert.doesNotThrow(() => decode(extraOperation)); + + // mandatory features must stay an exact match: a known-but-optional feature + // name added to the mandatory list must still be rejected. This is the + // contract boundary (unknown_mandatory: "reject") and must not regress when + // gates/projections become additive-tolerant below. + const extraMandatoryUnknownAddition = clone(source); + ((extraMandatoryUnknownAddition.features as JsonObject).mandatory as JsonObject[]).push({ name: "bounded_process_waits", supported: true, requires: [] }); + assert.throws(() => decode(extraMandatoryUnknownAddition), /mandatory/); +}); + +test("capabilities gates and projections accept an additive superset beyond the required floor", () => { + const source = fixture("capabilities.fixture.json"); + const decode = (value: unknown) => decodeReviewCapabilitiesV2(value, executableDigest); + + const extraGate = clone(source); + (extraGate.gates as string[]).push("future-gate"); + const decodedExtraGate = decode(extraGate) as { gates: ReadonlySet }; + assert.equal(decodedExtraGate.gates.has("future-gate"), false, "an unknown advertised gate must not leak into internal use"); + for (const gate of ["post-apply", "pre-commit", "pre-push", "pre-pr", "release"]) { + assert.equal(decodedExtraGate.gates.has(gate), true, gate); + } + assert.equal(decodedExtraGate.gates.size, 5); + + const extraProjection = clone(source); + (extraProjection.projections as string[]).push("future-projection"); + const decodedExtraProjection = decode(extraProjection) as { projections: ReadonlySet }; + assert.equal(decodedExtraProjection.projections.has("future-projection"), false, "an unknown advertised projection must not leak into internal use"); + for (const projection of ["staged", "workspace"]) { + assert.equal(decodedExtraProjection.projections.has(projection), true, projection); + } + assert.equal(decodedExtraProjection.projections.size, 2); +}); + +test("capabilities gates and projections still enforce the required floor", () => { + const source = fixture("capabilities.fixture.json"); + const decode = (value: unknown) => decodeReviewCapabilitiesV2(value, executableDigest); + + const missingGate = clone(source); + missingGate.gates = (missingGate.gates as string[]).filter((gate) => gate !== "release"); + assert.throws(() => decode(missingGate), /gates/); + + const missingProjection = clone(source); + missingProjection.projections = (missingProjection.projections as string[]).filter((projection) => projection !== "workspace"); + assert.throws(() => decode(missingProjection), /projections/); }); test("START independently binds base/candidate tree and the target-mode overlay pair", () => {