diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index ac84474..6deceee 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -28,7 +28,7 @@ "category": "development", "keywords": ["dotcms", "cms", "content-management", "vtl", "nextjs", "headless"], "tags": ["dotcms", "cms", "content-management", "vtl", "nextjs", "headless"], - "skills": ["./skills/dotcms-create-sites", "./skills/dotcms-best-practices"] + "skills": ["./skills/dotcms-create-sites", "./skills/dotcms-best-practices", "./skills/dot-ui-vtl-migration"] } ] } diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 1153c5c..fe2d2d7 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -40,3 +40,9 @@ jobs: - name: Agent Plugins manifests run: node scripts/validate-agent-plugin.mjs + + # The migration skill's local helper has no network and no dependencies beyond pytest. + - name: Migration script tests + run: | + python3 -m pip install --quiet pytest==8.3.4 + python3 -m pytest -q skills/dot-ui-vtl-migration/scripts/tests diff --git a/README.md b/README.md index 1c9f74c..d0053eb 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,9 @@ Cursor, Codex, and any [Agent Skills](https://agentskills.io)-compatible agent. |---|---| | **`dotcms-best-practices`** | Reference for doing any single thing correctly — create a content type, page, template or container; place content; author VTL; wire a headless Next.js frontend; or debug a page that renders blank. Indexed by intent, so the agent loads one file for the task at hand. | | **`dotcms-create-sites`** | Plans and builds a complete site in two phases: an interview that writes `PLAN.md` and `DESIGN.md`, then a build that hands off to `dotcms-best-practices` and holds a verify-and-fix loop until every page type renders. | +| **`dot-ui-vtl-migration`** | Migrates legacy Dojo/Dijit custom field VTL to `DotCustomFieldApi` — one file at a time, or every legacy custom field on an instance through the MCP server plus a local validation script — keeping the original code as a fallback behind the edit-mode switch. | -Both support **VTL-rendered** and **headless (Next.js)** delivery. +`dotcms-best-practices` and `dotcms-create-sites` support **VTL-rendered** and **headless (Next.js)** delivery. > **Why a skill and not just docs:** in dotCMS a missing piece renders blank or shell-only with > **HTTP 200**, never an error. A successful response is not evidence that anything rendered. Every @@ -27,7 +28,7 @@ Both support **VTL-rendered** and **headless (Next.js)** delivery. npx skills add dotCMS/agent-toolkit ``` -Installs both skills. Add `-g` for all projects, `--list` to preview without installing, or +Installs all skills. Add `-g` for all projects, `--list` to preview without installing, or `--skill dotcms-best-practices` for just one. ### Plugin — Claude Code diff --git a/skills/dot-ui-vtl-migration/SKILL.md b/skills/dot-ui-vtl-migration/SKILL.md new file mode 100644 index 0000000..8f149ac --- /dev/null +++ b/skills/dot-ui-vtl-migration/SKILL.md @@ -0,0 +1,393 @@ +--- +name: dot-ui-vtl-migration +description: > + Migrates VTL (Velocity Template Language) custom field templates from the legacy dotCMS Dojo/Dijit API to the modern DotCustomFieldApi. By default it returns a single inline file ready to paste into the Custom Field: the migrated code inside #if( $structures.isNewEditModeEnabled() ) and the original legacy code, untouched, inside #else, with no #parse and no extra files. On request it produces three files instead (_old.vtl, _new.vtl and a #parse router), and it converts existing migrations between the two shapes. Use this skill whenever a user asks to migrate, update, or convert a VTL file, custom field template, or dotCMS field that uses any of: DotCustomFieldApi.get(), DotCustomFieldApi.set(), DotCustomFieldApi.onChangeField(), dojo.ready(), dojo.byId(), dijit.byId(), dijit.form.*, dojoType attributes, or any Dojo/Dijit pattern. Also trigger when the user pastes a VTL snippet and asks "what needs to change" or "can you update this", asks for an inline, single-file or one-file migration ("un solo archivo"), wants to paste the result into the content type editor, or wants to collapse a _new/_old/router migration into one file or split an inline one. Also trigger when the user asks to migrate ALL custom fields, every legacy field, or "the whole instance": the skill finds, downloads, and safely republishes every legacy custom field on a live dotCMS instance through the dotCMS MCP server (see "Migrate a whole instance" below). If in doubt, use this skill. +--- + +# VTL Migration: Legacy API → DotCustomFieldApi + +You are migrating DotCMS VTL custom field templates from Dojo/Dijit-era APIs to the modern `DotCustomFieldApi`. The goal is **identical functionality with modern, clean code** and **semantic styling with DaisyUI**. + +For the full migration rules, all code examples, the DaisyUI styling section, and the step-by-step checklist, read `reference/migration-guide.md`. + +## Migrate a whole instance + +When the user asks to migrate **all** custom fields, every legacy field, or "the whole instance", not a single pasted VTL file, follow [reference/instance-migration.md](reference/instance-migration.md). The `dotcms` MCP server reads and writes the instance, `scripts/vtl_migration.py` (local, no network, plain `python3`) classifies the fields and runs the safety gates, and this skill migrates each file in the default inline mode. Work in batches the customer picks, and never write to the instance without their explicit confirmation. + +## The Core API Swap (Quick Reference) + +| Old (deprecated) | New | +|---|---| +| `DotCustomFieldApi.get('id')` | `DotCustomFieldApi.getField('id').getValue()` | +| `DotCustomFieldApi.set('id', val)` | `DotCustomFieldApi.getField('id').setValue(val)` | +| `DotCustomFieldApi.onChangeField('id', cb)` | `DotCustomFieldApi.getField('id').onChange(cb)` | +| Manual DOM show/hide of a field | `DotCustomFieldApi.getField('id').show()` / `.hide()` | +| Manual DOM enable/disable of a field | `DotCustomFieldApi.getField('id').enable()` / `.disable()` | +| Manual checks against dijit validation state | `DotCustomFieldApi.getField('id').getValidationState()` | +| No legacy equivalent | `DotCustomFieldApi.getField('id').onValidationChange(cb)` | +| `dojo.ready(fn)` | `DotCustomFieldApi.ready(fn)` | +| `dojo.byId('el')` | `document.getElementById('el')` | +| `dijit.byId('id')` | `DotCustomFieldApi.getField('id')` | +| `dijit.form.*` widgets | Native HTML + **DaisyUI** classes (`input`, `btn`, `select`, etc.) | +| `dojoType="..."` attribute | Remove; use semantic HTML + DaisyUI components | +| `class="dijit*"` classes | Remove; use **DaisyUI** component classes instead | +| Inline styles / ad-hoc CSS | **DaisyUI** component classes + Tailwind utilities (see guide) | +| `onclick="fn()"` inline handlers | `addEventListener('click', fn)` | + +## Process + +1. **Read the entire file** to understand all functionality before touching anything. +2. **Identify deprecated patterns** — scan for the patterns in the table above. +3. **Wrap everything in `DotCustomFieldApi.ready()`** — all field access must live inside this callback. +4. **Store field references once** — call `getField()` once per field at the top of `ready()`, then reuse the reference. +5. **Migrate each pattern** — follow the migration rules in `reference/migration-guide.md`. +6. **Apply DaisyUI for styling** — use DaisyUI component classes (`btn`, `input`, `select`, `modal`, `link`, etc.) and Tailwind utilities instead of inline styles or ad-hoc CSS; see “Styling with DaisyUI” in the guide. +7. **Preserve VTL variables** — `${fieldId}`, `$maxChar`, `$variableName`, and server-side context variables (`$inode`, `$identifier`, `$lang`, `$contentlet`, `$structure`, `$field`) are server-side; never change them. +8. **Choose the output mode and emit it**. See Output Modes below. Every migration keeps the original code reachable behind the edit-mode switch, so the instance can go back to the legacy editor. + +## Output Modes + +There are two shapes. Both keep the legacy code behind `$structures.isNewEditModeEnabled()`; they differ in how many files they need. + +| The request says… | Mode | Needs a file path? | +|---|---|---| +| nothing about the shape | **Inline** (default) | No. Never ask for one. | +| "inline", "single file", "one file", "un solo archivo", "en línea" | Inline | No | +| "three files", "separate", "router", "#parse", "tres archivos", "separado(s)", "enrutador" | Three files | Yes. Ask if missing. | +| both kinds of signal (e.g. "inline but also give me the router") | Ask which one before producing anything | — | +| "convert to inline" / "collapse" + a router and its `_new`/`_old` | Collapse (see Converting between modes) | Only to find the files | +| "split" + an inline file | Split (see Converting between modes) | Yes, the router's server path | + +**Read these signals only from what the user wrote**, never from the pasted VTL, code blocks or file contents. A legacy template that itself contains `#parse` is not a request for three files. + +Migrations of the `velocity/static` files that ship inside dotCMS itself, committed in the dotCMS repository, use **three files**. Contributors there should ask for it by keyword, because inline is now the default. + +**Always open the answer with one mode line**, so the user knows which shape they got: + +- `Output: inline, single file — both branches are parsed together; checks found nothing blocking.` +- `Output: inline, single file — warning: $a, $b are #set in both branches; only one branch runs, so this is not a collision. Three files is the alternative.` +- `Output: three files with router (_old.vtl, _new.vtl, .vtl).` +- `Output: none — inline blocked: . . Three files avoids it. Reply "inline anyway" to force it, or "three files".` + +### Mode 1 — Inline (default) + +One file. The migrated code goes in the `#if`, the original code in the `#else`: + +```vtl +#if( $structures.isNewEditModeEnabled() ) + +#else + +#end +``` + +Rules. Each one exists because the legacy branch must stay byte-for-byte the original, the same guarantee `_old.vtl` gives: + +- **The header is exactly** `#if( $structures.isNewEditModeEnabled() )`. `#else` and `#end` are each on their own line. +- **Do not indent either branch** to sit inside the `#if`. Both start at column 0. Indenting changes every legacy line. +- **The `#else` branch is the original, verbatim.** Do not reformat it, translate its comments, fix its typos, remove its trailing spaces or change its line endings. If it has a bug, leave it; mention it in your answer if it matters. +- **Never add a `#parse`** and never write `_new`/`_old` files in this mode. +- **Never ask for a filename or server path.** The user may have only the field's content, pasted from the content type editor; inline needs no path. If a path *was* given, the single file keeps the original file's name and replaces it. +- **Run the checks in "Before emitting inline" first.** They can stop the output. + +**When the original is a file on disk** (you were given a path), do not retype it. Write the migrated version to a temporary file, then assemble with the shell so the legacy bytes are copied, not regenerated. This is the only reliable way to keep CRLF line endings and a missing final newline: + +```sh +{ printf '#if( $structures.isNewEditModeEnabled() )\n'; cat migrated.vtl; printf '\n#else\n'; cat original.vtl; printf '\n#end'; } > result.vtl +``` + +When the content was pasted, copy the pasted legacy code into the `#else` exactly as the user sent it. + +### Before emitting inline + +Velocity parses the **whole file** before it renders either branch. With the router, the branch not taken is never even parsed; inline, both are. So some problems that only affected the legacy editor in the three-file shape would break the new editor too. Before emitting inline, read both branches and check: + +| Finding | Severity | What to do | +|---|---|---| +| A `#macro` with the same name in both branches | **Blocking** | Macros are registered at parse time, and the first definition wins. That is the migrated one, so the legacy editor would silently run the new macro. Emit nothing; name the macro; offer three files. | +| The legacy branch would not parse on its own: a `#if`/`#foreach`/`#macro`/`#define` without its `#end`, a stray `#end`, an `#else` outside an `#if` | **Blocking** | Inline would carry that parse error into the new edit mode. Emit nothing; give the line; offer three files. | +| The migrated branch would not parse on its own | **Blocking** | If you migrated it, that is a bug in your migration: fix it before emitting anything. If it came from the user (collapse or split), report the line like the legacy case and write nothing. | +| The same variable `#set` in both branches | Warning | `#set` runs at render time and only one branch renders, so this is not a collision. Emit, but list the names in the mode line. | + +Count directives the way Velocity does. Text inside `##`, `#* *#` and `#[[ ]]#` is not code, and `\#end` is escaped. A directive name also ends at the first character that is not a letter, digit, `_` or `@`, so `$('#end-date')` in a script **is** an `#end`, while `#endDate` is not. The full rules and examples are in `reference/migration-guide.md` → "Output Modes". + +After a blocking finding, emit inline only if the user explicitly insists ("inline anyway"), and restate the finding in the mode line when you do. + +### Mode 2 — Three files with a router + +Use this mode when the request asks for it (see the table above). Given an original file at `/static/personas/keytag_custom_field.vtl`, the three outputs are: + +--- + +#### File 1 — `keytag_custom_field_old.vtl` +The **original file content, completely unchanged**. Copy it verbatim — every deprecated API call, every dijit class, every dojo.ready. This is the fallback for the legacy editor. + +--- + +#### File 2 — `keytag_custom_field_new.vtl` +The **fully migrated file** with all deprecated patterns replaced per the migration rules. + +--- + +#### File 3 — `keytag_custom_field.vtl` (replaces the original) +The **conditional router** — this file takes the name of the original and delegates to `_new` or `_old` based on which edit mode is active: + +```vtl +#if( $structures.isNewEditModeEnabled() ) + #parse('/static/personas/keytag_custom_field_new.vtl') +#else + #parse('/static/personas/keytag_custom_field_old.vtl') +#end +``` + +The `#parse` paths must use the **full server path** of the file, not just the filename. Use the same directory as the original file. Indent the `#parse` lines with a tab and end the file right after `#end`, with no final newline, as the routers already in the repository do. On disk, write it with `printf` so no newline is added. + +--- + +This pattern lets both legacy and new edit modes coexist safely — old editor users continue using the deprecated code, new editor users get the modernized version. + +**When the user gives you a file path**, derive all three filenames automatically. In three-file mode only, if you receive file contents without a path, ask for the filename and its server path before outputting, because `#parse` cannot work without it. + +### Converting between modes + +Conversion only moves content. **Never apply a migration rule to either branch while converting.** It is a re-shaping, not a re-migration. + +**Collapse (router + `_new` + `_old` → one inline file).** Read the router's two `#parse` paths and find the `_new` and `_old` files. The `_new` content goes in the `#if` and the `_old` content in the `#else`, using the Mode 1 layout. Run "Before emitting inline" first; the same findings apply. If the router's paths do not match the files you were given (different names or directories), say so and stop. Do not guess. + +**Split (one inline file → router + `_new` + `_old`).** You need the router's full server path; ask for it if you do not have it. + +1. Find where the branches end. Line 1 is the header. The legacy branch starts after the **top-level** `#else`: the first `#else` line reached while no block opened after line 1 is still open. Count block openers (`#if`, `#foreach`, `#macro`, `#define`, `#literal`, `#@name`) and `#end`s from line 2 on. **Never just take the first `#else`**, because branches often contain their own `#if … #else … #end`. The file ends with the matching `#end`. +2. `_new.vtl` is everything between the header and the top-level `#else`. `_old.vtl` is everything between that `#else` and the final `#end`. In both cases, drop the one newline the inline layout added before the `#else` and before the `#end`. +3. Write the router in Mode 2's shape: tab-indented `#parse` lines with full server paths. + +If the file does not start with `#if( $structures.isNewEditModeEnabled() )`, or has no top-level `#else`, it is not an inline migration. Say so; do not guess. + +**On disk, use the shell for both directions** so bytes are copied, never retyped. Collapse is the Mode 1 command, with `_new` in place of `migrated.vtl` and `_old` in place of `original.vtl`. Split takes line ranges, where `E` is the line number of the top-level `#else` and `L` is the line number of the final `#end`: + +```sh +sed -n "2,$((E-1))p" inline.vtl | perl -pe 'chomp if eof' > name_new.vtl +sed -n "$((E+1)),$((L-1))p" inline.vtl | perl -pe 'chomp if eof' > name_old.vtl +``` + +## Non-Negotiables + +- All `getField()` calls must be inside `DotCustomFieldApi.ready()` +- Never use `DotCustomFieldApi.get()` or `DotCustomFieldApi.set()` (the old short forms) +- VTL variables stay exactly as-is +- Business logic stays exactly as-is — only the API calls and styling approach change +- Dijit CSS classes (any `class="dijit*"`) must be removed +- **Styling:** Prefer DaisyUI component classes + Tailwind utilities; keep custom CSS only when the guide says so +- Translate non-English comments to English **in the migrated code only** +- The legacy code (`#else` branch or `_old.vtl`) is never edited, not even its comments or whitespace +- Server-side VTL variables (`$inode`, `$identifier`, `$lang`, `$contentlet`, `$structure`, `$field`) are resolved at render time — do not confuse them with `DotCustomFieldApi` JavaScript APIs + +## Key Patterns to Know + +**Field reference lifecycle:** +```js +DotCustomFieldApi.ready(() => { + // Get once, reuse everywhere + const titleField = DotCustomFieldApi.getField('title'); + const urlField = DotCustomFieldApi.getField('url'); + + // Read + const current = titleField.getValue() || ''; + + // Write + urlField.setValue(slugify(current)); + + // Watch + titleField.onChange((value) => { + urlField.setValue(slugify(value)); + }); +}); +``` + +**Field visibility and state control:** +```js +DotCustomFieldApi.ready(() => { + const mediaField = DotCustomFieldApi.getField('media'); + const mediaFileField = DotCustomFieldApi.getField('mediafile'); + + // Show/hide based on current value + if (mediaField.getValue() === 'upload') { + mediaFileField.show(); + } else { + mediaFileField.hide(); + } + + // React to changes + mediaField.onChange((value) => { + if (value === 'upload') { + mediaFileField.show(); + } else { + mediaFileField.hide(); + } + }); + + // Enable/disable a field + mediaFileField.disable(); // blocks editing, applies disabled styling + mediaFileField.enable(); // restores interactivity +}); +``` + +**Reacting to validation state (required, errors, touched):** +```html + + + +``` + +> Use a self-contained `is-invalid` class with inline ` + + + + +``` + +**Important gotchas:** + +- **Always gate on `state.touched`.** A required + empty field is `invalid: true` from the moment it mounts. If you toggle only on `invalid`, the input flashes red on first render before the user has done anything. +- **Don't rely on DaisyUI's `input-error` modifier inside legacy iframe templates.** The legacy iframe page (`legacy-custom-field.jsp`) does NOT load DaisyUI or Tailwind. Ship your error styles inline in the template (the example above does that with a small ` + +
+
+ $maxChar characters +
+
Recommended Max $maxChar characters
+
+ + +``` + +**New (text-count.vtl):** Prefer DaisyUI/Tailwind for styling when replacing or adding styles. Example using semantic classes for the wrapper: +```html + +
+
+ $maxChar characters +
+
Recommended Max $maxChar characters
+
+ + +``` + +--- + +### Example 2: Title Field with Auto-generated URL and Friendly Name + +**Old (title_custom_field.vtl — using dijit.form API):** +```html + + + + + +``` + +> **Note:** In the old pattern, `dojo.byId("title")` refers to a hidden input element that dotCMS generates for each field (its `id` matches the field's variable name). The visible `` is the Dijit widget overlay. In the new pattern, `DotCustomFieldApi.getField("title")` replaces both — there is no need to manage hidden inputs directly. + +**New (title_custom_field.vtl):** Use DaisyUI `input` for semantic styling: +```html + + +``` + +--- + +### Example 3: Slug Generator with Suggestions + +**Old (slug-generator.vtl):** +```html + + + + +``` + +**New (slug-generator.vtl):** +```html + + + + +``` + +--- + +### Example 4: Conditional Field Visibility (show/hide) + +This example shows a "Media" form where the "Media File" field is shown or hidden based on the "Media Location" radio selection. The custom field stores the selection in one field and toggles visibility of another. + +**New (show_hide.vtl):** +```html + + +
+ Media Location +
+ + +
+
+``` + +Key points: +- `mediaFileField.show()` and `mediaFileField.hide()` control the visibility of the entire "mediafile" field (input + label) — no manual DOM manipulation needed. +- The radio buttons use DaisyUI classes (`radio radio-primary`) and are wrapped in a `fieldset` with `fieldset-legend`. +- The initial visibility is set based on `mediaField.getValue()` before attaching the change listener. + +--- + +### Example 5: Conditional Field Enable/Disable + +This example disables an "Expiration Date" field until the user checks an "Enable Expiration" checkbox. + +**New (expiration_toggle.vtl):** +```html + +``` + +Key points: +- `expirationDateField.disable()` prevents editing and applies disabled styling; `enable()` restores interactivity. +- No manual DOM attribute manipulation (`setAttribute("disabled", ...)`) is needed. +- The pattern is the same as show/hide: set initial state, then react to changes via `onChange`. diff --git a/skills/dot-ui-vtl-migration/scripts/.gitignore b/skills/dot-ui-vtl-migration/scripts/.gitignore new file mode 100644 index 0000000..6c56ff1 --- /dev/null +++ b/skills/dot-ui-vtl-migration/scripts/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +.pytest_cache/ diff --git a/skills/dot-ui-vtl-migration/scripts/tests/conftest.py b/skills/dot-ui-vtl-migration/scripts/tests/conftest.py new file mode 100644 index 0000000..ece8e0c --- /dev/null +++ b/skills/dot-ui-vtl-migration/scripts/tests/conftest.py @@ -0,0 +1,4 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) diff --git a/skills/dot-ui-vtl-migration/scripts/tests/test_field_body.py b/skills/dot-ui-vtl-migration/scripts/tests/test_field_body.py new file mode 100644 index 0000000..2457066 --- /dev/null +++ b/skills/dot-ui-vtl-migration/scripts/tests/test_field_body.py @@ -0,0 +1,120 @@ +"""`field-body` builds the PUT body for a field update. The v3 update replaces the field's +variables wholesale, so what matters is that nothing else is dropped.""" + +import json + +import pytest + +import vtl_migration as vm +from test_workflow import DA_ID, INLINE_MIGRATED, LEGACY, download, field, migrate, write_meta, write_scan + +OTHER_VARIABLE = {"clazz": vm.FIELD_VARIABLE_CLAZZ, "key": "keepMe", "value": "42", "id": "v1", "fieldId": "f-legacy"} + + +def fetched(values=LEGACY, variables=None): + return {"id": "f-legacy", "variable": "legacy", "values": values, "fieldVariables": variables or [], "required": True} + + +def write_field(workdir, **kwargs): + path = workdir / "fetched.json" + path.write_text(json.dumps(fetched(**kwargs))) + return path + + +@pytest.fixture +def instance(tmp_path): + write_scan( + tmp_path, + field("A", "legacy", LEGACY), + field("A", "viaAsset", f'#dotParse("/dA/{DA_ID}")'), + ) + write_meta(tmp_path) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path) + vm.cmd_build_manifest(tmp_path, None) + migrate(tmp_path, "fields/A.legacy.vtl", INLINE_MIGRATED) + return tmp_path + + +def test_a_field_gets_the_migrated_code_and_component_mode(instance): + result = vm.cmd_field_body(instance, "A.legacy", write_field(instance)) + updated = result["body"]["field"] + assert result["verdict"] == "ok" + assert updated["values"] == INLINE_MIGRATED + assert [(v["key"], v["value"]) for v in updated["fieldVariables"]] == [("newRenderMode", "component")] + assert updated["required"] is True + + +def test_every_other_field_variable_is_kept(instance): + result = vm.cmd_field_body(instance, "A.legacy", write_field(instance, variables=[OTHER_VARIABLE])) + keys = [v["key"] for v in result["body"]["field"]["fieldVariables"]] + assert keys == ["keepMe", "newRenderMode"] + + +def test_an_existing_render_mode_variable_is_updated_not_duplicated(instance): + existing = {"clazz": vm.FIELD_VARIABLE_CLAZZ, "key": "newRenderMode", "value": "iframe", "id": "keep-this-id", "fieldId": "f-legacy"} + result = vm.cmd_field_body(instance, "A.legacy", write_field(instance, variables=[existing, OTHER_VARIABLE])) + flags = [v for v in result["body"]["field"]["fieldVariables"] if v["key"] == "newRenderMode"] + assert len(flags) == 1 and flags[0]["value"] == "component" and flags[0]["id"] == "keep-this-id" + + +def test_a_field_changed_on_the_server_since_download_is_skipped(instance): + result = vm.cmd_field_body(instance, "A.legacy", write_field(instance, values=LEGACY + " // edited")) + assert result["verdict"] == "skipped" and "changed on the server" in result["reason"] + assert "body" not in result + + +def test_a_render_mode_switch_changes_only_the_variable(instance): + code = "

already migrated code

" + result = vm.cmd_field_body(instance, "A.viaAsset", write_field(instance, values=code, variables=[OTHER_VARIABLE])) + updated = result["body"]["field"] + assert updated["values"] == code + assert [v["key"] for v in updated["fieldVariables"]] == ["keepMe", "newRenderMode"] + + +def test_a_render_mode_switch_is_skipped_when_already_component(instance): + flag = {"clazz": vm.FIELD_VARIABLE_CLAZZ, "key": "newRenderMode", "value": "component", "id": "x", "fieldId": "f-legacy"} + result = vm.cmd_field_body(instance, "A.viaAsset", write_field(instance, variables=[flag])) + assert result["verdict"] == "skipped" and "already" in result["reason"] + + +def test_the_input_field_is_not_modified_in_place(instance): + original = fetched(variables=[OTHER_VARIABLE]) + vm.with_component_render_mode(original) + assert original["fieldVariables"] == [OTHER_VARIABLE] + + +def test_an_asset_entry_is_refused(instance): + with pytest.raises(vm.InputError): + vm.cmd_field_body(instance, DA_ID, write_field(instance)) + + +def test_a_field_file_without_an_id_is_refused(instance): + path = instance / "bad.json" + path.write_text(json.dumps({"values": LEGACY})) + with pytest.raises(vm.InputError): + vm.cmd_field_body(instance, "A.legacy", path) + + +def test_the_cli_prints_the_body(instance, capsys): + path = write_field(instance) + assert vm.main(["--workdir", str(instance), "field-body", "A.legacy", "--field-file", str(path)]) == 0 + assert json.loads(capsys.readouterr().out)["body"]["field"]["values"] == INLINE_MIGRATED + + +def test_crlf_in_the_original_is_compared_and_kept_verbatim(tmp_path): + crlf = "" + migrated = "#if( $structures.isNewEditModeEnabled() )\r\n

new

\r\n#else\r\n" + crlf + "\r\n#end" + write_scan(tmp_path, field("A", "legacy", crlf)) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + vm.cmd_build_manifest(tmp_path, None) + (tmp_path / "migrated" / "fields").mkdir(parents=True) + vm.write_text_verbatim(tmp_path / "migrated" / "fields" / "A.legacy.vtl", migrated) + server = {"id": "f-legacy", "values": crlf, "fieldVariables": []} + field_file = tmp_path / "tmp.json" + field_file.write_text(json.dumps(server)) + result = vm.cmd_field_body(tmp_path, "A.legacy", field_file) + assert result["verdict"] == "ok" + assert result["body"]["field"]["values"] == migrated diff --git a/skills/dot-ui-vtl-migration/scripts/tests/test_inline_problem.py b/skills/dot-ui-vtl-migration/scripts/tests/test_inline_problem.py new file mode 100644 index 0000000..612af11 --- /dev/null +++ b/skills/dot-ui-vtl-migration/scripts/tests/test_inline_problem.py @@ -0,0 +1,227 @@ +"""FR-010 gap fix: `inline_problem` must verify the original legacy code is verbatim +specifically INSIDE the top-level #else branch — not merely present anywhere in the +migrated file. Regression coverage for the gap both GitHub Copilot's and Claude's +review flagged on PR dotCMS/core#37768.""" + +import vtl_migration as mcf + +ORIGINAL = b"" + + +def test_verbatim_in_else_still_passes(): + """Baseline: unchanged behavior for the simple, honest case.""" + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n" + b"

new component

\n" + b"#else\n" + ORIGINAL + b"\n" + b"#end" + ) + assert mcf.inline_problem(ORIGINAL, migrated) is None + + +def test_missing_marker_still_reports_missing_branch(): + """Baseline: unchanged behavior when the marker itself is absent.""" + migrated = b"

no branch here, oops

" + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + assert "isNewEditModeEnabled" in problem + + +def test_not_verbatim_simple_else_still_rejected(): + """Baseline: a genuinely different, non-nested #else is still rejected.""" + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n" + b"

migrated

\n" + b"#else\n

NOT the original code

\n#end" + ) + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + assert "verbatim" in problem + + +def test_original_hidden_in_if_branch_is_rejected(): + """THE REGRESSION TEST: the old substring-anywhere check would wrongly PASS this, + because ORIGINAL appears inside a comment in the #if branch while the #else branch + has genuinely different (broken) legacy behavior.""" + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n" + b"\n" + b"

brand new modern UI

\n" + b"#else\n" + b"

TOTALLY DIFFERENT -- legacy behavior silently dropped

\n" + b"#end" + ) + assert ORIGINAL in migrated # sanity check the trap actually exists + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + assert "verbatim" in problem + + +def test_original_hidden_in_if_branch_with_empty_else_is_rejected(): + """Variant: the #else branch is empty instead of merely different.""" + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n" + b"\n" + b"

new

\n" + b"#else\n#end" + ) + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + assert "verbatim" in problem + + +def test_nested_if_else_in_both_branches_finds_true_top_level_else(): + """THE DEFEATING CASE: both the #if (migrated) and #else (legacy) halves contain + their own nested #if/#else/#end. A naive "take the first #else" would wrongly grab + the nested one inside the #if-branch instead of the real top-level one.""" + legacy_nested = ( + b"#if( $inode )\n

Editing existing legacy content

\n" + b"#else\n

New legacy content

\n#end" + ) + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n" + b"#if( $field.required() )\n

required field UI

\n" + b"#else\n

optional field UI

\n#end\n" + b"#else\n" + legacy_nested + b"\n" + b"#end" + ) + assert mcf.inline_problem(legacy_nested, migrated) is None + + +def test_nested_if_else_does_not_let_migrated_branch_content_leak_as_verbatim(): + """Companion: confirms the nested case is a real test, not a tautology — if the + "legacy" bytes we check for actually belong to the migrated half, it must still be + rejected.""" + not_the_legacy_branch = b"

required field UI

" + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n" + b"#if( $field.required() )\n" + not_the_legacy_branch + b"\n" + b"#else\n

optional field UI

\n#end\n" + b"#else\n#if( $inode )\n

Editing existing legacy content

\n" + b"#else\n

New legacy content

\n#end\n" + b"#end" + ) + problem = mcf.inline_problem(not_the_legacy_branch, migrated) + assert problem is not None + assert "verbatim" in problem + + +def test_no_top_level_else_is_reported_not_crashed(): + """Malformed input: marker present, but no #else at all.""" + migrated = b"#if( $structures.isNewEditModeEnabled() )\n

only if, no else

\n#end" + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + assert "else" in problem + + +def test_unbalanced_block_is_reported_not_crashed(): + """Malformed input: a #foreach in the #else branch is never closed.""" + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n

new

\n" + b"#else\n#foreach( $x in $list )\n

$x

\n" + ORIGINAL + b"\n#end" + ) + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + + +def test_missing_header_is_reported_not_crashed(): + """Malformed input: the marker string appears, but not as line 1's real header.""" + migrated = b"\n" + ORIGINAL + problem = mcf.inline_problem(ORIGINAL, migrated) + assert problem is not None + + +def test_end_date_selector_inside_legacy_code_is_flagged_not_silently_mismatched(): + """SKILL.md's own worked example: `#end-date` inside a jQuery-style selector IS a + real #end directive per Velocity's directive-name-boundary rule (name greedily + matches "end", "-date" is ordinary text) — not a coincidental string match. Because + this specific legacy snippet also wraps that selector in its own #if( $maxChar ) + ... #end, the #end-date directive actually closes that #if in real Velocity, leaving + the snippet's own trailing #end unmatched — i.e. this legacy code is *itself* + unbalanced (SKILL.md's own "Before emitting inline" blocking case #2: "the legacy + branch would not parse on its own"), which is out of scope for the skill's inline + mode to have produced in the first place. `inline_problem` must not crash on it, and + correctly refuses to treat it as a verified-verbatim match (better to flag it for a + human than silently publish a file carrying this latent parsing hazard).""" + original = ( + b"#if( $maxChar )\n" + b"\n" + b"#end" + ) + migrated = ( + b"#if( $structures.isNewEditModeEnabled() )\n

