address review comments

Author: skarim

cmd/unstack.go

@@ -72,7 +72,8 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error {
 	// GitHub by number (remote-first), so this works from anywhere in the
 	// repository whether or not the stack is tracked locally.
 	if opts.stackNumber > 0 {
-		result, ok, err := lookupStackByNumber(cfg, opts.stackNumber)
+		// --local must never contact GitHub, so it uses a strictly local lookup
+		result, ok, err := lookupStackByNumber(cfg, opts.stackNumber, !opts.local)
 		if err != nil {
 			return ErrNotInStack
 		}

cmd/unstack_test.go

@@ -632,6 +632,55 @@ func TestUnstack_ByStackNumber_NotTracked_LocalFlag(t *testing.T) {
 	require.Len(t, sf.Stacks, 1)
 }
 
+func TestUnstack_ByStackNumber_LocalFlag_LegacyStack_NoRemoteCall(t *testing.T) {
+	// --local must never contact GitHub. A legacy stack (Number == 0) can only
+	// be matched by number via a remote backfill (ListStacks); under --local
+	// that lookup must be skipped and the number reported as not tracked.
+	gitDir := t.TempDir()
+	restore := git.SetOps(&git.MockOps{
+		GitDirFn:        func() (string, error) { return gitDir, nil },
+		CurrentBranchFn: func() (string, error) { return "b1", nil },
+	})
+	defer restore()
+
+	// Legacy: internal ID present, Number unset (0).
+	writeStackFile(t, gitDir, stack.Stack{
+		ID:    "99",
+		Trunk: stack.BranchRef{Branch: "main"},
+		Branches: []stack.BranchRef{
+			{Branch: "b1", PullRequest: &stack.PullRequestRef{Number: 101}},
+			{Branch: "b2", PullRequest: &stack.PullRequestRef{Number: 102}},
+		},
+	})
+
+	listCalled := false
+	unstackCalled := false
+	cfg, outR, errR := config.NewTestConfig()
+	cfg.GitHubClientOverride = &github.MockClient{
+		ListStacksFn: func() ([]github.RemoteStack, error) {
+			listCalled = true
+			return []github.RemoteStack{{ID: 99, Number: 7, PullRequests: []int{101, 102}}}, nil
+		},
+		UnstackFn: func(int) (*github.RemoteStack, bool, error) {
+			unstackCalled = true
+			return nil, true, nil
+		},
+	}
+
+	err := runUnstack(cfg, &unstackOptions{stackNumber: 7, local: true})
+	output := collectOutput(cfg, outR, errR)
+
+	assert.ErrorIs(t, err, ErrNotInStack)
+	assert.False(t, listCalled, "--local must not contact GitHub (no ListStacks backfill)")
+	assert.False(t, unstackCalled, "--local must not contact GitHub")
+	assert.Contains(t, output, "stack #7 is not tracked locally")
+
+	// Local tracking is untouched.
+	sf, loadErr := stack.Load(gitDir)
+	require.NoError(t, loadErr)
+	require.Len(t, sf.Stacks, 1)
+}
+
 func TestUnstack_ByStackNumber_LegacyStackResolvedByID(t *testing.T) {
 	// A stack tracked before the number was recorded (Number == 0) is resolved
 	// by mapping its internal ID to the remote stack number, and the backfilled

cmd/utils.go

@@ -269,8 +269,11 @@ func loadStack(cfg *config.Config, branch string) (*loadStackResult, error) {
 //
 // Stack files created before the number was tracked store only the internal ID
 // (Number == 0); such legacy stacks are resolved by mapping their ID to a remote
-// stack number so they can still be targeted by number.
-func lookupStackByNumber(cfg *config.Config, number int) (result *loadStackResult, ok bool, err error) {
+// stack number so they can still be targeted by number. That mapping contacts
+// GitHub (ListStacks), so it is only attempted when allowRemote is true — callers
+// that must stay purely local (e.g. `--local`) pass false, and legacy stacks
+// whose number isn't recorded locally are reported as not tracked.
+func lookupStackByNumber(cfg *config.Config, number int, allowRemote bool) (result *loadStackResult, ok bool, err error) {
 	gitDir, err := git.GitDir()
 	if err != nil {
 		// Not a git repository — nothing can be tracked locally.
@@ -290,8 +293,9 @@ func lookupStackByNumber(cfg *config.Config, number int) (result *loadStackResul
 
 	// No direct match — backfill legacy stacks' numbers from the remote and
 	// retry, so `gh stack unstack <number>` also works for stacks tracked
-	// before the number was recorded locally.
-	if backfillLegacyStackNumbers(cfg, sf, gitDir) {
+	// before the number was recorded locally. This reaches GitHub, so it is
+	// skipped when the caller requires a purely local lookup.
+	if allowRemote && backfillLegacyStackNumbers(cfg, sf, gitDir) {
 		if result := stackResultByNumber(sf, gitDir, number); result != nil {
 			return result, true, nil
 		}

docs/src/content/docs/reference/cli.md

@@ -258,7 +258,7 @@ This is useful when you need to restructure a stack — remove a branch, insert
 # Unstack the current stack on GitHub and remove local tracking
 gh stack unstack
 
-# Unstack a specific stack by its numbe
+# Unstack a specific stack by its number
 gh stack unstack 7
 
 # Only remove local tracking