595 FIX Render heredoc diffs by gutter, not leading dashes#596
Merged
joshpollara merged 2 commits intoJul 24, 2026
Merged
Conversation
format_diff promoted any leading +/-/~ to column 0 so the diff syntax highlighter would colour the line. YAML list items inside a Terraform heredoc value (<<-EOT ... EOT) start with "- ", so they were mis-coloured as removals in both the UI plan view and PR comments. OpenTofu renders a changed heredoc body as a line-by-line diff with a fixed 2-char gutter: real +/- markers sit in the gutter, two columns left of the value content. Detect that gutter and promote only the markers that sit in it, leaving YAML content (including its "- " list items) verbatim. This keeps genuine add/remove colouring inside heredocs while no longer flagging unchanged list items as removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chris-downs
marked this pull request as ready for review
June 3, 2026 20:55
Contributor
|
@chris-downs Thank you! Will take a look this week. |
chris-downs
marked this pull request as draft
June 5, 2026 21:05
Contributor
Author
|
I found an edge case that isn't covered by this fix, so I'm putting this PR back in draft until it's figured out. |
…anges When a heredoc string value changes in place, Terraform renders its body as a line-by-line diff with a gutter prepended to every line, so real +/- markers sit to the left of the YAML content (including its `- ` list items). When a value is added or removed wholesale, the body is verbatim YAML with no gutter. Find the gutter as the shallowest body indent, treating the body as unchanged when a content line occupies that column. Emit the body verbatim when the heredoc opener itself is a +/- (whole-value add/remove); only an in-place (~) change renders the body as a line-by-line diff. This keeps genuine add/remove colouring inside heredocs without flagging unchanged YAML list items as removed. Add test_engine_tf.py covering markers outside heredocs, unchanged bodies, in-place gutter diffs, nested YAML lists, whole-value add/remove, and multiple/custom delimiters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chris-downs
marked this pull request as ready for review
June 9, 2026 20:48
Contributor
Author
|
@joshpollara we've been using this fork on our runner for a couple of weeks now with good results. It is ready for your review whenever you get a chance. Tests have also been added. |
Contributor
|
@chris-downs Hey sorry for the late reply here... I'm going to pull this in. Thank yo. |
joshpollara
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #595
Problem
When a Terraform/OpenTofu plan contains a heredoc with YAML inside (e.g. a
helm_releaseorargocd_applicationresource), the-that begins a YAML list item is interpreted as a removal by the diff formatting in the runner. The mis-formatted text is stored aspayload['plan']and rendered in both PR comments and the UI, making the plan look like it is removing things it is not.This is the runner-side root cause behind terrateamio/terrateam#339, terrateamio/terrateam#528 and terrateamio/terrateam#1062.
Cause
format_diffinterrat_runner/engine_tf.pypromoted any leading+/-/~marker (with its indentation) to column 0 so adiffsyntax highlighter would colour the line. YAML list items start with-, which is indistinguishable from a Terraform removal marker, so they were coloured as removals.Fix
Terraform renders a changed heredoc body as a line-by-line diff with a fixed 2-character gutter: real
+/-markers sit in the gutter, two columns to the left of where the value content begins; unchanged lines have a blank gutter. YAML list items always sit at the content column or deeper, never in the gutter.format_diffnow detects that gutter and promotes only the markers that sit in it, leaving the heredoc value content (including its-list items) verbatim. This keeps genuine add/remove colouring inside heredocs while no longer flagging unchanged list items as removed. Non-heredoc lines keep the original behaviour.Gutter detection: a
+only ever appears as a gutter marker (YAML content never starts a line with+), so a+column is the gutter directly — this also handles a heredoc whose YAML root is a bare list. It falls back to (shallowest content line − 2) for removal-only bodies, and treats a body with no gutter marker as unchanged (emitted verbatim).Before / after
A
helmvalue heredoc with one added and one removed env var.Before — unchanged YAML list items shown as removals:
After — only the genuinely added/removed lines are marked:
Testing
Verified against a real
argocd_applicationend to end through PR comments and the UI: the two genuinely changed env vars are coloured (green add / red remove) and ~20 unchanged YAML list items are left uncoloured, with no false positives. Also checked: unchanged heredocs (emitted verbatim), non-heredoc resource diffs (unchanged behaviour), custom and multiple heredoc delimiters, and a YAML-root-list heredoc with an addition.Note: duplicate logic in the terrateam server
The same marker-promotion logic exists in the terrateam server repo as
Terrat_plan_diff.transform(code/src/terrat_plan_diff/terrat_plan_diff.ml), and has the same heredoc bug. From tracing the code, it appears to be referenced in a few places but not in the live render path for current PR comments or the UI:terrat_vcs_github_comment_templates/terrat_vcs_gitlab_comment_templatesexpose it as aplan_difftemplate transformer, but only the olderplan_complete.tmpluses| plan_diff. The activeplan_complete2.tmplrenders the runner's already-formattedplanfield raw in a ```diff fence.terrat_ui_js_service(the older js_of_ocaml UI) calls it, but the current iris UI highlights the runner'splanfield client-side with highlight.js.So this runner-side fix is what actually corrects the output today. The server copy may be worth fixing for consistency / the legacy paths, but I haven't confirmed whether those paths are still reachable — flagging it for maintainers who know the server better.
🤖 Generated with Claude Code