Define provider-neutral planning pull request foundations

package.json

@@ -54,7 +54,7 @@
     "format:check": "prettier --check .",
     "prepare": "husky",
     "pretest": "npm run check:dependencies",
-    "test": "node --test \"bin/*.test.ts\" \"src/**/*.test.ts\" \"test-support/*.test.ts\" \"scripts/*.test.mjs\"",
+    "test": "npm run check:contract-tests && node --test \"bin/*.test.ts\" \"src/**/*.test.ts\" \"test-support/*.test.ts\" \"scripts/*.test.mjs\"",
     "test:coverage": "node --test --experimental-test-coverage --test-coverage-include='bin/**/*.ts' --test-coverage-include='src/**/*.ts' --test-coverage-include='test-support/**/*.ts' --test-coverage-exclude='**/*.test.ts' \"bin/*.test.ts\" \"src/**/*.test.ts\" \"test-support/*.test.ts\"",
     "check:architecture": "depcruise --config dependency-cruiser.config.mjs bin src extensions",
     "check:coverage": "c8 node --test \"bin/*.test.ts\" \"src/**/*.test.ts\" \"test-support/*.test.ts\"",
@@ -65,6 +65,7 @@
     "test:mutation": "node --test src/policy/todo-statuses.test.ts quality/property/todo-statuses.property.test.ts",
     "check:static": "eslint \"{bin,src,test-support,quality}/**/*.ts\" --max-warnings=0",
     "check:types": "tsc -p tsconfig.quality.json",
+    "check:contract-tests": "tsc -p tsconfig.contract-tests.json",
     "test:cli": "node --test bin/*.test.ts src/cli/main.test.ts",
     "test:triage": "node --test src/cli/commands/triage/*.test.ts",
     "test:run-once": "node --test src/cli/commands/run-once/*.test.ts",

src/git/planning-workspaces.test.ts

@@ -0,0 +1,128 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import {
+  PlanningWorkspaceConflictError,
+  type PlanningWorkspaceIdentity,
+  type PlanningWorkspaceLifecycle,
+  type PlanningWorkspaceSnapshot,
+  type PreparedPlanningWorkspace,
+} from "./planning-workspaces.ts";
+
+const identity: PlanningWorkspaceIdentity = {
+  branch: "agent/issue-184-foundations-spec",
+  worktreePath: ".worktrees/patchmill-issue-184-foundations-spec",
+};
+
+class FakePlanningWorkspace implements PlanningWorkspaceLifecycle {
+  readonly events: string[] = [];
+  private snapshot: PlanningWorkspaceSnapshot = {
+    state: "missing",
+    identity,
+  };
+
+  async prepare(input: {
+    identity: PlanningWorkspaceIdentity;
+    remote: string;
+    baseBranch: string;
+    resume?: { baseSha: string; headSha: string };
+  }): Promise<PreparedPlanningWorkspace> {
+    this.events.push("prepare");
+    const headSha = input.resume?.headSha ?? "head-1";
+    const snapshot: Extract<PlanningWorkspaceSnapshot, { state: "ready" }> = {
+      state: "ready",
+      identity: input.identity,
+      headSha,
+      clean: true,
+    };
+    this.snapshot = snapshot;
+    return {
+      created: input.resume === undefined,
+      remote: input.remote,
+      baseBranch: input.baseBranch,
+      baseSha: input.resume?.baseSha ?? "base-1",
+      snapshot,
+    };
+  }
+
+  async inspect(
+    _identity: PlanningWorkspaceIdentity,
+  ): Promise<PlanningWorkspaceSnapshot> {
+    this.events.push("inspect");
+    return this.snapshot;
+  }
+
+  async removeWorktree(
+    _identity: PlanningWorkspaceIdentity,
+  ): Promise<Extract<PlanningWorkspaceSnapshot, { state: "branch-only" }>> {
+    this.events.push("remove-worktree");
+    if (this.snapshot.state !== "ready") {
+      throw new PlanningWorkspaceConflictError(
+        "branch-owned-by-other-worktree",
+        identity,
+      );
+    }
+    if (!this.snapshot.clean) {
+      throw new PlanningWorkspaceConflictError("dirty-worktree", identity);
+    }
+    this.snapshot = {
+      state: "branch-only",
+      identity,
+      headSha: this.snapshot.headSha,
+    };
+    return this.snapshot;
+  }
+
+  async removeBranch(input: {
+    identity: PlanningWorkspaceIdentity;
+    pushedHeadSha: string;
+  }): Promise<Extract<PlanningWorkspaceSnapshot, { state: "missing" }>> {
+    this.events.push("remove-branch");
+    if (
+      this.snapshot.state !== "branch-only" ||
+      this.snapshot.headSha !== input.pushedHeadSha
+    ) {
+      throw new PlanningWorkspaceConflictError("head-not-pushed", identity);
+    }
+    this.snapshot = { state: "missing", identity: input.identity };
+    return this.snapshot;
+  }
+}
+
+test("a coordinator can use the workspace lifecycle without Git commands", async () => {
+  const workspace = new FakePlanningWorkspace();
+  const prepared = await workspace.prepare({
+    identity,
+    remote: "origin",
+    baseBranch: "main",
+  });
+
+  assert.equal(prepared.created, true);
+  assert.equal(prepared.baseSha, "base-1");
+  assert.deepEqual(await workspace.inspect(identity), prepared.snapshot);
+
+  const branchOnly = await workspace.removeWorktree(identity);
+  assert.equal(branchOnly.state, "branch-only");
+
+  const missing = await workspace.removeBranch({
+    identity,
+    pushedHeadSha: "head-1",
+  });
+  assert.equal(missing.state, "missing");
+  assert.deepEqual(workspace.events, [
+    "prepare",
+    "inspect",
+    "remove-worktree",
+    "remove-branch",
+  ]);
+});
+
+test("workspace conflict error keeps the reason and identity", () => {
+  const error = new PlanningWorkspaceConflictError(
+    "unregistered-path",
+    identity,
+  );
+
+  assert.equal(error.name, "PlanningWorkspaceConflictError");
+  assert.equal(error.reason, "unregistered-path");
+  assert.deepEqual(error.identity, identity);
+});

