feat(mcp): pre-push concurrency detection + 3-way merge

[sVIKs]

Depends on #75 (feat/scan-stage-go). This branch is stacked on top of #75, so until #75 merges the diff below also contains its commit (f67ab50). Please review #75 first, then this PR on top. Once #75 is merged into main I will rebase this branch onto main to drop that commit and leave only the concurrency changes.

Problem

push-process was last-write-wins. On push, ProcessJSON fetches the current server scheme and DeleteNotUsedNodes drops any server node absent from the local copy — so if someone edited the process on the server between your pull and your push, their edits are silently overwritten. Corezoid's API has no optimistic lock, so this is caught client-side (read-compare-before-write); the existing auto-snapshot covers the small TOCTOU window.

What this adds

• Baseline + ancestor capture on pull. pull-process/pull-folder record the pulled server version (.corezoid-baseline.json) and a copy of the pulled scheme (.corezoid-baseline/, the 3-way merge ancestor). Both gitignored; post-push refreshes them so push -> edit -> push keeps 3-way.
• Conflict gate on push. If the server moved since pull, the push blocks with a report:
  • who last changed it (commit list, falling back to the latest snapshot's user_name) and when;
  • three buckets — your local edits, the server's changes, and the overlap (nodes you both changed);
  • a clear choice stating what each option keeps and drops.
• Three ways to resolve:
  • merge=true — real 3-way merge: grafts the non-overlapping server changes into your file (nodes matched by title, links rewired across the server's id regeneration), keeps true conflicts as yours and lists them; advances the baseline only when there is no conflict, so the follow-up push deploys cleanly.
  • re-pull — take the server version and re-apply your edits by hand.
  • force=true — deploy your version, dropping the server changes (auto-snapshotted first, recoverable).
• Files with no baseline (never pulled) push normally with an advisory.

Scope / non-goals (v1)

Detect + assisted merge, matched by node title (a rename reads as delete+add). No server-side lock (API can't). No new MCP tool — everything is folded into pull/push.

Testing

• Unit tests for every classification case (theirs-edits-JS/API/condition, add, delete, both-edit conflict, disjoint auto-merge, rename, duplicate-title, no-op) plus report buckets, author fallback, wording. go test -race ./... green.
• Live E2E on the sandbox: pull -> simulate a concurrent edit -> block with the 3-way report -> merge=true grafts + rewires links + advances baseline + clean re-push; a both-edit-same-node conflict is kept as mine + force resolves; who/when line verified live. Node bodies (code, conditions, options, links) confirmed to round-trip through the export API and merge fully.

🤖 Generated with Claude Code

[gh-corezoid]

AI Review

Adds pre-push concurrency detection with a 3-way node-level merge to push-process/pull-process/pull-folder, plus broken-link checking in lint-process and removal of the corezoid-stage-scan skill (its per-process check is absorbed into lint). Note: this PR is stacked on #75 — the first commit (f67ab503 feat(lint)) belongs to that branch; the concurrency+merge work starts from e3132818.

Checklist

Check	Result
U1 — Conventional commit format	✅ pass
U2 — No leaked credentials	✅ pass
U3 — No merge commits	✅ pass
U4 — PR targets correct base branch (main)	✅ pass
U5 — Build & tests (Go)	✅ pass
U6 — Architectural & design consequences	⚠️ warning (see below)
C1 — Manifest version sync	⏭️ skip (no manifest files changed)
C2 — Node IDs in .conv.json files	⏭️ skip (no .conv.json changed)
C3 — No hardcoded env-specific values	⏭️ skip (no .conv.json changed)
C4 — extra fields with extra_type	⏭️ skip (no .conv.json changed)
C5 — README updated for new/removed skills	✅ pass

Build & test detail (U5)

• go build ./... — clean, exit 0.
• go test ./... on PR branch (no -race, 180 s timeout) — all pass, 50.1 s.
• go test ./... on base main — fails TestSkillPathsExist (missing docs/process/corezoid-api-integration.md); pre-existing failure on main, not caused by this PR.
• No regressions introduced.

Issues found

[warning — U6] Empty-titled nodes become blanket conflicts in the 3-way merge

canonicalizeNodes keys every node by its title string. Nodes with an empty title (title: "") all map to the same key "". The second (and any further) empty-titled node is flagged Ambiguous: true. Any node where Ambiguous is set on any version (base / theirs / mine) lands in clsConflict, even if only one side actually changed it.

In practice, Corezoid processes often leave auto-generated nodes untitled (the Start event node and some error-cluster nodes frequently have title: ""). A process with two or more untitled nodes will see every one of them classified as a conflict on every concurrent push, regardless of whether either party touched them. The merge then always requires manual force=true resolution for those nodes.

This is conservative (never silently merges the wrong thing) and the code comment calls it out explicitly ("duplicate title — cannot merge safely"), so it is intentional. But it means merge=true gives a weaker guarantee on real-world processes than the test suite suggests — a user with even one pair of untitled nodes will always be in the conflicts bucket.

Suggested fix (advisory): treat title: "" nodes as unresolvable by title and classify them immediately as clsConflict without the general ambiguity path, making the limitation explicit rather than implied by the shared-key collision. Or add a secondary match key (e.g. obj_type + stable order) so that only a genuine title-duplicate blocks a clean merge.

---

This review was generated automatically. A human maintainer should still make the merge decision.

[gh-corezoid]

AI Review

Re-review after synchronize: adds pre-push concurrency detection with 3-way node-level merge to push-process/pull-process/pull-folder, plus broken-link checking in lint-process and removal of the corezoid-stage-scan skill; one new commit (61e761a5) landed since the initial review directly addressing the previously flagged U6 warning.

Checklist

Check	Result
U1 — Conventional commit format	✅ pass
U2 — No leaked credentials	✅ pass
U3 — No merge commits	✅ pass
U4 — PR targets correct base branch (main)	✅ pass
U5 — Build & tests (Go)	✅ pass
U6 — Architectural & design consequences	⚠️ warning (residual — see below)
C1 — Manifest version sync	⏭️ skip (no manifest files changed)
C2 — Node IDs in .conv.json files	⏭️ skip (no .conv.json changed)
C3 — No hardcoded env-specific values	⏭️ skip (no .conv.json changed)
C4 — extra fields with extra_type	⏭️ skip (no .conv.json changed)
C5 — README updated for new/removed skills	✅ pass

Build & test detail (U5)

• go build ./... — clean, exit 0.
• go test ./... on PR branch (no -race, 180 s timeout) — all pass, 48.3 s.
• go test ./... on base main — all pass, 52.1 s (same pre-existing TestSkillPathsExist failure noted in the prior review; not caused by this PR).
• go test -race ./... with the default 3-minute timeout timed out on both branches: TestLayoutI8_NoWastedAir/mega alone takes ~36 s under the race detector, so the full race-enabled suite exceeds 3 min regardless of this PR. Verified the mega sub-test passes cleanly on both branches when given adequate time — pre-existing environment constraint, not a regression.
• No regressions introduced.

Fixed since last review ✅

[U6] Empty-titled nodes becoming blanket conflicts in the 3-way merge — RESOLVED

matchKeys() (new in 61e761a5) now keys untitled nodes by obj_type + ordinal-among-untitled-of-that-type (e.g. u:2:0, u:2:1) instead of collapsing them all onto the empty-string key. Two new tests confirm: TestMerge_UntitledNodeEditIsMergeable verifies a one-sided edit on an untitled node becomes a mergeable graft (not a conflict), and TestMerge_DuplicateTitleIsConflict verifies only a genuine duplicate non-empty title still yields a conflict. Evidence: commit message explicitly references the automated review finding and the full suite is green.

Issues found

[warning — U6, residual] Ordinal-based matching shifts when an untitled node is inserted mid-sequence

matchKeys() assigns ordinals positionally — the first untitled obj_type:2 node gets key u:2:0, the second gets u:2:1, and so on. This resolves the original collision (all empty-string keys → everyone a conflict) but introduces a narrower edge case: if a colleague inserts or deletes an untitled node of the same type before one of yours, every subsequent untitled node of that type gets a shifted ordinal key. The merge engine then sees those nodes as having changed on both sides even if neither party touched them, landing them in the conflicts bucket.

Scenario: process has two untitled end nodes (u:2:0 = success branch, u:2:1 = error branch). Colleague pushes an additional untitled end node at position 0. On re-pull their version shows u:2:0 (new), u:2:1 (was your success), u:2:2 (was your error). If you push concurrently, your u:2:0/u:2:1 match against their first two — nodes you haven't touched appear conflicted.

This is substantially rarer than the original blanket-collision bug (requires an untitled node insertion before an existing one, not merely the presence of multiple untitled nodes), and the conservative behaviour is safe (never silently merges the wrong content). Advisory: consider noting this limitation in the conflict-block user message so operators know why untitled nodes appear in the conflict bucket on a seemingly clean concurrent edit.

---

This review was generated automatically. A human maintainer should still make the merge decision.

[gh-corezoid]

AI Review

Re-review after synchronize: adds pre-push concurrency detection with 3-way node-level merge to push-process/pull-process/pull-folder, plus broken-link checking in lint-process and removal of the corezoid-stage-scan skill. One new commit (9b95700) landed since the last review, directly addressing the previously flagged residual U6 advisory.

Checklist

Check	Result
U1 — Conventional commit format	✅ pass
U2 — No leaked credentials	✅ pass
U3 — No merge commits	✅ pass
U4 — PR targets correct base branch (main)	✅ pass
U5 — Build & tests (Go)	✅ pass
U6 — Architectural & design consequences	✅ pass
C1 — Manifest version sync	⏭️ skip (no manifest files changed)
C2 — Node IDs in .conv.json files	⏭️ skip (no .conv.json changed)
C3 — No hardcoded env-specific values	⏭️ skip (no .conv.json changed)
C4 — extra fields with extra_type	⏭️ skip (no .conv.json changed)
C5 — README updated for new/removed skills	✅ pass

Build & test detail (U5)

• go build ./... — clean, exit 0.
• go test ./... on PR branch (180 s timeout) — all pass (ok convctl 50.7s). New test TestMerge_UntitledConflictShowsPositionNote passes.
• go test ./... on base main — all pass (ok convctl 55.3s).
• No regressions introduced.

Fixed since last review ✅

[U6, residual] Ordinal-based matching shifts for untitled node insertion — RESOLVED

The previous advisory noted that matchKeys() assigns ordinals positionally, so inserting an untitled node of the same type before an existing one could shift subsequent ordinals and surface false conflicts. The fix in 9b95700 adds a contextual note directly in formatMergePlan: when any untitled node appears in the conflict bucket, the report now states:

note: untitled nodes are matched by position; inserting/removing an untitled node on one side can shift that match and surface a false overlap — give nodes titles to avoid this.

This is the exact operator-facing message the advisory requested. The new test TestMerge_UntitledConflictShowsPositionNote verifies the note appears whenever an untitled node is in the conflict list. The wording change to force-recoverability ("a snapshot is attempted first; recoverable only if it succeeds") is also accurate and appropriately hedged.

Issues found

No issues found.

---

This review was generated automatically. A human maintainer should still make the merge decision.

[gh-corezoid]

AI Review

Re-review after synchronize: adds pre-push concurrency detection with 3-way node-level merge to push-process/pull-process/pull-folder, plus broken-link checking in lint-process and removal of the corezoid-stage-scan skill. This synchronize event reflects the force-push of commit 6d80d89c (honest force-recoverability wording + untitled overlap note), which directly addressed the residual U6 advisory from the previous review. Note: #75 has since merged into main; the PR branch has been rebased cleanly — none of the 4 commits belong to #75.

Checklist

Check	Result
U1 — Conventional commit format	✅ pass
U2 — No leaked credentials	✅ pass
U3 — No merge commits	✅ pass
U4 — PR targets correct base branch (main)	✅ pass
U5 — Build & tests (Go)	✅ pass
U6 — Architectural & design consequences	✅ pass
C1 — Manifest version sync	⏭️ skip (no manifest files changed)
C2 — Node IDs in .conv.json files	⏭️ skip (no .conv.json changed)
C3 — No hardcoded env-specific values	⏭️ skip (no .conv.json changed)
C4 — extra fields with extra_type	⏭️ skip (no .conv.json changed)
C5 — README updated for new/removed skills	✅ pass

Build & test detail (U5)

• go build ./... — clean, exit 0.
• go test ./... on PR branch (6d80d89) — all pass (ok convctl 51.2s).
• go test ./... on base main (c18717c, includes merged feat(lint): broken-link check + lint before push; drop scan-stage tool #75) — all pass (ok convctl 54.6s).
• No regressions introduced.

Fixed since last review ✅

[U6, residual] Ordinal-based matching shifts for untitled node insertion — RESOLVED

Commit 6d80d89c delivers the two changes the prior advisory requested:

1. Honest force-recoverability wording. All three force=true paths in conflict.go now say "a snapshot of the server version is attempted first — recoverable only if it succeeds" rather than the previous unconditional "auto-snapshotted first, so recoverable". The distinction matters: if the snapshot API call fails (e.g. transient server error), the overwrite is permanent.

2. Untitled-node position note in the conflict report. formatMergePlan now detects when any conflict node has Title == "" and appends the advisory: "untitled nodes are matched by position; inserting/removing an untitled node on one side can shift that match and surface a false overlap — give nodes titles to avoid this." Test TestMerge_UntitledConflictShowsPositionNote verifies the note fires exactly when expected; full suite green.

Issues found

No issues found.

---

This review was generated automatically. A human maintainer should still make the merge decision.

[gh-corezoid]

AI Review

Re-review after synchronize: adds pre-push concurrency detection with 3-way node-level merge to push-process/pull-process/pull-folder, plus broken-link checking in lint-process and a handleDeleteProcess stale-file hint. This synchronize is a clean rebase onto 1a01f0c9 (chore(release): v2.9.0, 2026-07-17 12:47 UTC) — the 4 PR commits carry the same code as the previous review; all previous findings remain resolved.

Checklist

Check	Result
U1 — Conventional commit format	✅ pass
U2 — No leaked credentials	✅ pass
U3 — No merge commits	✅ pass
U4 — PR targets correct base branch (main)	✅ pass
U5 — Build & tests (Go)	✅ pass
U6 — Architectural & design consequences	✅ pass
C1 — Manifest version sync	⏭️ skip (no manifest files changed)
C2 — Node IDs in .conv.json files	⏭️ skip (no .conv.json changed)
C3 — No hardcoded env-specific values	⏭️ skip (no .conv.json changed)
C4 — extra fields with extra_type	⏭️ skip (no .conv.json changed)
C5 — README updated for new/removed skills	✅ pass

Build & test detail (U5)

• go build ./... on PR branch (1758e320) — clean, exit 0.
• go test ./... on PR branch (180 s timeout) — all pass (ok convctl 89.9s). All 16 TestMerge_* tests and all baseline/conflict tests pass.
• go test ./... on base main (1a01f0c9, chore(release): v2.9.0) — all pass (ok convctl 122.8s).
• No regressions introduced.

Fixed since last review ✅

No new issues were raised in the previous review (review #4, 2026-07-17T10:22:52Z reported "No issues found"). This synchronize is a rebase-only update — content is unchanged. Carrying all prior resolutions forward:

• [U6] Empty-titled nodes causing blanket conflicts — RESOLVED via matchKeys() (commit 5e7e88ad): untitled nodes are keyed by obj_type + ordinal instead of collapsing onto the empty-string key.
• [U6, residual] Ordinal shift on untitled-node insertion — RESOLVED via formatMergePlan contextual note (commit 1758e320): when any untitled node appears in the conflict bucket the report states that untitled nodes are matched by position and an insertion/removal can surface a false overlap. Verified by TestMerge_UntitledConflictShowsPositionNote.

Issues found

No issues found.

---

This review was generated automatically. A human maintainer should still make the merge decision.