fix: register create_pull_request temporaryId and add synthetic update support#33853
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…e support - Change create_pull_request.cjs successful return shape to use camelCase fields (number, url, temporaryId) matching what safe_output_handler_manager expects, fixing the silent discard of temporary_id registrations - Add create_pull_request to getContentToCheck so PR bodies with forward #aw_* refs are tracked for synthetic updates - Add updatePullRequestBody helper and create_pull_request case in processSyntheticUpdates so PR bodies are patched once forward refs resolve - Update create_pull_request.test.cjs assertion to use result.number - Update safe_output_handler_manager.test.cjs mock to use new field names - Add three new tests covering temp ID registration, downstream resolution, and forward-ref tracking for create_pull_request Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes create_pull_request temporary ID registration by aligning the handler return shape with the safe-output manager contract, and extends synthetic-update processing so PR bodies that contain forward #aw_* references can be patched once those temporary IDs resolve.
Changes:
- Updated
create_pull_request.cjsto return{ number, url, temporaryId, repo }instead of snake_case PR fields. - Extended
safe_output_handler_manager.cjsto trackcreate_pull_requestbodies for unresolved temp IDs and to apply synthetic body updates viagithub.rest.pulls.update. - Updated/added tests to validate temp-ID registration, propagation, and forward-ref tracking; updated workflow lockfiles to exclude additional env vars when invoking
awf.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/safe_output_handler_manager.test.cjs | Adds/updates tests for PR temp-id registration, propagation, and forward-ref tracking. |
| actions/setup/js/safe_output_handler_manager.cjs | Tracks PR bodies for unresolved IDs and adds PR body synthetic updates via REST API. |
| actions/setup/js/create_pull_request.test.cjs | Updates assertion to reflect the new create_pull_request return shape. |
| actions/setup/js/create_pull_request.cjs | Aligns successful return fields with manager expectations (number, url, temporaryId). |
| .github/workflows/smoke-codex.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/smoke-call-workflow.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/schema-feature-coverage.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/necromancer.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/issue-arborist.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/grumpy-reviewer.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/duplicate-code-detector.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/daily-observability-report.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/daily-fact.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/daily-cache-strategy-analyzer.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/changeset.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
| .github/workflows/ai-moderator.lock.yml | Updates locked workflow command/env exclusions for awf invocation. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 17/17 changed files
- Comments generated: 2
| case "create_pull_request": | ||
| return message.body || ""; |
| pull_request_number: pullRequest.number, | ||
| pull_request_url: pullRequest.html_url, | ||
| number: pullRequest.number, | ||
| url: pullRequest.html_url, |
|
@copilot merge main, recompile, fix conflicts, review all comments |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… number/url fields - Add managedBody to create_pull_request success return so synthetic updates use the actual posted body (with header/footer/caution markers) instead of the raw agent-provided message.body - Update getContentToCheck to prefer result?.managedBody for create_pull_request - Fix safe_outputs_action_outputs.cjs to read r.number/r.url instead of the removed r.pull_request_number/r.pull_request_url fields - Update tests accordingly - Merge origin/main (codex --model flag fix + uses: upgrade) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — merged
All 307 test files (9 157 tests) pass. |
create_pull_requesttemporary IDs were silently dropped — never written totemporary-id-map.json— becausecreate_pull_request.cjsreturned snake_case prefixed fields (pull_request_number,temporary_id) whilesafe_output_handler_manager.cjsregistered temp IDs only whenresult.temporaryId && result.repo && result.numberwere all truthy. The guard was alwaysfalsefor PR results, making every#aw_prNreference in subsequent safe-output bodies render as literal text.Return shape fix (
create_pull_request.cjs)Aligned the successful return to match the manager contract:
Synthetic update support (
safe_output_handler_manager.cjs)create_pull_requesttogetContentToCheckso PR bodies containing forward#aw_*refs are tracked.updatePullRequestBody(viagithub.rest.pulls.update) and acreate_pull_requestcase inprocessSyntheticUpdatesso tracked PR bodies are retroactively patched once forward refs resolve.Tests
create_pull_request.test.cjsassertion (pull_request_number→number).create_pull_request, propagation of resolved IDs to subsequent handlers, and forward-ref tracking.