Skip to content

fix: revert multiline yaml parsing indentation change#638

Merged
zimeg merged 5 commits into
mainfrom
fix/637-js-yaml-multiline
Jul 11, 2026
Merged

fix: revert multiline yaml parsing indentation change#638
zimeg merged 5 commits into
mainfrom
fix/637-js-yaml-multiline

Conversation

@zimeg

@zimeg zimeg commented Jul 11, 2026

Copy link
Copy Markdown
Member

Summary

This pull request reverts js-yaml to 4.x to restore parsing of multiline payload strings, fixing a v3.0.4 regression.

  • js-yaml 5.x (bumped in build(deps): bump js-yaml from 4.2.0 to 5.2.0 #635) enforces strict YAML 1.2 flow-scalar indentation and throws deficient indentation when a double-quoted value wraps across a blank line onto a column-0 continuation.
  • A payload: | block strips its common indent, so keys and wrapped values land at column 0 — the exact shape of a multi-paragraph Slack message. Payloads that worked in v3.0.3 broke in v3.0.4.
  • No 5.x schema or option relaxes this (CORE_SCHEMA, YAML11_SCHEMA, and the default all throw identically), so this reverts js-yaml to ^4.2.0 and restores the @types/js-yaml devDependency.
  • Adds a unit regression test and a multiline-payload step in the integration workflow, both of which previously lacked coverage for this shape.

The first commit adds the failing regression test with production code unchanged (fails on 5.x); a later commit lands the revert that turns it green.

Fixes #637

Requirements

zimeg and others added 4 commits July 10, 2026 23:19
Adds a regression test for issue #637 using the multi-line quoted
payload shape that a `payload: |` block produces: keys at column 0
and a `text:` value wrapping across a blank line. This fails on
js-yaml 5.x (`deficient indentation` -> SlackError) with the
production code unchanged, proving the v3.0.4 regression.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
js-yaml 5.x adopts strict YAML 1.2 flow-scalar indentation and throws
`deficient indentation` on a double-quoted value that wraps across a
blank line onto a column-0 continuation line. A `payload: |` block
strips its common indent, so keys and wrapped values land at column 0
-- the exact shape of a multi-paragraph Slack message. Payloads that
worked in v3.0.3 broke in v3.0.4 (#635).

5.x offers no option or schema to relax this (CORE_SCHEMA, YAML11_SCHEMA
and the default all throw identically), so revert js-yaml to ^4.2.0 and
restore the @types/js-yaml devDependency. This makes the regression test
added in the previous commit pass.

Fixes #637

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The live-API suite only sent single-line `text:` values, so it shared
the unit tests' blind spot and would not have caught #637. Add a
chat.postMessage step whose `text:` wraps across a blank line onto a
column-0 continuation -- the shape a `payload: |` block produces for a
multi-paragraph Slack message -- so a future js-yaml bump that breaks
this parsing fails against the real API too.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@changeset-bot

changeset-bot Bot commented Jul 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c40c6bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@slack/slack-github-action Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (e1f137c) to head (c40c6bc).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #638   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files           7        7           
  Lines         730      730           
=======================================
  Hits          729      729           
  Misses          1        1           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zimeg zimeg self-assigned this Jul 11, 2026
@zimeg zimeg added this to the 3.1 milestone Jul 11, 2026
@zimeg zimeg added bug Something isn't working semver:patch labels Jul 11, 2026
- Drop "strings" from the unit test name.
- Reference issue #637 on the integration workflow step.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@zimeg

zimeg commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

🔭 Commit ba96dbd shows a similar error as #637 and the tests following 505dddf pass in CI!

        YAMLException: deficient indentation (4:1)
        
         1 | channel: C0123456789
         2 | text: "first line
         3 | 
         4 | second line"
        -----^
            at throwErrorAt (file:///home/runner/work/slack-github-action/slack-github-action/node_modules/js-yaml/dist/js-yaml.mjs:889:8)

@zimeg zimeg marked this pull request as ready for review July 11, 2026 06:44
@zimeg zimeg requested a review from a team as a code owner July 11, 2026 06:44
@zimeg

zimeg commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

🚀 Will fast merge to avoid ongoing CI errors!

@zimeg zimeg merged commit 96fddbe into main Jul 11, 2026
13 checks passed
@zimeg zimeg deleted the fix/637-js-yaml-multiline branch July 11, 2026 06:47
zimeg added a commit that referenced this pull request Jul 13, 2026
Reconcile the v8 web-api/webhook upgrade with main's @slack/webhook SDK
refactor (#630) and js-yaml revert (#638).

- Converge webhook.js on the @slack/webhook v8 IncomingWebhook and
  WebhookTrigger clients with retryConfig, dropping the rc.1-era
  customFetch/userAgent workaround now that rc.2 exports addAppMetadata.
- Extract a shared, side-effect-free proxiedFetch factory into
  src/proxies.js and inject it into both the WebClient and the webhook
  clients so proxy support stays testable without global dispatchers.
- config.js instruments both the web-api and webhook clients via
  addAppMetadata.
- Tests assert against the SDK send() clients across the webhook,
  config, send, and client suites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working semver:patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[3.0.4][Regression] SlackError: Invalid input! Failed to parse contents of the provided payload

1 participant