gstack/test/skill-e2e-plan-ceo-finding-count.test.ts

/**
 * /plan-ceo-review per-finding AskUserQuestion count (periodic, paid, real-PTY).
 *
 * Asserts the load-bearing rule "One issue = one AskUserQuestion call" by
 * driving /plan-ceo-review against a 5-finding seeded plan and counting
 * distinct review-phase AUQs. Passes when count is in [N-1, N+2].
 *
 * Two tests in this file:
 *   - 5-finding distinct fixture: count band assertion + D19 review-report-at-bottom.
 *   - 2-finding paired control (D12 positive control): related findings still
 *     produce 2 distinct AUQs, not 1 batched, when the rule is honored.
 *
 * Tier: periodic. Each run drives Step 0 + 11 review sections end-to-end
 * (~25 min, ~$5/run). Sequential by default per plan §D15. See
 * test/helpers/claude-pty-runner.ts for runPlanSkillCounting internals.
 */

import { test } from 'bun:test';
import { describeE2ETier } from './helpers/e2e-gate';
import { isCeoCompletionHandoff } from './helpers/ceo-completion-handoff';
import { pickCeoCountQuestion } from './helpers/ceo-approach-pick';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import {
  runPlanSkillCounting,
  ceoStep0Boundary,
  ceoFirstReviewAUQ,
  assertReviewReportAtBottom,
  type AskUserQuestionFingerprint,
} from './helpers/claude-pty-runner';

/**
 * /plan-ceo-review's first AUQ asks "what scope?" with options like
 *   1. Branch diff vs main
 *   2. A specific plan file or design doc
 *   3. An idea you'll describe inline
 *   ...
 *   7. Skip interview and plan immediately
 *
 * The default pick (1) routes to "branch diff vs main" — the wrong target
 * for our seeded fixture (the agent would review the gstack PR itself,
 * recursively). Picking "Skip interview and plan immediately" bypasses
 * Step 0 and routes the agent to review the fixture request, which is
 * already present in its initial project context.
 */
function pickSkipInterview(fp: AskUserQuestionFingerprint): number {
  const skipOpt = fp.options.find((o) =>
    /skip\s+interview|plan\s+immediately/i.test(o.label),
  );
  if (skipOpt) return skipOpt.index;
  // Fallback: "describe inline" also routes to using our pasted plan.
  const inlineOpt = fp.options.find((o) =>
    /describe.*inline|inline.*idea/i.test(o.label),
  );
  if (inlineOpt) return inlineOpt.index;
  return 1;
}

const describeE2E = describeE2ETier('periodic');

const N_DISTINCT = 5;
const FLOOR_DISTINCT = N_DISTINCT - 1; // 4 (D11)
const CEILING_DISTINCT = N_DISTINCT + 2; // 7 (D11)

const N_PAIRED = 2;
const FLOOR_PAIRED = 2;
const CEILING_PAIRED = 4;

