Skip to content

[jsweep] Clean dispatch_repository.cjs#28397

Merged
pelikhan merged 2 commits intomainfrom
jsweep/dispatch-repository-42b02ac678e66c15
Apr 25, 2026
Merged

[jsweep] Clean dispatch_repository.cjs#28397
pelikhan merged 2 commits intomainfrom
jsweep/dispatch-repository-42b02ac678e66c15

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Summary

Cleaned actions/setup/js/dispatch_repository.cjs as part of the daily jsweep unbloat process.

Context

File type: github-script context (uses core, context, github globals)

Changes Made

dispatch_repository.cjs

  • Used error_codes.cjs constants instead of raw string literals ("E001", "E099") — imports SAFE_OUTPUT_E001 and SAFE_OUTPUT_E099 for consistent, centralized error codes
  • Replaced manual for-of loop with spread operator when merging message.inputs into clientPayload, making the intent clearer and more concise:
    // Before
    if (message.inputs && typeof message.inputs === "object") {
      for (const [key, value] of Object.entries(message.inputs)) {
        clientPayload[key] = value;
      }
    }
    // After
    ...(message.inputs && typeof message.inputs === "object" ? message.inputs : {}),

dispatch_repository.test.cjs (new file)

Added 18 comprehensive tests covering:

  • Factory initialization (returns handler function, logs config)
  • Validation errors: missing tool_name, unknown tool, no target repository, invalid repo slug, missing event_type
  • Max count enforcement (per-tool counters)
  • Successful dispatch with correct API calls
  • message.inputs propagated to client_payload
  • aw_context injected into client_payload
  • message.repository override over toolConfig.repository
  • Default to first allowed_repositories entry
  • API error handling
  • Staged mode returns preview without dispatching
  • Numeric max parsed from string config

Validation ✅

  • Formatting: npm run format:cjs
  • Linting: npm run lint:cjs
  • Type checking: npm run typecheck
  • Tests: 18/18 passed (npx vitest run dispatch_repository.test.cjs) ✓

Generated by jsweep - JavaScript Unbloater · ● 3.4M ·

  • expires on Apr 27, 2026, 4:39 AM UTC

- Use error_codes.cjs constants (SAFE_OUTPUT_E001, SAFE_OUTPUT_E099) instead of raw string literals
- Replace manual for-of loop with spread operator when building clientPayload from message.inputs
- Add comprehensive test file with 18 tests covering all handler paths

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review April 25, 2026 10:17
Copilot AI review requested due to automatic review settings April 25, 2026 10:17
@github-actions
Copy link
Copy Markdown
Contributor Author

🧪 Test Quality Sentinel Report

Test Quality Score: 73/100

⚠️ Acceptable, with suggestions

Metric Value
New/modified tests analyzed 18
✅ Design tests (behavioral contracts) 18 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 14 (78%)
Duplicate test clusters 0
Test inflation detected Yes (251 lines test / 7 lines production = 35:1)
🚨 Coding-guideline violations None

Test Classification Details

View all 18 test classifications
Test File Classification Issues Detected
should return a handler function dispatch_repository.test.cjs ✅ Design Happy path only
should return a handler function with no config dispatch_repository.test.cjs ✅ Design Edge case (no config)
should log initialization info dispatch_repository.test.cjs ✅ Design Happy path only
should return error when tool_name is missing dispatch_repository.test.cjs ✅ Design Error path ✅
should return error when tool_name is whitespace-only dispatch_repository.test.cjs ✅ Design Edge case ✅
should return error when tool is not configured dispatch_repository.test.cjs ✅ Design Error path ✅
should return error when max count is reached dispatch_repository.test.cjs ✅ Design Boundary case ✅
should return error when no target repository is configured dispatch_repository.test.cjs ✅ Design Error path ✅
should return error when repository slug is invalid dispatch_repository.test.cjs ✅ Design Validation error ✅
should return error when event_type is not configured dispatch_repository.test.cjs ✅ Design Error path ✅
should dispatch successfully with valid config dispatch_repository.test.cjs ✅ Design Happy path only
should include message inputs in client_payload dispatch_repository.test.cjs ✅ Design Happy path only
should inject aw_context into client_payload dispatch_repository.test.cjs ✅ Design Happy path only
should use message.repository over toolConfig.repository when both set dispatch_repository.test.cjs ✅ Design Edge case (override precedence) ✅
should default to first allowed_repository when no target given dispatch_repository.test.cjs ✅ Design Edge case ✅
should return error on API failure dispatch_repository.test.cjs ✅ Design Error path ✅
should return staged result when isStaged is true dispatch_repository.test.cjs ✅ Design Behavioral variant ✅
should parse numeric max from string config dispatch_repository.test.cjs ✅ Design Edge case (type coercion) ✅

