diff --git a/README.md b/README.md index 33afb3a..c712935 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ ## Plugin Overview - [UI5](./plugins/ui5/README.md) +- [UI5 Modernization](./plugins/ui5-modernization/README.md) - [UI5 TypeScript Conversion](./plugins/ui5-typescript-conversion/README.md) ## Further Information diff --git a/knip.config.js b/knip.config.js new file mode 100644 index 0000000..1ba2bd7 --- /dev/null +++ b/knip.config.js @@ -0,0 +1,19 @@ +const config = { + /** + * We only need dependency checking at the moment, + * so all checks except for dependencies are turned off. + */ + rules: { + files: "off", + duplicates: "off", + unlisted: "off", + binaries: "off", + unresolved: "off", + catalog: "off", + exports: "off", + types: "off", + enumMembers: "off", + } +}; + +export default config; diff --git a/plugins/ui5-modernization/.github/plugin/plugin.json b/plugins/ui5-modernization/.github/plugin/plugin.json new file mode 100644 index 0000000..1fff9a5 --- /dev/null +++ b/plugins/ui5-modernization/.github/plugin/plugin.json @@ -0,0 +1,22 @@ +{ + "name": "ui5-modernization", + "version": "0.1.4", + "description": "Complete UI5 modernization toolkit with workflow and specialized fix patterns for modernizing SAPUI5/OpenUI5 applications", + "author": { + "name": "SAP SE" + }, + "homepage": "https://github.com/UI5/plugins-coding-agents", + "repository": "https://github.com/UI5/plugins-coding-agents", + "keywords": [ + "ui5", + "sapui5", + "openui5", + "modernization", + "linter", + "upgrade" + ], + "license": "Apache-2.0", + "skills": [ + "./skills/" + ] +} diff --git a/plugins/ui5-modernization/.mcp.json b/plugins/ui5-modernization/.mcp.json new file mode 100644 index 0000000..fe0a866 --- /dev/null +++ b/plugins/ui5-modernization/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "chrome-devtools": { + "command": "npx", + "args": ["-y", "chrome-devtools-mcp@latest"] + } + } +} diff --git a/plugins/ui5-modernization/README.md b/plugins/ui5-modernization/README.md new file mode 100644 index 0000000..53abc85 --- /dev/null +++ b/plugins/ui5-modernization/README.md @@ -0,0 +1,203 @@ +# UI5 Modernization Plugin + +A comprehensive plugin providing a complete toolkit for modernizing SAPUI5/OpenUI5 applications. + +## Overview + +This plugin provides: + +- **Autonomous modernization workflow** with end-to-end orchestration in five phases +- **Specialized fix skills** for every UI5 linter rule category +- **Verification gates** at every phase boundary (full autonomous, half autonomous, or manual) +- **Validation** at every step via UI5 linter integration + +## Installation + +### Via Claude CLI + +```bash +claude plugin install ui5-modernization@claude-plugins-official +``` + +### In Claude Code + +```bash +/plugin install ui5-modernization@claude-plugins-official +``` + +## How It Works + +The goal is to modernize your OpenUI5/SAPUI5 app — targeting manifest version 2.0.0 with a minimum framework version of 1.136.0. This means replacing deprecated APIs with their modern equivalents and enforcing strict module imports, eliminating reliance on globals and legacy patterns. + +This plugin is built around the [UI5 linter](https://github.com/UI5/linter) (`@ui5/linter`) — a static analysis tool that detects deprecated APIs, global namespace access, and other incompatibilities. The linter serves two roles in the modernization workflow: + +1. **Detection** — Each skill reads linter output to identify what needs fixing. The linter's rule IDs (e.g., `no-deprecated-api`, `no-globals`) map directly to specific fix skills. +2. **Verification** — After applying fixes, the linter is re-run to confirm errors are resolved. Zero remaining errors = phase complete. + +A few issues cannot be detected by the linter (runtime-only patterns, cyclic dependencies). For these, the plugin includes its own detection scripts that fill the gap. + +## Quick Start + +### 1. Start With the Modernization Workflow + +The main entry point is the orchestrator which runs all five phases: + +``` +/modernize-ui5-app +``` + +This will: +1. Ask which verification mode you want (full autonomous / half autonomous / manual) +2. Run Phase 1: UI5 linter autofix + test starter restructure +3. Run Phase 2: manifest.json + Component.js foundation +4. Run Phase 3: module system (globals, pseudo modules, cyclic dependencies, blind spots) +5. Run Phase 4: deprecated API replacements +6. Run Phase 5: CSP compliance +7. Generate `MODERNIZATION-REPORT.md` and `MODERNIZATION-ISSUES.md` + +Each phase creates a git commit and runs the verification gate per your chosen mode. + +### 2. Use Specialized Skills for Specific Issues + +When you encounter specific error patterns, use the targeted skills directly: + +``` +# Test infrastructure (Phase 1) +/modernize-test-starter # Test Starter modernization + +# Foundation (Phase 2) +/fix-component-async # Component.js async issues +/fix-manifest-json # manifest.json issues + +# Module system issues (Phase 3) +/fix-js-globals # no-globals errors in JS files +/fix-xml-globals # no-globals errors in XML views/fragments +/fix-pseudo-modules # no-pseudo-modules, no-implicit-globals +/fix-linter-blind-spots # Runtime patterns linter misses +/fix-cyclic-deps # Circular sap.ui.define dependencies + +# Deprecated API modernizations (Phase 4) +/fix-bootstrap-params # HTML bootstrap parameters +/fix-library-init # Library.init() apiVersion +/fix-control-renderer # Control renderer issues +/fix-deprecated-controls # Deprecated controls, classes, interfaces +/fix-fiori-elements-extensions # Fiori Elements V2 controller extensions +/fix-partially-deprecated-apis # Partially deprecated API calls +/fix-table-row-mode # Deprecated Table row properties +/fix-xml-native-html # Native HTML/SVG in XML views + +# CSP compliance (Phase 5) +/fix-csp-compliance # Unsafe inline scripts + +# FLP sandbox +/modernize-flp-sandbox # Dedicated FLP Sandbox modernization skill +``` + +## What Gets Changed + +The modernization workflow modifies your project in predictable ways: + +- **Files modified**: JS controllers, XML views/fragments, HTML test pages, `manifest.json`, `Component.js` +- **Files generated**: `MODERNIZATION-REPORT.md` (statistics), `MODERNIZATION-ISSUES.md` (unfixable errors) +- **Git commits**: One commit per phase (5–6 total) +- **Not touched**: `node_modules/`, `dist/`, build artifacts, or files outside the app source + +### Rollback + +Start with a clean working directory. Each phase is a separate git commit, so you can undo any phase: + +```bash +git revert HEAD # undo the last phase +git reset --hard HEAD~3 # undo the last 3 phases +``` + +## Skills Included + +### Orchestrator + +- **`/modernize-ui5-app`** - End-to-end workflow: runs five phases with verification gates, delegates to specialized skills via sub-agents, and generates a modernization report + +### Phase 1: Mechanical Baseline + +- **`/modernize-test-starter`** - Modernize QUnit unit tests and OPA5 integration tests to the Test Starter concept (handles both single-HTML + AllJourneys and many-individual-HTML patterns) + +### Phase 2: Foundation + +- **`/fix-manifest-json`** - Fix manifest.json issues: outdated manifest version, legacy OpenUI5/SAPUI5 version, deprecated libraries/components, deprecated view/model types, removed properties +- **`/fix-component-async`** - Fix Component.js async configuration: `IAsyncContentCreation` interface, manifest declaration, redundant async flags + +### Phase 3: Module System & Globals + +- **`/fix-js-globals`** - Fix JavaScript `no-globals` errors: global namespace assignments, `sap.ui.core.Core` direct access, jQuery/$ global calls, controller factories, legacy module wrapping +- **`/fix-xml-globals`** - Fix XML view/fragment globals: global variable access, ambiguous event handlers, formatters, type references in bindings, factory functions, app-namespace globals +- **`/fix-pseudo-modules`** - Fix pseudo module and implicit global issues: deprecated enum/DataType pseudo module access, direct `library.EnumName` access, OData expression addons +- **`/fix-linter-blind-spots`** - Fix runtime-breaking patterns the linter does NOT report: app-namespace globals in JS files, QUnit 1.x assertions, sinon mocking via global chains +- **`/fix-cyclic-deps`** - Detect and resolve cyclic module dependencies introduced during modernization (2-node direct cycles auto-fixed, 3+ node chains flagged) + +### Phase 4: Deprecated APIs + +- **`/fix-bootstrap-params`** - Fix HTML bootstrap parameter issues: missing/deprecated parameters (`async`, `compat-version`, `animation`, `binding-syntax`), deprecated theme values, deprecated libraries +- **`/fix-library-init`** - Fix Library.init() apiVersion issues and modernize library initialization +- **`/fix-control-renderer`** - Fix Control renderer issues: missing renderer declaration, string-based renderer, implicit auto-discovery, `apiVersion:2` configuration +- **`/fix-deprecated-controls`** - Fix deprecated controls, classes, interfaces, and types with modern replacements (e.g., `sap.m.MessagePage` to `IllustratedMessage`) +- **`/fix-fiori-elements-extensions`** - Handle Fiori elements V2 controller extensions during modernization (manifest-based `sap.ui.controllerExtensions`) +- **`/fix-partially-deprecated-apis`** - Fix partially deprecated API usage: `Parameters.get`, `JSONModel.loadData`, `Mobile.init`, `ODataModel.v2.createEntry`, `View.create`, `Fragment.load`, `Router` constructor +- **`/fix-table-row-mode`** - Modernize deprecated Table row properties (`visibleRowCountMode`, `visibleRowCount`, `rowHeight`, etc.) to structured `rowMode` aggregation +- **`/fix-xml-native-html`** - Fix native HTML and SVG usage in XML views/fragments: replace `html:div`, `html:span`, `html:a` with UI5 controls + +### Phase 5: CSP Compliance + +- **`/fix-csp-compliance`** - Fix Content Security Policy compliance: extract unsafe inline scripts to external files + +### Other Skills + +#### FLP Sandbox Modernization + +- **`/modernize-flp-sandbox`** - Modernize legacy FLP sandbox HTML files to new FLP Sandbox format: converts inline `window["sap-ushell-config"]` to declarative JSON. Requires framework version >= 1.147. This is a dedicated skill not part of the `/modernize-ui5-app` workflow and needs to be triggered separately. + +## Verification Modes + +The orchestrator asks once at the start which verification mode to use at every phase boundary: + +| Mode | Behavior | +|------|----------| +| **Full autonomous** | Run build + tests → attempt fix on failure → escalate after 3 retries | +| **Half autonomous** | Run build + tests → report results → wait for user input | +| **Manual** | Print summary → wait for user to verify externally | + +## Prerequisites + +Before using this plugin, ensure you have: + +1. **Node.js Version v20.11.0, v22.0.0, or higher** +2. **OpenUI5/SAPUI5 application** with `ui5.yaml` in project root (required for linter) +3. **Git repository** with clean working directory +4. **Chrome DevTools MCP** (optional) — only needed for automated test verification in full/half autonomous modes + +## Chrome DevTools MCP Integration + +This plugin leverages the [Chrome DevTools MCP](https://www.npmjs.com/package/chrome-devtools-mcp) server for browser-based test verification and debugging. The MCP server is configured in `.mcp.json` and provides: + +- **Automated test execution** — Run QUnit and OPA5 tests in a real browser during full/half autonomous verification modes +- **Page inspection** — Take accessibility snapshots, capture screenshots, and read console output to verify OpenUI5/SAPUI5 app behavior after modernization + +The Chrome DevTools MCP is optional — it is only used when running in full or half autonomous verification mode. The manual mode does not require it. + +## Troubleshooting + +```bash +# Get fix guidance for all errors +npx @ui5/linter --details + +# Get fix guidance for specific file +npx @ui5/linter --details path/to/file.js +``` + +## Resources + +- [Modernization Guide for OpenUI5](https://sdk.openui5.org/#/topic/db492368adbe490fa5d4ec7ebd98b187?q=modern) +- [Modernization Guide for SAPUI5](https://ui5.sap.com/#/topic/db492368adbe490fa5d4ec7ebd98b187) +- [Deprecated Core API for OpenUI5](https://sdk.openui5.org/#/topic/798dd9abcae24c8194922615191ab3f5?q=Deprecated) +- [Deprecated Core API for SAPUI5](https://ui5.sap.com/#/topic/798dd9abcae24c8194922615191ab3f5?q=Deprecated%20Core) +- [UI5 MCP Server](https://github.com/UI5/mcp-server) — query deprecated APIs, find modern replacements, get code examples +- [Commit history](https://github.com/UI5/plugins-coding-agents/commits/main) — changelog diff --git a/plugins/ui5-modernization/plugin.json b/plugins/ui5-modernization/plugin.json new file mode 100644 index 0000000..1fff9a5 --- /dev/null +++ b/plugins/ui5-modernization/plugin.json @@ -0,0 +1,22 @@ +{ + "name": "ui5-modernization", + "version": "0.1.4", + "description": "Complete UI5 modernization toolkit with workflow and specialized fix patterns for modernizing SAPUI5/OpenUI5 applications", + "author": { + "name": "SAP SE" + }, + "homepage": "https://github.com/UI5/plugins-coding-agents", + "repository": "https://github.com/UI5/plugins-coding-agents", + "keywords": [ + "ui5", + "sapui5", + "openui5", + "modernization", + "linter", + "upgrade" + ], + "license": "Apache-2.0", + "skills": [ + "./skills/" + ] +} diff --git a/plugins/ui5-modernization/skills/fix-bootstrap-params/SKILL.md b/plugins/ui5-modernization/skills/fix-bootstrap-params/SKILL.md new file mode 100644 index 0000000..b55876d --- /dev/null +++ b/plugins/ui5-modernization/skills/fix-bootstrap-params/SKILL.md @@ -0,0 +1,177 @@ +--- +name: fix-bootstrap-params +description: | + Fix HTML bootstrap parameter issues that UI5 linter reports but cannot auto-fix. Use this skill when linter outputs these rules: + - `no-deprecated-api` - For missing/deprecated bootstrap parameters (async, compat-version, animation, binding-syntax, etc.) + - `no-deprecated-theme` - For deprecated theme values in data-sap-ui-theme + - `no-deprecated-library` - For deprecated libraries in data-sap-ui-libs + Trigger on error messages containing: "Missing bootstrap parameter", "Abandoned bootstrap parameter", "Redundant bootstrap parameter", "deprecated value", "bootstrap" + Automatically applies safe fixes to HTML files containing UI5 bootstrap script tags. +--- + +# Fix Bootstrap Parameters + +## Key Rules + +1. **ONLY modify the bootstrap `` before or after the bootstrap tag, leave it untouched. + +This skill fixes HTML bootstrap parameter issues that the UI5 linter detects but cannot auto-fix because the changes may affect application behavior. + +## Linter Rules Handled + +| Rule ID | Message Pattern | This Skill's Action | +|---------|-----------------|---------------------| +| `no-deprecated-api` | Missing bootstrap parameter 'data-sap-ui-async' | Add `data-sap-ui-async="true"` | +| `no-deprecated-api` | Missing bootstrap parameter 'data-sap-ui-compat-version' | Add `data-sap-ui-compat-version="edge"` | +| `no-deprecated-api` | Use of deprecated value 'false' for bootstrap parameter 'data-sap-ui-async' | Change to `"true"` | +| `no-deprecated-api` | Use of deprecated value '...' for bootstrap parameter 'data-sap-ui-compat-version' | Change to `"edge"` | +| `no-deprecated-api` | Abandoned bootstrap parameter '...' should be removed | Remove the parameter | +| `no-deprecated-api` | Redundant bootstrap parameter '...' should be removed | Remove the parameter | +| `no-deprecated-api` | Bootstrap parameter '...' should be replaced with '...' | Replace with new parameter | +| `no-deprecated-theme` | Use of deprecated theme '...' | Change to `sap_horizon` | +| `no-deprecated-library` | Use of deprecated library '...' | Remove from libs | + +## When to Use + +Apply this skill when you see linter output like: +``` +index.html:8:3 error Missing bootstrap parameter 'data-sap-ui-async' no-deprecated-api +index.html:9:3 error Missing bootstrap parameter 'data-sap-ui-compat-version' no-deprecated-api +index.html:12:3 error Abandoned bootstrap parameter 'data-sap-ui-no-duplicate-ids' should be removed no-deprecated-api +index.html:15:3 error Use of deprecated theme 'sap_bluecrystal' no-deprecated-theme +``` + +## Fix Strategy + +### 1. Locate the Bootstrap Script Tag + +Find the ` + + + +``` + +#### `no-deprecated-api` - Deprecated Values + +**`data-sap-ui-async="false"`**: Change to `"true"` +**`data-sap-ui-compat-version` with non-edge value**: Change to `"edge"` + +#### `no-deprecated-api` - Abandoned Parameters (remove completely) + +Remove these parameters entirely: +- `data-sap-ui-no-duplicate-ids` - Enforced in modern UI5 +- `data-sap-ui-auto-aria-body-role` - Removed in modern UI5 +- `data-sap-ui-manifest-first` - Use Component.create manifest option instead +- `data-sap-ui-origin-info` - No longer supported +- `data-sap-ui-areas` - Use Control.placeAt instead +- `data-sap-ui-trace` - No longer supported +- `data-sap-ui-xx-no-less` - No longer supported + +#### `no-deprecated-api` - Redundant Parameters + +- `data-sap-ui-binding-syntax="simple"` - Remove; complex syntax is enforced in modern UI5 +- `data-sap-ui-binding-syntax="complex"` - Remove if `compat-version="edge"` is set +- `data-sap-ui-preload` with invalid values - Remove + +#### `no-deprecated-api` - Replaced Parameters + +- `data-sap-ui-animation` → `data-sap-ui-animation-mode` (convert `true`→`full`, `false`→`minimal`) + +#### `no-deprecated-theme` - Deprecated Themes + +Replace with modern theme: +- `sap_bluecrystal` → `sap_horizon` +- `sap_belize` → `sap_horizon` +- `sap_belize_plus` → `sap_horizon` +- `sap_belize_hcb` → `sap_horizon_hcb` +- `sap_belize_hcw` → `sap_horizon_hcw` +- `sap_hcb` → `sap_horizon_hcb` +- `sap_ux` → `sap_horizon` +- `sap_platinum` → `sap_horizon` +- `sap_goldreflection` → `sap_horizon` + +#### `no-deprecated-library` - Deprecated Libraries + +Remove deprecated libraries from `data-sap-ui-libs`: +- `sap.ui.commons` +- `sap.ui.ux3` +- `sap.makit` +- `sap.me` +- `sap.ca.ui` +- `sap.sac.grid` +- `sap.ui.suite` +- `sap.zen.commons` +- `sap.zen.crosstab` +- `sap.zen.dsh` + +## Implementation Steps + +1. Read the HTML file +2. Parse to find the bootstrap script tag +3. For each linter error (identified by rule ID and message): + - If missing parameter: Add the parameter with recommended value + - If deprecated value: Update to recommended value + - If abandoned/redundant: Remove the parameter + - If deprecated theme: Replace with modern theme + - If deprecated library: Remove from libs list +4. Preserve formatting (indentation, line breaks) as much as possible +5. Write the updated file + +## Example Fix + +Given linter output: +``` +index.html:8:3 error Missing bootstrap parameter 'data-sap-ui-async' no-deprecated-api +index.html:8:3 error Missing bootstrap parameter 'data-sap-ui-compat-version' no-deprecated-api +index.html:12:3 error Abandoned bootstrap parameter 'data-sap-ui-no-duplicate-ids' should be removed no-deprecated-api +index.html:10:3 error Use of deprecated theme 'sap_bluecrystal' no-deprecated-theme +``` + +Transform: +```html + + + + + +``` + +## Notes + +- Always add `data-sap-ui-async="true"` before other data attributes for consistency +- Setting `compat-version="edge"` enables complex binding syntax automatically, so `binding-syntax` becomes redundant +- Removed parameters should not leave trailing whitespace or empty lines +- If the file has multiple script tags, only modify the bootstrap tag — leave all others intact (including inline `` with ``. Even trivial config objects, debug flags, or seemingly unused code must be externalized — removal is a functional regression. +2. File naming: use a descriptive name matching the content's purpose (e.g., `appConfig.js` for configuration, `init.js` for initialization). + +This skill fixes Content Security Policy (CSP) compliance issues that the UI5 linter detects but cannot auto-fix because they require restructuring code into external files. + +## Linter Rule Handled + +| Rule ID | Message Pattern | Severity | This Skill's Action | +|---------|-----------------|----------|---------------------| +| `csp-unsafe-inline-script` | Use of unsafe inline script | Warning | Move to external JS file | + +## When to Use + +Apply this skill when you see linter output like: +``` +index.html:15:5 warning Use of unsafe inline script csp-unsafe-inline-script +test.html:20:5 warning Use of unsafe inline script csp-unsafe-inline-script +``` + +## Background: Why CSP Matters + +Content Security Policy (CSP) is a security feature that helps prevent: +- Cross-Site Scripting (XSS) attacks +- Data injection attacks +- Unauthorized script execution + +Inline scripts are considered unsafe because an attacker who manages to inject HTML can also inject malicious JavaScript. CSP-compliant apps use `script-src 'self'` which blocks inline scripts. + +Documentation: [Content Security Policy](https://ui5.sap.com/#/topic/fe1a6dba940e479fb7c3bc753f92b28c) + +## Detection + +The linter flags ` + + + + +``` + +**Not flagged:** +```html + + + + + +``` + +## Fix Strategy + +The fix for every inline script is the same: **move it to an external file and add a ` + + + + + + +``` + +**Fix Strategy**: Move inline scripts to external files. + +```html + + + + + + My App + + + + + + +``` + +```javascript +// config.js +window.myConfig = { + apiUrl: "/api/v1", + debug: true +}; +``` + +```javascript +// my/app/init.js +sap.ui.define([], function() { + "use strict"; + + return { + start: function() { + // Initialization code + } + }; +}); +``` + +### 2. UI5 Bootstrap with Inline Init → data-sap-ui-on-init + +**Problem**: Inline script after UI5 bootstrap. + +```html + + + +``` + +**Fix Strategy**: Use `data-sap-ui-on-init` attribute. + +```html + + +``` + +```javascript +// webapp/init.js +sap.ui.define([ + "sap/m/Shell", + "sap/ui/core/ComponentContainer" +], function(Shell, ComponentContainer) { + "use strict"; + + new Shell({ + app: new ComponentContainer({ + name: "my.app", + async: true + }) + }).placeAt("content"); +}); +``` + +### 3. Configuration Data → JSON or Module + +**Problem**: Inline configuration object. + +```html + + +``` + +**Fix Strategy A**: External JSON file loaded at runtime. + +```html + + +``` + +```javascript +// config.js - loads JSON +(function() { + var xhr = new XMLHttpRequest(); + xhr.open("GET", "config.json", false); // Sync for config + xhr.send(); + window.APP_CONFIG = JSON.parse(xhr.responseText); +})(); +``` + +```json +// config.json +{ + "apiEndpoint": "https://api.example.com", + "features": { + "darkMode": true, + "analytics": false + } +} +``` + +**Fix Strategy B**: UI5 module with configuration. + +```javascript +// my/app/config.js +sap.ui.define([], function() { + "use strict"; + + return { + apiEndpoint: "https://api.example.com", + features: { + darkMode: true, + analytics: false + } + }; +}); + +// Usage in other modules +sap.ui.define(["my/app/config"], function(config) { + console.log(config.apiEndpoint); +}); +``` + +### 4. Inline Event Handlers → External Scripts + +**Problem**: Inline event handlers in HTML attributes. + +```html + + + + +``` + +**Fix Strategy**: Use external script with event listeners. + +```html + + + + +``` + +```javascript +// app.js +document.addEventListener("DOMContentLoaded", function() { + document.getElementById("myButton").addEventListener("click", handleClick); + document.getElementById("logo").addEventListener("error", function() { + handleError(this); + }); + init(); +}); + +function handleClick() { + // Click handler +} + +function handleError(element) { + // Error handler +} + +function init() { + // Initialization +} +``` + +### 5. Test HTML Files + +**Problem**: QUnit test files with inline scripts. + +```html + + + + + + + + +
+ + +``` + +**Fix Strategy**: Use Test Starter (also fixes `prefer-test-starter`). + +```html + + + + + + + +
+ + +``` + +### 6. Dynamic Script Content + +**Problem**: Script content generated dynamically. + +```html + + +``` + +**Fix Strategy**: Use data attributes or meta tags. + +```html + + + + +``` + +```javascript +// app.js +var userId = document.querySelector('meta[name="user-id"]').content; +var token = document.querySelector('meta[name="csrf-token"]').content; +``` + +## Implementation Steps + +1. **Identify all inline scripts** from linter output + +2. **Categorize each script**: + - UI5 initialization → Use `data-sap-ui-on-init` + - Configuration → External JS file + - Event handlers → External script with `addEventListener` + - Test boilerplate → Use Test Starter + +3. **Create external files** for the script content + +4. **Update HTML** to reference external files + +5. **Test the application** to ensure functionality is preserved + +## Common Patterns + +| Inline Pattern | CSP-Compliant Solution | +|----------------|------------------------| +| `` | ` +``` + +The path must be `../resources/` (with `../`) — the HTML file is in +`webapp/test/`, so resources are one level up. Never write +`resources/` without the `../` prefix. + +**CDN variant.** If the sandbox script being removed (`sandbox.js` or +`sandbox2.js`) was loaded from an absolute CDN URL (e.g. +`src="https://ui5.sap.com/resources/sap/ushell/bootstrap/sandbox2.js"`), +detect the CDN origin (everything up to `/resources/`) and use it as +the prefix for `SandboxBootTask.js`: +```html + +``` +The `sap-ui-core.js` src will already use the CDN origin — carry it +over unchanged. Do not mix local `../resources/` and CDN +`https://...` paths in the same HTML. + +**Add to ` + + +
+
+ + +``` + +This single file replaces ALL individual test HTML files. Test Starter uses URL query parameters to select which test to run. + +## Phase 3: Update testsuite.qunit.html + +Replace the contents of `webapp/test/testsuite.qunit.html`: + +```html + + + + + QUnit test suite for <NAMESPACE-WITH-DOTS> + + + + + +``` + +This replaces `qunit-redirect.js` or `sap-ui-core.js` bootstrapping with `createSuite.js`. + +## Phase 4: Modernize Unit Test JS Files + +### 4.0 FIRST — Identify and delete redundant aggregators + +**Before converting any file**, scan `webapp/test/unit/` for redundant aggregators. A redundant aggregator is a JS file that: +- Uses `sap.ui.require([...], function() { QUnit.start(); })` to load other test modules, OR +- Uses `sap.ui.define([...])` to list dependencies with no test body, OR +- Contains NO actual test logic (`QUnit.module`, `QUnit.test`, `assert.*` calls) + +**IMPORTANT**: `QUnit.config.autostart = false` and `QUnit.start()` are NOT test logic — they are boot scaffolding. A file that ONLY does `sap.ui.require([deps], function() { QUnit.start(); })` is a redundant aggregator, even though it mentions QUnit. + +Common filenames: `allTests.js`, `AllTests.js`, `legacyTests.qunit.js`, `allTests.qunit.js` — but ANY file matching this "load-only, no tests" pattern is a redundant aggregator. + +**Action**: Note the test modules they load (these will go into `unitTests.qunit.js`), then **DELETE** the aggregator file immediately. Do NOT convert it to `sap.ui.define` format. Do NOT add QUnit.test stubs. Do NOT keep it as a test entry. Also delete its companion HTML file (e.g., `legacyTests.qunit.html`). DELETE BOTH FILES. + +Example — this is a redundant aggregator (DELETE both .js and .html): +```javascript +QUnit.config.autostart = false; +sap.ui.require([ + "my/app/test/unit/controller/App.controller" +], function() { + "use strict"; + QUnit.start(); +}); +``` +It has no `QUnit.module`/`QUnit.test` of its own — it just loads another module and starts QUnit. Delete it. + +### 4.1 Convert and rename real test files + +Unit test JS files that **contain actual test logic** (`QUnit.module`, `QUnit.test`, `assert.*`) and use `Core.ready()`, `Core.attachInit()`, or `sap.ui.require` with `QUnit.start()` need TWO changes: + +1. **Rename the file** to add `.qunit.js` suffix (e.g., `App.controller.js` → `App.controller.qunit.js`) +2. **Convert the content** to `sap.ui.define` format (remove QUnit.config.autostart, Core.ready wrappers) + +**⚠️ CRITICAL — File Rename**: Every real unit test file MUST be renamed to `.qunit.js` suffix. This is required because Test Starter resolves test entries by appending `.qunit` to the module path. Without the rename, the test cannot be found at runtime. + +Examples: +- `controller/App.controller.js` → `controller/App.controller.qunit.js` +- `model/formatter.js` → `model/formatter.qunit.js` +- `util/Helper.js` → `util/Helper.qunit.js` + +**Before** — old style with Core.ready (`webapp/test/unit/controller/App.controller.js`): +```javascript +QUnit.config.autostart = false; +sap.ui.getCore().attachInit(function() { + "use strict"; + sap.ui.require([ + "my/app/model/formatter" + ], function(formatter) { + QUnit.module("formatter"); + QUnit.test("formatValue", function(assert) { + assert.equal(formatter.formatValue(1), "One"); + }); + }); +}); +``` + +**After** — Test Starter style (`webapp/test/unit/controller/App.controller.qunit.js`): +```javascript +sap.ui.define([ + "my/app/model/formatter" +], function(formatter) { + "use strict"; + + QUnit.module("formatter"); + QUnit.test("formatValue", function(assert) { + assert.equal(formatter.formatValue(1), "One"); + }); +}); +``` + +### 4.2 Create unitTests.qunit.js aggregator + +The main testsuite entry `"unit/unitTests"` resolves to `unit/unitTests.qunit.js`. This file must directly list all **real** unit test modules (files with QUnit.module/QUnit.test). + +Build the list from: +- Test modules extracted from deleted aggregators (Step 4.0) +- Any additional `.qunit.js` files in `webapp/test/unit/` that contain actual tests + +Do NOT include deleted aggregator files in this list. + +**After** (`unitTests.qunit.js` — directly lists all tests): +```javascript +sap.ui.define([ + "./controller/Main.qunit", + "./model/formatter.qunit" +]); +``` + +Key rules for the aggregator: +- Use **relative paths** starting with `./`, not absolute namespace paths +- Add the **`.qunit` suffix** to each dependency (without `.js`) because the actual files were renamed to `.qunit.js` in Step 4.1 +- Example: if file was renamed to `controller/App.controller.qunit.js`, reference it as `"./controller/App.controller.qunit"` + +### jsUnitTestSuite conversion + +If the old `testsuite.qunit.js` used `jsUnitTestSuite`, it's already replaced in Phase 1. Delete the old content. + +## Phase 5: Modernize OPA Tests + +This phase differs based on the detected pattern. Read the full instructions in the corresponding reference file. + +### Pattern A — Single HTML + AllJourneys + +Read `references/pattern-a-modernization.md` for detailed instructions. + +Summary: +1. **Create OpaSetup.js** from AllJourneys.js — extract `Opa5.extendConfig` and all page object/utility imports. OpaSetup.js must NOT import `sap/ui/test/opaQunit` — that module belongs in each individual journey file. +2. **Rename journey files** to `.qunit.js` suffix so Test Starter can resolve them without a `module` override +3. **Update journey files** — add OpaSetup as a side-effect dependency using relative path `"./OpaSetup"` (same directory). Do NOT use `test-resources/` for same-directory imports. +4. **Handle autoWait overrides** — journeys needing `autoWait: false` get a per-journey `Opa5.extendConfig` override +5. **Preserve testLibs config** — Fiori Elements `testLibs` settings move to OpaSetup.js + +**Correct OpaSetup.js structure** (page objects use `test-resources/`, but opaQunit is absent): +```javascript +sap.ui.define([ + "sap/ui/test/Opa5", + "test-resources//integration/pages/App" +], function(Opa5) { + "use strict"; + + Opa5.extendConfig({ + viewNamespace: ".view.", + autoWait: true + }); +}); +``` + +**Correct journey file structure** (opaQunit here, OpaSetup via relative path): +```javascript +sap.ui.define([ + "sap/ui/test/opaQunit", + "sap/ui/test/Opa5", + "./OpaSetup" +], function(opaTest, Opa5) { + "use strict"; + // ... opaTest(...) calls +}); +``` + +### Pattern B — Many Individual HTML Files + +Read `references/pattern-b-modernization.md` for detailed instructions. + +Summary: +1. **Inventory utility modules** — find all modules that call `Opa5.createPageObjects` (side-effect imports) +2. **Create OpaSetup.js** — consolidate all utility imports + `Opa5.extendConfig` from the HTML files +3. **Rename journey files** to `.qunit.js` suffix and add OpaSetup as a side-effect dependency +4. **Handle autoWait overrides** — use the parse script's `autoWaitFalseFiles` list +5. **Handle multi-module HTML files** — when a legacy `*.qunit.html` loads more than one journey module in a single `sap.ui.require`, emit ONE testsuite entry per module. Never invent a synthetic combined name (e.g. `Combined`) — the file does not exist and the resulting entry is dangling. The parse script does this automatically via `_fromMultiModuleHtml`. Halt if any of the loaded modules has no corresponding `.qunit.js` file under `webapp/test/`. See `references/pattern-b-modernization.md` Step 6. + + +## Phase 5b: Migrate in-window OPA launcher to bare-Component iframe + +**Run this phase only when Phase 0.2 reported `needsIframeMigration: true`** (i.e. `launcher === "in-window"` AND `flpSandbox === true`). Skip entirely for any other combination — including plain in-window apps with no FLP sandbox load, where `iStartMyUIComponent` should stay as-is. + +Phase 5b assumes Phase 5 has already produced `OpaSetup.js`, renamed journey files, and the main `testsuite.qunit.js` — it then rewrites the launcher path so journeys run inside a fresh same-origin iframe loading the Component directly (no FLP shell). + +Read `references/pattern-u-iframe-migration.md` for the full step-by-step instructions. Summary: + +1. **Create `webapp/test/integration/opaIframe.qunit.html` + `opaIframeBoot.js`** — bare-Component iframe entry. HTML loads `sap/ushell` sandbox.js for API stubs but defines no `sap-ushell-config`, so no FLP shell renderer is built. Bootstrap uses `data-sap-ui-oninit="module:/test/integration/opaIframeBoot"` (no inline ` + + + + +
+ + +``` + +`webapp/test/integration/opaIframeBoot.js`: + +```js +sap.ui.define([ + "sap/ui/core/ComponentContainer", + "/localService/mockserver" +], function (ComponentContainer, mockserver) { + "use strict"; + + mockserver.init(); + new ComponentContainer({ + name: "", + async: true, + manifest: true, + height: "100%", + settings: { id: "" } + }).placeAt("content"); +}); +``` + +Notes: +- `data-sap-ui-oninit="module:..."` runs the named module after core init — replaces the deprecated `sap.ui.getCore().attachInit(...)` pattern. +- The module is plain `.js` (no `.qunit.js` suffix) — it is loaded by URL attribute, not by a testsuite entry. +- Hyphenated attribute names (`compat-version`, `frame-options`) match the modern UI5 form; the older camelCase variants still work but the linter prefers hyphens. + +If `mockserver.init()` returns a Promise, replace the body of `opaIframeBoot.js` with: + +```js +sap.ui.define([ + "sap/ui/core/ComponentContainer", + "/localService/mockserver" +], function (ComponentContainer, mockserver) { + "use strict"; + + mockserver.init().then(function () { + new ComponentContainer({ + name: "", + async: true, + manifest: true, + height: "100%", + settings: { id: "" } + }).placeAt("content"); + }); +}); +``` + +## 5b.3 Rewrite `webapp/test/integration/arrangements/Common.js` + +Remove the parent-frame mockserver dependency and the in-window component start. Source iframe URL is the new `opaIframe.qunit.html`. Hash is the Component router pattern directly — **no `#app-tile&/` prefix**. + +```js +sap.ui.define([ + "sap/ui/test/Opa5" +], function (Opa5) { + "use strict"; + + function getFrameUrl(sHash) { + var sBaseUrl = sap.ui.require.toUrl("test-resources//integration/opaIframe") + ".qunit.html"; + var sExtraHash = sHash ? "#" + (sHash.indexOf("/") === 0 ? sHash : "/" + sHash) : ""; + return sBaseUrl + sExtraHash; + } + + return Opa5.extend(".test.integration.arrangements.Common", { + + iStartMyApp: function (oOptionsParameter) { + var oOptions = oOptionsParameter || {}; + + this.iStartMyAppInAFrame({ + source: getFrameUrl(oOptions.hash), + timeout: 200, + autoWait: oOptions.autoWait !== false + }); + } + + }); + +}); +``` + +Notes: +- Drop the `localService/mockserver` import and any `mockserver.init(...)` / `iWaitForPromise` calls — mockserver now boots inside the iframe. +- Drop the previous `componentConfig` object and `iStartMyUIComponent` call. +- **Drop any `_clearSharedData` helper that resets `ODataModel.mSharedData` in the parent frame.** That cache is a static map on the parent's `ODataModel` class; under the iframe setup the app's `ODataModel` lives in the iframe's UI5 Core, a different class object with its own `mSharedData`. The iframe is destroyed and recreated per journey, so its cache dies with it. The parent's `ODataModel.mSharedData` is unused — resetting it has no effect on the app. Remove the helper and the `sap/ui/model/odata/v2/ODataModel` dependency from `Common.js`. + +## 5b.4 Rewrite every journey file's launcher call + +Every file under `webapp/test/integration/` that starts the app: + +```diff +- Given.iStartMyUIComponent({ +- componentConfig: { +- name: "", +- async: true +- } +- }); ++ Given.iStartMyApp(); +``` + +If the original call passed a `hash` or `autoWait`, forward it as `Given.iStartMyApp({ hash: "...", autoWait: false })`. + +## 5b.5 Strip parent-frame mockserver init from `OpaSetup.js` + +If `OpaSetup.js` (or whatever aggregator the Phase 5 step produced) requires `localService/mockserver` and calls `mockserver.init()`, remove both the dependency and the call. Mockserver runs in the iframe. + +```diff + sap.ui.define([ + "sap/ui/test/Opa5", +- "./arrangements/Common", +- "/localService/mockserver" +- ], function (Opa5, Common, mockserver) { ++ "./arrangements/Common" ++ ], function (Opa5, Common) { + "use strict"; + + Opa5.extendConfig({ + arrangements: new Common(), + viewNamespace: ".view.", + autoWait: true + }); +- +- mockserver.init(); + }); +``` + +## 5b.6 Cross-window control instantiation in page objects + +Test code runs in the QUnit (parent) window. The app — including its UI5 Core — runs in the iframe. Constructing UI5 controls with the parent's UI5 (`new Token({...})` after `sap.ui.define([..., "sap/m/Token"], ..., Token)`) registers the control on the wrong Core; later lookups inside the iframe miss it. + +**Rule**: anywhere a page object instantiates a UI5 control to feed into an `EnterText`, `MultiInput.addToken`, or similar action, resolve the constructor through the iframe's loader: + +```diff +- sap.ui.define([ +- ..., +- "sap/m/Token" +- ], function (..., Token) { ++ sap.ui.define([ ++ ... ++ ], function (...) { + ... + success: function (oControl) { +- var oToken = new Token({ key: "0001", text: "0001" }); ++ var FrameToken = Opa5.getWindow().sap.ui.require("sap/m/Token"); ++ var oToken = new FrameToken({ key: "0001", text: "0001" }); + ... + } +``` + +Apply this transform mechanically: drop UI5 control / class dependencies that are only used inside `success` (or `check`, `matchers`) callbacks of `waitFor` from `sap.ui.define`, and replace the constructor / `instanceof` lookup at use-site with `Opa5.getWindow().sap.ui.require(...)`. + +The exact set of UI5 classes that must be cross-resolved is **project-specific** — do not enumerate it. Detection is done by the bundled script (see §5b.6.2), which scans every page-object file for the two usage shapes inside `waitFor` callbacks: + +- `new (...)` where `` resolves to a `sap.ui.define` dep param NOT on the OPA-safe allowlist +- ` instanceof ` — same rule + +For each finding, the fix is uniform: + +1. Drop the dependency from `sap.ui.define`. +2. Re-resolve at the call site via `Opa5.getWindow().sap.ui.require("")`. +3. Use the re-resolved reference for `new …(...)` or `… instanceof …`. + +Plain JS classes / utility modules that do not extend `sap.ui.base.ManagedObject` are fine to keep on the parent loader; the rule only applies to UI5 classes whose identity is tied to a specific Core. + +Do not rewrite controls used only as **type checks** in OPA matchers (`controlType: "sap.m.Token"`) — string-keyed control types do not need cross-window resolution. + +### 5b.6.1 Cross-window jQuery / DOM lookups + +The same parent-vs-iframe split applies to **jQuery** and raw `document` references inside `check` / `success` / `matchers` callbacks. The page object's `sap.ui.define` block runs in the parent (QUnit) window — `$`, `jQuery`, and `document` resolve to that window's globals. The app DOM (toasts, popovers, busy indicators, anything UI5 renders) lives in the **iframe**. A naked `$(".sapMMessageToast")` query against the parent always returns an empty set even while the toast is visible in the iframe — the assertion times out. + +Rule: any DOM / jQuery query inside a `waitFor` callback that targets app-rendered elements must go through `Opa5.getJQuery()` (preferred) or `Opa5.getWindow().document` / `Opa5.getWindow().jQuery`. + +```diff + iShouldSeeToast: function () { + return this.waitFor({ + check: function () { +- return $(".sapMMessageToast").length > 0; ++ return Opa5.getJQuery()(".sapMMessageToast").length > 0; + }, + autoWait: false, + success: function () { + Opa5.assert.ok(true, "toast successfully"); + } + }); + } +``` + +Detection is folded into the §5b.6.2 hard gate (`detect-cross-window-imports.js`) — same script flags bare `$(`, `jQuery(`, `document.`, `window.` lines that don't already route through `Opa5.getWindow()` / `Opa5.getJQuery()`. For each finding, replace: + +| Parent-window form | Iframe-aware form | +|---|---| +| `$(...)` / `jQuery(...)` | `Opa5.getJQuery()(...)` | +| `document.querySelector(...)` | `Opa5.getWindow().document.querySelector(...)` | +| `window.foo` (app global) | `Opa5.getWindow().foo` | + +Plain DOM queries that target the **OPA / QUnit harness itself** (e.g. asserting on the QUnit DOM) should stay on the parent window — the rule only applies to code reading or writing the application's DOM. + +### 5b.6.2 Mandatory page-object enumeration (cross-window misuse gate) + +The 5b.6 / 5b.6.1 rules are easy to apply per file the agent happens to be editing — and easy to *miss* on every other page object that didn't otherwise need touching. Under Pattern U this is silent: a parent-loaded control instantiates against the wrong UI5 Core, the iframe lookup misses it, and the failure surfaces only at runtime as a timeout in a single journey. + +To prevent that, do not rely on case-by-case judgement. Iterate **every** file under `webapp/test/integration/pages/` (or whichever directory holds the page objects) and apply the transform unconditionally to every UI5 control / class instantiation and every bare DOM access. + +#### Why the gate checks usage shape, not module paths + +UI5 ships many control libraries: `sap/m`, `sap/ui/core`, `sap/uxap`, `sap/suite`, `sap/viz`, `sap/ndc`, `sap/f`, `sap/ui/layout`, `sap/gantt`, plus per-project libs. Any whitelist by module-path prefix is incomplete the moment a project pulls in one more library. The gate instead detects the **misuse shape** at the call site: + +1. `new (...)` where `` is a `sap.ui.define` dependency parameter, and the dep path is NOT on the OPA-safe allowlist. +2. ` instanceof ` — same identifier rule. +3. Bare `$(`, `jQuery(`, `document.`, `window.` not routed through `Opa5.getJQuery()` / `Opa5.getWindow()`. + +This means the gate adapts automatically as new control libraries appear in the deps array — the question is "is this identifier a parent-loaded module that we are using as a Core-affinity object?", not "does this path start with one of N hard-coded prefixes?". + +#### OPA-safe modules (keep on parent loader) + +Two dep paths are explicitly safe in the parent `sap.ui.define`: + +- `sap/ui/test/*` — `Opa5`, `Press`, `EnterText`, matchers, the `Common` arrangement class. Designed to drive both windows; `new Press()` etc. is correct on the parent loader. +- `sap/ui/core/routing/History` — used for read-only inspection of parent-window history (`History.getInstance().getDirection()`). If a page object instantiates anything from `sap/ui/core/routing/*` other than reading `History`, treat it as forbidden and re-resolve through the iframe. + +Plain JS utility / formatter / constants modules under the app namespace are also fine; they don't extend `ManagedObject` and have no Core affinity. The gate only flags identifiers used as **constructors** or **instanceof RHS** — utility modules consumed only as namespaces (`MyUtils.format(...)`) are not flagged. + +#### Per-file procedure + +For every page-object file: + +1. **Drop the dep** from the `sap.ui.define` deps array AND the function signature once you've moved its constructor / `instanceof` use to the iframe loader. +2. **At each use site** inside a `waitFor` callback (`success`, `check`, `matchers`), re-resolve through the iframe: + ```js + var FrameToken = Opa5.getWindow().sap.ui.require("sap/m/Token"); + var oToken = new FrameToken({ key: "0001", text: "0001" }); + ``` + Same for `instanceof` checks: `oFoo instanceof Opa5.getWindow().sap.ui.require("sap/m/ColumnListItem")`. +3. **Hoist the lookup** if a class is used more than once. Resolve once into a local `var` at the top of the function (or as a member of the `Opa5.extend` config) and reuse. +4. **Type-check matchers stay as-is** — `controlType: "sap.m.Token"` is a string-keyed lookup that OPA itself routes to the right window. Do not touch those. +5. **Replace bare DOM access** — `$(...)` / `jQuery(...)` → `Opa5.getJQuery()(...)`; `document.querySelector(...)` → `Opa5.getWindow().document.querySelector(...)`; `window.foo` → `Opa5.getWindow().foo` (when targeting the app window). + +#### Hard gate (run before Phase 7) + +A Node script ships next to this reference. Run it as the last step of Phase 5b — before the verification phase — and treat a non-zero exit as a halt condition: + +```bash +node /scripts/detect-cross-window-imports.js +``` + +The script parses every `.js` file under `webapp/test/integration/pages/`, builds a map of `sap.ui.define` dep params, and emits one finding per: + +- `new (...)` whose `` is a non-allowlisted dep param. +- ` instanceof ` — same. +- Bare `$(`, `jQuery(`, `document.`, `window.` on lines that don't already route through `Opa5.getWindow()` / `Opa5.getJQuery()`. + +Output is JSON to stdout (machine-readable) and a human listing to stderr with `file:line: kind` and a per-finding fix suggestion. Exit `0` = clean; exit `1` = at least one finding. + +Do not skip the gate "because the journeys passed" — Pattern U has tests that pass in CI and still leak the wrong-Core control: the symptom is a flaky timeout on a different journey weeks later when someone adds an `instanceof` check. + +## 5b.7 Routing helpers — keep plain hash, no FLP nav + +In-window apps usually have a `Function.js` (or similar) page object with `iGoToPageByPath` / `iPageGoBack` helpers. They must use the plain Component-router hash. Do **not** add `#app-tile&/` prefixes, and do **not** call `sap.ushell.Container.setDirtyFlag(false)` — without the FLP renderer there is no ShellNavigation interceptor, no beforeunload confirm tied to the dirty flag. + +```js +iGoToPageByPath: function (sPath) { + return this.waitFor({ + success: function () { + var oWin = Opa5.getWindow(); + var sInner = sPath.indexOf("/") === 0 ? sPath : "/" + sPath; + oWin.location.hash = sInner; + }, + errorMessage: "Wrong spath: " + sPath + }); +}, +iPageGoBack: function () { + return this.waitFor({ + success: function () { + Opa5.getWindow().history.go(-1); + }, + errorMessage: "Go back to previous page failed" + }); +} +``` + +## 5b.8 Mockserver POST handlers consumed by an app-side message collector need a `sap-message` header + +Narrow scope: this only affects function-import / action POST handlers whose **success callback** feeds the response into an app-written message-array collector — typically a helper that reads `response.headers["sap-message"]`, parses the JSON, and pushes it onto an `aErrorMsg` (or similar) array. The downstream code then **dereferences `aErrorMsg[0]`** (e.g. `aErrorMsg[0].severity === "Information"`) before deciding to show a toast. The collector lives under the app's `libs/` (or equivalent) — there is no UI5-library equivalent; each project ships its own. + +If such a handler answers with `200 + empty body + no headers`, the collector produces an empty `aErrorMsg`, the `aErrorMsg[0].severity` access throws `TypeError: Cannot read properties of undefined`, and the success-toast helper never runs. + +Controllers that call `MessageToast.show(...)` directly inside the function-import `success` callback are **not affected** — their toast does not depend on `sap-message`. Do not blanket-add the header to every POST handler; only the ones whose success path goes through a `sap-message`-driven collector. + +Fix: for each affected handler, return a `sap-message` envelope so `aErrorMsg[0]` exists: + +```js +oMockServerRequests.push({ + method: "POST", + path: new RegExp("(.*)"), // adjust per endpoint + response: function (oXhr) { + var oResult = { "d": {} }; + oXhr.respondJSON(200, { + "sap-message": JSON.stringify({ + code: "/000", + message: "", + longtext_url: "", + target: "", + severity: "info", + transition: false, + details: [] + }) + }, JSON.stringify(oResult)); + } +}); +``` + +Field meanings: `severity: "info"` keeps it out of the error path; `transition: false` marks it as a state message rather than a one-shot dialog trigger; `details: []` satisfies array-typed consumers. `code` and `message` are app-visible — pick values that match your app's i18n class so the rendered toast looks right. + +Detection: locate the project's message collector (it is app code, not UI5 — name varies: `BatchMessageHandler`, `MessageProcessor`, `collectMsg`, …) by grepping for the `sap-message` read pattern, then find call sites that deref the first array entry without a length guard: + +```bash +# find the collector(s) — read sap-message header and push to an array +grep -rnE 'response\.headers\[\s*["\x27]sap-message["\x27]\s*\]' webapp + +# call sites that consume the array's first entry +grep -rnE '\bsubmitChanges\b' webapp/controller webapp/libs +grep -rnE '\baErrorMsg\[0\]|\bmessages?\[0\]\.severity' webapp/controller webapp/libs +``` + +For each match, identify the function-import name the call belongs to, then add the `sap-message` header to that mockserver handler. + +Surface this in `MODERNIZATION_ISSUES.md` if the right endpoint signature cannot be derived from `manifest.json` / metadata alone — the project owner needs to map the message envelope to the app's i18n class. + +## 5b.9 ErrorHandler hardening for non-XML responses + +The mockserver in the iframe does not always return the same body shape as a real OData backend. Specifically, error responses (or success responses surfaced through the error path because of an unrelated issue, e.g. an unhandled batch endpoint) may be plain text or JSON rather than the OData XML envelope. App code that parses the response as XML and then dereferences the first `` node will throw a `TypeError` (`Cannot read properties of undefined (reading 'firstChild')` / `... of null`) — masking the real failure with a secondary throw inside the error handler's `catch` block. This is much more visible under the iframe setup because the iframe receives the raw mock response without any FLP-level message preprocessing. + +Detection grep: +```bash +grep -rnE 'getElementsByTagName\("message"\)\[0\]\.firstChild' webapp +grep -rnE 'parseFromString\([^,]+,\s*"text/xml"\)' webapp +``` + +Typical offending pattern (in `webapp/controller/ErrorHandler.js` or similar): +```js +var xmlDoc = parser.parseFromString(sResponseText, "text/xml"); +sShortErrorMessage = xmlDoc.getElementsByTagName("message")[0].firstChild.data; +``` + +Add a null-guard on both the node and its `firstChild`, and fall back to the raw response text: + +```diff +- sShortErrorMessage = xmlDoc.getElementsByTagName("message")[0].firstChild.data; ++ var oMessageNode = xmlDoc.getElementsByTagName("message")[0]; ++ if (oMessageNode && oMessageNode.firstChild) { ++ sShortErrorMessage = oMessageNode.firstChild.data; ++ } else { ++ sShortErrorMessage = sResponseText; ++ } +``` + +This is app code (not test code), but the bug only surfaces once OPA tests start running against the iframe-hosted mockserver. Ship the fix together with the iframe migration, or flag it in `MODERNIZATION_ISSUES.md` so the project owner picks it up in a separate PR. + +## 5b.10 testsuite.qunit.js — no extra path alias + +Do **not** register `/test/integration/opaIframe` in `loader.paths`. The iframe URL is resolved through `sap.ui.require.toUrl("test-resources//integration/opaIframe")`, which works because `Test.qunit.html` declares the `test-resources.` resource root pointing at `./` (the `webapp/test/` directory). Resolving via `/test/...` instead is wrong: under a packaged WAR, `webapp/test/` is served at `test-resources/`, not at `test/`, so the iframe URL 404s and `iStartMyAppInAFrame` hangs to its 90s timeout. Earlier intermediate commits added a `loader.paths` alias for `flpSandbox`; that line is dead code under the bare-Component setup and should not be reintroduced. + +If a stale `flpSandbox` alias is present from a previous attempt, delete it. + +## 5b.11 Files to delete + +- `webapp/test/integration/flpSandbox.qunit.html` — only delete if it exists from a prior intermediate attempt. Greenfield Pattern U projects will not have it. +- Any `webapp/test/integration/AllJourneys.js` / `AllJourneys.json` — already covered by Phase 5 / Phase 6. + +## 5b.12 Verification (manual) + +Automated assertion is not feasible without running the full suite headless; provide the developer a checklist: + +1. Serve the app (`mvn ui5:serve`, `npm start`, etc.). +2. Open `…/test/Test.qunit.html?testsuite=test-resources//testsuite.qunit&test=integration/`. +3. Confirm the iframe shows the Component directly — no FLP header, no shell bar, no `#app-tile` in the URL. +4. All journeys pass. +5. Hash deep-link smoke test: open `opaIframe.qunit.html#/` directly in a browser — Component should hit the deep route on first load. + +## 5b.13 Commit message template + +``` +test: switch OPA iframe to bare Component (drop FLP shell) + +The FLP sandbox iframe wires sap.ushell ShellNavigation as the hash +handler; combined with Container.setDirtyFlag(true) calls in app code, +this blocks programmatic hash changes during OPA tests. + +Replace any flpSandbox.qunit.html with opaIframe.qunit.html that loads +sandbox.js (for ushell API stubs) but boots the Component directly via +ComponentContainer.placeAt — no createRenderer, no shell DOM, no +ShellNavigation interceptor. Hash now routes through the Component +router. + +- Common.js: iStartMyAppInAFrame source -> opaIframe.qunit.html; + hash no longer wrapped in #app-tile&/ +- Journey files: Given.iStartMyApp() instead of iStartMyUIComponent +- OpaSetup.js: drop parent-frame mockserver init +- Page objects: cross-window control resolve via Opa5.getWindow() +``` + +--- + +## Risks / gotchas to surface to developer + +- **Real cross-app navigation** — apps that hard-depend on `getService("CrossApplicationNavigation").toExternal({...})` round-tripping through FLP intents will behave differently; sandbox.js stubs return placeholders. Usually fine for OPA, but flag it. +- **Component startup hash** — Component router reads `location.hash` at init. Iframe URL with `#/` should hit the deep route on first load; verify with a representative deep-link route. +- **Mockserver async** — if `mockserver.init` is async without returning a Promise, the iframe's `placeAt` may run before mocks are wired and the first OData calls 404. Either make `init` return a Promise, or wrap with a `setTimeout`/`Promise.resolve().then(placeAt)`. +- **Iframe + autoWait + slow mocks** — `iStartMyAppInAFrame({ timeout: 200 })` is fine on local serves; raise it if CI is slower. +- **`sap_belize` vs `sap_horizon`** — older flpSandbox HTMLs use `sap_belize`; the new iframe page should match the Test Starter's `ui5.theme` (typically `sap_horizon`) so the screenshot matches dev mode. + +## What NOT to do (intermediate-only patterns) + +These showed up during the source migration and were superseded. Do not re-introduce them: + +- Full FLP sandbox HTML with `window["sap-ushell-config"] = { defaultRenderer: "fiori2", applications: { "app-tile": ... } }`. +- ` + `; + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 1); + assert.equal(findings[0].patternId, "sap.ui.define-override"); + assert.equal(findings[0].file, "host.html"); + assert.match(findings[0].snippet, /sap\.ui\.define\s*=/); +}); + +test("flags sap.ui.require override", () => { + const html = "sap.ui.require = function (deps, cb) { cb(); };"; + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 1); + assert.equal(findings[0].patternId, "sap.ui.require-override"); +}); + +test("flags defineModuleSync calls", () => { + const html = ` + sap.ui.loader._.defineModuleSync("foo/Bar", {}); + defineModuleSync("baz", {}); + `; + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 2); + assert.ok(findings.every(f => f.patternId === "defineModuleSync")); +}); + +test("ignores patterns inside // comments", () => { + const html = ` + // sap.ui.define = oldRef; -- example only + var x = 1; + `; + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 0); +}); + +test("does not flag legitimate sap.ui.define([deps], cb) calls", () => { + const html = ` + sap.ui.define(["dep/A"], function (A) { + return A; + }); + `; + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 0); +}); + +test("returns line numbers (1-based) and trimmed snippets", () => { + const html = [ + "", + " ", + " sap.ui.define = function () {};", + " ", + "" + ].join("\n"); + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 1); + assert.equal(findings[0].line, 3); + assert.equal(findings[0].snippet, "sap.ui.define = function () {};"); +}); + +test("only one finding per line even when multiple patterns match", () => { + // Single line, both override patterns present — must not double-report. + const html = "sap.ui.define = sap.ui.require = function () {};"; + const findings = scanContentForBootstrapOverrides(html, "host.html"); + assert.equal(findings.length, 1); +}); + +// ----- listHtmlFiles + scanBootstrapOverrides (file-walking) ------------- + +test("listHtmlFiles walks subdirectories", () => { + withTempDir(dir => { + fs.mkdirSync(path.join(dir, "sub")); + fs.writeFileSync(path.join(dir, "a.html"), ""); + fs.writeFileSync(path.join(dir, "sub", "b.html"), ""); + fs.writeFileSync(path.join(dir, "skip.js"), ""); + + const files = listHtmlFiles(dir).map(f => path.relative(dir, f)).sort(); + assert.deepEqual(files, ["a.html", path.join("sub", "b.html")]); + }); +}); + +test("listHtmlFiles returns empty list when dir is missing", () => { + assert.deepEqual(listHtmlFiles("/no/such/path/here-xyz"), []); +}); + +test("scanBootstrapOverrides aggregates findings from all html files", () => { + withTempDir(dir => { + fs.writeFileSync( + path.join(dir, "host.html"), + "sap.ui.define = function () {};\n" + ); + fs.mkdirSync(path.join(dir, "sub")); + fs.writeFileSync( + path.join(dir, "sub", "page.html"), + "sap.ui.loader._.defineModuleSync('m', {});\n" + ); + fs.writeFileSync(path.join(dir, "clean.html"), "
nothing
\n"); + + const result = scanBootstrapOverrides(dir); + assert.equal(result.scannedFileCount, 3); + assert.equal(result.findings.length, 2); + + const ids = result.findings.map(f => f.patternId).sort(); + assert.deepEqual(ids, ["defineModuleSync", "sap.ui.define-override"]); + }); +}); + +// ----- Multi-module HTML rule (point 1) --------------------------------- + +test("parse() emits one entry per module for multi-module Pattern B HTML", () => { + withTempDir(dir => { + const namespace = "my/app"; + const testBaseDir = path.join(dir, "webapp", "test"); + fs.mkdirSync(path.join(testBaseDir, "opa", "Area"), { recursive: true }); + + // testsuite.qunit.html that registers one HTML test page (Pattern B style). + const testsuiteHtml = path.join(testBaseDir, "testsuite.qunit.html"); + fs.writeFileSync( + testsuiteHtml, + `` + ); + + // The HTML page loads TWO journey modules — the legacy multi-module pattern. + fs.writeFileSync( + path.join(testBaseDir, "opa", "Area", "Combined.qunit.html"), + `Combined` + ); + + const { parse } = require("./parse-testsuite.js"); + const out = parse(testsuiteHtml, testBaseDir, namespace); + + assert.equal(out.pattern, "B"); + // Critical: TWO entries, one per module — no synthetic *Combined key. + assert.ok( + out.entries["opa/view/Area/JourneyOne"], + "JourneyOne entry must exist" + ); + assert.ok( + out.entries["opa/view/Area/JourneyTwo"], + "JourneyTwo entry must exist" + ); + // No invented key like "opa/view/Area/JourneyOneCombined". + for (const key of Object.keys(out.entries)) { + assert.doesNotMatch(key, /Combined$/, `synthetic key emitted: ${key}`); + } + // Both entries trace back to the source HTML for diagnostics. + assert.equal( + out.entries["opa/view/Area/JourneyOne"]._fromMultiModuleHtml, + "opa/Area/Combined.qunit.html" + ); + }); +}); + +// ----- scanContentForLauncher / detectLauncher -------------------------- +// +// The launcher scan is what tells the modernize-test-starter skill whether +// the project arrived already in an iframe (continue normal flow) or in +// the in-window component shape (Pattern U — run Phase 5b iframe migration). +// Mixed projects must halt; a clean iframe/in-window classification is the +// only safe input to Phase 5b. + +test("scanContentForLauncher flags iStartMyAppInAFrame", () => { + const js = ` + sap.ui.define([], function () { + return { + iStartMyApp: function () { + this.iStartMyAppInAFrame({ source: "x.html" }); + } + }; + }); + `; + const findings = scanContentForLauncher(js, "Common.js"); + assert.equal(findings.length, 1); + assert.equal(findings[0].launcher, "iframe"); + assert.match(findings[0].snippet, /iStartMyAppInAFrame/); +}); + +test("scanContentForLauncher flags iStartMyUIComponent", () => { + const js = ` + Given.iStartMyUIComponent({ + componentConfig: { name: "my.app", async: true } + }); + `; + const findings = scanContentForLauncher(js, "Journey.js"); + assert.equal(findings.length, 1); + assert.equal(findings[0].launcher, "in-window"); +}); + +test("scanContentForLauncher ignores commented-out launcher calls", () => { + const js = ` + // Given.iStartMyUIComponent({ componentConfig: {} }); + Given.iStartMyAppInAFrame({ source: "x.html" }); + `; + const findings = scanContentForLauncher(js, "Journey.js"); + assert.equal(findings.length, 1); + assert.equal(findings[0].launcher, "iframe"); +}); + +test("detectLauncher classifies iframe-only project", () => { + withTempDir((dir) => { + const integ = path.join(dir, "integration"); + fs.mkdirSync(integ, { recursive: true }); + fs.writeFileSync( + path.join(integ, "Common.js"), + 'this.iStartMyAppInAFrame({ source: "x.html" });' + ); + const result = detectLauncher(dir); + assert.equal(result.launcher, "iframe"); + assert.equal(result.iframeHits.length, 1); + assert.equal(result.inWindowHits.length, 0); + }); +}); + +test("detectLauncher classifies in-window-only project", () => { + withTempDir((dir) => { + const integ = path.join(dir, "integration"); + fs.mkdirSync(integ, { recursive: true }); + fs.writeFileSync( + path.join(integ, "JourneyOne.js"), + 'Given.iStartMyUIComponent({ componentConfig: { name: "my.app" } });' + ); + fs.writeFileSync( + path.join(integ, "JourneyTwo.js"), + 'Given.iStartMyUIComponent({ componentConfig: { name: "my.app" } });' + ); + const result = detectLauncher(dir); + assert.equal(result.launcher, "in-window"); + assert.equal(result.iframeHits.length, 0); + assert.equal(result.inWindowHits.length, 2); + }); +}); + +test("detectLauncher reports mixed when both shapes coexist", () => { + withTempDir((dir) => { + const integ = path.join(dir, "integration"); + fs.mkdirSync(integ, { recursive: true }); + fs.writeFileSync( + path.join(integ, "OldJourney.js"), + 'Given.iStartMyUIComponent({ componentConfig: { name: "my.app" } });' + ); + fs.writeFileSync( + path.join(integ, "NewJourney.js"), + 'this.iStartMyAppInAFrame({ source: "x.html" });' + ); + const result = detectLauncher(dir); + assert.equal(result.launcher, "mixed"); + assert.equal(result.iframeHits.length, 1); + assert.equal(result.inWindowHits.length, 1); + }); +}); + +test("detectLauncher reports none when no launcher calls present", () => { + withTempDir((dir) => { + const integ = path.join(dir, "integration"); + fs.mkdirSync(integ, { recursive: true }); + fs.writeFileSync( + path.join(integ, "Helper.js"), + 'sap.ui.define([], function () { return {}; });' + ); + const result = detectLauncher(dir); + assert.equal(result.launcher, "none"); + assert.equal(result.iframeHits.length, 0); + assert.equal(result.inWindowHits.length, 0); + }); +}); + +// ----- scanContentForFlpSandbox / detectFlpSandbox ---------------------- +// +// FLP sandbox presence gates Phase 5b — Pattern U projects without any +// `sap/ushell/bootstrap/sandbox.js` load (or `window["sap-ushell-config"]` +// declaration) in their legacy test pages are plain in-window apps that +// don't depend on the ushell API and should be left alone. + +test("scanContentForFlpSandbox flags ushell sandbox script tag", () => { + const html = ` + + `; + const findings = scanContentForFlpSandbox(html, "flpSandbox.html"); + assert.equal(findings.length, 1); + assert.equal(findings[0].patternId, "sandbox-script"); +}); + +test("scanContentForFlpSandbox flags older flpSandbox.js script", () => { + const html = ''; + const findings = scanContentForFlpSandbox(html, "host.html"); + assert.equal(findings.length, 1); + assert.equal(findings[0].patternId, "sandbox-script"); +}); + +test("scanContentForFlpSandbox flags window['sap-ushell-config'] block", () => { + const html = ` + + `; + const findings = scanContentForFlpSandbox(html, "host.html"); + assert.equal(findings.length, 1); + assert.equal(findings[0].patternId, "sap-ushell-config"); +}); + +test("scanContentForFlpSandbox ignores commented-out markers", () => { + const html = ` + // +

plain text

+ `; + const findings = scanContentForFlpSandbox(html, "host.html"); + assert.equal(findings.length, 0); +}); + +test("scanContentForFlpSandbox does not flag unrelated script tags", () => { + const html = ''; + const findings = scanContentForFlpSandbox(html, "opaTests.qunit.html"); + assert.equal(findings.length, 0); +}); + +test("detectFlpSandbox reports false when no test HTML loads sandbox.js", () => { + withTempDir((dir) => { + fs.writeFileSync( + path.join(dir, "opaTests.qunit.html"), + '' + ); + const result = detectFlpSandbox(dir); + assert.equal(result.flpSandbox, false); + assert.equal(result.hits.length, 0); + }); +}); + +test("detectFlpSandbox reports true when any test HTML loads sandbox.js", () => { + withTempDir((dir) => { + fs.writeFileSync( + path.join(dir, "opaTests.qunit.html"), + '' + ); + fs.writeFileSync( + path.join(dir, "flpSandbox.qunit.html"), + '' + ); + const result = detectFlpSandbox(dir); + assert.equal(result.flpSandbox, true); + assert.equal(result.hits.length, 1); + }); +}); diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/SKILL.md b/plugins/ui5-modernization/skills/modernize-ui5-app/SKILL.md new file mode 100644 index 0000000..a4e5f6f --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/SKILL.md @@ -0,0 +1,662 @@ +--- +name: modernize-ui5-app +description: | + End-to-end workflow for modernizing a UI5 application. Use this skill when: + - User wants to modernize their UI5 app + - User mentions "UI5 modernization", "modernize UI5 app", "upgrade UI5", "make app modern UI5 compatible" + - User asks to "fix all linter errors", "run full modernization", "modernize my UI5 project" + This skill runs the modernization in five phases with a verification gate after each phase. The user picks the verification mode once at the start (full autonomous / half autonomous / manual). The agent applies that mode at every gate without re-asking. +--- + +# UI5 Modernization Workflow + +This skill modernizes a UI5 application in **five phases**, each followed by a **verification gate**. The user picks the gate behavior **once at the start** — full autonomous, half autonomous, or manual — and the orchestrator applies that choice at every phase boundary. + +## The five phases + +1. **Mechanical baseline** — autofix + test starter restructure. These touch many files but are low-risk and unlock test runs in later gates. +2. **Foundation** — `manifest.json` and `Component.js`. Everything downstream reads the manifest. +3. **Module system & globals** — get the dependency graph right (sap.ui.define arrays, lazy requires, no implicit globals). Cyclic-dep and blind-spot fixes belong here because they're symptoms of the same module-system work, not separate phases. +4. **Deprecated APIs** — pure name-for-name replacement. Independent of module structure, so safe to run in parallel after phase 3 stabilizes the graph. +5. **CSP compliance** — last, because it depends on every prior phase being CSP-clean (no inline scripts, no globals leaking through). + +A documentation pass writes `MODERNIZATION-REPORT.md` and `MODERNIZATION-ISSUES.md` after phase 5. + +## Rule ID to Skill Mapping + +When parsing linter output, use this table to determine which skill handles each rule ID. This is the authoritative routing — every error with a mapped rule MUST be processed by the corresponding skill in its designated phase. + +| Rule ID | Skill | Phase | +|---------|-------|-------| +| `no-deprecated-theme` | `fix-bootstrap-params` | 4 | +| `no-outdated-manifest-version` | `fix-manifest-json` | 2 | +| `no-legacy-ui5-version-in-manifest` | `fix-manifest-json` | 2 | +| `no-deprecated-library` | `fix-manifest-json` (manifest.json) or `fix-bootstrap-params` (HTML) | 2 / 4 | +| `no-deprecated-component` | `fix-manifest-json` | 2 | +| `no-removed-manifest-property` | `fix-manifest-json` or `fix-component-async` | 2 | +| `async-component-flags` | `fix-component-async` | 2 | +| `no-globals` | `fix-xml-globals` (ALL XML — sap.*, jQuery.*, AND app-namespace globals) or `fix-js-globals` (JS — sap.*/jQuery.* only) | 3 | +| `no-ambiguous-event-handler` | `fix-xml-globals` | 3 | +| `no-deprecated-control-renderer-declaration` | `fix-control-renderer` | 4 | +| `ui5-class-declaration` | `fix-control-renderer` | 4 | +| `no-pseudo-modules` | `fix-pseudo-modules` | 3 | +| `no-implicit-globals` | `fix-pseudo-modules` | 3 | +| `unsupported-api-usage` | `fix-partially-deprecated-apis` | 4 | +| `prefer-test-starter` | `modernize-test-starter` | 1 | +| `csp-unsafe-inline-script` | `fix-csp-compliance` | 5 | +| (structural — post all fix phases) | `fix-cyclic-deps` | 3 | +| (runtime — post all fix phases) | `fix-linter-blind-spots` | 3 | + +### Disambiguating `no-deprecated-api` + +The `no-deprecated-api` rule covers many cases. Determine which skill to use based on file type and message content: + +| File Type | Message Contains | Skill | Phase | +|-----------|------------------|-------|-------| +| `.html` | "bootstrap parameter" | `fix-bootstrap-params` | 4 | +| `.html` | "deprecated theme" | `fix-bootstrap-params` | 4 | +| `manifest.json` | "view type", "model type", "resources/js" | `fix-manifest-json` | 2 | +| `.js` | "Deprecated call to ... {apiVersion: 2}" (Lib.init/Library.init) | `fix-library-init` | 4 | +| `.js` | "deprecated renderer", "apiVersion" (renderer context) | `fix-control-renderer` | 4 | +| `.js` | "IconPool" | `fix-control-renderer` | 4 | +| `.js` | "rerender" | `fix-control-renderer` | 4 | +| `.js` | "deprecated class", "deprecated property", "deprecated interface" | `fix-deprecated-controls` | 4 | +| `.js` | "registerControllerExtensions" | `fix-fiori-elements-extensions` | 4 | +| `.js` | "sap.ui.controller" where controller name appears in manifest `sap.ui.controllerExtensions` | `fix-fiori-elements-extensions` | 4 | +| `.js` | "sap.ui.controller" where controller name is NOT in manifest `sap.ui.controllerExtensions` | `fix-js-globals` (case 9: controller factory) | 3 | +| `.js` | "jQuery.sap.declare", "jQuery.sap.require" | `fix-js-globals` (case 10: legacy module wrapping) | 3 | +| `.js` | "getLibraryResourceBundle" | `fix-js-globals` (Core API replacement) | 3 | +| `.js` | "MessagePage" | `fix-deprecated-controls` (MessagePage → IllustratedMessage) | 4 | +| `.js` | "Parameters.get", "loadData", "Mobile.init", "createEntry", "View.create", "Fragment.load", "Router" | `fix-partially-deprecated-apis` | 4 | +| `.xml` | "native HTML", "SVG" | `fix-xml-native-html` | 4 | +| `.xml` | "visibleRowCountMode", "visibleRowCount", "rowHeight", "fixedRowCount", "fixedBottomRowCount", "minAutoRowCount" | `fix-table-row-mode` | 4 | +| `.xml` | "MessagePage" | `fix-deprecated-controls` (MessagePage → IllustratedMessage) | 4 | +| any | *(no pattern matches above)* | Log to `MODERNIZATION-ISSUES.md` for manual review | — | + +## Phase 0: Prerequisites and Mode Selection + +### Prerequisite check + +```bash +npx @ui5/linter --version || echo "ERROR: @ui5/linter not available" +``` + +If the linter is not installed, tell the user to run `npm install --save-dev @ui5/linter` and stop. + +### Non-interactive mode (tests / pre-push hook) + +If the invoking prompt contains "Do not ask for confirmation" or similar instructions to skip interactive prompts, operate in **non-interactive mode**: + +- Do NOT ask for verification mode — there is no human in the loop. +- Do NOT run verification gates (build/tests) between phases. +- Do NOT create git commits (the test harness handles version control). +- Do NOT generate the documentation phase (no MODERNIZATION-REPORT.md or MODERNIZATION-ISSUES.md). +- Execute phases 1–5 sequentially without pausing. + +In this mode, skip the rest of Phase 0 and proceed directly to Phase 1. + +### Ask the user once: which verification mode? + +Before phase 1, the agent asks the user to pick a verification mode. This choice applies to **every** phase boundary — do not re-ask between phases. + +Present these three options (use AskUserQuestion): + +| Mode | What happens at each gate | +|---|---| +| **Full autonomous** | Run tests (see Build & test commands) → if anything fails, attempt to debug and apply a fix. Stop and escalate when EITHER (a) the same failure has been retried 3 times OR (b) tests are still red after one debug attempt. On stop, emit a structured report and ask the user how to proceed. | +| **Half autonomous** | Run tests (see Build & test commands) → emit a structured report (passed/failed/skipped + failure summaries). Wait for user to type "continue" or give corrective input. | +| **Manual** | Skip tests entirely. Print a one-line phase summary (files touched, errors fixed, errors deferred). Wait for the user to verify and type "continue". | + +Save the chosen mode in working memory. The mode determines **only** what happens at gates — phases themselves run identically regardless. + +### Build & test commands + +Detect the project type at the start and use the matching commands throughout all gates. Read `references/build-and-test-commands.md` for the full details (prerequisites, exact commands, troubleshooting). + +**Prerequisites:** Chrome DevTools MCP must be connected for browser-based test verification. If not available, install it via `/install-mcps` before proceeding. + +**Summary:** + +- **`pom.xml` exists** → Maven project. Ensure prerequisites (headless-chrome.json, pom.xml dev profile patch, .gitignore) are in place before the first test run. For verification gates, use §1.3 only: `mvn clean verify -P execute.qunit ...` (self-contained: compiles + tests). §1.2 is for interactive dev only — do not use it in gates. See the reference for exact flags and troubleshooting. +- **No `pom.xml`** → npm project. Read `package.json` `"scripts"` for build/test commands (typically `npm run build` / `npm test`). If no test script exists, fall back to `npx @ui5/linter --details`. + +## Operating Principles + +These apply throughout. They exist because past runs failed by skipping fixable work, deferring volume-heavy errors, or stopping early. + +1. **Fix every error that has a mapped skill.** Volume is not a reason to defer. 480 formatter references across 140 XML files is the work, not a reason to skip — sub-agents handle repetition. + +2. **Phases are mandatory in order.** Phase 3 in particular catches runtime patterns invisible to the linter (cyclic deps, app-namespace globals, QUnit assertions). A zero-error linter report does NOT mean phase 3 is complete — its sub-skills have their own detection scripts. + +3. **Never auto-modernize sync→async when the return type changes.** These are the exceptions where `fix-partially-deprecated-apis` does NOT apply — they cannot be done mechanically because they require restructuring all callers: + - `sap.ui.xmlfragment()` → `Fragment.load()` (returns control vs. Promise) + - `sap.ui.component()` → `Component.create()` + - `sap.ui.view()` → `View.create()` + - `sap.ui.controller()` (instantiation form) → `Controller.create()` + + Document these in `MODERNIZATION-ISSUES.md` with file path, line, the sync API, its async replacement, and which callers need restructuring. Continue fixing other errors in the same file. The disambiguation table routes `View.create`, `Fragment.load` to `fix-partially-deprecated-apis` only when the *caller already uses async/await or `.then()`* — if the caller expects a synchronous return value, defer to this principle. + +4. **`MODERNIZATION-ISSUES.md` is a last resort.** Valid entries: no skill exists for the rule, or a skill was applied but the fix genuinely failed (with explanation). If you find yourself writing "~N remaining errors for rule X" while a skill exists for rule X, go back and fix them. + +## Git Commit Strategy + +**One commit per phase, five commits total** (plus the documentation commit). Stage only the files modified in that phase — never use `git add -A` or `git add .`. Do not push. + +**Never stage these files** (they are local-only test infrastructure): +- `headless-chrome.json` — local headless Chrome config +- `pom.xml` — if the dev profile patch was applied for testing +- `.gitignore` — if modified to add `target/` and `headless-chrome.json` + +These files must remain locally for future test runs but must never be committed or pushed. + +| # | After Phase | Commit Message | +|---|---|---| +| 1 | Phase 1 | `chore: apply UI5 linter autofix and modernize test starter` | +| 2 | Phase 2 | `fix: modernize manifest.json and Component.js` | +| 3 | Phase 3 | `fix: modernize module system (globals, pseudo-modules, cycles, blind spots)` | +| 4 | Phase 4 | `fix: replace deprecated UI5 APIs` | +| 5 | Phase 5 | `fix: enforce CSP compliance` | +| 6 | Documentation | `docs: add modernization report and issues` | + +If a phase makes no changes (e.g., the project has no XML views with native HTML in phase 4), skip its commit. + +## Phase 1: Mechanical Baseline + +**Goal:** Apply low-risk mechanical fixes that prepare the project for verification gates in later phases. + +**Order matters within this phase:** autofix runs first (it modifies many files mechanically), then `modernize-test-starter` (which restructures test entry points and depends on a stable JS layout). + +### Step 1.1: Initial Analysis + +Run the linter to establish baseline: + +```bash +# Run linter with details and capture output +npx @ui5/linter --details 2>&1 | tee /tmp/ui5-linter-baseline.txt + +# Count total errors (lines containing " error " or " warning ") +grep -c " error \| warning " /tmp/ui5-linter-baseline.txt || echo "0" +``` + +Parse the output to extract: +- Total error count +- Total warning count +- Errors grouped by rule ID +- Errors grouped by file + +Store these metrics for the final comparison. + +### Step 1.2: Apply Autofix + +Run the linter's autofix mode: + +```bash +# Run autofix +npx @ui5/linter --fix + +# Run linter again to count remaining errors +npx @ui5/linter --details 2>&1 | tee /tmp/ui5-linter-post-autofix.txt +grep -c " error \| warning " /tmp/ui5-linter-post-autofix.txt || echo "0" +``` + +Calculate: +- Errors fixed by autofix = baseline count - post-autofix count +- Remaining errors to fix manually + +### Step 1.3: Test Starter modernization + +Launch a single sub-agent with the `modernize-test-starter` skill. This is a project-wide operation (not per-file), so use a custom prompt rather than the standard template: + +``` +Modernize test infrastructure to use the UI5 Test Starter concept. + +Project root: {project-path} + +Errors to fix: +{all prefer-test-starter errors from the linter output} + +Instructions: +1. Read {skills-dir}/modernize-test-starter/SKILL.md carefully. This is a complex skill with many phases — do NOT skip the documentation. +2. CRITICAL — Phase 0 (Detection) must run completely BEFORE any file changes. The + classification of the test infrastructure (launcher type, OPA pattern, test structure) + determines the entire conversion path. Skipping or rushing detection leads to missed + conversions and wrong transformation choices. +3. Follow ALL phases in order — every phase must complete +4. Verify against the Completion Checklist (14 items) before reporting done +5. Report: files created, files renamed, files deleted, detection/classification results +``` + +### Phase 1 commit + gate + +- Commit: `chore: apply UI5 linter autofix and modernize test starter` +- Run the verification gate per the chosen mode. + +## Phase 2: Foundation + +**Goal:** Modernize `manifest.json` and `Component.js`. These files are the foundation everything else assumes. + +**Sequential within this phase:** manifest first (because Component.js may need to reflect manifest changes like `IAsyncContentCreation`). + +### Step 2.1: manifest.json + +Launch a sub-agent with `fix-manifest-json`. Pass all errors targeting `manifest.json` from the rules: `no-outdated-manifest-version`, `no-legacy-ui5-version-in-manifest`, `no-deprecated-library`, `no-deprecated-component`, `no-removed-manifest-property`, and `no-deprecated-api` errors whose message mentions "view type", "model type", or "resources/js". + +Sub-agent prompt: +``` +Fix manifest.json modernization issues. + +Project root: {project-path} + +File: webapp/manifest.json +Errors to fix: +{manifest.json errors from the linter output} + +Instructions: +1. Read {skills-dir}/fix-manifest-json/SKILL.md — follow ALL fix strategies and implementation steps +2. Pay special attention to the Notes section — these are load-bearing constraints from past failures +3. Complete ALL implementation steps in order +4. Verify against the skill's completion criteria +5. Report: properties changed, properties removed, issues that could not be fixed +``` + +### Step 2.2: Component.js + +Launch a sub-agent with `fix-component-async`. Also pass any `no-deprecated-library`, `no-deprecated-component`, or `no-removed-manifest-property` errors targeting Component.js. + +Sub-agent prompt: +``` +Modernize Component.js async loading. + +Project root: {project-path} + +File: webapp/Component.js +Errors to fix: +{Component.js errors from the linter output, if any} + +Instructions: +1. Read {skills-dir}/fix-component-async/SKILL.md — follow ALL fix strategies and implementation steps +2. Pay special attention to Critical Rules section — these are load-bearing constraints from past failures +3. This is unconditional — apply even if linter didn't flag `async-component-flags` (the error + only appears after manifest.json removes `async: true` from rootView) +4. Complete ALL implementation steps in order +5. Report: changes applied, errors fixed +``` + +### Step 2.3: Verify Component.js (gate script) + +After Step 2.2 completes, run the gate script to catch common mistakes: + +```bash +node {skills-dir}/fix-component-async/scripts/verify-component.js {project-path} +``` + +If the script exits with code 1 (findings with severity "error"), fix the Component.js: +- `imported-interface`: Remove `sap/ui/core/IAsyncContentCreation` from the `sap.ui.define` dependency array and its function parameter. Use the string literal `"sap.ui.core.IAsyncContentCreation"` in the interfaces array instead. +- `interface-not-string`: Replace the variable reference in `interfaces: [IAsyncContentCreation]` with the string literal `interfaces: ["sap.ui.core.IAsyncContentCreation"]`. +- `missing-interface`: Add `interfaces: ["sap.ui.core.IAsyncContentCreation"]` inside the `metadata` object. + +Re-run the script to confirm `pass: true` before proceeding. + +### Phase 2 commit + gate + +- Commit: `fix: modernize manifest.json and Component.js` +- Run the verification gate. + +## Phase 3: Module System & Globals + +**Goal:** Get the module dependency graph right. Fix `sap.ui.define` arrays, eliminate global namespace access, replace pseudo-modules, and resolve any cycles introduced by the changes. + +**Strategy:** The three global/module skills are largely independent at the file level (different files have different issues), so they run **in parallel**. `fix-linter-blind-spots` and `fix-cyclic-deps` run **sequentially after** the parallel batch because they need a stable, complete view of the JS layer. + +**Critical: always use sub-agents for Step 3.1.** Do NOT rewrite controllers or JS files inline — the `fix-js-globals` SKILL.md contains Key Rules (dead code removal, this.byId() collapsing, merge-not-deepExtend) that the main agent does not have in context. Skipping the sub-agent means skipping those rules and producing incorrect output. + +### Step 3.1: Parallel batch — globals and pseudo-modules + +Launch sub-agents in parallel (foreground, single message, up to 8 concurrent — batch sequentially if more): + +- `fix-js-globals` for JS files with `no-globals` errors (sap.*/jQuery.*) plus the `no-deprecated-api` cases routed to it via the disambiguation table. +- `fix-pseudo-modules` for files with `no-pseudo-modules` or `no-implicit-globals` errors. +- `fix-xml-globals` for ALL XML files with `no-globals` or `no-ambiguous-event-handler` errors. This includes `sap.*`, `jQuery.*`, AND app-namespace globals (e.g., `com.example.app.utils.Handler.onPress`) — XML app-namespace globals ARE detected by the linter and belong here, not in blind-spots. + +**Test resources are included.** Files under `test/`, `webapp/test/`, `test/unit/`, `test/integration/`, etc. get the same treatment as app source files. The linter reports the same rule IDs across both, and the same skills apply. + +### Step 3.1b: Regression check + +After all Step 3.1 sub-agents complete, re-run the linter to catch regressions introduced by the globals/pseudo-modules fixes (e.g., missing imports, broken references): + +```bash +npx @ui5/linter --details 2>&1 | tee /tmp/ui5-linter-post-phase3-batch.txt +``` + +Compare against the post-autofix baseline. If new errors appear for rules already handled by Step 3.1 skills (`no-globals`, `no-pseudo-modules`, `no-implicit-globals`), launch a second parallel batch targeting only those new errors. If new errors belong to other phases (e.g., `no-deprecated-api`), ignore them — they'll be handled in Phase 4. + +### Step 3.2: Sequential — fix-linter-blind-spots + +After the parallel batch completes, launch **one sub-agent** with `fix-linter-blind-spots`. This skill catches runtime-breaking patterns the linter does NOT report: app-namespace globals in JS files, QUnit 1.x assertions, sinon mocking via global chains. + +Sub-agent prompt: +``` +Fix linter blind spots (runtime-breaking patterns the linter does NOT report). + +Project root: {project-path} + +Instructions: +1. Read {skills-dir}/fix-linter-blind-spots/SKILL.md +2. Read manifest.json for the app namespace +3. Run: node {skills-dir}/fix-linter-blind-spots/scripts/detect-blind-spots.js {project-path} +4. Fix all detected patterns in priority order +5. Re-run the script — confirm summary.total === 0 +6. Report: files modified, patterns fixed (count per type), unfixable issues +``` + +### Step 3.3: Sequential — fix-cyclic-deps + +After blind-spots completes, launch **one sub-agent** with `fix-cyclic-deps`. Earlier steps in this phase can introduce cyclic dependencies (e.g., when `fix-js-globals` converts a global read into a `sap.ui.define` dependency). Cycle detection requires a global view of the dependency graph, so it runs as a single agent at the end. + +Sub-agent prompt: +``` +Fix cyclic module dependencies in this UI5 project. + +Project root: {project-path} + +Instructions: +1. Read {skills-dir}/fix-cyclic-deps/SKILL.md +2. Build the internal module dependency graph from all sap.ui.define arrays +3. Detect and auto-fix 2-node direct cycles (lazy sap.ui.require on the lesser-used edge) +4. Detect 3+ node chains via Tarjan's SCC and flag them in MODERNIZATION-ISSUES.md +5. Report: files modified, cycles fixed (count), unfixable cycles (file pairs + reason) +``` + +### Phase 3 commit + gate + +- Run `npx @ui5/linter --details` to verify nothing regressed. +- Commit: `fix: modernize module system (globals, pseudo-modules, cycles, blind spots)` +- Run the verification gate. + +## Phase 4: Deprecated APIs + +**Goal:** Replace deprecated UI5 APIs with their modern equivalents. Pure name-for-name work — no module-system implications. + +**Strategy:** Skills target different rules and (mostly) different files, so they run **in parallel** by skill. Group errors by skill using the "Rule ID to Skill Mapping" and "Disambiguating `no-deprecated-api`" tables above, then launch one sub-agent per file (or per cluster of related files). + +### Execution + +Launch sub-agents in parallel (foreground, single message, up to 8 concurrent — batch sequentially if more). Use the sub-agent prompt template below. Group files by `{skill-name}`; one sub-agent can handle multiple files for the same skill. + +### Step 4.2: Regression check + +After all Phase 4 sub-agents complete, re-run the linter to catch regressions (e.g., a deprecated control replacement that breaks an import or introduces a new `no-globals` error): + +```bash +npx @ui5/linter --details 2>&1 | tee /tmp/ui5-linter-post-phase4.txt +``` + +If new errors appear for rules handled by Phase 4 skills, launch a second batch targeting only those new errors. If new errors belong to Phase 3 rules (unlikely regression), fix them inline or escalate to MODERNIZATION-ISSUES.md. + +### Phase 4 commit + gate + +- Run `npx @ui5/linter --details`. +- Commit: `fix: replace deprecated UI5 APIs` +- Run the verification gate. + +## Phase 5: CSP Compliance + +**Goal:** Make the app run under a strict Content Security Policy. Inline scripts, `eval`, and other CSP-incompatible patterns get extracted or rewritten. + +CSP comes last because earlier phases (notably phase 1's autofix and test-starter restructure, and phase 4's bootstrap-params) may have introduced inline blocks. Running CSP first would mean re-doing the work. + +**Note on test HTML files:** Phase 1's `modernize-test-starter` deletes legacy test entry points (e.g., `unitTests.qunit.html`). However, some test HTMLs may survive (e.g., an `opaTests.qunit.html` that wasn't covered by test-starter). If these have `csp-unsafe-inline-script` errors, they still need CSP fixes here. + +**Critical: never delete inline scripts — always externalize them.** The CSP fix is to move inline code to an external `.js` file and replace the `` with ``. Even trivial config objects or debug flags must be extracted, not removed — removing them is a functional regression. + +Launch sub-agents in parallel for files with `csp-unsafe-inline-script` errors. Use the sub-agent prompt template, `{skill-name}=fix-csp-compliance`. Do NOT fix CSP issues inline from the main agent — the skill contains extraction patterns the main agent doesn't have in context. + +### Phase 5 commit + gate + +- Run `npx @ui5/linter --details` for the final error count. +- Commit: `fix: enforce CSP compliance` +- Run the verification gate. + +## Verification Gate (post-phase, every phase) + +After every phase commit, run the gate matching the mode chosen in Phase 0. + +### Full autonomous + +**Never skip tests.** The user chose full autonomous because they want verified correctness at every phase boundary. You must run the build and tests before proceeding to the next phase, every single time — no exceptions, no deferring to "after the next phase". If no build/test command is available, fall back to `npx @ui5/linter --details` as the minimum verification — but never proceed without running something. + +**Use a 600000ms (10 minute) timeout** for all build and test commands. These commands (especially Maven builds and headless QUnit runs) routinely exceed the default 2-minute timeout. + +**Delegate to a sub-agent.** Launch a sub-agent to run the build, run the tests, and debug failures. This keeps the orchestrator's context clean and lets the sub-agent focus on test output analysis. + +**Before launching the sub-agent**, print: `⏳ Phase {N} gate: launching test sub-agent...` + +The sub-agent prompt: + +``` +Run the verification gate for Phase {N} of a UI5 modernization workflow. + +Project root: {project-path} + +Build & test reference: {skills-dir}/modernize-ui5-app/references/build-and-test-commands.md + +CRITICAL REQUIREMENTS: +- You MUST execute the actual test command (Maven headless QUnit or npm test). A successful + build alone is NOT verification. Running only the linter is NOT verification. +- The gate requires BOTH a passing build AND passing tests. Do not report success unless + you have actually run the test command and confirmed all tests passed. +- IMPORTANT for Maven: exit code 0 does NOT guarantee all tests passed. Maven returns 0 + as long as the BUILD succeeds. You MUST check the end of the command output for failing + test summaries AND check `target/surefire-reports/` (if it exists) for detailed results. +- If you skip tests or only verify via build/linter, you have FAILED your task. +- EXCEPTION: If the build & test reference determines there is NO test command available + (npm project with no test script in package.json), linter-only verification IS acceptable. + In that case, run `npx @ui5/linter --details` and report: + "⚠️ NO TEST COMMAND AVAILABLE — verified via linter only (N errors)." + +SCOPE CONSTRAINT — DO NOT EXCEED: +- Your ONLY job is to run the tests, and if tests fail, fix the FAILING TESTS. +- NEVER fix linter errors, modernize code, or apply changes that belong to other phases. +- When debugging a test failure, ONLY make the minimal change needed to make the test pass + (e.g., fix an import path, adjust a test assertion, correct a module reference). +- Fixes for test breakage caused by the CURRENT phase's changes ARE in scope. For example: + if Phase 2 renamed a manifest property and a test reads that property, updating the test + to use the new name is a valid fix. +- Fixes that require applying a DIFFERENT phase's modernization pattern are NOT in scope. + Report those as "❌ FAILED" — do NOT apply the modernization fix yourself. +- You are a test runner, not a modernizer. Stay in your lane. + +Log progress at every step so the user has feedback during execution. + +Instructions: +1. Read the build & test reference above. Follow its "Project Type Detection" flow to determine + whether this is a Maven or npm project. +2. For Maven projects: §1.3 is self-contained (compiles + tests in one command). Skip §1.2 + (that's an interactive dev server). For npm projects: use the build and test scripts from + package.json (§2). +3. Print: "⏳ Phase {N} gate: running tests..." +4. Run the test command with a 600000ms timeout. + - Maven: §1.3 (`mvn clean verify -P execute.qunit ...`) + - npm: the test script from package.json (e.g., `npm test`) +5. Determine test result — IMPORTANT for Maven: exit code 0 does NOT guarantee all tests + passed. Maven returns 0 as long as the BUILD succeeds, even if individual QUnit tests + failed. You MUST check the end of the command output for failing test summaries AND + check `target/surefire-reports/` (if it exists) for detailed test results. Only report + tests as passed if both the output shows no failures AND no surefire report indicates + red tests. +6. Print test result: "✅ Tests passed (N tests)." or "❌ Tests failed: N of M red." +7. If tests pass → report: "✅ TESTS OK" +8. If tests fail: + a. Print: "🔍 Analyzing failure..." + b. Capture the failing test error output. + c. Print: "🔧 Attempting fix: {brief description of what you're fixing}" + d. Only fix the immediate test failure (broken import, wrong path, missing dependency). + Do NOT fix linter warnings, modernize APIs, or apply changes from other phases. + e. Print: "🔄 Re-running tests..." + f. Re-run the test command with a 600000ms timeout. + g. If it now passes → report: "✅ Fix applied, gate passes." Include what was fixed. + h. If it still fails → report: "❌ FAILED" with: + - Test failure summary (≤20 lines) + - What was attempted + - Suggested next action +9. If you have retried 3 times for the same failure, stop and report. + +For Maven projects: if tests fail with ClassNotFoundException or test-resources/ 404s, +consult the build & test reference §1.4 and references/SAPUI5_Local_Build.md §1.2. +``` + +**After the sub-agent returns:** +- If "✅" → print "✅ Phase {N} gate: tests OK — continuing to Phase {N+1}." → proceed. +- If "❌" → print the failure summary and ask: "Continue with next phase / retry / abort?" + +### Half autonomous + +**Delegate to a sub-agent.** Launch a sub-agent to run the build and tests, then report results back to the orchestrator for user review. + +**Before launching the sub-agent**, print: `⏳ Phase {N} gate: launching test sub-agent...` + +The sub-agent prompt: + +``` +Run the verification gate for Phase {N} of a UI5 modernization workflow. + +Project root: {project-path} + +Build & test reference: {skills-dir}/modernize-ui5-app/references/build-and-test-commands.md + +CRITICAL REQUIREMENTS: +- You MUST execute the actual test command (Maven headless QUnit or npm test). A successful + build alone is NOT verification. Running only the linter is NOT verification. +- The gate requires BOTH a passing build AND passing tests. Do not report success unless + you have actually run the test command and confirmed all tests passed. +- IMPORTANT for Maven: exit code 0 does NOT guarantee all tests passed. Maven returns 0 + as long as the BUILD succeeds. You MUST check the end of the command output for failing + test summaries AND check `target/surefire-reports/` (if it exists) for detailed results. +- If you skip tests or only verify via build/linter, you have FAILED your task. +- EXCEPTION: If the build & test reference determines there is NO test command available + (npm project with no test script in package.json), linter-only verification IS acceptable. + In that case, run `npx @ui5/linter --details` and report: + "⚠️ NO TEST COMMAND AVAILABLE — verified via linter only (N errors)." + +Log progress at every step so the user has feedback during execution. + +Instructions: +1. Read the build & test reference above. Follow its "Project Type Detection" flow to determine + whether this is a Maven or npm project. +2. For Maven projects: §1.3 is self-contained (compiles + tests in one command). Skip §1.2 + (that's an interactive dev server). For npm projects: use the build and test scripts from + package.json (§2). +3. Print: "⏳ Phase {N} gate: running tests..." +4. Run the test command with a 600000ms timeout. + - Maven: §1.3 (`mvn clean verify -P execute.qunit ...`) + - npm: the test script from package.json (e.g., `npm test`) +5. Determine test result — for Maven: do NOT rely on exit code alone. Check the end of the + output for test failure summaries and check `target/surefire-reports/` if it exists. +6. Report results in this exact format: + ✅/❌ Tests: N passed, N failed, N skipped + Failed test names (≤10, then "...and X more") +6. Do NOT attempt to debug or fix failures — just report. +``` + +**After the sub-agent returns:** +1. Print the sub-agent's structured report to the user. +2. Wait for the user. Acceptable user inputs: "continue", "skip phase", "run tests", "abort". +3. Do not attempt to debug or fix unless the user gives explicit instructions. + +### Manual + +``` +1. Print a one-line phase summary: "Phase {N} done. Files modified: {count}. Errors fixed: {count}. Deferred: {count}." +2. Wait for the user to type "continue", verify externally, or give corrective input. +``` + +**Important: the agent does NOT re-ask which mode to use.** The mode was set in Phase 0 and applies to every gate. + +## Sub-Agent Prompt Template (Phases 1–5) + +This template is used by every phase that delegates to skill sub-agents. Phase 3.2 (blind-spots) and 3.3 (cycles) have inline prompts above because they're single-shot global operations. + +**Placeholders:** +- `{project-path}` — absolute path to the UI5 project root +- `{skills-dir}` — absolute path to the parent of this skill's directory (resolved once by the orchestrator) +- `{skill-name}` — the skill folder name (e.g. `fix-js-globals`) +- `{file-path}` — target file(s) +- `{errors}` — linter errors for those files (rule, line, message) + +``` +Fix UI5 linter errors in the following file(s) using the {skill-name} skill. + +Project root: {project-path} + +File: {file-path} +Errors to fix: +{errors} + +Instructions: +1. Read the skill at: {skills-dir}/{skill-name}/SKILL.md +2. Pay special attention to any "Key Rules" section at the top — these are load-bearing constraints from past failures. +3. Read any reference files mentioned in the skill. +4. Read the affected file(s). +5. Apply the fix patterns from the skill EXACTLY as documented — do not apply general web development best practices that conflict with the skill. +6. After fixing, verify each error is resolved. +7. Report back CONCISELY: + - Files modified (paths only) + - Count of errors fixed + - Errors that could NOT be fixed (one line each): + `{file}:{line} | {rule} | {reason} | {suggested fix}` +``` + +### Sub-agent execution rules + +- **Foreground mode only.** Do NOT use `run_in_background: true`. Launch all sub-agents for a phase step in a SINGLE message with multiple Agent tool calls — this blocks the main agent until all return, preventing mid-edit interference. +- **Cap at ~8 concurrent sub-agents per message.** If more files, batch sequentially — do NOT stop after one batch. +- **Group related files into one sub-agent.** A controller and its XML view both needing `core:require` changes should share a sub-agent. +- **No validation between sub-agent batches within a phase step.** Files may be in transient state. All linter/LSP checks happen AFTER the phase step is complete. +- **Every file with a mapped error MUST be processed.** Skipping due to volume is a failure mode. + +### Per-phase skill dispatch checklist + +Quick reference for which skills get dispatched in each phase. The "Rule ID to Skill Mapping" table above is authoritative — this is a convenience summary. + +| Phase | Skills dispatched | +|-------|-------------------| +| 1 | `modernize-test-starter` | +| 2 | `fix-manifest-json`, `fix-component-async` | +| 3 (parallel) | `fix-js-globals`, `fix-pseudo-modules`, `fix-xml-globals` | +| 3 (sequential) | `fix-linter-blind-spots`, `fix-cyclic-deps` | +| 4 | `fix-bootstrap-params`, `fix-library-init`, `fix-control-renderer`, `fix-deprecated-controls`, `fix-fiori-elements-extensions`, `fix-partially-deprecated-apis`, `fix-table-row-mode`, `fix-xml-native-html` | +| 5 | `fix-csp-compliance` | + +## Documentation Phase (after Phase 5) + +Create `MODERNIZATION-ISSUES.md` (unfixable errors) and `MODERNIZATION-REPORT.md` (final summary) using the templates in `references/documentation-templates.md`. + +Commit: `docs: add modernization report and issues` + +## Context Management + +This workflow touches 17+ skill files totaling ~8,000 lines. The main agent's context cannot hold them all. Strategy: **the main agent never reads child skill files — only sub-agents do.** + +1. **Never read child skills.** The routing table tells you which skill to assign. The sub-agent prompt tells each sub-agent to read its own skill. Trust the routing. +2. **Parse and discard linter output.** Parse into `[{file, line, rule, message}]`, optionally persist to `/tmp/ui5-modernization-state.json`, then drop the raw text. Give each sub-agent only its filtered errors. +3. **Compress sub-agent results.** Keep: count of errors fixed, unfixable errors (for MODERNIZATION-ISSUES.md), files modified (for the commit). Discard narrative. +4. **Prioritize completion over documentation.** If context runs low after Phase 4, always finish Phase 5 (CSP). Skip or minimize the documentation phase — the commit history already records what changed. + +## Error Handling + +If a fix attempt **genuinely fails** (not "there are too many"): +1. Log the error. +2. Add to MODERNIZATION-ISSUES.md. +3. Continue with the next error — do not stop the workflow. + +## Completion Checklist + +Before the documentation phase: + +- [ ] User picked verification mode in Phase 0; agent did not re-ask +- [ ] Each of phases 1–5 has a commit (or was skipped because no changes applied) +- [ ] Verification gate ran after every phase per the chosen mode +- [ ] Phase 3 ran ALL three steps (parallel batch + blind-spots + cycles), even if linter showed 0 errors after the batch +- [ ] Sub-agents launched in foreground, single-message batches; no validation mid-phase +- [ ] Files staged per-phase (no `git add -A`) +- [ ] MODERNIZATION-ISSUES.md contains only genuinely unfixable errors diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/references/SAPUI5_Local_Build.md b/plugins/ui5-modernization/skills/modernize-ui5-app/references/SAPUI5_Local_Build.md new file mode 100644 index 0000000..ff5bb72 --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/references/SAPUI5_Local_Build.md @@ -0,0 +1,209 @@ +# SAP UI5 Local Build & Test — Maven Parent-Pom Projects + +Reference for local development workflows on SAP UI5 / Fiori Maven projects that inherit from a shared parent pom. Covers three workflows and associated failure modes. + +## Applicability + +These workflows apply when the project's `pom.xml` inherits from a shared SAP UI5 parent pom and: + +- `webapp/test/testsuite.qunit.js` exists + +If `pom.xml` exists but does not inherit from the UI5 parent pom, these workflows do not apply. + +--- + +## §1. Local dev server (Tomcat, no tests) + +### §1.0 Preflight — patch pom BEFORE first boot + +Before booting the dev server, verify that the dev deploy profile in `pom.xml` is complete. A fresh checkout typically has an incomplete profile that fails at startup with: + +``` +SEVERE: Exception starting filter InstrumentationFilter +java.lang.ClassNotFoundException: +... +[ERROR] Failed to execute goal ... run-war + Could not start Tomcat: ... A child container failed during start +``` + +The `test-resources/` 404 case (§1.2) is the *follow-up* failure once the server does start — on a fresh checkout missing both pieces, the startup failure hits first. Patching upfront avoids the sequence. + +Detect with: + +```bash +grep -n "tomcat.dev.deploy" pom.xml # must exist +grep -nE "jscoverage|maven-war-plugin" pom.xml # both must hit inside the profile block +``` + +If the dev deploy profile exists but the instrumentation dependency and/or `maven-war-plugin` are missing from it, apply the patch in §1.2 before running the boot command. If the profile (or `` block) is entirely absent, see `pom-dev-profile-patch.md` for the full wrapper. + +Skip preflight only if the profile already contains both the instrumentation runtime dependency and the `maven-war-plugin` `webResources` block. + +### §1.1 Boot command + +```bash +mvn clean package tomcat7:run-war -Ptomcat.dev.deploy -DskipTests -Dmaven.tomcat.port=8090 +``` + +URLs after the server is up: + +- App: `http://localhost:8090//` +- Test bootstrap: `http://localhost:8090//test-resources/testFLPService.html` +- Test Starter: `http://localhost:8090//test-resources//Test.qunit.html?testsuite=test-resources//testsuite.qunit&test=` + +Replace `` with the value from `pom.xml` (e.g., `my.sample.app`) and `` with the slash-separated app namespace (e.g., `my/sample/app`). + +### §1.2 Failure: `test-resources/` 404s OR `ClassNotFoundException` at boot + +**Root cause:** The dev deploy profile in `pom.xml` is missing the `maven-war-plugin` `webResources` block and/or the instrumentation runtime dependency. The base `pom.xml` on `origin/main` typically does NOT include them — they are a local dev-only addition. + +Two distinct symptoms map to this single cause: + +- **Boot fails entirely** — `ClassNotFoundException` for the instrumentation filter followed by `A child container failed during start` and `BUILD FAILURE`. The parent pom wires an instrumentation filter into `web.xml` that needs a jar on the runtime classpath; without it Tomcat fails and the engine aborts. +- **Boot succeeds but `test-resources/*` returns 404** — server logs show no errors, app loads, but `testFLPService.html` and friends are absent. The `maven-war-plugin` `webResources` block is missing. + +A fresh checkout missing both hits the boot failure first; once the instrumentation dependency is added, the 404 surfaces. + +**Fix (single patch for both):** + +```diff + + tomcat.dev.deploy ++ ++ ++ ${ui5.groupId} ++ jscoverage ++ ${ui5.version} ++ runtime ++ ++ + + + + org.apache.tomcat.maven + tomcat7-maven-plugin + ++ ++ maven-war-plugin ++ ++ ++ ++ webapp/test ++ test-resources ++ ++ ++ ++ + + + +``` + +Why each piece matters: + +- **Instrumentation runtime dependency** — without it the deploy fails with `ClassNotFoundException` for the instrumentation filter wired in by the parent pom. +- **`maven-war-plugin` `webResources`** — copies `webapp/test/` into the WAR under `test-resources/`. That makes `testFLPService.html`, `testsuite.qunit.js`, and OPA `.qunit.js` files reachable from the browser. Production builds intentionally omit this; the dev profile re-enables it. + +The release/optimized build profile is unaffected — keep these additions scoped to the dev deploy profile. + +**Version selection:** The version of the instrumentation dependency should match the UI5 framework version used elsewhere in the project. Check other UI5 dependencies in `pom.xml` or use the version property (e.g., `${ui5.version}`) if the parent pom defines one. + +**Commit convention:** The conventional choice is to keep this patch local (uncommitted) and out of the published history. + +### §1.3 After code changes — rebuild + clear browser cache + +The Tomcat dev server serves resources from the WAR built at boot time. It does **not** hot-reload `webapp/` edits. After ANY change under `webapp/` (controllers, views, fragments, tests, i18n, manifest, etc.): + +1. Stop the running server (Ctrl+C, or `lsof -i :8090 -t | xargs kill` if detached). +2. Rerun the boot command from §1.1: + ```bash + mvn clean package tomcat7:run-war -Ptomcat.dev.deploy -DskipTests -Dmaven.tomcat.port=8090 + ``` +3. **Clear the browser cache before reloading.** The dev server emits long-lived `Cache-Control` / `ETag` headers, and Chrome returns the previous build's JS even after `mvn clean package`. Symptom: tests still fail (or pass) on old code despite a successful rebuild. + + Options: + - DevTools open + hard reload (`Cmd+Shift+R` / `Ctrl+Shift+R`), with "Disable cache" ticked in DevTools → Network tab. + - Open the URL in a fresh Incognito/Private window. + - Append a cache-buster query param (`?_=`) to the test URL. + +For rapid iteration on `webapp/` code, a standalone UI5 dev server (`ui5 serve`) hot-reloads and bypasses this cycle. Reserve the Tomcat server for verifying the production-shape WAR (servlet config, FLP integration, Selenium runs). + +--- + +## §2. OPA/QUnit tests with visible Chrome + +```bash +mvn clean verify -P execute.qunit -Dwebdriver.chrome.driver=$(which chromedriver) +``` + +Notes: + +- `execute.qunit` profile is inherited from the parent pom and wires Surefire to a QUnit test runner. +- `$(which chromedriver)` resolves to the chromedriver binary on PATH (`brew install --cask chromedriver` on macOS). +- chromedriver major version must match installed Chrome major version; otherwise: `session not created: This version of ChromeDriver only supports Chrome version N`. +- Reports: `target/surefire-reports/`. Failure screenshots: `target/screenshots/`. + +--- + +## §3. OPA/QUnit tests headless + +Same Selenium run as §2 but Chrome runs without a window. Requires a capabilities JSON and extra `-D` flags. + +### §3.1 Capabilities JSON location + +Place `headless-chrome.json` next to `pom.xml` at the project root. The parent pom forwards `-Dfiori.test.capabilities.file` as-is; absolute paths via `$(pwd)/headless-chrome.json` work regardless of directory. + +### §3.2 File contents + +```json +{ + "chrome": { + "goog:chromeOptions": { + "args": [ + "--headless=new", + "--disable-gpu", + "--no-sandbox", + "--disable-dev-shm-usage", + "--window-size=1920,1080" + ] + } + } +} +``` + +Do not deviate from the nesting — the capabilities parser is strict and silently breaks on common-looking alternatives. See `headless-capabilities.md` for the exact format rules. + +### §3.3 Command + +```bash +mvn clean verify -P execute.qunit \ + -Dwebdriver.chrome.driver=$(which chromedriver) \ + -Dfiori.test.browser=chrome \ + -Dfiori.test.capabilities.file=$(pwd)/headless-chrome.json +``` + +The two extra `-D` flags vs. §2: + +- `-Dfiori.test.browser=chrome` — selects the top-level browser key inside the JSON. +- `-Dfiori.test.capabilities.file=...` — absolute path to the JSON. Relative paths resolve against `${project.basedir}` but `$(pwd)/...` avoids surprises. + +### §3.4 Troubleshooting: `ExceptionInInitializerError` + +When the run dies with `java.lang.ExceptionInInitializerError` at the QUnit test runner init and the surefire report has no `Caused by:` line, the cause is almost always a malformed `headless-chrome.json`. The capabilities parser only catches IO errors — JSON parse and structural errors escape and surface as the bare `ExceptionInInitializerError`. + +Diagnostic order: + +1. Re-read the JSON — top-level keys must be browser names, NOT capability keys. A flat `{"browserName": "chrome", "chromeOptions": {...}}` throws `ClassCastException`. See `headless-capabilities.md` for the full format spec. +2. Check JSON syntax (valid JSON, no trailing commas). + +### §3.5 Troubleshooting: chromedriver version mismatch + +`session not created: This version of ChromeDriver only supports Chrome version N` — upgrade or downgrade chromedriver to match the installed Chrome major version. + +--- + +## Related references + +- `headless-capabilities.md` — full capabilities JSON format spec, parser quirks, extending for Firefox / proxy / custom prefs. +- `pom-dev-profile-patch.md` — extended rationale for dev profile additions and handling version drift. +- `headless-chrome.json` — canonical headless Chrome capabilities file (copy verbatim to project root). diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/references/build-and-test-commands.md b/plugins/ui5-modernization/skills/modernize-ui5-app/references/build-and-test-commands.md new file mode 100644 index 0000000..bf11982 --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/references/build-and-test-commands.md @@ -0,0 +1,169 @@ +# Build & Test Commands — Full Reference + +This reference contains the full details for detecting project type, setting up prerequisites, and running build/test commands at verification gates. + +--- + +## General Prerequisites + +### Chrome DevTools MCP + +The Chrome DevTools MCP server must be available for browser-based test verification. It is used to: +- Navigate to test runner URLs after the dev server starts +- Observe test results in the browser (pass/fail counts, error messages) +- Take screenshots of test failures for debugging + +If Chrome DevTools MCP is not connected, install it using the `/install-mcps` skill before proceeding. The agent needs browser access for verification gates. + +--- + +## Project Type Detection + +Check once at the start and reuse throughout all gates: + +1. **`pom.xml` exists** → Run §1.0 Applicability Check. If applicable, use §1 Maven commands. If not, continue to step 2. +2. **Read `package.json` scripts** (§2) — look for test scripts. If found, use those. Only if no test scripts exist at all, fall back to linter-only verification. + +--- + +## §1. Maven Projects (fnf-parent-pom) + +For full background on dev server boot, visible-Chrome tests, and headless tests, read `SAPUI5_Local_Build.md`. + +### §1.0 Applicability Check + +A `pom.xml` alone does not guarantee the Maven test workflow applies. The full criteria are documented in `SAPUI5_Local_Build.md` § Applicability. Before setting up prerequisites, verify the project has **at least one** of these markers: + +```bash +# 1. fnf-parent-pom in block — sufficient on its own (profiles are inherited) +grep -q "fnf-parent-pom" pom.xml && echo "MAVEN APPLICABLE: fnf-parent-pom found" || echo "No fnf-parent-pom" + +# 2. SAP UI5/Fiori/Shell groupId anywhere +grep -qE "com\.sap\.(ui5|fiori|ushell)" pom.xml && echo "MAVEN APPLICABLE: SAP UI5 groupId found" || echo "No SAP UI5 groupId" + +# 3. QUnit testsuite exists +test -f webapp/test/testsuite.qunit.js && echo "MAVEN APPLICABLE: testsuite.qunit.js found" || echo "No testsuite.qunit.js" +``` + +**Decision logic:** + +- **Any marker found → Maven project.** Proceed to §1.1. Note: `fnf-parent-pom` provides `tomcat.dev.deploy` and `execute.qunit` profiles via inheritance — you will NOT find them in the local `pom.xml`. The absence of local profiles does NOT mean the project isn't Maven-based. +- **No markers found → NOT a Maven project.** Fall through to §2 (npm-based). You MUST read `package.json` scripts before falling back to linter-only. + +For full background on dev server boot sequences, profile patches, and headless test setup, see `SAPUI5_Local_Build.md`. + +### §1.1 Prerequisites (before first test run) + +All three must be in place before running tests. Check once at the start of the workflow. + +#### headless-chrome.json + +Must exist at the project root. If missing, create it. + +**Important:** Never commit or push this file. It is a local-only test prerequisite. Add `headless-chrome.json` to `.gitignore` immediately after creating it. + +```json +{ + "chrome": { + "goog:chromeOptions": { + "args": [ + "--headless=new", + "--disable-gpu", + "--no-sandbox", + "--disable-dev-shm-usage", + "--window-size=1920,1080" + ] + } + } +} +``` + +#### pom.xml dev profile patch + +The `tomcat.dev.deploy` profile needs two additions for `test-resources/` to be reachable and the jscoverage filter to load. Check if the profile already has them; if not, apply the patch from `pom-dev-profile-patch.md`. + +**Important:** Never commit or push the dev profile patch to `pom.xml`. These additions are local-only test infrastructure. Exclude `pom.xml` from any commits (do not stage it). + +- Add runtime dependency `com.sap.ui5:jscoverage:` (match existing `com.sap.ui5:*` version in the project) +- Add `maven-war-plugin` `webResources` block copying `webapp/test/` into `test-resources/` + +Detect with: +```bash +grep -n "tomcat.dev.deploy" pom.xml +grep -nE "jscoverage|maven-war-plugin" pom.xml +``` + +If either is missing, see `SAPUI5_Local_Build.md` §1.0 and §1.2 for the full patch and variants. + +#### .gitignore + +Add the following entries if not already present: +- `target/` — Maven builds generate output here which must not be committed +- `headless-chrome.json` — local-only test configuration, never commit or push + +**Important:** Never commit or push the `.gitignore` changes themselves. Keep them locally — they are part of the local test infrastructure alongside the other prerequisites. Do not stage `.gitignore`. + +**Before committing**, also ensure the dev profile patch changes to `pom.xml` (jscoverage dependency and maven-war-plugin webResources block) are not staged. These are local-only test infrastructure and must not be pushed. Do not revert them — they are needed for local testing. + +### §1.2 Build command (interactive dev server) + +**This is for interactive browser-based verification only.** It starts a Tomcat dev server that blocks the terminal. Do NOT use this as a "build step" before headless tests — §1.3 is self-contained and handles compilation, packaging, and test execution in one command. + +```bash +mvn clean package tomcat7:run-war -Ptomcat.dev.deploy -DskipTests -Dmaven.tomcat.port=8090 +``` + +**Timeout:** Use a 600000ms (10 minute) timeout when running this command. + +### §1.3 Test command (headless — use this for verification gates) + +This command is **self-contained**: it compiles, packages, and runs all QUnit tests headlessly in one invocation. It does NOT require §1.2 to be running. **For verification gate sub-agents, this is the only command you need to run.** + +```bash +mvn clean verify -P execute.qunit \ + -Dwebdriver.chrome.driver=$(which chromedriver) \ + -Dfiori.test.browser=chrome \ + -Dfiori.test.capabilities.file=$(pwd)/headless-chrome.json +``` + +**Timeout:** Use a 600000ms (10 minute) timeout when running this command. + +### §1.4 Troubleshooting + +| Symptom | Cause | Fix | +|---------|-------|-----| +| `ClassNotFoundException: com.sap.ui5.jscoverage.InstrumentationFilter` | Missing jscoverage runtime dep | Apply profile patch (§1.1) | +| `test-resources/` returns 404 | Missing `maven-war-plugin` `webResources` block | Apply profile patch (§1.1) | +| `ExceptionInInitializerError at QUnitTest.:218` | Malformed `headless-chrome.json` | See `SAPUI5_Local_Build.md` §3.4 and `headless-capabilities.md` | +| `session not created: ChromeDriver only supports Chrome version N` | chromedriver/Chrome major version mismatch | Upgrade or downgrade chromedriver | + +--- + +## §2. npm-based Projects + +**You MUST read `package.json`** to determine the actual test commands. Do not assume command names, do not skip this step, and do not fall back to linter-only without first confirming that no test scripts exist: + +```bash +# Read the scripts block — this is mandatory before deciding on verification strategy +cat package.json | grep -A 30 '"scripts"' +``` + +Common patterns: + +```bash +# Build (look for "build" script) +npm run build + +# Test (look for "test", "test:unit", "test:opa", "test:integration", "test-runner") +npm test +``` + +Use whatever script names are defined. Look for: +- `"test"` or `"test:unit"` — unit test runner +- `"test:integration"` or `"test:opa"` — OPA5 integration tests +- `"test-runner"` — ui5-test-runner based setup (common with Test Starter) +- `"start"` — dev server (often `ui5 serve`) + +If `package.json` has no test script, fall back to running the linter only (`npx @ui5/linter --details`) as the verification step. + +**Timeout:** Use a 600000ms (10 minute) timeout when running build and test commands. diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/references/documentation-templates.md b/plugins/ui5-modernization/skills/modernize-ui5-app/references/documentation-templates.md new file mode 100644 index 0000000..f64f106 --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/references/documentation-templates.md @@ -0,0 +1,67 @@ +# Documentation Phase Templates + +After Phase 5 completes, create these two files and commit them. + +--- + +## MODERNIZATION-ISSUES.md + +Use this template for unfixable errors: + +```markdown +# UI5 Modernization - Unfixable Issues + +This document lists issues that could not be automatically fixed. + +Generated: [DATE] + +## Summary +- Total unfixable issues: [COUNT] + +## Issues by File + +### [file-path] + +#### Issue 1 +- **Rule**: [rule-id] +- **Line**: [line-number] +- **Error**: [error-message] +- **Attempted Fix**: [what was tried] +- **Reason Not Fixed**: [specific technical reason] +- **Suggested Manual Action**: [what the developer should do] +``` + +--- + +## MODERNIZATION-REPORT.md + +Use this template for the final summary: + +```markdown +# UI5 Modernization Summary + +Model: [MODEL-NAME] +Generated: [DATE] +Verification mode: [full autonomous / half autonomous / manual] + +## Statistics + +| Phase | Errors | Warnings | Total | +|---|---|---|---| +| Baseline | [X] | [Y] | [Z] | +| After Phase 1 (autofix + test starter) | [X] | [Y] | [Z] | +| After Phase 2 (foundation) | [X] | [Y] | [Z] | +| After Phase 3 (module system) | [X] | [Y] | [Z] | +| After Phase 4 (deprecated APIs) | [X] | [Y] | [Z] | +| After Phase 5 (CSP) | [X] | [Y] | [Z] | + +## Improvement +- **Resolved**: [N] issues ([P]%) +- **Remaining**: [N] (see MODERNIZATION-ISSUES.md) + +## Verification Results +[Per-phase build/test pass/fail summary, if gates were run] + +## Next Steps +[Recommended manual actions] +``` diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/references/headless-capabilities.md b/plugins/ui5-modernization/skills/modernize-ui5-app/references/headless-capabilities.md new file mode 100644 index 0000000..87b65eb --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/references/headless-capabilities.md @@ -0,0 +1,138 @@ +# Headless Capabilities JSON — Format Spec & Parser Internals + +This is the deep reference for `headless-chrome.json` and any sibling capabilities files (`headless-firefox.json`, `proxied-chrome.json`, etc.). Read it only when the inline workflow in SKILL.md isn't enough — typically because you need a non-default browser, custom prefs, a proxy, or you're debugging a parse failure that survived the SKILL.md troubleshooting. + +## The contract + +The capabilities file is consumed by `com.sap.ui5.selenium.qunit.JSONCapabilities.parseJSON(String json)` from the `qunit-utils` jar. The parser is hand-rolled with Gson and is strict in ways that don't match the W3C WebDriver spec. Treat it as a small DSL, not as generic Selenium JSON. + +## Top-level structure + +```json +{ + "": { ...capabilities... }, + "": { ...capabilities... } +} +``` + +Top-level keys are browser names, NOT capability keys. Valid keys: `chrome`, `firefox`, `safari`, `edge`, `ie`, `opera`, `phantomjs`, `htmlunit`. Only the entry whose key matches `-Dfiori.test.browser=` is consumed; others are ignored. + +A flat structure like `{"browserName": "chrome", "chromeOptions": {...}}` will throw `ClassCastException` from `JsonObject.getAsJsonObject(String)` because the parser tries to descend into `browserName` as if it were a browser object. The exception escapes `QUnitCapabilities`'s catch block (which only catches `IOException` / `FileNotFoundException`) and surfaces as `ExceptionInInitializerError at QUnitTest.:218` with no `Caused by:` line in the surefire report. + +## Capability keys inside a browser block + +The parser recognizes a few well-known keys and treats everything else as raw `DesiredCapabilities` set-via-string. + +### `goog:chromeOptions` (chrome only) + +Only honored when the surrounding browser key is `chrome`. Maps to a `org.openqa.selenium.chrome.ChromeOptions` instance. + +Supported sub-keys: + +- `args`: JSON array of strings. Each entry is appended via `ChromeOptions.addArguments(String...)`. + +The legacy alias `chromeOptions` is normalized to `goog:chromeOptions` by `JSONCapabilities.normalizeCapabilityKey` — both work, but the W3C form is preferred. No other Chrome-specific keys (binary path, extensions, prefs, mobileEmulation) are wired up; they would require a different code path in the parser. + +### `moz:firefoxOptions` (firefox only) + +Only honored when the surrounding browser key is `firefox`. Maps to `org.openqa.selenium.firefox.FirefoxOptions`. + +Supported sub-keys: + +- `prefs`: JSON array of objects, each a `{key: value}` map. The parser detects whether the value is `"true"` / `"false"` (boolean preference, set via `addPreference(String, boolean)`) or anything else (string preference). Numeric prefs go through the string overload — typically harmless, but Firefox occasionally rejects strings where it wants integers. + +### `proxy` + +Available for any browser. Maps to `org.openqa.selenium.Proxy`. Sub-keys are passed straight to `new Proxy(Map)`, so use the raw Selenium proxy keys: `httpProxy`, `sslProxy`, `socksProxy`, `noProxy`, `proxyType`, etc. + +### `custom_data` + +If the value is a JSON-encoded object as a string, it's parsed back out and the inner keys are merged in via `normalizeCapabilityKey`. Used to embed nested capability blobs that shouldn't be processed by `parseJSON`'s structural rules. + +### Anything else + +Other keys land in `DesiredCapabilities` as strings (or as parsed primitives via the `^\[.*\]$` array detection — values matching that regex are split on `,` and stored as `String[]`). + +## Examples + +### Headless Chrome (the canonical case — same as `assets/headless-chrome.json`) + +```json +{ + "chrome": { + "goog:chromeOptions": { + "args": [ + "--headless=new", + "--disable-gpu", + "--no-sandbox", + "--disable-dev-shm-usage", + "--window-size=1920,1080" + ] + } + } +} +``` + +### Headless Firefox + +```json +{ + "firefox": { + "moz:firefoxOptions": { + "prefs": [ + {"general.useragent.override": "Mozilla/5.0 (...)"}, + {"dom.webdriver.enabled": "false"} + ] + } + } +} +``` + +For headless Firefox you typically also need `args: ["-headless"]` — the parser doesn't currently honor an `args` key under `moz:firefoxOptions`, so add the flag via `MOZ_HEADLESS=1` env var or via `webdriver.firefox.binary.args` system property. If headless Firefox is critical, double-check the parser version in your local `qunit-utils-.jar` — newer versions may add `args` support. + +### Chrome behind a proxy + +```json +{ + "chrome": { + "goog:chromeOptions": { + "args": ["--headless=new", "--window-size=1920,1080"] + }, + "proxy": { + "httpProxy": "proxy.corp.example:8080", + "sslProxy": "proxy.corp.example:8080", + "noProxy": "localhost,127.0.0.1" + } + } +} +``` + +## Parser quirks worth knowing + +- Top-level browser keys are case-sensitive (`Chrome` won't match). +- `args` entries with `=` (e.g., `--window-size=1920,1080`) are passed through verbatim — Chrome accepts them. +- `goog:chromeOptions` is only consumed under `chrome`. Putting it under `firefox` (or anywhere else) silently no-ops — it ends up as a plain capability map with no effect. +- Values matching `^\[.*\]$` (a string starting with `[` and ending with `]`) get split on `,`. Avoid putting JSON-array-shaped strings in capability values — wrap as a real JSON array if you mean an array. +- The parser does NOT support the W3C `alwaysMatch` / `firstMatch` envelope. Don't wrap in `capabilities: { alwaysMatch: {...} }` — the top level must be browser names directly. + +## Diagnostic recipe + +When the surefire report shows `ExceptionInInitializerError at QUnitTest.:218` with no `Caused by:`, write a small Java repro to surface the real exception. The static-init catch only handles a couple of `Exception` subclasses; everything else propagates as the bare `ExceptionInInitializerError`. + +Repro template (adapt the path): + +```java +public class QCTest { + public static void main(String[] a) throws Exception { + // (long pageLoadTimeout, long scriptTimeout, String customCapabilitiesJsonPath, String customCapabilitiesJsonString, String driverSessionName) + new com.sap.ui5.selenium.qunit.QUnitCapabilities( + 60000L, 60000L, + "/abs/path/to/headless-chrome.json", + null, + "QUnit Test Runner"); + System.out.println("ok"); + } +} +``` + +Compile and run against the surefire test classpath (read it from the surefire report's `` block). The thrown exception in this small program is the same one that's being silently swallowed inside ``. diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/references/headless-chrome.json b/plugins/ui5-modernization/skills/modernize-ui5-app/references/headless-chrome.json new file mode 100644 index 0000000..aee8c51 --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/references/headless-chrome.json @@ -0,0 +1,13 @@ +{ + "chrome": { + "goog:chromeOptions": { + "args": [ + "--headless=new", + "--disable-gpu", + "--no-sandbox", + "--disable-dev-shm-usage", + "--window-size=1920,1080" + ] + } + } +} diff --git a/plugins/ui5-modernization/skills/modernize-ui5-app/references/pom-dev-profile-patch.md b/plugins/ui5-modernization/skills/modernize-ui5-app/references/pom-dev-profile-patch.md new file mode 100644 index 0000000..ab7001e --- /dev/null +++ b/plugins/ui5-modernization/skills/modernize-ui5-app/references/pom-dev-profile-patch.md @@ -0,0 +1,158 @@ +# Dev Deploy Profile — Patch Rationale & Variants + +This is the deep reference for the `pom.xml` change in §1.1 of SKILL.md. Read it when the simple diff doesn't apply cleanly — the surrounding repo has a non-standard dev deploy block, the parent pom version differs, or the user wants to understand why this change isn't already on `origin/main`. + +## What the patch does + +Two additions, both inside the dev deploy profile in `pom.xml`: + +1. A runtime `dependency` on the UI5 instrumentation library (jscoverage). +2. A `maven-war-plugin` `webResources` block that copies `webapp/test/` into the WAR under `test-resources/`. + +Both are dev-only. The optimized/release build profile must NOT receive them — production WARs intentionally exclude `webapp/test/` and don't load the instrumentation filter. + +## Why neither is on `origin/main` + +The parent pom wires an instrumentation filter into the deployed WAR's runtime classpath. The filter dependency is intentionally not declared as a default dependency — release builds skip it to avoid bundling instrumentation code into customer-facing artifacts. The dev profile is the right place to opt into it locally. + +The `webResources` block is similarly absent because release WARs must not contain the test bootstrap (`testFLPService.html`), the OPA `.qunit.js` files, or the testsuite definition. Including them in `origin/main`'s dev profile would mean every developer's local build silently differs from CI, but since this is a dev-only profile it's safer to make it explicit per checkout. + +The trade-off: every developer who wants test-resources reachable locally has to apply the patch. Some teams accept this; others ship a `pom.xml` with the dev profile patched and rely on `[skip ci]` commits or a sibling `pom-dev.xml` to keep CI green. + +## Picking the instrumentation dependency version + +The version should match the major UI5 framework version in use elsewhere in the project. To find it: + +1. Look for any other UI5 dependency in `pom.xml` or the parent — match its version. +2. If the parent pom uses property-driven versions (e.g., `${ui5.version}`), reuse the same property. +3. Run `mvn dependency:tree | grep ` to see what the project already resolves. + +A mismatched version usually still works at runtime (the API is stable), but a matched version avoids occasional class-version conflicts on startup. + +### Fallback when the SNAPSHOT version isn't resolvable + +If the internal Maven repository is unreachable or the desired SNAPSHOT was never published locally, Maven fails to resolve the dependency and the dev server won't start. Use the last released version that exists in your local Maven cache instead: + +```bash +ls ~/.m2/repository//jscoverage/ +``` + +Pick the highest non-SNAPSHOT entry and pin it in the profile dependency. Released versions are stable across minor releases — runtime behavior won't differ for the instrumentation filter. Treat the SNAPSHOT version as the "ideal" and the released version as the working fallback. + +## Variants + +### Repo already has SOME `webResources` in the dev profile + +Merge into the existing list rather than replacing: + +```xml + + + some/existing + existing-target + + + webapp/test + test-resources + + +``` + +### Repo uses a non-default test directory + +If `webapp/test/` isn't where the project keeps its test sources (e.g., `src/test/webapp` or `webapp/qunit-tests`), update `` to match. Find the right path by locating `testsuite.qunit.js`. + +### Repo's parent pom version differs + +The patch is independent of the parent pom version — it only depends on the child's `pom.xml` having a dev deploy profile to extend. If the profile doesn't exist at all, add the whole block: + +```xml + + tomcat.dev.deploy + + + ${ui5.groupId} + jscoverage + ${ui5.version} + runtime + + + + + + org.apache.tomcat.maven + tomcat7-maven-plugin + + + maven-war-plugin + + + + webapp/test + test-resources + + + + + + + +``` + +### Repo has no `` block at all + +Some lean child poms inherit everything from the parent pom and never declare a `` element themselves. Adding the profile alone won't be enough — Maven needs the wrapping `` element. Insert the full wrapper after the closing `` (or after `` if no `` exists): + +```xml + + + + tomcat.dev.deploy + + + ${ui5.groupId} + jscoverage + ${ui5.version} + runtime + + + + + + org.apache.tomcat.maven + tomcat7-maven-plugin + + + maven-war-plugin + + + + webapp/test + test-resources + + + + + + + + + +``` + +Detect this case: `grep -n "\|\|tomcat.dev.deploy" pom.xml` returns nothing. The `mvn ... -Ptomcat.dev.deploy` invocation succeeds anyway (Maven silently ignores unknown profiles when launched from CLI), but startup fails with `ClassNotFoundException` because the parent pom wires the instrumentation filter without the dependency being on the runtime classpath. + +### Repo uses a different Tomcat plugin version + +Keep the rest of the patch as-is. The `webResources` and instrumentation dependency additions are independent of which Tomcat plugin runs the WAR — the change is to the WAR packaging, not the runner. + +## Keeping the change local + +The conventional pattern is to keep this patch out of published commits. Mechanics: + +1. After applying, run `git status pom.xml` — should show `M pom.xml`. +2. Don't `git add pom.xml` when committing other work. +3. If the change accidentally landed in a commit, amend it out: save the working-tree pom, restore the parent's version, amend the commit, restore. +4. Use `git update-index --skip-worktree pom.xml` if you want git to stop showing the modification (caveat: rebases sometimes need this turned back off). + +If the team prefers committing the patch instead, that's also fine — the shape of the change is identical either way. diff --git a/release-please-config.json b/release-please-config.json index ae3c13d..ec9c5b3 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -83,6 +83,16 @@ "type": "json", "path": "plugins/ui5-typescript-conversion/.github/plugin/plugin.json", "jsonpath": "$..version" + }, + { + "type": "json", + "path": "plugins/ui5-modernization/plugin.json", + "jsonpath": "$..version" + }, + { + "type": "json", + "path": "plugins/ui5-modernization/.github/plugin/plugin.json", + "jsonpath": "$..version" } ] }