// Keep the five seeded defects distinct from already-satisfied surrounding
// contracts. Live controls correctly found extra ingress, missing-user,
// observability, and rollout gaps when those baseline facts were unspecified.
const planCeo5Findings = (planPath: string) => [
  `Please review this plan thoroughly in HOLD SCOPE mode. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`,
  'Proceed directly to the requested CEO review; skip the optional /office-hours prerequisite.',
  'Finish after this CEO review; I will handle subsequent reviews manually.',
  '',
  '# Plan: Payment Processing Integration',
  '',
  '## Existing contracts retained',
  'The approved motivation is to move payment orchestration out of the prior',
  'library-adapter handler into application-owned code while retaining the',
  'existing payment and receipt product behavior. The shared dispatcher remains',
  'available; the proposed bypass below is still an architectural choice to review.',
  'The existing ingress middleware verifies the Stripe signature against the',
  'raw request body and rejects invalid signatures before invoking handlers.',
  'The existing ingress forwards only `payment_intent.succeeded` events to',
  'this handler; other Stripe event types are acknowledged without invoking it.',
  'The existing payload adapter exposes `event.data.object.metadata.user_id`',
  'as `request.params.userId`. This params object is the parsed body-data map,',
  'not URL query/path parameters; all users share one webhook URL.',
  'The adapter acknowledges missing, nil, or empty user_id metadata with',
  'HTTP 200 and an event-correlated warning before invoking this handler.',
  'For every nonempty external string it performs no SQL-format validation.',
  'The adapter forwards that external string unchanged. It does not cast,',
  'escape, or SQL-sanitize it; a valid signature does not make it safe for SQL.',
  'User IDs are opaque TEXT values, including punctuation and Unicode. The',
  'lookup has no integer/UUID cast or ID-format restriction; every nonempty',
  'string is a valid identifier representation.',
  'An existing ingress ownership guard checks the PaymentIntent ID against',
  'its stored opaque user-ID binding before invoking the handler. A mismatch',
  'is acknowledged with HTTP 200 and an event-correlated warning. This is an',
  'identity comparison, not SQL-format validation; the adapter still forwards',
  'the original string unchanged.',
  'The existing webhook event guard deduplicates deliveries by Stripe event ID,',
  'and an existing per-user lock serializes payment updates.',
  'The event guard acquires the existing per-user lock before checking the',
  'committed completion marker, and rechecks after any lock wait. It holds',
  'that lock through the handler and completion bookkeeping; an overlapping',
  'completed duplicate does not invoke the handler.',
  'The new handler runs inside those unchanged guards; this plan does not',
  'replace signature verification, event deduplication, or update locking.',
  'The existing user update assigns payment_status=paid and the payment intent',
  'ID; it does not increment a balance or counter. Repeating the same payment',
  'intent assigns the same values, independently of the event-ID guard.',
  'The existing lookup-result guard acknowledges unknown/deleted users with',
  'HTTP 200, logs the event, and stops before user updates or email fan-out.',
  'The retained recipient-policy helper treats a nil or empty email address as',
  'skipped_missing_address: payment processing continues normally, and no mail',
  'client call is attempted. It persists an event/user/PaymentIntent-correlated',
  'skip record, emits a structured warning, and increments the existing counter.',
  'The existing notification runbook already covers that skip result: correct',
  'the account address, then retry only its recorded notification using the',
  'same PaymentIntent idempotency key. It never replays the payment for this case.',
  'That recipient policy does not catch failures from sends to nonempty addresses;',
  'the shared mail client still rethrows those exceptions to this handler.',
  'Account deletion uses the same per-user lock. The handler holds it from',
  'lookup through update and inline email, so deletion either precedes lookup',
  '(the existing unknown/deleted-user path) or follows the handler; it cannot',
  'remove the user between lookup and update.',
  'The ingress wrapper already logs event IDs, outcomes, and durations, with',
  'alerts for failed webhook processing. Those controls remain in place.',
  'The existing DB and mail clients attach the adapter user ID and event ID',
  'to outcome traces, including update success and email delivery success or',
  'failure. These shared clients rethrow exceptions unchanged; tracing does',
  'not rescue email errors or change the inline email call below.',
  'The shared mail client also publishes its delivery failure rate to the',
  'existing dashboard and tested on-call alert, including caught exceptions.',
  'The existing incident runbook uses the correlated DB and mail outcomes to',
  'distinguish committed payments from failed notifications. It directs on-call',
  'to check provider status and retry only the failed notification through the',
  'existing notification retry procedure, never replay the payment blindly.',
  'DB lookup/update exceptions propagate to that ingress wrapper, which logs',
  'the failure and returns HTTP 500 so Stripe retries the event. The existing',
  'event-ID dedup guard records completion only after the database transaction',
  'commits; failed or rolled-back database attempts remain retryable.',
  'The deployment already has a handler feature flag and a documented, tested',
  'rollback to the prior handler; this change uses that existing rollout path.',
  'That documented manual rollout checklist already requires a staging',
  'payment-event replay for this handler and verification of the user update,',
  'email delivery, and correlated outcome trace before enabling it broadly.',
  'This is manual deployment verification, not automated handler regression',
  'coverage; no new automated tests are planned in the Tests section below.',
  'The existing notification contract sends one payment receipt per PaymentIntent,',
  'including a summary of the user orders. With zero orders it still sends one',
  'receipt with an empty order summary; the order loop is data loading, never',
  'one email or payment update per order. These product semantics are retained.',
  'The shared mail client already derives a provider idempotency key from that',
  'PaymentIntent ID. The provider durably suppresses duplicate successful sends',
  'for the same key across process crashes, webhook retries, and manual retries.',
  'Before rethrowing a failed or timed-out send, that client durably records the',
  'notification attempt for the existing retry procedure. The dashboard and',
  'on-call alert already monitor failed-notification age and backlog after an',
  'outage clears, as well as failure rate; the runbook retries those records.',
  'The existing mail-client deadline is one second, enforced by cancellation',
  'of the provider request with no inline retries. It raises MailTimeout on',
  'expiry. The retained DB/ingress deadlines bound their combined work to two',
  'seconds, leaving headroom inside the existing ten-second webhook deadline.',
  'Neither deadlines nor retry records catch the mail exception for this handler;',
  'the shared client still rethrows it to the inline caller described below.',
  'Every existing event-correlated outcome trace includes the active handler',
  'identity (prior or new), so rollout attribution is already available.',
  'If a separate handler class is retained, its already-approved name is',
  '`Webhooks::StripePaymentWebhookHandler` in the application-owned namespace,',
  'never the Stripe library namespace. This naming choice is settled; whether',
  'to add a separate implementation or reuse WebhookDispatcher remains open.',
  '',
  '## Architecture',
  "We're adding a new `StripePaymentWebhookHandler` class that will handle Stripe webhooks.",
  'This bypasses the existing `WebhookDispatcher` module — we want a clean',
  'namespace separation.',
  '',
  '## Database access',
  'The new endpoint reads `request.params.userId` directly into a raw SQL',
  'fragment for the lookup query.',
  '',
  '## Webhook fan-out',
  'On payment success we update the user record AND fire a notification email.',
  'Both happen inline; no error handling on the email leg.',
  '',
  '## Tests',
  "None planned. We'll rely on the existing integration suite catching regressions.",
  '',
  '## Performance',
  'Each webhook lookup hits the database for the user, then fetches each',
  'order in a loop.',
].join('\n');

