Skip to content

Handle deleted PR branches in push-to-pull-request-branch with opt-in skip mode#27208

Merged
pelikhan merged 2 commits intomainfrom
copilot/add-ignore-failure-on-deleted-branches
Apr 19, 2026
Merged

Handle deleted PR branches in push-to-pull-request-branch with opt-in skip mode#27208
pelikhan merged 2 commits intomainfrom
copilot/add-ignore-failure-on-deleted-branches

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 19, 2026

design-decision-gate was failing when push_to_pull_request_branch ran after the target PR branch had been deleted. This change adds an opt-in mode to treat missing/deleted branch push failures as skipped instead of terminal, and enables it for that workflow.

  • Safe-output behavior

    • Added ignore-missing-branch-failure to push-to-pull-request-branch.
    • When enabled, missing/deleted remote branch conditions return skipped: true instead of a hard failure.
    • Applies across branch existence checks (ls-remote, fetch/rev-parse failure paths) for consistent handling.
  • Compiler + schema wiring

    • Added schema support for safe-outputs.push-to-pull-request-branch.ignore-missing-branch-failure in main_workflow_schema.json.
    • Extended workflow parsing/config structs to read ignore-missing-branch-failure.
    • Propagated the parsed value into runtime handler config (ignore_missing_branch_failure).
  • Workflow enablement

    • Enabled in .github/workflows/design-decision-gate.md:
      safe-outputs:
        push-to-pull-request-branch:
          allowed-files:
            - docs/adr/**
          patch-format: bundle
          ignore-missing-branch-failure: true
    • Recompiled lock workflow to include the new handler config.
  • Maintainability adjustments

    • Centralized missing-branch error text/pattern matching in the JS handler to avoid duplicated logic and keep behavior uniform.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot rename to ignore-missing-branch

@pelikhan pelikhan marked this pull request as ready for review April 19, 2026 18:19
Copilot AI review requested due to automatic review settings April 19, 2026 18:19
@pelikhan pelikhan merged commit 7dd9f43 into main Apr 19, 2026
35 of 36 checks passed
@pelikhan pelikhan deleted the copilot/add-ignore-failure-on-deleted-branches branch April 19, 2026 18:19
Copilot stopped work on behalf of pelikhan due to an error April 19, 2026 18:20
Copilot AI requested a review from pelikhan April 19, 2026 18:20
@github-actions github-actions Bot mentioned this pull request Apr 19, 2026
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

Adds an opt-in mode for push-to-pull-request-branch to treat “PR branch missing/deleted” push failures as a skipped outcome (instead of failing the workflow), and enables it for design-decision-gate.

Changes:

  • Introduces ignore-missing-branch-failure in workflow YAML schema + Go compiler parsing, and emits ignore_missing_branch_failure into the runtime handler config.
  • Updates the JS handler to detect missing-branch conditions across ls-remote/fetch/rev-parse paths and return skipped: true when configured, with accompanying JS tests.
  • Enables the option in .github/workflows/design-decision-gate.md and recompiles the lock workflow.
Show a summary per file
File Description
pkg/workflow/push_to_pull_request_branch.go Adds config field and parsing for ignore-missing-branch-failure.
pkg/workflow/compiler_safe_outputs_handlers.go Wires the parsed flag into handler-manager JSON config (ignore_missing_branch_failure).
pkg/parser/schemas/main_workflow_schema.json Extends schema to allow ignore-missing-branch-failure under safe outputs config.
pkg/workflow/push_to_pull_request_branch_test.go Adds compiler/lockfile regression test for the new config flag.
actions/setup/js/push_to_pull_request_branch.cjs Implements opt-in missing-branch skip behavior and centralizes error matching.
actions/setup/js/push_to_pull_request_branch.test.cjs Adds tests covering skip behavior for deleted/missing branches.
.github/workflows/design-decision-gate.md Enables ignore-missing-branch-failure for the workflow.
.github/workflows/design-decision-gate.lock.yml Updates compiled workflow config to include the new handler flag.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

actions/setup/js/push_to_pull_request_branch.cjs:538

  • The git rev-parse --verify failure path is skipped for any error when ignore_missing_branch_failure is enabled. That can hide unrelated failures (e.g., unexpected git errors) as a missing/deleted branch. Consider checking the error text with looksLikeMissingRemoteBranchError(...) (or a tighter rev-parse-specific pattern set like "needed a single revision"/"unknown revision") before returning skipped: true; otherwise return the current hard failure.
      const missingBranchError = MISSING_BRANCH_ERROR_TEMPLATE(branchName);
      if (ignoreMissingBranchFailure) {
        core.warning(`${missingBranchError} Skipping as configured by ignore-missing-branch-failure.`);
        return { success: false, error: missingBranchError, skipped: true };
      }
  • Files reviewed: 8/8 changed files
  • Comments generated: 1

"did not match any file(s) known to git",
"unknown revision or path not in the working tree",
"fatal: couldn't find remote ref",
"exit code 128",
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

looksLikeMissingRemoteBranchError treats any error text containing "exit code 128" as a missing-branch signal. Exit code 128 is a generic git failure (auth, network, repo state, etc.), so with ignore_missing_branch_failure enabled this can incorrectly mark real failures as skipped. Remove this pattern (or replace it with more specific stderr substrings tied to missing refs) so only true missing-branch cases are skipped.

This issue also appears on line 534 of the same file.

Suggested change
"exit code 128",

Copilot uses AI. Check for mistakes.
@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 3
✅ Design tests (behavioral contracts) 3 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 3 (100%)
Duplicate test clusters 0
Test inflation detected ⚠️ Yes — Go test file (4.6:1 ratio, see note)
🚨 Coding-guideline violations 0

Test Classification Details

Test File Classification Issues Detected
TestPushToPullRequestBranchIgnoreMissingBranchFailureConfig pkg/workflow/push_to_pull_request_branch_test.go:96 ✅ Design None — verifies new config option compiles correctly into the lock file
should skip deleted branch failure when ignore_missing_branch_failure is enabled actions/setup/js/push_to_pull_request_branch.test.cjs:648 ✅ Design None — tests skip mode + warning emission for the pre-fetch deleted-branch path
should skip rev-parse missing branch failure when ignore_missing_branch_failure is enabled actions/setup/js/push_to_pull_request_branch.test.cjs:708 ✅ Design None — tests skip mode + warning emission for the rev-parse deleted-branch path

Test Inflation Note

The Go test file gained 37 lines against 8 new production lines in push_to_pull_request_branch.go (4.6:1 ratio). This triggers the inflation penalty mechanically, but the high ratio is expected here: the test follows the standard codebase pattern that requires creating a temp directory, writing a markdown file, compiling it, reading the lock file, and asserting the output — substantial boilerplate for a single new configuration field. The 10-point deduction reflects the rubric; it is not a concern in practice.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 1 test — unit (//go:build !integration) ✅
  • 🟨 JavaScript (*.test.cjs): 2 tests (vitest)

JavaScript mocking note: vi.fn() is used to stub @actions/core, @actions/exec, and github.rest.*. All mocked targets are external I/O interfaces (GitHub Actions runtime, git subprocess, GitHub API) — this is the appropriate pattern for this codebase.


Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All three new tests directly verify the observable behavioral contract of the ignore-missing-branch-failure opt-in feature: the Go test confirms the config is compiled correctly into the workflow's handler JSON, and the two JavaScript tests confirm the runtime skip-with-warning behavior for both deleted-branch code paths.


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

🧪 Test quality analysis by Test Quality Sentinel · ● 995.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%). All 3 new tests enforce observable behavioral contracts for the ignore-missing-branch-failure feature.

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