feat(cli): b env resolve for merge conflicts (#125 phase 4) by fentas · Pull Request #138 · fentas/b

fentas · 2026-04-07T16:26:20Z

Summary

New `b env resolve` subcommand finds and bulk-resolves git-style merge conflict markers in synced env files
`--ours` keeps the local block, `--theirs` keeps the upstream block, no flag = list-only
Optional positional args restrict scope to specific env keys
Parser handles both diff3 form (which b writes) and bare 2-way form (for hand-edited files)
Returns errors on unterminated regions instead of producing garbage

Why

Phase 4 of issue #125 adds the resolution path: today, conflicts left by `b env update` are silently committed to the consumer's tree as raw `<<<<<<<` markers and the user has no `b`-native way to resolve them in bulk.

Out of scope (deliberate follow-up)

Phase 4 also covers writing in-place YAML comment markers (`# b: CONFLICT upstream=v1.31 base=v1.30 local=v1.32`) on the producer side. That conversion is its own PR — this command speaks the marker format that's actually written today, so it works on conflict regions left by current `b env update` runs.

Examples

```
$ b env resolve
github.com/org/infra → .bin/b.yaml

1 conflicted file(s). Re-run with --ours or --theirs to resolve in bulk, or edit them manually.

$ b env resolve --theirs github.com/org/infra
github.com/org/infra → .bin/b.yaml
→ resolved 2 region(s) in favor of upstream

Resolved 2 region(s) across 1 file(s).
```

Test plan

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

Refs #125

🤖 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

Pull request overview

Adds a new CLI subcommand to help users detect and bulk-resolve git-style merge conflict markers left in synced env files after b env update merges, with --ours / --theirs options and parser support for both diff3 and 2-way marker formats.

Changes:

Register b env resolve under the b env command.
Implement conflict-marker detection and bulk resolution logic.
Add unit tests for marker detection and resolution parsing.

Reviewed changes

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

File	Description
pkg/cli/env.go	Registers the new `env resolve` subcommand.
pkg/cli/env_resolve.go	Implements `b env resolve` command, marker detection, and resolution logic.
pkg/cli/env_resolve_test.go	Adds unit tests for conflict marker detection/resolution helpers.

- Path-traversal check on every dest before reading or writing. A malicious or hand-edited lockfile must not let \`b env resolve\` touch files outside the project root. Mirrors the same check the env sync / status / remove paths use. - Update b.lock with the new SHA after a successful resolve. Without this, the next sync / \`b verify\` would treat every resolved file as locally modified, defeating the point of the command. - Label support: env keys are now matched in the canonical ref#label form so labeled envs can be targeted unambiguously when multiple lock entries share the same ref. The status output also prints the labeled key. Integration tests are intentionally deferred to a follow-up; the existing unit tests cover hasConflictMarkers and resolveConflictMarkers, and a real CLI integration test needs a fixture lock + project directory that's substantial enough to warrant its own change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

…138 round 2) - Remove the --list flag and its unused EnvResolveOptions.List field. Listing is already the default behavior when no --ours/--theirs flag is set; the flag was a no-op pretending to be a feature. - New TestEnvResolveRun_RewritesFileAndUpdatesLock exercises the full Run flow against a temp project and lockfile, asserting the file's conflict markers are stripped AND the lock entry SHA is updated to match the new on-disk hash. - New TestEnvResolveRun_PathTraversalRejected verifies a malicious lock entry pointing outside the project root is rejected before any file I/O happens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…2.5) - After rewriting a conflicted file, lock.SHA256File errors are now propagated as a hard error. Previously a hashing failure silently left the lock pointing at the stale SHA, leaving the repo in a drifted state with no indication why. - envKey doc no longer claims to do canonical normalization. The helper is just TrimSpace; the matching loop in Run builds the canonical ref / ref#label form on the lock-entry side. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

…138 round 3) - hasConflictMarkers is now line-anchored (and CRLF-tolerant) so a marker substring inside a YAML string literal or a markdown rule can't trigger a false positive that makes Run() rewrite a clean file. - When resolveConflictMarkers returns n == 0 (false-positive detection on a pathological input), skip both the file write and the lock SHA update so we don't touch a file that doesn't actually need resolving. - The integration test now sets bVersion: "test" so the lock write doesn't stash an empty tool version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

…138 round 4) - hasConflictMarkers now uses a small state machine instead of bufio.Scanner, so files with very long lines (SOPS blobs, minified JSON, lockfiles) can't silently fall back the way ErrTooLong + ignored Err() would. Per-line state is bounded at 64 bytes regardless of file size. - In --ours/--theirs mode, defer the totalConflicts increment until after resolveConflictMarkers actually finds at least one region. A false-positive marker scan no longer counts toward the summary, so the final "Resolved N region(s) across M file(s)" output reflects what was actually actionable. - Reword the filter-arg comment to match envKey's actual TrimSpace behavior (not "canonical normalization"). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

Previously hasConflictMarkers required all three of <<<<<<<, =======, and >>>>>>> before reporting a conflict. That meant a file left in a half-merged state by a partial manual edit (start marker but no closing line) would slip past env resolve entirely. Treat the presence of a start marker as conflicted. List mode will surface the file; rewrite mode will hit resolveConflictMarkers' existing "unterminated conflict region" error path, which Run propagates as a hard error and the user fixes manually. Test cases extended with the unterminated region case and a stray-closing-marker negative case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

## 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 - [x] \`go test ./pkg/cli/... -run HasConflictMarkers_StatusHelper\` - [x] full \`go test ./...\` - [x] \`golangci-lint run ./pkg/cli/...\` Refs #125 #138 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

…round 6) hasConflictMarkers required \"<<<<<<< \" / \">>>>>>> \" with a trailing space, but resolveConflictMarkers below uses strings.HasPrefix(line, \"<<<<<<<\") with no trailing-space requirement. A hand-edited file with bare \"<<<<<<<\" / \">>>>>>>\" lines (no label suffix) would be resolvable but never detected, so env resolve would silently skip it. Loosen the detector prefix to match. New test case covers the bare-marker conflict shape. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

Adds \`b env resolve\` for finding and bulk-resolving git-style merge conflict markers left behind by \`b env update\` when a 3-way merge couldn't auto-merge a hunk. Behavior: - No flags: list every file in the lock that contains conflict markers, grouped by env - --ours: rewrite each file by keeping the local block of every conflict region - --theirs: rewrite each file by keeping the upstream block - Optional positional args limit the scope to specific env keys Conflict marker parser handles both diff3 form (\`<<<<<<< / ||||||| / ======= / >>>>>>>\`) which b's merge writes today, and the bare 2-way form so hand-edited files still resolve cleanly. Unterminated regions return an error rather than producing garbage output. Phase 4 of #125 also covers in-place YAML comment markers (e.g. \`# b: CONFLICT upstream=v1.31 base=v1.30 local=v1.32\`); that conversion is intentionally a separate PR — this command speaks the marker format that's actually written today. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Path-traversal check on every dest before reading or writing. A malicious or hand-edited lockfile must not let \`b env resolve\` touch files outside the project root. Mirrors the same check the env sync / status / remove paths use. - Update b.lock with the new SHA after a successful resolve. Without this, the next sync / \`b verify\` would treat every resolved file as locally modified, defeating the point of the command. - Label support: env keys are now matched in the canonical ref#label form so labeled envs can be targeted unambiguously when multiple lock entries share the same ref. The status output also prints the labeled key. Integration tests are intentionally deferred to a follow-up; the existing unit tests cover hasConflictMarkers and resolveConflictMarkers, and a real CLI integration test needs a fixture lock + project directory that's substantial enough to warrant its own change. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…138 round 2) - Remove the --list flag and its unused EnvResolveOptions.List field. Listing is already the default behavior when no --ours/--theirs flag is set; the flag was a no-op pretending to be a feature. - New TestEnvResolveRun_RewritesFileAndUpdatesLock exercises the full Run flow against a temp project and lockfile, asserting the file's conflict markers are stripped AND the lock entry SHA is updated to match the new on-disk hash. - New TestEnvResolveRun_PathTraversalRejected verifies a malicious lock entry pointing outside the project root is rejected before any file I/O happens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…2.5) - After rewriting a conflicted file, lock.SHA256File errors are now propagated as a hard error. Previously a hashing failure silently left the lock pointing at the stale SHA, leaving the repo in a drifted state with no indication why. - envKey doc no longer claims to do canonical normalization. The helper is just TrimSpace; the matching loop in Run builds the canonical ref / ref#label form on the lock-entry side. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…138 round 3) - hasConflictMarkers is now line-anchored (and CRLF-tolerant) so a marker substring inside a YAML string literal or a markdown rule can't trigger a false positive that makes Run() rewrite a clean file. - When resolveConflictMarkers returns n == 0 (false-positive detection on a pathological input), skip both the file write and the lock SHA update so we don't touch a file that doesn't actually need resolving. - The integration test now sets bVersion: "test" so the lock write doesn't stash an empty tool version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…138 round 4) - hasConflictMarkers now uses a small state machine instead of bufio.Scanner, so files with very long lines (SOPS blobs, minified JSON, lockfiles) can't silently fall back the way ErrTooLong + ignored Err() would. Per-line state is bounded at 64 bytes regardless of file size. - In --ours/--theirs mode, defer the totalConflicts increment until after resolveConflictMarkers actually finds at least one region. A false-positive marker scan no longer counts toward the summary, so the final "Resolved N region(s) across M file(s)" output reflects what was actually actionable. - Reword the filter-arg comment to match envKey's actual TrimSpace behavior (not "canonical normalization"). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Previously hasConflictMarkers required all three of <<<<<<<, =======, and >>>>>>> before reporting a conflict. That meant a file left in a half-merged state by a partial manual edit (start marker but no closing line) would slip past env resolve entirely. Treat the presence of a start marker as conflicted. List mode will surface the file; rewrite mode will hit resolveConflictMarkers' existing "unterminated conflict region" error path, which Run propagates as a hard error and the user fixes manually. Test cases extended with the unterminated region case and a stray-closing-marker negative case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…round 6) hasConflictMarkers required \"<<<<<<< \" / \">>>>>>> \" with a trailing space, but resolveConflictMarkers below uses strings.HasPrefix(line, \"<<<<<<<\") with no trailing-space requirement. A hand-edited file with bare \"<<<<<<<\" / \">>>>>>>\" lines (no label suffix) would be resolvable but never detected, so env resolve would silently skip it. Loosen the detector prefix to match. New test case covers the bare-marker conflict shape. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…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>

Copilot

Pull request overview

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

…rehash (#138 round 8) - resolveConflictMarkers now operates on []byte throughout instead of converting to string. Files containing non-UTF-8 bytes round- trip without corruption. - Separator detection requires the EXACT line "=======" (with optional trailing \\r for CRLF) instead of HasPrefix. A content line that happens to start with =======... is no longer misinterpreted as the separator. - Lock SHA is now computed from the in-memory \`resolved\` bytes rather than re-reading the file. Avoids the extra filesystem read AND closes a TOCTOU window where the file could be modified between WriteFile and the rehash. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

🤖 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).

fentas mentioned this pull request Apr 7, 2026

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

Merged

3 tasks

fentas requested a review from Copilot April 7, 2026 16:28

Copilot started reviewing on behalf of fentas April 7, 2026 16:29 View session