fix: restore offer-on-launch for Pi PR-boundary review (in-memory session baseline)

Bug A: the offer-once reconciliation derived inSessionContinuation from the
on-disk ack (isAncestor(lastAck, head)), which is true on every fresh pi launch
whose head descends from a prior ack, so reviewers auto-started on launch instead
of offering. Decide continuation from an in-memory per-session baseline instead:
reviewBaselineContinuation() autostarts only when the head advances beyond the
head first observed this session; an inherited head offers. Mirrored to ~/.pi.

Author: nikolanovoselec

preseed/agents/pi/extensions/review-enforcement.ts

@@ -14,7 +14,7 @@ import { basename, dirname, join } from "node:path";
 import { getMarkdownTheme, type ExtensionAPI } from "@earendil-works/pi-coding-agent";
 import { Markdown, Text } from "@earendil-works/pi-tui";
 import { ALL_REVIEW_LANES, bypassAckHeadForStatus, classifyReviewFiles, classifyReviewHead, commandTextFromEvent, createReadyOnceTracker, cwdFromBoundaryCommand, enforcedHeadDecision, extractBackgroundAgentId, isFailedToolExecution, isPrBoundaryTrigger, prBoundaryCommandBase, prUrlFromText, reusablePendingReview, selectReviewBase, type ReviewHeadStatus, type ReviewSpawnRequest } from "./review-helpers";
-import { compactDurableReviewStatus, countReviewSeverities, durableReviewAckReady, durableReviewEligibleLanes, durableReviewInitialLanes, durableReviewMessageKey, durableReviewRecommendation, formatMergedReviewSummary, requestReviewAutofixForRows, reviewAutofixModeFromUserMessages, shouldCheckOpenPrReconciliation, shouldReconcileOpenPr, reconcileBoundaryAction, type DurableReviewSummaryRecord, type DurableReviewSummaryRow, type ReviewSeverityCounts } from "./review-job-helpers";
+import { compactDurableReviewStatus, countReviewSeverities, durableReviewAckReady, durableReviewEligibleLanes, durableReviewInitialLanes, durableReviewMessageKey, durableReviewRecommendation, formatMergedReviewSummary, requestReviewAutofixForRows, reviewAutofixModeFromUserMessages, shouldCheckOpenPrReconciliation, shouldReconcileOpenPr, reconcileBoundaryAction, reviewBaselineContinuation, type DurableReviewSummaryRecord, type DurableReviewSummaryRow, type ReviewSeverityCounts } from "./review-job-helpers";
 import { appendReviewEvent, completedDurableReviewLanes, failedDurableReviewLanes, readDurableReviewJob, reapDurableReviewLanes, reviewJobDir, reviewResultPath, reviewResultsDir, runningDurableReviewLanes, startDurableReviewLanes } from "./review-jobs";
 
 const REVIEW_BYPASS = "/tmp/review-bypass";
@@ -32,6 +32,12 @@ const REVIEW_REQUEST_RETRY_MS = 60 * 1000;
 // to at most once per window; forced ticks (session_start, agent_end, PR-URL fallback) bypass it.
 const RECONCILE_THROTTLE_MS = 20 * 1000;
 let lastReconcileCheckAt = 0;