new

\n" + b"#else\n" + original + b"\n#end" + ) + problem = mcf.inline_problem(original, migrated) + assert problem is not None # never crashes; correctly declines to pass this as safe + + +def test_end_date_selector_without_surrounding_block_is_recognized_as_a_real_directive(): + """Companion, without the self-unbalancing #if/#end wrapper: confirms `#end-date` is + read as a real #end (per Velocity's grammar) rather than ignored as plain text — the + tokenizer must not be fooled into treating it as harmless string content either.""" + legacy = b"" + migrated = b"#if( $structures.isNewEditModeEnabled() )\n

new

\n#else\n" + legacy + b"\n#end" + # The embedded "#end" (from "#end-date") is a real directive that closes the header's + # own #if early in actual Velocity parsing, before this file's own final #end — so + # the located "legacy branch" is truncated at that point, and the verbatim check + # correctly does not consider the untruncated original verbatim-matched. + problem = mcf.inline_problem(legacy, migrated) + assert problem is not None + + +HEADER = b"#if( $structures.isNewEditModeEnabled() )\n" + + +def inline(new: bytes, original: bytes = ORIGINAL) -> bytes: + return HEADER + new + b"\n#else\n" + original + b"\n#end" + + +def test_a_stray_end_after_the_inline_block_is_rejected(): + problem = mcf.inline_problem(ORIGINAL, inline(b"

