fix: strip surrounding quotes from item_number in resolveIssueNumber

[Copilot]

The Copilot agent non-deterministically wraps string values in extra quotes (e.g. "aw_blazwasm" instead of aw_blazwasm), causing resolveIssueNumber to fail — the isTemporaryId regex rejects the value and parseInt returns NaN.

Changes

• temporary_id.cjs — add quote-stripping immediately after trim() and before the existing #-prefix strip, using a backreference regex to ensure only matching quote pairs are removed:

const unquoted = /^(["'])(.+)\1$/.test(valueStr) ? valueStr.slice(1, -1) : valueStr;
const valueWithoutHash = unquoted.startsWith("#") ? unquoted.substring(1) : unquoted;

• temporary_id.test.cjs — four new cases: double-quoted temp ID, single-quoted temp ID, double-quoted numeric issue number, mismatched quotes (should not be stripped).

[Copilot]

Pull request overview

Fixes resolveIssueNumber failures when agent-provided item_number values are wrapped in extra matching quotes (single or double), by stripping those quotes before temporary-ID and numeric parsing.

Changes:

• Strip matching surrounding ' or " from valueStr inside resolveIssueNumber before removing an optional leading #.
• Add tests covering quoted temporary IDs, quoted numeric issue numbers, and mismatched-quote inputs (which should not be stripped).

Show a summary per file

File	Description
actions/setup/js/temporary_id.cjs	Adds matching-quote stripping to make resolveIssueNumber resilient to "aw_..." / 'aw_...' inputs.
actions/setup/js/temporary_id.test.cjs	Adds regression tests for quoted inputs and a mismatched-quote non-strip case.

Copilot's findings

Tip

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

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

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 68/100

⚠️ Acceptable, with suggestions

Metric	Value
New/modified tests analyzed	4
✅ Design tests (behavioral contracts)	4 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (25%)
Duplicate test clusters	0
Test inflation detected	⚠️ Yes (formal ratio 36/3 = 12:1 — see note)
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Issues Detected
should strip surrounding double quotes from temporary ID	actions/setup/js/temporary_id.test.cjs:490	✅ Design	Happy-path; no error assertions
should strip surrounding single quotes from temporary ID	actions/setup/js/temporary_id.test.cjs:499	✅ Design	Happy-path; no error assertions
should strip surrounding double quotes from numeric issue number	actions/setup/js/temporary_id.test.cjs:508	✅ Design	Happy-path; no error assertions
should not strip mismatched surrounding quotes from temporary ID	actions/setup/js/temporary_id.test.cjs:517	✅ Design	✅ Covers error/edge case

---

Notes

⚠️ Test Inflation Ratio (contextually benign)

The formal ratio of added lines in the test file vs. production file is 12:1 (36 test lines added, 3 production lines added), which exceeds the 2:1 threshold. However, this is misleading in context: the PR also removes 88 test lines and ~51 production lines as part of deleting the replaceArtifactUrlReferences function and simplifying hasUnresolvedTemporaryIds. The net change to the test file is actually −52 lines, meaning total test coverage shrank proportionally to the code. The inflation flag is a formula artefact here, not a sign of padding.

⚠️ Edge/Error Case Coverage (3 happy-path tests out of 4)

Three of the four new tests verify that quote-stripping succeeds. Only the fourth (mismatched quotes) exercises an error path. Consider adding:

• Empty string after stripping: resolveIssueNumber('""', map) — quote-stripped value becomes "", which parses to an empty string. Does this produce a sensible error?
• Only open quote: resolveIssueNumber('"aw_foo123', map) — single unmatched leading quote, which is different from a mismatched pair.

These are minor gaps; the core contract (quoting behavior) is well covered for the typical cases.

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs): 4 tests (vitest)
• 🐹 Go (*_test.go): 0 tests changed in this PR

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All four tests verify observable output of resolveIssueNumber and constitute genuine behavioral contracts. The test inflation flag is a formula artefact due to simultaneous deletion of unrelated tests; no actual padding is present.

---

📖 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.2M · ◷

[github-actions]

✅ Test Quality Sentinel: 68/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All four new tests verify observable behavioral contracts of the resolveIssueNumber quote-stripping fix.