From d7c1921a5d1bc0183b698107734490ae56988337 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 27 Aug 2026 23:02:12 -0400 Subject: [PATCH] refactor(stack): simplify internals and avoid redundant work Reuse model fields and existing test fixtures, remove dead private repair state, and preserve checkpoint and undo behavior. Limit branch-specific worktree inspection to owners, read state files directly, and let static skill output run outside Git. Rely on prepack for a single packaging build. --- .changeset/leaner-stack-operations.md | 5 + package.json | 2 +- src/cli.ts | 7 +- src/services/Git.ts | 16 +- src/services/Stack.ts | 79 ++-- src/services/Store.ts | 16 +- src/services/code-host/GitLab.ts | 15 +- src/services/code-host/Memory.ts | 47 +- tests/cli.test.ts | 18 + tests/gitWorktrees.test.ts | 65 +++ tests/stack.test.ts | 628 ++++++++------------------ tests/store.test.ts | 50 ++ 12 files changed, 381 insertions(+), 567 deletions(-) create mode 100644 .changeset/leaner-stack-operations.md create mode 100644 tests/cli.test.ts create mode 100644 tests/gitWorktrees.test.ts create mode 100644 tests/store.test.ts diff --git a/.changeset/leaner-stack-operations.md b/.changeset/leaner-stack-operations.md new file mode 100644 index 0000000..dd84666 --- /dev/null +++ b/.changeset/leaner-stack-operations.md @@ -0,0 +1,5 @@ +--- +"@kitlangton/stack": patch +--- + +Allow `stack skill` to print instructions outside a Git repository. Avoid scanning unrelated worktree contents during branch-specific Git operations, read state files without a separate existence check, and read independent local status information concurrently. Preserve dirty-worktree preflight and undo checkpoint behavior while simplifying internal bookkeeping and shared GitLab model handling. diff --git a/package.json b/package.json index 5143798..9418ac3 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "format": "oxfmt --write .", "format:check": "oxfmt --check .", "lint": "oxlint", - "package:smoke": "bun run build && npm pack --dry-run", + "package:smoke": "npm pack --dry-run", "prepack": "bun run build", "release": "bun run package:smoke && npm exec --package @changesets/cli@2.31.0 -- changeset publish", "typecheck": "tsc --noEmit", diff --git a/src/cli.ts b/src/cli.ts index c723120..ab2155e 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -401,9 +401,10 @@ if (import.meta.main) { .slice(2) .some((arg) => arg === "--help" || arg === "-h" || arg === "--version"); - const app = help - ? runCli(process.argv.slice(2)).pipe(Effect.provide(docs)) - : runCli(process.argv.slice(2)).pipe(Effect.provide(live)); + const app = + help || process.argv[2] === "skill" + ? runCli(process.argv.slice(2)).pipe(Effect.provide(docs)) + : runCli(process.argv.slice(2)).pipe(Effect.provide(live)); const main = pipe( app, diff --git a/src/services/Git.ts b/src/services/Git.ts index 8c8216f..73ad268 100644 --- a/src/services/Git.ts +++ b/src/services/Git.ts @@ -92,7 +92,7 @@ export const live = Layer.effect( ), ); - const worktrees = Effect.fn("Git.worktrees")(function* () { + const worktrees = Effect.fn("Git.worktrees")(function* (branch?: string) { const out = yield* run("git", ["worktree", "list", "--porcelain", "-z"]); const records: Array<{ path: string; @@ -127,7 +127,9 @@ export const live = Layer.effect( if (current) records.push(current); return yield* Effect.forEach( - records.filter((record) => !record.prunable), + records.filter( + (record) => !record.prunable && (branch === undefined || record.branch === branch), + ), (record) => dirtyAt(record.path).pipe( Effect.map( @@ -279,7 +281,7 @@ export const live = Layer.effect( parent: string, commits: ReadonlyArray, ) { - const owner = (yield* worktrees()).find((worktree) => worktree.branch === branch) ?? null; + const owner = (yield* worktrees(branch))[0] ?? null; if (owner && owner.dirty.length > 0) { return yield* Effect.fail(checkedOutDirtyError(branch, owner)); } @@ -367,9 +369,7 @@ export const live = Layer.effect( ); const release = Effect.fn("Git.release")(function* (branch: string) { const owner = - (yield* worktrees()).find( - (worktree) => worktree.branch === branch && worktree.path !== cfg.root, - ) ?? null; + (yield* worktrees(branch)).find((worktree) => worktree.path !== cfg.root) ?? null; if (!owner) return; if (owner.dirty.length > 0) { return yield* Effect.fail(releaseDirtyError(branch, owner)); @@ -378,9 +378,7 @@ export const live = Layer.effect( }); const drop = Effect.fn("Git.drop")(function* (branch: string) { const owner = - (yield* worktrees()).find( - (worktree) => worktree.branch === branch && worktree.path !== cfg.root, - ) ?? null; + (yield* worktrees(branch)).find((worktree) => worktree.path !== cfg.root) ?? null; if (owner) { return yield* Effect.fail( new ExecError( diff --git a/src/services/Stack.ts b/src/services/Stack.ts index 3842336..d68cd12 100644 --- a/src/services/Stack.ts +++ b/src/services/Stack.ts @@ -420,12 +420,10 @@ ${note}`; const status: StackService["status"] = Effect.fn("Stack.status")(() => Effect.gen(function* () { - const [state, refs, current, remote] = yield* Effect.all([ - store.read(), - git.refs(), - git.current(), - git.remote(), - ]); + const [state, refs, current, remote] = yield* Effect.all( + [store.read(), git.refs(), git.current(), git.remote()], + { concurrency: 4 }, + ); const pulls = yield* codeHost.changes().pipe( Effect.catchTags({ ExecError: () => Effect.succeed([]), @@ -698,7 +696,6 @@ ${note}`; readonly apply: boolean; readonly saved?: Map; readonly journalState?: ReturnType; - readonly initialEntries?: ReadonlyArray; readonly journalActions?: ReadonlyArray; readonly initialActions?: ReadonlyArray; readonly replayAnchors?: ReadonlyMap; @@ -754,10 +751,10 @@ ${note}`; const tips = new Map(); const prior = new Map(); const moved = new Set(); - const entries: Array = Array.from(opts.initialEntries ?? []); + const entries: Array = []; const remoteUpdates: Array = []; const next: Array = []; - let journal = apply && (initialActions.length > 0 || entries.length > 0); + let journal = apply && initialActions.length > 0; const headRemote = Effect.fn("Stack.repairStack.headRemote")(function* ( headRepository: string | null, @@ -846,7 +843,6 @@ ${note}`; const plannedRepairBranches = Effect.fn("Stack.repairStack.plannedRepairBranches")( function* () { const branches = new Set(); - const plannedMoved = new Set(); const plannedTips = new Map(); for (const link of [...state.links].sort( @@ -867,12 +863,11 @@ ${note}`; const drift = replayAnchors.has(String(link.branch)) || parent !== link.parent || - plannedMoved.has(parent) || + branches.has(parent) || (want && (Option.isNone(have) || have.value !== want)); if (drift) { branches.add(String(link.branch)); - plannedMoved.add(String(link.branch)); } } @@ -929,8 +924,6 @@ ${note}`; (!apply && moved.has(parent)) || (want && (Option.isNone(have) || have.value !== want)); const base = pr?.base ?? null; - let backup: string | null = null; - let created: number | null = null; let num = pr?.number ?? link.pr; const previous = apply && !pr && link.pr @@ -963,7 +956,7 @@ ${note}`; return yield* git.novel(onto, link.branch, commits); }) : Array(); - backup = `backup/stack-sync-${stamp}-${link.branch}`; + const backup = `backup/stack-sync-${stamp}-${link.branch}`; const rebase = { branch: String(link.branch), parent, @@ -1073,7 +1066,7 @@ ${note}`; backup: null, pr: now.number, base, - created, + created: null, }), ); journal = true; @@ -1102,18 +1095,17 @@ ${note}`; const open = prs.get(link.branch) ?? null; if (!open) { if (apply) { - const prev = previous; - const nextPr = draft(link, parent, prev); - if (!entries.some((item) => item.branch === link.branch)) { - entries.push( - undoEntry({ - branch: link.branch, - backup: null, - pr: now?.number ?? link.pr ?? null, - base, - created: null, - }), - ); + const nextPr = draft(link, parent, previous); + let entry = entries.find((item) => item.branch === link.branch); + if (!entry) { + entry = undoEntry({ + branch: link.branch, + backup: null, + pr: now?.number ?? link.pr ?? null, + base, + created: null, + }); + entries.push(entry); journal = true; } yield* step(`create ${requestLabel} for ${link.branch} -> ${parent}`); @@ -1126,7 +1118,6 @@ ${note}`; nextPr.labels, headRepository, ); - created = made.number; num = made.number; prs.set(link.branch, made); const createdPull = { @@ -1135,27 +1126,14 @@ ${note}`; pr: Number(made.number), } satisfies RepairPlan.CreatePullPlan; actions.push(RepairPlan.createPull(createdPull, mode)); - const i = entries.findIndex((item) => item.branch === link.branch); - if (i >= 0) { - entries[i] = undoEntry({ - branch: entries[i]!.branch, - backup: entries[i]!.backup, - pr: entries[i]!.pr, - base: entries[i]!.base, - created: made.number, - ...(entries[i]!.pushRemotes ? { pushRemotes: entries[i]!.pushRemotes } : {}), - }); - } else { - entries.push( - undoEntry({ - branch: link.branch, - backup: null, - pr: now?.number ?? link.pr ?? null, - base, - created: made.number, - }), - ); - } + entries[entries.indexOf(entry)] = undoEntry({ + branch: entry.branch, + backup: entry.backup, + pr: entry.pr, + base: entry.base, + created: made.number, + ...(entry.pushRemotes ? { pushRemotes: entry.pushRemotes } : {}), + }); journal = true; yield* checkpoint(); } else { @@ -1688,7 +1666,6 @@ ${note}`; readonly apply?: boolean; readonly auto?: boolean; readonly admin?: boolean; - readonly through?: string; }, ) => Effect.gen(function* () { diff --git a/src/services/Store.ts b/src/services/Store.ts index e2590c8..d17ef73 100644 --- a/src/services/Store.ts +++ b/src/services/Store.ts @@ -28,14 +28,14 @@ export class Store extends Context.Service()("@stack/Store" const load = (file: string, miss: () => A, parse: (raw: string) => A) => Effect.gen(function* () { - const has = yield* fs - .exists(file) - .pipe(Effect.mapError((err) => new StateError(file, "exists", String(err)))); - if (!has) return miss(); - - const raw = yield* fs - .readFileString(file) - .pipe(Effect.mapError((err) => new StateError(file, "read", String(err)))); + const raw = yield* fs.readFileString(file).pipe( + Effect.catchIf( + (err) => err.reason._tag === "NotFound", + () => Effect.succeed(null), + ), + Effect.mapError((err) => new StateError(file, "read", String(err))), + ); + if (raw === null) return miss(); return yield* Effect.try({ try: () => parse(raw), diff --git a/src/services/code-host/GitLab.ts b/src/services/code-host/GitLab.ts index 8312937..0736c26 100644 --- a/src/services/code-host/GitLab.ts +++ b/src/services/code-host/GitLab.ts @@ -36,16 +36,10 @@ class MRData extends Schema.Class("MRData")({ }) {} class MRView extends Schema.Class("MRView")({ - iid: Schema.Number, - title: Schema.String, + ...MRData.fields, description: Schema.NullOr(Schema.String), - source_branch: Schema.String, - target_branch: Schema.String, - web_url: Schema.String, - draft: Schema.Boolean, state: Schema.String, labels: Schema.Array(LabelEntry), - source_project_id: Schema.NullOr(Schema.Number), }) {} class MRWatch extends Schema.Class("MRWatch")({ @@ -100,14 +94,9 @@ const ref = (row: MRData, headRepository: string | null) => const meta = (row: MRView, headRepository: string | null) => pullMeta({ - number: row.iid, + ...ref(row, headRepository), title: row.title, body: row.description ?? "", - head: row.source_branch, - headRepository, - base: row.target_branch, - url: row.web_url, - draft: row.draft, state: row.state, labels: row.labels.map((item) => new PullLabel({ name: labelName(item) })), }); diff --git a/src/services/code-host/Memory.ts b/src/services/code-host/Memory.ts index 2a97720..e2d047a 100644 --- a/src/services/code-host/Memory.ts +++ b/src/services/code-host/Memory.ts @@ -70,40 +70,13 @@ export const layer = (opts: Options) => yield* requireOpen(pr); yield* record(`edit ${pr} ${base}`); yield* Ref.update(pullsRef, (pulls) => - pulls.map((item) => - item.number === pr - ? pullRef({ - number: item.number, - title: item.title, - head: item.head, - headRepository: item.headRepository, - base, - url: item.url, - draft: item.draft, - checks: item.checks, - }) - : item, - ), + pulls.map((item) => (item.number === pr ? pullRef({ ...item, base }) : item)), ); yield* Ref.update(metasRef, (metas) => { const nextMetas = new Map(metas); const current = nextMetas.get(pr); if (current) { - nextMetas.set( - pr, - pullMeta({ - number: current.number, - title: current.title, - body: current.body, - head: current.head, - headRepository: current.headRepository, - base, - url: current.url, - draft: current.draft, - state: current.state, - labels: current.labels, - }), - ); + nextMetas.set(pr, pullMeta({ ...current, base })); } return nextMetas; }); @@ -118,21 +91,7 @@ export const layer = (opts: Options) => const nextMetas = new Map(metas); const current = nextMetas.get(pr); if (current) { - nextMetas.set( - pr, - pullMeta({ - number: current.number, - title: current.title, - body, - head: current.head, - headRepository: current.headRepository, - base: current.base, - url: current.url, - draft: current.draft, - state: current.state, - labels: current.labels, - }), - ); + nextMetas.set(pr, pullMeta({ ...current, body })); } return nextMetas; }); diff --git a/tests/cli.test.ts b/tests/cli.test.ts new file mode 100644 index 0000000..5830c14 --- /dev/null +++ b/tests/cli.test.ts @@ -0,0 +1,18 @@ +import { expect, it } from "@effect/vitest"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { Effect, Layer } from "effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; +import * as Proc from "../src/platform/proc.ts"; + +it.effect("skill prints the packaged instructions outside a Git repository", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const proc = yield* Proc.Service; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "stack-skill-" }); + const output = yield* proc.exec(root, "bun", [path.resolve("src/cli.ts"), "skill"]); + expect(output).toContain("name: stack"); + expect(output).toContain("stack sync"); + }).pipe(Effect.provide(Proc.live.pipe(Layer.provideMerge(NodeServices.layer)))), +); diff --git a/tests/gitWorktrees.test.ts b/tests/gitWorktrees.test.ts new file mode 100644 index 0000000..4001bbe --- /dev/null +++ b/tests/gitWorktrees.test.ts @@ -0,0 +1,65 @@ +import { expect, it } from "@effect/vitest"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { Effect, Layer } from "effect"; +import { ExecError } from "../src/domain/model.ts"; +import * as Proc from "../src/platform/proc.ts"; +import { StackConfig } from "../src/services/Config.ts"; +import { Git } from "../src/services/Git.ts"; + +const scenario = (owner = false) => { + const checked: Array = []; + const proc = Layer.succeed(Proc.Service, { + exec: (cwd, _tool, args) => + Effect.sync(() => { + if (args[0] === "worktree") + return [ + "worktree /repo\0HEAD root\0branch refs/heads/dev\0", + `worktree /other\0HEAD other\0branch refs/heads/${owner ? "topic" : "other"}\0`, + ].join("\0"); + if (args[0] === "status") { + checked.push(cwd); + return owner && cwd === "/other" ? " M file.txt" : ""; + } + return args[0] === "branch" && args[1] === "--show-current" ? "dev" : ""; + }), + }); + const layer = Git.live.pipe( + Layer.provide(StackConfig.layer({ root: "/repo" }).pipe(Layer.provide(NodeServices.layer))), + Layer.provide(proc), + ); + return { checked, layer }; +}; + +it.effect.each(["release", "drop", "replay"] as const)( + "%s does not inspect worktree contents for unrelated branches", + (operation) => { + const s = scenario(); + return Effect.gen(function* () { + const git = yield* Git.Service; + yield* operation === "replay" ? git.replay("topic", "dev", []) : git[operation]("topic"); + expect(s.checked).toEqual([]); + }).pipe(Effect.provide(s.layer)); + }, +); + +it.effect.each(["release", "replay"] as const)( + "%s still checks and refuses the dirty owning worktree", + (operation) => { + const s = scenario(true); + return Effect.gen(function* () { + const git = yield* Git.Service; + const result = operation === "replay" ? git.replay("topic", "dev", []) : git.release("topic"); + expect(yield* Effect.flip(result)).toBeInstanceOf(ExecError); + expect(s.checked).toEqual(["/other"]); + }).pipe(Effect.provide(s.layer)); + }, +); + +it.effect("full worktree inspection still reads all worktree contents", () => { + const s = scenario(); + return Effect.gen(function* () { + const git = yield* Git.Service; + expect(yield* git.worktrees()).toHaveLength(2); + expect(s.checked.sort()).toEqual(["/other", "/repo"]); + }).pipe(Effect.provide(s.layer)); +}); diff --git a/tests/stack.test.ts b/tests/stack.test.ts index d3eaa63..0dfcbd0 100644 --- a/tests/stack.test.ts +++ b/tests/stack.test.ts @@ -346,6 +346,7 @@ const realStack = (opts: { readonly base?: ReadonlyArray; readonly current?: string; readonly state?: StackState; + readonly metas?: ReadonlyArray>; }) => Effect.gen(function* () { const root = yield* tempDir(); @@ -400,19 +401,21 @@ const realStack = (opts: { draft: false, }), ), - metas: opts.branches.map((branch) => - pullMeta({ - number: branch.number, - title: branch.name, - body: `Stacked on ${branch.parent}.`, - head: branch.name, - base: branch.parent, - url: `u${branch.number}`, - draft: false, - state: "OPEN", - labels: [], - }), - ), + metas: + opts.metas ?? + opts.branches.map((branch) => + pullMeta({ + number: branch.number, + title: branch.name, + body: `Stacked on ${branch.parent}.`, + head: branch.name, + base: branch.parent, + url: `u${branch.number}`, + draft: false, + state: "OPEN", + labels: [], + }), + ), }), ), Layer.provideMerge( @@ -832,14 +835,21 @@ Footer }; }; -const makeLand = ( - dirty: ReadonlyArray = [], +const makeLand = ({ + dirty = [], currentBranch = "stack-a", - progress: Array | null = null, - codeHost: Partial = {}, + progress = null, + codeHost = {}, includeUnrelatedRoot = false, forkStackC = false, -) => { +}: { + readonly dirty?: ReadonlyArray; + readonly currentBranch?: string; + readonly progress?: Array | null; + readonly codeHost?: Partial; + readonly includeUnrelatedRoot?: boolean; + readonly forkStackC?: boolean; +} = {}) => { const seen: Array = []; const refs = new Map([ ["dev", branchRef({ name: "dev", head: "dev-2" })], @@ -3491,70 +3501,25 @@ describe("Stack", () => { pullRef({ number: 3, head: "stack-c", base: "stack-a", url: "u3", draft: false }), ]; const metas = new Map( - pulls.map((pull) => [ - Number(pull.number), - pullMeta({ - number: pull.number, - title: String(pull.head), - body: `body ${pull.head}`, - head: pull.head, - base: pull.base, - url: pull.url, - draft: pull.draft, - state: "OPEN", - labels: [], - }), - ]), - ); - const layer = Stack.layer.pipe( - Layer.provideMerge(Progress.noop), - Layer.provideMerge(cfg), - Layer.provideMerge( - gitAndCodeHost({ - dirty: () => Effect.succeed([]), - fetch: () => Effect.void, - auto: () => Effect.void, - merge: () => Effect.void, - wait: () => Effect.void, - refs: () => - Effect.succeed([ - branchRef({ name: "dev", head: "dev" }), - branchRef({ name: "stack-a", head: "a" }), - branchRef({ name: "stack-b", head: "b" }), - branchRef({ name: "stack-c", head: "c" }), - ]), - changes: () => Effect.succeed(pulls), - change: (pr: number) => Effect.succeed(metas.get(pr)!), - current: () => Effect.succeed("stack-b"), - switch: () => Effect.void, - head: () => Effect.succeed(Option.none()), - base: () => Effect.succeed(Option.none()), - commits: () => Effect.succeed([]), - novel: (_parent, _branch, commits) => Effect.succeed(commits), - replay: () => Effect.void, - backup: () => Effect.void, - drop: () => Effect.void, - restore: () => Effect.void, - push: () => Effect.void, - edit: () => Effect.void, - body: (pr: number, body: string) => Effect.sync(() => void bodies.set(pr, body)), - close: () => Effect.void, - create: () => Effect.fail(new ExecError("gh", ["pr", "create"], 1, "unused")), - }), - ), - Layer.provideMerge( - Store.memory( - new StackState({ - version: 1, - links: [ - stackLink({ branch: "stack-a", parent: "dev", anchor: "dev", pr: 1 }), - stackLink({ branch: "stack-b", parent: "stack-a", anchor: "a", pr: 2 }), - stackLink({ branch: "stack-c", parent: "stack-a", anchor: "a", pr: 3 }), - ], - }), - ), - ), + pulls.map((pull) => [Number(pull.number), metaFor(pull, `body ${pull.head}`)]), ); + const layer = stackTestLayer({ + refs: [ref("dev"), ref("stack-a", "a"), ref("stack-b", "b"), ref("stack-c", "c")], + pulls, + current: "stack-b", + state: stackState([ + stackLink({ branch: "stack-a", parent: "dev", anchor: "dev", pr: 1 }), + stackLink({ branch: "stack-b", parent: "stack-a", anchor: "a", pr: 2 }), + stackLink({ branch: "stack-c", parent: "stack-a", anchor: "a", pr: 3 }), + ]), + service: { + change: (pr: number) => Effect.succeed(metas.get(pr)!), + head: () => Effect.succeed(Option.none()), + remoteHead: () => Effect.succeed(Option.none()), + body: (pr: number, body: string) => Effect.sync(() => void bodies.set(pr, body)), + create: () => Effect.fail(new ExecError("gh", ["pr", "create"], 1, "unused")), + }, + }); return Effect.gen(function* () { const stack = yield* Stack; @@ -3724,8 +3689,10 @@ describe("Stack", () => { }); it.effect("land journals child retargets before a failed root merge", () => { - const test = makeLand([], "stack-a", null, { - merge: () => Effect.fail(new ExecError("gh", ["pr", "merge", "4"], 1, "blocked")), + const test = makeLand({ + codeHost: { + merge: () => Effect.fail(new ExecError("gh", ["pr", "merge", "4"], 1, "blocked")), + }, }); return Effect.gen(function* () { @@ -3768,7 +3735,7 @@ describe("Stack", () => { }); it.effect("land infers the root from the current stack branch", () => { - const test = makeLand([], "stack-c"); + const test = makeLand({ currentBranch: "stack-c" }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4011,7 +3978,7 @@ describe("Stack", () => { }); it.effect("land infers the only stack root when current branch is off-stack", () => { - const test = makeLand([], "dev"); + const test = makeLand({ currentBranch: "dev" }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4043,16 +4010,19 @@ describe("Stack", () => { }); it.effect("land apply releases a clean checked-out target before deleting it", () => { - const test = makeLand([], "dev", null, { - worktrees: () => - Effect.succeed([ - { - path: "/tmp/stack-a-worktree", - head: "stack-a-1", - branch: "stack-a", - dirty: [], - }, - ]), + const test = makeLand({ + currentBranch: "dev", + codeHost: { + worktrees: () => + Effect.succeed([ + { + path: "/tmp/stack-a-worktree", + head: "stack-a-1", + branch: "stack-a", + dirty: [], + }, + ]), + }, }); return Effect.gen(function* () { @@ -4068,16 +4038,19 @@ describe("Stack", () => { }); it.effect("land auto releases a clean checked-out target before deleting it", () => { - const test = makeLand([], "dev", null, { - worktrees: () => - Effect.succeed([ - { - path: "/tmp/stack-a-worktree", - head: "stack-a-1", - branch: "stack-a", - dirty: [], - }, - ]), + const test = makeLand({ + currentBranch: "dev", + codeHost: { + worktrees: () => + Effect.succeed([ + { + path: "/tmp/stack-a-worktree", + head: "stack-a-1", + branch: "stack-a", + dirty: [], + }, + ]), + }, }); return Effect.gen(function* () { @@ -4110,7 +4083,7 @@ describe("Stack", () => { }); it.effect("land auto through stays on the selected stack when another root exists", () => { - const test = makeLand([], "stack-a", null, {}, true); + const test = makeLand({ includeUnrelatedRoot: true }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4127,7 +4100,7 @@ describe("Stack", () => { }); it.effect("land auto through follows the selected sibling branch", () => { - const test = makeLand([], "stack-a", null, {}, false, true); + const test = makeLand({ forkStackC: true }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4189,11 +4162,13 @@ describe("Stack", () => { }); it.effect("land rejects GitLab admin merge before mutation", () => { - const test = makeLand([], "stack-a", null, { - provider: "gitlab", - capabilities: { adminMerge: false }, - requestLabel: "MR", - reference: (number) => `!${number}`, + const test = makeLand({ + codeHost: { + provider: "gitlab", + capabilities: { adminMerge: false }, + requestLabel: "MR", + reference: (number) => `!${number}`, + }, }); return Effect.gen(function* () { @@ -4207,7 +4182,7 @@ describe("Stack", () => { it.effect("land auto emits progress while waiting for merge", () => { const events: Array = []; - const test = makeLand([], "stack-a", events); + const test = makeLand({ progress: events }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4234,81 +4209,30 @@ describe("Stack", () => { it.effect("sync rebases descendants when an older PR branch changes", () => Effect.gen(function* () { - const root = yield* tempDir(); - const origin = join(root, "origin.git"); - const repo = join(root, "repo"); - - yield* shell(root, "git", ["init", "--bare", origin]); - yield* mkdirp(repo); - yield* shell(repo, "git", ["init", "-b", "dev"]); - yield* shell(repo, "git", ["config", "user.email", "stack@example.com"]); - yield* shell(repo, "git", ["config", "user.name", "Stack Test"]); - yield* shell(repo, "git", ["remote", "add", "origin", origin]); - - yield* commitFile(repo, "base.txt", "base\n", "base"); - yield* shell(repo, "git", ["push", "-u", "origin", "dev"]); - const dev = yield* shell(repo, "git", ["rev-parse", "dev"]); - - yield* shell(repo, "git", ["checkout", "-b", "stack-b"]); - yield* commitFile(repo, "b.txt", "b1\n", "b1"); - yield* shell(repo, "git", ["push", "-u", "origin", "stack-b"]); - const oldStackB = yield* shell(repo, "git", ["rev-parse", "stack-b"]); - - yield* shell(repo, "git", ["checkout", "-b", "stack-c"]); - yield* commitFile(repo, "c.txt", "c\n", "c"); - yield* shell(repo, "git", ["push", "-u", "origin", "stack-c"]); + const { repo, layer } = yield* realStack({ + branches: [ + { + name: "stack-b", + parent: "dev", + number: 2, + commits: [{ file: "b.txt", body: "b1\n", message: "b1" }], + }, + { + name: "stack-c", + parent: "stack-b", + number: 3, + commits: [{ file: "c.txt", body: "c\n", message: "c" }], + }, + ], + metas: [pr(2, "stack-b", "dev"), pr(3, "stack-c", "stack-b")].map((pull) => + pullMeta({ ...metaFor(pull, ""), title: `stack: ${pull.head}` }), + ), + }); yield* shell(repo, "git", ["checkout", "stack-b"]); yield* commitFile(repo, "b2.txt", "b2\n", "b2"); yield* shell(repo, "git", ["push", "origin", "stack-b"]); - const cfgLayer = StackConfig.layer({ root: repo, trunks: ["dev"] }).pipe( - Layer.provide(NodeServices.layer), - ); - const layer = Stack.layer.pipe( - Layer.provideMerge(Progress.noop), - Layer.provideMerge(NodeServices.layer), - Layer.provideMerge(Proc.live), - Layer.provideMerge(cfgLayer), - Layer.provideMerge(Git.live.pipe(Layer.provide(cfgLayer))), - Layer.provideMerge( - CodeHostGitHub.memory({ - pulls: [ - pullRef({ - number: 2, - head: "stack-b", - base: "dev", - url: "u2", - draft: false, - }), - pullRef({ - number: 3, - head: "stack-c", - base: "stack-b", - url: "u3", - draft: false, - }), - ], - }), - ), - Layer.provideMerge( - Store.memory( - new StackState({ - version: 1, - links: [ - stackLink({ branch: "stack-b", parent: "dev", anchor: dev, pr: 2 }), - stackLink({ - branch: "stack-c", - parent: "stack-b", - anchor: oldStackB, - pr: 3, - }), - ], - }), - ), - ), - ); - const items = yield* Effect.gen(function* () { const stack = yield* Stack; return yield* stack.sync({ apply: true }); @@ -4549,116 +4473,37 @@ describe("Stack", () => { it.effect( "sync --apply does not re-push a child that is already current when its parent is repaired", () => { - const refs = new Map([ - ["dev", branchRef({ name: "dev", head: "dev-2" })], - ["stack-a", branchRef({ name: "stack-a", head: "stack-a-1" })], - ["stack-b", branchRef({ name: "stack-b", head: "stack-b-1" })], - ]); - const pulls = [ - pullRef({ number: 4, head: "stack-a", base: "dev", url: "u4", draft: false }), - pullRef({ number: 5, head: "stack-b", base: "stack-a", url: "u5", draft: false }), - ]; - const bases = new Map([ - ["stack-a:dev", "dev-1"], - ["stack-a:origin/dev", "dev-1"], - ["stack-b:stack-a", "stack-a-1"], - ]); - const metas = new Map([ - [ - 4, - pullMeta({ - number: 4, - title: "stack-a", - body: "body", - head: "stack-a", - base: "dev", - url: "u4", - draft: false, - state: "OPEN", - labels: [], - }), - ], - [ - 5, - pullMeta({ - number: 5, - title: "stack-b", - body: "body", - head: "stack-b", - base: "stack-a", - url: "u5", - draft: false, - state: "OPEN", - labels: [], - }), - ], - ]); const seen: Array = []; - const layer = Stack.layer.pipe( - Layer.provideMerge(Progress.noop), - Layer.provideMerge(NodeServices.layer), - Layer.provideMerge( - StackConfig.layer({ root: "/tmp/stack", trunks: ["dev"] }).pipe( - Layer.provide(NodeServices.layer), - ), - ), - Layer.provideMerge( - gitAndCodeHost({ - dirty: () => Effect.succeed([]), - worktrees: () => Effect.succeed([]), - fetch: () => Effect.void, - refs: () => Effect.succeed(Array.from(refs.values())), - changes: () => Effect.succeed(pulls), - change: (pr: number) => Effect.succeed(metas.get(pr)!), - current: () => Effect.succeed("stack-b"), - head: (name: string) => - Effect.succeed( - Option.fromNullishOr( - refs.get(name)?.head ?? - (name.startsWith("origin/") ? refs.get(name.slice(7))?.head : undefined), - ), - ), - base: (branch: string, parent: string) => - Effect.succeed(Option.fromNullishOr(bases.get(`${branch}:${parent}`))), - commits: () => Effect.succeed(["x"]), - novel: (_p: string, _b: string, commits: ReadonlyArray) => - Effect.succeed(commits), - backup: (branch: string, name: string) => - Effect.sync(() => void seen.push(`backup ${branch} ${name}`)), - drop: () => Effect.void, - restore: () => Effect.void, - replay: (branch: string, parent: string) => - Effect.sync(() => { - seen.push(`rebase ${branch} ${parent}`); - }), - push: (branch: string) => Effect.sync(() => void seen.push(`push ${branch}`)), - edit: (pr: number, base: string) => - Effect.sync(() => void seen.push(`edit ${pr} ${base}`)), - body: (pr: number, body: string) => - Effect.sync(() => void seen.push(`body ${pr} ${body.includes("### [Stack]")}`)), - close: () => Effect.void, - create: () => - Effect.succeed( - pullRef({ number: 99, head: "x", base: "dev", url: "u", draft: false }), - ), - remote: () => Effect.succeed(Option.some("git@github.com:example/repo.git")), - remotes: () => - Effect.succeed([{ name: "origin", url: "git@github.com:example/repo.git" }]), - }), - ), - Layer.provideMerge( - Store.memory( - new StackState({ - version: 1, - links: [ - stackLink({ branch: "stack-a", parent: "dev", anchor: "dev-1", pr: 4 }), - stackLink({ branch: "stack-b", parent: "stack-a", anchor: "stack-a-1", pr: 5 }), - ], - }), - ), - ), - ); + const layer = stackTestLayer({ + refs: [ref("dev", "dev-2"), ref("stack-a", "stack-a-1"), ref("stack-b", "stack-b-1")], + pulls: [pr(4, "stack-a", "dev"), pr(5, "stack-b", "stack-a")], + bases: bases(["stack-a", "dev", "dev-1"], ["stack-b", "stack-a", "stack-a-1"]), + current: "stack-b", + state: stackState([ + stackLink({ branch: "stack-a", parent: "dev", anchor: "dev-1", pr: 4 }), + stackLink({ branch: "stack-b", parent: "stack-a", anchor: "stack-a-1", pr: 5 }), + ]), + service: { + remoteHead: () => Effect.succeed(Option.none()), + commits: () => Effect.succeed(["x"]), + backup: (branch: string, name: string) => + Effect.sync(() => void seen.push(`backup ${branch} ${name}`)), + // Replaying the parent intentionally leaves its tip unchanged. + replay: (branch: string, parent: string) => + Effect.sync(() => void seen.push(`rebase ${branch} ${parent}`)), + push: (branch: string) => Effect.sync(() => void seen.push(`push ${branch}`)), + edit: (pr: number, base: string) => + Effect.sync(() => void seen.push(`edit ${pr} ${base}`)), + body: (pr: number, body: string) => + Effect.sync(() => void seen.push(`body ${pr} ${body.includes("### [Stack]")}`)), + create: () => + Effect.succeed(pullRef({ number: 99, head: "x", base: "dev", url: "u", draft: false })), + remote: () => Effect.succeed(Option.some("git@github.com:example/repo.git")), + remotes: () => + Effect.succeed([{ name: "origin", url: "git@github.com:example/repo.git" }]), + }, + }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4820,7 +4665,7 @@ describe("Stack", () => { ); it.effect("land apply refuses a dirty worktree before merging", () => { - const test = makeLand([" M foo.ts", "?? scratch/"]); + const test = makeLand({ dirty: [" M foo.ts", "?? scratch/"] }); return Effect.gen(function* () { const stack = yield* Stack; @@ -4838,16 +4683,19 @@ describe("Stack", () => { }); it.effect("land apply refuses a dirty target worktree before merging the root", () => { - const test = makeLand([], "dev", null, { - worktrees: () => - Effect.succeed([ - { - path: "/tmp/stack-a-worktree", - head: "stack-a-1", - branch: "stack-a", - dirty: ["?? dirty.txt"], - }, - ]), + const test = makeLand({ + currentBranch: "dev", + codeHost: { + worktrees: () => + Effect.succeed([ + { + path: "/tmp/stack-a-worktree", + head: "stack-a-1", + branch: "stack-a", + dirty: ["?? dirty.txt"], + }, + ]), + }, }); return Effect.gen(function* () { @@ -4863,16 +4711,19 @@ describe("Stack", () => { }); it.effect("land auto refuses a dirty target worktree before merging the root", () => { - const test = makeLand([], "dev", null, { - worktrees: () => - Effect.succeed([ - { - path: "/tmp/stack-a-worktree", - head: "stack-a-1", - branch: "stack-a", - dirty: ["?? dirty.txt"], - }, - ]), + const test = makeLand({ + currentBranch: "dev", + codeHost: { + worktrees: () => + Effect.succeed([ + { + path: "/tmp/stack-a-worktree", + head: "stack-a-1", + branch: "stack-a", + dirty: ["?? dirty.txt"], + }, + ]), + }, }); return Effect.gen(function* () { @@ -4935,132 +4786,33 @@ describe("Stack", () => { "land repairs descendants in a real git repository", () => Effect.gen(function* () { - const root = yield* tempDir(); - const origin = join(root, "origin.git"); - const repo = join(root, "repo"); - const log: Array = []; - - yield* shell(root, "git", ["init", "--bare", origin]); - yield* mkdirp(repo); - yield* shell(repo, "git", ["init", "-b", "dev"]); - yield* shell(repo, "git", ["config", "user.email", "stack@example.com"]); - yield* shell(repo, "git", ["config", "user.name", "Stack Test"]); - yield* shell(repo, "git", ["remote", "add", "origin", origin]); - - yield* commitFile(repo, "base.txt", "base\n", "base"); - yield* shell(repo, "git", ["push", "-u", "origin", "dev"]); - const dev = yield* shell(repo, "git", ["rev-parse", "dev"]); - - yield* shell(repo, "git", ["checkout", "-b", "stack-a"]); - yield* commitFile(repo, "a.txt", "a\n", "a"); - yield* shell(repo, "git", ["push", "-u", "origin", "stack-a"]); - const stackA = yield* shell(repo, "git", ["rev-parse", "stack-a"]); - - yield* shell(repo, "git", ["checkout", "-b", "stack-b"]); - yield* commitFile(repo, "b.txt", "b\n", "b"); - yield* shell(repo, "git", ["push", "-u", "origin", "stack-b"]); - const stackB = yield* shell(repo, "git", ["rev-parse", "stack-b"]); - - yield* shell(repo, "git", ["checkout", "-b", "stack-c"]); - yield* commitFile(repo, "c.txt", "c\n", "c"); - yield* shell(repo, "git", ["push", "-u", "origin", "stack-c"]); - - const cfgLayer = StackConfig.layer({ root: repo, trunks: ["dev"] }).pipe( - Layer.provide(NodeServices.layer), - ); - const layer = Stack.layer.pipe( - Layer.provideMerge(Progress.noop), - Layer.provideMerge(NodeServices.layer), - Layer.provideMerge(Proc.live), - Layer.provideMerge(cfgLayer), - Layer.provideMerge(Git.live.pipe(Layer.provide(cfgLayer))), - Layer.provideMerge( - integrationGitHub({ - repo, - log, - pulls: [ - pullRef({ - number: 1, - head: "stack-a", - base: "dev", - url: "u1", - draft: false, - }), - pullRef({ - number: 2, - head: "stack-b", - base: "stack-a", - url: "u2", - draft: false, - }), - pullRef({ - number: 3, - head: "stack-c", - base: "stack-b", - url: "u3", - draft: false, - }), - ], - metas: [ - pullMeta({ - number: 1, - title: "stack-a", - body: "Stacked on #0.", - head: "stack-a", - base: "dev", - url: "u1", - draft: false, - state: "OPEN", - labels: [], - }), - pullMeta({ - number: 2, - title: "stack-b", - body: "Stacked on #1.", - head: "stack-b", - base: "stack-a", - url: "u2", - draft: false, - state: "OPEN", - labels: [], - }), - pullMeta({ - number: 3, - title: "stack-c", - body: "Stacked on #2.", - head: "stack-c", - base: "stack-b", - url: "u3", - draft: false, - state: "OPEN", - labels: [], - }), - ], - }), - ), - Layer.provideMerge( - Store.memory( - new StackState({ - version: 1, - links: [ - stackLink({ branch: "stack-a", parent: "dev", anchor: dev, pr: 1 }), - stackLink({ - branch: "stack-b", - parent: "stack-a", - anchor: stackA, - pr: 2, - }), - stackLink({ - branch: "stack-c", - parent: "stack-b", - anchor: stackB, - pr: 3, - }), - ], - }), - ), - ), - ); + const { repo, log, layer } = yield* realStack({ + branches: [ + { + name: "stack-a", + parent: "dev", + number: 1, + commits: [{ file: "a.txt", body: "a\n", message: "a" }], + }, + { + name: "stack-b", + parent: "stack-a", + number: 2, + commits: [{ file: "b.txt", body: "b\n", message: "b" }], + }, + { + name: "stack-c", + parent: "stack-b", + number: 3, + commits: [{ file: "c.txt", body: "c\n", message: "c" }], + }, + ], + metas: [ + metaFor(pr(1, "stack-a", "dev"), "Stacked on #0."), + metaFor(pr(2, "stack-b", "stack-a"), "Stacked on #1."), + metaFor(pr(3, "stack-c", "stack-b"), "Stacked on #2."), + ], + }); const result = yield* Effect.gen(function* () { const stack = yield* Stack; diff --git a/tests/store.test.ts b/tests/store.test.ts new file mode 100644 index 0000000..6500294 --- /dev/null +++ b/tests/store.test.ts @@ -0,0 +1,50 @@ +import { expect, it } from "@effect/vitest"; +import { Effect, Layer } from "effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; +import * as PlatformError from "effect/PlatformError"; +import { stackState, StateError } from "../src/domain/model.ts"; +import { StackConfig } from "../src/services/Config.ts"; +import { Store } from "../src/services/Store.ts"; + +it.effect.each(["missing", "valid", "corrupt", "denied"] as const)( + "Store reads %s files directly without an existence check", + (kind) => + Effect.gen(function* () { + const reads: Array = []; + const fs = FileSystem.layerNoop({ + exists: () => Effect.die("read the file directly"), + readFileString: (file) => + Effect.gen(function* () { + reads.push(file); + if (kind === "valid") return '{"version":1,"links":[]}'; + if (kind === "corrupt") return "invalid json"; + return yield* Effect.fail( + PlatformError.systemError({ + _tag: kind === "missing" ? "NotFound" : "PermissionDenied", + module: "FileSystem", + method: "readFileString", + pathOrDescriptor: file, + }), + ); + }), + }); + const layer = Store.live.pipe( + Layer.provide(StackConfig.layer({ root: "/repo" })), + Layer.provide(fs), + Layer.provide(Path.layer), + ); + yield* Effect.gen(function* () { + const store = yield* Store; + if (kind === "missing" || kind === "valid") { + expect(yield* store.read()).toEqual(stackState([])); + if (kind === "missing") expect(yield* store.readUndo()).toBeNull(); + } else { + const error = yield* Effect.flip(store.read()); + expect(error).toBeInstanceOf(StateError); + expect(String(error)).toContain(kind === "corrupt" ? "decode" : "read"); + } + }).pipe(Effect.provide(layer)); + expect(reads).toHaveLength(kind === "missing" ? 2 : 1); + }), +);