address review comments

Author: skarim

cmd/checkout.go

@@ -248,9 +248,11 @@ func checkoutRemoteStack(cfg *config.Config, sf *stack.StackFile, gitDir string,
 		// Case A: branch is in a local stack — check composition
 		if stackCompositionMatches(localStack, remoteStack.PRNumbers()) {
 			// Composition matches — checkout
-			if localStack.ID == "" {
-				localStack.ID = remoteStackID
-			}
+			// remoteStack is authoritative for both identifiers here, so
+			// refresh them together. Updating only one (e.g. the number while
+			// keeping a stale ID) breaks later ID-based discovery when the old
+			// remote stack was replaced by a new one holding the same PRs.
+			localStack.ID = remoteStackID
 			localStack.Number = remoteStack.Number
 			if err := stack.Save(gitDir, sf); err != nil {
 				return nil, "", handleSaveError(cfg, err)

internal/github/github_test.go

@@ -1,10 +1,12 @@
 package github
 
 import (
+	"encoding/json"
 	"testing"
 
 	graphql "github.com/cli/shurcooL-graphql"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestPRURL(t *testing.T) {
@@ -80,3 +82,69 @@ func TestToGraphQLInt(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+// TestRemoteStack_UnmarshalJSON verifies the custom decoder that flattens the
+// Stacks REST API's pull_requests object array into ordered PullRequests
+// numbers while preserving the full entries in PRDetails. This is the only path
+// that populates those fields from the wire, so a shape regression here would
+// silently break every stack endpoint.
+func TestRemoteStack_UnmarshalJSON(t *testing.T) {
+	payload := `{
+		"id": 6154,
+		"number": 360,
+		"node_id": "S_kwABCD",
+		"url": "https://api.github.com/repos/o/r/stacks/360",
+		"base": {"ref": "main", "sha": "basesha"},
+		"open": true,
+		"created_at": "2026-01-01T00:00:00Z",
+		"pull_requests": [
+			{"number": 12, "state": "open", "draft": true, "merged_at": null, "head": {"ref": "feat-1", "sha": "sha1"}},
+			{"number": 15, "state": "closed", "draft": false, "merged_at": "2026-01-02T00:00:00Z", "head": {"ref": "feat-2", "sha": "sha2"}}
+		]
+	}`
+
+	var s RemoteStack
+	require.NoError(t, json.Unmarshal([]byte(payload), &s))
+
+	// Top-level metadata.
+	assert.Equal(t, 6154, s.ID)
+	assert.Equal(t, 360, s.Number)
+	assert.Equal(t, "S_kwABCD", s.NodeID)
+	assert.Equal(t, "https://api.github.com/repos/o/r/stacks/360", s.URL)
+	assert.Equal(t, "main", s.Base.Ref)
+	assert.Equal(t, "basesha", s.Base.Sha)
+	assert.True(t, s.Open)
+	assert.Equal(t, "2026-01-01T00:00:00Z", s.CreatedAt)
+
+	// Ordered PR numbers (bottom to top) derived from the object array.
+	assert.Equal(t, []int{12, 15}, s.PullRequests)
+	assert.Equal(t, []int{12, 15}, s.PRNumbers())
+
+	// Full PR entries preserved in PRDetails, including nullable merged_at.
+	require.Len(t, s.PRDetails, 2)
+	assert.Equal(t, 12, s.PRDetails[0].Number)
+	assert.Equal(t, "open", s.PRDetails[0].State)
+	assert.True(t, s.PRDetails[0].Draft)
+	assert.Nil(t, s.PRDetails[0].MergedAt)
+	assert.False(t, s.PRDetails[0].IsMerged())
+	assert.Equal(t, "feat-1", s.PRDetails[0].Head.Ref)
+	assert.Equal(t, "sha1", s.PRDetails[0].Head.Sha)
+
+	assert.Equal(t, 15, s.PRDetails[1].Number)
+	assert.Equal(t, "closed", s.PRDetails[1].State)
+	require.NotNil(t, s.PRDetails[1].MergedAt)
+	assert.Equal(t, "2026-01-02T00:00:00Z", *s.PRDetails[1].MergedAt)
+	assert.True(t, s.PRDetails[1].IsMerged())
+	assert.Equal(t, "feat-2", s.PRDetails[1].Head.Ref)
+}
+
+// TestRemoteStack_UnmarshalJSON_EmptyPRs ensures a stack with no pull_requests
+// decodes to empty (non-nil) slices rather than panicking.
+func TestRemoteStack_UnmarshalJSON_EmptyPRs(t *testing.T) {
+	var s RemoteStack
+	require.NoError(t, json.Unmarshal([]byte(`{"id": 1, "number": 2, "pull_requests": []}`), &s))
+	assert.Equal(t, 1, s.ID)
+	assert.Equal(t, 2, s.Number)
+	assert.Empty(t, s.PullRequests)
+	assert.Empty(t, s.PRDetails)
+}