Score Breakdown

Component Score Notes
Behavioral Coverage (40 pts) 40/40 100% design tests
Error/Edge Case Coverage (30 pts) 23/30 14/18 tests include an error path or edge case
Low Duplication (20 pts) 20/20 No duplicate clusters detected
Proportional Growth (10 pts) 0/10 251 test lines added vs 7 production lines (35:1 ratio)

i️ Note on inflation penalty: The 35:1 ratio triggers the inflation flag, but context matters — this is a new test file being added to a production file that previously had zero test coverage. The penalty is applied per rubric, but the addition of comprehensive tests for pre-existing code is a net positive.


Observations

The tests in this PR are high quality overall. All 18 tests assert on observable return values (result.success, result.error, result.repository, result.event_type, dispatchEventCalls) — none merely verify that an internal function was called. Mocking is appropriately scoped to external runtime dependencies (global.core, global.github, global.context, and handler_auth.createAuthenticatedGitHubClient), not internal business logic.

The 4 happy-path-only tests (should return a handler function, should return a handler function with no config, should log initialization info, should dispatch successfully with valid config) could be enhanced, but they verify distinct behavioral contracts and are not duplicates.


Language Support

  • 🟨 JavaScript (*.test.cjs): 18 tests (vitest)
  • 🐹 Go (*_test.go): 0 tests changed

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). No coding-guideline violations detected.


📖 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: §24928655226

🧪 Test quality analysis by Test Quality Sentinel · ● 363.6K ·

Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 73/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 18 new tests verify behavioral contracts with strong error/edge case coverage.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Cleans up the dispatch_repository safe-output handler by standardizing selected error code prefixes and simplifying how message.inputs is merged into the dispatch client_payload, and adds a new test suite for the handler.

Changes:

  • Use error_codes.cjs constants for E001/E099 error prefixes in dispatch_repository.cjs.
  • Replace the manual Object.entries loop with object spread when merging message.inputs into client_payload.
  • Add dispatch_repository.test.cjs with coverage for validation, dispatch behavior, staged mode, and max enforcement.
Show a summary per file
File Description
actions/setup/js/dispatch_repository.cjs Standardizes E001/E099 prefix usage and simplifies client_payload construction.
actions/setup/js/dispatch_repository.test.cjs Adds a dedicated Vitest suite covering common error paths and successful dispatch behavior.

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

Comment on lines +3 to +5

const { main } = require("./dispatch_repository.cjs");

Comment on lines +50 to +53
// Stub handler_auth to return the mock github client
const handlerAuth = require("./handler_auth.cjs");
vi.spyOn(handlerAuth, "createAuthenticatedGitHubClient").mockResolvedValue(mockGithub);
});
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 25, 2026

@copilot review all comments

Addressed both reviewer comments in commit 7e6c10e:

  1. Broken spy (dispatch_repository.test.cjs:3-5): Removed the vi.spyOn(handlerAuth, "createAuthenticatedGitHubClient") call. Since dispatch_repository.cjs destructures createAuthenticatedGitHubClient at module load time, the spy on the module export cannot affect the already-captured reference. The mock is also unnecessary: since no test config includes a github-token, createAuthenticatedGitHubClient already returns global.github — which is correctly set to mockGithub in beforeEach.

  2. Missing cleanup (dispatch_repository.test.cjs:50-53): Added afterEach that restores the original global.core, global.github, and global.context values (saved into savedGlobals at the start of each beforeEach) and calls vi.restoreAllMocks() to prevent cross-test leakage.

Copilot AI requested a review from pelikhan April 25, 2026 10:40
@pelikhan pelikhan merged commit 3bf15de into main Apr 25, 2026
@pelikhan pelikhan deleted the jsweep/dispatch-repository-42b02ac678e66c15 branch April 25, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants