feat(cli): add nn w attach subcommand for existing branches by nshen · Pull Request #11 · nshen/nn-stack

nshen · 2026-04-18T17:17:16Z

Attach a worktree to an existing local or remote branch and populate
state.pr via gh lookup so /nn-dev can resume review polling. Extracts
showExitPrompt + abnormal-exit handling into _shared.ts for reuse
between create and attach.

Attach a worktree to an existing local or remote branch and populate state.pr via gh lookup so /nn-dev can resume review polling. Extracts showExitPrompt + abnormal-exit handling into _shared.ts for reuse between create and attach.

Copilot

Pull request overview

Adds a new nn w attach CLI subcommand to attach a worktree to an existing branch (local or remote) and populate state.pr via gh PR lookup, while refactoring shared subshell-exit + post-exit prompting logic for reuse across worktree commands.

Changes:

Add nn w attach <branch> command with --as, --pr, and --print-path options.
Extract subshell abnormal-exit handling + exit prompt into commands/worktree/_shared.ts.
Add GitHub PR lookup helper (lib/gh.ts) and git helpers for remote-branch checks/fetching (lib/git.ts).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
reviews/nn-attach-review-20260419-005541.md	Adds an internal review report describing findings/fixes for the attach feature.
packages/cli/src/cli.ts	Registers the new `nn w attach` command and validates `--pr` input.
packages/cli/src/commands/worktree/create.ts	Uses shared subshell-exit handling instead of inlined logic.
packages/cli/src/commands/worktree/_shared.ts	New shared exit prompt + abnormal subshell-exit handling used by create/attach.
packages/cli/src/commands/worktree/attach.ts	New attach implementation: resolves branch, creates/resumes worktree, looks up PR, writes state, enters subshell.
packages/cli/src/lib/gh.ts	New `gh pr list`-based PR lookup returning a discriminated result.
packages/cli/src/lib/git.ts	Adds `remoteBranchExists()` and `fetchBranch()` used by attach.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T17:22:14Z

+    await $`git show-ref --verify --quiet refs/remotes/${remote}/${name}`
+    return true


remoteBranchExists() currently checks for a local remote-tracking ref (refs/remotes/<remote>/<name>). This does not actually answer whether the branch exists on the remote, so attach can incorrectly error out if the user hasn't fetched recently (even though the branch exists on the server). Either (a) switch to git ls-remote --heads <remote> <name> (or refs/heads/<name>) for a true remote check, or (b) remove this pre-check and just attempt git fetch and handle the error message.

Suggested change

await $`git show-ref --verify --quiet refs/remotes/${remote}/${name}`

return true

const out = (

await $`git ls-remote --heads ${remote} ${`refs/heads/${name}`}`

).stdout.trim()

return out.length > 0

Copilot · 2026-04-18T17:22:14Z


+w.command('attach')
+  .description('Attach a worktree to an existing branch (and its PR)')
+  .argument('<branch>', 'existing branch name (local or origin/<branch>)')


The attach command help says the <branch> argument can be origin/<branch>, but the implementation treats the value as an unqualified branch name and will fail for inputs like origin/foo. Either adjust the help text to match actual supported input, or update the implementation to accept/normalize origin/<branch> (and ideally <remote>/<branch>).

Suggested change

.argument('<branch>', 'existing branch name (local or origin/<branch>)')

.argument('<branch>', 'existing local branch name')

Copilot · 2026-04-18T17:22:14Z

