Skip to content

fix(cli): preserve verbatim contract bytes when splitting apply documents#3233

Open
javirln wants to merge 1 commit into
chainloop-dev:mainfrom
javirln:dry-run-contract
Open

fix(cli): preserve verbatim contract bytes when splitting apply documents#3233
javirln wants to merge 1 commit into
chainloop-dev:mainfrom
javirln:dry-run-contract

Conversation

@javirln

@javirln javirln commented Jun 23, 2026

Copy link
Copy Markdown
Member

Description

After a clean apply, re-running chainloop apply -f <contracts> --dry-run reported a contract as "would be updated" even though nothing changed, while chainloop wf contract apply correctly reported it "unchanged".

The cause was entirely client-side: the batch apply path re-serialized each YAML document through a yaml.v3 Node (yaml.Marshal(&node)), which re-indented and reflowed the content. That produced different bytes than wf contract apply, which sends the file verbatim. Since the server detects changes by comparing the stored raw bytes against the incoming ones, the reformatted bytes always looked like a change.

SplitYAMLDocuments now splits the stream on document markers and keeps each document's original bytes verbatim, so every apply path sends exactly what the user authored. Re-applying the same file is idempotent regardless of path, and genuine edits — including comment or formatting-only changes — still produce a new revision. No server-side change is needed; raw-byte change detection is left as-is.

Closes PFM-6465

AI disclosure

Claude Code assisted in producing this change.

@chainloop-platform

chainloop-platform Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

AI Session Analysis

Avg score Sessions Failing policies Attribution Files Lines Total Duration
🟡 82% 1 ✅ 0 100% AI / 0% Human 3 +150 / -17 433h22m43s

🟡 82% — 100% AI — ✅ All policies passing

Jun 23, 2026 08:05 UTC · 433h22m43s · $39.06 · 87.7k in / 199.2k out · claude-code 2.1.186 (claude-opus-4-8)

View session details ↗

Change Summary

  • Preserves verbatim YAML document bytes in SplitYAMLDocuments so batch apply matches wf contract apply.
  • Adds CLI regression tests for single-doc, multi-doc, comment-preserving, and validation-error cases.
  • Reverts the abandoned semantic-comparison approach, leaving server raw-byte checks in place with a clarifying comment.

AI Session Overall Score

🟡 82% — Verified client-side fix, but it lands as the narrower fallback after review feedback.

AI Session Analysis Breakdown

🟢 92% · alignment

No notes.

🟢 90% · scope-discipline

🟢 AI trimmed the abandoned server-side work back to a focused 3-file diff. · High Impact

🟢 88% · context-and-planning

🟢 Opening prompt supplied root cause, fallback path, constraints, and live verification steps. · High Impact

🟢 82% · user-trust-signal

🟡 The user had one sharp "don't answer just fix it" correction, but the session recovered immediately. · Low Severity

🟡 78% · verification

🟢 New splitter tests ran red-to-green, then broader suites passed after the pivot. · High Impact

🟠 Local tests passed, but nobody re-ran the user-facing scenario or confirmed the post-pivot behavior before publish. · Medium Severity

💡 Before publishing a reworked fix, rerun the user-facing scenario once and capture the observed result.

🟡 72% · solution-quality

🟠 The shipped fix is the user-authorized CLI fallback, so formatting drift outside these client paths still relies on raw-byte comparison. · Medium Severity

💡 When review feedback forces a fallback, state the remaining gap explicitly and leave a follow-up ticket for the broader fix.


File Attribution

████████████████████ 100% AI / 0% Human

Status Attribution File Lines
created ai app/cli/pkg/action/apply_test.go +110 / -0
modified ai app/cli/pkg/action/apply.go +37 / -16
modified ai app/controlplane/pkg/biz/workflowcontract.go +3 / -1

Policies (4)

Status Policy Material Messages
✅ Passed ai-config-ai-agents-allowed ai-coding-session-606de1 -
✅ Passed ai-config-no-dangerous-commands ai-coding-session-606de1 -
✅ Passed ai-config-no-secrets ai-coding-session-606de1 -
✅ Passed ai-config-mcp-servers-allowed ai-coding-session-606de1 -

Powered by Chainloop and Chainloop Trace

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

@migmartri migmartri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, this looks strange, shouldn't we take a look at why different clients produce different outputs? at the end, we have the raw content.

Also, does this mean that if I add a comment now in the yaml it will not be saved because the marshalled output is the same?

@javirln javirln marked this pull request as draft June 24, 2026 14:28
@javirln javirln force-pushed the dry-run-contract branch from e569b5a to 9b292af Compare July 11, 2026 09:15
…ents

The batch `apply` path re-serialized each YAML document through a yaml.v3
Node, which re-indented and reflowed the content. This produced different
bytes than `wf contract apply` (which sends the file verbatim), so the
server's raw-byte change detection reported spurious contract revisions and
false drift on `apply --dry-run` for a contract that had not changed.

SplitYAMLDocuments now splits the stream on document markers and keeps each
document's original bytes verbatim, so every apply path sends exactly what
the user authored. Re-applying the same file is idempotent regardless of
path, while genuine edits (including comment or formatting changes) still
produce a new revision.

Assisted-by: Claude Code
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>

Chainloop-Trace-Sessions: 606de1a5-e4ab-480b-8009-722416f5e18d
@javirln javirln force-pushed the dry-run-contract branch from 9b292af to 24f9d89 Compare July 11, 2026 09:28
@javirln javirln changed the title fix(controlplane): compare contracts semantically for change detection fix(cli): preserve verbatim contract bytes when splitting apply documents Jul 11, 2026
@javirln

javirln commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

Posted by an AI agent (Claude Code) on behalf of @javirln.

Thanks for the pushback @migmartri — you were right on both counts, so I've reworked the PR to fix it client-side instead of changing the server comparison.

On "why do different clients produce different outputs? at the end we have the raw content": it wasn't really two different clients — it was the same file serialized two ways by the CLI. wf contract apply sends the file verbatim (os.ReadFile), while the batch apply path ran each document through SplitYAMLDocuments, which decoded into a yaml.Node and re-marshalled it (yaml.Marshal(&node)), re-indenting and reflowing the content. That reformatted output is what diverged from the stored bytes.

On "if I add a comment now, will it not be saved because the marshalled output is the same?": exactly the reason I dropped the semantic-comparison approach. That would have ignored comment/formatting-only edits and skipped the revision. We don't want that.

New approach: SplitYAMLDocuments now splits the stream on document markers and keeps each document's original bytes verbatim, so every apply path sends exactly what the user authored. The server keeps its existing raw-byte change detection unchanged — no server-side change at all. Result:

  • re-applying the same file is idempotent regardless of path (wf contract apply / apply / apply --dry-run);
  • genuine edits, including comment- or whitespace-only changes, still produce a new revision.

Added CLI unit tests covering verbatim single/multi-document preservation, comment-only-document skipping, and the missing kind/metadata.name errors.

PTAL when you get a chance.

@javirln javirln marked this pull request as ready for review July 11, 2026 09:31
@javirln javirln requested review from a team and migmartri July 11, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants