Skip to content

Ignore link_sub_issue failure when already linked to same parent#27735

Merged
pelikhan merged 4 commits intomainfrom
copilot/ignore-failure-sub-issue
Apr 22, 2026
Merged

Ignore link_sub_issue failure when already linked to same parent#27735
pelikhan merged 4 commits intomainfrom
copilot/ignore-failure-sub-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 22, 2026

  • Reproduced and inspected the failing workflow/job logs for sub-issue linking
  • Located link_sub_issue handler behavior causing failure on already-linked sub-issues
  • Update link_sub_issue logic to treat "already linked to the same parent" as a successful no-op
  • Add/adjust unit tests for same-parent vs different-parent existing link cases
  • Run targeted JavaScript tests for link_sub_issue
  • Run required validation (make agent-finish) before commit
  • Remove unrelated lockfile changes from the branch
  • Commit and push changes

Smoke CI scheduled run completed: https://github.com/github/gh-aw/actions/runs/24755774748

Generated by Smoke CI · ● 308.1K ·

Copilot AI and others added 3 commits April 22, 2026 01:29
This reverts commit 46cea52.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan April 22, 2026 01:37
@pelikhan pelikhan marked this pull request as ready for review April 22, 2026 02:04
Copilot AI review requested due to automatic review settings April 22, 2026 02:04
@pelikhan pelikhan merged commit b13c618 into main Apr 22, 2026
19 checks passed
@pelikhan pelikhan deleted the copilot/ignore-failure-sub-issue branch April 22, 2026 02:04
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

Adjusts link_sub_issue handling so that attempting to link a sub-issue that is already linked to the same parent is treated as a successful no-op rather than a failure.

Changes:

  • Update link_sub_issue to return { success: true, skipped: true } when the sub-issue is already linked to the requested parent.
  • Refine unit tests to distinguish “already linked to different parent” (failure) vs “already linked to same parent” (success + skipped).
Show a summary per file
File Description
actions/setup/js/link_sub_issue.cjs Returns success for the “already linked to same parent” case while preserving failure for “different parent” case.
actions/setup/js/link_sub_issue.test.cjs Adds coverage for same-parent no-op behavior and clarifies the different-parent failure test.

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 github-actions Bot mentioned this pull request Apr 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 90/100

Excellent test quality

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

Test Classification Details

Test File Classification Issues Detected
should succeed when sub-issue is already linked to the requested parent actions/setup/js/link_sub_issue.test.cjs:73 ✅ Design None
(rename only) should fail when sub-issue already has a different parent actions/setup/js/link_sub_issue.test.cjs:58 Name clarification only, no body change

Test Analysis

should succeed when sub-issue is already linked to the requested parent

What design invariant does this test enforce?
The idempotency contract for sub-issue linking: when a sub-issue is already linked to the same requested parent, the operation must succeed (result.success = true, result.skipped = true) without issuing a redundant addSubIssue mutation.

What would break if deleted?
A regression that reverts the behavior back to returning a failure (success: false) for the same-parent case would go undetected. The test directly maps to the 8-line production change introduced in this PR.

Quality observations:

  • Verifies observable return values (result.success, result.skipped) — high-value behavioral assertions
  • Asserts that the GraphQL mutation addSubIssue was not called — correctly guards against unnecessary API side effects
  • Checks that the warning message is emitted for observability
  • Asserts graphql call count = 1 (only the parent-check query, not the mutation)

The sole 10-point deduction comes from the marginal 2.1:1 test-line-to-production-line ratio (17 test additions / 8 production additions). This is borderline — the extra lines are blank lines and closing braces structurally required by the test format, and the actual assertion density is healthy.


Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 1 new test (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). The new test directly verifies the behavioral contract introduced by the PR — idempotent sub-issue linking when already linked to the correct parent.


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

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

Copy link
Copy Markdown
Contributor

@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: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The single new test directly verifies the behavioral contract introduced by this PR: idempotent sub-issue linking when already linked to the correct parent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants