s360-reporter: fix broken item links and enforce full pagination, Fixes AB#3683197#446
Open
shahzaibj wants to merge 1 commit into
Open
s360-reporter: fix broken item links and enforce full pagination, Fixes AB#3683197#446shahzaibj wants to merge 1 commit into
shahzaibj wants to merge 1 commit into
Conversation
Fixes AB#3683197 — two first-user bugs reported by Sowmya Malayanur (Jun 2026). Bug 1 — Broken item links - generate-report.js rendered every item title as an anchor around item.s360Url unconditionally, so a missing S360 URL shipped as literal href="undefined" (404 on click). - Added safeHref() and linkOrText() helpers: only wrap the title in an anchor when s360Url is a non-empty http(s) URL. Otherwise render plain text so the reviewer notices before sending. - Added a red banner and stderr WARN listing every item that ended up without a link, so a broken run is visible in-report rather than shipping silently. Bug 2 — Incomplete item coverage - Pagination was only a manual instruction in SKILL.md, so a first-time user could ship the report using only page 1 of the MCP response (up to 50 items). - New fetch-items.js consolidator loops through paginated response files and hard-fails if the last page still has a populated nextCursor. - merge-items.js now refuses to run against any input JSON that still has a populated nextCursor (defense-in-depth). - SKILL.md Steps 1a/1b/1c updated to require fetch-items.js and document both guards. Self-check (AC#3) - Coverage summary printed to stderr on every run (item counts by SLA plus resolved and new-this-week counts). - In-report banner + stderr WARN when any item has no valid link. Verified end-to-end against a synthetic fixture: - fetch-items.js exits 1 when last page has nextCursor; exits 0 when exhausted. - merge-items.js exits 1 on unspent-cursor input. - generate-report.js emits banner and WARN for the broken URLs, valid URLs still render as anchors, and no href="undefined" or href="null" appears in the output HTML. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
changed the title
|
✅ Work item link check complete. Description contains link AB#3683197 to an Azure Boards work item. |
1 similar comment
|
✅ Work item link check complete. Description contains link AB#3683197 to an Azure Boards work item. |
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
1 participant
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.
Fixes AB#3683197 — two first-user bugs in the
s360-reporterskill reported by Sowmya Malayanur (Jun 2026, ★4).Bugs
Bug 1 — Broken links to S360 items.
generate-report.jsrendered every item title as<a href="${item.s360Url}">unconditionally. When the S360 API returned noURLfield, the anchor shipped as literalhref="undefined"(404 on click). Verified in Sowmya's actual report at~/Desktop/s360-report-2026-04-08.html— zero item links pointed tos360.msftcloudes.com.Bug 2 — Incomplete item coverage. SKILL.md said "If more than 50 items, paginate using the
nextCursorfield" but there was no enforcement. A first-time user who missed that step shipped a report with only page 1 of the MCP response.Fixes
Link safety (
generate-report.js)safeHref(url)— only accepts non-emptyhttp(s)URLs. Rejectsnull,undefined, literal"undefined", empty,javascript:, etc.linkOrText(url, innerHtml, style)— wraps in<a>only whensafeHrefreturns a URL; else renders plain text so the reviewer notices.<a href="${item.s360Url}">call sites (item card + table row).WARNlisting every item that ended up without a link, so a broken run is visible in-report instead of shipping silently.Coverage enforcement
fetch-items.js— consolidates paginated MCP responses into one JSON file. Exits non-zero if the last input page still has a populatednextCursor.merge-items.js— refuses to run against any input JSON that still has a populatednextCursor(defense-in-depth).SKILL.mdSteps 1a/1b/1c updated: pagination is mandatory and routed throughfetch-items.js.Self-check (AC#3)
Coverage summary: N active items rendered (X out-of-SLA, Y approaching, Z in SLA, W missing ETA). N resolved. N new this week.Verification
Ran end-to-end against a synthetic fixture:
fetch-items.jswith page1-only (cursor present)fetch-items.jswith page1+page2 (last cursor null)merge-items.jswith unspent-cursor inputmerge-items.jswith clean inputgenerate-report.jswith mix of valid + broken URLshref="undefined"/href="null"/href=""Notes for reviewers
pbiUrl(r.pbi)— builds the URL from an ID and is guaranteed safe, so no change needed there.reduce-items.jsgrouping/dedup logic. Fix is additive.fetch-items.jsis a pure Node consolidator — the actual MCP calls still happen from the model perSKILL.md.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com