new

") + b"\n#end") + assert problem and "unmatched #end" in problem + + +def test_a_stray_else_after_the_inline_block_is_rejected(): + assert mcf.inline_problem(ORIGINAL, inline(b"

new

") + b"\n#else") is not None + + +def test_balanced_blocks_after_the_inline_block_are_allowed(): + assert mcf.inline_problem(ORIGINAL, inline(b"

new

") + b"\n#if( $x )a#end") is None + + +def test_a_second_top_level_else_is_rejected(): + migrated = HEADER + b"

new

\n#else\n" + ORIGINAL + b"\n#else\nmore\n#end" + assert "more than one top-level #else" in mcf.inline_problem(ORIGINAL, migrated) + + +def test_extra_code_in_the_else_branch_is_rejected(): + migrated = HEADER + b"

new

\n#else\nextra code\n" + ORIGINAL + b"\n#end" + assert "verbatim" in mcf.inline_problem(ORIGINAL, migrated) + + +def test_the_else_branch_keeps_crlf_and_a_missing_final_newline(): + original = b"line one\r\nline two dojo.ready(x)" + assert mcf.inline_problem(original, inline(b"

new

", original)) is None + + +def test_legacy_copied_into_both_branches_is_rejected(): + problem = mcf.inline_problem(ORIGINAL, inline(ORIGINAL)) + assert problem and "#if branch still has legacy code" in problem + + +def test_the_same_macro_in_both_branches_is_rejected(): + original = b"#macro( show $x )old $x#end\n

legacy dojo.byId('a')

