fix: preserve workflow evidence and independently validate repairs

.gitignore

@@ -7,3 +7,4 @@ memory-bank/
 .idea/
 TODO.md
 plans/
+.grok

.pi/extensions/pi-workflow-engine/src/advisory-challenge.ts

@@ -0,0 +1,75 @@
+import { Type, type Static } from "typebox";
+import { collectAdvisoryStage, type AdvisoryStageCoverage } from "./advisory-evidence.ts";
+import { DEFAULT_ADVISORY_TOOLS, DEFAULT_ADVISORY_TOOL_HINTS, type AdvisoryVerified } from "./workflow-advisory-utils.ts";
+import type { WorkflowApi } from "./types.ts";
+
+const ChallengeSchema = Type.Object({
+  outcome: Type.Union([Type.Literal("counterexample"), Type.Literal("alternative-explanation"), Type.Literal("supports-original"), Type.Literal("no-counterexample")]),
+  evidence: Type.Array(Type.String()),
+  experiment: Type.String({ description: "Smallest distinguishing experiment, with observed result if actually run." }),
+});
+const AdjudicationSchema = Type.Object({
+  outcome: Type.Union([Type.Literal("upheld"), Type.Literal("refuted"), Type.Literal("unresolved")]),
+  evidence: Type.Array(Type.String()),
+  reason: Type.String(),
+});
+export type ChallengeRecord =
+  | { status: "complete"; challenge: Static<typeof ChallengeSchema>; adjudication: Static<typeof AdjudicationSchema> }
+  | { status: "failed"; challenge?: Static<typeof ChallengeSchema> };
+export interface AdvisoryChallengeOptions {
+  maxChallenges: number;
+  shouldChallenge?: (finding: AdvisoryVerified) => boolean;
+}
+
+/** Opt in with --challenge or --challenge=N (hard bound of 10). */
+export function parseChallengeArgs(args: string): { args: string; options: AdvisoryChallengeOptions } {
+  let maxChallenges = 0;
+  const remaining = args.replace(/(?:^|\s)--challenge(?:=([^\s]*))?(?=\s|$)/g, (_match, limit: string | undefined) => {
+    if (limit !== undefined && !/^\d+$/.test(limit)) {
+      throw new Error(`Invalid --challenge value "${limit}". Use --challenge or --challenge=N with a non-negative integer; zero disables challenges.`);
+    }
+    maxChallenges = Math.min(10, limit === undefined ? 3 : Number(limit));
+    return " ";
+  });
+  return { args: remaining.trim(), options: { maxChallenges } };
+}
+
+export function needsChallenge(finding: AdvisoryVerified): boolean {
+  return finding.verdict === "PLAUSIBLE" || finding.verdict === "NOT_SUBSTANTIATED" || finding.evidence.length === 0 ||
+    /\b(security|concurren\w*|race|persist\w*|cancel\w*|retr(?:y|ies)|data loss|corrupt\w*|high impact)\b/i.test(`${finding.category} ${finding.summary} ${finding.impact}`);
+}
+
+/** A bounded recipe using existing agent/parallel calls, not a new runtime primitive. */
+export async function challengeFindings<T extends AdvisoryVerified>(
+  api: Pick<WorkflowApi, "agent" | "parallel">,
+  findings: T[],
+  context: string,
+  options: AdvisoryChallengeOptions,
+  coverage: AdvisoryStageCoverage[],
+): Promise<T[]> {
+  const limit = Number.isFinite(options.maxChallenges) ? Math.max(0, Math.min(10, Math.trunc(options.maxChallenges))) : 0;
+  if (limit === 0) return findings;
+  const selected = findings.filter((finding) => finding.verdict !== "REFUTED" && (options.shouldChallenge ?? needsChallenge)(finding)).slice(0, limit);
+  const replacements = new Map<string, T>();
+  await collectAdvisoryStage(api, "Challenge", selected.map((finding) => ({ id: finding.candidateId, run: async () => {
+    // Preserve the candidate as unresolved if either independent stage fails.
+    replacements.set(finding.candidateId, { ...finding, verdict: "NOT_SUBSTANTIATED", challenge: { status: "failed" } });
+    const challenge = await api.agent(
+      `Assume this finding is a false positive. Try to DISPROVE it. Find the strongest concrete counterexample or alternative root cause. Inspect callers, invariants, tests and control flow. For a repair, seek an input, race or error path that still fails. State the smallest experiment distinguishing explanations. Do not edit files or claim tests you did not run. No counterexample found is not proof.\n\nExact review context:\n${context}\n\nCandidate and verifier evidence:\n${JSON.stringify(finding)}`,
+      { label: `challenge:${finding.candidateId}`, phase: "Challenge", profile: "medium", tools: DEFAULT_ADVISORY_TOOLS, toolHints: DEFAULT_ADVISORY_TOOL_HINTS, schema: ChallengeSchema },
+    );
+    replacements.set(finding.candidateId, { ...finding, verdict: "NOT_SUBSTANTIATED", challenge: { status: "failed", challenge } });
+    const adjudication = await api.agent(
+      `Adjudicate the original finding, independent verifier evidence and falsification attempt below. Preserve unresolved conflict; do not force consensus. A missing counterexample alone cannot upgrade a plausible claim. Cite concrete evidence and observed test results; never invent experiments.\nContext:\n${context}\nOriginal and verifier:\n${JSON.stringify(finding)}\nChallenger:\n${JSON.stringify(challenge)}`,
+      { label: `adjudicate:${finding.candidateId}`, phase: "Challenge", profile: "medium", tools: [], schema: AdjudicationSchema },
+    );
+    replacements.set(finding.candidateId, {
+      ...finding,
+      verdict: adjudication.outcome === "refuted" ? "REFUTED" : adjudication.outcome === "unresolved" ? "NOT_SUBSTANTIATED" : finding.verdict,
+      evidence: [...finding.evidence, ...challenge.evidence, ...(challenge.experiment ? [challenge.experiment] : []), ...adjudication.evidence, adjudication.reason],
+      challenge: { status: "complete", challenge, adjudication },
+    });
+    return finding.candidateId;
+  } })), coverage);
+  return findings.map((finding) => replacements.get(finding.candidateId) ?? finding);
+}

