diff --git a/.github/workflows/commit-msg.yml b/.github/workflows/commit-msg.yml index c4147f8..ac9126b 100644 --- a/.github/workflows/commit-msg.yml +++ b/.github/workflows/commit-msg.yml @@ -37,11 +37,13 @@ jobs: set -euo pipefail if [[ "${{ github.event_name }}" == "pull_request" ]]; then - npm exec --yes --package=conventional-commit-msg@${COMMITLINT_PACKAGE_VERSION} -- \ - conventional-commit-msg \ - --from "${{ github.event.pull_request.base.sha }}" \ + commitlint_args=( + --from "${{ github.event.pull_request.base.sha }}" --to "${{ github.event.pull_request.head.sha }}" + ) else - npm exec --yes --package=conventional-commit-msg@${COMMITLINT_PACKAGE_VERSION} -- \ - conventional-commit-msg --last + commitlint_args=(--last) fi + + npm exec --yes --package=conventional-commit-msg@${COMMITLINT_PACKAGE_VERSION} -- \ + conventional-commit-msg "${commitlint_args[@]}" diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 3d948b0..096dc57 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -14,29 +14,24 @@ on: jobs: approach: if: ${{ github.event_name == 'workflow_dispatch' || github.event.action == 'opened' }} - permissions: + permissions: &review-permissions contents: read pull-requests: write actions: read - uses: Jercik/axgithub/.github/workflows/pr-review.yml@v1 + uses: &review-workflow Jercik/axgithub/.github/workflows/pr-review.yml@v1 with: label: approach recipes: '["pr-review-approach-1","pr-review-approach-2","pr-review-approach-3","pr-review-approach-4"]' - pr_number: ${{ github.event.pull_request.number || inputs.pr_number }} - secrets: + pr_number: &review-pr-number ${{ github.event.pull_request.number || inputs.pr_number }} + secrets: &review-secrets NPM_TOKEN: ${{ secrets.NPM_TOKEN }} AXRECIPE_API_KEY: ${{ secrets.AXRECIPE_API_KEY }} code: - permissions: - contents: read - pull-requests: write - actions: read - uses: Jercik/axgithub/.github/workflows/pr-review.yml@v1 + permissions: *review-permissions + uses: *review-workflow with: label: code recipes: '["pr-review-code-1","pr-review-code-2"]' - pr_number: ${{ github.event.pull_request.number || inputs.pr_number }} - secrets: - NPM_TOKEN: ${{ secrets.NPM_TOKEN }} - AXRECIPE_API_KEY: ${{ secrets.AXRECIPE_API_KEY }} + pr_number: *review-pr-number + secrets: *review-secrets diff --git a/.gitignore b/.gitignore index 98a42af..8ad0240 100644 --- a/.gitignore +++ b/.gitignore @@ -141,3 +141,4 @@ dist # Local artifacts .codex/ +.clawpatch diff --git a/src/cli.ts b/src/cli.ts index e72dd13..9c11354 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,16 +1,4 @@ #!/usr/bin/env -S node --experimental-strip-types -/** - * worktree-remove.ts - * - * Remove one or more Git worktrees and/or their directories for given targets. - * Handles registered worktrees, detached HEAD worktrees, and orphaned directories. - * - * This script: - * 1. Resolves the targets (branch/path/directory name) - * 2. Checks if they're registered as Git worktrees - * 3. Unregisters them if needed - * 4. Offers to delete the directories if they exist - */ import { Command } from "@commander-js/extra-typings"; import chalk from "chalk"; @@ -29,15 +17,6 @@ process.on("SIGINT", () => { process.exit(130); }); -interface CliOptions { - interactive?: boolean; - yes?: boolean; - force?: boolean; - dryRun?: boolean; - verbose?: boolean; - quiet?: boolean; -} - const program = new Command() .name(packageJson.name) .description(packageJson.description) @@ -55,7 +34,7 @@ const program = new Command() .option("--quiet", "suppress non-error output") .showHelpAfterError("(add --help for additional information)") .showSuggestionAfterError() - .action(async (targets: string[], options: CliOptions) => { + .action(async (targets, options) => { try { ensureGitAvailable(); diff --git a/src/cli/output-writer.ts b/src/cli/output-writer.ts index 23eaad5..1203e03 100644 --- a/src/cli/output-writer.ts +++ b/src/cli/output-writer.ts @@ -8,9 +8,6 @@ export interface OutputWriter { info: (message: string) => void; warn: (message: string) => void; error: (message: string) => void; - isDryRun: boolean; - isVerbose: boolean; - isQuiet: boolean; } export function prefixOutput(output: OutputWriter, prefix: string): OutputWriter { @@ -21,9 +18,6 @@ export function prefixOutput(output: OutputWriter, prefix: string): OutputWriter info: wrap(output.info), warn: wrap(output.warn), error: wrap(output.error), - isDryRun: output.isDryRun, - isVerbose: output.isVerbose, - isQuiet: output.isQuiet, }; } @@ -56,8 +50,5 @@ export function createOutputWriter(options: OutputWriterOptions): OutputWriter { info, warn, error: writeError, - isDryRun: options.dryRun, - isVerbose: options.verbose, - isQuiet: options.quiet, }; } diff --git a/src/cli/resolve-batch-targets.ts b/src/cli/resolve-batch-targets.ts index 49871b8..dbcca4d 100644 --- a/src/cli/resolve-batch-targets.ts +++ b/src/cli/resolve-batch-targets.ts @@ -11,13 +11,7 @@ import { hasUncommittedChanges } from "../git/check-uncommitted-changes.js"; import { assertRemovalSafe } from "../worktree/assert-removal-safe.js"; import { getRemovalDisplayInfo, type RemovalDisplayInfo } from "../worktree/get-removal-display.js"; import { resolveRemovalTarget } from "../worktree/resolve-removal-target.js"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "../git/worktree-entry.js"; export interface ResolvedTarget { input: string; diff --git a/src/cli/select-worktree.ts b/src/cli/select-worktree.ts index c36ccb3..d27fec0 100644 --- a/src/cli/select-worktree.ts +++ b/src/cli/select-worktree.ts @@ -9,9 +9,6 @@ import { getWorktreeInfo } from "../git/get-worktree-info.js"; import type { OutputWriter } from "./output-writer.js"; import { isPathEqualOrWithin } from "../worktree/is-path-equal-or-within.js"; -const shouldUseColor = process.stderr.isTTY && !process.env.NO_COLOR; -chalk.level = shouldUseColor ? 3 : 0; - export async function selectWorktrees(output: OutputWriter): Promise { const { mainPath, worktrees } = getWorktreeInfo(); const cwd = process.cwd(); diff --git a/src/git/get-worktree-info.ts b/src/git/get-worktree-info.ts index ecd6dfc..3464519 100644 --- a/src/git/get-worktree-info.ts +++ b/src/git/get-worktree-info.ts @@ -5,13 +5,7 @@ import { git, exitWithMessage } from "./git-helpers.js"; import { normalizeGitPath } from "./normalize-git-path.js"; import { parseWorktreeListPorcelain } from "./parse-worktree-list.js"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "./worktree-entry.js"; interface WorktreeInfo { mainPath: string; diff --git a/src/git/git-helpers.ts b/src/git/git-helpers.ts index 181e456..d29b1e4 100644 --- a/src/git/git-helpers.ts +++ b/src/git/git-helpers.ts @@ -2,9 +2,6 @@ import { spawnSync } from "node:child_process"; import * as readline from "node:readline/promises"; import chalk from "chalk"; -const shouldUseColor = process.stderr.isTTY && !process.env.NO_COLOR; -chalk.level = shouldUseColor ? 3 : 0; - function getGitExecutable(): string { const configuredPath = process.env.WORKTREE_REMOVE_GIT_PATH?.trim(); return configuredPath && configuredPath.length > 0 ? configuredPath : "git"; diff --git a/src/git/parse-worktree-list.ts b/src/git/parse-worktree-list.ts index 53c0198..352da0a 100644 --- a/src/git/parse-worktree-list.ts +++ b/src/git/parse-worktree-list.ts @@ -1,11 +1,5 @@ import { normalizeBranchName } from "./git-helpers.js"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "./worktree-entry.js"; interface ParsedWorktreeList { mainPath: string; diff --git a/src/git/worktree-entry.ts b/src/git/worktree-entry.ts new file mode 100644 index 0000000..75b28d5 --- /dev/null +++ b/src/git/worktree-entry.ts @@ -0,0 +1,6 @@ +export interface WorktreeEntry { + path: string; + head: string | undefined; + branch: string | undefined; + isDetached: boolean; +} diff --git a/src/worktree/assert-removal-safe.ts b/src/worktree/assert-removal-safe.ts index f9fb80c..48d7797 100644 --- a/src/worktree/assert-removal-safe.ts +++ b/src/worktree/assert-removal-safe.ts @@ -1,6 +1,6 @@ import { exitWithMessage } from "../git/git-helpers.js"; import { normalizePathKey } from "../fs/normalize-path-key.js"; -import { isPathStrictlyWithin } from "./is-path-equal-or-within.js"; +import { isPathStrictlyWithin } from "./is-path-strictly-within.js"; interface RemovalSafetyInput { targetPath: string; diff --git a/src/worktree/get-removal-display.test.ts b/src/worktree/get-removal-display.test.ts index 3e9594f..88cf571 100644 --- a/src/worktree/get-removal-display.test.ts +++ b/src/worktree/get-removal-display.test.ts @@ -1,12 +1,6 @@ import { describe, it, expect } from "vitest"; import { getRemovalDisplayInfo } from "./get-removal-display.js"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "../git/worktree-entry.js"; const worktreeWithBranch: WorktreeEntry = { path: "/Users/acme/repo-feature", diff --git a/src/worktree/get-removal-display.ts b/src/worktree/get-removal-display.ts index 80364f5..c8c34ea 100644 --- a/src/worktree/get-removal-display.ts +++ b/src/worktree/get-removal-display.ts @@ -1,11 +1,5 @@ import path from "node:path"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "../git/worktree-entry.js"; interface RemovalDisplayInput { cwd: string; diff --git a/src/worktree/is-path-equal-or-within.test.ts b/src/worktree/is-path-equal-or-within.test.ts index 73e5a96..71b6aa5 100644 --- a/src/worktree/is-path-equal-or-within.test.ts +++ b/src/worktree/is-path-equal-or-within.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { isPathEqualOrWithin, isPathStrictlyWithin } from "./is-path-equal-or-within.js"; +import { isPathEqualOrWithin } from "./is-path-equal-or-within.js"; describe("isPathEqualOrWithin", () => { it("returns true for the same POSIX path", () => { @@ -32,21 +32,11 @@ describe("isPathEqualOrWithin", () => { ).not.toBe(true); }); - it("allows names prefixed with '..' that do not traverse upward", () => { - expect( - isPathEqualOrWithin({ - basePath: "/Users/acme/repo-feature", - candidatePath: "/Users/acme/repo-feature/..not-parent/tmp", - platform: "linux", - }), - ).toBe(true); - }); - - it("treats Windows paths case-insensitively", () => { + it("treats equal Windows paths case-insensitively", () => { expect( isPathEqualOrWithin({ basePath: String.raw`C:\Users\Acme\Repo-Feature`, - candidatePath: String.raw`c:\users\acme\repo-feature\src`, + candidatePath: String.raw`c:\users\acme\repo-feature`, platform: "win32", }), ).toBe(true); @@ -66,31 +56,9 @@ describe("isPathEqualOrWithin", () => { expect( isPathEqualOrWithin({ basePath: "repo-feature", - candidatePath: "/Users/acme/repo-feature/src", + candidatePath: "repo-feature", platform: "linux", }), ).not.toBe(true); }); }); - -describe("isPathStrictlyWithin", () => { - it("returns false for equal paths", () => { - expect( - isPathStrictlyWithin({ - basePath: "/Users/acme/repo-feature", - candidatePath: "/Users/acme/repo-feature", - platform: "linux", - }), - ).not.toBe(true); - }); - - it("returns true when the candidate path is inside the base path", () => { - expect( - isPathStrictlyWithin({ - basePath: "/Users/acme/repo-feature", - candidatePath: "/Users/acme/repo-feature/src", - platform: "linux", - }), - ).toBe(true); - }); -}); diff --git a/src/worktree/is-path-equal-or-within.ts b/src/worktree/is-path-equal-or-within.ts index 57c1675..d9ea6e8 100644 --- a/src/worktree/is-path-equal-or-within.ts +++ b/src/worktree/is-path-equal-or-within.ts @@ -1,4 +1,6 @@ import path from "node:path"; +import { normalizePathKey } from "../fs/normalize-path-key.js"; +import { isPathStrictlyWithin } from "./is-path-strictly-within.js"; interface PathContainmentInput { basePath: string; @@ -6,39 +8,14 @@ interface PathContainmentInput { platform: NodeJS.Platform; } -function isPathWithin(input: PathContainmentInput, includeEqual: boolean): boolean { +export function isPathEqualOrWithin(input: PathContainmentInput): boolean { const pathApi = input.platform === "win32" ? path.win32 : path.posix; - // This helper is designed for already-resolved absolute paths. - // Forcing absolute inputs avoids host/target path-API mismatches such as - // `path.win32.resolve` behavior on POSIX hosts for relative values. - if (!pathApi.isAbsolute(input.basePath)) { - return false; - } - if (!pathApi.isAbsolute(input.candidatePath)) { + // Require absolute inputs so the equality branch upholds the same contract as isPathStrictlyWithin. + if (!pathApi.isAbsolute(input.basePath) || !pathApi.isAbsolute(input.candidatePath)) { return false; } - - const relative = pathApi.relative( - pathApi.resolve(input.basePath), - pathApi.resolve(input.candidatePath), + return ( + normalizePathKey(input.basePath, input.platform) === + normalizePathKey(input.candidatePath, input.platform) || isPathStrictlyWithin(input) ); - - if (relative === "") { - return includeEqual; - } - if (pathApi.isAbsolute(relative)) { - return false; - } - if (relative === ".." || relative.startsWith(`..${pathApi.sep}`)) { - return false; - } - return true; -} - -export function isPathEqualOrWithin(input: PathContainmentInput): boolean { - return isPathWithin(input, true); -} - -export function isPathStrictlyWithin(input: PathContainmentInput): boolean { - return isPathWithin(input, false); } diff --git a/src/worktree/is-path-strictly-within.test.ts b/src/worktree/is-path-strictly-within.test.ts new file mode 100644 index 0000000..416e8c2 --- /dev/null +++ b/src/worktree/is-path-strictly-within.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest"; +import { isPathStrictlyWithin } from "./is-path-strictly-within.js"; + +describe("isPathStrictlyWithin", () => { + it("returns false for equal paths", () => { + expect( + isPathStrictlyWithin({ + basePath: "/Users/acme/repo-feature", + candidatePath: "/Users/acme/repo-feature", + platform: "linux", + }), + ).not.toBe(true); + }); + + it("returns true when the candidate path is inside the base path", () => { + expect( + isPathStrictlyWithin({ + basePath: "/Users/acme/repo-feature", + candidatePath: "/Users/acme/repo-feature/src", + platform: "linux", + }), + ).toBe(true); + }); + + it("returns false when the candidate path is outside the base path", () => { + expect( + isPathStrictlyWithin({ + basePath: "/Users/acme/repo-feature", + candidatePath: "/Users/acme/repo-main", + platform: "linux", + }), + ).not.toBe(true); + }); + + it("allows names prefixed with '..' that do not traverse upward", () => { + expect( + isPathStrictlyWithin({ + basePath: "/Users/acme/repo-feature", + candidatePath: "/Users/acme/repo-feature/..not-parent/tmp", + platform: "linux", + }), + ).toBe(true); + }); + + it("treats Windows paths case-insensitively", () => { + expect( + isPathStrictlyWithin({ + basePath: String.raw`C:\Users\Acme\Repo-Feature`, + candidatePath: String.raw`c:\users\acme\repo-feature\src`, + platform: "win32", + }), + ).toBe(true); + }); + + it("returns false for Windows paths on another drive", () => { + expect( + isPathStrictlyWithin({ + basePath: String.raw`C:\Users\Acme\Repo-Feature`, + candidatePath: String.raw`D:\Users\Acme\Repo-Feature\src`, + platform: "win32", + }), + ).not.toBe(true); + }); + + it("returns false when one of the paths is relative", () => { + expect( + isPathStrictlyWithin({ + basePath: "repo-feature", + candidatePath: "/Users/acme/repo-feature/src", + platform: "linux", + }), + ).not.toBe(true); + }); +}); diff --git a/src/worktree/is-path-strictly-within.ts b/src/worktree/is-path-strictly-within.ts new file mode 100644 index 0000000..c84c42a --- /dev/null +++ b/src/worktree/is-path-strictly-within.ts @@ -0,0 +1,34 @@ +import path from "node:path"; + +interface PathContainmentInput { + basePath: string; + candidatePath: string; + platform: NodeJS.Platform; +} + +export function isPathStrictlyWithin(input: PathContainmentInput): boolean { + const pathApi = input.platform === "win32" ? path.win32 : path.posix; + // Require absolute inputs: path.win32.resolve mishandles relative paths on POSIX hosts. + if (!pathApi.isAbsolute(input.basePath)) { + return false; + } + if (!pathApi.isAbsolute(input.candidatePath)) { + return false; + } + + const relative = pathApi.relative( + pathApi.resolve(input.basePath), + pathApi.resolve(input.candidatePath), + ); + + if (relative === "") { + return false; + } + if (pathApi.isAbsolute(relative)) { + return false; + } + if (relative === ".." || relative.startsWith(`..${pathApi.sep}`)) { + return false; + } + return true; +} diff --git a/src/worktree/resolve-removal-target.ts b/src/worktree/resolve-removal-target.ts index 592f3a3..caa9977 100644 --- a/src/worktree/resolve-removal-target.ts +++ b/src/worktree/resolve-removal-target.ts @@ -2,13 +2,7 @@ import path from "node:path"; import { exitWithMessage } from "../git/git-helpers.js"; import { directoryExists } from "../fs/check-directory-exists.js"; import { resolveWorktreeTarget } from "./resolve-worktree-target.js"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "../git/worktree-entry.js"; interface ResolveRemovalTargetInput { input: string; diff --git a/src/worktree/resolve-worktree-target.ts b/src/worktree/resolve-worktree-target.ts index 757eba5..216e208 100644 --- a/src/worktree/resolve-worktree-target.ts +++ b/src/worktree/resolve-worktree-target.ts @@ -3,13 +3,7 @@ import path from "node:path"; import { normalizeBranchName } from "../git/git-helpers.js"; import { normalizeGitPath } from "../git/normalize-git-path.js"; import { normalizePathKey } from "../fs/normalize-path-key.js"; - -interface WorktreeEntry { - path: string; - head: string | undefined; - branch: string | undefined; - isDetached: boolean; -} +import type { WorktreeEntry } from "../git/worktree-entry.js"; interface ResolveWorktreeTargetInput { input: string;