[Bugfix #710] Skip E2E tests that depend on removed porch run orchestrator

[waleedkadous]

Fixes #710

Root Cause

The four affected E2E scenario files (single-phase, happy-path, feedback-loop, benchmark) all spawn porch run or porch run --single-phase via the helpers in runner.ts:

• runPorchUntilGate, runPorchWithAutoApprove → porch run
• runPorchSinglePhase → porch run --single-phase
• runPorchOnce → porch run --once

porch run was removed in spec 0095 (commit ed2012ae, "[Spec 0095][Phase: phase_2] refactor: Remove porch orchestrator"). The CLI now responds with:

Error: 'porch run' has been removed. Use 'porch next <id>' instead.

…and exits with code 1 before writing anything to stdout. The runner's gate / __PORCH_RESULT__ regexes therefore never match, singlePhaseResult stays null, and hitGate stays null — which is exactly the failure shape described in the issue.

Fix

The orchestration loop these tests describe was deliberately moved out of porch — the parent agent now executes tasks emitted by porch next, so an "orchestrator-style" E2E test no longer applies. Reviving the coverage requires a planner-aware harness and belongs to its own spec.

This PR:

• Marks the four affected scenario describes as describe.skip with a clear annotation pointing to issue E2E: 12 porch single-phase / happy-path failures (singlePhaseResult is null) #710 and spec 0095.
• Adds __tests__/run-removed.test.ts, a regression test that pins the migration message so anyone (including the previously-broken runner) who still invokes porch run keeps getting a clear pointer to porch next. The test invokes cli() directly (mocking console.error + process.exit) so it does not depend on dist/ — same pattern as bugfix-711-pending-and-version.test.ts.

Test Plan

• All 277 porch unit tests pass (pnpm test src/commands/porch/__tests__)
• New regression test passes locally (and in CI without a prior build, after the cli()-rewrite fix)
• E2E scenario tests now report as skipped (vitest run --config vitest.e2e.config.ts on each affected file shows 0 failed, all skipped)
• Net diff: 110 LOC (well under 300)

CMAP Review

Three-way review run with --protocol bugfix --type pr:

Reviewer	Verdict	Confidence	Notes
Gemini	APPROVE	HIGH	"Correctly skips obsolete E2E tests relying on the deleted 'porch run' orchestrator and adds a robust regression test."
Codex	REQUEST_CHANGES	HIGH	Flagged stale in-progress status.yaml and missing CMAP section in PR body. Both addressed.
Claude	COMMENT	HIGH	Flagged the regression test's dist/ dependency via spawnSync. Addressed by switching to direct cli() invocation.

All blocking issues addressed:

• status.yaml advanced from investigate → fix via porch done with --pr 715 --branch builder/bugfix-710 (recorded in pr_history)
• Regression test rewritten to call cli() directly so it works without dist/
• This PR body updated with CMAP results

[waleedkadous]

Architect review — CI failure needs fixing before merge

CI Unit Tests job fails on run-removed.test.ts:

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../dist/commands/porch/index.js'
imported from .../bin/porch.js
    at finalizeResolution (node:internal/modules/esm/resolve:283:11)

The regression test spawns node bin/porch.js run some-id as a subprocess. bin/porch.js imports from dist/, but the unit test job doesn't build first (only test:e2e does pnpm build && vitest run). Locally it passes because dist is already built — CI starts clean.

Fix: rewrite the test to call cli() directly instead of via subprocess. The bugfix-711 PR (#714) does this for porch --version:

import { cli } from '../index.js';

it('exits 1 with a migration message pointing to porch next', async () => {
  const stderr: string[] = [];
  const spy = vi.spyOn(console, 'error').mockImplementation((...a) => {
    stderr.push(a.map(String).join(' '));
  });
  // Note: cli() throws or process.exits — wrap accordingly
  await expect(cli(['run', 'some-id'])).rejects.toThrow(); // or check process.exit mock
  expect(stderr.join('\n')).toContain("'porch run' has been removed");
  expect(stderr.join('\n')).toContain('porch next');
  spy.mockRestore();
});

(Adjust to the actual exit mechanism — looking at line 929 of index.ts, it logs to console.error and presumably process.exit(1)s. You may need to mock process.exit too.)

The rest of the PR (4 describe.skips with clear comments + the migration message regression test) is correctly scoped. Please fix the test approach and CI should go green.

— Architect