" + problem = mcf.inline_problem(original, inline(b"#macro( show $x )new $x#end", original)) + assert problem and "show" in problem + + +def test_different_macros_in_each_branch_are_allowed(): + original = b"#macro( showOld $x )old $x#end\n

legacy

" + assert mcf.inline_problem(original, inline(b"#macro( showNew $x )new $x#end", original)) is None + + +def test_is_inline_migrated_needs_the_real_wrapper_not_a_mention(): + assert mcf.is_inline_migrated(inline(b"

new

")) + assert not mcf.is_inline_migrated(b"") + assert not mcf.is_inline_migrated(b"") diff --git a/skills/dot-ui-vtl-migration/scripts/tests/test_workflow.py b/skills/dot-ui-vtl-migration/scripts/tests/test_workflow.py new file mode 100644 index 0000000..6f2fb46 --- /dev/null +++ b/skills/dot-ui-vtl-migration/scripts/tests/test_workflow.py @@ -0,0 +1,445 @@ +"""End-to-end coverage of the local workflow: classify -> assets-plan -> build-manifest -> +check -> payload -> record. No network: the tests write the files the agent would write.""" + +import base64 +import json + +import pytest + +import vtl_migration as vm + +LEGACY = "" +INLINE_MIGRATED = "#if( $structures.isNewEditModeEnabled() )\n

new

\n#else\n" + LEGACY + "\n#end" +DA_ID = "0ec62ffd3666f23fc5228ef84a481b09" + + +def field(type_, name, values, component=False): + return {"type": type_, "typeId": f"t-{type_}", "field": name, "fieldId": f"f-{name}", "values": values, "component": component} + + +def write_scan(workdir, *records): + (workdir / "scan").mkdir(parents=True, exist_ok=True) + (workdir / "scan" / "part-1.json").write_text(json.dumps(list(records))) + + +def write_meta(workdir, da_id=DA_ID, **overrides): + meta = { + "identifier": da_id, + "inode": "inode-1", + "languageId": 1, + "contentType": "dotAsset", + "live": True, + "fileName": "count.vtl", + "binaryField": "asset", + **overrides, + } + (workdir / "assets").mkdir(parents=True, exist_ok=True) + (workdir / "assets" / f"{da_id}.json").write_text(json.dumps(meta)) + + +def download(workdir, name="count.vtl", text=LEGACY, da_id=DA_ID): + path = workdir / "original" / "assets" / f"{da_id}__{name}" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + +def build(workdir): + vm.cmd_classify(workdir) + vm.cmd_assets_plan(workdir) + return vm.cmd_build_manifest(workdir, "demo.dotcms.dev") + + +def test_classify_sorts_every_category(tmp_path): + write_scan( + tmp_path, + field("A", "viaAsset", f'#dotParse("/dA/{DA_ID}")'), + field("A", "other", '#dotParse("/application/x.vtl")'), + field("A", "core", "$velutil.mergeTemplate('/static/personas/x.vtl')"), + field("A", "done", INLINE_MIGRATED), + field("A", "legacy", LEGACY), + field("A", "clean", "

hello

"), + ) + result = vm.cmd_classify(tmp_path) + assert result["counts"] == { + "customFields": 6, + "assetReferences": 1, + "inlineLegacy": 1, + "alreadyMigratedInline": 1, + "coreFiles": 1, + "inlineClean": 1, + "otherPaths": 1, + } + assert result["assetsToResolve"] == [DA_ID] + assert (tmp_path / "original" / "fields" / "A.legacy.vtl").read_text() == LEGACY + + +def test_classify_ignores_a_field_sent_twice(tmp_path): + write_scan(tmp_path, field("A", "legacy", LEGACY), field("A", "legacy", LEGACY)) + assert vm.cmd_classify(tmp_path)["counts"]["customFields"] == 1 + + +def test_classify_rejects_a_record_missing_ids(tmp_path): + write_scan(tmp_path, {"type": "A", "field": "x", "values": LEGACY}) + with pytest.raises(vm.InputError): + vm.cmd_classify(tmp_path) + + +def test_classify_neutralizes_path_traversal_in_variables(tmp_path): + write_scan(tmp_path, field("../../etc", "passwd", LEGACY)) + vm.cmd_classify(tmp_path) + assert (tmp_path / "original" / "fields" / ".._.._etc.passwd.vtl").exists() + assert not (tmp_path.parent / "etc").exists() + + +def test_assets_plan_reports_unresolved_pinned_and_downloads(tmp_path): + write_scan( + tmp_path, + field("A", "ok", f'#dotParse("/dA/{DA_ID}")'), + field("A", "pinned", '#dotParse("/dA/pinned-inode")'), + field("A", "missing", '#dotParse("/dA/nope")'), + field("A", "noname", '#dotParse("/dA/noname")'), + ) + write_meta(tmp_path) + write_meta(tmp_path, "pinned-inode", identifier="the-identifier") + write_meta(tmp_path, "noname", fileName=None) + vm.cmd_classify(tmp_path) + plan = vm.cmd_assets_plan(tmp_path) + assert [d["id"] for d in plan["toDownload"]] == [DA_ID] + assert plan["toDownload"][0]["destination"].endswith(f"original/assets/{DA_ID}__count.vtl") + assert len(plan["pinned"]) == 1 and "the-identifier" in plan["pinned"][0] + assert len(plan["unresolved"]) == 2 + + +def test_assets_plan_neutralizes_a_hostile_file_name(tmp_path): + write_scan(tmp_path, field("A", "ok", f'#dotParse("/dA/{DA_ID}")')) + write_meta(tmp_path, fileName="../../evil.vtl") + vm.cmd_classify(tmp_path) + plan = vm.cmd_assets_plan(tmp_path) + assert plan["toDownload"][0]["file"] == f"assets/{DA_ID}__.._.._evil.vtl" + + +def test_build_manifest_queues_assets_fields_and_render_mode(tmp_path): + write_scan( + tmp_path, + field("A", "viaAsset", f'#dotParse("/dA/{DA_ID}")'), + field("A", "legacy", LEGACY), + field("A", "done", INLINE_MIGRATED), + ) + write_meta(tmp_path) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path) + result = vm.cmd_build_manifest(tmp_path, "demo.dotcms.dev") + assert result["counts"]["pending"] == 2 + assert [e["kind"] for e in result["entries"]] == ["asset", "field", "renderMode", "renderMode"] + by_key = {e["key"]: e for e in result["entries"] if e["kind"] == "renderMode"} + assert by_key["A.viaAsset"]["requires"] == [DA_ID] + assert by_key["A.done"]["requires"] == [] + + +def test_build_manifest_flags_a_failed_download_and_an_already_migrated_file(tmp_path): + write_scan(tmp_path, field("A", "one", f'#dotParse("/dA/{DA_ID}")'), field("A", "two", '#dotParse("/dA/other")')) + write_meta(tmp_path) + write_meta(tmp_path, "other", fileName="done.vtl") + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path, "done.vtl", INLINE_MIGRATED, "other") # DA_ID never downloaded + result = vm.cmd_build_manifest(tmp_path, None) + assert result["counts"]["unresolved"] == 1 + assert result["counts"]["pending"] == 0 + manifest = vm.read_manifest(tmp_path) + assert manifest["knownMigrated"] == {"other": "inode-1"} + + +def test_render_mode_is_skipped_when_the_field_already_uses_component_mode(tmp_path): + write_scan(tmp_path, field("A", "done", INLINE_MIGRATED, component=True)) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + assert vm.cmd_build_manifest(tmp_path, None)["counts"]["renderMode"] == 0 + + +def test_render_mode_is_skipped_for_a_field_with_an_unhandled_path(tmp_path): + write_scan(tmp_path, field("A", "mixed", f'#dotParse("/dA/{DA_ID}") #dotParse("/application/x.vtl")')) + write_meta(tmp_path) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path) + assert vm.cmd_build_manifest(tmp_path, None)["counts"]["renderMode"] == 0 + + +def migrate(workdir, relative, text): + path = workdir / "migrated" / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + +@pytest.fixture +def instance(tmp_path): + write_scan(tmp_path, field("A", "viaAsset", f'#dotParse("/dA/{DA_ID}")'), field("A", "legacy", LEGACY)) + write_meta(tmp_path) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path) + vm.cmd_build_manifest(tmp_path, "demo.dotcms.dev") + return tmp_path + + +def verdicts(result): + return {e["key"]: e["verdict"] for e in result["entries"]} + + +def test_check_waits_for_files_that_are_not_migrated_yet(instance): + result = vm.cmd_check(instance, [], False, False) + assert verdicts(result) == {DA_ID: "waiting", "A.legacy": "waiting", "A.viaAsset": "waiting"} + + +def test_check_accepts_a_genuine_inline_migration(instance): + migrate(instance, "fields/A.legacy.vtl", INLINE_MIGRATED) + result = vm.cmd_check(instance, ["A.legacy"], False, False) + entry = result["entries"][0] + assert entry["verdict"] == "ready" + assert entry["action"] == {"type": "update-field", "typeId": "t-A", "fieldId": "f-legacy"} + + +def test_check_skips_and_records_a_migration_that_loses_the_original(instance): + migrate(instance, "fields/A.legacy.vtl", "#if( $structures.isNewEditModeEnabled() )\n

new

\n#else\n

edited

\n#end") + result = vm.cmd_check(instance, ["A.legacy"], False, False) + assert result["entries"][0]["verdict"] == "skipped" + assert "verbatim" in result["entries"][0]["reason"] + entry = next(e for e in vm.read_manifest(instance)["entries"] if e["key"] == "A.legacy") + assert entry["status"] == "skipped" + + +def test_check_dry_run_does_not_record_anything(instance): + migrate(instance, "fields/A.legacy.vtl", "not a migration") + vm.cmd_check(instance, ["A.legacy"], True, False) + entry = next(e for e in vm.read_manifest(instance)["entries"] if e["key"] == "A.legacy") + assert entry["status"] == "pending" + + +def test_check_skips_a_file_identical_to_the_original(instance): + migrate(instance, "fields/A.legacy.vtl", LEGACY) + assert vm.cmd_check(instance, ["A.legacy"], False, False)["entries"][0]["reason"] == "migrated file is identical to original" + + +def test_one_bad_item_does_not_block_the_rest(instance): + migrate(instance, "fields/A.legacy.vtl", "garbage") + migrate(instance, f"assets/{DA_ID}__count.vtl", INLINE_MIGRATED) + result = vm.cmd_check(instance, [DA_ID, "A.legacy"], False, False) + assert verdicts(result)[DA_ID] == "ready" + assert verdicts(result)["A.legacy"] == "skipped" + + +def test_selecting_an_asset_pulls_in_its_render_mode_switch(instance): + result = vm.cmd_check(instance, [DA_ID], False, False) + assert set(verdicts(result)) == {DA_ID, "A.viaAsset"} + + +def test_render_mode_waits_until_its_asset_is_published(instance): + migrate(instance, f"assets/{DA_ID}__count.vtl", INLINE_MIGRATED) + first = vm.cmd_check(instance, [DA_ID], False, False) + assert verdicts(first) == {DA_ID: "ready", "A.viaAsset": "waiting"} + vm.cmd_record(instance, DA_ID, "published", None, "inode-2") + second = vm.cmd_check(instance, [DA_ID], False, False) + assert verdicts(second) == {DA_ID: "done", "A.viaAsset": "ready"} + + +def test_dry_run_treats_a_ready_asset_as_published_for_its_render_mode(instance): + migrate(instance, f"assets/{DA_ID}__count.vtl", INLINE_MIGRATED) + result = vm.cmd_check(instance, [DA_ID], True, False) + assert verdicts(result) == {DA_ID: "ready", "A.viaAsset": "ready"} + + +def test_finished_entries_are_not_reprocessed_unless_retried(instance): + migrate(instance, "fields/A.legacy.vtl", INLINE_MIGRATED) + vm.cmd_record(instance, "A.legacy", "published", None, None) + assert vm.cmd_check(instance, [], False, False)["counts"].get("done") is None # not selected without --only + assert vm.cmd_check(instance, ["A.legacy"], False, False)["entries"][0]["verdict"] == "done" + assert vm.cmd_check(instance, ["A.legacy"], False, True)["entries"][0]["verdict"] == "ready" + + +def test_check_warns_when_only_matches_nothing(instance): + assert "matched none" in vm.cmd_check(instance, ["nope"], False, False)["warning"] + + +def test_payload_for_an_asset_carries_the_migrated_bytes(instance): + migrate(instance, f"assets/{DA_ID}__count.vtl", INLINE_MIGRATED) + payload = vm.cmd_payload(instance, DA_ID) + assert base64.b64decode(payload["contentBase64"]).decode() == INLINE_MIGRATED + assert payload["binaryField"] == "asset" and payload["expectedInode"] == "inode-1" + + +def test_payload_for_a_field_carries_only_ids(instance): + migrate(instance, "fields/A.legacy.vtl", INLINE_MIGRATED) + payload = vm.cmd_payload(instance, "A.legacy") + assert payload["type"] == "update-field" and "migrated" not in payload and "contentBase64" not in payload + + +def test_payload_without_a_migrated_file_is_an_input_error(instance): + with pytest.raises(vm.InputError): + vm.cmd_payload(instance, "A.legacy") + + +def test_record_a_published_asset_requires_and_stores_the_new_inode(instance): + with pytest.raises(vm.InputError): + vm.cmd_record(instance, DA_ID, "published", None, None) + vm.cmd_record(instance, DA_ID, "published", None, "inode-2") + entry = next(e for e in vm.read_manifest(instance)["entries"] if e["key"] == DA_ID) + assert (entry["status"], entry["inode"], entry["previousInode"]) == ("published", "inode-2", "inode-1") + + +def test_a_published_asset_is_not_downloaded_again_on_a_later_run(instance): + vm.cmd_record(instance, DA_ID, "published", None, "inode-2") + write_meta(instance, inode="inode-2") + vm.cmd_classify(instance) + plan = vm.cmd_assets_plan(instance) + assert plan["toDownload"] == [] + assert len(plan["alreadyMigrated"]) == 1 + + +def test_status_counts_entries(instance): + vm.cmd_record(instance, "A.legacy", "failed", "boom", None) + result = vm.cmd_status(instance) + assert result["counts"] == {"pending": 2, "failed": 1} + + +def test_manifest_write_is_atomic_and_leaves_no_temp_file(instance): + vm.cmd_record(instance, "A.legacy", "failed", "boom", None) + assert not list(instance.glob("*.tmp")) + json.loads((instance / "manifest.json").read_text()) + + +def test_cli_prints_one_json_document_and_exits_2_on_bad_input(tmp_path, capsys): + assert vm.main(["--workdir", str(tmp_path), "status"]) == 2 + assert "error" in json.loads(capsys.readouterr().err) + write_scan(tmp_path, field("A", "legacy", LEGACY)) + assert vm.main(["--workdir", str(tmp_path), "classify"]) == 0 + assert json.loads(capsys.readouterr().out)["command"] == "classify" + + +# ─── regressions found in review ───────────────────────────────────────── + + +def test_rebuilding_the_manifest_keeps_the_progress_already_made(instance): + vm.cmd_record(instance, "A.legacy", "failed", "boom", None) + vm.cmd_record(instance, DA_ID, "skipped", "changed", None) + vm.cmd_classify(instance) + vm.cmd_assets_plan(instance) + vm.cmd_build_manifest(instance, "demo.dotcms.dev") + by_key = {e["key"]: e for e in vm.read_manifest(instance)["entries"]} + assert (by_key["A.legacy"]["status"], by_key["A.legacy"]["statusReason"]) == ("failed", "boom") + assert by_key[DA_ID]["status"] == "skipped" + assert by_key["A.viaAsset"]["status"] == "pending" + + +def test_a_new_version_of_a_published_asset_starts_over_as_pending(instance): + vm.cmd_record(instance, DA_ID, "published", None, "inode-2") + write_meta(instance, inode="inode-3") # edited on the server after publishing: queued again + vm.cmd_classify(instance) + vm.cmd_assets_plan(instance) + download(instance) + vm.cmd_build_manifest(instance, None) + entry = next(e for e in vm.read_manifest(instance)["entries"] if e["key"] == DA_ID) + assert entry["inode"] == "inode-3" and entry["status"] == "pending" + assert "previousInode" not in entry + + +def test_a_skipped_asset_keeps_its_status_while_the_inode_is_unchanged(instance): + vm.cmd_record(instance, DA_ID, "skipped", "changed", None) + vm.cmd_classify(instance) + vm.cmd_assets_plan(instance) + vm.cmd_build_manifest(instance, None) + entry = next(e for e in vm.read_manifest(instance)["entries"] if e["key"] == DA_ID) + assert entry["status"] == "skipped" + + +def test_a_legacy_field_that_only_mentions_the_marker_is_still_migrated(tmp_path): + mention = LEGACY + "\n// later: $structures.isNewEditModeEnabled()" + write_scan(tmp_path, field("A", "legacy", mention)) + result = vm.cmd_classify(tmp_path) + assert result["counts"]["inlineLegacy"] == 1 and result["counts"]["alreadyMigratedInline"] == 0 + + +def test_a_legacy_asset_that_only_mentions_the_marker_gets_an_entry(tmp_path): + write_scan(tmp_path, field("A", "viaAsset", f'#dotParse("/dA/{DA_ID}")')) + write_meta(tmp_path) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path, text=LEGACY + "\n## isNewEditModeEnabled") + result = vm.cmd_build_manifest(tmp_path, None) + assert [e["key"] for e in result["entries"] if e["kind"] == "asset"] == [DA_ID] + + +def test_an_asset_not_confirmed_by_this_run_is_not_treated_as_migrated(instance): + vm.cmd_record(instance, DA_ID, "published", None, "inode-2") + (instance / "assets" / f"{DA_ID}.json").unlink() # this run cannot resolve it any more + vm.cmd_classify(instance) + vm.cmd_assets_plan(instance) + vm.cmd_build_manifest(instance, None) + manifest = vm.read_manifest(instance) + assert manifest["knownMigrated"] == {} + assert not [e for e in manifest["entries"] if e["kind"] == "renderMode" and e["key"] == "A.viaAsset"] + + +def test_invalid_utf8_in_a_metadata_file_does_not_block_the_others(tmp_path): + write_scan(tmp_path, field("A", "one", '#dotParse("/dA/abc")'), field("A", "two", '#dotParse("/dA/def")')) + write_meta(tmp_path, "def") + (tmp_path / "assets").mkdir(exist_ok=True) + (tmp_path / "assets" / "abc.json").write_bytes(b"\xff\xfe{") + vm.cmd_classify(tmp_path) + plan = vm.cmd_assets_plan(tmp_path) + assert [d["id"] for d in plan["toDownload"]] == ["def"] + assert any("abc" in u for u in plan["unresolved"]) + + +def test_a_corrupt_asset_metadata_file_does_not_block_the_others(tmp_path): + write_scan(tmp_path, field("A", "one", '#dotParse("/dA/abc")'), field("A", "two", '#dotParse("/dA/def")')) + write_meta(tmp_path, "def") + (tmp_path / "assets" / "abc.json").write_text("{bad") + vm.cmd_classify(tmp_path) + plan = vm.cmd_assets_plan(tmp_path) + assert [d["id"] for d in plan["toDownload"]] == ["def"] + assert len(plan["unresolved"]) == 1 and "abc" in plan["unresolved"][0] + + +def test_a_corrupt_metadata_file_found_at_build_time_is_unresolved(tmp_path): + write_scan(tmp_path, field("A", "one", f'#dotParse("/dA/{DA_ID}")')) + write_meta(tmp_path) + vm.cmd_classify(tmp_path) + vm.cmd_assets_plan(tmp_path) + download(tmp_path) + (tmp_path / "assets" / f"{DA_ID}.json").write_text("{bad") + result = vm.cmd_build_manifest(tmp_path, None) + assert result["counts"]["unresolved"] == 1 and result["counts"]["pending"] == 0 + + +def test_an_asset_edited_on_the_server_after_publishing_is_downloaded_again(instance): + vm.cmd_record(instance, DA_ID, "published", None, "inode-2") + write_meta(instance, inode="inode-9") # someone edited it since + vm.cmd_classify(instance) + plan = vm.cmd_assets_plan(instance) + assert [d["id"] for d in plan["toDownload"]] == [DA_ID] + assert plan["alreadyMigrated"] == [] + + +def test_a_finished_render_mode_switch_is_not_selected_again_through_its_asset(instance): + vm.cmd_record(instance, "A.viaAsset", "published", None, None) + assert "A.viaAsset" not in verdicts(vm.cmd_check(instance, [DA_ID], False, False)) + + +def test_recording_an_unknown_status_is_an_input_error_not_an_assertion(instance): + manifest = vm.read_manifest(instance) + with pytest.raises(vm.InputError): + vm.record_status(instance, manifest, "A.legacy", "bogus", None) + + +def test_a_missing_original_is_an_input_error(instance): + migrate(instance, "fields/A.legacy.vtl", INLINE_MIGRATED) + (instance / "original" / "fields" / "A.legacy.vtl").unlink() + with pytest.raises(vm.InputError): + vm.cmd_check(instance, ["A.legacy"], False, False) + + +def test_classify_reports_how_many_scan_parts_it_read(tmp_path): + write_scan(tmp_path, field("A", "legacy", LEGACY)) + assert vm.cmd_classify(tmp_path)["scanParts"] == 1 diff --git a/skills/dot-ui-vtl-migration/scripts/vtl_migration.py b/skills/dot-ui-vtl-migration/scripts/vtl_migration.py new file mode 100644 index 0000000..625c0b8 --- /dev/null +++ b/skills/dot-ui-vtl-migration/scripts/vtl_migration.py @@ -0,0 +1,980 @@ +#!/usr/bin/env python3 +""" +Local helper for the dot-ui-vtl-migration skill's "migrate a whole instance" workflow. + +It never talks to dotCMS. The dotCMS MCP server does all the I/O (reading content types, +downloading and publishing files, updating fields); this script does the parts that must be +exact and are easy to get wrong by hand, with no network and no credentials: + + classify Sort every custom field into one of the migration categories. + assets-plan Decide which /dA/ files to download, from the metadata the agent fetched. + build-manifest Build manifest.json from the downloaded files. + status List the manifest entries (what is pending, what is done). + check Run the safety gates on the migrated files and say what is ready to publish. + payload Print what the agent needs to publish one asset. + field-body Build the PUT body for a field update or a render-mode switch. + record Persist one entry's outcome in the manifest, right after the write. + +Working directory layout (default ./vtl-migration, standard library only, Python 3.9+): + + scan/*.json input: arrays of custom fields written by the agent (see `classify`) + assets/.json input: metadata of each /dA/ reference (see `assets-plan`) + original/assets/__ downloaded files + original/fields/.vtl in-field VTL that still uses legacy patterns + migrated/... the migrated result, same relative path as original/ + manifest.json the handoff state; written by build-manifest and record + +Every command prints one JSON document on stdout. Exit codes: 0 success, 2 invalid +arguments or missing/corrupt input. +""" + +from __future__ import annotations + +import argparse +import base64 +import copy +import json +import os +import re +import sys +from datetime import datetime +from pathlib import Path + +MANIFEST_STATUSES = {"pending", "published", "skipped", "failed"} +TERMINAL_STATUSES = {"published", "skipped", "failed"} + +CUSTOM_FIELD_CLAZZ = "com.dotcms.contenttype.model.field.ImmutableCustomField" +MIGRATED_MARKER = "isNewEditModeEnabled" +DOTPARSE_RE = re.compile(r"""#dotParse\(\s*["']([^"']+)["']\s*\)""") +DA_RE = re.compile(r"^/dA/([A-Za-z0-9-]+)(?:/.*)?$") +# $velutil.mergeTemplate('/static/...') / #parse('static/...'): files shipped inside dotCMS itself +CORE_FILE_RE = re.compile(r"""(?:mergeTemplate|#parse)\s*\(\s*["']([^"']+)["']""") +LEGACY_RE = re.compile(r"dojo\.|dijit|dojoType|DotCustomFieldApi\.(?:get|set|onChangeField)\s*\(") +_PATH_SEPARATORS_RE = re.compile(r"[\\/]") + + +class InputError(Exception): + """Missing, corrupt or invalid input. Maps to exit code 2.""" + + +# ─── files ─────────────────────────────────────────────────────────────── + + +def sanitize_path_component(value: str, *, fallback: str) -> str: + """Neutralizes a server-controlled string (a content-type/field `variable`, or an + asset's `fileName`) before it becomes part of a path under the working directory. + Strips path separators and rejects a bare "." or "..". Never raises.""" + cleaned = _PATH_SEPARATORS_RE.sub("_", value.strip()) + return cleaned if cleaned not in ("", ".", "..") else fallback + + +def write_inside(root: Path, relative: str, data) -> None: + """Writes `data` to `root / relative`; refuses a path that resolves outside `root`.""" + target = (root / relative).resolve() + if not target.is_relative_to(root.resolve()): + raise InputError(f"refusing to write outside {root}: {relative!r} resolved to {target}") + target.parent.mkdir(parents=True, exist_ok=True) + if isinstance(data, bytes): + target.write_bytes(data) + else: + write_text_verbatim(target, data) + + +def read_text_verbatim(path: Path) -> str: + """Reads text without newline translation, so CRLF survives a comparison or a PUT body.""" + with open(path, encoding="utf-8", newline="") as f: + return f.read() + + +def write_text_verbatim(path: Path, data: str) -> None: + with open(path, "w", encoding="utf-8", newline="") as f: + f.write(data) + + +def read_json(path: Path): + if not path.exists(): + raise InputError(f"missing {path}") + try: + return json.loads(read_text_verbatim(path)) + except (ValueError, OSError) as e: # ValueError covers JSONDecodeError and UnicodeDecodeError + raise InputError(f"{path} is unreadable or corrupt: {e}") from e + + +def write_json(path: Path, data) -> None: + """Writes atomically: a crash between the temp write and the rename leaves the previous + file intact, never a truncated one.""" + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_suffix(path.suffix + ".tmp") + write_text_verbatim(tmp, json.dumps(data, indent=2)) + os.replace(tmp, path) + + +def manifest_path(workdir: Path) -> Path: + return workdir / "manifest.json" + + +def read_manifest(workdir: Path) -> dict: + path = manifest_path(workdir) + if not path.exists(): + raise InputError(f"No manifest at {path}; run classify, assets-plan and build-manifest first.") + return read_json(path) + + +def try_read_previous_manifest(workdir: Path) -> dict: + """Best-effort read of an earlier manifest, used only to skip re-downloading what an + earlier run already confirmed migrated. A missing or unreadable file is always safe.""" + try: + return read_json(manifest_path(workdir)) + except InputError: + return {} + + +# ─── classify ──────────────────────────────────────────────────────────── + + +class Scan: + """Classification of every custom field on the instance.""" + + def __init__(self) -> None: + self.asset_refs: dict[str, list[str]] = {} # dA id -> [Type.field, ...] + self.inline_fields: list[dict] = [] # inline VTL with legacy patterns (values kept apart) + self.core_files: list[str] = [] + self.inline_clean: list[str] = [] + self.inline_migrated: list[str] = [] + self.other_paths: list[str] = [] + self.other_path_owners: list[str] = [] + # Type.field -> {"typeId", "fieldId", "component"}: every custom field seen + self.field_refs: dict[str, dict] = {} + + def add_field(self, field: dict) -> str | None: + """Classifies one field record. Returns the inline VTL to write when it is legacy.""" + owner = f"{field['type']}.{field['field']}" + values = field.get("values") or "" + self.field_refs[owner] = { + "typeId": field["typeId"], + "fieldId": field["fieldId"], + "component": bool(field.get("component")), + } + + paths = DOTPARSE_RE.findall(values) + for path in paths: + if match := DA_RE.match(path): + self.asset_refs.setdefault(match.group(1), []).append(owner) + else: + self.other_paths.append(f"{owner} -> {path}") + if owner not in self.other_path_owners: + self.other_path_owners.append(owner) + if paths: + return None + + if match := CORE_FILE_RE.search(values): + self.core_files.append(f"{owner} -> {match.group(1)}") + elif is_inline_migrated(values.encode("utf-8")): + self.inline_migrated.append(owner) + elif LEGACY_RE.search(values): + safe_owner = sanitize_path_component(owner, fallback=f"field-{field['fieldId']}") + self.inline_fields.append( + { + "kind": "field", + "key": owner, + "typeId": field["typeId"], + "fieldId": field["fieldId"], + "file": f"fields/{safe_owner}.vtl", + } + ) + return values + else: + self.inline_clean.append(owner) + return None + + def to_json(self) -> dict: + return { + "assetRefs": self.asset_refs, + "inlineFields": self.inline_fields, + "coreFiles": self.core_files, + "inlineClean": self.inline_clean, + "inlineMigrated": self.inline_migrated, + "otherPaths": self.other_paths, + "otherPathOwners": self.other_path_owners, + "fieldRefs": self.field_refs, + } + + @classmethod + def from_json(cls, data: dict) -> "Scan": + scan = cls() + scan.asset_refs = data["assetRefs"] + scan.inline_fields = data["inlineFields"] + scan.core_files = data["coreFiles"] + scan.inline_clean = data["inlineClean"] + scan.inline_migrated = data["inlineMigrated"] + scan.other_paths = data["otherPaths"] + scan.other_path_owners = data["otherPathOwners"] + scan.field_refs = data["fieldRefs"] + return scan + + +def cmd_classify(workdir: Path) -> dict: + """Reads scan/*.json: arrays of {type, typeId, field, fieldId, values, component}, one + record per custom field, where `type`/`field` are the variables and `component` says the + field already has newRenderMode=component. Writes each legacy inline field to + original/fields/ and the classification to scan-result.json.""" + parts = sorted((workdir / "scan").glob("*.json")) + if not parts: + raise InputError(f"no scan/*.json files in {workdir}: write the custom fields there first") + + scan = Scan() + seen: set[str] = set() + original_dir = workdir / "original" + for part in parts: + records = read_json(part) + if not isinstance(records, list): + raise InputError(f"{part} must hold a JSON array of custom fields") + for record in records: + missing = [k for k in ("type", "typeId", "field", "fieldId") if not record.get(k)] + if missing: + raise InputError(f"{part}: a record is missing {', '.join(missing)}") + owner = f"{record['type']}.{record['field']}" + if owner in seen: + continue # the same field sent twice (overlapping batches) + seen.add(owner) + code = scan.add_field(record) + if code is not None: + file = next(f["file"] for f in scan.inline_fields if f["key"] == owner) + write_inside(original_dir, file, code) + + write_json(workdir / "scan-result.json", scan.to_json()) + return { + "command": "classify", + "scanParts": len(parts), + "counts": { + "customFields": len(seen), + "assetReferences": len(scan.asset_refs), + "inlineLegacy": len(scan.inline_fields), + "alreadyMigratedInline": len(scan.inline_migrated), + "coreFiles": len(scan.core_files), + "inlineClean": len(scan.inline_clean), + "otherPaths": len(scan.other_paths), + }, + "assetsToResolve": sorted(scan.asset_refs), + "report": { + "coreFiles": scan.core_files, + "otherPaths": scan.other_paths, + "alreadyMigratedInline": scan.inline_migrated, + }, + } + + +# ─── assets-plan ───────────────────────────────────────────────────────── + + +def known_migrated_inodes(previous_manifest: dict) -> dict[str, str]: + """da_id -> inode already confirmed migrated by an earlier run in this workdir.""" + known = dict(previous_manifest.get("knownMigrated") or {}) + for entry in previous_manifest.get("entries", []): + if entry.get("kind") == "asset" and entry.get("status") == "published": + known[entry["key"]] = entry["inode"] + return known + + +def asset_file_name(meta: dict) -> str | None: + return meta.get("fileName") or meta.get("name") or meta.get("title") + + +def cmd_assets_plan(workdir: Path) -> dict: + """Reads scan-result.json and assets/.json, one per /dA/ reference, each holding the + fields of `GET /api/v1/content/` that matter: identifier, inode, languageId, + contentType, live, fileName (or name/title), binaryField (the variable of the binary + field), plus path/hostName if the download needs them. Decides what to download.""" + scan = Scan.from_json(read_json(workdir / "scan-result.json")) + known = known_migrated_inodes(try_read_previous_manifest(workdir)) + + to_download, unresolved, pinned, already_migrated = [], [], [], [] + already_migrated_ids: dict[str, str] = {} + for da_id, owners in scan.asset_refs.items(): + used_by = ", ".join(owners) + meta_path = workdir / "assets" / f"{da_id}.json" + try: + meta = read_json(meta_path) if meta_path.exists() else None + except InputError as e: + unresolved.append(f"{da_id} ({e}, used by {used_by})") + continue + if not meta or not meta.get("binaryField") or not meta.get("identifier") or not meta.get("inode"): + unresolved.append(f"{da_id} (no usable content metadata, used by {used_by})") + continue + if meta["identifier"] != da_id: + # The field pins one version (an inode), not the identifier. PUBLISH always lands + # on the latest version, so this can never be migrated safely. + pinned.append( + f"{da_id} -> identifier {meta['identifier']} (used by {used_by}); " + "edit the field to reference the identifier if it needs migrating" + ) + continue + file_name = asset_file_name(meta) + if known.get(da_id) == meta["inode"]: + already_migrated.append(f"{meta.get('contentType')} {file_name or da_id} ({da_id})") + already_migrated_ids[da_id] = meta["inode"] + continue + if not file_name: + unresolved.append(f"{da_id} (no fileName/name/title on this content item, used by {used_by})") + continue + safe = sanitize_path_component(file_name, fallback=f"asset-{da_id}") + to_download.append( + { + "id": da_id, + "file": f"assets/{da_id}__{safe}", + "destination": str(workdir / "original" / f"assets/{da_id}__{safe}"), + "fileName": file_name, + "path": meta.get("path"), + "hostName": meta.get("hostName"), + "usedBy": owners, + } + ) + + plan = { + "toDownload": to_download, + "unresolved": unresolved, + "pinned": pinned, + "alreadyMigrated": already_migrated, + # id -> inode, established by this run's metadata; build-manifest trusts only these + "alreadyMigratedIds": already_migrated_ids, + } + write_json(workdir / "assets-plan.json", plan) + return {"command": "assets-plan", **plan} + + +# ─── build-manifest ────────────────────────────────────────────────────── + + +def render_mode_entries(scan: Scan, queued_assets: set[str], migrated_assets: set[str]) -> list[dict]: + """One "renderMode" entry per custom field that still needs newRenderMode=component + and whose code is (or, once published, will be) migrated: + - owners of a /dA/ asset, depending on every asset of theirs that still has to be + published (`requires`); skipped if any of their assets was unresolved or pinned; + - fields whose inline code already carries the edit-mode switch. + Inline legacy fields need no entry: the flag is set in the same update as their code. + Fields that also #dotParse a non-/dA/ path are left alone: part of their code is out of + scope, so flagging them could switch unmigrated code to the new editor.""" + owner_assets: dict[str, set[str]] = {} + for da_id, owners in scan.asset_refs.items(): + for owner in owners: + owner_assets.setdefault(owner, set()).add(da_id) + + candidates: list[tuple[str, list[str]]] = [] + for owner, assets in owner_assets.items(): + if owner in scan.other_path_owners or not assets <= (queued_assets | migrated_assets): + continue + candidates.append((owner, sorted(assets & queued_assets))) + candidates.extend((owner, []) for owner in scan.inline_migrated) + + entries = [] + for owner, requires in candidates: + ref = scan.field_refs[owner] + if ref["component"]: + continue + entries.append( + { + "kind": "renderMode", + "key": owner, + "typeId": ref["typeId"], + "fieldId": ref["fieldId"], + "requires": requires, + "label": f"{owner} (enable component render mode)", + "usedBy": [owner], + "status": "pending", + "statusReason": None, + } + ) + return entries + + +def carry_over_progress(entries: list[dict], previous: dict) -> None: + """A rebuild must not undo work: copy status, reason and previousInode from the previous + manifest's entry with the same key and kind, unless an asset's inode changed since. + An entry that no longer shows up (a field already published is now classified as + migrated) is simply dropped.""" + earlier = {(e["kind"], e["key"]): e for e in previous.get("entries", [])} + for entry in entries: + old = earlier.get((entry["kind"], entry["key"])) + if not old: + continue + if entry["kind"] == "asset" and old.get("inode") != entry["inode"]: + continue # a newer version of the asset: earlier progress belongs to another file + entry["status"] = old.get("status", entry["status"]) + entry["statusReason"] = old.get("statusReason") + if old.get("previousInode"): + entry["previousInode"] = old["previousInode"] + + +def cmd_build_manifest(workdir: Path, instance: str | None) -> dict: + """Reads scan-result.json, assets-plan.json and the downloaded files, writes manifest.json.""" + scan = Scan.from_json(read_json(workdir / "scan-result.json")) + plan = read_json(workdir / "assets-plan.json") + previous = try_read_previous_manifest(workdir) + original_dir = workdir / "original" + + entries: list[dict] = [] + unresolved = list(plan["unresolved"]) + already_migrated = list(plan["alreadyMigrated"]) + unpublished: list[str] = [] + confirmed_migrated = dict(plan.get("alreadyMigratedIds") or {}) + + for item in plan["toDownload"]: + da_id = item["id"] + try: + meta = read_json(workdir / "assets" / f"{da_id}.json") + except InputError as e: + unresolved.append(f"{da_id} ({e}, used by {', '.join(item['usedBy'])})") + continue + downloaded = original_dir / item["file"] + label = f"{meta.get('contentType')} {item['fileName']} ({da_id})" + if not downloaded.is_file(): + unresolved.append(f"{da_id} (download failed, used by {', '.join(item['usedBy'])})") + continue + if is_inline_migrated(downloaded.read_bytes()): + already_migrated.append(label) + confirmed_migrated[da_id] = meta["inode"] + continue + if not meta.get("live"): + unpublished.append(label) + entries.append( + { + "kind": "asset", + "key": da_id, + "identifier": meta["identifier"], + "inode": meta["inode"], + "languageId": meta.get("languageId", 1), + "contentType": meta.get("contentType"), + "binaryField": meta["binaryField"], + "fileName": item["fileName"], + "label": label, + "usedBy": item["usedBy"], + "file": item["file"], + "status": "pending", + "statusReason": None, + } + ) + + for field in scan.inline_fields: + entries.append( + {**field, "label": field["key"], "usedBy": [field["key"]], "status": "pending", "statusReason": None} + ) + + queued_assets = {e["key"] for e in entries if e["kind"] == "asset"} + flags = render_mode_entries(scan, queued_assets, set(confirmed_migrated)) + migration_entries = list(entries) + entries.extend(flags) # after the assets they may depend on: check processes in order + + carry_over_progress(entries, previous) + manifest = { + "instance": instance, + "builtAt": datetime.now().isoformat(), + "knownMigrated": confirmed_migrated, + "entries": entries, + } + write_json(manifest_path(workdir), manifest) + + return { + "command": "build-manifest", + "counts": { + "pending": len(migration_entries), + "renderMode": len(flags), + "alreadyMigrated": len(already_migrated) + len(scan.inline_migrated), + "coreFiles": len(scan.core_files), + "inlineClean": len(scan.inline_clean), + "otherPaths": len(scan.other_paths), + "pinnedVersions": len(plan["pinned"]), + "unresolved": len(unresolved), + }, + "entries": [{k: e[k] for k in ("kind", "key", "label", "usedBy", "file", "requires") if k in e} for e in entries], + "report": { + "alreadyMigrated": already_migrated + scan.inline_migrated, + "notLive": unpublished, + "unresolved": unresolved, + "coreFiles": scan.core_files, + "inlineClean": scan.inline_clean, + "otherPaths": scan.other_paths, + "pinnedVersions": plan["pinned"], + }, + } + + +# ─── inline validity ───────────────────────────────────────────────────── + +_INLINE_HEADER_RE = re.compile(rb"^#if\(\s*\$structures\.isNewEditModeEnabled\(\)\s*\)") +_DIRECTIVE_NAME_RE = re.compile(rb"[A-Za-z0-9_@]+") +_BLOCK_OPENERS = {b"if", b"foreach", b"macro", b"define", b"literal"} + + +# The inline-validity logic below (_iter_directives, locate_top_level_else, inline_problem) is +# copied from dotCMS/core's migrate_custom_fields.py and mirrors the "Before emitting inline" and +# "Converting between modes" rules in SKILL.md and reference/migration-guide.md. Change them together. + + +def _iter_directives(data: bytes): + """Yields (name, start, end) for every real Velocity directive in `data` relevant to + block nesting/branching (`if`/`foreach`/`macro`/`define`/`literal`/`@name` as block + openers, plus `elseif`/`else`/`end`), in source order, following the rules SKILL.md's + "Split" algorithm specifies: `##` line comments, `#* *#` block comments and `#[[ ]]#` + unparsed blocks are skipped; `\\#end` is an escaped literal, not a directive; a + directive name ends at the first character that is not a letter/digit/`_`/`@` (so + `#end-date` IS a real `#end`, while `#endDate` is not one of the names tracked here).""" + i, n = 0, len(data) + while i < n: + b = data[i : i + 1] + if b == b"\\": + if data[i + 1 : i + 2] in (b"\\", b"#"): + i += 2 + continue + i += 1 + continue + if b == b"#": + two = data[i : i + 2] + if two == b"##": + nl = data.find(b"\n", i) + i = n if nl == -1 else nl + continue + if two == b"#*": + end = data.find(b"*#", i + 2) + i = n if end == -1 else end + 2 + continue + if data[i : i + 3] == b"#[[": + end = data.find(b"]]#", i + 3) + i = n if end == -1 else end + 3 + continue + j = i + 1 + brace = data[j : j + 1] == b"{" + if brace: + j += 1 + m = _DIRECTIVE_NAME_RE.match(data, j) + if not m: + i += 1 + continue + name, end_j = m.group(0), m.end() + if brace: + if data[end_j : end_j + 1] == b"}": + end_j += 1 + else: + i += 1 + continue + lname = name.lower() + if lname in (b"elseif", b"else", b"end") or lname in _BLOCK_OPENERS: + yield (lname, i, end_j) + elif name.startswith(b"@"): + yield (b"@", i, end_j) + i = end_j + continue + i += 1 + + +def locate_top_level_else(migrated: bytes) -> tuple[int, int] | str: + """Locates the legacy (`#else`) branch's body in an inline file: line 1 is the fixed + header; counting block openers and `#end`s from there, the top-level `#else` is the first + one reached while nothing opened after the header is still open, and the matching final + `#end` closes the header's own `#if`. Returns (start, end) byte offsets spanning the + legacy body (exclusive of the `#else`/`#end` directives), or a diagnostic string, never + raising, for a malformed shape. Every directive after the outer `#end` is checked too: the + whole file must parse, so a second top-level `#else` or an unmatched `#end` is rejected.""" + header = _INLINE_HEADER_RE.match(migrated) + if not header: + return "migrated file does not start with #if( $structures.isNewEditModeEnabled() ) on line 1" + + depth = 0 + else_body_start = None + end_start = None + for name, start, end in _iter_directives(migrated): + if start < header.end(): + continue # the header's own #if is not itself counted + if end_start is not None: + # trailing content: only balanced blocks may follow the outer #end + if name in _BLOCK_OPENERS or name == b"@": + depth += 1 + elif name == b"end": + if depth == 0: + return "unmatched #end after the end of the inline block" + depth -= 1 + elif name in (b"else", b"elseif") and depth == 0: + return f"#{name.decode()} after the end of the inline block" + continue + if name == b"end": + if depth == 0: + end_start = start + continue + depth -= 1 + elif name == b"else": + if depth == 0: + if else_body_start is not None: + return "more than one top-level #else in the inline block" + else_body_start = end + elif name == b"elseif": + if depth == 0 and else_body_start is not None: + return "#elseif after the top-level #else" + elif name in _BLOCK_OPENERS or name == b"@": + depth += 1 + + if else_body_start is None: + return "no top-level #else branch found in migrated content (legacy code not located)" + if end_start is None: + return ( + "no matching top-level #end found in migrated content " + "(unbalanced #if/#foreach/#macro/#define/#literal/#@ block)" + ) + if depth != 0: + return "unbalanced #if/#foreach/#macro/#define/#literal/#@ block after the inline block" + if end_start <= else_body_start: + return "malformed structure: #end appears before the top-level #else" + return (else_body_start, end_start) + + +_MACRO_NAME_RE = re.compile(rb"\(\s*([A-Za-z_][A-Za-z0-9_\-]*)") + + +def macro_names(data: bytes, lo: int, hi: int) -> set[bytes]: + """Names of the `#macro( name ...)` definitions that start between `lo` and `hi`.""" + names = set() + for name, start, end in _iter_directives(data): + if name == b"macro" and lo <= start < hi and (m := _MACRO_NAME_RE.match(data, end)): + names.add(m.group(1)) + return names + + +def _strip_layout_newline(body: bytes) -> bytes: + """Drops the one newline the inline layout adds after `#else` and before `#end`.""" + for nl in (b"\r\n", b"\n"): + if body.startswith(nl): + body = body[len(nl):] + break + for nl in (b"\r\n", b"\n"): + if body.endswith(nl): + return body[: -len(nl)] + return body + + +def is_inline_migrated(data: bytes) -> bool: + """True when `data` is an inline migration: the edit-mode wrapper opens line 1 and closes + around a top-level `#else`. A legacy file that merely mentions `isNewEditModeEnabled` in a + comment, a string or unrelated JavaScript is not one.""" + return not isinstance(locate_top_level_else(data.lstrip()), str) + + +def inline_problem(original: bytes, migrated: bytes) -> str | None: + """The inline output keeps the legacy code verbatim under #else, next to the new code + under #if( $structures.isNewEditModeEnabled() ). Returns why `migrated` is not a valid + inline migration of `original`, or None. The #else branch must equal the original + (minus the layout newlines), the #if branch must not carry legacy patterns, and the two + branches must not define the same #macro.""" + if MIGRATED_MARKER.encode() not in migrated: + return f"no {MIGRATED_MARKER}() branch (three-file output or blocked migration?)" + + located = locate_top_level_else(migrated) + if isinstance(located, str): + return located + + start, end = located + if _strip_layout_newline(migrated[start:end]) != original: + return "the #else branch is not the original code verbatim" + + header_end = _INLINE_HEADER_RE.match(migrated).end() + new_branch = migrated[header_end:start].decode("utf-8", errors="replace") + if match := LEGACY_RE.search(new_branch): + return f"the #if branch still has legacy code ({match.group(0).strip()!r}); migrate it or offer three files" + + shared = macro_names(migrated, header_end, start) & macro_names(migrated, start, end) + if shared: + names = ", ".join(sorted(n.decode("utf-8", errors="replace") for n in shared)) + return f"#macro defined in both branches ({names}): the first definition wins, so the legacy editor would run the new macro" + return None + + +# ─── field-body ────────────────────────────────────────────────────────── + +RENDER_MODE_KEY = "newRenderMode" +RENDER_MODE_COMPONENT = "component" +FIELD_VARIABLE_CLAZZ = "com.dotcms.contenttype.model.field.ImmutableFieldVariable" + + +def has_component_render_mode(field: dict) -> bool: + return any( + v.get("key") == RENDER_MODE_KEY and v.get("value") == RENDER_MODE_COMPONENT + for v in field.get("fieldVariables") or [] + ) + + +def with_component_render_mode(field: dict) -> dict: + """A copy of `field` whose `newRenderMode` variable is "component". Every other field + variable is kept as-is: the v3 field PUT replaces the variables list wholesale, so any + variable left out would be deleted. An existing `newRenderMode` keeps its id.""" + updated = copy.deepcopy(field) + variables = updated.get("fieldVariables") or [] + existing = next((v for v in variables if v.get("key") == RENDER_MODE_KEY), {}) + flag = { + **existing, + "clazz": FIELD_VARIABLE_CLAZZ, + "key": RENDER_MODE_KEY, + "value": RENDER_MODE_COMPONENT, + "fieldId": field.get("id"), + } + updated["fieldVariables"] = [v for v in variables if v.get("key") != RENDER_MODE_KEY] + [flag] + return updated + + +def cmd_field_body(workdir: Path, key: str, field_file: Path) -> dict: + """Builds the body of `PUT /api/v3/contenttype/{typeId}/fields/{fieldId}` from the field + as the agent just fetched it (`field_file`). For a `field` entry it first checks the field + still holds the original text, then sets the migrated code; for a `renderMode` entry it + only switches the render mode. Either way `newRenderMode` ends as "component" and every + other field variable is kept. Returns a verdict: ok (with `body`) or skipped (with `reason`).""" + manifest = read_manifest(workdir) + entry = find_entry(manifest, key) + if entry["kind"] == "asset": + raise InputError(f"{entry['key']} is an asset; field-body is for fields and render-mode switches") + field = read_json(field_file) + if not isinstance(field, dict) or not field.get("id"): + raise InputError(f"{field_file} must hold the field object as fetched (with its id)") + + if entry["kind"] == "field": + migrated = workdir / "migrated" / entry["file"] + original = workdir / "original" / entry["file"] + if not migrated.exists() or not original.exists(): + raise InputError(f"missing the original or migrated file for {entry['key']}") + if (field.get("values") or "") != read_text_verbatim(original): + return {"command": "field-body", "key": entry["key"], "verdict": "skipped", + "reason": "changed on the server since download"} + updated = with_component_render_mode(field) + updated["values"] = read_text_verbatim(migrated) + else: + if has_component_render_mode(field): + return {"command": "field-body", "key": entry["key"], "verdict": "skipped", + "reason": "already in component render mode"} + updated = with_component_render_mode(field) + return {"command": "field-body", "key": entry["key"], "verdict": "ok", "body": {"field": updated}} + + +# ─── check / payload / record / status ─────────────────────────────────── + + +def entry_matches_only(entry: dict, only: list[str]) -> bool: + if not only: + return True + return entry["key"] in only or entry.get("identifier") in only + + +def select_entries(manifest: dict, only: list[str]) -> list[dict]: + """Without --only: every entry that is not finished. With --only: the named entries, plus + the render-mode switches of any named asset (selecting an asset also switches the fields + that load it, in the same run).""" + named_assets = { + e["key"] for e in manifest["entries"] if e["kind"] == "asset" and only and entry_matches_only(e, only) + } + selected = [] + for entry in manifest["entries"]: + if entry_matches_only(entry, only): + if only or entry.get("status") not in TERMINAL_STATUSES: + selected.append(entry) + elif ( + entry["kind"] == "renderMode" + and set(entry.get("requires") or []) & named_assets + and entry.get("status") not in TERMINAL_STATUSES + ): + selected.append(entry) + return selected + + +def gate_file_entry(workdir: Path, entry: dict) -> tuple[str, str | None]: + """The safety gates for one asset/field entry: (verdict, reason).""" + migrated_file = workdir / "migrated" / entry["file"] + original_file = workdir / "original" / entry["file"] + if not migrated_file.exists(): + return "waiting", "no migrated file yet" + if not original_file.exists(): + raise InputError(f"missing {original_file}: re-run classify and build-manifest") + migrated, original = migrated_file.read_bytes(), original_file.read_bytes() + if migrated == original: + return "skipped", "migrated file is identical to original" + problem = inline_problem(original, migrated) + if problem: + return "skipped", problem + return "ready", None + + +def action_for(entry: dict) -> dict: + if entry["kind"] == "asset": + return { + "type": "publish-asset", + "identifier": entry["identifier"], + "expectedInode": entry["inode"], + "contentType": entry["contentType"], + "binaryField": entry["binaryField"], + "languageId": entry["languageId"], + "fileName": entry["fileName"], + } + if entry["kind"] == "field": + return {"type": "update-field", "typeId": entry["typeId"], "fieldId": entry["fieldId"]} + return {"type": "set-render-mode", "typeId": entry["typeId"], "fieldId": entry["fieldId"]} + + +def cmd_check(workdir: Path, only: list[str], dry_run: bool, retry: bool) -> dict: + """Runs the gates on the selected entries and reports a verdict per entry: + ready safe to publish now; `action` says what to do + skipped a gate failed; recorded in the manifest as skipped unless --dry-run + waiting no migrated file yet, or a /dA/ asset it depends on is not published yet; + stays pending + done already finished (published/skipped/failed); named again only with --retry + Read-only apart from recording `skipped` outcomes.""" + manifest = read_manifest(workdir) + statuses = {e["key"]: e.get("status") for e in manifest["entries"]} + results = [] + run_verdicts: dict[str, str] = {} + + for entry in select_entries(manifest, only): + if entry.get("status") in TERMINAL_STATUSES and not retry: + verdict, reason = "done", f"already {entry['status']}" + elif entry["kind"] == "renderMode": + unmet = [ + a + for a in entry.get("requires") or [] + if statuses.get(a) != "published" and not (dry_run and run_verdicts.get(a) == "ready") + ] + verdict, reason = ("waiting", f"{', '.join(unmet)} not published yet") if unmet else ("ready", None) + else: + verdict, reason = gate_file_entry(workdir, entry) + run_verdicts[entry["key"]] = verdict + + result = {"key": entry["key"], "label": entry["label"], "kind": entry["kind"], "verdict": verdict} + if reason: + result["reason"] = reason + if verdict == "ready": + result["action"] = action_for(entry) + results.append(result) + + if verdict == "skipped" and not dry_run: + record_status(workdir, manifest, entry["key"], "skipped", reason) + + counts: dict[str, int] = {} + for r in results: + counts[r["verdict"]] = counts.get(r["verdict"], 0) + 1 + out = {"command": "check", "dryRun": dry_run, "counts": counts, "entries": results} + if only and not results: + out["warning"] = f"--only {only} matched none of the {len(manifest['entries'])} manifest entries" + return out + + +def find_entry(manifest: dict, key: str) -> dict: + entry = next((e for e in manifest["entries"] if entry_matches_only(e, [key])), None) + if not entry: + raise InputError(f"no manifest entry with key {key!r}") + return entry + + +def cmd_payload(workdir: Path, key: str) -> dict: + """What the agent needs to publish one entry. For an asset: the migrated file as base64 + (small VTL files). A field or render-mode entry needs only its ids: use `field-body`.""" + manifest = read_manifest(workdir) + entry = find_entry(manifest, key) + out = {"command": "payload", "key": entry["key"], "kind": entry["kind"], **action_for(entry)} + if entry["kind"] == "renderMode": + return out + migrated = workdir / "migrated" / entry["file"] + original = workdir / "original" / entry["file"] + if not migrated.exists(): + raise InputError(f"no migrated file at {migrated}") + if entry["kind"] == "asset": + out["contentBase64"] = base64.b64encode(migrated.read_bytes()).decode("ascii") + return out + + +def record_status(workdir: Path, manifest: dict, key: str, status: str, reason: str | None) -> None: + """Updates one entry and immediately persists the whole manifest, so an interrupted batch + still leaves an accurate record of what happened to each entry attempted so far.""" + if status not in MANIFEST_STATUSES: + raise InputError(f"unknown status {status!r}") + for entry in manifest["entries"]: + if entry["key"] == key: + entry["status"] = status + entry["statusReason"] = reason + break + write_json(manifest_path(workdir), manifest) + + +def cmd_record(workdir: Path, key: str, status: str, reason: str | None, inode: str | None) -> dict: + """Persists the outcome of one write. For a published asset, pass the new inode from the + publish response: it is what a later run compares against to skip re-downloading.""" + if status not in MANIFEST_STATUSES: + raise InputError(f"status must be one of {sorted(MANIFEST_STATUSES)}") + manifest = read_manifest(workdir) + entry = find_entry(manifest, key) + if status == "published" and entry["kind"] == "asset": + if not inode: + raise InputError("recording a published asset needs --inode (the new inode from the publish response)") + entry["previousInode"] = entry["inode"] + entry["inode"] = inode + record_status(workdir, manifest, entry["key"], status, reason) + return {"command": "record", "key": entry["key"], "status": status} + + +def cmd_status(workdir: Path) -> dict: + manifest = read_manifest(workdir) + counts: dict[str, int] = {} + for e in manifest["entries"]: + counts[e["status"]] = counts.get(e["status"], 0) + 1 + entries = [ + {k: e.get(k) for k in ("kind", "key", "label", "usedBy", "file", "requires", "status", "statusReason")} + for e in manifest["entries"] + ] + return {"command": "status", "counts": counts, "entries": entries} + + +# ─── cli ───────────────────────────────────────────────────────────────── + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--workdir", type=Path, default=Path("vtl-migration"), help="default: %(default)s") + sub = parser.add_subparsers(dest="command", required=True) + + sub.add_parser("classify", help="classify the custom fields in scan/*.json") + sub.add_parser("assets-plan", help="decide which /dA/ files to download") + build = sub.add_parser("build-manifest", help="build manifest.json from the downloaded files") + build.add_argument("--instance", help="host of the instance, recorded in the manifest") + sub.add_parser("status", help="list the manifest entries") + check = sub.add_parser("check", help="run the safety gates on migrated files") + check.add_argument("--only", nargs="+", default=[], metavar="KEY", help="dA ids, identifiers or Type.field") + check.add_argument("--dry-run", action="store_true", help="do not record skipped entries") + check.add_argument("--retry", action="store_true", help="re-check entries that already finished") + body = sub.add_parser("field-body", help="build the PUT body for a field or render-mode switch") + body.add_argument("key") + body.add_argument("--field-file", required=True, type=Path, help="the field as just fetched, as JSON") + payload = sub.add_parser("payload", help="print what is needed to publish one entry") + payload.add_argument("key") + record = sub.add_parser("record", help="persist the outcome of one write") + record.add_argument("key") + record.add_argument("--status", required=True, choices=sorted(MANIFEST_STATUSES)) + record.add_argument("--reason") + record.add_argument("--inode", help="new inode of a published asset") + args = parser.parse_args(argv) + + try: + if args.command == "classify": + result = cmd_classify(args.workdir) + elif args.command == "assets-plan": + result = cmd_assets_plan(args.workdir) + elif args.command == "build-manifest": + result = cmd_build_manifest(args.workdir, args.instance) + elif args.command == "status": + result = cmd_status(args.workdir) + elif args.command == "check": + result = cmd_check(args.workdir, args.only, args.dry_run, args.retry) + elif args.command == "field-body": + result = cmd_field_body(args.workdir, args.key, args.field_file) + elif args.command == "payload": + result = cmd_payload(args.workdir, args.key) + else: + result = cmd_record(args.workdir, args.key, args.status, args.reason, args.inode) + except InputError as e: + print(json.dumps({"error": str(e)}), file=sys.stderr) + return 2 + print(json.dumps(result)) + return 0 + + +if __name__ == "__main__": + sys.exit(main())