.pi/extensions/pi-workflow-engine/src/advisory-evidence.ts

@@ -0,0 +1,90 @@
+import { createHash } from "node:crypto";
+import { Type, type Static } from "typebox";
+import { AdvisorySeveritySchema, type IdentifiedAdvisoryCandidate, type AdvisoryCandidate, type AdvisoryReport } from "./advisory-schema.ts";
+import type { WorkflowApi } from "./types.ts";
+
+export interface AdvisoryStageCoverage {
+  stage: string;
+  expected: number;
+  completed: number;
+  failed: number;
+  failures: { branch: string; reason: string; candidate?: AdvisoryCandidate }[];
+}
+
+/** Settled results are required here: a missing branch is not a negative finding. */
+export async function collectAdvisoryStage<T>(
+  api: Pick<WorkflowApi, "parallel">,
+  stage: string,
+  branches: { id: string; candidate?: AdvisoryCandidate; run(): Promise<T> }[],
+  coverage: AdvisoryStageCoverage[],
+): Promise<T[]> {
+  const results = await api.parallel(branches.map((branch) => branch.run), { settled: true });
+  const failures = results.flatMap((result, index) => result.ok ? [] : [{ branch: branches[index]!.id, reason: result.error.message, ...(branches[index]!.candidate ? { candidate: branches[index]!.candidate } : {}) }]);
+  coverage.push({ stage, expected: branches.length, completed: results.length - failures.length, failed: failures.length, failures });
+  return results.flatMap((result) => result.ok ? [result.value] : []);
+}
+
+export function identifyCandidates(candidates: readonly AdvisoryCandidate[], lens: string): IdentifiedAdvisoryCandidate[] {
+  return candidates.map((candidate, index) => {
+    const candidateId = createHash("sha256").update(JSON.stringify([lens, index, candidate])).digest("hex").slice(0, 20);
+    // Identities are assigned by the workflow, never accepted from a finder.
+    return { ...candidate, candidateId, sourceCandidateIds: [candidateId] };
+  });
+}
+
+function normalizedSummary(summary: string): string {
+  return summary.toLowerCase().replace(/\s+/g, " ").trim().replace(/[.!?]+$/, "");
+}
+
+function candidateDedupKey(candidate: AdvisoryCandidate): string {
+  const location = candidate.locations[0];
+  return JSON.stringify([candidate.category, location?.file.replace(/^\.\//, "").replace(/^[ab]\//, ""),
+    location?.symbol ?? location?.line ?? null, normalizedSummary(candidate.summary)]);
+}
+
+/** Collapse only equivalent claims at the same anchor; keep every discovery ID and location. */
+export function dedupeCandidates<T extends IdentifiedAdvisoryCandidate>(candidates: readonly T[]): T[] {
+  const seen = new Map<string, T>();
+  for (const candidate of candidates) {
+    const key = candidateDedupKey(candidate);
+    const previous = seen.get(key);
+    if (!previous) seen.set(key, { ...candidate });
+    else {
+      previous.sourceCandidateIds = [...new Set([...previous.sourceCandidateIds, ...candidate.sourceCandidateIds])];
+      previous.locations = uniqueLocations([...previous.locations, ...candidate.locations]);
+      previous.impact = [...new Set([previous.impact, candidate.impact])].join("\n");
+      previous.discoveryEvidence = [...new Set([...(previous.discoveryEvidence ?? []), ...(candidate.discoveryEvidence ?? [])])];
+    }
+  }
+  return [...seen.values()];
+}
+
+export function uniqueLocations(locations: AdvisoryCandidate["locations"]): AdvisoryCandidate["locations"] {
+  return [...new Map(locations.map((location) => [JSON.stringify(location), location])).values()];
+}
+
+// Synthesis selects evidence records. It has no fields with which to replace their evidence.
+export const AdvisorySynthesisSchema = Type.Object({
+  summary: Type.String(),
+  findings: Type.Array(Type.Object({
+    sourceCandidateIds: Type.Array(Type.String(), { minItems: 1 }),
+    severity: AdvisorySeveritySchema,
+    recommendation: Type.String(),
+  })),
+  nextSteps: Type.Array(Type.String()),
+});
+export type AdvisorySynthesis = Static<typeof AdvisorySynthesisSchema>;
+export const SYNTHESIS_ID_INSTRUCTIONS = "Select or merge findings only by sourceCandidateIds shown below. Return severity and an advisory recommendation per selection. Never select an ID absent from these verified records. ";
+
+export function withAdvisoryCoverage<T extends AdvisoryReport>(report: T, coverage: AdvisoryStageCoverage[]) {
+  const incomplete = coverage.some((stage) => stage.failed > 0);
+  const gaps = coverage.flatMap((stage) => stage.failures.map((failure) => `${stage.stage}/${failure.branch}: ${failure.reason}`));
+  return {
+    ...report,
+    status: incomplete ? "incomplete" as const : "complete" as const,
+    summary: incomplete ? `Incomplete review: ${gaps.length} branch(es) failed. ${report.findings.length} finding(s) available; no clean conclusion is possible.` : report.summary,
+    nextSteps: incomplete ? ["Inspect failed branches and rerun missing work.", ...(report.findings.length > 0 ? report.nextSteps : [])] : report.nextSteps,
+    coverage,
+    gaps,
+  };
+}

.pi/extensions/pi-workflow-engine/src/advisory-schema.ts

@@ -9,7 +9,7 @@ export const AdvisoryConfidenceSchema = Type.Union([Type.Literal("low"), Type.Li
   description: "Confidence that the finding is actionable.",
 });
 
-export const AdvisoryVerifierVerdictSchema = Type.Union([Type.Literal("CONFIRMED"), Type.Literal("PLAUSIBLE"), Type.Literal("REFUTED")], {
+export const AdvisoryVerifierVerdictSchema = Type.Union([Type.Literal("CONFIRMED"), Type.Literal("PLAUSIBLE"), Type.Literal("REFUTED"), Type.Literal("NOT_SUBSTANTIATED")], {
   description: "Verifier judgment for a candidate finding.",
 });
 
@@ -20,6 +20,8 @@ export const AdvisoryLocationSchema = Type.Object({
 });
 
 export const AdvisoryCandidateSchema = Type.Object({
+  discoveryEvidence: Type.Optional(Type.Array(Type.String())),
+  reviewAnchor: Type.Optional(AdvisoryLocationSchema),
   summary: Type.String({ description: "One-line candidate finding or hypothesis." }),
   category: Type.String({ description: "Workflow-specific category such as bug, duplication, root-cause, or io." }),
   locations: Type.Array(AdvisoryLocationSchema, { description: "Relevant code or configuration locations." }),
@@ -38,6 +40,9 @@ export const AdvisoryVerdictSchema = Type.Object({
 });
 
 export const AdvisoryFindingSchema = Type.Object({
+  verdict: Type.Optional(AdvisoryVerifierVerdictSchema),
+  sourceCandidateIds: Type.Optional(Type.Array(Type.String())),
+  reviewAnchor: Type.Optional(AdvisoryLocationSchema),
   summary: Type.String({ description: "One-line final finding." }),
   category: Type.String({ description: "Workflow-specific category for grouping and display." }),
   severity: AdvisorySeveritySchema,
@@ -49,6 +54,12 @@ export const AdvisoryFindingSchema = Type.Object({
 });
 
 export const AdvisoryReportSchema = Type.Object({
+  status: Type.Optional(Type.Union([Type.Literal("complete"), Type.Literal("incomplete")])),
+  gaps: Type.Optional(Type.Array(Type.String())),
+  coverage: Type.Optional(Type.Array(Type.Object({
+    stage: Type.String(), expected: Type.Number(), completed: Type.Number(), failed: Type.Number(),
+    failures: Type.Array(Type.Object({ branch: Type.String(), reason: Type.String(), candidate: Type.Optional(AdvisoryCandidateSchema) })),
+  }))),
   summary: Type.String({ description: "Short overall advisory summary." }),
   findings: Type.Array(AdvisoryFindingSchema, { description: "Verified and ranked advisory findings." }),
   nextSteps: Type.Array(Type.String({ description: "Concrete follow-up commands, inspections, or decisions." })),
@@ -61,6 +72,7 @@ export const AdvisoryReportWithStatsSchema = Type.Object({
 
 export type AdvisoryLocation = Static<typeof AdvisoryLocationSchema>;
 export type AdvisoryCandidate = Static<typeof AdvisoryCandidateSchema>;
+export type IdentifiedAdvisoryCandidate = AdvisoryCandidate & { candidateId: string; sourceCandidateIds: string[] };
 export type AdvisoryVerdict = Static<typeof AdvisoryVerdictSchema>;
 export type AdvisoryFinding = Static<typeof AdvisoryFindingSchema>;
 export type AdvisoryReport = Static<typeof AdvisoryReportSchema>;

.pi/extensions/pi-workflow-engine/src/agent-workspace.ts

@@ -21,7 +21,7 @@ export type AgentWorkspace = SharedAgentWorkspace | IsolatedAgentWorkspace;
 
 export interface AgentWorkspaceContext {
   readonly cwd: string;
-  readonly worktrees: Pick<WorktreeRegistry, "probe" | "add" | "capturePatch" | "remove">;
+  readonly worktrees: Pick<WorktreeRegistry, "probe" | "add" | "capturePatch" | "applyPatch" | "remove">;
   readonly signal: AbortSignal | undefined;
   readonly progress: Pick<AgentProgress, "log">;
 }
@@ -31,6 +31,7 @@ export async function createAgentWorkspace(
   opts: AgentExecutionOptions,
   label: string,
 ): Promise<AgentWorkspace> {
+  if (opts.candidatePatch && opts.isolation !== "worktree") throw new Error("Candidate evaluation requires worktree isolation");
   if (opts.isolation !== "worktree") {
     return {
       kind: "shared",
@@ -53,6 +54,19 @@ export async function createAgentWorkspace(
   const added = await rc.worktrees.add(rc.signal, opts.worktreeBaseline);
   if ("error" in added) throw new Error(`Failed to create isolated worktree: ${added.error}`);
   const worktreePath = added.path;
+  if (opts.candidatePatch) {
+    try {
+      if (opts.candidatePatch.baselineOid !== added.baselineOid) throw new Error("Candidate baseline differs from evaluator baseline");
+      if (opts.candidatePatch.patch.trim()) {
+        const applied = await rc.worktrees.applyPatch(worktreePath, opts.candidatePatch.patch, rc.signal);
+        if (!applied.ok) throw new Error(`Candidate patch could not be applied: ${applied.error ?? applied.stderr}`);
+      }
+    } catch (error) {
+      const removed = await rc.worktrees.remove(worktreePath);
+      if (!removed.ok) rc.progress.log(`${label}: evaluator setup cleanup failed: ${removed.error ?? removed.stderr}`);
+      throw error;
+    }
+  }
   rc.progress.log(`${label}: using isolated worktree ${worktreePath}`);
 
   return {
@@ -62,7 +76,7 @@ export async function createAgentWorkspace(
     async wrapResult(result) {
       const patch = await rc.worktrees.capturePatch(worktreePath, added.baselineOid, rc.signal);
       if ("error" in patch) throw new Error(`Failed to capture isolated worktree patch: ${patch.error}`);
-      return { result, patch: patch.patch, changed: patch.changed };
+      return { result, patch: patch.patch, changed: patch.changed, baselineOid: added.baselineOid };
     },
     async dispose() {
       const removed = await rc.worktrees.remove(worktreePath);

.pi/extensions/pi-workflow-engine/src/concurrency.ts

@@ -14,22 +14,28 @@ export class Semaphore {
   private active = 0;
   private readonly waiters: Array<() => void> = [];
 
-  constructor(private readonly max: number) {}
+  constructor(private readonly max: number) {
+    if (!Number.isInteger(max) || max < 1) throw new RangeError("Semaphore capacity must be a positive integer");
+  }
 
   async run<T>(fn: () => Promise<T>, options: { onQueueWaitMs?: (durationMs: number) => void; signal?: AbortSignal } = {}): Promise<T> {
     throwIfAborted(options.signal);
     const queuedAt = performance.now();
     if (this.active >= this.max) {
       await this.waitForSlot(options.signal);
+    } else {
+      this.active++;
     }
-    options.onQueueWaitMs?.(performance.now() - queuedAt);
-    throwIfAborted(options.signal);
-    this.active++;
     try {
+      options.onQueueWaitMs?.(performance.now() - queuedAt);
+      throwIfAborted(options.signal);
       return await fn();
     } finally {
-      this.active--;
-      this.waiters.shift()?.();
+      // Transfer ownership directly. A selected waiter owns this reservation
+      // even before its continuation runs, including if it is then cancelled.
+      const next = this.waiters.shift();
+      if (next) next();
+      else this.active--;
     }
   }
 

.pi/extensions/pi-workflow-engine/src/journal.ts

@@ -115,6 +115,7 @@ function captureAgentCallHash(
         skills: opts.skills,
         schema: opts.schema,
         isolation: opts.isolation,
+        candidatePatch: opts.candidatePatch ? { baselineOid: opts.candidatePatch.baselineOid, hash: createHash("sha256").update(opts.candidatePatch.patch).digest("hex") } : undefined,
         worktreeBaseline: worktreeBaseline
           ? {
               ref: worktreeBaseline.ref,

.pi/extensions/pi-workflow-engine/src/review/code-review-orchestration.ts

@@ -1,6 +1,3 @@
-import type { AdvisoryCandidate } from "../advisory-schema.ts";
-import { normalizePath, primaryLocation } from "../workflow-advisory-utils.ts";
-
 const DIFF_EMBED_CAP = 60_000;
 
 export function buildCodeReviewScopeBlock(input: {
@@ -25,25 +22,3 @@ export function buildCodeReviewScopeBlock(input: {
     (input.target ? `\n## User instructions (verbatim)\n${input.target}\n` : "")
   );
 }
-
-export function dedupeCodeReviewCandidates<Context>(
-  groups: readonly { readonly angle: Context; readonly candidates: readonly AdvisoryCandidate[] }[],
-): Array<{ readonly angle: Context; readonly candidate: AdvisoryCandidate }> {
-  const seen = new Set<string>();
-  return groups.flatMap(({ angle, candidates }) =>
-    candidates
-      .filter((candidate) => {
-        const key = codeReviewDedupKey(candidate);
-        if (seen.has(key)) return false;
-        seen.add(key);
-        return true;
-      })
-      .map((candidate) => ({ angle, candidate })),
-  );
-}
-
-function codeReviewDedupKey(candidate: AdvisoryCandidate): string {
-  const location = primaryLocation(candidate);
-  const lineKey = location.line != null ? Math.round(location.line / 5) * 5 : candidate.summary.slice(0, 40).toLowerCase();
-  return `${normalizePath(location.file)}:${lineKey}`;
-}