Report an error when a /backport comment doesn't match the expected format

[Copilot]

Comments like /backport release/9.0 (missing to) or /backport with no branch produced no bot response at all — either the job never triggered (the condition required a literal /backport with trailing space) or the extractor threw without commenting. Users were left waiting for a backport that never started.

Changes

• Job trigger — matches /backport instead of /backport , so bare or newline-separated commands reach the workflow.
• Command parsing — only lines starting with /backport count as a command; the to keyword is stripped explicitly and the following token is validated against the allowed branch character set.
• Error feedback — an unparseable command posts a comment stating the expected format and fails the run, rather than failing silently.
• Non-commands — a comment that merely mentions /backport mid-sentence (or /backporting) yields an empty result; the remaining steps are gated on that output so the run is a no-op with no comment.

Behavior by comment body:

Comment	Before	After
/backport to release/9.0	backport runs	unchanged
/backport release/9.0	backport runs	comment with expected format, run fails
/backport to	job runs, throws, no comment	comment with expected format, run fails
/backport	job never triggers	comment with expected format, run fails
see the /backport docs	job triggers, throws, no comment	no-op

Note the stricter validation: a malformed token such as release/9.0; echo hi was previously prefix-matched and backported to release/9.0; it is now reported as an error.

To double check:

• The right tests are in and the right validation has happened. Guidance: https://github.com/dotnet/arcade/blob/main/Documentation/Validation.md

There is no test harness for these workflows; the parsing logic was exercised standalone against the comment forms in the table above.

• Fixes Backport automation should report an error when expected format is not used #15860

[Copilot]

Copilot wasn't able to review any files in this pull request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The invalid-format path posts a comment before the unlock/re-lock logic, which can prevent error reporting on locked PRs and undermine the core goal of the change.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The updated parser behavior conflicts with the PR’s documented command grammar, and the /backport detection regex can fail to recognize end-of-line commands under CRLF line endings.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

.github/workflows/backport-base.yml:81

• The parser currently only accepts /backport to <target-branch> (two arguments with a literal to). This conflicts with the PR description table stating /backport release/9.0 remains supported/unchanged; with the current logic that form will be treated as invalid_command and will fail the run. Please clarify the intended grammar and either update the parser to match the documented behavior or update the PR description/expectations accordingly.

          const command_arguments = command[1].trim().split(/\s+/).filter(argument => argument.length > 0);
          const target_branch = command_arguments.length === 2 && command_arguments[0] === 'to'
            ? command_arguments[1]
            : '';

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[baronfel]

@copilot respond to review feedback

[Copilot]

🔵 Needs a closer look

There’s a mismatch between documented vs implemented command grammar, and the locked-PR unlock/re-lock flow for invalid commands needs alignment to avoid unintended side effects or missed feedback.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/backport-base.yml:119

• The failure message is now used for any invalid /backport command (missing branch, missing to, invalid characters), but it still says "No backport branch found". Updating it to mention the expected format would make the workflow failure easier to understand when looking at the Actions run.

.github/workflows/backport-base.yml:81

• The extractor currently only accepts /backport to <target-branch> (it requires two arguments where the first is to). The PR description/table says /backport <target-branch> remains supported and should behave the same as before; with the current parsing it will be treated as invalid and fail the run. Please either update the intended behavior/documentation or relax parsing to accept the one-argument form if that’s still desired.

          const command_arguments = command[1].trim().split(/\s+/).filter(argument => argument.length > 0);
          const target_branch = command_arguments.length === 2 && command_arguments[0] === 'to'
            ? command_arguments[1]
            : '';

.github/workflows/backport-base.yml:92

• This step unlocks PR comments not only for a successful parse, but also for an invalid command (invalid_command == 'true'). That means a malformed /backport will temporarily unlock/re-lock a PR just to post an error, which conflicts with the earlier guidance to only post feedback when the PR isn’t locked. Consider limiting unlock/re-lock to actual backport runs (successful parse) and gating invalid-command feedback on the PR being unlocked.

      if: ${{ github.event.issue.locked && (fromJSON(steps.target-branch-extractor.outputs.parsing_succeeded) || fromJSON(steps.target-branch-extractor.outputs.invalid_command)) }}

.github/workflows/backport-base.yml:103

• If you limit the unlock/re-lock steps to successful parses, this invalid-command comment should also be gated on the PR being unlocked; otherwise createComment will fail on locked PRs and users still won’t get feedback.

      if: ${{ fromJSON(steps.target-branch-extractor.outputs.invalid_command) }}

.github/workflows/backport-base.yml:429