const planCeo2PairedFindings = (planPath: string) => [
  `Please review this plan thoroughly in HOLD SCOPE mode. As you go, write your plan-mode plan to ${planPath} (use Edit/Write to that exact path).`,
  'Proceed directly to the requested CEO review; skip the optional /office-hours prerequisite.',
  'Finish after this CEO review; I will handle subsequent reviews manually.',
  '',
  '# Plan: Payment Processing — Test Coverage',
  '',
  '## Existing coverage and test infrastructure retained',
  'This changes unit tests only; processPayment() production behavior stays as-is.',
  'The Stripe adapter suite already covers network timeouts, card declines (402),',
  'rate limits (429), and recovery when an initial 502 is followed by a successful',
  'charge. Receipt-builder failure behavior has its own passing regression tests.',
  'The payment test factory explicitly configures max_retries=1 and exposes the',
  'Stripe mock call history. Its injected virtual sleeper records backoff without',
  'real delays, so an exhausted 502 operation makes exactly two charge attempts.',
  'These existing helpers and regression suites remain in use for this change.',
  '',
  '## Existing behavior retained',
  'A successful charge returns a receipt with chargeId copied from Stripe,',
  'amountCents equal to the requested integer amount, and currency equal to',
  'the requested currency. For a 1000-cent USD charge returning id ch_paid,',
  'the receipt is { chargeId: "ch_paid", amountCents: 1000, currency: "USD" }.',
  'On repeated 502 responses, max_retries=1 means two total charge attempts',
  'separated by one recorded 100 ms backoff, followed by PaymentUnavailable.',
  'These contracts are already implemented; this plan adds their unit coverage.',
  '',
  '## Proposed tests',
  'Add two tests in the existing processPayment suite using its current factory,',
  'Stripe mock and virtual sleeper. Other tests and production code stay as-is.',
  '',
  '1. Successful charge: arrange the Stripe mock to return id ch_paid, call',
  '   processPayment with amountCents=1000 and currency=USD, and assert only',
  '   that the returned receipt is truthy. This is the complete planned assertion.',
  '2. Repeated 502: arrange two consecutive Stripe 502 responses, call',
  '   processPayment, and assert only that it rejects with PaymentUnavailable.',
  '   No assertion about the mock call history or virtual sleeper record',
  '   is planned for this test.',
].join('\n');

