fix: tighten repo-memory push concurrency key to actual branch targets

pkg/workflow/repo_memory.go

@@ -13,6 +13,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
@@ -547,6 +548,42 @@ func generateRepoMemorySteps(builder *strings.Builder, data *WorkflowData) {
 	}
 }
 
+// buildPushRepoMemoryConcurrencyGroup builds a concurrency group key that is scoped to the
+// specific (target-repo, branch) pairs being written by this push job.  Using the actual
+// write targets—rather than a single repo-wide key—ensures that workflows pushing to
+// different memory branches do not unnecessarily serialise or cancel each other.
+//
+// Key format: "push-repo-memory-${{ github.repository }}|<key1>[|<key2>…]"
+//
+// Each key component is percent-encoded (only `%` and `|` are encoded) before joining
+// with "|", so the separator is always unambiguous even if a user-supplied branch name
+// or target-repo contains a literal "|".  For memories that target a non-default
+// repository, the target repo is prepended to the branch name
+// (e.g., "other-owner%2Fother-repo:memory%2Fbranch" would be encoded if needed) so that
+// distinct targets produce distinct concurrency groups.  The branches are sorted for a
+// deterministic key regardless of the order memories are declared in the frontmatter.
+func buildPushRepoMemoryConcurrencyGroup(memories []RepoMemoryEntry) string {
+	branchKeys := make([]string, 0, len(memories))
+	for _, m := range memories {
+		key := encodeConcurrencyKeyPart(m.BranchName)
+		if m.TargetRepo != "" {
+			key = encodeConcurrencyKeyPart(m.TargetRepo) + ":" + key
+		}
+		branchKeys = append(branchKeys, key)
+	}
+	sort.Strings(branchKeys)
+	return "push-repo-memory-${{ github.repository }}|" + strings.Join(branchKeys, "|")
+}
+
+// encodeConcurrencyKeyPart percent-encodes the characters that would otherwise make the
+// concurrency group key ambiguous: "%" (to avoid double-encoding) and "|" (the separator).
+// All other characters are left as-is so the key remains human-readable in workflow UIs.
+func encodeConcurrencyKeyPart(s string) string {
+	s = strings.ReplaceAll(s, "%", "%25")
+	s = strings.ReplaceAll(s, "|", "%7C")
+	return s
+}
+
 // buildPushRepoMemoryJob creates a job that downloads repo-memory artifacts and pushes them to git branches
 // This job runs after the agent job completes (even if it fails) and requires contents: write permission
 // If threat detection is enabled, only runs if no threats were detected
@@ -729,9 +766,12 @@ func (c *Compiler) buildPushRepoMemoryJob(data *WorkflowData, threatDetectionEna
 		outputs["patch_size_exceeded_"+memory.ID] = fmt.Sprintf("${{ steps.%s.outputs.patch_size_exceeded }}", stepID)
 	}
 
