fix: tighten repo-memory push concurrency key to actual branch targets#23489
Merged
fix: tighten repo-memory push concurrency key to actual branch targets#23489
Conversation
The push_repo_memory job previously used a single repository-wide
concurrency group (push-repo-memory-${{ github.repository }}), causing
all workflows that use repo-memory to serialise or cancel each other
even when they push to completely different memory/* branches.
This change introduces buildPushRepoMemoryConcurrencyGroup(), which
builds the concurrency group key from the sorted set of (target-repo,
branch) pairs that the push job actually writes to. Different workflows
pushing to different branches now run in parallel; workflows pushing to
the same branch still serialise correctly.
All 178 lock files have been recompiled with their per-branch keys.
Fixes #<issue>"
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bfc3a2b6-31fa-4406-9f29-97b0018892ae
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Replace the hyphen separator between branch names in the concurrency group key with a pipe (|) character, which cannot appear in valid git branch names. This prevents accidental key collisions between workflows whose combined branch names could otherwise produce the same string (e.g., memory/foo-bar vs memory/foo + bar). Also update test expected values and add a test case explicitly demonstrating that hyphenated branch names produce unambiguous keys. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/bfc3a2b6-31fa-4406-9f29-97b0018892ae Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix concurrency handling for repo-memory push jobs
fix: tighten repo-memory push concurrency key to actual branch targets
Mar 29, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the push_repo_memory job’s GitHub Actions concurrency grouping so runs only serialize when they are pushing to the same repo-memory branch target(s), avoiding unnecessary contention across unrelated workflows.
Changes:
- Add
buildPushRepoMemoryConcurrencyGroup()to compute a deterministic concurrency group from the job’s repo-memory targets. - Update
buildPushRepoMemoryJobto use the new computed concurrency group. - Recompile workflow lock files to embed the new per-branch concurrency group keys, and add unit tests covering the helper/job behavior.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/repo_memory.go | Adds helper to build per-target concurrency group and applies it to push_repo_memory job generation. |
| pkg/workflow/repo_memory_test.go | Adds tests validating concurrency key construction and job concurrency configuration. |
| .github/workflows/workflow-health-manager.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/weekly-blog-post-writer.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/technical-doc-writer.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/security-compliance.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/pr-triage-agent.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/metrics-collector.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/glossary-maintainer.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/firewall-escape.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/discussion-task-miner.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/developer-docs-consolidator.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/delight.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/deep-report.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/daily-news.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/daily-copilot-token-report.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/daily-community-attribution.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/daily-code-metrics.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/daily-cli-performance.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/copilot-session-insights.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/copilot-agent-analysis.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/code-scanning-fixer.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/audit-workflows.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
| .github/workflows/agent-performance-analyzer.lock.yml | Updates push_repo_memory concurrency group to include the workflow’s repo-memory branch. |
Comments suppressed due to low confidence (1)
pkg/workflow/repo_memory.go:571
buildPushRepoMemoryConcurrencyGroupbuilds keys from the rawTargetRepo/BranchName, butbuildPushRepoMemoryJobnormalizes the effective target repo (defaults emptyTargetRepoto${{ github.repository }}and appends.wikifor wiki mode). This mismatch means the concurrency group is not actually derived from the real (repo, branch) write targets and can split or merge groups incorrectly (e.g.,${{ github.repository }}explicitly set vs empty, or wiki vs non-wiki with same branch name). Suggest deriving the concurrency key from the same normalized effective target repo used in the push/clone steps (including.wiki) so serialization matches the actual push destination.
func buildPushRepoMemoryConcurrencyGroup(memories []RepoMemoryEntry) string {
branchKeys := make([]string, 0, len(memories))
for _, m := range memories {
key := m.BranchName
if m.TargetRepo != "" {
key = m.TargetRepo + ":" + key
}
branchKeys = append(branchKeys, key)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/workflow/repo_memory_test.go
Outdated
Comment on lines
+1235
to
+1240
| name: "branches with hyphens produce unambiguous keys", | ||
| memories: []RepoMemoryEntry{ | ||
| {ID: "a", BranchName: "memory/workflow-a"}, | ||
| {ID: "b", BranchName: "memory/workflow-b"}, | ||
| }, | ||
| // "|" separator ensures "memory/workflow-a|memory/workflow-b" ≠ "memory/workflow-a-b" |
There was a problem hiding this comment.
This test comment/assertion relies on the assumption that | cannot appear in git branch names to guarantee an unambiguous key. If the production code switches to encoding branch names or to a delimiter that is truly invalid for refs (recommended), this test should be updated to validate that property (e.g., round-trippable encoding or delimiter choice) rather than hard-coding | as "invalid in branch names".
Suggested change
| name: "branches with hyphens produce unambiguous keys", | |
| memories: []RepoMemoryEntry{ | |
| {ID: "a", BranchName: "memory/workflow-a"}, | |
| {ID: "b", BranchName: "memory/workflow-b"}, | |
| }, | |
| // "|" separator ensures "memory/workflow-a|memory/workflow-b" ≠ "memory/workflow-a-b" | |
| 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. |
pkg/workflow/repo_memory.go
Outdated
Comment on lines
+556
to
+560
| // Key format: "push-repo-memory-${{ github.repository }}|<branch1>[|<branch2>…]" | ||
| // | ||
| // The "|" separator is used because it cannot appear in valid git branch names, making | ||
| // the key unambiguous even when branch names contain hyphens or slashes. For memories | ||
| // that target a non-default repository, the target repo is prepended to the branch name |
There was a problem hiding this comment.
The comment and implementation assume "|" cannot appear in valid git branch names, but git check-ref-format does not forbid | (and our ref validator in pkg/workflow/glob_validation.go only flags spaces, ~, ^, :, [, ?, *, etc.). That means branch names containing | would make this concurrency key ambiguous and could cause unintended collisions. Consider either escaping/encoding branch names before joining, or switching to a delimiter that is guaranteed invalid in ref names (and updating the comment accordingly).
This issue also appears on line 564 of the same file.
Collaborator
|
@copilot Review comments |
The "|" separator used between branch name components in the concurrency group key is not actually forbidden by git check-ref-format, so a user-supplied explicit branch-name could contain "|" and produce an ambiguous key. Introduce encodeConcurrencyKeyPart() which percent-encodes "%" (to avoid double-encoding) and "|" before the components are joined. Normal branch names (auto-generated or using plain alphanumeric/hyphen characters) are unaffected since they contain neither character. Also rename the "branches with hyphens produce unambiguous keys" test to "branches with hyphens use pipe separator in key" and update its comment to describe the current encoding strategy rather than making an incorrect claim about git ref format rules. Add a new test case that explicitly verifies "|" in a branch name is encoded as "%7C". Agent-Logs-Url: https://github.com/github/gh-aw/sessions/903fc18e-9422-49d3-93a3-dea8cb70e5e5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Addressed in 023e7a0:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
push_repo_memoryjob used a single repo-wide concurrency group, causing unrelated workflows pushing to differentmemory/*branches to unnecessarily serialise or cancel each other—marking workflow conclusions as failed even when the primary agent job succeeded.Changes
buildPushRepoMemoryConcurrencyGroup()— new helper that builds the concurrency key from the sorted set of(target-repo, branch)pairs the job actually writes to, rather than a fixed repo-wide stringencodeConcurrencyKeyPart()— percent-encodes%and|in each key component before joining, ensuring the|separator is always unambiguous even if a user-suppliedbranch-namecontains a literal||separator between branch entries with percent-encoding for robustness — multi-memory keys are unambiguous regardless of branch name contenttarget-repovalues are prefixed asowner/repo:branchto keep cross-repo pushes in distinct groupsBefore → After
Workflows pushing to different branches now run in parallel; workflows pushing to the same branch still serialise correctly.
⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.