+    // Resolve branch: local first, then origin/<branch>.
+    const hasLocal = await branchExists(branch)
+    if (!hasLocal) {
+      const hasRemote = await remoteBranchExists(branch)
+      if (!hasRemote) {
+        console.error(
+          `Branch "${branch}" not found locally or on origin. Fetch it first, or use "nn w <name>" to create a new branch.`,
+        )
+        process.exit(1)
+      }
+      try {
+        await fetchBranch('origin', branch)
+      } catch (err) {
+        const stderr =
+          err && typeof err === 'object' && 'stderr' in err
+            ? String(err.stderr).trim()
+            : String(err)
+        console.error(`Failed to fetch branch: ${stderr}`)
+        process.exit(1)
+      }
+    }
+
+    await fs.mkdir(path.dirname(wtPath), { recursive: true })
+
+    try {
+      await addWorktreeExisting(wtPath, branch)


branch is documented as accepting origin/<branch> (see CLI help), but this code passes the raw string into remoteBranchExists(), fetchBranch(), and addWorktreeExisting(). If the user provides origin/foo, remoteBranchExists() checks refs/remotes/origin/origin/foo and fetch tries to fetch origin origin/foo:origin/foo, so attach will fail. Normalize inputs (e.g., strip a leading origin/ or parse <remote>/<branch>), and ensure all downstream git/gh calls use the normalized branch name.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T17:29:24Z

+}
+
+export async function fetchBranch(remote: string, branch: string) {
+  await $`git fetch ${remote} ${branch}:${branch}`


When the branch doesn’t exist locally, fetchBranch() creates/updates the local branch via git fetch <remote> <branch>:<branch>, but it does not set upstream tracking to <remote>/<branch>. As a result, aheadBehind() will report hasUpstream: false in the attached worktree, and the exit prompt / rm safeguards will treat the branch as “never pushed” even though it came from the remote. Consider setting the upstream after fetching (or creating the worktree/branch with tracking in one step) so the attached branch behaves like a normal tracked branch.

Suggested change

await $`git fetch ${remote} ${branch}:${branch}`

const root = await getRepoRoot()

await $`git -C ${root} fetch ${remote} ${branch}:${branch}`

await $`git -C ${root} branch --set-upstream-to=${remote}/${branch} ${branch}`

Agree with the concern — applied a different fix in 224f3df because the suggested patch wouldn't actually work: git fetch origin branch:branch with an explicit refspec does not update refs/remotes/origin/branch, so --set-upstream-to=origin/branch would fail with 'requested upstream branch does not exist'. Fixed it with plain git fetch origin <branch> (which updates the remote-tracking ref via the configured refspec) followed by git branch --track <branch> origin/<branch>.

Copilot · 2026-04-18T17:29:25Z

+  try {
+    const out = (
+      await $`git ls-remote --heads ${remote} ${`refs/heads/${name}`}`
+    ).stdout.trim()
+    return out.length > 0
+  } catch {
+    return false
+  }


remoteBranchExists() returns false for any failure (e.g., no network, auth issues, misconfigured remote), which makes callers treat operational errors as “branch doesn’t exist” and emit a misleading message. It would be better to surface the underlying failure (or return a discriminated result like the gh PR lookup) so attach can distinguish “remote unavailable” from “branch not found”.

Suggested change

try {

const out = (

await $`git ls-remote --heads ${remote} ${`refs/heads/${name}`}`

).stdout.trim()

return out.length > 0

} catch {

return false

}

const out = (

await $`git ls-remote --heads ${remote} ${`refs/heads/${name}`}`

).stdout.trim()

return out.length > 0

Applied in 224f3df. Removed the try/catch from remoteBranchExists so genuine failures (network, auth, bad remote config) surface, and added a scoped try/catch at the call site in attach.ts that prints stderr — mirroring how fetchBranch errors are already handled.

- branchExists: use `show-ref --verify` instead of `branch --list` to avoid glob expansion - attach: verify-before-strip `origin/` prefix so legitimate `origin/foo` branches survive - attach: print "Updated PR: #N" when resuming with --pr so state mutation isn't silent - remove stale reviews/ artifact

- fetchBranch: also set upstream tracking so aheadBehind/exit-prompt safeguards work correctly on attached branches. Use plain `git fetch <remote> <branch>` (which updates refs/remotes via the default refspec) instead of the explicit `<branch>:<branch>` refspec that would leave --set-upstream-to with no remote ref to point at. - remoteBranchExists: let errors propagate instead of swallowing them as "branch not found"; add a scoped try/catch at the call site in attach.ts. - origin/ verify-before-strip: drop the remote probe (too fragile under network failure) — local-only check is enough to protect legit `origin/foo` branches.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T19:04:10Z

+    // Worktree already exists — resume.
+    isResume = true
+    resolvedBranch = found.branch?.replace('refs/heads/', '') ?? null


When the target worktree path already exists, this command unconditionally treats it as a resume and ignores the requested . This can be surprising (and potentially dangerous) when --as points at an existing worktree on a different branch: resolvedBranch will be whatever is currently checked out, not the branch the user asked for. Consider validating that the existing worktree’s branch matches the requested branch (after the optional origin/ normalization) and erroring/warning when they differ, or require an explicit --resume flag to enter an existing worktree.

Suggested change

// Worktree already exists — resume.

isResume = true

resolvedBranch = found.branch?.replace('refs/heads/', '') ?? null

// Worktree already exists — only resume if it matches the requested branch.

resolvedBranch = found.branch?.replace('refs/heads/', '') ?? null

if (resolvedBranch !== branch) {

const actual = resolvedBranch ?? '(detached HEAD)'

console.error(

`Worktree already exists at "${wtPath}" but is on "${actual}", not requested branch "${branch}". Use a different --as name or switch the existing worktree to the requested branch first.`,

)

process.exit(1)

}

isResume = true

Copilot · 2026-04-18T19:04:11Z

+        prNumber = lookup.pr
+      } else if (!opts.printPath) {
+        if (lookup.kind === 'unavailable') {
+          console.log('note: gh not available — skipping PR lookup')


This log message treats any getPrForBranch failure as “gh not available”, but getPrForBranch currently returns kind: 'unavailable' for all errors (not installed, auth failure, network, non-GitHub remote, etc). Consider changing the message to something less specific (e.g. “PR lookup failed”) or returning a more detailed reason from getPrForBranch so the CLI can print actionable guidance (install/authenticate gh).

Suggested change

console.log('note: gh not available — skipping PR lookup')

console.log('note: PR lookup failed — skipping PR lookup')

- attach: error out when resume target worktree is on a different branch than requested, instead of silently ignoring the requested branch - attach: rename misleading "gh not available" message to "PR lookup failed"

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T19:21:55Z

+  // Resolve PR number.
+  let prNumber: number | null = null
+  if (!isResume || opts.pr !== undefined) {
+    if (opts.pr !== undefined) {
+      prNumber = opts.pr
+    } else {
+      const lookup = await getPrForBranch(branch)
+      if (lookup.kind === 'found') {
+        prNumber = lookup.pr
+      } else if (!opts.printPath) {
+        if (lookup.kind === 'unavailable') {
+          console.log('note: PR lookup failed — skipping PR lookup')
+        } else {
+          console.log(`note: no open PR found for "${branch}"`)
+        }
+      }
+    }
+  }
+
+  if (!found) {
+    const { root } = await getRepoInfo()
+    const state: NNState = {
+      name,
+      branch: resolvedBranch ?? branch,
+      repo: root,
+      createdAt: new Date().toISOString(),
+      pr: prNumber,
+      lastReviewId: null,
+    }
+    await writeState(wtPath, state)
+
+    if (!opts.printPath) {
+      console.log(`Attached worktree: ${wtPath}`)
+      console.log(`  branch: ${resolvedBranch ?? branch}`)
+      if (prNumber !== null) console.log(`  PR:     #${prNumber}`)
+    }
+  } else if (opts.pr !== undefined) {
+    // Resume with explicit --pr: update state.pr only.
+    const prev = await readState(wtPath)
+    if (prev) {
+      await writeState(wtPath, { ...prev, pr: prNumber })
+      if (!opts.printPath && prNumber !== null) {
+        console.log(`Updated PR: #${prNumber}`)
+      }
+    } else if (!opts.printPath) {
+      console.log(`note: state file missing — --pr ${prNumber} not persisted`)


In the resume path (found worktree), PR lookup is skipped unless --pr is provided (if (!isResume || opts.pr !== undefined)). This means running nn w attach <branch> on an existing worktree won’t repopulate state.pr via gh (and if the state file is missing, nothing is written at all), which undermines the goal of restoring review polling state. Consider: on resume, read existing state and (a) if it’s missing, create a new state file, and/or (b) if state.pr is null/absent, run getPrForBranch() and persist the result.

Suggested change

// Resolve PR number.

let prNumber: number | null = null

if (!isResume || opts.pr !== undefined) {

if (opts.pr !== undefined) {

prNumber = opts.pr

} else {

const lookup = await getPrForBranch(branch)

if (lookup.kind === 'found') {

prNumber = lookup.pr

} else if (!opts.printPath) {

if (lookup.kind === 'unavailable') {

console.log('note: PR lookup failed — skipping PR lookup')

} else {

console.log(`note: no open PR found for "${branch}"`)

}

}

}

}

if (!found) {

const { root } = await getRepoInfo()

const state: NNState = {

name,

branch: resolvedBranch ?? branch,

repo: root,

createdAt: new Date().toISOString(),

pr: prNumber,

lastReviewId: null,

}

await writeState(wtPath, state)

if (!opts.printPath) {

console.log(`Attached worktree: ${wtPath}`)

console.log(` branch: ${resolvedBranch ?? branch}`)

if (prNumber !== null) console.log(` PR: #${prNumber}`)

}

} else if (opts.pr !== undefined) {

// Resume with explicit --pr: update state.pr only.

const prev = await readState(wtPath)

if (prev) {

await writeState(wtPath, { ...prev, pr: prNumber })

if (!opts.printPath && prNumber !== null) {

console.log(`Updated PR: #${prNumber}`)

}

} else if (!opts.printPath) {

console.log(`note: state file missing — --pr ${prNumber} not persisted`)

const prevState = found ? await readState(wtPath) : null

// Resolve PR number.

let prNumber: number | null = prevState?.pr ?? null

if (opts.pr !== undefined) {

prNumber = opts.pr

} else if (!isResume || prevState?.pr == null) {

const lookup = await getPrForBranch(branch)

if (lookup.kind === 'found') {

prNumber = lookup.pr

} else if (!opts.printPath) {

if (lookup.kind === 'unavailable') {

console.log('note: PR lookup failed — skipping PR lookup')

} else {

console.log(`note: no open PR found for "${branch}"`)

}

}

}

const shouldCreateResumeState = found && !prevState

const shouldUpdateResumePr =

found && !!prevState && prevState.pr !== prNumber && (opts.pr !== undefined || prevState.pr == null)

if (!found || shouldCreateResumeState) {

const { root } = await getRepoInfo()

const state: NNState = prevState ?? {

name,

branch: resolvedBranch ?? branch,

repo: root,

createdAt: new Date().toISOString(),

pr: null,

lastReviewId: null,

}

await writeState(wtPath, {

...state,

name,

branch: resolvedBranch ?? branch,

repo: state.repo ?? root,

pr: prNumber,

})

if (!found && !opts.printPath) {

console.log(`Attached worktree: ${wtPath}`)

console.log(` branch: ${resolvedBranch ?? branch}`)

if (prNumber !== null) console.log(` PR: #${prNumber}`)

}

} else if (shouldUpdateResumePr) {

await writeState(wtPath, { ...prevState, pr: prNumber })

if (!opts.printPath && prNumber !== null) {

console.log(`Updated PR: #${prNumber}`)

Agree with the concern — applied the same fix (resume now reads prior state, recreates it when missing, and reruns PR lookup when state.pr is null) but with two deviations: (1) added an explicit Restored state: <path> log when recreating the state file so silent recovery is visible, and (2) the resume-update condition is simplified to prevState.pr !== prNumber rather than the compound gate — if prNumber changed it came from --pr or a successful lookup, so writing it is always the right move.

attach on resume: read prior state, recreate it when missing, and re-run PR lookup when state.pr is null — so re-attaching can recover a PR number that wasn't persisted earlier and worktrees imported from outside nn get a state file.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T19:29:23Z

+      prNumber = lookup.pr
+    } else if (!opts.printPath) {
+      if (lookup.kind === 'unavailable') {
+        console.log('note: PR lookup failed — skipping PR lookup')


Message note: PR lookup failed — skipping PR lookup is redundant (repeats “PR lookup”), which makes CLI output noisier than necessary. Consider shortening to a single mention of the failure (e.g., “note: PR lookup failed — skipping”).

Suggested change

console.log('note: PR lookup failed — skipping PR lookup')

console.log('note: PR lookup failed — skipping')

Copilot · 2026-04-18T19:29:23Z

+  // Accept "origin/foo" as a convenience when users paste from `git branch -r`.
+  // Only strip the prefix if no local branch exists with that literal name —
+  // this preserves legitimate local branches named "origin/foo".
+  let branch = rawBranch
+  if (rawBranch.startsWith('origin/') && !(await branchExists(rawBranch))) {
+    branch = rawBranch.slice('origin/'.length)


attachCommand normalizes origin/<branch> input, but not other common ref formats like refs/heads/<branch> (e.g. from git worktree list --porcelain). Passing refs/heads/foo will currently look for refs/heads/refs/heads/foo and fail even if the branch exists. Consider stripping a leading refs/heads/ (and possibly refs/remotes/origin/) in the same normalization block before proceeding.

Suggested change

// Accept "origin/foo" as a convenience when users paste from `git branch -r`.

// Only strip the prefix if no local branch exists with that literal name —

// this preserves legitimate local branches named "origin/foo".

let branch = rawBranch

if (rawBranch.startsWith('origin/') && !(await branchExists(rawBranch))) {

branch = rawBranch.slice('origin/'.length)

// Accept common ref formats as a convenience when users paste from Git output.

// Only strip a prefix if no local branch exists with that literal name —

// this preserves legitimate local branches named like "origin/foo" or

// "refs/heads/foo".

let branch = rawBranch

if (!(await branchExists(rawBranch))) {

if (rawBranch.startsWith('refs/heads/')) {

branch = rawBranch.slice('refs/heads/'.length)

} else if (rawBranch.startsWith('refs/remotes/origin/')) {

branch = rawBranch.slice('refs/remotes/origin/'.length)

} else if (rawBranch.startsWith('origin/')) {

branch = rawBranch.slice('origin/'.length)

}

Keeping as-is: the convenience layer targets the common origin/<branch> case that users paste from git branch -r. Raw refs/heads/... and refs/remotes/... pastes are rare in practice (users type short names or copy from git branch), and each added prefix makes it harder to address a legitimate local branch literally named with that prefix. Revisit if we get user reports of the refs/heads/ case.

Copilot · 2026-04-18T19:29:23Z

+export async function fetchBranch(remote: string, branch: string) {
+  const root = await getRepoRoot()
+  // Plain `git fetch <remote> <branch>` uses the configured refspec and updates
+  // refs/remotes/<remote>/<branch>; an explicit `<branch>:<branch>` refspec
+  // would skip that, leaving --set-upstream-to with no remote-tracking ref.
+  await $`git -C ${root} fetch ${remote} ${branch}`
+  await $`git -C ${root} branch --track ${branch} ${`${remote}/${branch}`}`
 }



fetchBranch() now does more than fetching: it also creates a local branch and sets upstream tracking via git branch --track. The name implies a network-only operation, and callers (and their error handling/messages) may become misleading if the tracking step fails. Consider renaming to something like fetchAndTrackBranch, or split it into fetchRemoteBranch() + trackBranch() so errors can be reported accurately.

Suggested change

export async function fetchBranch(remote: string, branch: string) {

const root = await getRepoRoot()

// Plain `git fetch <remote> <branch>` uses the configured refspec and updates

// refs/remotes/<remote>/<branch>; an explicit `<branch>:<branch>` refspec

// would skip that, leaving --set-upstream-to with no remote-tracking ref.

await $`git -C ${root} fetch ${remote} ${branch}`

await $`git -C ${root} branch --track ${branch} ${`${remote}/${branch}`}`

}

export async function fetchRemoteBranch(remote: string, branch: string) {

const root = await getRepoRoot()

// Plain `git fetch <remote> <branch>` uses the configured refspec and updates

// refs/remotes/<remote>/<branch>; an explicit `<branch>:<branch>` refspec

// would skip that, leaving branch tracking setup with no remote-tracking ref.

await $`git -C ${root} fetch ${remote} ${branch}`

}

export async function trackBranch(remote: string, branch: string) {

const root = await getRepoRoot()

await $`git -C ${root} branch --track ${branch} ${`${remote}/${branch}`}`

}

export async function fetchAndTrackBranch(remote: string, branch: string) {

await fetchRemoteBranch(remote, branch)

await trackBranch(remote, branch)

}

export async function fetchBranch(remote: string, branch: string) {

await fetchAndTrackBranch(remote, branch)

}

Agree with the concern — renamed fetchBranch to fetchAndTrackBranch in 6f33ada's follow-up so the name matches the operations. Didn't split into three functions (fetchRemoteBranch + trackBranch + alias) because there's only one caller; the single renamed function keeps the surface tight. If a future caller needs fetch-only or track-only, we can split then.

Copilot · 2026-04-18T19:29:23Z

+          err && typeof err === 'object' && 'stderr' in err
+            ? String(err.stderr).trim()
+            : String(err)
+        console.error(`Failed to fetch branch: ${stderr}`)


The try/catch around fetchBranch() reports any failure as Failed to fetch branch, but fetchBranch() can also fail during the git branch --track step (e.g., if the remote-tracking ref wasn’t created due to a nonstandard fetch refspec). This can mislead users while debugging. Consider improving the error message (e.g., “Failed to fetch/track branch”) or catching/labeling fetch vs tracking failures separately.

Suggested change

console.error(`Failed to fetch branch: ${stderr}`)

console.error(`Failed to fetch/track branch: ${stderr}`)

- rename fetchBranch to fetchAndTrackBranch to reflect that it also creates the local branch and sets upstream tracking - relabel the attach-side error to "Failed to fetch/track branch" so a failure during the tracking step isn't mis-reported as a fetch failure - shorten redundant "note: PR lookup failed — skipping PR lookup" to "note: PR lookup failed — skipping"

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nshen requested a review from Copilot April 18, 2026 17:18

Copilot started reviewing on behalf of nshen April 18, 2026 17:19 View session