diff --git a/docs/plans/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations.md b/docs/plans/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations.md new file mode 100644 index 00000000..1ad30205 --- /dev/null +++ b/docs/plans/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations.md @@ -0,0 +1,1247 @@ +# Issue 184 Provider-Neutral Planning Pull Request Foundations Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add stable provider-neutral contracts and pure planning rules for +later planning pull request work. + +**Architecture:** Add one host contract module, one pure workflow module, and +one Git workspace contract module. Keep the current host factory and run-once +pipeline unchanged. + +**Tech Stack:** TypeScript, Node.js 24, NodeNext modules, `node:test`, strict +TypeScript checks, ESLint, Prettier, and dependency-cruiser. + +**Spec:** +`docs/specs/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations-design.md` + +## Global Constraints + +- Treat the issue #180 workflow decisions in the spec as fixed constraints. +- Use `RepositoryIdentity`, not `CanonicalRepositoryIdentity`. +- Keep `PLANNING_PR_WORKFLOW_VERSION` equal to `planning-pr-v1`. +- Keep the marker format exact: + ``. +- A successful pull request search means that the search is exhaustive. +- Keep incomplete search, proven absence, and identity mismatch as distinct + errors. +- Keep the new `PullRequestHost` separate from `RunOnceHostProvider`. +- Do not modify the current host factory, provider adapters, pipeline, run + state, labels, or publication code. +- Do not add production methods that throw `not implemented` errors. +- Do not implement Git commands in this issue. +- Keep the host and workspace fake adapters inside their colocated test files. +- Keep each production module below 200 meaningful lines. +- Do not add dependencies. +- Existing runtime and CLI behavior must stay unchanged. + +--- + +## File and Module Map + +### New production modules + +- `src/host/pull-requests.ts` owns repository identity, normalized pull request + contracts, host search semantics, and host contract errors. +- `src/workflow/planning-pull-requests.ts` owns phase derivation, phase + workspace names, markers, planning titles, and planning bodies. +- `src/git/planning-workspaces.ts` owns local workspace snapshots, lifecycle + operations, and workspace conflict errors. + +### New test modules + +- `src/host/pull-requests.test.ts` protects host contract behavior and contains + one fake host adapter. +- `src/workflow/planning-pull-requests.test.ts` protects all pure workflow + behavior. +- `src/git/planning-workspaces.test.ts` protects workspace contract behavior and + contains one fake workspace adapter. + +No existing source file changes in this issue. + +--- + +### Task 1: Define the provider-neutral pull request host contract + +**Files:** + +- Create: `src/host/pull-requests.ts` +- Create: `src/host/pull-requests.test.ts` + +**Interfaces:** + +- Consumes: `PatchmillHostProviderId` from `src/config/types.ts`. +- Produces: `RepositoryIdentity`, `PullRequestStatus`, `PullRequestReference`, + `PullRequestSummary`, `CreatePullRequestInput`, and `FindPullRequestsQuery`. +- Produces: `PullRequestHost` for later GitHub, Forgejo, and coordinator + adapters. +- Produces: `sameRepositoryIdentity` for all repository identity comparisons. +- Produces: `IncompletePullRequestSearchError`, `PullRequestNotFoundError`, and + `PullRequestIdentityError`. +- A `merged` summary requires `mergeCommit`. Other statuses prohibit it. +- A successful `findPullRequests` result is exhaustive by contract. + +- [ ] **Step 1: Write the repository identity and error tests** + +Create `src/host/pull-requests.test.ts` with these initial tests: + +```ts +import assert from "node:assert/strict"; +import test from "node:test"; +import { + IncompletePullRequestSearchError, + PullRequestIdentityError, + PullRequestNotFoundError, + sameRepositoryIdentity, + type FindPullRequestsQuery, + 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" }); +}); +``` + +- [ ] **Step 2: Add a fake host contract test** + +Add these imports and fixtures to the same test file: + +```ts +import type { + CreatePullRequestInput, + PullRequestHost, + PullRequestReference, + PullRequestSummary, +} from "./pull-requests.ts"; + +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; + +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"); +}); +``` + +This test adapter is local to this test file. Do not create a shared test +helper. + +- [ ] **Step 3: Run the focused test to prove RED** + +Run: + +```sh +node --test src/host/pull-requests.test.ts +``` + +Expected: FAIL because `src/host/pull-requests.ts` does not exist. + +- [ ] **Step 4: Implement the complete host contract module** + +Create `src/host/pull-requests.ts` with this public surface and implementation: + +```ts +import type { PatchmillHostProviderId } from "../config/types.ts"; + +export type RepositoryIdentity = { + provider: PatchmillHostProviderId; + host: string; + owner: string; + repository: string; +}; + +export type PullRequestStatus = "open" | "merged" | "closed-unmerged"; + +export type PullRequestReference = { + targetRepository: RepositoryIdentity; + number: number; +}; + +type PullRequestSummaryIdentity = { + number: number; + url: string; + targetRepository: RepositoryIdentity; + baseBranch: string; + headRepository: RepositoryIdentity; + headBranch: string; + headSha: string; + body: string; +}; + +export type PullRequestSummary = PullRequestSummaryIdentity & + ( + | { status: "open"; mergeCommit?: undefined } + | { status: "merged"; mergeCommit: string } + | { status: "closed-unmerged"; mergeCommit?: undefined } + ); + +export type CreatePullRequestInput = { + title: string; + body: string; + baseBranch: string; + headBranch: string; +}; + +export type FindPullRequestsQuery = { + targetRepository: RepositoryIdentity; + baseBranch: string; + headRepository: RepositoryIdentity; + headBranch: string; +}; + +function sameCaseInsensitiveValue(left: string, right: string): boolean { + return left.toLowerCase() === right.toLowerCase(); +} + +export function sameRepositoryIdentity( + left: RepositoryIdentity, + right: RepositoryIdentity, +): boolean { + return ( + left.provider === right.provider && + sameCaseInsensitiveValue(left.host, right.host) && + sameCaseInsensitiveValue(left.owner, right.owner) && + sameCaseInsensitiveValue(left.repository, right.repository) + ); +} + +export class IncompletePullRequestSearchError extends Error { + readonly query: FindPullRequestsQuery; + readonly limit?: number; + + constructor(query: FindPullRequestsQuery, limit?: number) { + super( + limit === undefined + ? "Pull request search was incomplete" + : `Pull request search stopped at provider limit ${limit}`, + ); + this.name = "IncompletePullRequestSearchError"; + this.query = query; + if (limit !== undefined) this.limit = limit; + } +} + +export class PullRequestNotFoundError extends Error { + readonly reference: PullRequestReference; + + constructor(reference: PullRequestReference) { + super(`Pull request #${reference.number} was not found`); + this.name = "PullRequestNotFoundError"; + this.reference = reference; + } +} + +export class PullRequestIdentityError extends Error { + readonly reason: string; + readonly expected?: RepositoryIdentity; + readonly actual?: Partial; + + constructor( + reason: string, + identities: { + expected?: RepositoryIdentity; + actual?: Partial; + } = {}, + ) { + super(`Pull request identity is invalid: ${reason}`); + this.name = "PullRequestIdentityError"; + this.reason = reason; + if (identities.expected !== undefined) { + this.expected = identities.expected; + } + if (identities.actual !== undefined) { + this.actual = identities.actual; + } + } +} + +export interface PullRequestHost { + readonly id: PatchmillHostProviderId; + + resolveTargetRepositoryIdentity(): Promise; + + resolveRemoteRepositoryIdentity(remote: string): Promise; + + createPullRequest(input: CreatePullRequestInput): Promise; + + findPullRequests( + query: FindPullRequestsQuery, + ): Promise; + + getPullRequest(reference: PullRequestReference): Promise; + + readPullRequestBody(reference: PullRequestReference): Promise; + + updatePullRequestBody( + reference: PullRequestReference, + body: string, + ): Promise; +} +``` + +Do not import or extend `RunOnceHostProvider`. Do not modify either production +provider. + +- [ ] **Step 5: Format and run focused validation** + +Run: + +```sh +npx --no-install prettier --write \ + src/host/pull-requests.ts \ + src/host/pull-requests.test.ts +node --test src/host/pull-requests.test.ts +npm run check:types +``` + +Expected: the focused tests and strict TypeScript checks PASS. + +- [ ] **Step 6: Commit the host contract** + +```sh +git add src/host/pull-requests.ts src/host/pull-requests.test.ts +git commit -m "feat(host): define pull request contract" +``` + +--- + +### Task 2: Implement pure planning phase and pull request rules + +**Files:** + +- Create: `src/workflow/planning-pull-requests.ts` +- Create: `src/workflow/planning-pull-requests.test.ts` + +**Interfaces:** + +- Consumes: `GitWorktreeStrategyConfig` from `src/git/types.ts`. +- Consumes: `buildIssueBranchName` and `buildIssueWorktreePath` from + `src/git/worktree-strategy.ts`. +- Produces: `PlanningPhaseKind`, `PlanningArtifactKind`, `PlanningGateSnapshot`, + and `PlannedPhase`. +- Produces: `planningPhasePlan` with the exact four-row workflow matrix. +- Produces: `phaseWorkspaceIdentity` with the phase suffix outside `slugLength`. +- Produces: strict marker rendering and parsing for `planning-pr-v1`. +- Produces: fixed non-closing planning titles and bodies. +- Produces: `PlanningPullRequestMarkerError` for malformed identity markers. + +- [ ] **Step 1: Write the four-combination phase planner test** + +Create `src/workflow/planning-pull-requests.test.ts` with this table-driven +test: + +```ts +import assert from "node:assert/strict"; +import test from "node:test"; +import { + planningPhasePlan, + type PlannedPhase, + type PlanningGateSnapshot, +} from "./planning-pull-requests.ts"; + +const phaseCases: Array<{ + gates: PlanningGateSnapshot; + expected: readonly PlannedPhase[]; +}> = [ + { + gates: { specRequired: false, planRequired: false }, + expected: [ + { + kind: "implementation", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + ], + }, + { + gates: { specRequired: true, planRequired: false }, + expected: [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + ], + }, + { + gates: { specRequired: false, planRequired: true }, + expected: [ + { + kind: "plan", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ], + }, + { + gates: { specRequired: true, planRequired: true }, + expected: [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "plan", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ], + }, +]; + +test("planning phase plan covers all spec and plan gate combinations", () => { + for (const entry of phaseCases) { + assert.deepEqual(planningPhasePlan(entry.gates), entry.expected); + } +}); +``` + +- [ ] **Step 2: Add workspace identity and marker tests** + +Add these imports and tests to the same file: + +```ts +import { + PlanningPullRequestMarkerError, + parsePlanningPullRequestMarker, + phaseWorkspaceIdentity, + renderPlanningPullRequestMarker, +} from "./planning-pull-requests.ts"; +import type { GitWorktreeStrategyConfig } from "../git/types.ts"; + +const strategy: GitWorktreeStrategyConfig = { + baseBranch: "main", + baseRef: "HEAD", + remote: "origin", + branchPrefix: "agent/issue-", + worktreeDir: ".worktrees", + worktreePrefix: "patchmill-issue-", + slugLength: 48, + allowDirectLand: false, +}; + +test("phase workspace identity appends the phase outside the issue slug", () => { + assert.deepEqual( + phaseWorkspaceIdentity({ + issueNumber: 184, + title: "Define provider-neutral planning pull request foundations", + phase: "plan", + strategy, + }), + { + branch: + "agent/issue-184-define-provider-neutral-planning-pull-request-fo-plan", + worktreePath: + ".worktrees/patchmill-issue-184-define-provider-neutral-planning-pull-request-fo-plan", + }, + ); +}); + +test("planning pull request marker renders and parses exact identity", () => { + const marker = renderPlanningPullRequestMarker({ + issueNumber: 184, + phase: "spec", + }); + + assert.equal( + marker, + "", + ); + assert.deepEqual(parsePlanningPullRequestMarker(`Header\n\n${marker}\n`), { + workflowVersion: "planning-pr-v1", + issueNumber: 184, + phase: "spec", + }); +}); + +test("marker parser returns undefined when no planning marker exists", () => { + assert.equal(parsePlanningPullRequestMarker("Refs #184"), undefined); +}); + +test("marker parser rejects duplicate and invalid planning markers", () => { + const invalidBodies = [ + [ + "", + "", + ].join("\n"), + "", + "", + "", + "/u); + assert.doesNotMatch(body, closingKeyword); +}); +``` + +- [ ] **Step 4: Run the focused test to prove RED** + +Run: + +```sh +node --test src/workflow/planning-pull-requests.test.ts +``` + +Expected: FAIL because `src/workflow/planning-pull-requests.ts` does not exist. + +- [ ] **Step 5: Implement the pure workflow module** + +Create `src/workflow/planning-pull-requests.ts` with this implementation: + +```ts +import { + buildIssueBranchName, + buildIssueWorktreePath, +} from "../git/worktree-strategy.ts"; +import type { GitWorktreeStrategyConfig } from "../git/types.ts"; + +export const PLANNING_PR_WORKFLOW_VERSION = "planning-pr-v1" as const; + +export type PlanningPhaseKind = "spec" | "plan" | "implementation"; +export type PlanningArtifactKind = "spec" | "plan"; + +export type PlanningGateSnapshot = { + specRequired: boolean; + planRequired: boolean; +}; + +export type PlannedPhase = { + kind: PlanningPhaseKind; + artifactKinds: readonly PlanningArtifactKind[]; + pullRequestRequired: true; +}; + +export function planningPhasePlan( + gates: PlanningGateSnapshot, +): readonly PlannedPhase[] { + if (gates.specRequired && gates.planRequired) { + return [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "plan", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ]; + } + if (gates.specRequired) { + return [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + ]; + } + if (gates.planRequired) { + return [ + { + kind: "plan", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ]; + } + return [ + { + kind: "implementation", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + ]; +} + +export function phaseWorkspaceIdentity(input: { + issueNumber: number; + title: string; + phase: PlanningPhaseKind; + strategy: GitWorktreeStrategyConfig; +}): { branch: string; worktreePath: string } { + const branch = buildIssueBranchName( + input.issueNumber, + input.title, + input.strategy, + ); + const worktreePath = buildIssueWorktreePath( + input.issueNumber, + input.title, + input.strategy, + ); + return { + branch: `${branch}-${input.phase}`, + worktreePath: `${worktreePath}-${input.phase}`, + }; +} + +const markerCandidatePattern = //gu; +const validMarkerPattern = + /^$/u; +const markerPrefix = "patchmill:planning-pr-"; + +export class PlanningPullRequestMarkerError extends Error { + readonly reason: string; + readonly marker: string; + + constructor(reason: string, marker: string) { + super(`Planning pull request marker is invalid: ${reason}`); + this.name = "PlanningPullRequestMarkerError"; + this.reason = reason; + this.marker = marker; + } +} + +function assertPositiveIssueNumber(issueNumber: number): void { + if (!Number.isSafeInteger(issueNumber) || issueNumber < 1) { + throw new RangeError("Issue number must be a positive safe integer"); + } +} + +export function renderPlanningPullRequestMarker(input: { + issueNumber: number; + phase: PlanningPhaseKind; +}): string { + assertPositiveIssueNumber(input.issueNumber); + return ``; +} + +export function parsePlanningPullRequestMarker(body: string): + | { + workflowVersion: typeof PLANNING_PR_WORKFLOW_VERSION; + issueNumber: number; + phase: PlanningPhaseKind; + } + | undefined { + const candidates = body.match(markerCandidatePattern) ?? []; + if (candidates.length === 0) { + if (body.includes(markerPrefix)) { + throw new PlanningPullRequestMarkerError("malformed marker", body); + } + return undefined; + } + if (candidates.length !== 1) { + throw new PlanningPullRequestMarkerError( + "multiple markers", + candidates.join("\n"), + ); + } + const marker = candidates[0]!; + const match = validMarkerPattern.exec(marker); + if (!match) { + throw new PlanningPullRequestMarkerError("unsupported marker", marker); + } + const issueNumber = Number(match[2]); + if (!Number.isSafeInteger(issueNumber)) { + throw new PlanningPullRequestMarkerError("invalid issue number", marker); + } + return { + workflowVersion: PLANNING_PR_WORKFLOW_VERSION, + issueNumber, + phase: match[3] as PlanningPhaseKind, + }; +} + +export function planningPullRequestTitle(input: { + issueNumber: number; + phase: PlanningArtifactKind; +}): string { + assertPositiveIssueNumber(input.issueNumber); + const label = input.phase === "spec" ? "Spec" : "Plan"; + return `${label} for #${input.issueNumber}`; +} + +export function planningPullRequestBody(input: { + issueNumber: number; + phase: PlanningArtifactKind; + artifactPaths: readonly string[]; +}): string { + assertPositiveIssueNumber(input.issueNumber); + const artifactPaths = [...new Set(input.artifactPaths)]; + if (artifactPaths.length === 0) { + throw new RangeError("Planning pull request requires an artifact path"); + } + const label = input.phase === "spec" ? "Spec" : "Plan"; + return [ + `Refs #${input.issueNumber}`, + "", + "## Planning phase", + "", + label, + "", + "## Artifacts", + "", + ...artifactPaths.map((path) => `- \`${path}\``), + "", + "Merge this pull request to unlock the next phase.", + "", + renderPlanningPullRequestMarker(input), + ].join("\n"); +} +``` + +Keep marker parsing strict. Do not accept unknown versions or phases. + +- [ ] **Step 6: Format and run focused validation** + +Run: + +```sh +npx --no-install prettier --write \ + src/workflow/planning-pull-requests.ts \ + src/workflow/planning-pull-requests.test.ts +node --test src/workflow/planning-pull-requests.test.ts +npm run check:types +npm run check:architecture +``` + +Expected: the focused tests, strict TypeScript checks, and architecture check +PASS. + +- [ ] **Step 7: Commit the pure workflow rules** + +```sh +git add \ + src/workflow/planning-pull-requests.ts \ + src/workflow/planning-pull-requests.test.ts +git commit -m "feat(workflow): define planning pull request phases" +``` + +--- + +### Task 3: Define the planning workspace lifecycle contract + +**Files:** + +- Create: `src/git/planning-workspaces.ts` +- Create: `src/git/planning-workspaces.test.ts` + +**Interfaces:** + +- Produces: `PlanningWorkspaceIdentity` for an expected branch and worktree + path. +- Produces: a discriminated `PlanningWorkspaceSnapshot` with `missing`, + `branch-only`, and `ready` states. +- Produces: `PreparedPlanningWorkspace` with the fetched remote base and ready + snapshot. +- Produces: `PlanningWorkspaceLifecycle` with `prepare`, `inspect`, + `removeWorktree`, and `removeBranch`. +- Produces: `PlanningWorkspaceConflictReason` and + `PlanningWorkspaceConflictError` for fail-closed state. +- The interface binds each concrete adapter to one repository root. +- The interface does not expose `CommandRunner` or Git command details. + +- [ ] **Step 1: Write the fake workspace contract test** + +Create `src/git/planning-workspaces.test.ts` with this in-memory adapter and +contract test: + +```ts +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 { + this.events.push("prepare"); + const headSha = input.resume?.headSha ?? "head-1"; + const snapshot: Extract = { + 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 { + this.events.push("inspect"); + return this.snapshot; + } + + async removeWorktree( + _identity: PlanningWorkspaceIdentity, + ): Promise> { + 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> { + 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); +}); +``` + +Do not move this adapter to `test-support`. Later issue #188 can extract a +shared adapter after it has a second consumer. + +- [ ] **Step 2: Run the focused test to prove RED** + +Run: + +```sh +node --test src/git/planning-workspaces.test.ts +``` + +Expected: FAIL because `src/git/planning-workspaces.ts` does not exist. + +- [ ] **Step 3: Implement the workspace lifecycle types and interface** + +Create `src/git/planning-workspaces.ts` with this code: + +```ts +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; +}; + +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; + + inspect( + identity: PlanningWorkspaceIdentity, + ): Promise; + + removeWorktree( + identity: PlanningWorkspaceIdentity, + ): Promise>; + + removeBranch(input: { + identity: PlanningWorkspaceIdentity; + pushedHeadSha: string; + }): Promise>; +} +``` + +Do not import `CommandRunner`. Do not add a production Git adapter in this task. + +- [ ] **Step 4: Format and run focused validation** + +Run: + +```sh +npx --no-install prettier --write \ + src/git/planning-workspaces.ts \ + src/git/planning-workspaces.test.ts +node --test src/git/planning-workspaces.test.ts +npm run check:types +npm run check:architecture +``` + +Expected: the focused tests, strict TypeScript checks, and architecture check +PASS. + +- [ ] **Step 5: Commit the workspace contract** + +```sh +git add \ + src/git/planning-workspaces.ts \ + src/git/planning-workspaces.test.ts +git commit -m "feat(git): define planning workspace lifecycle" +``` + +--- + +## Final Validation + +After Task 3, run all focused contract tests together: + +```sh +node --test \ + src/host/pull-requests.test.ts \ + src/workflow/planning-pull-requests.test.ts \ + src/git/planning-workspaces.test.ts +``` + +Expected: all focused tests PASS. + +Run the repository validation commands: + +```sh +npm run check:types +npm run build +npm run check:architecture +npm run lint +npm test +git diff --check +``` + +Expected: every command exits with status 0. + +Inspect the branch file list: + +```sh +git diff --name-only main...HEAD +``` + +Expected branch files: + +```text +docs/plans/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations.md +docs/specs/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations-design.md +src/git/planning-workspaces.test.ts +src/git/planning-workspaces.ts +src/host/pull-requests.test.ts +src/host/pull-requests.ts +src/workflow/planning-pull-requests.test.ts +src/workflow/planning-pull-requests.ts +``` + +No other production file changes belong in issue #184. + +No Nix build is required because this issue does not change npm dependencies. + +## Implementation Completion Report + +The implementation worker reports: + +- The three implementation commit hashes. +- The focused test result. +- The strict TypeScript, build, architecture, lint, and full test results. +- Any deviation from the six planned implementation files. +- Any residual risk in the contracts for issues #185, #186, or #187. diff --git a/docs/specs/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations-design.md b/docs/specs/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations-design.md new file mode 100644 index 00000000..d894c663 --- /dev/null +++ b/docs/specs/2026-09-01-issue-184-provider-neutral-planning-pull-request-foundations-design.md @@ -0,0 +1,527 @@ +# Issue 184 provider-neutral planning pull request foundations design + +## Status + +The design decisions are approved in chat. This written specification awaits +document review. + +Issue #184 is the first delivery slice of issue #180. This design treats the +approved workflow decisions from issue #180 as constraints. + +## Summary + +Patchmill needs stable foundations for planning pull requests before it adds +GitHub, Forgejo, and Git workspace implementations. + +This change defines three focused modules: + +1. A provider-neutral pull request host contract. +2. Pure planning phase and pull request rules. +3. A planning workspace lifecycle contract. + +This change does not connect the new contracts to the current run-once workflow. +Existing runtime behavior stays unchanged. + +## Goals + +- Define complete repository and pull request identities. +- Define pull request search completeness and error semantics. +- Define the phase sequence for all four planning gate combinations. +- Define deterministic planning pull request markers, titles, and bodies. +- Define deterministic phase branch and worktree names. +- Hide Git command sequences behind a small workspace interface. +- Prove the host and workspace seams with test adapters. +- Give later issues stable types and functions to implement and consume. + +## Non-goals + +- Add GitHub commands. +- Add Forgejo commands. +- Implement Git worktree operations. +- Change the host factory or `RunOnceHostProvider`. +- Add production methods that throw `not implemented` errors. +- Add run recovery state, locks, phase coordination, or publication. +- Change issue labels, artifact comments, or pipeline behavior. +- Publish or merge a planning pull request. + +## Module map + +### `src/host/pull-requests.ts` + +This module owns provider-neutral repository and pull request contracts. It +contains no CLI commands and no provider response fields. + +### `src/workflow/planning-pull-requests.ts` + +This module owns pure planning workflow rules. It derives phases and renders or +parses deterministic identity text. + +### `src/git/planning-workspaces.ts` + +This module owns the seam for the local phase workspace lifecycle. It defines +observable snapshots and high-level lifecycle operations. + +Each module has one colocated test file. The test adapters stay in those test +files until another test needs the same implementation. + +## Repository identity + +The public type is `RepositoryIdentity`. The word `Canonical` is not part of the +identifier. + +```ts +export type RepositoryIdentity = { + provider: PatchmillHostProviderId; + host: string; + owner: string; + repository: string; +}; +``` + +A `RepositoryIdentity` is complete and provider-normalized: + +- `provider` identifies the active host adapter. +- `host` contains the canonical host name without a URL scheme or path. +- `owner` contains the provider-resolved repository owner. +- `repository` contains the provider-resolved repository name. + +An owner and repository slug without a host is not a complete identity. A raw +Git remote URL is not a `RepositoryIdentity`. + +`sameRepositoryIdentity(left, right)` compares `provider` exactly. It compares +`host`, `owner`, and `repository` without case sensitivity. + +## Pull request contract + +### Types + +The module defines these public types: + +```ts +export type PullRequestStatus = "open" | "merged" | "closed-unmerged"; + +export type PullRequestReference = { + targetRepository: RepositoryIdentity; + number: number; +}; + +export type PullRequestSummary = { + number: number; + url: string; + targetRepository: RepositoryIdentity; + baseBranch: string; + headRepository: RepositoryIdentity; + headBranch: string; + headSha: string; + body: string; +} & ( + | { status: "open"; mergeCommit?: undefined } + | { status: "merged"; mergeCommit: string } + | { status: "closed-unmerged"; mergeCommit?: undefined } +); + +export type CreatePullRequestInput = { + title: string; + body: string; + baseBranch: string; + headBranch: string; +}; + +export type FindPullRequestsQuery = { + targetRepository: RepositoryIdentity; + baseBranch: string; + headRepository: RepositoryIdentity; + headBranch: string; +}; +``` + +A merged summary always has a merge commit. An open or closed-unmerged summary +does not have a merge commit. + +### Host interface + +```ts +export interface PullRequestHost { + readonly id: PatchmillHostProviderId; + + resolveTargetRepositoryIdentity(): Promise; + + resolveRemoteRepositoryIdentity(remote: string): Promise; + + createPullRequest(input: CreatePullRequestInput): Promise; + + findPullRequests( + query: FindPullRequestsQuery, + ): Promise; + + getPullRequest(reference: PullRequestReference): Promise; + + readPullRequestBody(reference: PullRequestReference): Promise; + + updatePullRequestBody( + reference: PullRequestReference, + body: string, + ): Promise; +} +``` + +The interface is separate from `RunOnceHostProvider`. Issues #185 and #186 can +implement this interface without temporary production stubs. + +### Completeness semantics + +A successful `findPullRequests` call means that the result is exhaustive. The +adapter must search all pull request states and all available pages. + +The adapter throws `IncompletePullRequestSearchError` if it cannot prove that +the result is complete. A partial result must not appear as a successful result. + +The adapter throws `PullRequestNotFoundError` only after it proves that the +requested pull request does not exist. + +The adapter throws `PullRequestIdentityError` for incomplete identity data or a +repository mismatch. + +The errors expose these stable fields: + +```ts +export class IncompletePullRequestSearchError extends Error { + readonly query: FindPullRequestsQuery; + readonly limit?: number; +} + +export class PullRequestNotFoundError extends Error { + readonly reference: PullRequestReference; +} + +export class PullRequestIdentityError extends Error { + readonly reason: string; + readonly expected?: RepositoryIdentity; + readonly actual?: Partial; +} +``` + +Authentication, transport, rate-limit, and malformed-response errors retain +their original error type. The host contract does not misclassify these errors +as a missing pull request. + +## Planning phases + +The workflow version is `planning-pr-v1`. + +```ts +export const PLANNING_PR_WORKFLOW_VERSION = "planning-pr-v1" as const; + +export type PlanningPhaseKind = "spec" | "plan" | "implementation"; +export type PlanningArtifactKind = "spec" | "plan"; + +export type PlanningGateSnapshot = { + specRequired: boolean; + planRequired: boolean; +}; + +export type PlannedPhase = { + kind: PlanningPhaseKind; + artifactKinds: readonly PlanningArtifactKind[]; + pullRequestRequired: true; +}; + +export function planningPhasePlan( + gates: PlanningGateSnapshot, +): readonly PlannedPhase[]; +``` + +The function returns these exact phase sequences: + +| Spec gate | Plan gate | Phase sequence | +| --------- | --------- | ----------------------------------------------------------------------- | +| Disabled | Disabled | Implementation contains the spec and plan. | +| Enabled | Disabled | Spec contains the spec. Implementation contains the plan. | +| Disabled | Enabled | Plan contains the spec and plan. Implementation follows. | +| Enabled | Enabled | Spec contains the spec. Plan contains the plan. Implementation follows. | + +Every listed phase ends in a pull request. The implementation phase also +contains code, but `artifactKinds` lists planning artifacts only. + +A remote-base artifact can satisfy a planning gate without a new pull request. +Later reconciliation logic owns that decision. The pure phase plan does not +inspect repositories or run state. + +## Phase workspace identity + +`phaseWorkspaceIdentity` uses the existing issue slug and strategy helpers. It +appends the phase suffix after the complete issue branch and worktree name. + +```ts +export function phaseWorkspaceIdentity(input: { + issueNumber: number; + title: string; + phase: PlanningPhaseKind; + strategy: GitWorktreeStrategyConfig; +}): { + branch: string; + worktreePath: string; +}; +``` + +The phase suffix does not count against `slugLength`. + +For issue #184, the names have this form: + +```text +agent/issue-184--spec +.worktrees/patchmill-issue-184--spec +``` + +The `plan` and `implementation` phases use their matching suffix. + +## Planning pull request identity + +### Marker + +The marker format is: + +```html + +``` + +For the spec phase of issue #184, the marker is: + +```html + +``` + +The marker is an invisible pull request body comment. It is not part of the +committed spec or plan. + +The marker helps Patchmill recover from an uncertain create response. A later +run can find the pull request by repository and branch identity. The marker then +proves the workflow version, issue, and phase. + +The parser returns `undefined` when the body has no Patchmill planning marker. +It throws `PlanningPullRequestMarkerError` for these conditions: + +- More than one marker exists. +- A marker is malformed. +- The workflow version is unsupported. +- The issue number is not a positive integer. +- The phase is not `spec`, `plan`, or `implementation`. + +Human-readable title or body text is not an identity source. + +### Titles + +Planning pull request titles use these forms: + +```text +Spec for # +Plan for # +``` + +The title identifies the issue and phase without untrusted issue text. It does +not contain an issue-closing keyword. + +### Bodies + +A planning pull request body contains these items in order: + +1. `Refs #` as a non-closing reference. +2. The planning phase. +3. The repository-relative artifact paths. +4. A statement that the merge unlocks the next phase. +5. The planning pull request marker. + +The renderer preserves the first occurrence of each artifact path. It omits +later duplicate occurrences. It does not inspect the filesystem. + +Only the implementation pull request can use `Closes #`. Implementation +pull request body rules remain outside issue #184. + +## Planning workspace lifecycle + +The workspace module defines an interface. Issue #187 will add the Git +implementation. + +### Identity and snapshots + +```ts +export type PlanningWorkspaceIdentity = { + branch: string; + worktreePath: string; +}; + +export type PlanningWorkspaceSnapshot = + | { + state: "missing"; + identity: PlanningWorkspaceIdentity; + } + | { + state: "branch-only"; + identity: PlanningWorkspaceIdentity; + headSha: string; + } + | { + state: "ready"; + identity: PlanningWorkspaceIdentity; + headSha: string; + clean: boolean; + }; + +export type PreparedPlanningWorkspace = { + created: boolean; + remote: string; + baseBranch: string; + baseSha: string; + snapshot: Extract; +}; +``` + +A valid snapshot has one of three states. Unexpected filesystem and Git +combinations do not create more snapshot variants. + +### Interface + +```ts +export interface PlanningWorkspaceLifecycle { + prepare(input: { + identity: PlanningWorkspaceIdentity; + remote: string; + baseBranch: string; + resume?: { + baseSha: string; + headSha: string; + }; + }): Promise; + + inspect( + identity: PlanningWorkspaceIdentity, + ): Promise; + + removeWorktree( + identity: PlanningWorkspaceIdentity, + ): Promise>; + + removeBranch(input: { + identity: PlanningWorkspaceIdentity; + pushedHeadSha: string; + }): Promise>; +} +``` + +A concrete adapter binds this interface to one repository root. + +`prepare` fetches the configured remote base before it creates a new workspace. +It creates the branch from the fetched remote base commit. It can resume only +when the expected identity and saved commits agree with live Git state. + +`removeWorktree` does not force removal. A dirty worktree blocks the operation. + +`removeBranch` proves that the exact local head exists on the remote branch. It +then removes the local branch. + +The two cleanup operations stay separate. Later run recovery state can record +worktree removal before branch removal. + +Push and pull request creation are not workspace responsibilities. Later +publication code owns those operations. + +### Workspace errors + +`PlanningWorkspaceConflictError` reports unsafe or inconsistent state. Its +reason identifies at least these conditions: + +- The expected path belongs to another branch. +- The expected branch belongs to another worktree. +- An unregistered directory exists at the expected path. +- A worktree is detached. +- A saved base or head commit does not match live state. +- A worktree is dirty during removal. +- The local branch head is not proven on the remote branch. + +The adapter blocks instead of using force flags or deleting uncertain state. + +## Test design + +### Pull request contract tests + +`src/host/pull-requests.test.ts` covers: + +- Case-insensitive comparison of host, owner, and repository. +- Exact comparison of provider IDs. +- The status and merge commit invariants. +- Error fields for incomplete search, missing PR, and identity mismatch. +- A small fake host that satisfies `PullRequestHost`. + +### Pure workflow tests + +`src/workflow/planning-pull-requests.test.ts` covers: + +- The exact phase sequence for all four gate combinations. +- Phase suffix placement outside the configured slug limit. +- Exact marker rendering. +- Valid marker parsing. +- Rejection of duplicate, malformed, and unsupported markers. +- Non-closing planning titles and bodies. +- One rendered list item for each artifact path. + +### Workspace contract tests + +`src/git/planning-workspaces.test.ts` covers: + +- The three snapshot states. +- The return state for each lifecycle operation. +- A small in-memory adapter that satisfies `PlanningWorkspaceLifecycle`. +- Coordinator-style calls through the interface without Git command details. + +The host and workspace test adapters stay local to their test files. A later +issue can extract shared adapters after a second test needs the same behavior. + +### Validation commands + +Implementation work will use these commands: + +```sh +node --test src/host/pull-requests.test.ts +node --test src/workflow/planning-pull-requests.test.ts +node --test src/git/planning-workspaces.test.ts +npm run build +npm run lint +npm test +``` + +These tests pass the Testing Value Gate. They protect reusable contracts, +workflow decisions, parsing, validation, and fail-closed errors. + +No dependency changes are planned. A Nix build is not required for this slice. + +## Module size + +Each production module has one reason to change: + +- Host contract changes affect `src/host/pull-requests.ts`. +- Planning policy changes affect `src/workflow/planning-pull-requests.ts`. +- Workspace lifecycle changes affect `src/git/planning-workspaces.ts`. + +Each production module targets fewer than 200 meaningful lines. If the workflow +module exceeds that target, marker parsing is the first private implementation +to extract. + +## Compatibility + +This slice adds contracts and pure functions only. It does not change current +host adapters, factories, pipeline stages, run state, labels, or publication. + +Existing runtime behavior and public CLI behavior stay unchanged. + +## Acceptance criteria + +- `RepositoryIdentity` contains provider, host, owner, and repository identity. +- Pull request summaries use normalized status and complete repository identity. +- Successful pull request searches are exhaustive. +- Incomplete searches, proven absence, and identity mismatches remain distinct. +- The phase planner covers all four planning gate combinations. +- Planning markers, titles, bodies, and workspace names are deterministic. +- The workspace interface hides Git command sequencing. +- Workspace cleanup blocks unsafe or uncertain deletion. +- Fake host and workspace adapters satisfy the production interfaces. +- Existing runtime behavior stays unchanged. diff --git a/package.json b/package.json index 926a0d19..ed485168 100644 --- a/package.json +++ b/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", diff --git a/src/git/planning-workspaces.test.ts b/src/git/planning-workspaces.test.ts new file mode 100644 index 00000000..c4a614a1 --- /dev/null +++ b/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 { + this.events.push("prepare"); + const headSha = input.resume?.headSha ?? "head-1"; + const snapshot: Extract = { + 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 { + this.events.push("inspect"); + return this.snapshot; + } + + async removeWorktree( + _identity: PlanningWorkspaceIdentity, + ): Promise> { + 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> { + 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); +}); diff --git a/src/git/planning-workspaces.ts b/src/git/planning-workspaces.ts new file mode 100644 index 00000000..547fd8a8 --- /dev/null +++ b/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; +}; + +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; + + inspect( + identity: PlanningWorkspaceIdentity, + ): Promise; + + removeWorktree( + identity: PlanningWorkspaceIdentity, + ): Promise>; + + removeBranch(input: { + identity: PlanningWorkspaceIdentity; + pushedHeadSha: string; + }): Promise>; +} diff --git a/src/host/pull-requests.test.ts b/src/host/pull-requests.test.ts new file mode 100644 index 00000000..e3b2ba18 --- /dev/null +++ b/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"); +}); diff --git a/src/host/pull-requests.ts b/src/host/pull-requests.ts new file mode 100644 index 00000000..521b5522 --- /dev/null +++ b/src/host/pull-requests.ts @@ -0,0 +1,136 @@ +import type { PatchmillHostProviderId } from "../config/types.ts"; + +export type RepositoryIdentity = { + provider: PatchmillHostProviderId; + host: string; + owner: string; + repository: string; +}; + +export type PullRequestStatus = "open" | "merged" | "closed-unmerged"; + +export type PullRequestReference = { + targetRepository: RepositoryIdentity; + number: number; +}; + +type PullRequestSummaryIdentity = { + number: number; + url: string; + targetRepository: RepositoryIdentity; + baseBranch: string; + headRepository: RepositoryIdentity; + headBranch: string; + headSha: string; + body: string; +}; + +export type PullRequestSummary = PullRequestSummaryIdentity & + ( + | { status: "open"; mergeCommit?: undefined } + | { status: "merged"; mergeCommit: string } + | { status: "closed-unmerged"; mergeCommit?: undefined } + ); + +export type CreatePullRequestInput = { + title: string; + body: string; + baseBranch: string; + headBranch: string; +}; + +export type FindPullRequestsQuery = { + targetRepository: RepositoryIdentity; + baseBranch: string; + headRepository: RepositoryIdentity; + headBranch: string; +}; + +function sameCaseInsensitiveValue(left: string, right: string): boolean { + return left.toLowerCase() === right.toLowerCase(); +} + +export function sameRepositoryIdentity( + left: RepositoryIdentity, + right: RepositoryIdentity, +): boolean { + return ( + left.provider === right.provider && + sameCaseInsensitiveValue(left.host, right.host) && + sameCaseInsensitiveValue(left.owner, right.owner) && + sameCaseInsensitiveValue(left.repository, right.repository) + ); +} + +export class IncompletePullRequestSearchError extends Error { + readonly query: FindPullRequestsQuery; + readonly limit?: number; + + constructor(query: FindPullRequestsQuery, limit?: number) { + super( + limit === undefined + ? "Pull request search was incomplete" + : `Pull request search stopped at provider limit ${limit}`, + ); + this.name = "IncompletePullRequestSearchError"; + this.query = query; + if (limit !== undefined) this.limit = limit; + } +} + +export class PullRequestNotFoundError extends Error { + readonly reference: PullRequestReference; + + constructor(reference: PullRequestReference) { + super(`Pull request #${reference.number} was not found`); + this.name = "PullRequestNotFoundError"; + this.reference = reference; + } +} + +export class PullRequestIdentityError extends Error { + readonly reason: string; + readonly expected?: RepositoryIdentity; + readonly actual?: Partial; + + constructor( + reason: string, + identities: { + expected?: RepositoryIdentity; + actual?: Partial; + } = {}, + ) { + super(`Pull request identity is invalid: ${reason}`); + this.name = "PullRequestIdentityError"; + this.reason = reason; + if (identities.expected !== undefined) { + this.expected = identities.expected; + } + if (identities.actual !== undefined) { + this.actual = identities.actual; + } + } +} + +export interface PullRequestHost { + readonly id: PatchmillHostProviderId; + + resolveTargetRepositoryIdentity(): Promise; + + resolveRemoteRepositoryIdentity(remote: string): Promise; + + createPullRequest(input: CreatePullRequestInput): Promise; + + findPullRequests( + query: FindPullRequestsQuery, + ): Promise; + + getPullRequest(reference: PullRequestReference): Promise; + + readPullRequestBody(reference: PullRequestReference): Promise; + + updatePullRequestBody( + reference: PullRequestReference, + body: string, + ): Promise; +} diff --git a/src/workflow/planning-pull-request-markers.ts b/src/workflow/planning-pull-request-markers.ts new file mode 100644 index 00000000..a02faeed --- /dev/null +++ b/src/workflow/planning-pull-request-markers.ts @@ -0,0 +1,112 @@ +export const PLANNING_PR_WORKFLOW_VERSION = "planning-pr-v1" as const; + +export type PlanningPhaseKind = "spec" | "plan" | "implementation"; + +export class PlanningPullRequestMarkerError extends Error { + readonly reason: string; + readonly marker: string; + + constructor(reason: string, marker: string) { + super(`Planning pull request marker is invalid: ${reason}`); + this.name = "PlanningPullRequestMarkerError"; + this.reason = reason; + this.marker = marker; + } +} + +function assertPositiveIssueNumber(issueNumber: number): void { + if (!Number.isSafeInteger(issueNumber) || issueNumber < 1) { + throw new RangeError("Issue number must be a positive safe integer"); + } +} + +export function renderPlanningPullRequestMarker(input: { + issueNumber: number; + phase: PlanningPhaseKind; +}): string { + assertPositiveIssueNumber(input.issueNumber); + return ``; +} + +const markerPrefix = "$/u; +const openingFencePattern = /^(`{3,}|~{3,})(.*)$/u; +const closingFencePattern = /^(`+|~+)[ \t]*$/u; + +type MarkerLine = { line: string; index: number }; + +function openingFence(line: string): string | undefined { + const match = line.match(openingFencePattern); + if (match === null) return undefined; + const delimiter = match[1]!; + return delimiter[0] === "`" && match[2]!.includes("`") + ? undefined + : delimiter; +} + +function topLevelMarkerLines(lines: readonly string[]): MarkerLine[] { + const markers: MarkerLine[] = []; + let fence: string | undefined; + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]!; + if (fence !== undefined) { + const closingFence = line.match(closingFencePattern)?.[1]; + if ( + closingFence !== undefined && + closingFence[0] === fence[0] && + closingFence.length >= fence.length + ) { + fence = undefined; + } + continue; + } + const opener = openingFence(line); + if (opener !== undefined) { + fence = opener; + } else if (line.startsWith(markerPrefix)) { + markers.push({ line, index }); + } + } + return markers; +} + +function finalNonblankLineIndex(lines: readonly string[]): number { + for (let index = lines.length - 1; index >= 0; index -= 1) { + if (lines[index]!.trim() !== "") return index; + } + return -1; +} + +export function parsePlanningPullRequestMarker(body: string): + | { + workflowVersion: typeof PLANNING_PR_WORKFLOW_VERSION; + issueNumber: number; + phase: PlanningPhaseKind; + } + | undefined { + const lines = body.split("\n"); + const markers = topLevelMarkerLines(lines); + if (markers.length === 0) return undefined; + if (markers.length !== 1) { + throw new PlanningPullRequestMarkerError("multiple markers", body); + } + const marker = markers[0]!; + if (marker.index !== finalNonblankLineIndex(lines)) return undefined; + const match = validMarkerPattern.exec(marker.line); + if (match === null) { + throw new PlanningPullRequestMarkerError("unsupported marker", marker.line); + } + const issueNumber = Number(match[2]); + if (!Number.isSafeInteger(issueNumber)) { + throw new PlanningPullRequestMarkerError( + "invalid issue number", + marker.line, + ); + } + return { + workflowVersion: PLANNING_PR_WORKFLOW_VERSION, + issueNumber, + phase: match[3] as PlanningPhaseKind, + }; +} diff --git a/src/workflow/planning-pull-requests.test.ts b/src/workflow/planning-pull-requests.test.ts new file mode 100644 index 00000000..697f4e1c --- /dev/null +++ b/src/workflow/planning-pull-requests.test.ts @@ -0,0 +1,306 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { GitWorktreeStrategyConfig } from "../git/types.ts"; +import { + PlanningPullRequestMarkerError, + parsePlanningPullRequestMarker, + phaseWorkspaceIdentity, + planningPhasePlan, + planningPullRequestBody, + planningPullRequestTitle, + renderPlanningPullRequestMarker, + type PlannedPhase, + type PlanningGateSnapshot, +} from "./planning-pull-requests.ts"; + +const phaseCases: Array<{ + gates: PlanningGateSnapshot; + expected: readonly PlannedPhase[]; +}> = [ + { + gates: { specRequired: false, planRequired: false }, + expected: [ + { + kind: "implementation", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + ], + }, + { + gates: { specRequired: true, planRequired: false }, + expected: [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + ], + }, + { + gates: { specRequired: false, planRequired: true }, + expected: [ + { + kind: "plan", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ], + }, + { + gates: { specRequired: true, planRequired: true }, + expected: [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "plan", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ], + }, +]; + +test("planning phase plan covers all spec and plan gate combinations", () => { + for (const entry of phaseCases) { + assert.deepEqual(planningPhasePlan(entry.gates), entry.expected); + } +}); + +const strategy: GitWorktreeStrategyConfig = { + baseBranch: "main", + baseRef: "HEAD", + remote: "origin", + branchPrefix: "agent/issue-", + worktreeDir: ".worktrees", + worktreePrefix: "patchmill-issue-", + slugLength: 48, + allowDirectLand: false, +}; + +test("phase workspace identity appends the phase outside the issue slug", () => { + assert.deepEqual( + phaseWorkspaceIdentity({ + issueNumber: 184, + title: "Define provider-neutral planning pull request foundations", + phase: "plan", + strategy, + }), + { + branch: + "agent/issue-184-define-provider-neutral-planning-pull-request-fo-plan", + worktreePath: + ".worktrees/patchmill-issue-184-define-provider-neutral-planning-pull-request-fo-plan", + }, + ); +}); + +test("planning pull request marker renders and parses exact identity", () => { + const marker = renderPlanningPullRequestMarker({ + issueNumber: 184, + phase: "spec", + }); + + assert.equal( + marker, + "", + ); + assert.deepEqual(parsePlanningPullRequestMarker(`Header\n\n${marker}\n`), { + workflowVersion: "planning-pr-v1", + issueNumber: 184, + phase: "spec", + }); +}); + +test("marker parser returns undefined when no planning marker exists", () => { + assert.equal(parsePlanningPullRequestMarker("Refs #184"), undefined); +}); + +test("marker parser requires the exact final top-level marker line", () => { + const marker = renderPlanningPullRequestMarker({ + issueNumber: 184, + phase: "spec", + }); + + for (const body of [ + `\`${marker}\``, + `> ${marker}`, + ` ${marker}`, + ["`", marker, "`"].join("\n"), + ]) { + assert.equal(parsePlanningPullRequestMarker(body), undefined); + } +}); + +test("marker parser ignores fenced and indented Markdown code blocks", () => { + const marker = renderPlanningPullRequestMarker({ + issueNumber: 184, + phase: "spec", + }); + + const codeBodies = [ + ["```html", marker, "```"].join("\n"), + ["~~~html", marker, "~~~"].join("\n"), + ["> ~~~html", `> ${marker}`, "> ~~~"].join("\n"), + ["- ~~~html", ` ${marker}`, " ~~~"].join("\n"), + `- ${marker}`, + ` ${marker}`, + `\t${marker}`, + ]; + for (const body of codeBodies) { + assert.equal(parsePlanningPullRequestMarker(body), undefined); + } + assert.deepEqual( + parsePlanningPullRequestMarker(["```", marker, "```", marker].join("\n")), + { + workflowVersion: "planning-pr-v1", + issueNumber: 184, + phase: "spec", + }, + ); +}); + +test("marker parser recognizes a top-level marker after a container fence ends", () => { + const marker = renderPlanningPullRequestMarker({ + issueNumber: 184, + phase: "spec", + }); + + assert.deepEqual( + parsePlanningPullRequestMarker(["> ```", "> example", marker].join("\n")), + { + workflowVersion: "planning-pr-v1", + issueNumber: 184, + phase: "spec", + }, + ); +}); + +test("planning body round-trips paths that resemble invalid fenced code blocks", () => { + const body = planningPullRequestBody({ + issueNumber: 184, + phase: "spec", + artifactPaths: ["docs/``.md"], + }); + + assert.deepEqual(parsePlanningPullRequestMarker(body), { + workflowVersion: "planning-pr-v1", + issueNumber: 184, + phase: "spec", + }); +}); + +test("marker parser keeps markers inside a top-level fence after container text", () => { + const marker = renderPlanningPullRequestMarker({ + issueNumber: 184, + phase: "spec", + }); + + assert.equal( + parsePlanningPullRequestMarker(["```", "> ```", marker, "```"].join("\n")), + undefined, + ); +}); + +test("marker parser rejects duplicate and invalid planning markers", () => { + const invalidBodies = [ + [ + "", + "", + ].join("\n"), + "", + "", + "", + "", + "/u); + assert.doesNotMatch(body, closingKeyword); +}); + +test("planning body rejects artifact paths that can create Markdown blocks", () => { + for (const artifactPath of [ + "docs/specs/issue-184.md\n\nCloses #184", + "docs/specs/issue-184.md\n~~~", + ]) { + assert.throws( + () => + planningPullRequestBody({ + issueNumber: 184, + phase: "spec", + artifactPaths: [artifactPath], + }), + RangeError, + ); + } +}); diff --git a/src/workflow/planning-pull-requests.ts b/src/workflow/planning-pull-requests.ts new file mode 100644 index 00000000..1e4ad91a --- /dev/null +++ b/src/workflow/planning-pull-requests.ts @@ -0,0 +1,168 @@ +import type { GitWorktreeStrategyConfig } from "../git/types.ts"; +import { + buildIssueBranchName, + buildIssueWorktreePath, +} from "../git/worktree-strategy.ts"; +import { + renderPlanningPullRequestMarker, + type PlanningPhaseKind, +} from "./planning-pull-request-markers.ts"; + +export { + PLANNING_PR_WORKFLOW_VERSION, + PlanningPullRequestMarkerError, + parsePlanningPullRequestMarker, + renderPlanningPullRequestMarker, +} from "./planning-pull-request-markers.ts"; +export type { PlanningPhaseKind } from "./planning-pull-request-markers.ts"; + +export type PlanningArtifactKind = "spec" | "plan"; + +export type PlanningGateSnapshot = { + specRequired: boolean; + planRequired: boolean; +}; + +export type PlannedPhase = { + kind: PlanningPhaseKind; + artifactKinds: readonly PlanningArtifactKind[]; + pullRequestRequired: true; +}; + +export function planningPhasePlan( + gates: PlanningGateSnapshot, +): readonly PlannedPhase[] { + if (gates.specRequired && gates.planRequired) { + return [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "plan", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ]; + } + if (gates.specRequired) { + return [ + { + kind: "spec", + artifactKinds: ["spec"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: ["plan"], + pullRequestRequired: true, + }, + ]; + } + if (gates.planRequired) { + return [ + { + kind: "plan", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + { + kind: "implementation", + artifactKinds: [], + pullRequestRequired: true, + }, + ]; + } + return [ + { + kind: "implementation", + artifactKinds: ["spec", "plan"], + pullRequestRequired: true, + }, + ]; +} + +export function phaseWorkspaceIdentity(input: { + issueNumber: number; + title: string; + phase: PlanningPhaseKind; + strategy: GitWorktreeStrategyConfig; +}): { branch: string; worktreePath: string } { + const branch = buildIssueBranchName( + input.issueNumber, + input.title, + input.strategy, + ); + const worktreePath = buildIssueWorktreePath( + input.issueNumber, + input.title, + input.strategy, + ); + return { + branch: `${branch}-${input.phase}`, + worktreePath: `${worktreePath}-${input.phase}`, + }; +} + +function assertPositiveIssueNumber(issueNumber: number): void { + if (!Number.isSafeInteger(issueNumber) || issueNumber < 1) { + throw new RangeError("Issue number must be a positive safe integer"); + } +} + +export function planningPullRequestTitle(input: { + issueNumber: number; + phase: PlanningArtifactKind; +}): string { + assertPositiveIssueNumber(input.issueNumber); + const label = input.phase === "spec" ? "Spec" : "Plan"; + return `${label} for #${input.issueNumber}`; +} + +function markdownCodeSpan(value: string): string { + const delimiter = "`".repeat( + 1 + Math.max(0, ...(value.match(/`+/gu) ?? []).map((run) => run.length)), + ); + return `${delimiter} ${value} ${delimiter}`; +} + +function assertSingleLineArtifactPath(path: string): void { + if (/\r|\n/u.test(path)) { + throw new RangeError("Planning artifact paths must be single-line"); + } +} + +export function planningPullRequestBody(input: { + issueNumber: number; + phase: PlanningArtifactKind; + artifactPaths: readonly string[]; +}): string { + assertPositiveIssueNumber(input.issueNumber); + const artifactPaths = [...new Set(input.artifactPaths)]; + if (artifactPaths.length === 0) { + throw new RangeError("Planning pull request requires an artifact path"); + } + for (const path of artifactPaths) assertSingleLineArtifactPath(path); + const label = input.phase === "spec" ? "Spec" : "Plan"; + return [ + `Refs #${input.issueNumber}`, + "", + "## Planning phase", + "", + label, + "", + "## Artifacts", + "", + ...artifactPaths.map((path) => `- ${markdownCodeSpan(path)}`), + "", + "Merge this pull request to unlock the next phase.", + "", + renderPlanningPullRequestMarker(input), + ].join("\n"); +} diff --git a/test-support/contract-test-node.d.ts b/test-support/contract-test-node.d.ts new file mode 100644 index 00000000..5c222fef --- /dev/null +++ b/test-support/contract-test-node.d.ts @@ -0,0 +1,42 @@ +type ContractTestAssertion = (...arguments_: readonly any[]) => void; + +declare module "node:assert/strict" { + const assert: { + deepEqual: ContractTestAssertion; + doesNotMatch: ContractTestAssertion; + equal: ContractTestAssertion; + match: ContractTestAssertion; + throws: ContractTestAssertion; + }; + export default assert; +} + +declare module "node:test" { + type TestFunction = () => void | Promise; + + function test(name: string, fn: TestFunction): void; + + export default test; +} + +declare module "node:crypto" { + export type BinaryLike = any; + export const createHash: (...arguments_: readonly any[]) => any; +} + +declare module "node:fs" { + export const existsSync: (...arguments_: readonly any[]) => any; + export const statSync: (...arguments_: readonly any[]) => any; +} + +declare module "node:path" { + export const dirname: (...arguments_: readonly any[]) => any; + export const isAbsolute: (...arguments_: readonly any[]) => any; + export const join: (...arguments_: readonly any[]) => any; + export const resolve: (...arguments_: readonly any[]) => any; + export const win32: any; +} + +declare module "node:url" { + export const fileURLToPath: (...arguments_: readonly any[]) => any; +} diff --git a/tsconfig.contract-tests.json b/tsconfig.contract-tests.json new file mode 100644 index 00000000..0d47c461 --- /dev/null +++ b/tsconfig.contract-tests.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.quality.json", + "include": [ + "src/git/planning-workspaces.test.ts", + "src/host/pull-requests.test.ts", + "src/workflow/planning-pull-requests.test.ts", + "test-support/contract-test-node.d.ts" + ], + "exclude": [] +}