+// Per-repo enforced head observed when THIS Pi session first reconciled (≈ session start).
+// In-memory only (never persisted): a head present at launch is not an in-session push, so it
+// must offer, not auto-start. Only a head that later advances beyond this baseline is treated
+// as in-session continuation (a dropped on-tool-end push) and auto-starts. Process-scoped so a
+// fresh `pi` launch always re-baselines and offers — restoring the offer-on-start behavior.
+const reviewSessionBaselineHead = new Map<string, string>();
 
 type PrState = {
   state?: string;
@@ -1119,16 +1125,16 @@ export default function (pi: ExtensionAPI) {
     }
     // REQ-AGENT-058 (revised): decide whether this missed boundary is in-session continuation
     // work whose onToolEnd auto-start was dropped (compound `&&`, here-doc, `gh pr edit`, or a
-    // reload between the command and its event), or a fresh clone/checkout of a repo that already
-    // has an open PR. The new head is in-session continuation when it DESCENDS from a head we
-    // already acked in this repo; a fresh clone has no prior ack (the ack lives in .git, which a
-    // clone does not carry). Continuation AUTO-STARTS exactly like the onToolEnd boundary path
-    // (the locked design: an in-session push always auto-starts), so a dropped tool event no
-    // longer silently skips the review. A fresh clone falls through to OFFER once — `git clone`
-    // never auto-spawns an unstoppable review (the reaper that re-spawned killed lanes was the
-    // original reason the catch-up was made offer-only).
-    const lastAck = lastAckHead(resolvedRepo);
-    const inSessionContinuation = Boolean(lastAck) && lastAck !== head && isAncestor(resolvedRepo, lastAck, head);
+    // reload between the command and its event), or a head we simply inherited at launch (a fresh
+    // clone/checkout, or just relaunching `pi` on an existing repo with an open PR). The signal is
+    // an IN-MEMORY, per-session baseline: the head this session first observed here. A head present
+    // at launch (baseline unset on this first reconcile, or unchanged since) is NOT an in-session
+    // push, so it OFFERS; only a head that later ADVANCES beyond the baseline auto-starts. This
+    // restores the offer-on-launch behavior — keying off the on-disk ack instead made every launch
+    // whose head descended from a prior ack auto-start, which is the regression being fixed.
+    const baseline = reviewSessionBaselineHead.get(resolvedRepo);
+    if (baseline === undefined) reviewSessionBaselineHead.set(resolvedRepo, head);
+    const inSessionContinuation = reviewBaselineContinuation(baseline, head, (a, b) => isAncestor(resolvedRepo, a, b));
     const action = reconcileBoundaryAction({
       reconcile: decision.reconcile,
       alreadyOffered: offered(resolvedRepo, head),

preseed/agents/pi/extensions/review-job-helpers.ts

@@ -854,6 +854,24 @@ export function reconcileBoundaryAction(input: ReconcileBoundaryInput): Reconcil
   return "offer";
 }
 
+// In-session continuation = the enforced head ADVANCED during THIS Pi session, i.e. it
+// differs from and descends from the head observed when this session first reconciled
+// (the in-memory baseline, captured at session start). That is the only signal of an
+// in-session push whose on-tool-end auto-start was dropped (so reconciliation should
+// auto-start). A head present at session start (baseline undefined, or baseline === head)
+// was NOT pushed during this session — it is a fresh launch/clone/checkout and must OFFER,
+// never auto-start. Deriving continuation from the on-disk ack marker instead (the prior
+// implementation) wrongly treated ANY descendant-of-a-prior-ack head as continuation, so
+// every bare Pi start auto-spawned reviewers — the regression this restores to offering.
+export function reviewBaselineContinuation(
+  baseline: string | undefined,
+  head: string,
+  isAncestor: (ancestor: string, current: string) => boolean,
+): boolean {
+  if (!baseline || !head || baseline === head) return false;
+  return isAncestor(baseline, head);
+}
+
 // Npm package source strings a durable review lane should load as additionalExtensionPaths.
 // Graphify is a first-party LOCAL extension (graphify-native.ts), loaded directly by the lane
 // runner alongside codeflare-pi - it is not an npm package and never appears here.

sdd/spec/agents.md

@@ -1359,7 +1359,7 @@ None.
 
 **Acceptance Criteria:**
 
-1. Lifecycle reconciliation recovers enforced open PRs with unacknowledged heads: in-session head advances auto-start, while inherited heads are offered once and remain merge-blocking until user choice. <!-- @impl: preseed/agents/pi/extensions/review-enforcement.ts::reconcileOpenPrReview --> <!-- @impl: preseed/agents/pi/extensions/review-job-helpers.ts::shouldReconcileOpenPr --> <!-- @impl: preseed/agents/pi/extensions/review-job-helpers.ts::reconcileBoundaryAction -->
+1. Lifecycle reconciliation recovers enforced open PRs with unacknowledged heads using an in-memory per-session baseline (the head first observed this session): a head that advances beyond the baseline auto-starts, while an inherited head (baseline unset or unchanged — fresh launch/clone/checkout) is offered once and remains merge-blocking until user choice. <!-- @impl: preseed/agents/pi/extensions/review-enforcement.ts::reconcileOpenPrReview --> <!-- @impl: preseed/agents/pi/extensions/review-job-helpers.ts::shouldReconcileOpenPr --> <!-- @impl: preseed/agents/pi/extensions/review-job-helpers.ts::reconcileBoundaryAction --> <!-- @impl: preseed/agents/pi/extensions/review-job-helpers.ts::reviewBaselineContinuation --> <!-- @test: src/__tests__/lib/review-state.test.ts (reviewBaselineContinuation: bare launch offers, only an in-session advance autostarts -> AC1) -->
 2. Boundary-command and reconciliation paths call one shared routine, so reconciled windows match captured-command windows in lanes, review base, durable job, and audit trail. <!-- @impl: preseed/agents/pi/extensions/review-enforcement.ts::ensureReviewWindow -->
 3. Head resolution reviews local HEAD during metadata lag only when it is on the PR branch, descends from GitHub's PR head, and the remote-tracking ref contains it. <!-- @impl: preseed/agents/pi/extensions/review-enforcement.ts::resolveEnforcedHead --> <!-- @impl: preseed/agents/pi/extensions/review-helpers.ts::enforcedHeadDecision --> <!-- @test: src/__tests__/lib/review-trigger.test.ts (enforcedHeadDecision pushed-vs-unpushed table -> AC3) -->
 4. A boundary-shaped command that does not start a review appends a durable `boundary_candidate_ignored` audit event naming the gate reason, so a skipped review is always reconstructable from disk. <!-- @impl: preseed/agents/pi/extensions/review-enforcement.ts::onToolEnd --> <!-- @impl: preseed/agents/pi/extensions/review-job-helpers.ts::shouldReconcileOpenPr --> <!-- @impl: preseed/agents/pi/extensions/review-jobs.ts::appendReviewEvent --> <!-- @test: src/__tests__/lib/review-state.test.ts (REQ-AGENT-058 AC4: every suppressed reconcile gate names a distinct non-empty reason to stamp into boundary_candidate_ignored -> AC4) -->

sdd/spec/changes.md

@@ -2,6 +2,12 @@
 
 Semantic changes to the specification. Git history captures diffs; this file captures intent.
 
+## 2026-06-10
+
+- **Pi PR-boundary review OFFERS on a fresh launch again (bug-A regression fix)** ([REQ-AGENT-058](agents.md#req-agent-058-pr-boundary-review-reconciliation-and-missed-event-recovery) AC1 mechanism revised). The 2026-06-09 offer-once work derived `inSessionContinuation` from the on-disk ack (`isAncestor(lastAck, head)`), which is true on every fresh `pi` launch whose head descends from a prior ack — so reconciliation auto-started reviewers on launch instead of offering (user-reported: "offering worked yesterday you broke something today"; proven by `codeflare-review-events.jsonl` showing `trigger:"open-PR reconciliation (in-session continuation)"` at session start). Continuation is now decided from an **in-memory per-session baseline** (`reviewSessionBaselineHead`, the head first observed this session): the new pure helper `reviewBaselineContinuation(baseline, head, isAncestor)` returns true only when the head ADVANCED beyond the baseline (a genuine in-session push whose on-tool-end auto-start was dropped); a head present at launch (baseline unset or unchanged) offers. Process-scoped so a bare relaunch always re-baselines and offers. Mirrored into `~/.pi/agent/extensions/`. Tests: `review-state.test.ts` (`reviewBaselineContinuation`: bare launch offers, only an in-session advance autostarts). **Status:** REQ-AGENT-058 stays Implemented.
+- **`gh pr edit --base main|master` now arms PR-boundary review across all enforcement surfaces** ([REQ-AGENT-058](agents.md#req-agent-058-pr-boundary-review-reconciliation-and-missed-event-recovery) AC3; [REQ-AGENT-061](agents.md#req-agent-061-pr-boundary-review-idle-durable-reaper) AC3/AC4 split). A PR opened against a non-protected base (or via the web UI) and later retargeted onto `main`/`master` with `gh pr edit --base` is the same boundary `gh pr create --base main` establishes, applied after creation — previously it armed no review until the durable reconciliation catch-up. The Pi extension (`prBoundaryCommandBase = prCreateBoundaryBase || prEditBoundaryBase`, `isBoundaryWords` recognizing `gh pr edit` with a protected `--base`) and both Claude hooks (`enforce-review-spawn.sh` awk across Bash/`ctx_execute`/`ctx_batch_execute`; `git-push-review-reminder.sh` `pr-retarget` trigger taking the base from the command itself, not a stale `gh view`) now detect it; metadata-only `gh pr edit` (title/body/labels) and non-protected `--base develop` remain ignored. REQ-AGENT-061 AC3 split so off-turn finalization's "summary deferred, marker left unclaimed for the on-turn emit" is its own AC4. Tests: `review-trigger.test.ts` (`prBoundaryCommandBase`), `agent-seed-manifest.test.ts` (`isPrBoundaryCommand`/`cwdFromBoundaryCommand` for `gh pr edit`), `enforce-review-spawn.test.js` + `git-push-review-reminder.test.js` (real-hook block/directive across `--base`/`--base=`/`-B` forms). **Status:** REQ-AGENT-058 stays Implemented; REQ-AGENT-061 stays Planned.
+- **Reloaded enterprise config falls back to the first route as default** ([REQ-ENTERPRISE-012](enterprise-mode.md#req-enterprise-012-setup-configured-dynamic-route-catalog-and-access-group-list) AC6/AC7 split). On a setup re-run, `loadExistingConfig` left `defaultRouteName` empty when no default was stored, so a configured-but-default-less enterprise tenant reloaded with no default selected; it now resolves to `dynamicRoutes[0]` (reasoning off), matching the first-route-is-default rule. REQ-ENTERPRISE-012 AC6 split so the "Continue blocked until ≥1 route" UI gate (AC6) and the "`POST /api/setup/configure` rejects empty `dynamicRoutes` with 400" backend gate (AC7) are separate; [REQ-ENTERPRISE-006](enterprise-mode.md#req-enterprise-006-deploy-time-aig-secrets-and-enterprise_mode-var) AC7 added for the `enterprise`/`enterprise integration` deploy targets. Tests: `stores/setup.test.ts` (reload with `defaultRoute:null` → first route serialized as default). **Status:** REQ-ENTERPRISE-012 stays Implemented; REQ-ENTERPRISE-006 stays Planned.
+
 ## 2026-06-09
 
 - **Enterprise dynamic routes are now required (≥1), the first route added is the default, and the group/route chips get the accent border** ([REQ-ENTERPRISE-012](enterprise-mode.md#req-enterprise-012-setup-configured-dynamic-route-catalog-and-access-group-list) AC2 revised + AC6 added). Acceptance testing on the live integration tenant surfaced two issues: (1) the access-group and dynamic-route chips rendered without the blue accent border the admin chips have — the accent CSS lived in a modifier (`email-tag--admin`) applied only to admin chips; (2) the "Default Route" selector offered a misleading "(no default)" option, implying a request could reach no destination, even though the interceptor and container already resolve an unset default to the first catalog route. Fix: the accent modifier is generalized to `email-tag--accent` and applied to the admin, access-group, and dynamic-route chips (non-enterprise regular-user chips stay plain to preserve the admin distinction); the wizard now requires ≥1 dynamic route in enterprise mode (`ConfigureStep` blocks Continue; `POST /api/setup/configure` rejects empty/absent `dynamicRoutes` with `400` before any KV write) and the store auto-assigns the first route added as the default (re-points to the new first route when the default is removed), dropping the "(no default)" option so routing always resolves to a gateway route. The interceptor's empty-catalog passthrough is retained as a defensive fallback, no longer a reachable enterprise configuration. Tests: `setup.test.ts` (enterprise empty-routes → `400`, no partial KV write), `stores/setup.test.ts` (first-route auto-default + remove-reassign + first route serialized as the default), `ConfigureStep.test.tsx` (Continue gate + route select offers no empty option). A gating-hardening pass (per acceptance-test feedback that every non-review change stay behind enterprise mode) confirmed no non-enterprise leak across the epic: the `/prefill` enterprise extras are now gated on `isEnterpriseMode` so a non-enterprise response is byte-identical (`handlers.test.ts` non-enterprise omission, AC5), the container-router `defaultRoute`/`defaultReasoning` writes are nested under catalog presence so they travel as a unit with the catalog (no stray empty-string write without a catalog), and removing the default route resets its reasoning grade to off. **Status:** REQ-ENTERPRISE-012 stays Implemented (UI gate + backend `400` + store auto-default + non-enterprise prefill omission unit-tested).

src/__tests__/lib/review-state.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from 'vitest';
-import { computeReviewStateFrom, shouldReconcileOpenPr, reconcileBoundaryAction, type ComputeReviewStateInput, type OpenPrReconcileInput, type ReconcileBoundaryInput } from '../../../preseed/agents/pi/extensions/review-job-helpers';
+import { computeReviewStateFrom, shouldReconcileOpenPr, reconcileBoundaryAction, reviewBaselineContinuation, type ComputeReviewStateInput, type OpenPrReconcileInput, type ReconcileBoundaryInput } from '../../../preseed/agents/pi/extensions/review-job-helpers';
 
 /**
  * computeReviewStateFrom is the canonical review-state definition (review.md §17.2).
@@ -173,6 +173,41 @@ describe('reconcileBoundaryAction (REQ-AGENT-058 revised: autostart in-session,
   });
 });
 
+/**
+ * reviewBaselineContinuation feeds reconcileBoundaryAction's `inSessionContinuation` flag.
+ * It decides continuation from an IN-MEMORY per-session baseline (the head this Pi session
+ * first observed), NOT the on-disk ack. That distinction is the bug-A fix: keying off the
+ * on-disk ack made every fresh `pi` launch whose head descended from a prior ack report
+ * continuation → auto-start reviewers on launch (regression: "offering worked yesterday").
+ * Continuation is true ONLY when the head ADVANCED beyond the baseline during this session.
+ * These fail if a bare launch (baseline undefined or unchanged) is ever treated as
+ * continuation, or if a genuine in-session advance stops being treated as one.
+ */
+describe('reviewBaselineContinuation (bug-A fix: offer on launch, autostart only on in-session advance)', () => {
+  const descends = () => true;   // isAncestor stub: head descends from baseline
+  const unrelated = () => false; // isAncestor stub: head does NOT descend from baseline
+
+  it('returns false on the first reconcile of a session (baseline undefined → fresh launch offers)', () => {
+    expect(reviewBaselineContinuation(undefined, 'headsha', descends)).toBe(false);
+  });
+
+  it('returns false when the head is unchanged since the session baseline (relaunch on same head offers)', () => {
+    expect(reviewBaselineContinuation('samehead', 'samehead', descends)).toBe(false);
+  });
+
+  it('returns true when the head advanced beyond the baseline and descends from it (dropped in-session push)', () => {
+    expect(reviewBaselineContinuation('basehead', 'newhead', descends)).toBe(true);
+  });
+
+  it('returns false when the new head does not descend from the baseline (unrelated branch, not a continuation)', () => {
+    expect(reviewBaselineContinuation('basehead', 'newhead', unrelated)).toBe(false);
+  });
+
+  it('returns false for an empty head regardless of baseline', () => {
+    expect(reviewBaselineContinuation('basehead', '', descends)).toBe(false);
+  });
+});
+
 /**
  * REQ-AGENT-058 AC4: a boundary-shaped command that does not start a review appends a
  * durable `boundary_candidate_ignored` audit event NAMING the gate reason, so a skipped