describeE2E('/plan-ceo-review per-finding AskUserQuestion count (periodic)', () => {
  test(
    `5-finding plan emits ${FLOOR_DISTINCT}-${CEILING_DISTINCT} review-phase AskUserQuestions`,
    async () => {
      // Per-run artifact dir: a hardcoded shared /tmp path collides under
      // --retry, EVALS_JOBS>1, or concurrent worktrees (a sibling's finally-
      // rmSync deletes this run's artifact → spurious D19 failure).
      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e2e-plan-ceo-'));
      const planPath = path.join(tmpDir, 'gstack-test-plan-ceo.md');

      try {
        const obs = await runPlanSkillCounting({
          skillName: 'plan-ceo-review',
          slashCommand: '/plan-ceo-review',
          followUpPrompt: planCeo5Findings(planPath),
          expectedPlanPath: planPath,
          isLastStep0AUQ: ceoStep0Boundary,
          isFirstReviewAUQ: ceoFirstReviewAUQ,
          isCompletionHandoffAUQ: isCeoCompletionHandoff,
          pickAUQ: pickCeoCountQuestion,
          reviewCountCeiling: CEILING_DISTINCT + 1, // hard cap above assertion ceiling
          firstAUQPick: pickSkipInterview, // bypass scope-selection, route to review
          timeoutMs: 1_500_000, // 25 min
          env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' },
        });

        if (!['plan_ready', 'completion_summary', 'ceiling_reached'].includes(obs.outcome)) {
          throw new Error(
            `plan-ceo-review finding-count FAILED: outcome=${obs.outcome}\n` +
              `step0=${obs.step0Count} review=${obs.reviewCount} elapsed=${obs.elapsedMs}ms\n` +
              `fingerprints (last 8):\n` +
              obs.fingerprints
                .slice(-8)
                .map(
                  (f, i) =>
                    `  ${i}. preReview=${f.preReview} sig=${f.signature.slice(0, 12)} prompt="${f.promptSnippet.slice(0, 60)}"`,
                )
                .join('\n') +
              `\n--- evidence (last 3KB) ---\n${obs.evidence}`,
          );
        }
        if (obs.reviewCount < FLOOR_DISTINCT) {
          throw new Error(
            `BAND FAIL (below floor): reviewCount=${obs.reviewCount} < FLOOR=${FLOOR_DISTINCT}.\n` +
              `Likely batching regression — agent collapsed multiple findings into fewer questions.\n` +
              `Fingerprints (review-phase only):\n` +
              obs.fingerprints
                .filter((f) => !f.preReview && !f.administrative)
                .map((f) => `  - "${f.promptSnippet.slice(0, 80)}"`)
                .join('\n'),
          );
        }
        if (obs.reviewCount > CEILING_DISTINCT) {
          throw new Error(
            `BAND FAIL (above ceiling): reviewCount=${obs.reviewCount} > CEILING=${CEILING_DISTINCT}.\n` +
              `Captured observation:\n${JSON.stringify(obs, null, 2)}`,
          );
        }

        // D19: review report at bottom of plan file.
        if (!fs.existsSync(planPath)) {
          throw new Error(
            `D19 FAIL: agent did not produce expected plan file at ${planPath}.\n` +
              `Either the agent ignored the path instruction in the follow-up prompt, or\n` +
              `the helper exited before the agent wrote the file. ` +
              `outcome=${obs.outcome} review=${obs.reviewCount}`,
          );
        }
        const planContent = fs.readFileSync(planPath, 'utf-8');
        const verdict = assertReviewReportAtBottom(planContent);
        if (!verdict.ok) {
          throw new Error(
            `D19 FAIL: plan file at ${planPath} ${verdict.reason}\n` +
              (verdict.trailingHeadings
                ? `Trailing headings: ${verdict.trailingHeadings.join(' | ')}\n`
                : '') +
              `--- plan content (last 1KB) ---\n${planContent.slice(-1024)}`,
          );
        }
      } finally {
        try {
          fs.rmSync(tmpDir, { recursive: true, force: true });
        } catch {
          /* best-effort */
        }
      }
    },
    1_500_000 /* physical ceiling: the 25-min CI job + 1800s shard wall cap what can actually execute */,
  );

  test(
    `paired-finding positive control: ${N_PAIRED} related findings produce ${FLOOR_PAIRED}-${CEILING_PAIRED} AskUserQuestions`,
    async () => {
      // Per-run artifact dir — see the distinct-findings test above.
      const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e2e-plan-ceo-paired-'));
      const planPath = path.join(tmpDir, 'gstack-test-plan-ceo-paired.md');

      try {
        const obs = await runPlanSkillCounting({
          skillName: 'plan-ceo-review',
          slashCommand: '/plan-ceo-review',
          followUpPrompt: planCeo2PairedFindings(planPath),
          expectedPlanPath: planPath,
          isLastStep0AUQ: ceoStep0Boundary,
          isFirstReviewAUQ: ceoFirstReviewAUQ,
          isCompletionHandoffAUQ: isCeoCompletionHandoff,
          pickAUQ: pickCeoCountQuestion,
          reviewCountCeiling: CEILING_PAIRED + 1,
          timeoutMs: 1_500_000,
          env: { QUESTION_TUNING: 'false', EXPLAIN_LEVEL: 'default' },
        });

        if (!['plan_ready', 'completion_summary', 'ceiling_reached'].includes(obs.outcome)) {
          throw new Error(
            `paired-finding control FAILED: outcome=${obs.outcome}\n` +
              `step0=${obs.step0Count} review=${obs.reviewCount}\n` +
              `--- evidence (last 3KB) ---\n${obs.evidence}`,
          );
        }
        if (obs.reviewCount < FLOOR_PAIRED) {
          throw new Error(
            `PAIRED CONTROL FAIL: reviewCount=${obs.reviewCount} < FLOOR=${FLOOR_PAIRED}.\n` +
              `Two deliberately related findings were batched into <2 questions — the rule failed under D12.\n` +
              `Review-phase fingerprints:\n` +
              obs.fingerprints
                .filter((f) => !f.preReview && !f.administrative)
                .map((f) => `  - "${f.promptSnippet.slice(0, 80)}"`)
                .join('\n'),
          );
        }
        if (obs.reviewCount > CEILING_PAIRED) {
          throw new Error(
            `PAIRED CONTROL FAIL: reviewCount=${obs.reviewCount} > CEILING=${CEILING_PAIRED} (over-asking on a 2-finding fixture).\n` +
              `Captured observation:\n${JSON.stringify(obs, null, 2)}`,
          );
        }
      } finally {
        try {
          fs.rmSync(tmpDir, { recursive: true, force: true });
        } catch {
          /* best-effort */
        }
      }
    },
    1_500_000 /* physical ceiling: the 25-min CI job + 1800s shard wall cap what can actually execute */,
  );
});