fix: typed error for workflows-scope rejection in review/fallback branch push

[Copilot]

When threat detection forces a review-branch push and the branch contains .github/workflows/*.lock.yml changes, GitHub's pre-receive hook rejects the push for missing workflows scope. The previous code used exec.exec(), which swallows stderr — the agent received only git exit code 1, logged nothing actionable, and silently discarded the changeset commit.

Changes

push_to_pull_request_branch.cjs

• isWorkflowsScopeRejection(stderr) — null-safe, case-insensitive helper matching GitHub's two rejection variants:
  • `workflows` scope may be required
  • workflow can be created or updated due to timeout
• buildWorkflowsScopeError(context, core) — emits two core.error() messages with remediation steps and returns { success: false, error_type: "workflows_scope_required", error: "..." }
• Both the threat-detection review branch push and the non-fast-forward fallback push are switched from exec.exec → exec.getExecOutput({ ignoreReturnCode: true }) so stderr is captured; matched rejections return buildWorkflowsScopeError(...) directly (bypasses the outer catch that would strip error_type)

push_to_pull_request_branch.test.cjs — 3 new tests in describe("threat detection: review branch push"):

• Timeout variant → error_type: "workflows_scope_required"
• Backtick variant → error_type: "workflows_scope_required"
• Unrelated push failure → generic catch, no typed error

// before
await exec.exec("git", ["push", "origin", reviewBranchName], { env: gitAuthEnv });

// after
const pushResult = await exec.getExecOutput("git", ["push", "origin", reviewBranchName], {
  ignoreReturnCode: true,
  env: gitAuthEnv,
});
if (pushResult.exitCode !== 0) {
  if (isWorkflowsScopeRejection(pushResult.stderr)) {
    return buildWorkflowsScopeError("Review branch", core);
  }
  throw new Error(`Review branch push failed:\n${pushResult.stderr}`);
}

The proper long-term fix is push-to-pull-request-branch.allow-workflows: true with a GitHub App carrying workflows: write — the error messages now explicitly direct operators there.

[github-actions]

Great work on this fix! 🎉 The typed error propagation for workflows-scope rejections in push_to_pull_request_branch.cjs is exactly the kind of improvement that makes agent-run failures diagnosable instead of silently swallowed.

The PR is well-structured: the root cause is clearly explained, both the review-branch push and the non-fast-forward fallback paths are covered, and the three new test cases in push_to_pull_request_branch.test.cjs cover the two rejection message variants plus the unrelated-failure pass-through. The before/after code snippet and the pointer to the long-term workflows: write GitHub App fix are a nice touch for operators hitting this.

This looks ready for review. 🚀

Generated by ✅ Contribution Check · 113.9 AIC · ⌖ 19.9 AIC · ⊞ 6K · ◷

[github-actions]

PR Triage — Run §28315307719

Category	bug
Risk	low
Priority	medium
Score	47/100 — impact 28 · urgency 8 · quality 11
Action	defer

⚠️ Draft PR. Fixes silent error swallowing when the workflows-scope rejection occurs during review/fallback branch push (+143/-3, 2 files). Ready to re-evaluate once marked ready for review.

Generated by 🔧 PR Triage Agent · 82.5 AIC · ⌖ 10.6 AIC · ⊞ 5.4K · ◷

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #42001 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

TQS analysis for PR #42001 already completed in this workflow run: comment posted and APPROVE review submitted (score 100/100, 3 design tests, 0 violations). No further action needed.

[Copilot]

Pull request overview

Improves push_to_pull_request_branch’s diagnostics when GitHub rejects a fallback/review-branch git push due to missing workflows permission, returning a typed error and logging actionable remediation steps instead of a generic push failure.

Changes:

• Add isWorkflowsScopeRejection(stderr) and buildWorkflowsScopeError(...) helpers to detect and report missing-workflows-scope push rejections.
• Switch review-branch and non-fast-forward fallback pushes from exec.exec() to exec.getExecOutput(..., { ignoreReturnCode: true }) to capture stderr and emit a typed failure.
• Add tests covering both known rejection variants plus a generic push failure case.

Show a summary per file

File	Description
actions/setup/js/push_to_pull_request_branch.cjs	Detects “workflows scope required” push rejections and returns a typed, actionable error from review/fallback push paths.
actions/setup/js/push_to_pull_request_branch.test.cjs	Adds regression tests for typed error behavior for review-branch push failures.

Review details

Tip

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

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Low

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 100/100 — Excellent

Analyzed 3 test(s): 3 design, 0 implementation, 0 guideline violation(s).

📊 Metrics & Test Classification (3 tests analyzed)

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	No (test/prod ratio: 86/57 = 1.51:1)
🚨 Coding-guideline violations	0

Test	File	Classification	Issues Detected
should return typed workflows_scope_required error ... (timeout variant)	push_to_pull_request_branch.test.cjs	✅ Design	—
should return typed workflows_scope_required error ... (backtick variant)	push_to_pull_request_branch.test.cjs	✅ Design	Sparse (2 assertions; could also assert error message content)
should wrap generic review branch push failure in actionable error message	push_to_pull_request_branch.test.cjs	✅ Design	—

Go: 0 (*_test.go); JavaScript: 3 (*.test.cjs). Other languages detected but not scored.

Notes on mocking: All three tests use vi.fn() to stub mockExec.getExecOutput and simulate GitHub's remote rejection responses. This mocks the external @actions/exec I/O layer — acceptable per project guidelines. Every test asserts on observable return values (result.success, result.error_type, result.error) and/or logger calls, not on internal call counts alone.

Verdict

✅ Check passed. 0% implementation tests (threshold: 30%). All three new tests verify behavioral contracts: (1) the timeout variant of GitHub's workflows-scope rejection produces a typed workflows_scope_required error with actionable message content; (2) the backtick variant produces the same typed error; (3) a generic push failure is correctly not typed as workflows_scope_required. The negative test (case 3) is particularly valuable for preventing future regressions in error discrimination.

🧪 Test quality analysis by Test Quality Sentinel · 71.1 AIC · ⌖ 14.4 AIC · ⊞ 8.2K · ◷

[github-actions]

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%).

[github-actions]

Skills-Based Review 🧠

Applied /tdd and /diagnose — the core fix is sound, with a meaningful test coverage gap for the fallback branch path.

📋 Key Themes & Highlights

Key Themes

• Fallback branch tests missing: isWorkflowsScopeRejection was added to both push paths, but only the review-branch path has new test coverage. The return buildWorkflowsScopeError("Fallback branch", core) code path is entirely untested.
• Timeout pattern not isolated: The "timeout variant" test input matches both includes() branches simultaneously, so the timeout-only code path in isWorkflowsScopeRejection is never exclusively exercised.
• Mock not restored: mockExec.getExecOutput is overridden in each test but never cleaned up — worth confirming or documenting vitest's reset behaviour for this shared object.
• Timeout pattern breadth: The second includes() check omits the word workflows, making it slightly broader than the first; low practical risk but easy to tighten.

Positive Highlights

• ✅ exec.exec → exec.getExecOutput + ignoreReturnCode: true is exactly the right pattern to capture stderr without relying on a thrown exception
• ✅ Typed error_type: "workflows_scope_required" with a direct return that bypasses the outer catch is a clean, intentional design
• ✅ buildWorkflowsScopeError actionable messages pointing operators to allow-workflows + GitHub App config are well-crafted
• ✅ Null-safe, case-insensitive matching in isWorkflowsScopeRejection is defensive
• ✅ Test 3 (generic auth failure → no typed error) provides solid negative coverage

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 86.9 AIC · ⌖ 10.2 AIC · ⊞ 6.6K

[gh-aw-bot]

@copilot please run the pr-finisher skill, address the review feedback on fallback-branch test coverage, isolate the timeout-only case, and restore/reset the exec mock between tests. If you push a follow-up commit, refresh the branch and rerun checks.

Generated by 👨‍🍳 PR Sous Chef · 70.2 AIC · ⌖ 1.04 AIC · ⊞ 17.1K · ◷