fix: strip redundant resource identifier from integrity filtering note (#21988)#22019
fix: strip redundant resource identifier from integrity filtering note (#21988)#22019
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0d6be7f9-8413-4240-86e2-6956d5d10f7d
…21988) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0d6be7f9-8413-4240-86e2-6956d5d10f7d
There was a problem hiding this comment.
Pull request overview
Fixes duplicated resource identifiers in the “Integrity filtering filtered …” note by stripping the redundant Resource 'X' prefix from DIFC filtered event reasons when rendering list items.
Changes:
- Update DIFC filtered section rendering to remove the leading
Resource '…'prefix from reason text. - Add a unit test to assert the prefix is stripped in the duplicated-resource scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| actions/setup/js/gateway_difc_filtered.cjs | Adjusts reason formatting to avoid repeating the resource identifier in integrity filtering list items. |
| actions/setup/js/gateway_difc_filtered.test.cjs | Adds coverage for the new reason-prefix stripping behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| const tool = event.tool_name ? `\`${event.tool_name}\`` : "-"; | ||
| const reason = (event.reason || "-").replace(/\n/g, " "); | ||
| const reason = (event.reason || "-").replace(/^Resource '[^']*' /, "").replace(/\n/g, " "); |
There was a problem hiding this comment.
The prefix stripping is unconditional: if an event lacks description/html_url (so the rendered reference falls back to the tool name), but reason starts with Resource '…', this will remove the only place the resource identifier appears and make the output less informative. Consider stripping only when the extracted resource matches the rendered reference (e.g., event.description matches the Resource '…' value, or when html_url is present), otherwise keep the full reason text.
| const reason = (event.reason || "-").replace(/^Resource '[^']*' /, "").replace(/\n/g, " "); | |
| const rawReason = (event.reason || "-").replace(/\n/g, " "); | |
| let reason = rawReason; | |
| const resourceMatch = /^Resource '([^']*)' (.*)$/.exec(rawReason); | |
| if (resourceMatch) { | |
| const resourceName = resourceMatch[1]; | |
| const rest = resourceMatch[2]; | |
| if (event.html_url || (event.description && event.description === resourceName)) { | |
| reason = rest; | |
| } | |
| } |
| // The description (reference) should still be present | ||
| expect(result).toContain("issue:github/gh-aw#21982"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The new test covers the deduplication case where description is present, but it doesn’t protect against the scenario where description/html_url are absent and the reason begins with Resource '…'. Adding a test for that case will help ensure we don’t accidentally strip the resource identifier when it isn’t duplicated in the rendered reference.
| it("should keep 'Resource X' prefix when no description or html_url is present", () => { | |
| const events = [ | |
| { | |
| type: "DIFC_FILTERED", | |
| tool_name: "list_issues", | |
| // No description or html_url, so the resource identifier only appears in the reason | |
| reason: "Resource 'issue:github/gh-aw#21982' has lower integrity than agent requires. The agent cannot read data with integrity below \"approved\".", | |
| }, | |
| ]; | |
| const result = generateDifcFilteredSection(events); | |
| // When there is no separate description/html_url, we must not strip the resource identifier | |
| expect(result).toContain("Resource 'issue:github/gh-aw#21982'"); | |
| expect(result).toContain("has lower integrity than agent requires"); | |
| }); |
Summary
Fixes #21988.
The integrity filtering note rendered each filtered item with the resource identifier appearing twice:
Before:
After:
Changes
actions/setup/js/gateway_difc_filtered.cjs: strip theResource 'X'prefix from the reason text when building the list item, since the resource identifier is already shown as the line reference.actions/setup/js/gateway_difc_filtered.test.cjs: add a test covering the deduplication behaviour.