Skip to content

feat(copy-process): add copy-process MCP tool - #63

Open
gh-corezoid wants to merge 1 commit into
mainfrom
feat/copy-process-tool
Open

feat(copy-process): add copy-process MCP tool#63
gh-corezoid wants to merge 1 commit into
mainfrom
feat/copy-process-tool

Conversation

@gh-corezoid

Copy link
Copy Markdown
Contributor

Summary

  • Adds copy-process MCP tool — duplicates a process or state diagram without loading its JSON into the AI context
  • Equivalent to the "Duplicate" action in the Corezoid UI
  • Supports optional new_name (defaults to <title> (Copy)) and optional folder_path (defaults to the source's parent folder)
  • The copy is saved as a .conv.json file in the resolved local directory, ready for further editing

Context

tool: create-process | version: 2.3.5 | install: 104e5afc

Implementation

copy-process(source_process_id, new_name?, folder_path?) in the MCP server:

  1. Exports the source process JSON server-side via the existing ExportProcess API
  2. Creates an empty target process in the destination folder via CreateEmptyConv
  3. Deploys the source scheme to the new process ID via ProcessJSON
  4. Saves the result as <newID>_<title>.conv.json in the correct local directory

The AI context never sees the process JSON — context-cost is O(1) regardless of process size.

Checklist

  • make build passes
  • make vet passes
  • make test passes (TestToolRegistryMatchesREADME, TestToolRegistryNoDuplicates, etc.)
  • README.md tools table updated
  • README.md architecture diagram updated
  • No version bump / CHANGELOG.md change included
  • No credentials or private URLs in any file

Test plan

  • Call copy-process with a valid source_process_id — verify a new .conv.json appears in the correct folder with a new process ID
  • Call with new_name set — verify the copy uses that title
  • Call with folder_path pointing to a different subfolder — verify placement
  • Omit both optional args — verify default title <original> (Copy) and same-folder placement

Introduces copy-process(source_process_id, new_name?, folder_path?) — the
equivalent of the Corezoid UI "Duplicate" action. The tool exports the source
process server-side, creates an empty target in the destination folder, and
deploys the source scheme to the new process ID without ever loading the full
JSON into the AI context.

tool: create-process | install: 104e5afc | version: 2.3.5

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gh-corezoid

Copy link
Copy Markdown
Contributor Author

AI Review

Adds copy-process MCP tool that duplicates a Corezoid process or state diagram server-side (export → create empty → deploy scheme) without loading the process JSON into the AI context.

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 files version-synced ⏭️ skip (no manifest files changed)
C2 — Node IDs in .conv.json valid ⏭️ skip (no .conv.json files changed)
C3 — No hardcoded env-specific values ⏭️ skip (no .conv.json files changed)
C4 — extra fields have extra_type ⏭️ skip (no .conv.json files changed)
C5 — README.md updated for new skills ⏭️ skip (no new skill directory added)

U5 details: go build ./... — clean. go test ./... on PR branch: ok convctl 7.3s. Same on main: ok convctl 11.8s. No regressions introduced.

Issues found

warning — Orphaned empty process on deploy failure (executor_process.go, CopyProcess)

Steps 5 → 7 of CopyProcess create a new empty process via CreateEmptyConv, then attempt to deploy via ProcessJSON. If the deploy fails, the code removes the local .conv.json file but does not delete the empty process it just created in Corezoid. The user's workspace is left with a dangling empty process titled "<title> (Copy)" that has no way to identify itself as a failed copy attempt.

Fix: add a DeleteProcess(newID) call in the error path after ProcessJSON fails:

if _, err := v.ProcessJSON(finalPath, string(srcJSON)); err != nil {
    os.Remove(finalPath) //nolint:errcheck
    // Also clean up the orphaned empty process:
    v.DeleteProcess(newID)
    return 0, "", fmt.Errorf("deploy copy: %w", err)
}

warning — Source JSON id field not updated before deploy (executor_process.go, step 6)

The exported source JSON contains the original process's id field. The code updates srcMap["title"] but does not update srcMap["id"] to newID. Whether ProcessJSON targets the process by v.ProcessID (the executor field, correctly set to newID) or by the id field embedded in the JSON depends on the API call's internal routing. If the API honours the embedded id, the deploy could silently target the source process rather than the copy. Adding srcMap["id"] = newID alongside the title update would make the intent explicit and safe in either case.


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

@gh-corezoid

Copy link
Copy Markdown
Contributor Author

CI note: The Validate JSON manifests check failure is a pre-existing version mismatch in the main branch (plugins/corezoid/.claude-plugin/plugin.json is at 2.7.6 while the other three manifests are at 2.7.1). The same failure appears on PR #60 and other open PRs targeting main. My changes do not touch any manifest file. MCP server build & test and Docs link check both pass.

@salimovartem

Copy link
Copy Markdown
Collaborator

Serious findings only — merge conflicts, style, and formatting intentionally skipped.

  1. Correctness / Dataplugins/corezoid/mcp-server/executor_process.go:645-655 — The saved .conv.json inherits obj_id and parent_id from the source export; only title is rewritten. The file is named <newID>_... but internally obj_id = sourceProcessID and parent_id = source folder. On a subsequent push-process of this copy, fixStruct (main.go:477) does NOT overwrite obj_id when it is non-nil, and extractObjIDFromJSON (main.go:680) returns sourceProcessID — so the auto-snapshot in handlePushProcess (mcp_handlers_process.go:185-195) is created against the original process, not the copy. The copy gets no pre-push safety net, and the original accumulates stray snapshots. Fix: set srcMap["obj_id"] = float64(newID) and srcMap["parent_id"] = float64(folderID) before MarshalIndent.

  2. Data / Operationsplugins/corezoid/mcp-server/executor_process.go:687-691 — On ProcessJSON failure the local file is removed, but the empty process created by CreateEmptyConv on the platform is not cleaned up. Every failed copy leaves an orphan empty process in the destination folder. Delete newID on the error path (or reuse an existing rollback path).

  3. Correctness (minor)plugins/corezoid/mcp-server/executor_process.go:665v.ProcessID = newID is redundant: CreateEmptyConv already sets it (executor_process.go:557). Not a bug, but misleads readers into thinking the call does not mutate v when it does.

  4. Correctness (inherited)plugins/corezoid/mcp-server/executor_process.go:669safeName := strings.ReplaceAll(newTitle, " ", "_") does not sanitize /, \, .., or control chars. Same pattern exists in pull-process (mcp_handlers_process.go:61), so this is preexisting debt rather than a regression, but with a user-controlled new_name containing / the destination file can land in an unintended subdirectory. Worth centralizing path sanitization.

Unrelated to this PR: the failing Validate JSON manifests CI check does not touch any manifest in this diff — likely an infra issue. Re-run after addressing #1 and #2.

@salimovartem

Copy link
Copy Markdown
Collaborator

Serious findings only — merge conflicts, style, and formatting intentionally skipped.

  1. Data / Operationsplugins/corezoid/mcp-server/executor_process.go:687-692 — If ProcessJSON fails after CreateEmptyConv succeeds (node validation, git_call build failure, Commit rejection, transient network error), we delete the local file but leave an orphan empty process on the server. The caller doesn't even get newID back to clean it up. Add v.DeleteProcess(newID) (or a deferred rollback) in the deploy-error branch.

  2. Critical test gapsplugins/corezoid/mcp-server/executor_process.go:601-692, mcp_handlers_process.go:674-700 — ~124 lines of new production code with zero tests. Untested branches include empty vs. explicit newTitle, folderID==0 fallback to source's parent_id, conv_type = process vs. state, dir degradation to cwd on resolveFolderPathFromAPI failure, and each of the three failure paths (ExportProcess, CreateEmptyConv, ProcessJSON). Adjacent code has coverage (TestExportProcess_Error, TestCreateEmptyConv_*, TestResolveFolderIDFromDir); this tool mutates both server state and local disk, so untested branches are a real regression risk.

  3. Correctnessplugins/corezoid/mcp-server/executor_process.go:645-648CreateEmptyConv returns int (0 on any failure), so the synthesized error "failed to create target process 'X' in folder Y" discards the underlying cause (permissions? bad folder ID? network?). On main the signature has already moved to (int, error) — the rebase is a natural time to propagate the real error to the user.

Side note (not a blocker): the tool is described as equivalent to the UI "Duplicate", but the source JSON is deployed largely as-is, including commits, obj_id, and process-level params. If the source has its own alias (short_name), redeploying it into the copy will conflict with the existing stage-scoped alias. Worth verifying manually against a process that owns an alias, and stripping aliases before ProcessJSON if needed.

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.

3 participants