fix(advisor): validate hosted specialist artifacts

.github/workflows/pr-review-advisor.yaml

@@ -98,17 +98,6 @@ jobs:
       fail-fast: false
       matrix:
         advisor: ${{ fromJSON(needs.discover-specialists.outputs.matrix) }}
-    env:
-      # Pin runtime packages to reviewed versions. Updates go through normal
-      # dependency review rather than floating in a secret-bearing job.
-      PI_SDK_VERSION: "0.80.6"
-      # The advisor tools import TypeBox directly. Pi 0.80.6 shrinkwraps its
-      # own copy, so the advisor runtime must install this direct dependency.
-      TYPEBOX_VERSION: "1.1.38"
-      # Workflow-boundary modules parse YAML before the advisor session starts.
-      YAML_VERSION: "2.8.3"
-      # Embedded Pi SDK sessions use Pi's proxy-aware Undici transport.
-      UNDICI_VERSION: "8.10.0"
       FD_FIND_VERSION: "9.0.0-1"
       RIPGREP_VERSION: "14.1.0-1"
       OPENSHELL_GATEWAY_ENDPOINT: http://127.0.0.1:8080

test/automation/pull-requests/pr-review-advisor-openshell.test.ts

@@ -147,11 +147,13 @@ describe("PR review advisor specialist lifecycle", () => {
       startGateway: () => ({ configure: Promise.resolve() }),
       create: () => undefined,
       run: () => undefined,
-      download: () =>
-        void fs.writeFileSync(
+      download: () => {
+        fs.writeFileSync(
           path.join(artifactPath, "pr-review-behavior-summary.md"),
           "# Behavior specialist\n\nNo behavior finding.\n",
-        ),
+        );
+        fs.writeFileSync(path.join(artifactPath, "pr-review-behavior-session.jsonl"), "{}\n");
+      },
       remove: () => undefined,
     };
 
@@ -171,6 +173,38 @@ describe("PR review advisor specialist lifecycle", () => {
     );
   });
 
+  it("rejects an incomplete hosted specialist artifact before publication", async () => {
+    const workspace = temporaryDirectory();
+    const artifactDirectory = "pr-review-specialist-behavior";
+    const artifactPath = path.join(workspace, "artifacts", artifactDirectory);
+    const jobSummary = path.join(workspace, "job-summary.md");
+    fs.mkdirSync(artifactPath, { recursive: true });
+    fs.writeFileSync(jobSummary, "Existing summary.\n");
+    const lifecycle: AdvisorSpecialistLifecycle = {
+      prepare: async () => undefined,
+      startGateway: () => ({ configure: Promise.resolve() }),
+      create: () => undefined,
+      run: () => undefined,
+      download: () =>
+        void fs.writeFileSync(path.join(artifactPath, "pr-review-behavior-summary.md"), "# Summary\n"),
+      remove: () => undefined,
+    };
+
+    await expect(
+      runAdvisorSpecialistCommand(
+        "analysis",
+        {
+          GITHUB_STEP_SUMMARY: jobSummary,
+          GITHUB_WORKSPACE: workspace,
+          PR_REVIEW_ADVISOR_ARTIFACT_DIR: artifactDirectory,
+          PR_REVIEW_ADVISOR_INTEREST: "behavior",
+        },
+        lifecycle,
+      ),
+    ).rejects.toThrow("Specialist artifacts do not match");
+    expect(fs.readFileSync(jobSummary, "utf8")).toBe("Existing summary.\n");
+  });
+
   it("does not publish a specialist summary after cancellation during cleanup", async () => {
     const workspace = temporaryDirectory();
     const artifactDirectory = "pr-review-specialist-behavior";

tools/pr-review-advisor/local-review-implementation.mts

@@ -15,6 +15,7 @@ import {
   defaultAdvisorSpecialistLifecycle,
   redactAdvisorDiagnostic,
   runAdvisorSpecialist,
+  validateSpecialistArtifacts,
   type AdvisorSpecialistLifecycle,
 } from "./specialist-lifecycle.mts";
 import { ADVISOR_SPECIALISTS, type AdvisorSpecialist } from "./specialist-catalog.mts";
@@ -285,20 +286,6 @@ function stageArtifacts(
     },
   };
 }
-
-function validateSpecialistArtifacts(root: string, interest: string): void {
-  const directory = path.join(root, "artifacts", "pr-review-specialist-" + interest);
-  const expected = [`pr-review-${interest}-session.jsonl`, `pr-review-${interest}-summary.md`];
-  if (JSON.stringify(fs.readdirSync(directory).sort()) !== JSON.stringify(expected))
-    throw new Error("Specialist artifacts do not match the existing Markdown and JSONL contract");
-  if (
-    expected.some((name) => {
-      const stat = fs.lstatSync(path.join(directory, name));
-      return !stat.isFile() || stat.isSymbolicLink();
-    })
-  )
-    throw new Error("Specialist artifact must be a regular file");
-}
 function specialistEnvironment(
   advisorDirectory: string,
   output: string,
@@ -402,7 +389,7 @@ export async function runLocalReview(input: {
         ),
         lifecycle,
         prepare: false,
-        validate: () => validateSpecialistArtifacts(output, specialist.interest),
+        validate: () => validateSpecialistArtifacts(output, `pr-review-specialist-${specialist.interest}`, specialist.interest),
         setActiveCleanup: (value) => {
           activeCleanup = value;
         },

tools/pr-review-advisor/specialist-lifecycle.mts

@@ -178,6 +178,22 @@ export async function runAdvisorSpecialist(input: {
   if (cleanupError) throw cleanupError;
   return result;
 }
+export function validateSpecialistArtifacts(
+  root: string,
+  artifactDirectory: string,
+  interest: string,
+): void {
+  const directory = path.join(root, "artifacts", artifactDirectory);
+  const expected = [`pr-review-${interest}-session.jsonl`, `pr-review-${interest}-summary.md`];
+  if (JSON.stringify(fs.readdirSync(directory).sort()) !== JSON.stringify(expected))
+    throw new Error("Specialist artifacts do not match the existing Markdown and JSONL contract");
+  if (expected.some((name) => {
+    const stat = fs.lstatSync(path.join(directory, name));
+    return !stat.isFile() || stat.isSymbolicLink();
+  }))
+    throw new Error("Specialist artifact must be a regular file");
+}
+
 export function publishSpecialistJobSummary(env: NodeJS.ProcessEnv): void {
   const interest = env.PR_REVIEW_ADVISOR_INTEREST;
   const artifactDirectory = env.PR_REVIEW_ADVISOR_ARTIFACT_DIR;
@@ -248,8 +264,16 @@ export async function runAdvisorSpecialistCommand(
       },
       cancelled: () => received !== undefined,
     });
-    if (result === "complete" && received === undefined && env.GITHUB_STEP_SUMMARY)
+    if (result === "complete" && received === undefined && env.GITHUB_STEP_SUMMARY) {
+      if (!env.GITHUB_WORKSPACE || !env.PR_REVIEW_ADVISOR_ARTIFACT_DIR || !env.PR_REVIEW_ADVISOR_INTEREST)
+        throw new Error("Hosted specialist artifact environment is incomplete");
+      validateSpecialistArtifacts(
+        env.GITHUB_WORKSPACE,
+        env.PR_REVIEW_ADVISOR_ARTIFACT_DIR,
+        env.PR_REVIEW_ADVISOR_INTEREST,
+      );
       publishSpecialistJobSummary(env);
+    }
   } catch (error) {
     cancellationFailure ??= error;
     if (!received) throw error;