[jsweep] Clean add_reaction.cjs

[github-actions]

Summary

Cleaned actions/setup/js/add_reaction.cjs and followed up on review feedback with a small regression fix in the extracted control flow, plus a jsweep workflow update to use a deterministic git-based query when selecting outdated files for future cleanup runs.

Context

Execution context: github-script (uses core, github, context globals)

Changes

Refactoring

• Extracted resolveRestEndpoint(): replaces the inline switch that set let reactionEndpoint: string | undefined. The helper returns a string | null — null for GraphQL/unsupported events — eliminating the need for the undefined sentinel and the dead-code guard if (!reactionEndpoint) return.
• Extracted handleGraphQLOrUnknownEvent(): consolidates the discussion, discussion_comment, and default branches from the original switch into a focused async helper, keeping main() lean.
• Destructuring: const { owner, repo } = context.repo instead of two separate assignments.
• Exported new helpers: both new functions are exported so they can be unit-tested directly.

Follow-up fixes

• Prevented duplicate failure paths for malformed REST events: main() now returns after resolveRestEndpoint() validation failures for REST-backed events instead of falling through to the unsupported-event handler.
• Updated jsweep selection guidance: .github/workflows/jsweep.md now specifies a deterministic fallback query using git log -1 --format='%ct' -- <file> over candidates sorted by path, instead of filesystem modification timestamps.

Test Improvements

• Before: 43 tests
• After: 57 tests (+14)
• New resolveRestEndpoint (direct) suite: 9 tests covering each event type, missing-payload cases, and non-REST events returning null cleanly.
• New handleGraphQLOrUnknownEvent (direct) suite: 5 tests covering discussion, discussion_comment, missing payloads, and unsupported event fallback.
• Strengthened the existing malformed-REST-event test to verify only the original validation failure is reported.
• Added jsweep workflow coverage in pkg/workflow/jsweep_workflow_test.go for the deterministic fallback selection instructions.

Validation ✅

• Formatting: npm run format:cjs ✓
• Linting: npm run lint:cjs ✓
• Type checking: npm run typecheck ✓
• Tests: npx vitest run add_reaction.test.cjs — 57/57 passed ✓
• Workflow tests: go test ./pkg/workflow -run TestJSweepWorkflow ✓

> Generated by jsweep - JavaScript Unbloater · ● 21.2M · ◷
> - [x] expires on May 12, 2026, 5:03 AM UTC

[pelikhan]

/q update selection strategy using some deterministic query to find outdated files that need cleanup.

[github-actions]

🎩 Mission equipment ready! Q has optimized your workflow. Use wisely, 007! 🔫

[Copilot]

Pull request overview

Refactors actions/setup/js/add_reaction.cjs by extracting REST endpoint resolution and GraphQL/unsupported-event handling into exported helpers, and expands unit test coverage for the new helper functions.

Changes:

• Extracts resolveRestEndpoint() and handleGraphQLOrUnknownEvent() to simplify main() and enable direct unit testing.
• Updates control flow to use the new helpers (REST vs GraphQL paths).
• Adds new direct test suites for both helpers (increasing test count).

Show a summary per file

File	Description
actions/setup/js/add_reaction.cjs	Refactors event-to-endpoint logic into helpers and exports them.
actions/setup/js/add_reaction.test.cjs	Adds direct unit tests for the newly exported helper functions.

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: 1

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 77/100

⚠️ Acceptable — minor improvements suggested

Metric	Value
New/modified tests analyzed	14
✅ Design tests (behavioral contracts)	14 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	8 (57%)
Duplicate test clusters	0
Test inflation detected	⚠️ Yes (2.2:1 ratio)
🚨 Coding-guideline violations	None

---

Test Classification Details

View all 14 test classifications

Test	File	Classification	Issues Detected
should return issues endpoint	add_reaction.test.cjs	✅ Design	Happy path — verifies exact URL contract
should return null and setFailed when issue number is missing	add_reaction.test.cjs	✅ Design	Error case ✅
should return issue_comment endpoint	add_reaction.test.cjs	✅ Design	Happy path — verifies exact URL contract
should return null and setFailed when comment id is missing	add_reaction.test.cjs	✅ Design	Error case ✅
should return pull_request endpoint using issues path	add_reaction.test.cjs	✅ Design	Happy path — note: PR uses issues API path
should return null and setFailed when PR number is missing	add_reaction.test.cjs	✅ Design	Error case ✅
should return pull_request_review_comment endpoint	add_reaction.test.cjs	✅ Design	Happy path — verifies exact URL contract
should return null for discussion events (handled via GraphQL)	add_reaction.test.cjs	✅ Design	Boundary: null without setFailed ✅
should return null for unknown events	add_reaction.test.cjs	✅ Design	Boundary: unknown event ✅
should handle discussion event with GraphQL	add_reaction.test.cjs	✅ Design	Verifies GraphQL called 2×, output set
should setFailed when discussion number missing	add_reaction.test.cjs	✅ Design	Error case ✅
should handle discussion_comment event with GraphQL	add_reaction.test.cjs	✅ Design	Verifies mutation called with correct content
should setFailed when discussion_comment node_id missing	add_reaction.test.cjs	✅ Design	Error case ✅
should setFailed for unknown event types	add_reaction.test.cjs	✅ Design	Error case ✅

---

Flagged Tests — Requires Review

No tests are flagged for low behavioral value. The following is a minor improvement suggestion for the 6 happy-path endpoint tests that do not include an error sub-assertion:

i️ Happy-path endpoint tests (4 tests: issues, issue_comment, pull_request, pull_request_review_comment)

Classification: Design test — no action required
Observation: Each verifies a specific URL contract, which is high-value behavioral coverage. Consider adding a test for the pull_request_review_comment missing-comment-id error path (symmetric with the other event types) to complete the error coverage matrix.
Suggested addition (optional): A test for resolveRestEndpoint("pull_request_review_comment", ...) when payload.comment.id is absent.

---

Language Support

Tests analyzed:

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

---

Test Inflation Note

File	Test lines added	Production lines added	Ratio
add_reaction.test.cjs	108	49	2.2:1 ⚠️

The ratio slightly exceeds the 2:1 threshold. However, the new tests cover two previously untested exported helpers (resolveRestEndpoint and handleGraphQLOrUnknownEvent) that were refactored out of the main function in the production-code changes. The higher ratio is justified by the new surface area being tested for the first time and does not indicate test inflation in a problematic sense.

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All 14 new tests verify observable behavioral contracts — correct URL generation, error propagation via setFailed, and GraphQL output assertions. No coding-guideline violations detected. Mocking targets (mockGithub.graphql, mockCore) are external I/O, which is the acceptable pattern for this codebase.

---

📖 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.

References: §25620805627

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

[github-actions]

✅ Test Quality Sentinel: 77/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 14 new tests verify behavioral contracts across REST endpoint URL generation, error handling via setFailed, and GraphQL output assertions.

[github-actions]

Issue created: #31304

🎩 Equipped by Q

[github-actions]

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 2.9M

[pelikhan]

@copilot review all comments

[Copilot]

/q update selection strategy using some deterministic query to find outdated files that need cleanup.

Addressed in 93f176f: the jsweep fallback selection guidance now uses a deterministic git log -1 --format='%ct' -- <file> query over candidates sorted by path instead of filesystem mtimes.

@copilot review all comments

Addressed in 93f176f: I fixed the add_reaction.cjs REST validation fallthrough from the review thread, updated the regression coverage, and applied the jsweep deterministic-selection follow-up.