src/git/planning-workspaces.ts

@@ -0,0 +1,79 @@
+export type PlanningWorkspaceIdentity = {
+  readonly branch: string;
+  readonly worktreePath: string;
+};
+
+export type PlanningWorkspaceSnapshot =
+  | {
+      readonly state: "missing";
+      readonly identity: PlanningWorkspaceIdentity;
+    }
+  | {
+      readonly state: "branch-only";
+      readonly identity: PlanningWorkspaceIdentity;
+      readonly headSha: string;
+    }
+  | {
+      readonly state: "ready";
+      readonly identity: PlanningWorkspaceIdentity;
+      readonly headSha: string;
+      readonly clean: boolean;
+    };
+
+export type PreparedPlanningWorkspace = {
+  readonly created: boolean;
+  readonly remote: string;
+  readonly baseBranch: string;
+  readonly baseSha: string;
+  readonly snapshot: Extract<PlanningWorkspaceSnapshot, { state: "ready" }>;
+};
+
+export type PlanningWorkspaceConflictReason =
+  | "path-owned-by-other-branch"
+  | "branch-owned-by-other-worktree"
+  | "unregistered-path"
+  | "detached-worktree"
+  | "base-sha-mismatch"
+  | "head-sha-mismatch"
+  | "dirty-worktree"
+  | "head-not-pushed";
+
+export class PlanningWorkspaceConflictError extends Error {
+  readonly reason: PlanningWorkspaceConflictReason;
+  readonly identity: PlanningWorkspaceIdentity;
+
+  constructor(
+    reason: PlanningWorkspaceConflictReason,
+    identity: PlanningWorkspaceIdentity,
+  ) {
+    super(`Planning workspace is unsafe: ${reason}`);
+    this.name = "PlanningWorkspaceConflictError";
+    this.reason = reason;
+    this.identity = identity;
+  }
+}
+
+export interface PlanningWorkspaceLifecycle {
+  prepare(input: {
+    identity: PlanningWorkspaceIdentity;
+    remote: string;
+    baseBranch: string;
+    resume?: {
+      baseSha: string;
+      headSha: string;
+    };
+  }): Promise<PreparedPlanningWorkspace>;
+
+  inspect(
+    identity: PlanningWorkspaceIdentity,
+  ): Promise<PlanningWorkspaceSnapshot>;
+
+  removeWorktree(
+    identity: PlanningWorkspaceIdentity,
+  ): Promise<Extract<PlanningWorkspaceSnapshot, { state: "branch-only" }>>;
+
+  removeBranch(input: {
+    identity: PlanningWorkspaceIdentity;
+    pushedHeadSha: string;
+  }): Promise<Extract<PlanningWorkspaceSnapshot, { state: "missing" }>>;
+}

src/host/pull-requests.test.ts

