diff --git a/src/agents/contract-extract.ts b/src/agents/contract-extract.ts index 6f5ebfa..1998f02 100644 --- a/src/agents/contract-extract.ts +++ b/src/agents/contract-extract.ts @@ -7,6 +7,104 @@ export type Endpoint = { path: string; }; +export type ContractDiff = { + // Endpoints in Rails not implemented by EITHER mobile client. Informational + // only — Rails has admin / server-only endpoints that aren't called from + // mobile apps; this list helps surface what's available but unused. + railsOnly: Endpoint[]; + // Endpoints called by a mobile client that Rails doesn't expose. Real + // failure: client will get 404 at runtime. + iosOrphan: Endpoint[]; + androidOrphan: Endpoint[]; + // Endpoints implemented by exactly one mobile client. Real failure if + // both should be in feature parity (per "mobile clients must agree"). + iosOnly: Endpoint[]; + androidOnly: Endpoint[]; +}; + +export type CanonicalizationContext = { + // Lowercased rename target for "Shopkeeper" — the API role segment in the + // URL (`/vet/...` for Vet, `/shopkeeper/...` for the unrenamed substrate). + // Defaults to "shopkeeper" if the rename plan doesn't include Shopkeeper. + role: string; +}; + +const TENANT_PLACEHOLDER = "{account_id}"; +const UUID_RE = /^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/i; +const OPENAPI_PARAM_RE = /^\{[^}]+\}$/; +const SWIFT_INTERPOLATION_RE = /^\\\([^)]+\)$/; + +// Strip tenant prefix, /api/v1, role segment, and normalize path parameter +// names to {*} so the three platforms' encodings reduce to the same +// comparable string. Rails OpenAPI server URLs (e.g. `/api/v1/vet`) are +// already factored out by extractRailsContract, so Rails endpoints arrive +// here with just the per-operation path. +export function canonicalizeEndpoint( + endpoint: Endpoint, + ctx: CanonicalizationContext, +): Endpoint { + const role = ctx.role.toLowerCase(); + const segments = endpoint.path.split("/").filter(Boolean); + + // Drop tenant segment (literal placeholder or runtime UUID). + if (segments[0] === TENANT_PLACEHOLDER || (segments[0] && UUID_RE.test(segments[0]))) { + segments.shift(); + } + // Drop /api/v/. + if (segments[0] === "api") { + segments.shift(); + if (segments[0] && /^v\d+$/.test(segments[0])) segments.shift(); + } + // Drop role segment. + if (segments[0] === role) segments.shift(); + + // Normalize path-param names to {*} so {shopId} === {id} === \(id). + const normalized = segments.map((s) => + OPENAPI_PARAM_RE.test(s) || SWIFT_INTERPOLATION_RE.test(s) ? "{*}" : s, + ); + + return { method: endpoint.method, path: "/" + normalized.join("/") }; +} + +// Three-way diff. Endpoints are first canonicalized so cross-platform +// encoding differences don't show up as drift. +export function diffContracts( + rails: readonly Endpoint[], + ios: readonly Endpoint[], + android: readonly Endpoint[], + ctx: CanonicalizationContext, +): ContractDiff { + const canon = (e: Endpoint) => canonicalizeEndpoint(e, ctx); + const key = (e: Endpoint) => `${e.method} ${e.path}`; + + const railsSet = new Map(rails.map(canon).map((e) => [key(e), e])); + const iosSet = new Map(ios.map(canon).map((e) => [key(e), e])); + const androidSet = new Map(android.map(canon).map((e) => [key(e), e])); + + const railsOnly: Endpoint[] = []; + for (const [k, e] of railsSet) { + if (!iosSet.has(k) && !androidSet.has(k)) railsOnly.push(e); + } + const iosOrphan: Endpoint[] = []; + for (const [k, e] of iosSet) { + if (!railsSet.has(k)) iosOrphan.push(e); + } + const androidOrphan: Endpoint[] = []; + for (const [k, e] of androidSet) { + if (!railsSet.has(k)) androidOrphan.push(e); + } + const iosOnly: Endpoint[] = []; + for (const [k, e] of iosSet) { + if (railsSet.has(k) && !androidSet.has(k)) iosOnly.push(e); + } + const androidOnly: Endpoint[] = []; + for (const [k, e] of androidSet) { + if (railsSet.has(k) && !iosSet.has(k)) androidOnly.push(e); + } + + return { railsOnly, iosOrphan, androidOrphan, iosOnly, androidOnly }; +} + export type RailsContract = { openapiVersion: string; title: string; diff --git a/src/agents/judge.ts b/src/agents/judge.ts index d1cd50f..795af41 100644 --- a/src/agents/judge.ts +++ b/src/agents/judge.ts @@ -77,13 +77,15 @@ export async function runJudge(input: JudgeInput): Promise { const visualPass = visualReport ? Object.values(visualReport).every((r): r is VisualJudgePlatformReport => Boolean(r) && r!.pass) : true; - const overallPass = layer1Layer2Pass && visualPass; + const reviewerPass = input.reviewer.contractParity === "pass"; + const overallPass = layer1Layer2Pass && visualPass && reviewerPass; const l1Total = reports.filter((r) => r.layer1Pass).length; const l2Total = reports.filter((r) => r.layer2Pass).length; + const reviewerSummary = reviewerPass ? "reviewer PASS" : "reviewer FAIL"; return { overallPass, - summary: `Layer 1 ${l1Total}/3 pass · Layer 2 ${l2Total}/3 pass · ${layer3Summary}`, + summary: `Layer 1 ${l1Total}/3 pass · Layer 2 ${l2Total}/3 pass · ${layer3Summary} · ${reviewerSummary}`, ...(visualReport ? { visual: visualReport } : {}), }; } diff --git a/src/agents/reviewer.ts b/src/agents/reviewer.ts index d453bbf..bb20d3a 100644 --- a/src/agents/reviewer.ts +++ b/src/agents/reviewer.ts @@ -5,6 +5,9 @@ import { extractRailsContract, extractAndroidEndpoints, extractIosEndpoints, + diffContracts, + type ContractDiff, + type Endpoint, } from "./contract-extract.js"; import type { DomainSpec, ReviewerResult, WorkerResult } from "./types.js"; @@ -15,18 +18,19 @@ export type ReviewerInput = { android: WorkerResult; }; -// Phase 2: extract API surface from all three platforms — Rails OpenAPI, -// Android Retrofit interfaces, iOS Request structs — and surface counts in -// the trace + result. This proves rename left every platform's network -// layer parseable and provides the contract objects Phase 3 will use to -// detect actual drift (paths in Rails but missing from a client, methods -// that disagree, etc.). +// Phase 3: extract from all three platforms (Rails OpenAPI, iOS Request +// structs, Android Retrofit interfaces), canonicalize their path encodings +// (strip tenant {account_id}, /api/v, role segment, normalize path-param +// names), then diff. // -// For now reviewer stays at contractParity: "pass" unless extraction itself -// fails — count mismatches alone don't fail the run, since iOS / Android -// may legitimately implement a subset of Rails endpoints (e.g. the Rails -// admin namespace isn't called from mobile clients). Phase 3 wires the -// pass/fail logic against a normalized path-level diff. +// Pass/fail policy: +// - PASS if every iOS and Android endpoint maps to a Rails endpoint AND +// the two mobile clients implement the same set. +// - FAIL if any mobile-orphan exists (client calls something Rails +// doesn't expose — runtime 404), or if iOS and Android implement +// different subsets ("mobile clients must agree" per project USP). +// - rails-only endpoints are informational (admin / server-only routes +// that aren't supposed to be called from mobile). export async function runReviewer(input: ReviewerInput): Promise { if (isStub("reviewer")) return runStubReviewer(input); @@ -49,28 +53,74 @@ export async function runReviewer(input: ReviewerInput): Promise `Rails: ${railsContract.endpoints.length} endpoints, ${railsContract.schemaCount} schemas, openapi=${railsContract.openapiVersion}, title="${railsContract.title}"`, ); - trace("reviewer", `extracting iOS Request structs from ${ios.outDir}`); const iosEndpoints = await extractIosEndpoints(iosDir); - trace("reviewer", `iOS: ${iosEndpoints.length} request endpoints`); - - trace("reviewer", `extracting Android Retrofit annotations from ${android.outDir}`); const androidEndpoints = await extractAndroidEndpoints(androidDir); + trace("reviewer", `iOS: ${iosEndpoints.length} request endpoints`); trace("reviewer", `Android: ${androidEndpoints.length} Retrofit endpoints`); - trace("reviewer", "three-way diff — not yet implemented (Phase 3+)"); - trace("reviewer", `${domain.displayName}: contract parity PASS (Phase 2 extraction-only)`); - - return { - contractParity: "pass", - diffs: [ - `rails:openapi=${railsContract.openapiVersion}`, - `rails:title=${railsContract.title}`, - `rails:endpoints=${railsContract.endpoints.length}`, - `rails:schemas=${railsContract.schemaCount}`, - `ios:endpoints=${iosEndpoints.length}`, - `android:endpoints=${androidEndpoints.length}`, - ], - }; + const role = deriveRole(domain); + const diff = diffContracts(railsContract.endpoints, iosEndpoints, androidEndpoints, { role }); + + const summary = formatDiffSummary(diff); + for (const line of summary) trace("reviewer", line); + + const fatalCount = + diff.iosOrphan.length + diff.androidOrphan.length + diff.iosOnly.length + diff.androidOnly.length; + + const baseDiffs: string[] = [ + `rails:openapi=${railsContract.openapiVersion}`, + `rails:title=${railsContract.title}`, + `rails:endpoints=${railsContract.endpoints.length}`, + `rails:schemas=${railsContract.schemaCount}`, + `ios:endpoints=${iosEndpoints.length}`, + `android:endpoints=${androidEndpoints.length}`, + `role=${role}`, + `rails-only=${diff.railsOnly.length}`, + `ios-orphan=${diff.iosOrphan.length}`, + `android-orphan=${diff.androidOrphan.length}`, + `ios-only=${diff.iosOnly.length}`, + `android-only=${diff.androidOnly.length}`, + ]; + const findings: string[] = [ + ...formatFindings("ios-orphan", diff.iosOrphan), + ...formatFindings("android-orphan", diff.androidOrphan), + ...formatFindings("ios-only", diff.iosOnly), + ...formatFindings("android-only", diff.androidOnly), + ]; + + if (fatalCount > 0) { + trace("reviewer", `${domain.displayName}: contract parity FAIL — ${fatalCount} drift(s)`); + return { contractParity: "fail", diffs: [...baseDiffs, ...findings] }; + } + + trace("reviewer", `${domain.displayName}: contract parity PASS`); + return { contractParity: "pass", diffs: [...baseDiffs, ...findings] }; +} + +function deriveRole(domain: DomainSpec): string { + const pair = domain.renamePlan.find((p) => p.from === "Shopkeeper"); + return (pair?.to ?? "Shopkeeper").toLowerCase(); +} + +function formatDiffSummary(diff: ContractDiff): string[] { + const lines: string[] = []; + lines.push( + `diff: rails-only=${diff.railsOnly.length}, ios-orphan=${diff.iosOrphan.length}, android-orphan=${diff.androidOrphan.length}, ios-only=${diff.iosOnly.length}, android-only=${diff.androidOnly.length}`, + ); + if (diff.iosOrphan.length > 0) lines.push(`iOS calls endpoints not in Rails: ${preview(diff.iosOrphan)}`); + if (diff.androidOrphan.length > 0) lines.push(`Android calls endpoints not in Rails: ${preview(diff.androidOrphan)}`); + if (diff.iosOnly.length > 0) lines.push(`iOS-only (Android missing): ${preview(diff.iosOnly)}`); + if (diff.androidOnly.length > 0) lines.push(`Android-only (iOS missing): ${preview(diff.androidOnly)}`); + return lines; +} + +function formatFindings(label: string, endpoints: readonly Endpoint[]): string[] { + return endpoints.map((e) => `${label}:${e.method} ${e.path}`); +} + +function preview(endpoints: readonly Endpoint[], max: number = 3): string { + const sample = endpoints.slice(0, max).map((e) => `${e.method} ${e.path}`).join(", "); + return endpoints.length > max ? `${sample}, ... (+${endpoints.length - max} more)` : sample; } const delay = (ms: number): Promise => new Promise((r) => { setTimeout(r, ms); }); diff --git a/tests/smoke.test.ts b/tests/smoke.test.ts index ecd3e2c..fb26d8f 100644 --- a/tests/smoke.test.ts +++ b/tests/smoke.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { runLayer1, runLayer2, runLayer3, captureScreenshot, installAndLaunch, runVisualJudge, DEFAULT_STAGE1_RUBRIC, discoverIosArtifact, discoverAndroidArtifact, runStage1Visual } from "../src/validation/index.js"; import { dispatch } from "../src/dispatch.js"; import { runReviewer } from "../src/agents/reviewer.js"; +import { canonicalizeEndpoint, diffContracts } from "../src/agents/contract-extract.js"; test("validation layers are exported as functions", () => { assert.equal(typeof runLayer1, "function"); @@ -165,6 +166,38 @@ test("runReviewer in stub mode passes without touching disk", async () => { assert.deepEqual(result.diffs, []); }); +test("canonicalizeEndpoint reduces all three platform encodings to the same string", async () => { + const ctx = { role: "vet" }; + // Rails OpenAPI: server is /api/v1/vet, path is just /clinics + const rails = canonicalizeEndpoint({ method: "GET", path: "/clinics" }, ctx); + // iOS Request struct: "/vet/shops/\(id)" (literal Swift interpolation in extracted string) + const ios = canonicalizeEndpoint({ method: "GET", path: "/vet/clinics/\\(id)" }, ctx); + // Android Retrofit: literal full path + const android = canonicalizeEndpoint( + { method: "GET", path: "{account_id}/api/v1/vet/clinics/{id}" }, + ctx, + ); + assert.equal(rails.path, "/clinics"); + assert.equal(ios.path, "/clinics/{*}"); + assert.equal(android.path, "/clinics/{*}"); +}); + +test("diffContracts surfaces ios orphan when client calls endpoint not in rails", async () => { + const ctx = { role: "vet" }; + const rails = [{ method: "GET" as const, path: "/clinics" }]; + const ios = [ + { method: "GET" as const, path: "/vet/clinics" }, + { method: "DELETE" as const, path: "/vet/clinics/\\(id)/reset" }, + ]; + const android = [{ method: "GET" as const, path: "{account_id}/api/v1/vet/clinics" }]; + const diff = diffContracts(rails, ios, android, ctx); + assert.equal(diff.iosOrphan.length, 1); + assert.equal(diff.iosOrphan[0]?.method, "DELETE"); + assert.equal(diff.iosOrphan[0]?.path, "/clinics/{*}/reset"); + assert.equal(diff.iosOnly.length, 0); + assert.equal(diff.androidOnly.length, 0); +}); + test("dispatch runs planner + workers + reviewer + judge end-to-end (stub pipeline)", async () => { const result = await dispatch("a walk-in clinic queue for small veterinary practices"); assert.equal(result.overallPass, true);