-	// Serialize all push_repo_memory jobs per repository to prevent concurrent git pushes
-	// cancel-in-progress is false so that updates from concurrent agents are queued, not dropped
-	concurrency := c.indentYAMLLines("concurrency:\n  group: \"push-repo-memory-${{ github.repository }}\"\n  cancel-in-progress: false", "    ")
+	// Build a concurrency key scoped to the actual branches being written.
+	// This prevents false serialisation between workflows that push to different memory
+	// branches while still serialising concurrent pushes to the *same* branch.
+	// cancel-in-progress is false so queued pushes are not dropped.
+	concurrencyGroup := buildPushRepoMemoryConcurrencyGroup(data.RepoMemoryConfig.Memories)
+	concurrency := c.indentYAMLLines(fmt.Sprintf("concurrency:\n  group: %q\n  cancel-in-progress: false", concurrencyGroup), "    ")
 
 	job := &Job{
 		Name:        "push_repo_memory",

pkg/workflow/repo_memory_test.go

@@ -1191,3 +1191,93 @@ func TestRepoMemoryNonWikiPushNoAllowedRepos(t *testing.T) {
 	assert.NotContains(t, pushJobOutput, "REPO_MEMORY_ALLOWED_REPOS",
 		"Non-wiki push step should not set REPO_MEMORY_ALLOWED_REPOS")
 }
+
+// TestBuildPushRepoMemoryConcurrencyGroup verifies that the concurrency group key is scoped
+// to the actual branch targets, so workflows pushing to different memory branches do not
+// contend with each other.
+func TestBuildPushRepoMemoryConcurrencyGroup(t *testing.T) {
+	tests := []struct {
+		name     string
+		memories []RepoMemoryEntry
+		expected string
+	}{
+		{
+			name: "single memory uses branch name in key",
+			memories: []RepoMemoryEntry{
+				{ID: "default", BranchName: "memory/daily-news"},
+			},
+			expected: "push-repo-memory-${{ github.repository }}|memory/daily-news",
+		},
+		{
+			name: "multiple memories sorted for deterministic key",
+			memories: []RepoMemoryEntry{
+				{ID: "b", BranchName: "memory/workflow-b"},
+				{ID: "a", BranchName: "memory/workflow-a"},
+			},
+			expected: "push-repo-memory-${{ github.repository }}|memory/workflow-a|memory/workflow-b",
+		},
+		{
+			name: "non-default target repo is prefixed to branch",
+			memories: []RepoMemoryEntry{
+				{ID: "default", BranchName: "memory/shared", TargetRepo: "other-org/other-repo"},
+			},
+			expected: "push-repo-memory-${{ github.repository }}|other-org/other-repo:memory/shared",
+		},
+		{
+			name: "mix of default and custom target repos",
+			memories: []RepoMemoryEntry{
+				{ID: "local", BranchName: "memory/local"},
+				{ID: "remote", BranchName: "memory/remote", TargetRepo: "other-org/other-repo"},
+			},
+			expected: "push-repo-memory-${{ github.repository }}|memory/local|other-org/other-repo:memory/remote",
+		},
+		{
+			name: "branches with hyphens use pipe separator in key",
+			memories: []RepoMemoryEntry{
+				{ID: "a", BranchName: "memory/workflow-a"},
+				{ID: "b", BranchName: "memory/workflow-b"},
+			},
+			// This expectation documents the current use of "|" as the list separator in the key.
+			// If the concurrency key encoding changes (e.g., different delimiter or encoding scheme),
+			// update this test to match the new encoding strategy.
+			expected: "push-repo-memory-${{ github.repository }}|memory/workflow-a|memory/workflow-b",
+		},
+		{
+			name: "pipe in branch name is percent-encoded so the separator stays unambiguous",
+			memories: []RepoMemoryEntry{
+				{ID: "unusual", BranchName: "memory/foo|bar"},
+			},
+			expected: "push-repo-memory-${{ github.repository }}|memory/foo%7Cbar",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := buildPushRepoMemoryConcurrencyGroup(tt.memories)
+			assert.Equal(t, tt.expected, got, "Concurrency group key should match expected value")
+		})
+	}
+}
+
+// TestPushRepoMemoryJobConcurrencyKey verifies that buildPushRepoMemoryJob sets a concurrency
+// key scoped to the actual memory branch rather than a repo-wide key.
+func TestPushRepoMemoryJobConcurrencyKey(t *testing.T) {
+	data := &WorkflowData{
+		RepoMemoryConfig: &RepoMemoryConfig{
+			Memories: []RepoMemoryEntry{
+				{ID: "default", BranchName: "memory/my-workflow"},
+			},
+		},
+	}
+
+	compiler := NewCompiler()
+	pushJob, err := compiler.buildPushRepoMemoryJob(data, false)
+	require.NoError(t, err, "Should build push job without error")
+	require.NotNil(t, pushJob, "Should produce a push job")
+
+	assert.Contains(t, pushJob.Concurrency, "memory/my-workflow",
+		"Concurrency key should include the memory branch name")
+	// Ensure the old repo-wide-only key is not used
+	assert.NotContains(t, pushJob.Concurrency, "push-repo-memory-${{ github.repository }}\"",
+		"Concurrency key should not be the old repo-wide-only key format")
+}