@@ -0,0 +1,161 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import {
+  IncompletePullRequestSearchError,
+  PullRequestIdentityError,
+  PullRequestNotFoundError,
+  sameRepositoryIdentity,
+  type CreatePullRequestInput,
+  type FindPullRequestsQuery,
+  type PullRequestHost,
+  type PullRequestReference,
+  type PullRequestSummary,
+  type RepositoryIdentity,
+} from "./pull-requests.ts";
+
+const githubRepository: RepositoryIdentity = {
+  provider: "github-gh",
+  host: "github.com",
+  owner: "rochecompaan",
+  repository: "patchmill",
+};
+
+test("repository identity comparison normalizes host, owner, and repository case", () => {
+  assert.equal(
+    sameRepositoryIdentity(githubRepository, {
+      provider: "github-gh",
+      host: "GITHUB.COM",
+      owner: "RocheCompaan",
+      repository: "Patchmill",
+    }),
+    true,
+  );
+});
+
+test("repository identity comparison requires the same provider", () => {
+  assert.equal(
+    sameRepositoryIdentity(githubRepository, {
+      ...githubRepository,
+      provider: "forgejo-tea",
+    }),
+    false,
+  );
+});
+
+test("incomplete search error keeps the exact query and provider limit", () => {
+  const query: FindPullRequestsQuery = {
+    targetRepository: githubRepository,
+    baseBranch: "main",
+    headRepository: githubRepository,
+    headBranch: "agent/issue-184-foundations-spec",
+  };
+  const error = new IncompletePullRequestSearchError(query, 100);
+
+  assert.equal(error.name, "IncompletePullRequestSearchError");
+  assert.deepEqual(error.query, query);
+  assert.equal(error.limit, 100);
+});
+
+test("not found error keeps the exact pull request reference", () => {
+  const reference = {
+    targetRepository: githubRepository,
+    number: 404,
+  };
+  const error = new PullRequestNotFoundError(reference);
+
+  assert.equal(error.name, "PullRequestNotFoundError");
+  assert.deepEqual(error.reference, reference);
+});
+
+test("identity error keeps expected and partial actual identity", () => {
+  const error = new PullRequestIdentityError("repository mismatch", {
+    expected: githubRepository,
+    actual: { host: "forge.example.test" },
+  });
+
+  assert.equal(error.name, "PullRequestIdentityError");
+  assert.equal(error.reason, "repository mismatch");
+  assert.deepEqual(error.expected, githubRepository);
+  assert.deepEqual(error.actual, { host: "forge.example.test" });
+});
+
+const openPullRequest = {
+  number: 12,
+  url: "https://github.com/rochecompaan/patchmill/pull/12",
+  status: "open",
+  targetRepository: githubRepository,
+  baseBranch: "main",
+  headRepository: githubRepository,
+  headBranch: "agent/issue-184-foundations-spec",
+  headSha: "abc123",
+  body: "Refs #184",
+} satisfies PullRequestSummary;
+
+const mergedPullRequest = {
+  ...openPullRequest,
+  status: "merged",
+  mergeCommit: "def456",
+} satisfies PullRequestSummary;
+
+const closedPullRequest = {
+  ...openPullRequest,
+  status: "closed-unmerged",
+} satisfies PullRequestSummary;
+
+// @ts-expect-error A merged pull request always has a merge commit.
+void ({ ...openPullRequest, status: "merged" } satisfies PullRequestSummary);
+void ({
+  ...openPullRequest,
+  mergeCommit: "def456",
+  // @ts-expect-error An open pull request never has a merge commit.
+} satisfies PullRequestSummary);
+void ({
+  ...openPullRequest,
+  status: "closed-unmerged",
+  mergeCommit: "def456",
+  // @ts-expect-error A closed-unmerged pull request never has a merge commit.
+} satisfies PullRequestSummary);
+
+test("pull request summary status preserves merge commit invariants", () => {
+  assert.equal(mergedPullRequest.mergeCommit, "def456");
+  assert.equal("mergeCommit" in openPullRequest, false);
+  assert.equal("mergeCommit" in closedPullRequest, false);
+});
+
+function fakePullRequestHost(summary: PullRequestSummary): PullRequestHost {
+  let body = summary.body;
+  return {
+    id: "github-gh",
+    resolveTargetRepositoryIdentity: async () => githubRepository,
+    resolveRemoteRepositoryIdentity: async () => githubRepository,
+    createPullRequest: async (_input: CreatePullRequestInput) => summary,
+    findPullRequests: async (_query: FindPullRequestsQuery) => [summary],
+    getPullRequest: async (_reference: PullRequestReference) => summary,
+    readPullRequestBody: async (_reference: PullRequestReference) => body,
+    updatePullRequestBody: async (
+      _reference: PullRequestReference,
+      nextBody: string,
+    ) => {
+      body = nextBody;
+    },
+  };
+}
+
+test("a fake host satisfies the same contract as production adapters", async () => {
+  const host = fakePullRequestHost(openPullRequest);
+  const reference = {
+    targetRepository: githubRepository,
+    number: openPullRequest.number,
+  };
+  const query: FindPullRequestsQuery = {
+    targetRepository: githubRepository,
+    baseBranch: "main",
+    headRepository: githubRepository,
+    headBranch: openPullRequest.headBranch,
+  };
+
+  assert.deepEqual(await host.findPullRequests(query), [openPullRequest]);
+  assert.deepEqual(await host.getPullRequest(reference), openPullRequest);
+  await host.updatePullRequestBody(reference, "updated body");
+  assert.equal(await host.readPullRequestBody(reference), "updated body");
+});