From 5b919ff26df352eb734c846a1e40380a9871ef3b Mon Sep 17 00:00:00 2001 From: Claude Agent Date: Thu, 16 Jul 2026 11:44:28 +0000 Subject: [PATCH] fix(smart-forms): mime type, conflict detection, dup guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - defaultMimeType: add pages//style → text/css rule; the file is named "style" (no extension) but is always Less/CSS; old code returned application/json here (unfix-able by PUT) - push diff phase detects MIME-only drift and includes those files in modifiedFilePaths so a PUT self-heals the wrong Content-Type without a delete/recreate cycle - Phase 3 PUT re-derives mimeType from path instead of the stale manifest value; updates manifest.MimeType accordingly - Before creating new files, fetch the live server tree and block the push if a file already occupies that (folder,title) slot — mirrors the UI one-style-file-per-page constraint and surfaces the orphan in the error with guidance to re-pull - pullSmartForm conflict detection: if any local file's hash differs from last-pulled manifest hash, pull aborts listing conflicts; pass force=true to discard local edits and proceed - Add force bool param to pullSmartForm tool registration - Tests: TestDefaultMimeType + TestDetectPullConflicts Co-Authored-By: Claude Sonnet 4.6 --- .../internal/engines/smartform/pull.go | 66 +++++++- .../internal/engines/smartform/push.go | 122 ++++++++++++++- .../internal/engines/smartform/push_test.go | 147 ++++++++++++++++++ .../internal/engines/smartform/register.go | 5 +- 4 files changed, 331 insertions(+), 9 deletions(-) create mode 100644 plugins/simulator/mcp-server/internal/engines/smartform/push_test.go diff --git a/plugins/simulator/mcp-server/internal/engines/smartform/pull.go b/plugins/simulator/mcp-server/internal/engines/smartform/pull.go index b745b8c..48b0def 100644 --- a/plugins/simulator/mcp-server/internal/engines/smartform/pull.go +++ b/plugins/simulator/mcp-server/internal/engines/smartform/pull.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "github.com/corezoid/simulator-ai-plugin/plugins/simulator/mcp-server/internal/engines/ecore" "github.com/mark3labs/mcp-go/mcp" @@ -144,6 +145,12 @@ func writeEnvTree(node appTreeNode, dir, relDir string, files map[string]manifes // (falling back to cwd when the env var is unset — see ecore.WorkDir). A // .manifest.json is written in each env folder to track file IDs and content // hashes (used by pushSmartForm for diffing). +// +// Conflict detection: if a prior manifest exists and any local file has been +// modified since the last pull (local hash ≠ manifest hash), the pull refuses +// to overwrite those local edits unless force=true is passed. This prevents +// silent loss of work when a pushSmartForm succeeded server-side but the caller +// treated it as failed and edited locally a second time. func handlePullSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { if authResult := ecore.EnsureAuth(ctx); authResult != nil { return authResult, nil @@ -157,6 +164,7 @@ func handlePullSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal if r := ecore.RequireUUID("actorId", actorID); r != nil { return r, nil } + force, _ := args["force"].(bool) envs, err := fetchAppEnvs(ctx, actorID) if err != nil { @@ -175,11 +183,32 @@ func handlePullSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal baseDir := ecore.ResolvePath(actorID) for _, env := range envs { + envDir := filepath.Join(baseDir, env.Title) + + // Conflict detection: compare local files against the stored manifest + // hashes. A file is "locally modified" when it exists on disk with a + // hash that differs from what was recorded at the last pull — that means + // the caller edited it (possibly after a push that succeeded server-side + // but appeared to fail locally). Overwriting such a file silently would + // discard unsynced work. + if !force { + if conflicts := detectPullConflicts(envDir); len(conflicts) > 0 { + out, _ := json.Marshal(map[string]interface{}{ + "actorId": actorID, + "env": env.Title, + "conflicts": conflicts, + "message": "pull aborted: local files have unsaved edits that differ from the last-pulled " + + "state. Push them first (pushSmartForm) or re-run pullSmartForm with force=true to " + + "discard local changes.", + }) + return mcp.NewToolResultError(string(out)), nil + } + } + tree, err := fetchEnvStruct(ctx, actorID, env.ID) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("[Error] fetch struct for env %q (id=%d): %v", env.Title, env.ID, err)), nil } - envDir := filepath.Join(baseDir, env.Title) files := make(map[string]manifestNode) folders := make(map[string]int) var rootFolderID int @@ -211,3 +240,38 @@ func handlePullSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal }) return mcp.NewToolResultText(string(out)), nil } + +// detectPullConflicts loads the existing .manifest.json from envDir (if any) +// and returns the env-relative paths of every locally-modified file — i.e. +// files whose on-disk content hash differs from the hash recorded at the last +// pull. Returns nil when no manifest exists yet (first-ever pull is always safe) +// or when no local modifications are detected. +func detectPullConflicts(envDir string) []string { + manifestPath := filepath.Join(envDir, manifestFileName) + raw, err := os.ReadFile(manifestPath) + if err != nil { + // No manifest → first pull, no conflicts possible. + return nil + } + var manifest smartFormManifest + if err := json.Unmarshal(raw, &manifest); err != nil { + // Corrupt manifest: can't tell; treat as no conflicts (pull will fix it). + return nil + } + + var conflicts []string + for relPath, node := range manifest.Files { + localPath := filepath.Join(envDir, filepath.FromSlash(relPath)) + data, err := os.ReadFile(localPath) + if err != nil { + // File not present locally: not a conflict (it's an orphan the + // pull will restore; that's expected behaviour). + continue + } + if hashSource(string(data)) != node.Hash { + conflicts = append(conflicts, relPath) + } + } + sort.Strings(conflicts) + return conflicts +} diff --git a/plugins/simulator/mcp-server/internal/engines/smartform/push.go b/plugins/simulator/mcp-server/internal/engines/smartform/push.go index eb85401..ba85326 100644 --- a/plugins/simulator/mcp-server/internal/engines/smartform/push.go +++ b/plugins/simulator/mcp-server/internal/engines/smartform/push.go @@ -121,7 +121,10 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal return newFolderPaths[i] < newFolderPaths[j] }) - // Diff files: new (not in manifest) vs modified (in manifest but hash differs). + // Diff files: new (not in manifest) vs modified (in manifest but hash or + // mimeType differs). MIME drift — where content is unchanged but the stored + // type is wrong (e.g. application/json on a pages//style file) — is + // treated as "modified" so the PUT corrects the server's Content-Type. var newFilePaths []string var modifiedFilePaths []string newHashes := make(map[string]string, len(localFiles)) @@ -133,7 +136,9 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal switch { case !ok: newFilePaths = append(newFilePaths, relPath) - case node.Hash != h: + case node.Hash != h || node.MimeType != defaultMimeType(relPath): + // Include MIME-only drift so a wrong Content-Type is healed via PUT + // even when the file's content bytes haven't changed. modifiedFilePaths = append(modifiedFilePaths, relPath) default: unchanged++ @@ -207,14 +212,44 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal } // Phase 2 — create missing files in one batch. + // Before POSTing, check the live server tree for files that already exist + // at the same (folder, title) slot — i.e. files that are "new" from the + // manifest's perspective but are already present on the server (e.g. a + // style file manually deleted+recreated via the UI, or a duplicate orphan + // produced by a previous push with a since-deleted manifest entry). + // The Simulator UI enforces a hard one-style-file-per-page limit; the + // create API path does not, so we mirror that check here to prevent silent + // accumulation of duplicate files on the server. createdFiles := 0 if len(newFilePaths) > 0 { + // Fetch the current server tree to detect pre-existing file slots. + serverFiles, serverFetchErr := fetchServerFilesBySlot(ctx, actorID, manifest.EnvID) + if serverFetchErr != nil { + // Non-fatal: log the warning but proceed — a missing tree fetch + // should not block a push. Duplicate detection is best-effort. + serverFiles = nil + } + + var duplicateFiles []string batch := make([]createItem, 0, len(newFilePaths)) for _, relPath := range newFilePaths { parentID, ok := resolveParentID(relPath, manifest.Folders, manifest.EnvRootFolderID) if !ok { return mcp.NewToolResultError(fmt.Sprintf("[Error] cannot resolve parent folder id for %q — run pullSmartForm to refresh manifest", relPath)), nil } + slot := fmt.Sprintf("%d/%s", parentID, filepath.Base(relPath)) + if serverFiles != nil { + if existing, dup := serverFiles[slot]; dup { + // A file already occupies this (folder, title) slot on the + // server. Creating a second one would produce a silent + // orphan, bypassing the UI's uniqueness constraint. + duplicateFiles = append(duplicateFiles, fmt.Sprintf( + "%s (server already has fileId=%d, mimeType=%s — run pullSmartForm to re-sync manifest)", + relPath, existing.ID, existing.Type, + )) + continue + } + } batch = append(batch, createItem{ ObjType: "file", FolderID: parentID, @@ -223,6 +258,15 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal Source: localFiles[relPath], }) } + if len(duplicateFiles) > 0 { + out, _ := json.Marshal(map[string]interface{}{ + "actorId": actorID, + "env": "develop", + "duplicateFiles": duplicateFiles, + "message": "push aborted: server already contains files at the listed paths; run pullSmartForm to re-sync the manifest before pushing", + }) + return mcp.NewToolResultError(string(out)), nil + } body, _ := json.Marshal(batch) respBytes, err := ecore.PapiPOST(ctx, apiURL, body) if err != nil { @@ -263,6 +307,9 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal } // Phase 3 — update modified existing files (one batch PUT). + // Always re-derive the MIME type from the path rather than trusting the + // manifest's stored value, so a previously wrong Content-Type is corrected + // on the server and in the manifest in the same PUT that updates the source. updated := 0 if len(modifiedFilePaths) > 0 { batch := make([]updateItem, 0, len(modifiedFilePaths)) @@ -273,7 +320,7 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal ObjType: "file", FolderID: node.FolderID, Title: filepath.Base(relPath), - Type: node.MimeType, + Type: defaultMimeType(relPath), // re-derive; heals stale wrong type Source: localFiles[relPath], }) } @@ -283,6 +330,7 @@ func handlePushSmartForm(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal } for _, relPath := range modifiedFilePaths { node := manifest.Files[relPath] + node.MimeType = defaultMimeType(relPath) // keep manifest in sync node.Hash = newHashes[relPath] manifest.Files[relPath] = node updated++ @@ -404,13 +452,34 @@ func parseCreatedObjs(respBytes []byte) ([]createdObj, error) { return resp.Data, nil } -// defaultMimeType returns the MIME type to assign to a newly-created file -// based on its env-relative path. The default skeleton uses application/json -// everywhere except the styles tree, which is Less/CSS source. +// defaultMimeType returns the MIME type to assign to a newly-created (or +// repaired) file based on its env-relative path. +// +// CSS detection rules (evaluated in order): +// 1. Top-level styles/ directory — Less/CSS source files. +// 2. pages//style — per-page stylesheet. The platform always names this +// file exactly "style" (no extension); the UI enforces this convention and +// the backend stores it as text/css. Key off the base name within the +// pages/ tree so any page depth is covered, regardless of extension. +// 3. Explicit .css extension — explicit fallback for any other CSS file. +// +// Everything else defaults to application/json (config, viewModel, locale …). func defaultMimeType(relPath string) string { if relPath == "styles" || strings.HasPrefix(relPath, "styles/") { return "text/css" } + // pages//style — the file is named "style" (no extension) but + // contains Less/CSS. The old code missed this case and assigned + // application/json, which cannot be corrected by a content-only PUT. + if strings.HasPrefix(relPath, "pages/") { + base := relPath[strings.LastIndex(relPath, "/")+1:] + if base == "style" { + return "text/css" + } + } + if strings.HasSuffix(relPath, ".css") { + return "text/css" + } return "application/json" } @@ -459,3 +528,44 @@ func collectFoldersFromTree(node appTreeNode, relDir string, folders map[string] collectFoldersFromTree(child, childRel, folders, rootFolderID) } } + +// serverFileSlot carries the server-side metadata for one file needed by +// duplicate detection: its numeric id and current MIME type. +type serverFileSlot struct { + ID int + Type string +} + +// fetchServerFilesBySlot fetches the live env tree and returns a map of +// "parentFolderID/title" → serverFileSlot for every file currently on the +// server. This lets push detect files that already occupy a slot that a new +// local file would be created into — the symptom of a manifest/server drift +// (e.g. the server file was deleted+recreated via the UI with a different id). +// +// The returned map is nil on error; callers treat a nil map as "skip check". +func fetchServerFilesBySlot(ctx context.Context, actorID string, envID int) (map[string]serverFileSlot, error) { + if envID == 0 { + return nil, fmt.Errorf("envId unknown — cannot fetch server tree") + } + tree, err := fetchEnvStruct(ctx, actorID, envID) + if err != nil { + return nil, err + } + slots := make(map[string]serverFileSlot) + collectFileSlots(*tree, slots) + return slots, nil +} + +// collectFileSlots walks a tree node recursively and populates slots with +// every file's "folderId/title" key so callers can detect slot collisions. +func collectFileSlots(node appTreeNode, slots map[string]serverFileSlot) { + for _, child := range node.Children { + switch child.ObjType { + case "file": + key := fmt.Sprintf("%d/%s", child.FolderID, child.Title) + slots[key] = serverFileSlot{ID: child.ID, Type: child.Type} + case "folder": + collectFileSlots(child, slots) + } + } +} diff --git a/plugins/simulator/mcp-server/internal/engines/smartform/push_test.go b/plugins/simulator/mcp-server/internal/engines/smartform/push_test.go new file mode 100644 index 0000000..eccc8a1 --- /dev/null +++ b/plugins/simulator/mcp-server/internal/engines/smartform/push_test.go @@ -0,0 +1,147 @@ +package smartform + +import ( + "encoding/json" + "os" + "path/filepath" + "sort" + "testing" +) + +// TestDefaultMimeType locks in the MIME-type derivation for every path +// pattern the Smart Form file tree can produce. The key regression case is +// pages//style, which was incorrectly typed as application/json before +// this fix and could not be corrected by a content-only PUT. +func TestDefaultMimeType(t *testing.T) { + cases := []struct { + path string + want string + }{ + // ── styles/ tree ───────────────────────────────────────────────── + {"styles", "text/css"}, + {"styles/main.less", "text/css"}, + {"styles/components/button.less", "text/css"}, + + // ── pages//style regression ──────────────────────────────── + // The file is named exactly "style" (no extension) but is Less/CSS. + // Old code returned application/json here. + {"pages/credit-application/style", "text/css"}, + {"pages/index/style", "text/css"}, + {"pages/my-page/style", "text/css"}, + + // ── explicit .css extension ─────────────────────────────────────── + {"components/button.css", "text/css"}, + {"pages/landing/custom.css", "text/css"}, + + // ── application/json (everything else) ─────────────────────────── + {"pages/credit-application/config", "application/json"}, + {"pages/credit-application/locale", "application/json"}, + {"pages/credit-application/viewModel", "application/json"}, + {"viewModel", "application/json"}, + {"locale", "application/json"}, + {"definitions/types", "application/json"}, + {"widgets/counter/config", "application/json"}, + // "style" at root (no pages/ prefix) is not a CSS file + {"style", "application/json"}, + // "style" nested elsewhere is not CSS + {"definitions/style", "application/json"}, + } + + for _, tc := range cases { + got := defaultMimeType(tc.path) + if got != tc.want { + t.Errorf("defaultMimeType(%q) = %q, want %q", tc.path, got, tc.want) + } + } +} + +// TestDetectPullConflicts verifies that detectPullConflicts correctly +// identifies local files whose content has been modified since the last pull +// (hash differs from manifest) and ignores files that are unchanged or absent. +func TestDetectPullConflicts(t *testing.T) { + dir := t.TempDir() + + // Helper: write file + update hash table + writeFile := func(relPath, content string) { + full := filepath.Join(dir, filepath.FromSlash(relPath)) + if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(full, []byte(content), 0600); err != nil { + t.Fatalf("write %s: %v", relPath, err) + } + } + + // Helper: write a manifest with given (path → hash) entries. + writeManifest := func(hashes map[string]string) { + files := make(map[string]manifestNode, len(hashes)) + for p, h := range hashes { + files[p] = manifestNode{Hash: h} + } + m := smartFormManifest{Files: files} + b, _ := json.MarshalIndent(m, "", " ") + if err := os.WriteFile(filepath.Join(dir, manifestFileName), b, 0600); err != nil { + t.Fatalf("write manifest: %v", err) + } + } + + t.Run("no manifest → no conflicts (first pull)", func(t *testing.T) { + tmpDir := t.TempDir() + if got := detectPullConflicts(tmpDir); len(got) != 0 { + t.Errorf("expected nil/empty, got %v", got) + } + }) + + t.Run("all files unchanged → no conflicts", func(t *testing.T) { + content := "body { color: red; }" + relPath := "pages/home/style" + writeFile(relPath, content) + writeManifest(map[string]string{ + relPath: hashSource(content), + }) + if got := detectPullConflicts(dir); len(got) != 0 { + t.Errorf("expected no conflicts, got %v", got) + } + }) + + t.Run("modified file → flagged as conflict", func(t *testing.T) { + relPath := "pages/home/config" + writeFile(relPath, `{"title":"v1"}`) + // Manifest records the original hash + writeManifest(map[string]string{ + relPath: hashSource(`{"title":"original"}`), + }) + got := detectPullConflicts(dir) + if len(got) != 1 || got[0] != relPath { + t.Errorf("expected [%q], got %v", relPath, got) + } + }) + + t.Run("missing local file → not a conflict", func(t *testing.T) { + // File is in manifest but not on disk — the pull will restore it; + // that is expected behaviour, not a conflict. + writeManifest(map[string]string{ + "pages/missing/config": hashSource("original"), + }) + if got := detectPullConflicts(dir); len(got) != 0 { + t.Errorf("expected no conflicts for missing file, got %v", got) + } + }) + + t.Run("mixed: one modified + one unchanged → only modified listed", func(t *testing.T) { + clean := "pages/clean/style" + dirty := "pages/dirty/style" + cleanContent := "p { margin: 0; }" + writeFile(clean, cleanContent) + writeFile(dirty, "p { margin: 8px; }") // edited locally + writeManifest(map[string]string{ + clean: hashSource(cleanContent), + dirty: hashSource("p { margin: 0; }"), // manifest has original + }) + got := detectPullConflicts(dir) + sort.Strings(got) + if len(got) != 1 || got[0] != dirty { + t.Errorf("expected [%q], got %v", dirty, got) + } + }) +} diff --git a/plugins/simulator/mcp-server/internal/engines/smartform/register.go b/plugins/simulator/mcp-server/internal/engines/smartform/register.go index d8f7524..c9959d8 100644 --- a/plugins/simulator/mcp-server/internal/engines/smartform/register.go +++ b/plugins/simulator/mcp-server/internal/engines/smartform/register.go @@ -26,15 +26,16 @@ func Register(s *server.MCPServer) { s.AddTool( mcp.NewTool("pullSmartForm", - mcp.WithDescription("Fetch all environment file trees (pages, locale, viewModel, styles, definitions, widgets) of a smart form (CDU / Script application) and write them to //... in the current working directory. Also writes a .manifest.json in each env folder with file IDs and content hashes for use by pushSmartForm. Requires actors.management scope."), + mcp.WithDescription("Fetch all environment file trees (pages, locale, viewModel, styles, definitions, widgets) of a smart form (CDU / Script application) and write them to //... in the current working directory. Also writes a .manifest.json in each env folder with file IDs and content hashes for use by pushSmartForm. Conflict detection: if a prior manifest exists and any local file differs from its last-pulled hash, the pull is refused — push your changes first or pass force=true to discard local edits. Requires actors.management scope."), mcp.WithString("actorId", mcp.Description("Smart form actor UUID."), mcp.Required()), + mcp.WithBoolean("force", mcp.Description("Overwrite local files even when they have unsaved edits (local hash differs from the last-pulled manifest hash). Default false — the pull aborts and lists the conflicting files so you can decide what to do.")), ), handlePullSmartForm, ) s.AddTool( mcp.NewTool("pushSmartForm", - mcp.WithDescription("Reconcile the local develop tree with the server: POST any new folders (parents first) and new files (e.g. a new page like pages//config + locale), PUT any modified files, and update .manifest.json with the returned ids and content hashes. Files/folders present in the manifest but missing locally are reported as orphanFiles but never deleted server-side. MIME defaults: text/css under styles/, application/json elsewhere. Only the develop env is writable; run pullSmartForm first to create the manifest. Requires actors.management scope."), + mcp.WithDescription("Reconcile the local develop tree with the server: POST any new folders (parents first) and new files, PUT any modified files (including MIME-only drift), and update .manifest.json. MIME rules: text/css for styles/, pages//style, and *.css; application/json for everything else. Duplicate guard: before creating a new file, the server tree is checked — if a file already occupies that (folder, title) slot the push aborts with guidance to re-run pullSmartForm. PUT always re-derives the correct MIME type so a previously wrong Content-Type is self-healed without manual delete/recreate. Files in the manifest but missing locally are reported as orphanFiles. Only develop is writable; run pullSmartForm first. Requires actors.management scope."), mcp.WithString("actorId", mcp.Description("Smart form actor UUID — directory /develop/ must exist with a .manifest.json."), mcp.Required()), ), handlePushSmartForm,