Conversation
The add_labels and remove_labels safe output handlers were using parseInt() to parse item_number, causing "Invalid item number: aw_report1" errors when agents referenced issues created earlier in the same run via temporary IDs. This fix implements temporary ID resolution consistent with add_comment: - add_labels.cjs: import resolveRepoIssueTarget/loadTemporaryIdMapFromResolved from temporary_id.cjs and resolve item_number before parsing - remove_labels.cjs: same fix applied - safe_output_validation_config.go: change item_number field for add_labels and remove_labels from IssueOrPRNumber to IssueNumberOrTemporaryID to allow temporary ID strings to pass validation - temporary_id.cjs: add item_number to the idFields checked by extractTemporaryIdReferences so the scheduler can detect and defer add_labels/remove_labels messages with unresolved temporary ID dependencies Fixes #21035 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
@copilot update tests |
…, and temporary_id Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Added tests in commit 6f30a64:
Also included the missing lock.yml updates that reflect the |
Contributor
There was a problem hiding this comment.
Pull request overview
Resolves failures when add_labels / remove_labels receive a temporary ID (e.g., aw_report1) in item_number, by adding temporary-ID resolution + deferral behavior consistent with other handlers.
Changes:
- Resolve
item_numbervia temporary ID map inadd_labels.cjsandremove_labels.cjs, returning{ deferred: true }when unresolved. - Expand temporary-ID dependency detection (
extractTemporaryIdReferences) to includeitem_number, and add/extend unit tests. - Update safe-output validation schema (
issueOrPRNumber→issueNumberOrTemporaryId) and recompile workflow lock files accordingly.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_output_validation_config.go | Updates schema flags so item_number can be a temporary ID for label handlers. |
| actions/setup/js/temporary_id.cjs | Adds item_number to dependency extraction field list. |
| actions/setup/js/temporary_id.test.cjs | Adds tests for extracting temporary IDs from item_number (including #-prefixed). |
| actions/setup/js/add_labels.cjs | Resolves temporary IDs for item_number and defers when unresolved. |
| actions/setup/js/add_labels.test.cjs | Adds tests for resolved/deferred temporary IDs and #aw_... format. |
| actions/setup/js/remove_labels.cjs | Mirrors temporary ID resolution/deferral logic for remove-labels handler. |
| actions/setup/js/remove_labels.test.cjs | Adds tests for resolved/deferred temporary IDs and #aw_... format. |
| .github/workflows/smoke-project.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/smoke-gemini.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/smoke-copilot.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/smoke-copilot-arm.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/smoke-codex.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/smoke-claude.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/scout.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/pr-triage-agent.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/poem-bot.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/issue-triage-agent.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/draft-pr-cleanup.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/contribution-check.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/code-scanning-fixer.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/auto-triage-issues.lock.yml | Regenerated lock to reflect schema key rename. |
| .github/workflows/ai-moderator.lock.yml | Regenerated lock to reflect schema key rename. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+551
to
+567
| it("should resolve temporary ID with hash prefix in item_number", async () => { | ||
| const handler = await main({ max: 10 }); | ||
| const addLabelsCalls = []; | ||
|
|
||
| mockGithub.rest.issues.addLabels = async params => { | ||
| addLabelsCalls.push(params); | ||
| return {}; | ||
| }; | ||
|
|
||
| const result = await handler( | ||
| { | ||
| item_number: "#aw_report1", | ||
| labels: ["enhancement"], | ||
| }, | ||
| { aw_report1: { repo: "test-owner/test-repo", number: 99 } } | ||
| ); | ||
|
|
Comment on lines
+572
to
+588
| it("should resolve temporary ID with hash prefix in item_number", async () => { | ||
| const handler = await main({ max: 10 }); | ||
| const removeLabelCalls = []; | ||
|
|
||
| mockGithub.rest.issues.removeLabel = async params => { | ||
| removeLabelCalls.push(params); | ||
| return {}; | ||
| }; | ||
|
|
||
| const result = await handler( | ||
| { | ||
| item_number: "#aw_report1", | ||
| labels: ["enhancement"], | ||
| }, | ||
| { aw_report1: { repo: "test-owner/test-repo", number: 99 } } | ||
| ); | ||
|
|
| Fields: map[string]FieldValidation{ | ||
| "labels": {Required: true, Type: "array", ItemType: "string", ItemSanitize: true, ItemMaxLength: 128}, | ||
| "item_number": {IssueOrPRNumber: true}, | ||
| "item_number": {IssueNumberOrTemporaryID: true}, |
|
|
||
| // Check for other resolution errors | ||
| if (resolvedTarget.errorMessage || !resolvedTarget.resolved) { | ||
| const error = `Invalid item number: ${message.item_number}`; |
|
|
||
| // Check for other resolution errors | ||
| if (resolvedTarget.errorMessage || !resolvedTarget.resolved) { | ||
| const error = `Invalid item number: ${message.item_number}`; |
pelikhan
deleted the
fix/add-labels-temporary-id-resolution-de341fe7cdf04f26
branch
Contributor
Author
|
Hey A quick checklist rundown:
Verdict: 🟢 Aligned — looks ready for maintainer review. The three scenarios covered by the new tests (resolved temporary ID, deferred/unresolved, and hash-prefix
|
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.
Summary
Implements the recommended Option 1: temporary ID resolution in the
add_labels(andremove_labels) safe output handlers, consistent withadd_commentbehavior.Root Cause
When an agent creates an issue with
create_issueand assigns it atemporary_id: aw_report1, then usesadd_labelswithitem_number: aw_report1, the handler fails with:This happened because
add_labels.cjs(andremove_labels.cjs) usedparseInt(String(message.item_number), 10)which returnsNaNfor temporary ID strings.Changes
add_labels.cjs: ImportresolveRepoIssueTarget+loadTemporaryIdMapFromResolvedfromtemporary_id.cjs; use them to resolveitem_numberbefore processing. Returnsdeferred: truewhen the temporary ID hasn't been resolved yet (so the handler manager retries after the first pass).remove_labels.cjs: Same fix applied.safe_output_validation_config.go: Changeditem_numberforadd_labelsandremove_labelsfromIssueOrPRNumber: truetoIssueNumberOrTemporaryID: trueso temporary ID strings pass Go-level validation.temporary_id.cjs: Addeditem_numberto theidFieldsarray inextractTemporaryIdReferences, so the scheduler can detect and deferadd_labels/remove_labelsmessages that depend on as-yet-unresolved temporary IDs.add_labels.test.cjs: Added tests for temporary ID resolution — resolved temporary ID, deferred (unresolved), and hash-prefix#aw_report1formats.remove_labels.test.cjs: Same tests added forremove_labels.temporary_id.test.cjs: Added tests foritem_numberfield inextractTemporaryIdReferences, including verification that numericitem_numbervalues produce no temporary ID references.*.lock.yml: Recompiled workflow lock files to reflect theissueOrPRNumber→issueNumberOrTemporaryIdschema change.🤖 Generated with Claude Code via
/cloclo📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.