• Re-locking is currently triggered for invalid commands as well. If the workflow no longer unlocks for invalid commands, this should be restricted to the cases where the workflow actually unlocked (successful parse) to avoid trying to lock an issue that was never unlocked by this run.

      if: ${{ github.event.issue.locked && (fromJSON(steps.target-branch-extractor.outputs.parsing_succeeded) || fromJSON(steps.target-branch-extractor.outputs.invalid_command)) && (success() || failure()) }}

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The PR description’s documented behavior conflicts with the implemented parser (and there’s an actionable mismatch in the failure message for invalid commands).

Review details

Suppressed comments (2)

.github/workflows/backport-base.yml:119

• The failure message doesn’t match the new parsing/validation behavior: invalid_command can mean missing to, extra tokens, or invalid characters, not just “no branch found”. Align core.setFailed(...) with the user-facing guidance so the run output is self-explanatory (especially when the comment-posting step fails for any reason).

    - name: Fail invalid backport command
      uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
      if: ${{ fromJSON(steps.target-branch-extractor.outputs.invalid_command) && (success() || failure()) }}
      with:
        script: |
          core.setFailed("Error: No backport branch found in the trigger phrase.");

.github/workflows/backport-base.yml:81

• This parser currently requires the to keyword (command_arguments.length === 2 && command_arguments[0] === 'to'). That means a comment like /backport release/9.0 will now be treated as invalid_command (comment + failed run), which contradicts the PR description/behavior table stating that form should remain accepted/unchanged. Either the description/table needs updating to reflect the stricter grammar, or the parser should be adjusted to accept both /backport <branch> and /backport to <branch>.

          const command_arguments = command[1].trim().split(/\s+/).filter(argument => argument.length > 0);
          const target_branch = command_arguments.length === 2 && command_arguments[0] === 'to'
            ? command_arguments[1]
            : '';

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[baronfel]

🔵 Needs a closer look

The PR description’s documented behavior conflicts with the implemented parser (and there’s an actionable mismatch in the failure message for invalid commands).
Review details

Suppressed comments (2)

.github/workflows/backport-base.yml:119

* The failure message doesn’t match the new parsing/validation behavior: `invalid_command` can mean missing `to`, extra tokens, or invalid characters, not just “no branch found”. Align `core.setFailed(...)` with the user-facing guidance so the run output is self-explanatory (especially when the comment-posting step fails for any reason).

    - name: Fail invalid backport command
      uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
      if: ${{ fromJSON(steps.target-branch-extractor.outputs.invalid_command) && (success() || failure()) }}
      with:
        script: |
          core.setFailed("Error: No backport branch found in the trigger phrase.");

.github/workflows/backport-base.yml:81

* This parser currently _requires_ the `to` keyword (`command_arguments.length === 2 && command_arguments[0] === 'to'`). That means a comment like `/backport release/9.0` will now be treated as `invalid_command` (comment + failed run), which contradicts the PR description/behavior table stating that form should remain accepted/unchanged. Either the description/table needs updating to reflect the stricter grammar, or the parser should be adjusted to accept both `/backport <branch>` and `/backport to <branch>`.

          const command_arguments = command[1].trim().split(/\s+/).filter(argument => argument.length > 0);
          const target_branch = command_arguments.length === 2 && command_arguments[0] === 'to'
            ? command_arguments[1]
            : '';

* **Files reviewed:** 1/1 changed files

* **Comments generated:** 0 new

* **Review effort level:** Lite

we should make the errors more readable, so point 1 is valid.
for point 2, we want to strengthen the parsing here, so that invalid syntax produces a clear error. So the description should be updated to align.

@copilot do both of these things.

[Copilot]

🔵 Needs a closer look

The PR description’s documented behavior conflicts with the implemented parser (and there’s an actionable mismatch in the failure message for invalid commands...

Addressed in 4e33e3f (check latest commit hash): the invalid-command comment and failure message now describe the specific reason parsing failed (missing to, wrong number of tokens/no branch, or invalid characters) instead of the generic "no branch found" text. The parser still requires to — I've updated the PR description/behavior table to reflect that /backport <branch> without to is intentionally rejected as invalid, per the stricter grammar.

🔵 Needs a closer look

The PR description’s documented behavior conflicts with the implemented parser (and there’s an actionable mismatch in the failure message for invalid commands...

Correction: the commit is b3df4c2, not the placeholder in my previous reply.

[Copilot]

🟡 Changes recommended

The new invalid-reason logic reports an incorrect message for /backport to (missing branch), and the workflow behavior is currently inconsistent with the PR description’s stated behavior table.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The workflow changes align with the described behavior matrix and correctly gate side effects based on parse outcomes while adding clear error feedback for invalid commands.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[akoeplinger]

Note that the to is not required anymore since #17361. This change regresses that.

[baronfel]

Oh heck yeah I had no idea. Will revert that part.