test: add tests for artifact URL mapping in processMessages

[Copilot]

The CI for PR #26108 (copilot/validate-temporary-ids-urls) failed with a TypeScript typecheck error:

safe_output_handler_manager.cjs(970,79): error TS2345: Argument of type 'Map<string, string> | undefined' is not assignable to parameter of type 'Map<string, string>'.

Root cause: The processSyntheticUpdates function had artifactUrlMap as an optional JSDoc parameter ([artifactUrlMap]), making its type Map<string, string> | undefined. The replaceArtifactUrlReferences function in temporary_id.cjs only accepted Map<string, string>, so passing the optional parameter caused a TypeScript error.

Fix (already in main): The JSDoc for replaceArtifactUrlReferences was updated to @param {Map<string, string>|null|undefined} artifactUrlMap, matching the function's actual runtime behavior (it handles undefined internally).

This PR adds test coverage for the artifact URL mapping feature introduced by PR #26108, which was missing from safe_output_handler_manager.test.cjs:

1. Test that artifactUrlMap is populated in the result when upload_artifact succeeds
2. Test that issues with artifact references in their body are tracked in outputsWithUnresolvedIds when the artifact is uploaded after
3. Test that artifact URL references in issue bodies are pre-resolved when the artifact was already uploaded before the issue is created

[Copilot]

Pull request overview

Adds missing test coverage for artifact temporary-ID (#aw_*) to artifact URL mapping and resolution behavior within processMessages, ensuring the safe output handler manager properly tracks unresolved references and pre-resolves bodies when possible.

Changes:

• Adds a test that artifactUrlMap is populated after a successful upload_artifact.
• Adds a test ensuring outputs created before an artifact upload (but referencing it) are tracked in outputsWithUnresolvedIds.
• Adds a test ensuring outputs created after an artifact upload have #aw_* references replaced in the handler input body.

Show a summary per file

File	Description
actions/setup/js/safe_output_handler_manager.test.cjs	Adds tests validating artifact URL map population, unresolved-reference tracking, and pre-resolution of artifact references in message bodies.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 1/1 changed files
• Comments generated: 0

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	3
✅ Design tests (behavioral contracts)	3 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	2 (67%)
Duplicate test clusters	0
Test inflation detected	⚠️ Yes (test-only PR — 100 new test lines, 0 new production lines)
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Issues Detected
should populate artifactUrlMap when upload_artifact succeeds	actions/setup/js/safe_output_handler_manager.test.cjs	✅ Design	Happy-path only; no error cases
should track issue with artifact reference as needing synthetic update when artifact is uploaded after	actions/setup/js/safe_output_handler_manager.test.cjs	✅ Design	Covers ordering edge case (issue before artifact)
should replace artifact URL references in issue body when artifact is uploaded before issue	actions/setup/js/safe_output_handler_manager.test.cjs	✅ Design	Covers ordering edge case (artifact before issue)

---

Score Breakdown

Component	Score
Behavioral Coverage (40 pts)	40/40 — all 3 tests verify observable outputs
Error/Edge Case Coverage (30 pts)	20/30 — 2 of 3 tests cover ordering edge cases
Low Duplication (20 pts)	20/20 — each test covers a distinct scenario
Proportional Growth (10 pts)	0/10 — test-only PR; ratio exceeds 2:1 threshold

Note on test inflation: This PR intentionally adds tests for pre-existing production functionality (no production code was changed). The inflation penalty is applied per the scoring rules, but this is the expected pattern for a test backfill PR.

---

Test Classification Details

All three tests cover behavioral contracts for processMessages's artifact URL resolution feature, specifically the artifactUrlMap output field:

Test 1 — should populate artifactUrlMap when upload_artifact succeeds
Verifies the baseline contract: when an upload_artifact message is processed, the result's artifactUrlMap contains a temporaryId → artifactUrl entry. Mocking targets (vi.fn() for the upload handler) are external I/O (GitHub artifact upload) — legitimate use of mocks. No error paths covered.

Test 2 — should track issue with artifact reference as needing synthetic update when artifact is uploaded after
Covers the "late upload" ordering scenario: an issue body references #aw_chart1 but the artifact is uploaded in a subsequent message. The contract says outputsWithUnresolvedIds must track the issue for a post-processing synthetic update. This is the important edge case for cross-message reference resolution.

Test 3 — should replace artifact URL references in issue body when artifact is uploaded before issue
Covers the "early upload" ordering scenario (the complement of Test 2): the artifact is uploaded first, so by the time create_issue is processed the URL is already known and the body is pre-substituted. The test inspects mockCreateIssueHandler.mock.calls[0][0].body — this is checking the transformed message passed to the handler, which is the correct observable signal for this contract. Also asserts outputsWithUnresolvedIds.length === 0 (no post-processing needed).

---

Minor Suggestion

Test 1 could be strengthened by adding one error-path case (e.g., what happens when the upload_artifact handler returns without tmpId or artifactUrl). Tests 2 and 3 already cover the two meaningful ordering scenarios, so the behavioral contract is well-exercised overall.

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 0 tests
• 🟨 JavaScript (*.test.cjs, *.test.js): 3 tests (vitest)

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All three tests verify behavioral contracts for the artifactUrlMap artifact URL resolution feature in processMessages.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 1.4M · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 3 new tests verify behavioral contracts for artifact URL mapping in processMessages.