Validate expression block-scalar chomping

[ericsciple]

Summary

Adds validation for multi-line expressions. Block scalars (| and >) add trailing newlines by default, which can cause unintentional expression results.

In fields where trailing newlines don't make sense (booleans, numbers, and many strings), the validation produces an error:

Boolean:

jobs:
  test:
    if: |                # ❌ Error: Block scalar adds trailing newline which breaks boolean evaluation. Use '|-' to strip trailing newlines.
      ${{ github.event_name == 'push' }}
    if: |-               # ✅ OK
      ${{ github.event_name == 'push' }}

Number:

jobs:
  test:
    timeout-minutes: |   # ❌ Error: Block scalar adds trailing newline which breaks number parsing. Use '|-' to strip trailing newlines.
      ${{ fromJSON(vars.TIMEOUT) }}
    timeout-minutes: |-  # ✅ OK
      ${{ fromJSON(vars.TIMEOUT) }}

String:

jobs:
  test:
    runs-on: |           # ❌ Error: Block scalar adds trailing newline. Use '|-' to strip trailing newlines.
      ${{ matrix.os }}
    runs-on: |-          # ✅ OK
      ${{ matrix.os }}

In other fields where newlines are unlikely, a warning is produced:

steps:
  - uses: actions/checkout@v4
    with:
      token: |           # ⚠️ Warning: Block scalar adds trailing newline to expression result. Use '|-' to strip or '|+' to keep trailing newlines.
        ${{ secrets.PAT }}

The run step is the exception to the rule, since trailing newlines don't matter:

steps:
  - run: |               # ✅ OK: trailing newlines don't matter
      echo "Commit: ${{ github.sha }}"

Testing

Added comprehensive test coverage across 6 test files organized by severity and field type:

• Error - Job and step if fields. Special because this is the only place ${{ }} is optional.
• Error - Boolean fields, e.g. continue-on-error
• Error - Number fields, e.g. timeout-minutes, ports, volumes
• Error - String fields, e.g. name, runs-on, container, environment
• Warning fields - Action inputs, reusable workflow inputs, etc
• Allowed - Only run

Each test validates behavior across different chomping indicators - e.g. |, |-, |+, >, etc.

[ericsciple]

I opened an issue and a corresponding PR with a proposed solution

[Copilot]

Pull request overview

This PR adds comprehensive validation for block scalar chomping indicators in GitHub Actions workflow expressions. Block scalars (| and >) add trailing newlines by default, which can break expression evaluation in fields expecting booleans, numbers, or single-line strings.

Key changes:

• Enhanced token classes to capture and propagate block scalar metadata (type and chomp style) from YAML parsing through expression validation
• Implemented validation logic that produces errors for boolean/number/string fields and warnings for other fields when improper chomping is used
• Added extensive test coverage across 6 test files covering different field types and severity levels

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
workflow-parser/src/workflows/yaml-object-reader.ts	Extracts block scalar header information from YAML tokens using undocumented internal YAML library APIs
workflow-parser/src/templates/tokens/string-token.ts	Extends StringToken to store scalar type and block scalar header metadata
workflow-parser/src/templates/tokens/basic-expression-token.ts	Extends BasicExpressionToken with scalar type and chomp style fields to enable validation
workflow-parser/src/templates/template-reader.ts	Parses block scalar headers to extract chomp indicators and propagates this information to expression tokens
languageservice/src/validate.ts	Implements validation logic that errors on improper chomping for booleans/numbers/strings and warns for other fields
workflow-parser/src/expressions.test.ts	Unit tests verifying block scalar metadata preservation across various chomping indicators
languageservice/src/validate.expressions-chomp-*.test.ts	Integration tests for validation across error/warning cases organized by field type

---

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

[Copilot]

The regex pattern /^(\||>)([-+])?(\d)?/ only matches chomp indicator before indentation (e.g., |-2), but YAML allows both orders: |-2 and |2-. The pattern should be /^(\||>)(?:([-+])(\d)?|(\d)([-+])?)$/ to correctly parse both |2- and |-2 formats.

Suggested change

	const blockScalarMatch = header.match(/^(\||>)([-+])?(\d)?/);
	if (!blockScalarMatch) {
	// Assume clip if we can't parse the indicator
	return {scalarType, chompStyle: "clip"};
	}
	const chompIndicator = blockScalarMatch[2];
	const blockScalarMatch = header.match(/^(\||>)(?:([-+])(\d)?|(\d)([-+])?)$/);
	if (!blockScalarMatch) {
	// Assume clip if we can't parse the indicator
	return {scalarType, chompStyle: "clip"};
	}
	// The chomp indicator may be in group 2 or 5 depending on the order
	const chompIndicator = blockScalarMatch[2] || blockScalarMatch[5];

[Copilot]

The volumes field contains an array of volume mount strings (e.g., /data:/data), not numbers. It should be categorized as a string field (line 283-306) rather than a number field. The error message "breaks number parsing" is incorrect for volumes.

[Copilot]

The code assumes scalarType is either BLOCK_LITERAL or BLOCK_FOLDED when determining the scalar indicator. For defensive programming, consider adding a check to ensure scalarType is defined, or adding a fallback case to handle unexpected values. For example: const scalarIndicator = token.scalarType === Scalar.BLOCK_LITERAL ? "|" : token.scalarType === Scalar.BLOCK_FOLDED ? ">" : "|";

Suggested change

	const scalarIndicator = token.scalarType === Scalar.BLOCK_LITERAL ? "|" : ">";
	const scalarIndicator =
	token.scalarType === Scalar.BLOCK_LITERAL
	? "|"
	: token.scalarType === Scalar.BLOCK_FOLDED
	? ">"
	: "|";

[ericsciple]

Actually we might be able to safely trim the trailing newlines on the server for all boolean cases.

[ericsciple]

Then we won't need an error here.

[ericsciple]

Again, for numbers we can likely safely trim trailing newlines on the server for all number cases

[ericsciple]

For all of these cases as well, we can likely safely trim trailing newlines on the server

[ericsciple]

For everything else, it's ambiguous. Warn and suggest the user specify keep or strip (clear intent)