-
Notifications
You must be signed in to change notification settings - Fork 331
Temporary ID references in patch content are not resolved before git am #25130
Description
Temporary ID references in patch content are not resolved before git am
Summary
When a create_pull_request safe output references a create_issue temporary ID inside the patch content (i.e., in the actual code being committed), the #aw_* placeholder is not resolved to the real issue number. The framework correctly resolves temporary IDs in the PR body text, but the patch file is applied verbatim — so the placeholder string ends up in the committed source code.
Reproduction
- Create a workflow where the agent:
- Calls
create_issuewithtemporary_id: "aw_myid" - Calls
create_pull_requestwith a patch that contains#aw_myidin the diff content (e.g., inside a string literal in source code)
- Calls
- Run the workflow.
- Observe that the PR body correctly resolves
#aw_myid→#66195(or whatever issue number was assigned), but the committed code still contains the literal string#aw_myid.
Real-world example
In dotnet/aspnetcore's test-quarantine workflow, the agent quarantines a flaky test by adding a [QuarantinedTest] attribute that references the tracking issue:
[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#aw_navqry1")]The safe_outputs job creates issue #66195 and correctly resolves the reference in the PR body ("Associated issue: #66195"). But the committed code in the PR still contains the unresolved placeholder:
// Actual (wrong):
[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#aw_navqry1")]
// Expected:
[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/66195")]Root cause
In actions/setup/js/create_pull_request.cjs, replaceTemporaryIdReferences() is called on the PR body (~line 693):
processedBody = replaceTemporaryIdReferences(processedBody, tempIdMap, itemRepo);
core.info(`Resolved ${tempIdMap.size} temporary ID references in PR body`);But it is never called on the patch content. The patch is read from disk (line 524), validated, and then applied directly via git am --3way (~line 1010) with the raw, unresolved content.
The safe_outputs log confirms this — note the asymmetry:
Resolved 1 temporary ID references in PR body ← body is resolved ✓
...
+ [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#aw_navqry1")] ← patch is NOT resolved ✗
...
[command]/usr/bin/git am --3way /tmp/gh-aw/aw-quarantine-....patch
Patch applied successfully
Additional consideration: URL-context replacement
Even if the patch content were processed, the current replacement logic would produce an incorrect result in URL context. The TEMPORARY_ID_PATTERN in temporary_id.cjs (line 30) replaces #aw_navqry1 → #66195:
Input: https://github.com/dotnet/aspnetcore/issues/#aw_navqry1
Output: https://github.com/dotnet/aspnetcore/issues/#66195 ← '#' makes this a URL fragment anchor
Correct: https://github.com/dotnet/aspnetcore/issues/66195 ← should be a path segment
In PR body markdown, #66195 is a valid GitHub issue shorthand. But inside a URL, the # character creates a fragment identifier instead of a path component, making the link point to the wrong location.
Suggested fix
In create_pull_request.cjs, before applying the patch (around line 995), resolve temporary IDs in the patch content:
- Read
patchContent(already done at line 524) - Apply a URL-aware replacement that handles both contexts:
- URL context: Replace
/#aw_XXX(preceded by/) with/NUMBER— no#prefix - Text context: Replace
#aw_XXXwith#NUMBER(existing behavior)
- URL context: Replace
- Write the modified content back to
patchFilePathbefore callinggit am
A simple approach for step 2 could be to first replace the URL-context pattern, then fall through to the standard text-context pattern:
// URL-context: /issues/#aw_XXX → /issues/NUMBER
patchContent = patchContent.replace(/\/(#aw_[A-Za-z0-9_]{3,12})\b/gi, (match, tempIdWithHash) => {
const tempId = tempIdWithHash.substring(1); // strip leading #
const resolved = tempIdMap.get(normalizeTemporaryId(tempId));
if (resolved) {
return `/${resolved.number}`;
}
return match;
});
// Then apply standard text-context replacement
patchContent = replaceTemporaryIdReferences(patchContent, tempIdMap, itemRepo);
// Write back before git am
fs.writeFileSync(patchFilePath, patchContent, "utf8");Note: modifying the patch content changes the diff lines but not the patch metadata (commit hashes, etc.), so git am may need --3way to handle any resulting mismatch — which is already the default path.
Environment
- gh-aw version: v0.67.1
- Workflow: dotnet/aspnetcore test-quarantine
- Affected PR: [test-quarantine] Quarantine ServerRoutingTest.CanNavigateToQueryStringPageWithNoQuery dotnet/aspnetcore#66196
- Created issue (correctly): Quarantine ServerRoutingTest.CanNavigateToQueryStringPageWithNoQuery dotnet/aspnetcore#66195