Skip to content

feat(cli): surface conflict markers in env status#139

Merged
fentas merged 6 commits into
mainfrom
feat/env-status-conflicts
Apr 7, 2026
Merged

feat(cli): surface conflict markers in env status#139
fentas merged 6 commits into
mainfrom
feat/env-status-conflicts

Conversation

@fentas

@fentas fentas commented Apr 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • `b env status` now reports files containing git-style merge conflict markers as a distinct category
  • New line: `✗ N file(s) with unresolved conflict markers — run \`b env resolve\``
  • Scan is runs on every synced file (the previous "drift-bounded" wording was wrong: a conflicted file can match its lock SHA exactly so we have to scan unconditionally), so cost is proportional to actual problems

Why

Today, an unresolved conflict left by `b env update` is silently rolled into the generic "modified locally" counter, so consumers have no in-CLI signal that something needs human attention. This is the natural status-side complement to `b env resolve` (#138) and the broader #125 transparency push.

Note on overlap

`hasConflictMarkers` is intentionally inlined here. PR #138 (`b env resolve`) defines the same helper. When both merge, one side will need a trivial rebase to dedupe the function — flagging here so it's not a surprise.

Test plan

  • `go test ./pkg/cli/... -run HasConflictMarkers_StatusHelper`
  • full `go test ./...`
  • `golangci-lint run ./pkg/cli/...`

Refs #125 #138

🤖 Generated with Claude Code

Extends \`b env status\` to count files containing git-style merge
conflict markers as a distinct, actionable category. Today these
files are silently lumped into the generic \"modified locally\"
counter, so a consumer who got an unresolved conflict during the
last sync has no in-CLI signal that something needs human attention.

New status output:

  github.com/org/infra v2.0 @ abc1234
    ✎ 3 file(s) modified locally
    ✗ 1 file(s) with unresolved conflict markers — run \`b env resolve\`

The conflict scan only runs on files that already drifted from the
lock SHA, so the cost is bounded by drift count rather than the
full set of synced files.

Note: \`hasConflictMarkers\` is intentionally inlined here. PR #138
(\`b env resolve\`) defines the same helper, so when both merge one
side will need a trivial rebase to dedupe the function.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds explicit reporting of git-style merge conflict markers to b env status, complementing the upcoming b env resolve workflow and improving visibility for env-sync conflict states.

Changes:

  • Introduces a conflict-marker detector and counts conflicted env files during b env status.
  • Prints a new status line pointing users to b env resolve when conflicted files are found.
  • Adds a unit test for the conflict-marker helper.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
pkg/cli/env.go Adds conflict-marker detection and a new conflicted-files status line in EnvStatusOptions.Run().
pkg/cli/env_status_conflict_test.go Adds unit tests for the conflict-marker detection helper.

Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go
- Conflict detection no longer gated on hash drift. env update
  records the post-merge bytes (markers and all) into the lock
  SHA, so a conflicted file would match the lock and previously
  show as "✓ up to date". Now we scan EVERY synced file and
  block the up-to-date shortcut on conflictedFiles > 0.
- Single-pass hashAndScanConflicts streams the file once with an
  io.TeeReader, computing SHA-256 and the line-anchored marker
  scan together. Avoids the previous double read on drifted
  files and removes the need to load whole files into memory
  for the marker check.
- hasConflictMarkers is now line-anchored (bufio.Scanner with
  bytes.HasPrefix / bytes.Equal). A markdown rule like `=======`
  or a stray `<<<<<<<` inside a string literal can no longer
  trip the detector. The duplication with pkg/env/splice.go's
  containsConflictMarkers is acknowledged in a comment; merging
  them into one shared helper is a separate cleanup.
- New test cases cover the markdown-rule and string-literal false
  positives plus a sanity check that hashAndScanConflicts
  produces the same digest as crypto/sha256 over the file body.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env_status_conflict_test.go
…d 2)

- Replace bufio.Scanner with a state-machine conflictMarkerScanner
  that has no per-line memory cap. The previous 1 MiB token limit
  would silently fall back on files with very long lines (SOPS
  blobs, minified JSON, lockfiles). The state machine bounds
  per-line state at conflictPendingMax bytes regardless of file
  size, so a degenerate single long line is harmless.
- Tolerate CRLF endings: the scanner trims a trailing \\r before
  comparing the separator line, so \"=======\\r\\n\" files are
  detected correctly. Previously bytes.Equal(line, \"=======\")
  would miss those.
- hashAndScanConflicts switches to a chunked Read loop that feeds
  both sha256.Hash and the marker scanner — same single-pass
  semantics, no Scanner buffer cap.
- Update the comment that incorrectly said conflicted files
  \"still count as drift internally\". They explicitly DON'T:
  env update writes the post-merge bytes (markers and all) into
  the lock SHA, so a conflicted file may match its lock SHA
  exactly and the conflict counter is the actionable signal.
- New test cases cover the CRLF separator and a 200 KiB single
  line followed by a real conflict region.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread pkg/cli/env.go
Comment thread pkg/cli/env.go
…d 3)

The previous logic incremented both localDrift and conflictedFiles
when a file had markers AND its SHA differed from the lock. The
status message would then read both '✎ N modified locally' and
'✗ N with unresolved conflict markers' for the same file, which
is misleading. The conflict counter is the actionable signal;
treat conflicted files as ONLY conflicted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env.go Outdated
Comment thread pkg/cli/env_status_conflict_test.go Outdated
Comment thread pkg/cli/env_status_conflict_test.go Outdated
…d 4)

- Reword the two doc comments that said bufio.Scanner could
  "panic / fall back silently". The actual failure is
  Scan() returning false with bufio.ErrTooLong; it only goes
  silent if the caller skips scanner.Err(). Spell that out.
- Test comment no longer claims env status calls
  hasConflictMarkers — env status uses hashAndScanConflicts,
  and the standalone helper test covers the shared
  classification logic at unit level.
- Failure output for hasConflictMarkers cases now prints the
  case name instead of the (potentially 200 KiB) input string.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread pkg/cli/env.go
…und 5)

The per-line consumeLine path was creating a fresh []byte("...")
for each marker comparison on every line scanned. Hoist the three
marker slices to package-level vars so they're allocated once at
init and reused across every line of every file.

Pure micro-optimization but it shows up under heavy
b env status runs over many large files.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@fentas fentas merged commit 1e54c73 into main Apr 7, 2026
12 checks passed
@fentas fentas deleted the feat/env-status-conflicts branch April 7, 2026 19:52
fentas added a commit that referenced this pull request Apr 7, 2026
…rs (#138 round 7)

After #139 merged to main with its own hasConflictMarkers /
conflictPendingMax in env.go, this branch's duplicates collided.
Rebased on main, then:

- Renamed the local detector to hasResolvableConflictMarkers and
  the doc explains why it's intentionally distinct from env
  status's strict variant: env resolve needs the loose prefix
  (bare <<<<<<< / >>>>>>>) and unterminated-region detection so
  the cleanup command surfaces files left in a half-merged state
  by a partial manual edit. env status uses the strict variant so
  partial files don't show up as drifted.
- Bulk resolve now collects per-file errors instead of bailing
  on the first one. The lock is persisted at the end if anything
  changed (so a half-completed run doesn't leave the working tree
  resolved while the lock points at stale SHAs), then the
  aggregate error is returned.
- TestNewEnvCmd_Subcommands now exercises \`b env resolve --help\`
  so future help/flag/template regressions get caught.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fentas pushed a commit that referenced this pull request Apr 8, 2026
🤖 I have created a release *beep* *boop*
---


## [4.14.0](v4.13.0...v4.14.0)
(2026-04-08)


### Features

* **cli:** b env resolve for merge conflicts
([#125](#125) phase 4)
([#138](#138))
([b3d00c8](b3d00c8))
* **cli:** plan-json polish — destructive row in error + effectiveDryRun
helper ([#132](#132))
([4ad5dfa](4ad5dfa))
* **cli:** surface conflict markers in env status
([#139](#139))
([1e54c73](1e54c73))
* **env:** format-preserving byte-level YAML splice fast path
([#133](#133))
([94cd711](94cd711))
* **env:** JSON splice support for scoped selects
([#135](#135))
([89ccf9d](89ccf9d))
* **env:** orphan-file delete plumbing
([#125](#125) phase 3)
([#137](#137))
([5891781](5891781))
* **env:** per-key b.pin annotation
([#125](#125) phase 2)
([#136](#136))
([9ed59db](9ed59db))
* **env:** structural 3-way merge for YAML/JSON
([#134](#134))
([fd787b0](fd787b0))
* **env:** wrapKeyFor extracts leading identifier for filter expressions
([#131](#131))
([f542f28](f542f28))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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