Skip to content

feat(cli): explicit projects merge --from/--to with preview - #1453

Closed
danielgap wants to merge 5 commits into
Gentleman-Programming:mainfrom
danielgap:feat/1296-projects-merge
Closed

danielgap wants to merge 5 commits into
Gentleman-Programming:mainfrom
danielgap:feat/1296-projects-merge

Conversation

@danielgap

@danielgap danielgap commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

🔗 Linked Issue

Closes #1296


🏷️ PR Type

  • type:bug — Bug fix
  • type:feature — New feature
  • type:question — Question requiring tracked work
  • type:docs — Documentation only
  • type:refactor — Code refactoring (no behavior change)
  • type:chore — Maintenance, dependencies, tooling
  • type:breaking-change — Breaking change

📝 Summary

  • Adds engram projects merge --from <name> --to <name> [--apply]: the operator names the pair the consolidate detector cannot group (e.g. acmeapi and acme-api).
  • Without --apply it prints the observation/session/prompt counts that would move and opens no write path; --apply migrates them through MergeProjectsByName and prints consolidate's post-merge report with honest nothing-merged reporting.
  • A pair that normalizes to the same project is refused outright (in preview and apply) with a pointer to consolidate. DOCS.md documents the command; the --candidates companion ask from feat(cli): merge two explicitly named projects when "projects consolidate --all" finds no groups #1296 stays out of scope.

📂 Changes

File Change
cmd/engram/main.go projects merge routing, usage line, cmdProjectsMerge (flag parsing, unconditional same-normalized refusal, dry-run preview, apply report)
cmd/engram/main_test.go seed fixture + 5 CLI tests: required flags, unknown flag, preview-no-mutation (before/after counts), apply moves records, same-normalized refusal exits non-zero
DOCS.md command list entry + retroactive-cleanup section

🧪 Test Plan

  • go test ./cmd/engram/... ./internal/store/... -count=1 -skip 'TestCmdServeSignalClosesUnixSocket' passes (skipped test is a pre-existing host-environment failure, reproduced on base without this diff)
  • Live smoke on a throwaway store: preview leaves the DB unchanged; --apply moves all records; both same-normalized spellings refused non-zero; empty source reports honestly
  • gofmt clean; go build ./... OK

✅ Contributor Checklist

Chain Context

Field Value
Chain engram#1296 explicit projects merge
Tracker PR Not needed (2-slice stacked chain)
Position 2 of 2
Base main (see note below; logically builds on #1452)
Depends on #1452 (store seam; branch feat/1296-store-by-name-merge)
Follow-up None — closes #1296
Review budget 270 / 400 once retargeted (270 additions)
Starts at commit c9de012 (tip of #1452)
Ends with The full #1296 user-facing delivery: CLI merge command + docs

Chain Overview

main
 └── #1452 Store seam (355 lines)
      └── 📍 #<this PR> CLI + docs (270 lines, closes #1296)

Scope

Autonomy

  • CI is expected to pass for this PR branch
  • This PR has one deliverable scope
  • This PR can be rolled back without unrelated changes
  • Tests, docs, and manual verification cover this unit

Draft until #1452 merges. The head branch contains both chain commits, and stacked bases are not targetable across forks, so this diff currently shows slice 1 too. Once #1452 merges, GitHub retargets this PR and the diff collapses to the CLI slice only (270 lines). Reviewing before that point would double-count the store seam.

MergeProjectsByName migrates one explicitly named source project spelling
onto an explicitly named target in a single transaction, reusing the Gentleman-Programming#1415
migration core via mergeProjectRecordsTx. The pair does not need to
normalize-equivalent — that is the point of an explicit rename-merge such
as acmeapi -> acme-api — and both existing admission contracts
(MergeProjects, MergeExplicitProjectVariants) are pinned unchanged by
regression tests. CountProjectRecords reports the exact-spelling counts
the dry-run preview prints, mirroring the merge UPDATE predicate so the
preview can neither overstate nor understate what --apply moves.

Refs Gentleman-Programming#1296
engram projects merge --from <name> --to <name> names the pair the
consolidate detector cannot group (e.g. acmeapi and acme-api). Without
--apply it prints the observation/session/prompt counts that would move
and opens no write path; --apply migrates them through
MergeProjectsByName and prints consolidate's post-merge report with
honest nothing-merged reporting. A pair that normalizes to the same
project is refused outright with a pointer to consolidate. DOCS.md
documents the command.

Checks: gofmt clean; go build ./... OK; go test ./internal/store/...
./cmd/engram/... -count=1 -skip 'TestCmdServeSignalClosesUnixSocket'
(pre-existing host-environment failure) ok. Live smoke on a throwaway
store verified preview-no-mutation, apply, both refusal spellings, and
honest empty-source reporting.

Refs Gentleman-Programming#1296
@coderabbitai

coderabbitai Bot commented Sep 26, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

MergeProjectsByName now computes sync-identity presence for the exact
source spelling (sync_enrolled_projects rows plus pending sync_mutations,
mirroring mergeProjects' explicit check) before the record migration and
enqueues backfillProjectSyncMutationsTx only when the merge moved record
rows or sync identity. A missing source stays a successful no-op that
writes zero sync mutations. Adds the guard regression test (RED against
the unconditional backfill), a CountProjectRecords error-path test, and
ACMEAPI exact-spelling pins in the merge and count tests.

Addresses CodeRabbit findings on Gentleman-Programming#1452. Refs Gentleman-Programming#1451
golangci-lint runs with only-new-issues, so the new cmdProjectsMerge and
its tests must honor the s.Close error like the store tests do. Rebased
onto the seam branch including its backfill guard fix.

Addresses the Lint failures on Gentleman-Programming#1453. Refs Gentleman-Programming#1296
@dnlrsls dnlrsls added the type:feature New feature label Sep 26, 2026
@danielgap

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #1457, which landed the explicit project-merge variants and closed #1296. My implementation here covers the same surface (same files), so keeping it open would only invite drift.

#1452 stays draft until #1451 is triaged, and I will re-scope or close it against the merged #1457 baseline as needed. Thanks for landing the feature line.

@danielgap danielgap closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): merge two explicitly named projects when "projects consolidate --all" finds no groups

2 participants